feat: Add standalone McpAutomationBridge mouse and keyboard control, simulated cursor, and tool catalog updates#321
Open
punal100 wants to merge 6 commits intoChiR24:mainfrom
Open
feat: Add standalone McpAutomationBridge mouse and keyboard control, simulated cursor, and tool catalog updates#321punal100 wants to merge 6 commits intoChiR24:mainfrom
punal100 wants to merge 6 commits intoChiR24:mainfrom
Conversation
…trol, simulated cursor, and tool catalog updates Summary: 1. Added newer standalone McpAutomationBridge UI automation and control features to support targeted mouse and keyboard input inside editor windows and tabs. 2. Added a simulated visual mouse cursor so automated mouse movement, clicks, wheel input, and drag flows have visible on-screen targeting feedback. 3. Added visible-window discovery, richer UI target discovery, and stronger screenshot targeting diagnostics for deterministic editor automation. 4. Added a plugin-owned public tool catalog so manage_pipeline metadata and published bridge capabilities are sourced from one place. 5. Updated standalone documentation and repo hygiene to describe the new bridge surface and ignore generated Saved output. 6. Total: 13 files changed, +6480 / -2382 lines. Features: 1. Mouse and Keyboard Controls: Added targeted control_editor simulated input support for keyboard, text input, mouse movement, button presses, wheel scrolling, and drag interactions. 2. Simulated Visual Mouse Cursor: Added a virtual cursor overlay that shows the active automated pointer position during targeted input flows. 3. Window and Tab Targeting: Added manage_ui visible-window listing and explicit tab or window targeting so automation does not depend on the active viewport. 4. UI Discovery and Command Wiring: Added discovery for JSON-defined UI targets, registered editor commands, ToolMenus integration, and Editor Utility widget open and close flows. 5. Catalog-Backed Pipeline Metadata: Added McpAutomationBridgeToolCatalog so manage_pipeline list_categories and get_status return plugin-owned tool and action metadata. 6. Better Screenshot Diagnostics: Added stronger screenshot target resolution and reporting for editor-window capture workflows. 7. UI Discovery Configuration: Added UiDefinitionRoots, KnownToolMenuNames, and JsonToolTabIdPrefix settings for standalone UI discovery control. Changes: 1. Added plugins/McpAutomationBridge/Source/McpAutomationBridge/Public/McpAutomationBridgeToolCatalog.h, Specifically introduced public tool catalog structs and helper APIs for tool lookup and action counts. 2. Added plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeToolCatalog.cpp, Specifically defined the standalone bridge catalog entries, summaries, subactions, and public count helpers. 3. Modified plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_ControlHandlers.cpp, Specifically added targeted mouse and keyboard input routing, simulated cursor overlay support, explicit window and tab targeting, and improved screenshot target helpers. 4. Modified plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp, Specifically added visible-window listing, richer UI target discovery, editor command registration, ToolMenus entry support, tab closing, and Editor Utility widget flows. 5. Modified plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp, Specifically replaced duplicated hardcoded category reporting with catalog-backed list_categories and get_status responses. 6. Modified plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeSubsystem.cpp, Specifically imported the tool catalog and added registration drift checks so the public bridge surface stays aligned with the catalog. 7. Modified plugins/McpAutomationBridge/Source/McpAutomationBridge/Public/McpAutomationBridgeSubsystem.h, Specifically added subsystem declarations and public state needed by the simulated-input and cursor flow. 8. Modified plugins/McpAutomationBridge/Source/McpAutomationBridge/Public/McpAutomationBridgeSettings.h and plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeSettings.cpp, Specifically added standalone UI discovery settings and defaults for JSON UI roots, ToolMenus names, and generated tab id prefix. 9. Modified plugins/McpAutomationBridge/Source/McpAutomationBridge/McpAutomationBridge.Build.cs, Specifically added the Blutility dependency and retained required standalone build support for the new UI discovery flow. 10. Modified plugins/McpAutomationBridge/README.md, Specifically documented visible-window discovery, targeted simulated input, catalog-backed pipeline metadata, and updated troubleshooting guidance. 11. Modified README.md, Specifically updated the standalone root documentation to call out the catalog-backed bridge surface, visible-window discovery, and window-aware screenshot and input capabilities. 12. Modified .gitignore, Specifically added Saved/ so generated standalone plugin output is excluded from source control. Bug Fixes: 1. Fixed manage_pipeline metadata drift by sourcing public tool, category, and action information from a single plugin-owned catalog instead of duplicated hardcoded lists. 2. Fixed editor input automation ambiguity by allowing explicit tab and window targeting instead of relying on whichever editor surface happens to be active. 3. Fixed screenshot targeting ambiguity by returning stronger target information and exposing visible Slate window discovery. 4. Fixed standalone repo hygiene by ignoring generated Saved output. Tests: 1. Standalone plugin package build: PASSED. 2. Runtime standalone bridge validation: PASSED. 3. Visible-window discovery validation: PASSED. 4. UI target discovery validation: PASSED. 5. Mouse movement, click, wheel, and drag input validation with simulated cursor overlay: PASSED. 6. Keyboard and text input validation: PASSED. 7. Screenshot target capture validation: PASSED. 8. manage_pipeline catalog response validation: PASSED. Task: Standalone McpAutomationBridge input, cursor, and catalog update Signed-off-by: Punal Manalan <punal100@gmail.com>
Contributor
|
👋 Thanks for your first Pull Request! We love contributions. Please ensure you have signed off your commits and followed the contribution guidelines. |
Contributor
📏 Large PR DetectedThis pull request is quite large (1000+ lines changed), which can make reviewing challenging. Suggestions:
This helps reviewers provide better feedback and speeds up the merge process. Thank you! 🙏 |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a static in-memory MCP tool catalog and public catalog APIs; uses the catalog to produce pipeline responses; implements extensive editor UI discovery and control handlers with new UI Discovery settings; registers additional automation handlers; updates build deps and docs; adds Changes
Sequence DiagramsequenceDiagram
participant Client as MCP Client
participant Subsystem as McpAutomationBridgeSubsystem
participant Catalog as ToolCatalog
participant UIHandler as UI Handler
participant Slate as Slate (Editor)
participant EditorCmd as Editor Command System
Client->>Subsystem: manage_ui list_visible_windows
Subsystem->>UIHandler: HandleUiAction(manage_ui:list_visible_windows)
UIHandler->>Slate: Query visible windows
Slate-->>UIHandler: Visible window metadata
UIHandler-->>Subsystem: JSON response
Subsystem-->>Client: Return visible windows
Client->>Subsystem: manage_pipeline list_categories
Subsystem->>Catalog: GetPublicMcpAutomationBridgeToolCatalog()
Catalog-->>Subsystem: public catalog
Subsystem->>Subsystem: BuildCatalogResponse (tools, categories, counts)
Subsystem-->>Client: Return catalog-backed categories
Client->>Subsystem: manage_ui open_ui_target
Subsystem->>UIHandler: resolve target (tabId/command/toolmenu)
UIHandler->>EditorCmd: execute command or open tab
EditorCmd->>Slate: open/focus tab
Slate-->>UIHandler: tabId/confirmation
UIHandler-->>Client: Return tabId/result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…andler Summary: 1. Restored project-relative path validation for the legacy UI screenshot handler in the standalone McpAutomationBridge plugin. 2. Reintroduced filename sanitization so screenshot writes cannot use traversal segments or embedded path separators. 3. Reapplied the project-directory boundary check before saving PNG output to disk. 4. Fixed a security regression that allowed arbitrary filesystem writes through unvalidated screenshot path and filename payload fields. 5. Total: 1 file changed, +49 / -2 lines. Features: 1. Screenshot Path Hardening: The UI screenshot handler now sanitizes the user-supplied `path` field with `SanitizeProjectFilePath()` before any file write occurs. 2. Project Boundary Enforcement: Screenshot output paths are converted to absolute project-rooted paths and rejected if they escape the project directory. 3. Filename Sanitization: The `filename` field is cleaned with `FPaths::GetCleanFilename()` and suspicious traversal input falls back to a safe generated filename. Changes: 1. Modified `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp`, Specifically restored `path` sanitization using `SanitizeProjectFilePath()`, re-added project-root boundary validation, and reintroduced safe filename cleaning for the `screenshot` subaction. Bug Fixes: 1. Fixed arbitrary file write risk in the legacy UI screenshot handler by blocking traversal and absolute-path style payload input. 2. Fixed missing filename sanitization that allowed path separator and traversal content in the screenshot output filename. 3. Fixed security drift from the plugin's documented path-handling convention by reusing the existing file path sanitization helper. Tests: 1. VS Code error check on modified file: PASSED (no errors found). 2. Diff verification of the screenshot handler patch: PASSED. 3. Manual scope verification with `git status --short` and `git diff --stat`: PASSED. Task: Restore screenshot sanitization Signed-off-by: Punal Manalan <punal100@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp (3)
697-700: Complex ternary with comma operator reduces readability.The nested comma operators inside the ternary expression make this hard to read and maintain.
♻️ Proposed refactor for clarity
case EMcpEditorCommandKind::ConsoleCommand: default: { UWorld *World = GEditor ? GEditor->GetEditorWorldContext().World() : nullptr; - return GEditor && GEditor->Exec(World, *Definition.Command) - ? (OutMessage = FString::Printf(TEXT("Executed command %s"), *Definition.Command), true) - : (OutMessage = FString::Printf(TEXT("Failed to execute command %s"), *Definition.Command), false); + if (GEditor && GEditor->Exec(World, *Definition.Command)) + { + OutMessage = FString::Printf(TEXT("Executed command %s"), *Definition.Command); + return true; + } + OutMessage = FString::Printf(TEXT("Failed to execute command %s"), *Definition.Command); + return false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp` around lines 697 - 700, The current return uses a complex ternary with comma operators around GEditor->Exec(World, *Definition.Command) which reduces readability; replace this expression in McpAutomationBridge_UiHandlers.cpp by using a clear if/else: compute UWorld* World the same way, call GEditor->Exec(World, *Definition.Command) into a bool result, set OutMessage with FString::Printf(TEXT("Executed command %s") or TEXT("Failed to execute command %s"), *Definition.Command) based on that result, and then return the bool; reference the existing variables and call (World, GEditor, Definition.Command, OutMessage) and remove the nested comma/ternary expression.
176-201: Settings are cached on first access; runtime changes won't apply.The lazy initialization caches discovery settings permanently. If users modify
UMcpAutomationBridgeSettingsat runtime, changes won't take effect until the next editor session. Consider documenting this behavior or adding a mechanism to refresh settings if needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp` around lines 176 - 201, GetUiDiscoverySettings currently caches UMcpAutomationBridgeSettings on first call using static bInitialized and Settings, so runtime edits to UMcpAutomationBridgeSettings are ignored; add a refresh mechanism or make caching optional by (a) introducing a new function RefreshUiDiscoverySettings() that resets the static bInitialized to false (and clears Settings) so subsequent calls to GetUiDiscoverySettings re-read GetDefault<UMcpAutomationBridgeSettings>(), or (b) add an optional bool parameter to GetUiDiscoverySettings(bool bForceReload = false) that, when true, clears bInitialized before loading; reference GetUiDiscoverySettings, bInitialized, Settings and UMcpAutomationBridgeSettings when implementing the change and update call sites accordingly (or wire RefreshUiDiscoverySettings to settings-change notifications if preferred).
203-216: Path traversal validation missing.This function resolves paths but does not sanitize against directory traversal attacks (e.g.,
../../sensitive). While the input currently comes from project settings (trusted), the coding guidelines require path sanitization for all file paths.As per coding guidelines: "Use
SanitizeProjectRelativePath()to validate and sanitize all file paths to prevent directory traversal attacks."🛡️ Proposed fix to add path containment check
FString ResolveProjectScopedPath(const FString &RelativePath) { const FString Normalized = RelativePath.TrimStartAndEnd(); if (Normalized.IsEmpty()) { return FPaths::ProjectDir(); } - if (FPaths::IsRelative(Normalized)) - { - return FPaths::ConvertRelativePathToFull( - FPaths::Combine(FPaths::ProjectDir(), Normalized)); - } - return FPaths::ConvertRelativePathToFull(Normalized); + + FString FullPath; + if (FPaths::IsRelative(Normalized)) + { + FullPath = FPaths::ConvertRelativePathToFull( + FPaths::Combine(FPaths::ProjectDir(), Normalized)); + } + else + { + FullPath = FPaths::ConvertRelativePathToFull(Normalized); + } + + // Verify resolved path is within project directory + const FString ProjectDir = FPaths::ConvertRelativePathToFull(FPaths::ProjectDir()); + if (!FullPath.StartsWith(ProjectDir)) + { + UE_LOG(LogTemp, Warning, TEXT("Path traversal attempt blocked: %s"), *RelativePath); + return FPaths::ProjectDir(); + } + return FullPath; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp` around lines 203 - 216, ResolveProjectScopedPath currently returns paths without sanitizing against traversal; call SanitizeProjectRelativePath on the trimmed RelativePath (or the computed project-relative combined path) and reject or normalize any path that fails validation so traversal like ../../ is prevented. Specifically, in ResolveProjectScopedPath use SanitizeProjectRelativePath to validate the Normalized input (or the result of FPaths::Combine(FPaths::ProjectDir(), Normalized)), and if sanitization fails return FPaths::ProjectDir() (or otherwise clamp to the project root); then return the full absolute path using FPaths::ConvertRelativePathToFull on the sanitized/contained path. Ensure you reference ResolveProjectScopedPath, SanitizeProjectRelativePath, FPaths::ProjectDir, FPaths::Combine, and FPaths::ConvertRelativePathToFull when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/McpAutomationBridge/README.md`:
- Around line 29-42: The README's PR Merge Highlights claims "36 MCP tools" but
the catalog in McpAutomationBridgeToolCatalog.cpp currently defines 49 public
MakeTool(...) entries, so update the README to reflect the actual catalog count
(or adjust the catalog to match the documented count); locate the tool
definitions in McpAutomationBridgeToolCatalog.cpp (search for MakeTool(...)
calls) and either change the README number to 49 or reconcile the
catalog/toolset so both sources agree.
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp`:
- Around line 188-203: BuildCatalogResponse is currently populating the
Categories/Tools arrays with tool names, which breaks the public response
contract (categories and toolCategories should represent categories, not tool
IDs/counts). Update BuildCatalogResponse (or immediately post-process its
outputs before calling Result->SetArrayField) so that the TArray passed to
Result->SetArrayField(TEXT("categories")) contains actual category objects/IDs,
the array passed to Result->SetArrayField(TEXT("tools")) contains tool
objects/metadata (not jammed into the categories array), and any field or
endpoint that exposes "toolCategories" or a category count (e.g.,
get_status.toolCategories) returns the number of categories or the per-tool
category lists as intended; ensure the numeric counts set via
Result->SetNumberField("count"/"groupCount"/"actionCount") still reflect
category count, category group count, and action count respectively. Make the
same fixes for the other occurrence mentioned (lines 239-246) so response shape
remains backward-compatible.
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp`:
- Around line 1276-1277: GEditor is dereferenced when retrieving
UEditorUtilitySubsystem (the UtilitySubsystem assignment) without a null check;
add a guard like "if (!GEditor) { /* handle: log/warn and return or skip */ }"
before calling GEditor->GetEditorSubsystem to avoid crashes during shutdown or
edge cases. Update the function in McpAutomationBridge_UiHandlers (where
UtilitySubsystem is retrieved) to early-return or safely skip downstream logic
when GEditor is null, and optionally log a warning via UE_LOG or ensureMsgf for
diagnostics.
- Around line 1937-1942: The ScreenshotPath is taken from the payload without
sanitization, allowing directory traversal; update the handler in
McpAutomationBridge_UiHandlers.cpp (around the code that calls
Payload->TryGetStringField and sets ScreenshotPath) to call
FPaths::SanitizeProjectRelativePath (or SanitizeProjectRelativePath()) on the
incoming ScreenshotPath, resolve it against FPaths::ProjectSavedDir() /
"Screenshots/WindowsEditor", and ensure the resulting path is contained inside
the project's Saved directory (reject or reset to the default if not).
Specifically: sanitize the raw payload path, construct an absolute/resolved
path, validate containment under FPaths::ProjectSavedDir(), and log and fallback
to the existing default ScreenshotPath if validation fails.
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeToolCatalog.cpp`:
- Around line 32-81: The catalog exposes public tools that lack handler
registration; add corresponding handler declarations in
McpAutomationBridgeSubsystem.h and RegisterHandler(TEXT("...")) calls in
InitializeHandlers() inside McpAutomationBridgeSubsystem.cpp for each advertised
tool (control_editor, manage_material_graph, manage_niagara_authoring,
manage_niagara_graph, manage_tests, manage_logs, manage_debug, manage_insights
and any sub-actions like list_categories/get_status if they need dispatch), or
remove/mark those entries non-public in the Catalog; ensure the handler function
names match the declared methods so dispatch from the catalog entries resolves
correctly.
In `@README.md`:
- Around line 93-96: The fenced code block showing the copy instructions lacks a
language tag and triggers markdownlint MD040; update the triple-backtick fence
that surrounds the lines "Copy: Unreal_mcp/plugins/McpAutomationBridge/" and
"To: YourUnrealProject/Plugins/McpAutomationBridge/" to include the language
tag `text` (i.e., change ``` to ```text) so the block is marked as plain text
and the lint error is resolved.
---
Nitpick comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp`:
- Around line 697-700: The current return uses a complex ternary with comma
operators around GEditor->Exec(World, *Definition.Command) which reduces
readability; replace this expression in McpAutomationBridge_UiHandlers.cpp by
using a clear if/else: compute UWorld* World the same way, call
GEditor->Exec(World, *Definition.Command) into a bool result, set OutMessage
with FString::Printf(TEXT("Executed command %s") or TEXT("Failed to execute
command %s"), *Definition.Command) based on that result, and then return the
bool; reference the existing variables and call (World, GEditor,
Definition.Command, OutMessage) and remove the nested comma/ternary expression.
- Around line 176-201: GetUiDiscoverySettings currently caches
UMcpAutomationBridgeSettings on first call using static bInitialized and
Settings, so runtime edits to UMcpAutomationBridgeSettings are ignored; add a
refresh mechanism or make caching optional by (a) introducing a new function
RefreshUiDiscoverySettings() that resets the static bInitialized to false (and
clears Settings) so subsequent calls to GetUiDiscoverySettings re-read
GetDefault<UMcpAutomationBridgeSettings>(), or (b) add an optional bool
parameter to GetUiDiscoverySettings(bool bForceReload = false) that, when true,
clears bInitialized before loading; reference GetUiDiscoverySettings,
bInitialized, Settings and UMcpAutomationBridgeSettings when implementing the
change and update call sites accordingly (or wire RefreshUiDiscoverySettings to
settings-change notifications if preferred).
- Around line 203-216: ResolveProjectScopedPath currently returns paths without
sanitizing against traversal; call SanitizeProjectRelativePath on the trimmed
RelativePath (or the computed project-relative combined path) and reject or
normalize any path that fails validation so traversal like ../../ is prevented.
Specifically, in ResolveProjectScopedPath use SanitizeProjectRelativePath to
validate the Normalized input (or the result of
FPaths::Combine(FPaths::ProjectDir(), Normalized)), and if sanitization fails
return FPaths::ProjectDir() (or otherwise clamp to the project root); then
return the full absolute path using FPaths::ConvertRelativePathToFull on the
sanitized/contained path. Ensure you reference ResolveProjectScopedPath,
SanitizeProjectRelativePath, FPaths::ProjectDir, FPaths::Combine, and
FPaths::ConvertRelativePathToFull when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 07f5ae05-34dd-4d99-9a5e-614cd7aac83d
📒 Files selected for processing (13)
.gitignoreREADME.mdplugins/McpAutomationBridge/README.mdplugins/McpAutomationBridge/Source/McpAutomationBridge/McpAutomationBridge.Build.csplugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeSettings.cppplugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeSubsystem.cppplugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeToolCatalog.cppplugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_ControlHandlers.cppplugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cppplugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cppplugins/McpAutomationBridge/Source/McpAutomationBridge/Public/McpAutomationBridgeSettings.hplugins/McpAutomationBridge/Source/McpAutomationBridge/Public/McpAutomationBridgeSubsystem.hplugins/McpAutomationBridge/Source/McpAutomationBridge/Public/McpAutomationBridgeToolCatalog.h
...AutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp
Show resolved
Hide resolved
...ns/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp
Outdated
Show resolved
Hide resolved
Comment on lines
1937
to
1942
| Payload->TryGetStringField(TEXT("path"), ScreenshotPath); | ||
| if (ScreenshotPath.IsEmpty()) | ||
| { | ||
| ScreenshotPath = | ||
| FPaths::ProjectSavedDir() / TEXT("Screenshots/WindowsEditor"); | ||
| } else { | ||
| FString SafePath = SanitizeProjectFilePath(RawScreenshotPath); | ||
| if (SafePath.IsEmpty()) { | ||
| Message = FString::Printf(TEXT("Invalid or unsafe screenshot path: %s. Path must be relative to project."), *RawScreenshotPath); | ||
| ErrorCode = TEXT("SECURITY_VIOLATION"); | ||
| Resp->SetStringField(TEXT("error"), Message); | ||
| SendAutomationResponse(RequestingSocket, RequestId, false, Message, Resp, ErrorCode); | ||
| return true; | ||
| } | ||
|
|
||
| ScreenshotPath = FPaths::ProjectDir() / SafePath; | ||
| ScreenshotPath = FPaths::ConvertRelativePathToFull(ScreenshotPath); | ||
| FPaths::NormalizeFilename(ScreenshotPath); | ||
|
|
||
| FString NormalizedProjectDir = FPaths::ConvertRelativePathToFull(FPaths::ProjectDir()); | ||
| FPaths::NormalizeDirectoryName(NormalizedProjectDir); | ||
| if (!NormalizedProjectDir.EndsWith(TEXT("/"))) { | ||
| NormalizedProjectDir += TEXT("/"); | ||
| } | ||
|
|
||
| if (!ScreenshotPath.StartsWith(NormalizedProjectDir, ESearchCase::IgnoreCase)) { | ||
| Message = FString::Printf(TEXT("Invalid or unsafe screenshot path: %s. Path escapes project directory."), *RawScreenshotPath); | ||
| ErrorCode = TEXT("SECURITY_VIOLATION"); | ||
| Resp->SetStringField(TEXT("error"), Message); | ||
| SendAutomationResponse(RequestingSocket, RequestId, false, Message, Resp, ErrorCode); | ||
| return true; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Screenshot path is not sanitized—potential arbitrary file write.
The path parameter from the request payload is used directly without validation. A malicious caller could write screenshots to arbitrary filesystem locations (e.g., /etc/, C:\Windows\System32\). The AI summary confirms that path containment/sanitization was removed.
As per coding guidelines: "Use SanitizeProjectRelativePath() to validate and sanitize all file paths to prevent directory traversal attacks."
🛡️ Proposed fix to add path containment
FString ScreenshotPath;
Payload->TryGetStringField(TEXT("path"), ScreenshotPath);
if (ScreenshotPath.IsEmpty())
{
ScreenshotPath =
FPaths::ProjectSavedDir() / TEXT("Screenshots/WindowsEditor");
}
+ else
+ {
+ // Validate path is within project directory
+ ScreenshotPath = FPaths::ConvertRelativePathToFull(
+ FPaths::IsRelative(ScreenshotPath)
+ ? FPaths::Combine(FPaths::ProjectDir(), ScreenshotPath)
+ : ScreenshotPath);
+ const FString ProjectDir = FPaths::ConvertRelativePathToFull(FPaths::ProjectDir());
+ if (!ScreenshotPath.StartsWith(ProjectDir))
+ {
+ Message = TEXT("Screenshot path must be within project directory");
+ ErrorCode = TEXT("INVALID_PATH");
+ Resp->SetStringField(TEXT("error"), Message);
+ SendAutomationResponse(RequestingSocket, RequestId, false, Message, Resp, ErrorCode);
+ return true;
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp`
around lines 1937 - 1942, The ScreenshotPath is taken from the payload without
sanitization, allowing directory traversal; update the handler in
McpAutomationBridge_UiHandlers.cpp (around the code that calls
Payload->TryGetStringField and sets ScreenshotPath) to call
FPaths::SanitizeProjectRelativePath (or SanitizeProjectRelativePath()) on the
incoming ScreenshotPath, resolve it against FPaths::ProjectSavedDir() /
"Screenshots/WindowsEditor", and ensure the resulting path is contained inside
the project's Saved directory (reject or reset to the default if not).
Specifically: sanitize the raw payload path, construct an absolute/resolved
path, validate containment under FPaths::ProjectSavedDir(), and log and fallback
to the existing default ScreenshotPath if validation fails.
Comment on lines
93
to
96
| ``` | ||
| Copy: Unreal_mcp/plugins/McpAutomationBridge/ | ||
| To: YourUnrealProject/Plugins/McpAutomationBridge/ | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language tag to this fenced block.
markdownlint MD040 is firing here. Marking this block as text keeps the README lint-clean without changing the content.
📝 Suggested fix
-```
+```text
Copy: Unreal_mcp/plugins/McpAutomationBridge/
To: YourUnrealProject/Plugins/McpAutomationBridge/</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>
[warning] 93-93: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @README.md around lines 93 - 96, The fenced code block showing the copy
instructions lacks a language tag and triggers markdownlint MD040; update the
triple-backtick fence that surrounds the lines "Copy:
Unreal_mcp/plugins/McpAutomationBridge/" and "To:
YourUnrealProject/Plugins/McpAutomationBridge/" to include the language tag
text (i.e., change totext) so the block is marked as plain text and
the lint error is resolved.
</details>
<!-- fingerprinting:phantom:medusa:grasshopper:689bcaaf-7e2b-477d-a1f6-5199600404f2 -->
<!-- This is an auto-generated comment by CodeRabbit -->
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…I target errors Summary: 1. Aligned the standalone McpAutomationBridge public catalog contract with the registered handler surface. 2. Fixed `manage_pipeline` category reporting so `categories` and `toolCategories` reflect real category groups instead of tool ids. 3. Hardened `manage_ui` target-opening flows to preserve specific execution failures and guard editor-only helpers when `GEditor` is unavailable. 4. Removed stale README merge-note wording and replaced hard-coded numeric tool-count language with catalog-backed runtime guidance. 5. Added explicit contract-style docstrings to the touched catalog, pipeline, subsystem, and UI helper seams. 6. Total: 6 files changed, +190 / -43 lines. Features: 1. Catalog/Registry Alignment: Added missing `RegisterHandler()` bindings for `control_editor`, `manage_material_graph`, `manage_niagara_authoring`, `manage_niagara_graph`, `manage_tests`, `manage_logs`, `manage_debug`, and `manage_insights`. 2. Category-Safe Pipeline Metadata: `manage_pipeline list_categories` and `manage_pipeline get_status` now return real category names while preserving the existing response keys used by the local wrapper. 3. UI Error Preservation: `open_ui_target` now distinguishes resolved-but-failed opens from true not-found cases and surfaces editor-unavailable failures explicitly. 4. Contract Documentation: Added explicit contract docstrings for touched catalog declarations/definitions, pipeline helpers, the subsystem registration seam, and UI-opening helpers. 5. README Contract Cleanup: The plugin README now points readers to the catalog-backed runtime contract instead of stale merge notes and hard-coded tool counts. Changes: 1. Modified `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeSubsystem.cpp`, Specifically documented `RegisterHandler()` and registered the missing public catalog tool families in `InitializeHandlers()`. 2. Modified `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeToolCatalog.cpp`, Specifically added contract docstrings for catalog helpers and exported count/lookup functions. 3. Modified `plugins/McpAutomationBridge/Source/McpAutomationBridge/Public/McpAutomationBridgeToolCatalog.h`, Specifically documented the public catalog helper declarations. 4. Modified `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp`, Specifically fixed `BuildCatalogResponse()` to populate categories from category names, preserved response compatibility, and documented the touched helpers. 5. Modified `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp`, Specifically preserved execution-specific error codes for resolved targets, added `GEditor` guards to editor helper paths, and documented the touched helpers and entrypoint. 6. Modified `plugins/McpAutomationBridge/README.md`, Specifically removed stale merge-note prose and hard-coded numeric tool-count wording in favor of catalog-backed runtime guidance. Bug Fixes: 1. Fixed public-tool contract drift where cataloged bridge tool families were advertised without matching `RegisterHandler()` bindings. 2. Fixed `manage_pipeline` metadata regression where `categories` and `toolCategories` reflected tool names instead of category groups. 3. Fixed `open_ui_target` returning generic `NOT_FOUND` for resolved registered-command failures. 4. Fixed editor-helper null-dereference risk by guarding `GEditor` before editor subsystem access. 5. Fixed standalone README drift by removing temporary merge-note wording and stale hard-coded tool-count prose. Tests: 1. Host-project Development build verification: PASSED. 2. Source catalog verification via local MCP wrapper: PASSED (`sourceCatalogCount: 49`). 3. Project-to-standalone plugin parity verification: PASSED. 4. Standalone plugin-subtree scope verification: PASSED. Task: Few corrections for McpAutomationBridge plugin Signed-off-by: Punal Manalan <punal100@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp (1)
2014-2177:⚠️ Potential issue | 🟠 MajorCheck the disk write before reporting screenshot success.
bSavedis computed but never used. A read-only directory or failed write still returnssuccess=truewith ascreenshotPaththat does not exist.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp` around lines 2014 - 2177, The code sets bSaved = FFileHelper::SaveArrayToFile(PngData, *FullPath) but never checks it, so bSuccess is set true and response fields (screenshotPath, filename, width, height, sizeBytes) are sent even if the disk write failed; update the block after SaveArrayToFile to check bSaved and only set bSuccess = true and populate Resp fields when bSaved is true, otherwise set bSuccess = false, set an appropriate Message and ErrorCode (e.g., "WRITE_FAILED"), set Resp->SetStringField("error", Message) and avoid returning a non-existent FullPath; ensure any caller path that uses bSuccess/Resp (e.g., SendAutomationResponse) gets the correct failure state.plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp (1)
149-167:⚠️ Potential issue | 🟠 MajorUse platform-specific path resolution for UnrealBuildTool executable.
The hardcoded
Engine/Binaries/DotNET/UnrealBuildTool/UnrealBuildTool.exepath is Windows-only. On macOS and Linux, the binary is located at the same relative path but without the.exeextension (e.g.,UnrealBuildTool). This will causemanage_pipeline run_ubtto fail on non-Windows platforms even though the plugin advertises macOS and Linux support.Use Unreal Engine's platform detection macros (e.g.,
#if PLATFORM_WINDOWS) orFPlatformMisc::GetExecutableExtension()to conditionally construct the correct executable name for each platform.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp` around lines 149 - 167, The code builds UBTPath using a hardcoded "UnrealBuildTool.exe" which is Windows-only; update the construction of the executable name used in UBTPath to append the platform-specific executable extension (use FPlatformMisc::GetExecutableExtension() or platform macros like `#if` PLATFORM_WINDOWS) so Linux/macOS use "UnrealBuildTool" while Windows uses "UnrealBuildTool.exe"; replace the literal "UnrealBuildTool.exe" with a computed ExecName (e.g., FString ExecName = FString::Printf(TEXT("UnrealBuildTool%s"), *FPlatformMisc::GetExecutableExtension())) and then build UBTPath from FPaths::EngineDir() / TEXT("Binaries/DotNET/UnrealBuildTool") / ExecName before calling FPlatformProcess::CreateProc with that UBTPath and the existing Params.
♻️ Duplicate comments (1)
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp (1)
2192-2217:⚠️ Potential issue | 🔴 CriticalGuard
GEditorbefore callingExec().
play_in_editorfalls through toGEditor->Exec(...)whenGEditoris null, andsave_alldereferencesGEditorunconditionally. During editor startup/shutdown that is a straight null-pointer crash.Also applies to: 2253-2268
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp` around lines 2192 - 2217, The play_in_editor and save_all branches currently call GEditor->Exec (and dereference GEditor) without confirming GEditor is valid; add null checks guarding any use of GEditor in the play_in_editor block (and the save_all block referenced around lines 2253-2268) so that before calling GEditor->Exec(...) or accessing GEditor->PlayWorld you verify GEditor != nullptr and return a clear error (e.g., set Resp["error"] and ErrorCode) when GEditor is null; update the play_in_editor branch (the LowerSub == TEXT("play_in_editor") handling) and the save_all handler to early-fail when GEditor is null rather than dereferencing it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp`:
- Around line 1526-1533: The code currently treats any asset at TargetPath as
the requested widget; instead, after
UEditorAssetLibrary::DoesAssetExist(TargetPath) returns true, load the asset
(e.g., via LoadObject/StaticLoadObject or AssetRegistry) and verify its class
matches the expected widget blueprint/class (the one used when creating widgets
in this file); if the loaded asset is not of the expected type, set
Resp->SetBoolField("exists", false) and return a type-mismatch error message
(keep Resp->SetStringField("widgetPath", TargetPath) only when the type
matches), and apply the same fix in the other similar block around the 1797-1809
region.
- Around line 988-1003: Remove the use of caller-supplied EntryName for removing
menu entries and for the FToolMenuEntry ID: instead, derive a namespaced
internal ID (e.g. PrefixRequestEntryName like McpBridge_<EntryName>) and use
that namespaced ID when calling ToolMenus->RemoveEntry(MenuName, SectionName,
<NamespacedID>) and when constructing the FToolMenuEntry (the
InitToolBarButton/InitMenuEntry call). Keep the original
Definition->Label/Tooltip (and the original EntryName only for user-visible text
if needed) but never call RemoveEntry or set Entry.Owner with the raw EntryName
from the request; use the namespaced ID consistently before Section.AddEntry and
ToolMenus->RefreshMenuWidget.
- Around line 203-215: ResolveProjectScopedPath currently returns absolute
filesystem paths which leak workstation-specific locations when used to populate
the Json/definitionPath sent to the MCP client; change it to return a
project-relative, sanitized path by converting the normalized input to a path
relative to FPaths::ProjectDir() and then calling SanitizeProjectRelativePath()
before returning. Ensure any code that stores or sends JsonPath/definitionPath
uses the sanitized project-relative value (not the absolute returned by
FPaths::ConvertRelativePathToFull) and update other similar sites that produce
definitionPath (the other discovery/export locations flagged in the review) to
use the same SanitizeProjectRelativePath(projectRelative) flow so no absolute
Windows paths are emitted to the client.
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeToolCatalog.cpp`:
- Around line 35-84: The C++ catalog declares 14 tools (e.g., "manage_ui",
"manage_blueprint_graph", "manage_material_graph", "manage_world_partition",
"manage_render", "manage_animation_authoring", "manage_audio_authoring",
"manage_niagara_authoring", "manage_niagara_graph", "control_environment",
"manage_tests", "manage_logs", "manage_debug", "manage_insights") via MakeTool
but there are no matching TypeScript definitions or routes; to fix, add
corresponding action enum entries and JSON schemas for each named tool in
src/tools/consolidated-tool-definitions.ts and then register handlers for them
using toolRegistry.register(...) either in
src/tools/consolidated-tool-handlers.ts or appropriate files under
src/tools/handlers/, ensuring each registered handler accepts the defined schema
and routes requests for the exact tool names above.
---
Outside diff comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp`:
- Around line 149-167: The code builds UBTPath using a hardcoded
"UnrealBuildTool.exe" which is Windows-only; update the construction of the
executable name used in UBTPath to append the platform-specific executable
extension (use FPlatformMisc::GetExecutableExtension() or platform macros like
`#if` PLATFORM_WINDOWS) so Linux/macOS use "UnrealBuildTool" while Windows uses
"UnrealBuildTool.exe"; replace the literal "UnrealBuildTool.exe" with a computed
ExecName (e.g., FString ExecName = FString::Printf(TEXT("UnrealBuildTool%s"),
*FPlatformMisc::GetExecutableExtension())) and then build UBTPath from
FPaths::EngineDir() / TEXT("Binaries/DotNET/UnrealBuildTool") / ExecName before
calling FPlatformProcess::CreateProc with that UBTPath and the existing Params.
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp`:
- Around line 2014-2177: The code sets bSaved =
FFileHelper::SaveArrayToFile(PngData, *FullPath) but never checks it, so
bSuccess is set true and response fields (screenshotPath, filename, width,
height, sizeBytes) are sent even if the disk write failed; update the block
after SaveArrayToFile to check bSaved and only set bSuccess = true and populate
Resp fields when bSaved is true, otherwise set bSuccess = false, set an
appropriate Message and ErrorCode (e.g., "WRITE_FAILED"), set
Resp->SetStringField("error", Message) and avoid returning a non-existent
FullPath; ensure any caller path that uses bSuccess/Resp (e.g.,
SendAutomationResponse) gets the correct failure state.
---
Duplicate comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp`:
- Around line 2192-2217: The play_in_editor and save_all branches currently call
GEditor->Exec (and dereference GEditor) without confirming GEditor is valid; add
null checks guarding any use of GEditor in the play_in_editor block (and the
save_all block referenced around lines 2253-2268) so that before calling
GEditor->Exec(...) or accessing GEditor->PlayWorld you verify GEditor != nullptr
and return a clear error (e.g., set Resp["error"] and ErrorCode) when GEditor is
null; update the play_in_editor branch (the LowerSub == TEXT("play_in_editor")
handling) and the save_all handler to early-fail when GEditor is null rather
than dereferencing it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18208bec-6657-4f8f-a7ad-0aef5927a49c
📒 Files selected for processing (6)
plugins/McpAutomationBridge/README.mdplugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeSubsystem.cppplugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeToolCatalog.cppplugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cppplugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cppplugins/McpAutomationBridge/Source/McpAutomationBridge/Public/McpAutomationBridgeToolCatalog.h
...ns/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp
Show resolved
Hide resolved
...ns/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp
Show resolved
Hide resolved
...ns/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp
Show resolved
Hide resolved
Comment on lines
+35
to
+84
| static const TArray<FMcpAutomationBridgeToolCatalogEntry> Catalog = { | ||
| MakeTool(TEXT("control_actor"), TEXT("Core Editor"), TEXT("Spawn, transform, tag, and inspect actors and components inside the editor.")), | ||
| MakeTool(TEXT("control_editor"), TEXT("Core Editor"), TEXT("Drive editor viewport, screenshots, PIE controls, and simulated input."), {MakeSubAction(TEXT("screenshot"), TEXT("Capture viewport or editor-window screenshots.")), MakeSubAction(TEXT("simulate_input"), TEXT("Send keyboard, text, mouse, wheel, and drag input into a target tab or window."))}), | ||
| MakeTool(TEXT("manage_asset"), TEXT("Content"), TEXT("Browse, create, duplicate, move, rename, import, and delete Unreal assets.")), | ||
| MakeTool(TEXT("manage_level"), TEXT("World"), TEXT("Load, save, validate, duplicate, and stream levels and level assets.")), | ||
| MakeTool(TEXT("manage_sequence"), TEXT("Cinematics"), TEXT("Create and edit level sequences, bindings, tracks, and playback state.")), | ||
| MakeTool(TEXT("manage_ui"), TEXT("UI Automation"), TEXT("Discover UI targets, visible windows, commands, menus, and editor utility widgets."), {MakeSubAction(TEXT("list_ui_targets"), TEXT("List bridge-discovered UI targets, commands, and known menus.")), MakeSubAction(TEXT("list_visible_windows"), TEXT("List current Slate windows with outer and client bounds.")), MakeSubAction(TEXT("open_ui_target"), TEXT("Open a discovered target by identifier or tab id.")), MakeSubAction(TEXT("register_editor_command"), TEXT("Register a bridge-owned command for a session.")), MakeSubAction(TEXT("add_menu_entry"), TEXT("Attach a registered command to a ToolMenus menu or toolbar."))}), | ||
| MakeTool(TEXT("manage_input"), TEXT("UI Automation"), TEXT("Create and configure input mappings and related editor input assets.")), | ||
| MakeTool(TEXT("manage_blueprint"), TEXT("Blueprint Authoring"), TEXT("Create and modify Blueprint assets, classes, and components.")), | ||
| MakeTool(TEXT("manage_blueprint_graph"), TEXT("Graph Editing"), TEXT("Edit Blueprint graphs, nodes, pins, and execution flow.")), | ||
| MakeTool(TEXT("manage_material_authoring"), TEXT("Material Authoring"), TEXT("Create materials, material instances, parameters, and render targets.")), | ||
| MakeTool(TEXT("manage_material_graph"), TEXT("Graph Editing"), TEXT("Edit material expression graphs, nodes, and pin connections."), {MakeSubAction(TEXT("add_node"), TEXT("Add a material expression node to a material graph.")), MakeSubAction(TEXT("connect_nodes"), TEXT("Connect two material expressions or route an expression into a material input.")), MakeSubAction(TEXT("remove_node"), TEXT("Remove a material expression from the graph.")), MakeSubAction(TEXT("get_node_details"), TEXT("Inspect nodes and current graph state."))}), | ||
| MakeTool(TEXT("manage_texture"), TEXT("Material Authoring"), TEXT("Create, inspect, and update texture assets and texture metadata.")), | ||
| MakeTool(TEXT("manage_geometry"), TEXT("World"), TEXT("Author geometry, dynamic meshes, and related mesh-processing workflows.")), | ||
| MakeTool(TEXT("manage_world_partition"), TEXT("World"), TEXT("Inspect and control World Partition data and loaded cells.")), | ||
| MakeTool(TEXT("manage_render"), TEXT("Core Editor"), TEXT("Drive render-oriented editor actions and rendering utilities.")), | ||
| MakeTool(TEXT("manage_skeleton"), TEXT("Animation"), TEXT("Inspect and modify skeletons, sockets, bones, and physics assets.")), | ||
| MakeTool(TEXT("animation_physics"), TEXT("Animation"), TEXT("Drive animation, ragdoll, and animation-physics workflows.")), | ||
| MakeTool(TEXT("manage_animation_authoring"), TEXT("Animation"), TEXT("Author animation assets, tracks, and related animation workflows.")), | ||
| MakeTool(TEXT("manage_audio"), TEXT("Audio"), TEXT("Control audio runtime/editor utilities, sound playback, and audio assets.")), | ||
| MakeTool(TEXT("manage_audio_authoring"), TEXT("Audio"), TEXT("Author sound cues, MetaSounds, attenuation assets, dialogue assets, and sound classes."), {MakeSubAction(TEXT("create_sound_cue"), TEXT("Create a Sound Cue asset.")), MakeSubAction(TEXT("add_sound_node"), TEXT("Add a node to a Sound Cue graph.")), MakeSubAction(TEXT("create_meta_sound"), TEXT("Create a MetaSound asset when supported by the engine version.")), MakeSubAction(TEXT("create_sound_class"), TEXT("Create and configure a Sound Class or related mix asset."))}), | ||
| MakeTool(TEXT("manage_niagara_authoring"), TEXT("Visual Effects"), TEXT("Author Niagara systems, emitters, modules, parameters, and simulation settings."), {MakeSubAction(TEXT("create_niagara_system"), TEXT("Create a Niagara system asset.")), MakeSubAction(TEXT("add_emitter_to_system"), TEXT("Attach an emitter to a Niagara system.")), MakeSubAction(TEXT("add_system_parameter"), TEXT("Create or expose a Niagara parameter.")), MakeSubAction(TEXT("configure_gpu_simulation"), TEXT("Configure GPU simulation and related stages."))}), | ||
| MakeTool(TEXT("manage_niagara_graph"), TEXT("Graph Editing"), TEXT("Edit Niagara system and emitter graphs, modules, pins, and parameters."), {MakeSubAction(TEXT("add_module"), TEXT("Add a Niagara module node to the target graph.")), MakeSubAction(TEXT("connect_pins"), TEXT("Connect Niagara graph pins.")), MakeSubAction(TEXT("remove_node"), TEXT("Remove a Niagara graph node.")), MakeSubAction(TEXT("set_parameter"), TEXT("Set an exposed parameter value on the target graph."))}), | ||
| MakeTool(TEXT("manage_effect"), TEXT("Visual Effects"), TEXT("Create and configure generic visual effects and debug-shape workflows.")), | ||
| MakeTool(TEXT("manage_lighting"), TEXT("World"), TEXT("Inspect and modify lighting assets, settings, and lighting workflows.")), | ||
| MakeTool(TEXT("build_environment"), TEXT("World"), TEXT("Build landscapes, foliage, and environment authoring assets.")), | ||
| MakeTool(TEXT("control_environment"), TEXT("World"), TEXT("Control authored environment state and world-space environment helpers.")), | ||
| MakeTool(TEXT("manage_gas"), TEXT("Gameplay Systems"), TEXT("Author and configure Gameplay Ability System assets and metadata.")), | ||
| MakeTool(TEXT("manage_character"), TEXT("Gameplay Systems"), TEXT("Configure character, movement, and character-related editor assets.")), | ||
| MakeTool(TEXT("manage_combat"), TEXT("Gameplay Systems"), TEXT("Configure combat, weapon, and combat-related gameplay assets.")), | ||
| MakeTool(TEXT("manage_ai"), TEXT("Gameplay Systems"), TEXT("Author AI-focused assets, behavior, and supporting gameplay data.")), | ||
| MakeTool(TEXT("manage_inventory"), TEXT("Gameplay Systems"), TEXT("Create and update inventory, item, and related gameplay assets.")), | ||
| MakeTool(TEXT("manage_interaction"), TEXT("Gameplay Systems"), TEXT("Author interaction systems, traces, and widget-driven interaction helpers.")), | ||
| MakeTool(TEXT("manage_widget_authoring"), TEXT("UI Automation"), TEXT("Author widget assets and widget-related editor metadata.")), | ||
| MakeTool(TEXT("manage_networking"), TEXT("Gameplay Systems"), TEXT("Configure networking and multiplayer-related assets and settings.")), | ||
| MakeTool(TEXT("manage_game_framework"), TEXT("Gameplay Systems"), TEXT("Configure GameMode, GameState, PlayerController, and framework assets.")), | ||
| MakeTool(TEXT("manage_sessions"), TEXT("Gameplay Systems"), TEXT("Configure sessions and local multiplayer/session-oriented assets.")), | ||
| MakeTool(TEXT("manage_level_structure"), TEXT("World"), TEXT("Author level structure, streaming relationships, and organization assets.")), | ||
| MakeTool(TEXT("manage_volumes"), TEXT("World"), TEXT("Create and configure volumes, zones, and related world helpers.")), | ||
| MakeTool(TEXT("manage_navigation"), TEXT("World"), TEXT("Build and configure navigation assets and navigation-related state.")), | ||
| MakeTool(TEXT("manage_splines"), TEXT("World"), TEXT("Create and update spline actors, spline components, and spline-driven content.")), | ||
| MakeTool(TEXT("manage_pipeline"), TEXT("Diagnostics"), TEXT("Report bridge tool exposure and launch build and test-oriented editor automation."), {MakeSubAction(TEXT("run_ubt"), TEXT("Launch UnrealBuildTool with the provided target, platform, and configuration.")), MakeSubAction(TEXT("list_categories"), TEXT("Return the public bridge-owned MCP tool catalog.")), MakeSubAction(TEXT("get_status"), TEXT("Return bridge status, engine details, and catalog-derived capability counts."))}), | ||
| MakeTool(TEXT("manage_performance"), TEXT("Diagnostics"), TEXT("Profile performance, show FPS, and run performance-oriented editor commands.")), | ||
| MakeTool(TEXT("manage_tests"), TEXT("Diagnostics"), TEXT("Trigger automation test runs from the editor."), {MakeSubAction(TEXT("run_tests"), TEXT("Start automation tests by filter and report initiation status."))}), | ||
| MakeTool(TEXT("manage_logs"), TEXT("Diagnostics"), TEXT("Read recent editor logs and stream log output for the current session."), {MakeSubAction(TEXT("get_recent_logs"), TEXT("Return recent editor log output.")), MakeSubAction(TEXT("subscribe"), TEXT("Subscribe to streamed log output for the current bridge session."))}), | ||
| MakeTool(TEXT("manage_debug"), TEXT("Diagnostics"), TEXT("Drive gameplay-debugger and related debug-visualization workflows."), {MakeSubAction(TEXT("spawn_category"), TEXT("Toggle a gameplay debugger category."))}), | ||
| MakeTool(TEXT("manage_insights"), TEXT("Diagnostics"), TEXT("Control Unreal Insights trace capture from the editor."), {MakeSubAction(TEXT("start_session"), TEXT("Start a trace session with optional channels."))}), | ||
| MakeTool(TEXT("system_control"), TEXT("Diagnostics"), TEXT("Run high-level system control, cvar, quality, and utility editor operations.")), | ||
| MakeTool(TEXT("inspect"), TEXT("Diagnostics"), TEXT("Inspect actors, objects, metadata, settings, and scene/runtime state.")), | ||
| MakeTool(TEXT("manage_behavior_tree"), TEXT("Graph Editing"), TEXT("Author and modify behavior tree graphs and nodes."))}; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expected result: the script exits 0 and prints the final success line.
catalog_file="$(fd -i '^McpAutomationBridgeToolCatalog\.cpp$')"
defs_file="$(fd -i '^consolidated-tool-definitions\.ts$' src/tools)"
CATALOG_FILE="$catalog_file" DEFS_FILE="$defs_file" python - <<'PY'
import os, pathlib, re, subprocess, sys
catalog_text = pathlib.Path(os.environ["CATALOG_FILE"]).read_text()
defs_file = os.environ["DEFS_FILE"]
tools = re.findall(r'MakeTool\(TEXT\("([^"]+)"\)', catalog_text)
missing = []
for tool in tools:
defs_pat = r"(['\"])%s\1" % re.escape(tool)
route_pat = r"toolRegistry\.register\((['\"])%s\1" % re.escape(tool)
defs = subprocess.run(
["rg", "-nP", defs_pat, defs_file],
capture_output=True,
text=True,
)
routes = subprocess.run(
["rg", "-nP", route_pat, "src/tools"],
capture_output=True,
text=True,
)
if defs.returncode != 0 or routes.returncode != 0:
missing.append(tool)
if missing:
print("Catalog tools missing TS definitions or direct tool registrations:")
for tool in missing:
print(f" - {tool}")
sys.exit(1)
print("All catalog tools were found in consolidated-tool-definitions.ts and a toolRegistry.register(...) route.")
PYRepository: ChiR24/Unreal_mcp
Length of output: 445
14 tools in the C++ catalog are missing TypeScript definitions and routing.
The C++ catalog exposes these tools without corresponding src/tools/consolidated-tool-definitions.ts entries or toolRegistry.register(...) routes, causing manage_pipeline to advertise tools the server cannot dispatch:
- manage_ui
- manage_blueprint_graph
- manage_material_graph
- manage_world_partition
- manage_render
- manage_animation_authoring
- manage_audio_authoring
- manage_niagara_authoring
- manage_niagara_graph
- control_environment
- manage_tests
- manage_logs
- manage_debug
- manage_insights
Add action enum entries and JSON schemas for each tool in consolidated-tool-definitions.ts, then register TS handlers in consolidated-tool-handlers.ts or domain-specific handler modules under src/tools/handlers/ using toolRegistry.register().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeToolCatalog.cpp`
around lines 35 - 84, The C++ catalog declares 14 tools (e.g., "manage_ui",
"manage_blueprint_graph", "manage_material_graph", "manage_world_partition",
"manage_render", "manage_animation_authoring", "manage_audio_authoring",
"manage_niagara_authoring", "manage_niagara_graph", "control_environment",
"manage_tests", "manage_logs", "manage_debug", "manage_insights") via MakeTool
but there are no matching TypeScript definitions or routes; to fix, add
corresponding action enum entries and JSON schemas for each named tool in
src/tools/consolidated-tool-definitions.ts and then register handlers for them
using toolRegistry.register(...) either in
src/tools/consolidated-tool-handlers.ts or appropriate files under
src/tools/handlers/, ensuring each registered handler accepts the defined schema
and routes requests for the exact tool names above.
Summary: 1. Restored backward-compatible `manage_pipeline` metadata so the standalone bridge again reports tool-name categories and counts expected by the local MCP wrapper. 2. Expanded `manage_ui list_ui_targets` to surface the full Level Editor menubar dropdown set and richer ToolMenus-backed child entry metadata at runtime. 3. Hardened UI discovery, widget creation, screenshot writes, and editor-only command paths with safer validation and guard behavior. 4. Refined `open_ui_target` so missing identifiers and direct tab ids return specific not-found messages while resolved execution failures keep their branch-specific errors. 5. Kept the standalone bridge aligned with the verified host-project McpAutomationBridge implementation. 6. Total: 2 files changed. Features: 1. Backward-Compatible Pipeline Metadata: `manage_pipeline list_categories` now returns public tool names in `categories`, preserves tool count in `count`, and exposes grouped labels separately through `categoryGroups` and `groupCount`. 2. Cross-Platform UBT Resolution: `run_ubt` now resolves the platform-specific UnrealBuildTool executable name instead of hard-coding a Windows-only path. 3. Full Menubar Discovery: `manage_ui list_ui_targets` now surfaces `File`, `Edit`, `Window`, `Tools`, `Help`, `Build`, `Select`, `Platforms`, and the dynamic `Actions` menu with `Actor` aliases. 4. Tool Menu Entry Discovery: Added runtime discovery for ToolMenus child entries with section, entry, source-menu, alias, and direct-open metadata. 5. Safer UI Contract: JSON tool `definitionPath` values stay project-relative, bridge-owned menu entries are namespaced, pre-existing widget assets are type-checked, screenshot success requires a successful file write, and editor-only helpers guard unavailable editor state. 6. Precise Target Errors: `open_ui_target` now distinguishes missing identifiers, missing direct tab ids, Slate-unavailable paths, editor-unavailable paths, and resolved-but-failed execution flows. Changes: 1. Modified `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp`, Specifically restored tool-name semantics for `manage_pipeline` category and count fields, preserved grouped category metadata separately, and made `run_ubt` use platform-aware UnrealBuildTool executable naming. 2. Modified `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp`, Specifically refreshed UI discovery settings on each read, emitted project-relative `definitionPath` values, namespaced bridge-owned menu entry ids, added widget type validation, enforced screenshot save success, and guarded editor-only command paths. 3. Modified `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp`, Specifically added main-menu hierarchy discovery, ToolMenus child-entry metadata helpers, direct menu-entry execution support, and richer alias coverage for runtime UI target discovery. 4. Modified `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp`, Specifically updated `open_ui_target` to return `UI target not found: ...` for missing identifiers, `UI tab not found: ...` for missing direct tab ids, and to preserve resolved-target execution failures instead of collapsing them into generic not-found responses. 5. Modified `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp`, Specifically kept the standalone UI handler aligned with the verified host-project implementation after the live menubar discovery follow-up. Bug Fixes: 1. Fixed pipeline contract drift where `manage_pipeline` category and count fields no longer matched the legacy local-wrapper expectations. 2. Fixed a Windows-only UnrealBuildTool path assumption in `run_ubt`. 3. Fixed stale UI discovery settings behavior that required an editor restart before project-setting changes took effect. 4. Fixed absolute JSON definition path leakage in discovered UI targets. 5. Fixed bridge-owned menu registration collisions by scoping menu entry ids under `McpAutomationBridge.*`. 6. Fixed widget-creation success paths that previously accepted wrong pre-existing asset types. 7. Fixed screenshot success reporting when the PNG write failed on disk. 8. Fixed editor-helper null-dereference risk by guarding `GEditor` before editor-only command execution. 9. Fixed incomplete live UI discovery where top-level editor dropdowns such as `Window`, `Help`, `Build`, `Select`, and `Platforms` were not surfaced consistently through `manage_ui list_ui_targets`. 10. Fixed `open_ui_target` returning generic not-found responses for missing identifiers and direct tab ids instead of specific failure messages. Tests: 1. Host-project Development build verification: PASSED. 2. Source catalog verification via local MCP wrapper: PASSED (`sourceCatalogCount: 49`). 3. Live `manage_pipeline list_categories` verification: PASSED (`count: 49`, `groupCount: 13`, tool-name `categories`). 4. Live `manage_pipeline get_status` verification: PASSED (`toolCategories: 49`, `categoryGroups: 13`, `totalActions: 69`). 5. Live `manage_ui list_ui_targets` verification: PASSED (project-relative JSON `definitionPath` values and full Level Editor menubar discovery). 6. Live `manage_ui open_ui_target` verification: PASSED (`PythonEditorUtility.BuildLightingTool` opened successfully and remained discoverable as open). 7. Live `open_ui_target` error verification: PASSED (missing identifier and missing direct tab id returned specific not-found messages). 8. Visible-window verification after UI open: PASSED (`Build Lighting` window surfaced through `list_visible_windows`). 9. Project-to-standalone parity verification for `McpAutomationBridge_PipelineHandlers.cpp` and `McpAutomationBridge_UiHandlers.cpp`: PASSED. 10. VS Code error check on modified standalone files: PASSED. Task: Few corrections for McpAutomationBridge plugin Signed-off-by: Punal Manalan <punal100@gmail.com>
punal100
changed the title
Comment on lines
+251
to
+254
| Result->SetNumberField(TEXT("categoryGroups"), CategoryGroups.Num()); | ||
| Result->SetStringField(TEXT("catalogSource"), TEXT("McpAutomationBridgeToolCatalog")); | ||
| Result->SetArrayField(TEXT("categories"), ToolNames); | ||
| Result->SetArrayField(TEXT("categoryGroupNames"), CategoryGroups); |
Contributor
There was a problem hiding this comment.
🟡 categoryGroups JSON field has inconsistent types between list_categories and get_status responses
The categoryGroups key is set as an array in the list_categories response (McpAutomationBridge_PipelineHandlers.cpp:208) but as a number in the get_status response (McpAutomationBridge_PipelineHandlers.cpp:251). Any client parsing both responses under a unified schema will encounter a type mismatch. In list_categories, the count is stored as groupCount (line 210) and the array as categoryGroups (line 208). In get_status, the count is stored as categoryGroups (line 251) and the array as categoryGroupNames (line 254). The field names and types should be consistent across both responses.
Suggested change
| Result->SetNumberField(TEXT("categoryGroups"), CategoryGroups.Num()); | |
| Result->SetStringField(TEXT("catalogSource"), TEXT("McpAutomationBridgeToolCatalog")); | |
| Result->SetArrayField(TEXT("categories"), ToolNames); | |
| Result->SetArrayField(TEXT("categoryGroupNames"), CategoryGroups); | |
| Result->SetNumberField(TEXT("groupCount"), CategoryGroups.Num()); | |
| Result->SetStringField(TEXT("catalogSource"), TEXT("McpAutomationBridgeToolCatalog")); | |
| Result->SetArrayField(TEXT("categories"), ToolNames); | |
| Result->SetArrayField(TEXT("categoryGroups"), CategoryGroups); |
Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp (1)
135-159: Validate requiredrun_ubtinputs before launching the process.
target,platform, andconfigurationare optional in practice here; empty values still trigger process launch and may return a misleading success response.Proposed fix
FString ExtraArgs; Payload->TryGetStringField(TEXT("extraArgs"), ExtraArgs); + if (Target.IsEmpty() || Platform.IsEmpty() || Configuration.IsEmpty()) + { + SendAutomationError(RequestingSocket, RequestId, + TEXT("Missing required run_ubt fields: target, platform, configuration."), + TEXT("INVALID_PAYLOAD")); + return true; + } + // Construct the platform-specific UBT executable path. const FString UBTExecutableName =🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp` around lines 135 - 159, Validate that required inputs Target, Platform, and Configuration (obtained via Payload->TryGetStringField) are non-empty before constructing Params or launching the UBT process; if any are empty, abort the run, log or set an error response and do not proceed to use UBTExecutableName/UBTPath or start the process. Specifically, after reading Target, Platform, and Configuration and before building Params or invoking the process launcher that uses UBTPath/Params, check each string and return a failure status (with a clear log message referencing the missing field(s)) so an empty value cannot trigger a misleading success. Ensure ExtraArgs remains optional and only appended when present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp`:
- Around line 49-73: The current manual serialization in MakeSubActionJson and
MakeToolJson should be replaced with FJsonObjectConverter::UStructToJsonObject;
to do that either annotate FMcpAutomationBridgeToolSubActionCatalogEntry and
FMcpAutomationBridgeToolCatalogEntry with USTRUCT() (and UPROPERTY() on their
fields) or create new USTRUCT() wrapper types mirroring their fields, then
include "JsonObjectConverter.h" and call
FJsonObjectConverter::UStructToJsonObject on the USTRUCT instance to produce the
TSharedPtr<FJsonObject> (use the wrapper or convert the plain struct into the
USTRUCT instance first), and remove the per-field
SetStringField/SetBoolField/SetArrayField code in MakeSubActionJson and
MakeToolJson.
---
Nitpick comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp`:
- Around line 135-159: Validate that required inputs Target, Platform, and
Configuration (obtained via Payload->TryGetStringField) are non-empty before
constructing Params or launching the UBT process; if any are empty, abort the
run, log or set an error response and do not proceed to use
UBTExecutableName/UBTPath or start the process. Specifically, after reading
Target, Platform, and Configuration and before building Params or invoking the
process launcher that uses UBTPath/Params, check each string and return a
failure status (with a clear log message referencing the missing field(s)) so an
empty value cannot trigger a misleading success. Ensure ExtraArgs remains
optional and only appended when present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e93e78f7-cc4b-4dc6-a050-59d6525454f8
📒 Files selected for processing (2)
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cppplugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_UiHandlers.cpp
Comment on lines
+49
to
+73
| TSharedPtr<FJsonObject> MakeSubActionJson(const FMcpAutomationBridgeToolSubActionCatalogEntry &Entry) | ||
| { | ||
| TSharedPtr<FJsonObject> Json = MakeShared<FJsonObject>(); | ||
| Json->SetStringField(TEXT("name"), Entry.Name); | ||
| Json->SetStringField(TEXT("summary"), Entry.Summary); | ||
| return Json; | ||
| } | ||
|
|
||
| /** @brief Contract: serializes one public tool row from the canonical tool catalog. */ | ||
| TSharedPtr<FJsonObject> MakeToolJson(const FMcpAutomationBridgeToolCatalogEntry &Entry) | ||
| { | ||
| TSharedPtr<FJsonObject> Json = MakeShared<FJsonObject>(); | ||
| Json->SetStringField(TEXT("toolName"), Entry.ToolName); | ||
| Json->SetStringField(TEXT("category"), Entry.Category); | ||
| Json->SetStringField(TEXT("summary"), Entry.Summary); | ||
| Json->SetBoolField(TEXT("public"), Entry.bPublic); | ||
|
|
||
| TArray<TSharedPtr<FJsonValue>> SubActions; | ||
| for (const FMcpAutomationBridgeToolSubActionCatalogEntry &SubAction : Entry.SubActions) | ||
| { | ||
| SubActions.Add(MakeShared<FJsonValueObject>(MakeSubActionJson(SubAction))); | ||
| } | ||
| Json->SetArrayField(TEXT("subActions"), SubActions); | ||
| Json->SetNumberField(TEXT("subActionCount"), Entry.SubActions.Num()); | ||
| return Json; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Inspect catalog struct declarations (check if USTRUCT-backed for converter usage):"
fd -i 'McpAutomationBridgeToolCatalog.h' -x sed -n '1,220p' {}
echo
echo "2) Inspect JSON serialization implementation and converter usage in pipeline handlers:"
rg -n -C2 'MakeSubActionJson|MakeToolJson|SetStringField|SetArrayField|FJsonObjectConverter' \
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp \
plugins/McpAutomationBridge/Source/McpAutomationBridge/Public/McpAutomationBridgeToolCatalog.hRepository: ChiR24/Unreal_mcp
Length of output: 13711
Refactor to use FJsonObjectConverter for catalog struct serialization.
The handler code manually serializes catalog structs field-by-field using SetStringField(), SetBoolField(), etc., which violates the repo guideline requiring FJsonObjectConverter for struct serialization in handler code. However, FJsonObjectConverter only works with USTRUCT() decorated types. The catalog structs FMcpAutomationBridgeToolSubActionCatalogEntry and FMcpAutomationBridgeToolCatalogEntry are plain C++ structs without USTRUCT() decoration.
To comply with the guideline:
- Either convert the existing catalog structs to be
USTRUCT()decorated, or - Create
USTRUCT()wrapper types for serialization purposes.
Once the structs are USTRUCT()-compatible, refactor the MakeSubActionJson() and MakeToolJson() functions to use FJsonObjectConverter::UStructToJsonObject() instead of manual field assignment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp`
around lines 49 - 73, The current manual serialization in MakeSubActionJson and
MakeToolJson should be replaced with FJsonObjectConverter::UStructToJsonObject;
to do that either annotate FMcpAutomationBridgeToolSubActionCatalogEntry and
FMcpAutomationBridgeToolCatalogEntry with USTRUCT() (and UPROPERTY() on their
fields) or create new USTRUCT() wrapper types mirroring their fields, then
include "JsonObjectConverter.h" and call
FJsonObjectConverter::UStructToJsonObject on the USTRUCT instance to produce the
TSharedPtr<FJsonObject> (use the wrapper or convert the plain struct into the
USTRUCT instance first), and remove the per-field
SetStringField/SetBoolField/SetArrayField code in MakeSubActionJson and
MakeToolJson.
Summary: 1. Added menu-inclusive editor-window screenshot composition so `control_editor` captures open dropdowns and popup child windows by default. 2. Introduced an explicit `includeMenus` opt-out while preserving the old single-window behavior when callers disable it. 3. Returned screenshot metadata that reports whether menu inclusion was requested and how many popup child windows were composited. 4. Kept the standalone MCP schema aligned with the native bridge by exposing `includeMenus` on the `control_editor` input surface. 5. Total: 2 files changed, +204 / -9 lines. Features: 1. Child-Window Screenshot Composition: The standalone bridge now captures the selected Slate window into memory, recursively gathers visible child popup windows, and alpha-composites them into one saved PNG. 2. Default-On Menu Capture Toggle: `control_editor screenshot` now honors an optional `includeMenus` field that defaults to `true` when omitted. 3. Response Metadata Expansion: Screenshot responses now include `includeMenus` and `includedMenuWindowCount` so callers can confirm which capture path ran. 4. Schema Surface Alignment: `src/tools/consolidated-tool-definitions.ts` now declares `includeMenus` for `control_editor`, keeping the standalone MCP contract aligned with the native implementation. Changes: 1. Modified `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_ControlHandlers.cpp`, Specifically refactored editor-window screenshot capture into bitmap helpers, added visible child-window collection and alpha compositing, and wired the default-on `includeMenus` toggle plus response metadata into `HandleControlEditorScreenshot()`. 2. Modified `src/tools/consolidated-tool-definitions.ts`, Specifically added the optional `includeMenus` boolean to the standalone `control_editor` schema. Bug Fixes: 1. Fixed editor-window screenshots dropping open dropdown menus and similar popup Slate windows from the saved PNG. 2. Fixed the lack of an explicit caller control for menu-inclusive screenshot behavior by adding `includeMenus=false` as an opt-out. 3. Fixed screenshot contract ambiguity by returning whether menu inclusion was active and how many popup windows were merged. 4. Fixed project-to-standalone drift by mirroring the verified native screenshot behavior into the standalone repo and schema surface. Tests: 1. Host-project Development editor build verification: PASSED. 2. Live bridge connectivity and visible popup-window verification: PASSED. 3. Live default screenshot verification: PASSED (`includeMenus: true`, `includedMenuWindowCount: 1`, saved image includes the open `Tools` dropdown). 4. Live opt-out screenshot verification: PASSED (`includeMenus: false`, `includedMenuWindowCount: 0`, saved image excludes the dropdown). 5. Standalone modified-file VS Code error check: PASSED. 6. Project-to-standalone parity verification for the screenshot path and schema surface: PASSED. Task: Fix UE MCP screenshot capture so dropdown menus are optionally included by default Signed-off-by: Punal Manalan <punal100@gmail.com>
Summary: 1. Added stable canonical aliases for manage_pipeline category-group metadata without breaking the currently verified legacy response shapes. 2. Kept `list_categories` returning tool-name `categories`, structured `tools`, and array-shaped `categoryGroups` while also exposing `categoryGroupNames`. 3. Kept `get_status` returning numeric `toolCategories`, numeric `categoryGroups`, and array-shaped `categoryGroupNames` while also exposing `groupCount`. 4. Documented the intended contract in the pipeline handler so future callers can rely on canonical names while older callers remain compatible. 5. Total: 1 file changed, +17 / -12 lines. Features: 1. Canonical Category Group Aliases: `manage_pipeline` now exposes `categoryGroupNames` as the stable array field and `groupCount` as the stable numeric count field. 2. Backward-Compatible Legacy Fields: `categoryGroups` remains intact for both subactions with its existing verified shapes preserved for compatibility. 3. Contract Clarification: The pipeline handler now explicitly documents which fields are canonical and which field is a legacy alias. Changes: 1. Modified `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp`, Specifically renamed the internal catalog helper terminology to category-group names, added the contract note for canonical versus legacy fields, added `categoryGroupNames` to `list_categories`, and added `groupCount` to `get_status` while preserving the existing legacy response shapes. Bug Fixes: 1. Fixed the inconsistent manage_pipeline category-group contract by adding one stable array alias and one stable numeric alias across both subactions. 2. Fixed future client ambiguity around category-group metadata by documenting the canonical field names directly in the handler. 3. Fixed project-to-standalone contract drift by mirroring the verified host-project pipeline correction into the standalone plugin copy only after live verification passed. Tests: 1. Host-project Development editor build verification: PASSED. 2. Live `manage_pipeline list_categories` verification: PASSED (`categories` remained tool names, `categoryGroups` remained the array, `categoryGroupNames` matched it, `groupCount: 13`). 3. Live `manage_pipeline get_status` verification: PASSED (`toolCategories: 49`, `categoryGroups: 13`, `groupCount: 13`, `categoryGroupNames` length matched the count). 4. Live `manage_tools list_tools` compatibility-wrapper verification: PASSED. 5. Live `manage_tools get_status` compatibility-wrapper verification: PASSED. 6. Project-to-standalone file parity verification: PASSED. 7. Standalone plugin-subtree scope verification: PASSED. Task: Unreal_mcp corrections Signed-off-by: Punal Manalan <punal100@gmail.com>
coderabbitai
bot
added
documentation
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp (2)
147-162: Consider validating command line arguments.The
extraArgsfield is passed directly to the UBT command line without validation. While this is an editor-only build tool, untrusted input could potentially inject malicious arguments.Consider adding basic validation or escaping for the command line parameters:
💡 Suggested validation approach
FString ExtraArgs; Payload->TryGetStringField(TEXT("extraArgs"), ExtraArgs); +// Basic validation: reject if extraArgs contains shell metacharacters +if (ExtraArgs.Contains(TEXT(";")) || ExtraArgs.Contains(TEXT("&")) || + ExtraArgs.Contains(TEXT("|")) || ExtraArgs.Contains(TEXT("`"))) +{ + SendAutomationError(RequestingSocket, RequestId, + TEXT("Invalid characters in extraArgs."), TEXT("INVALID_ARGS")); + return true; +} + // Construct the platform-specific UBT executable path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp` around lines 147 - 162, Validate and safely handle the ExtraArgs before composing Params: ensure the string read via Payload->TryGetStringField(TEXT("extraArgs"), ExtraArgs) is sanitized (trimmed and limited in length), reject or escape shell meta-characters (e.g., ; | & > < ` $) and control characters, or better split into tokens and quote each token to prevent argument injection; then rebuild Params using the sanitized/quoted ExtraArgs alongside Target, Platform, Configuration (references: ExtraArgs, Payload->TryGetStringField, Params, UBTPath, Target, Platform, Configuration). Ensure any rejected input returns an error or is omitted rather than being passed verbatim to the UBT command line.
253-260: Clarify thetoolCategoriesfield semantics.The field
toolCategoriesis set toToolNames.Num(), which is actually the tool count, not the number of categories. This naming could mislead API consumers who expect it to represent category counts.If this is intentional for backward compatibility, consider adding an inline comment or updating API documentation to clarify the actual semantics. Alternatively, consider adding a more accurately named field:
Result->SetNumberField(TEXT("toolCategories"), ToolNames.Num()); +Result->SetNumberField(TEXT("toolCount"), ToolNames.Num()); // Canonical field🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp` around lines 253 - 260, The field "toolCategories" is currently using ToolNames.Num() (which is the tool count) and can mislead API consumers; change the call Result->SetNumberField(TEXT("toolCategories"), ...) to use the actual category count (e.g., CategoryGroupNames.Num()), and add a new field Result->SetNumberField(TEXT("toolCount"), ToolNames.Num()) so callers can see the tool count explicitly; also add a short inline comment near these Result->SetNumberField calls explaining the chosen semantics for backward compatibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp`:
- Around line 147-162: Validate and safely handle the ExtraArgs before composing
Params: ensure the string read via Payload->TryGetStringField(TEXT("extraArgs"),
ExtraArgs) is sanitized (trimmed and limited in length), reject or escape shell
meta-characters (e.g., ; | & > < ` $) and control characters, or better split
into tokens and quote each token to prevent argument injection; then rebuild
Params using the sanitized/quoted ExtraArgs alongside Target, Platform,
Configuration (references: ExtraArgs, Payload->TryGetStringField, Params,
UBTPath, Target, Platform, Configuration). Ensure any rejected input returns an
error or is omitted rather than being passed verbatim to the UBT command line.
- Around line 253-260: The field "toolCategories" is currently using
ToolNames.Num() (which is the tool count) and can mislead API consumers; change
the call Result->SetNumberField(TEXT("toolCategories"), ...) to use the actual
category count (e.g., CategoryGroupNames.Num()), and add a new field
Result->SetNumberField(TEXT("toolCount"), ToolNames.Num()) so callers can see
the tool count explicitly; also add a short inline comment near these
Result->SetNumberField calls explaining the chosen semantics for backward
compatibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 817f9709-2388-4250-8cbd-b488af7ca392
📒 Files selected for processing (3)
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_ControlHandlers.cppplugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cppsrc/tools/consolidated-tool-definitions.ts
✅ Files skipped from review due to trivial changes (1)
- src/tools/consolidated-tool-definitions.ts
Author
|
Seems like all reviews are finally passing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR updates the standalone
McpAutomationBridgeplugin with richer UI automation and input control capabilities. It adds targeted mouse and keyboard simulation with a visible simulated cursor, improves window and tab targeting for screenshots and input, and introduces a plugin-owned tool catalog somanage_pipelinemetadata stays aligned with the actual standalone bridge surface.Changes
manage_pipelinemetadata to use itSaved/outputRelated Issues
None specified.
Type of Change
Testing
Manual verification performed:
manage_pipelinecatalog-backed responsesPre-Merge Checklist