Skip to content

feat(manage_blueprint): add reparent_blueprint action#303

Open
Kaen-SG wants to merge 3 commits intoChiR24:mainfrom
Kaen-SG:feat/reparent-blueprint-action
Open

feat(manage_blueprint): add reparent_blueprint action#303
Kaen-SG wants to merge 3 commits intoChiR24:mainfrom
Kaen-SG:feat/reparent-blueprint-action

Conversation

@Kaen-SG
Copy link
Copy Markdown

@Kaen-SG Kaen-SG commented Mar 20, 2026

Title: feat: Add reparent_blueprint action to manage_blueprint tool

Description:

What

Adds a new reparent_blueprint action to the manage_blueprint tool, allowing the parent class of a Blueprint asset to be changed programmatically.

Why

There was no way to change a Blueprint's parent class through the MCP tool layer. This is a common operation during development — for example, reparenting a Blueprint from a generic base class to a project-specific C++ class after the C++ class has been authored. Without this action, the operation required manual steps in the UE editor and could not be scripted or automated via MCP.

Changes

File | Change -- | -- src/tools/consolidated-tool-definitions.ts | Added 'reparent_blueprint' to the manage_blueprint action enum src/tools/handlers/blueprint-handlers.ts | Added case 'reparent_blueprint' dispatch block routing to the C++ plugin

Usage

json
{
  "action": "reparent_blueprint",
  "blueprintPath": "/Game/Blueprints/BP_MyActor",
  "parentClass": "/Script/MyModule.MyBaseClass"
}

The parentClass parameter accepts either a short class name or a fully-qualified /Script/Module.ClassName path. The C++ plugin resolves the class, recompiles the Blueprint, and returns oldParentClass, newParentClass, compiled, and saved in the response.

Testing

Verified end-to-end against a live UE 5.7.3 project — action routes correctly through the TypeScript layer to the C++ plugin and returns a valid response.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to change)
  • 📚 Documentation update
  • 🔧 Configuration/build change
  • ♻️ Refactoring (no functional changes)
  • 🧪 Test addition/update

Testing

  • Tested with Unreal Engine (version: 5.7.3)
  • Tested MCP client integration (client: 0.5.19)
  • Added/updated tests

Pre-Merge Checklist

  • Code follows project style guidelines
  • Self-reviewed the code
  • Updated relevant documentation (if needed)
  • Added/updated tests (if applicable)
  • CI passes

Open with Devin

Add support for reparenting Blueprint assets to a new parent class via the manage_blueprint tool.
- Add 'reparent_blueprint' to the action enum in consolidated-tool-definitions.ts
- Add dispatch case in blueprint-handlers.ts routing to the C++ plugin's reparent_blueprint handler via executeAutomationRequest
- Accepts blueprintPath (also accepts name/path aliases) and parentClass parameters, consistent with other blueprint action conventions
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a crucial new capability to the manage_blueprint tool, allowing developers to programmatically change the parent class of Unreal Engine Blueprint assets. This enhancement addresses a previous limitation where such operations required manual intervention within the Unreal Editor, thereby significantly improving automation and scripting possibilities for common development workflows like reparenting Blueprints from generic to specific C++ classes.

Highlights

  • New reparent_blueprint action: Introduced a new reparent_blueprint action within the manage_blueprint tool, enabling programmatic modification of a Blueprint asset's parent class.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 20, 2026

📝 Walkthrough

Summary by CodeRabbit

Release Notes

  • New Features

    • Added blueprint function and event renaming and removal capabilities.
    • Added blueprint reparenting to change parent classes.
    • Added node search and graph inspection tools for blueprint analysis.
    • Added log reading with filtering by type, severity, and line count.
    • Enhanced widget creation with automatic name collision prevention.
    • Added support for enhanced input actions and subsystem nodes.
    • Implemented console command output capturing.
  • Bug Fixes

    • Fixed response parsing for nested automation responses.
    • Improved world cleanup stability.
  • Refactor

    • Streamlined log file handling and retrieval mechanisms.

Walkthrough

Extended blueprint management (rename/remove/reparent, graph introspection, node search) and system log-reading capabilities across TypeScript tools and C++ McpAutomationBridge handlers; multiple editor-level blueprint/graph operations, console capture, log file retrieval, and world/level handling behaviors were added or adjusted.

Changes

Cohort / File(s) Summary
Tool definitions & client APIs
src/tools/consolidated-tool-definitions.ts, src/types/tool-interfaces.ts, src/tools/blueprint.ts, src/tools/consolidated-tool-handlers.ts
Added new blueprint actions (rename_function, remove_function, rename_event, reparent_blueprint, find_nodes_by_title, get_graph_connections, get_node_properties, list_graphs) and log-read aliases; added client methods renameEvent/renameFunction/removeFunction; expanded graph action routing.
TypeScript handlers / types
src/tools/handlers/graph-handlers.ts, src/tools/handlers/system-handlers.ts, src/types/handler-types.ts, src/utils/response-validator.ts, src/tools/consolidated-tool-definitions.ts
Propagated titleFilter through processed graph args and handler types; unified log-read routing to bridge-based read_log variants; adjusted response summarization formatting and array serialization behavior.
Blueprint handler logic (C++)
plugins/McpAutomationBridge/Source/.../McpAutomationBridge_BlueprintHandlers.cpp, ..._BlueprintGraphHandlers.cpp
Added rename/remove function & event handlers, reparenting, improved cast/array/EnhancedInput node creation, expanded graph inspection subActions (find_nodes_by_title, get_graph_connections, get_node_properties, list_graphs), richer pin/node property handling, and switched to throttled save signature with force flag.
Console & Control handlers (C++)
plugins/McpAutomationBridge/Source/.../McpAutomationBridge_ConsoleCommandHandlers.cpp, ..._ControlHandlers.cpp, Public/McpAutomationBridgeSubsystem.h
Implemented console command output capture/merge and batch handling; added HandleReadLog implementation and subsystem declaration; Control handler responses now carry output arrays and structured log-reading results.
Pipeline / logs (C++)
plugins/McpAutomationBridge/Source/.../McpAutomationBridge_PipelineHandlers.cpp, plugins/McpAutomationBridge/Source/.../McpAutomationBridge_SystemControlHandlers.cpp
Added get_ubt_log and get_livecoding_log pipeline handlers; system control routing now recognizes read_log/read_logs/get_log/get_logs and delegates to read-log handler; adjusted asset export sanitization removal.
Level & world handling (C++)
plugins/McpAutomationBridge/Source/.../McpAutomationBridge_LevelStructureHandlers.cpp, McpSafeOperations.h, McpAutomationBridgeHelpers.h
Removed several in-memory cleanup/unroot steps and FTickTaskManagerInterface::EndFrame() calls; simplified WorldPartition and sublevel creation flows and removed OFPA/external-actors gating logic.
Widget authoring & UI handlers (C++)
plugins/McpAutomationBridge/Source/.../McpAutomationBridge_WidgetAuthoringHandlers.cpp
Switched widget naming to prefer payload["name"], added uniqueness suffixing to avoid collisions, explicit parent validation/errors, and visibility handling for vertical box creation.
Utility & validation (C++)
plugins/McpAutomationBridge/Source/.../McpKeyValidation.cpp, plugins/McpAutomationBridge/Source/.../McpHandlerUtils.cpp
Added McpKeyValidation::IsKeyNameRegistered with static key registry; moved EngineUtils.h includes inside #if WITH_EDITOR.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client (TS tools)
    participant BridgeTS as Consolidated handlers (TS)
    participant BridgeCPP as McpAutomationBridge (C++)
    participant EditorFS as Editor/Filesystem

    Client->>BridgeTS: sendAction('rename_function', payload)
    BridgeTS->>BridgeCPP: executeAutomationRequest('blueprint_rename_function', args)
    BridgeCPP->>EditorFS: Load Blueprint asset, find function graph
    EditorFS-->>BridgeCPP: function graph found / renamed
    BridgeCPP->>EditorFS: Compile, SaveLoadedAssetThrottled(..., bForce=true)
    EditorFS-->>BridgeCPP: save/compile result
    BridgeCPP-->>BridgeTS: automation response (success/error)
    BridgeTS-->>Client: normalized response
Loading
sequenceDiagram
    participant Client as Client (TS tools)
    participant BridgeTS as Consolidated handlers (TS)
    participant BridgeCPP as McpAutomationBridge (C++)
    participant OSFS as OS/FileSystem

    Client->>BridgeTS: sendAction('read_logs', {logType, tail, severity, filter})
    BridgeTS->>BridgeCPP: executeAutomationRequest('read_log', args)
    BridgeCPP->>OSFS: Locate log files (current/previous/crash), open with shared-safe handle
    OSFS-->>BridgeCPP: file contents / errors
    BridgeCPP->>BridgeCPP: apply severity/filter/tail, parse crashes if needed
    BridgeCPP-->>BridgeTS: structured log response (lines/files/metadata)
    BridgeTS-->>Client: normalized response
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

bug, javascript

Poem

🐰 Hoppity hops through blueprint trees I go,
Renaming functions, finding nodes in a row,
I nibble logs and stitch their lines so neat,
Reparenting branches with a tiny heartbeat,
A little rabbit cheers — automation's sweet!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description does not follow the repository template structure. Required sections like Summary (with key details), Changes (itemized list), Related Issues, and a properly filled Type of Change/Testing/Checklist are missing or incomplete. Provide a concise summary of what the PR changes and why; itemize key changes; fill in the Related Issues section if applicable; ensure all Type of Change/Testing/Checklist items are properly completed.
Docstring Coverage ⚠️ Warning Docstring coverage is 27.59% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'feat(manage_blueprint): add reparent_blueprint action' clearly summarizes the main change: adding a new reparent_blueprint action to the manage_blueprint tool.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch feat/reparent-blueprint-action

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai bot added enhancement New feature or request size/s area/plugin labels Mar 20, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/tools/consolidated-tool-definitions.ts (1)

190-190: Declare reparent_blueprint response fields in manage_blueprint.outputSchema.

The new action is added correctly, but its documented response contract is still implicit. Please add explicit output properties for oldParentClass, newParentClass, compiled, and saved to keep schema discoverability strong for clients.

Suggested schema addition
   outputSchema: {
     type: 'object',
     properties: {
       ...commonSchemas.outputBase,
       blueprintPath: commonSchemas.blueprintPath,
-      blueprint: { oneOf: [{ type: 'object' }, { type: 'string' }], description: 'Blueprint data object or path string.' }
+      blueprint: { oneOf: [{ type: 'object' }, { type: 'string' }], description: 'Blueprint data object or path string.' },
+      oldParentClass: commonSchemas.stringProp,
+      newParentClass: commonSchemas.stringProp,
+      compiled: commonSchemas.booleanProp,
+      saved: commonSchemas.booleanProp
     }
   }

As per coding guidelines, "Tool schemas and action enums live in src/tools/consolidated-tool-definitions.ts. Treat that file as the source of truth for inputs and outputs."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/consolidated-tool-definitions.ts` at line 190, The manage_blueprint
tool added the 'reparent_blueprint' action but its output schema isn't declared;
update manage_blueprint.outputSchema in
src/tools/consolidated-tool-definitions.ts to include explicit response
properties for the action: add oldParentClass (string | null), newParentClass
(string), compiled (boolean), and saved (boolean) to the schema for
discoverability so clients can rely on these fields when 'reparent_blueprint' is
returned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/tools/handlers/blueprint-handlers.ts`:
- Around line 365-370: The reparent_blueprint branch must validate and normalize
inputs before calling executeAutomationRequest: use requireAction(args) to
ensure the action is allowed, normalize the blueprint path using normalizePath()
when falling back from argsTyped.name to argsRecord.path, and verify parentClass
is present (argsTyped.parentClass or argsRecord.parentClass) failing fast with a
contextual error string prefixed by "manage_blueprint.reparent_blueprint" if
missing or invalid; only after these checks call
executeAutomationRequest('reparent_blueprint', ...) with the validated
blueprintPath and parentClass values.

---

Nitpick comments:
In `@src/tools/consolidated-tool-definitions.ts`:
- Line 190: The manage_blueprint tool added the 'reparent_blueprint' action but
its output schema isn't declared; update manage_blueprint.outputSchema in
src/tools/consolidated-tool-definitions.ts to include explicit response
properties for the action: add oldParentClass (string | null), newParentClass
(string), compiled (boolean), and saved (boolean) to the schema for
discoverability so clients can rely on these fields when 'reparent_blueprint' is
returned.
🪄 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: 95b310e8-8367-48df-99b2-55ba581742dd

📥 Commits

Reviewing files that changed from the base of the PR and between fd94f7a and 28e1c4a.

📒 Files selected for processing (2)
  • src/tools/consolidated-tool-definitions.ts
  • src/tools/handlers/blueprint-handlers.ts

Comment on lines +365 to +370
case 'reparent_blueprint': {
const res = await executeAutomationRequest(tools, 'reparent_blueprint', {
blueprintPath: argsTyped.blueprintPath || argsTyped.name || (argsRecord.path as string) || '',
parentClass: argsTyped.parentClass ?? (argsRecord.parentClass as string) ?? '',
timeoutMs: argsRecord.timeoutMs as number | undefined
}) as Record<string, unknown>;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Harden reparent_blueprint input validation before bridge dispatch.

This branch accepts name as a path alias but doesn’t re-check traversal on that fallback and doesn’t fail fast when parentClass is missing. Validate both fields in this case and return contextual errors (manage_blueprint.reparent_blueprint) before calling C++.

Suggested hardening patch
     case 'reparent_blueprint': {
+      const resolvedBlueprintPath = normalizeBlueprintPath(
+        argsTyped.blueprintPath || (argsRecord.path as string | undefined) || argsTyped.name
+      );
+      const resolvedParentClass =
+        (typeof argsTyped.parentClass === 'string' ? argsTyped.parentClass : undefined) ??
+        (typeof argsRecord.parentClass === 'string' ? (argsRecord.parentClass as string) : undefined);
+
+      if (!resolvedBlueprintPath || hasBlueprintPathTraversal(resolvedBlueprintPath)) {
+        throw new Error('manage_blueprint.reparent_blueprint: invalid blueprintPath');
+      }
+      if (!resolvedParentClass || resolvedParentClass.trim() === '') {
+        throw new Error('manage_blueprint.reparent_blueprint: missing required parentClass');
+      }
+
       const res = await executeAutomationRequest(tools, 'reparent_blueprint', {
-        blueprintPath: argsTyped.blueprintPath || argsTyped.name || (argsRecord.path as string) || '',
-        parentClass: argsTyped.parentClass ?? (argsRecord.parentClass as string) ?? '',
+        blueprintPath: resolvedBlueprintPath,
+        parentClass: resolvedParentClass.trim(),
         timeoutMs: argsRecord.timeoutMs as number | undefined
       }) as Record<string, unknown>;
       return cleanObject(res);
     }

As per coding guidelines, "Validate action using requireAction(args) and normalize paths using normalizePath() before sending parameters to the C++ bridge" and "Include tool and action names in all error messages for proper error context."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/handlers/blueprint-handlers.ts` around lines 365 - 370, The
reparent_blueprint branch must validate and normalize inputs before calling
executeAutomationRequest: use requireAction(args) to ensure the action is
allowed, normalize the blueprint path using normalizePath() when falling back
from argsTyped.name to argsRecord.path, and verify parentClass is present
(argsTyped.parentClass or argsRecord.parentClass) failing fast with a contextual
error string prefixed by "manage_blueprint.reparent_blueprint" if missing or
invalid; only after these checks call
executeAutomationRequest('reparent_blueprint', ...) with the validated
blueprintPath and parentClass values.

@ChiR24
Copy link
Copy Markdown
Owner

ChiR24 commented Mar 20, 2026

Thanks for the PR! Reparenting blueprints is a useful feature.

I dug into this a bit (including checking the UE engine source) and found a missing piece.


Missing C++ Handler

The TypeScript side looks good, but there's no C++ handler to actually do the work:

$ rg "reparent_blueprint" plugins/ --type cpp
# nothing found :(

Right now this would just hit the "Unknown blueprint action" fallback and error out.


Good News

The engine API you need already exists and works across all UE 5.0-5.7:

#include "BlueprintEditorLibrary.h"

// This is all you need:
UBlueprintEditorLibrary::ReparentBlueprint(Blueprint, NewParentClass);

No version checks needed - it's been there since 5.0!


What's Needed

Just need to add a handler in McpAutomationBridge_BlueprintHandlers.cpp (somewhere before the "Unknown blueprint action" fallback around line 5900). Something like:

if (ActionMatchesPattern(TEXT("reparent_blueprint")) ||
    AlphaNumLower.Contains(TEXT("reparentblueprint"))) {
  
  FString BPPath = ResolveBlueprintRequestedPath();
  FString ParentClassSpec;
  LocalPayload->TryGetStringField(TEXT("parentClass"), ParentClassSpec);
  
  // ... validation, load blueprint, resolve class ...
  
  UBlueprintEditorLibrary::ReparentBlueprint(Blueprint, NewParentClass);
  
  // compile, save, return result
}

Don't forget to add the include at the top:

#include "BlueprintEditorLibrary.h"

Quick Question

You mentioned testing this on UE 5.7.3 - did you have a local fork with the C++ code added? Just curious since without the handler it would've returned UNKNOWN_ACTION.


Once the C++ side is in, this should be good to go!

@Kaen-SG
Copy link
Copy Markdown
Author

Kaen-SG commented Mar 20, 2026

Thanks for the PR! Reparenting blueprints is a useful feature.

I dug into this a bit (including checking the UE engine source) and found a missing piece.

Missing C++ Handler

The TypeScript side looks good, but there's no C++ handler to actually do the work:

$ rg "reparent_blueprint" plugins/ --type cpp
# nothing found :(

Right now this would just hit the "Unknown blueprint action" fallback and error out.

Good News

The engine API you need already exists and works across all UE 5.0-5.7:

#include "BlueprintEditorLibrary.h"

// This is all you need:
UBlueprintEditorLibrary::ReparentBlueprint(Blueprint, NewParentClass);

No version checks needed - it's been there since 5.0!

What's Needed

Just need to add a handler in McpAutomationBridge_BlueprintHandlers.cpp (somewhere before the "Unknown blueprint action" fallback around line 5900). Something like:

if (ActionMatchesPattern(TEXT("reparent_blueprint")) ||
    AlphaNumLower.Contains(TEXT("reparentblueprint"))) {
  
  FString BPPath = ResolveBlueprintRequestedPath();
  FString ParentClassSpec;
  LocalPayload->TryGetStringField(TEXT("parentClass"), ParentClassSpec);
  
  // ... validation, load blueprint, resolve class ...
  
  UBlueprintEditorLibrary::ReparentBlueprint(Blueprint, NewParentClass);
  
  // compile, save, return result
}

Don't forget to add the include at the top:

#include "BlueprintEditorLibrary.h"

Quick Question

You mentioned testing this on UE 5.7.3 - did you have a local fork with the C++ code added? Just curious since without the handler it would've returned UNKNOWN_ACTION.

Once the C++ side is in, this should be good to go!

Oh yeah I didnt PR the plugin changes.. I have had a lot more changes done on the plugin to fix issues i encountered.
Everything works quite well on my local fixed version. I'll find the update for this one in a bit.

@ChiR24
Copy link
Copy Markdown
Owner

ChiR24 commented Mar 20, 2026

No worries! Thanks for clarifying. Feel free to keep this open and push the C++ changes when ready - I'll review once they're in. Using draft PRs for WIP is a good idea if that helps your workflow. 👍

Kaen-SG added 2 commits March 27, 2026 10:36
…ructure, world partition, HLOD, level instances, blueprints, and system control.
… automation handlers and supporting TypeScript tools.
@github-actions github-actions bot added size/xl and removed size/s labels Mar 27, 2026
@github-actions
Copy link
Copy Markdown
Contributor

📏 Large PR Detected

This pull request is quite large (1000+ lines changed), which can make reviewing challenging.

Suggestions:

  • Consider breaking this into smaller, focused PRs
  • Separate refactoring from new features
  • Split bug fixes from feature additions

This helps reviewers provide better feedback and speeds up the merge process. Thank you! 🙏

@coderabbitai coderabbitai bot added bug Something isn't working javascript Pull requests that update javascript code labels Mar 27, 2026
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 4 new potential issues.

View 11 additional findings in Devin Review.

Open in Devin Review

Comment on lines +533 to +549
// Add streaming level
ULevelStreamingDynamic* StreamingLevel = NewObject<ULevelStreamingDynamic>(World, ULevelStreamingDynamic::StaticClass());
if (!StreamingLevel)
{
Subsystem->SendAutomationResponse(Socket, RequestId, false,
FString::Printf(TEXT("Failed to create world for sublevel: %s"), *SublevelName), nullptr, TEXT("WORLD_CREATION_FAILED"));
TEXT("Failed to create streaming level object"), nullptr);
return true;
}

// Initialize the world if not already initialized
if (!NewSublevelWorld->bIsWorldInitialized)
{
NewSublevelWorld->InitWorld();
}
// Configure the streaming level
StreamingLevel->SetWorldAssetByPackageName(FName(*SublevelPath));
StreamingLevel->LevelTransform = FTransform::Identity;
StreamingLevel->SetShouldBeVisible(true);
StreamingLevel->SetShouldBeLoaded(true);

// Mark package dirty
SublevelPackage->MarkPackageDirty();
// Add to world's streaming levels
World->AddStreamingLevel(StreamingLevel);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 HandleCreateSublevel no longer creates the sublevel .umap file on disk

The HandleCreateSublevel function was rewritten to only create a ULevelStreamingDynamic reference pointing to SublevelPath, but no actual level package/world/.umap file is created on disk at that path. The old code used CreatePackage, UWorld::CreateWorld, InitWorld, and McpSafeLevelSave to produce a real .umap file, then created the streaming reference. The new code skips all of that and just creates a dangling streaming reference. When UE attempts to load this streaming level at runtime, it will fail because no level file exists at the referenced path. The function also lost its idempotency check (FPackageName::DoesPackageExist) so calling it twice creates duplicate streaming references. The response reports success: true despite nothing loadable being produced.

Prompt for agents
In plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_LevelStructureHandlers.cpp, function HandleCreateSublevel (around line 458-568): The function must actually create the sublevel package and world on disk before creating the streaming reference. Restore the level creation logic: 1) Call CreatePackage() with the SublevelPath to create a UPackage. 2) Call UWorld::CreateWorld(EWorldType::Inactive, false, SublevelName, Package) to create the world. 3) Initialize the world if needed. 4) Save the level using McpSafeLevelSave(). 5) Add an idempotency check at the top using FPackageName::DoesPackageExist() to return early if the sublevel already exists. 6) Clean up the in-memory world after saving (same pattern as HandleCreateLevel). 7) THEN create the ULevelStreamingDynamic and add it to the parent world. Without these steps, the streaming reference points to a non-existent file and will fail to load.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

// Pass false to allow creation of Level Blueprint if it doesn't exist
ULevelScriptBlueprint* LevelBP = PersistentLevel->GetLevelScriptBlueprint(false);
ULevelScriptBlueprint* LevelBP = PersistentLevel->GetLevelScriptBlueprint(true);
if (!LevelBP)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 GetLevelScriptBlueprint parameter inverted from false to true in HandleOpenLevelBlueprint

The call to GetLevelScriptBlueprint was changed from false to true. The parameter is bDontCreatefalse means "create if missing" and true means "return nullptr if missing". The old code intentionally passed false to create the level blueprint on demand (with the comment "Pass false to allow creation of Level Blueprint if it doesn't exist"). The new code passes true, which prevents creation. This means HandleOpenLevelBlueprint will now fail with an error on any level that doesn't already have a level script blueprint, rather than creating one as intended. The copied comment (now saying GetLevelScriptBlueprint(true) may fail to create) is self-contradictory since true means "don't create" — it won't even attempt creation.

Suggested change
if (!LevelBP)
ULevelScriptBlueprint* LevelBP = PersistentLevel->GetLevelScriptBlueprint(false);
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

// Pass false to allow creation of Level Blueprint if it doesn't exist
ULevelScriptBlueprint* LevelBP = CurrentLevel->GetLevelScriptBlueprint(false);
ULevelScriptBlueprint* LevelBP = CurrentLevel->GetLevelScriptBlueprint(true);
if (!LevelBP)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 GetLevelScriptBlueprint parameter inverted from false to true in HandleAddLevelBlueprintNode

Same issue as HandleOpenLevelBlueprint: GetLevelScriptBlueprint(true) was changed from the original false. With bDontCreate=true, the function returns nullptr if the level blueprint doesn't exist yet. The old code passed false (create if missing) with the explicit comment "Pass false to allow creation of Level Blueprint if it doesn't exist". This means HandleAddLevelBlueprintNode will fail on levels without an existing level script blueprint instead of creating one on demand.

Suggested change
if (!LevelBP)
ULevelScriptBlueprint* LevelBP = CurrentLevel->GetLevelScriptBlueprint(false);
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines 394 to +397
}

// Load the asset
UObject* Asset = UEditorAssetLibrary::LoadAsset(SafeAssetPath);
UObject* Asset = UEditorAssetLibrary::LoadAsset(AssetPath);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 export_asset removed SanitizeProjectRelativePath on user-provided assetPath

The export_asset handler in HandleSystemControlAction removed the SanitizeProjectRelativePath() call on the user-provided AssetPath. The old code validated AssetPath through SanitizeProjectRelativePath and used the sanitized result (SafeAssetPath) for all subsequent operations (DoesAssetExist, LoadAsset, building the expected export path). The new code passes the raw, unsanitized AssetPath directly to UEditorAssetLibrary calls. This violates the AGENTS.md rule: "Path First: Always call sanitizePath() before using any user-provided path" (src/utils/AGENTS.md) and the plugin AGENTS.md: "Path Security: SanitizeProjectRelativePath() — Block traversal attacks" (plugins/McpAutomationBridge/AGENTS.md).

(Refers to lines 382-397)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 19

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_LevelStructureHandlers.cpp (3)

513-530: ⚠️ Potential issue | 🟠 Major

Sanitize the synthesized sublevel path too.

Only the caller-provided sublevelPath is sanitized. When this branch derives the path from SublevelName, values like ../Foo or invalid asset-name characters bypass the only path guard in the handler.

Minimal fix
     if (SublevelPath.IsEmpty())
     {
-        FString WorldPath = World->GetOutermost()->GetName();
-        SublevelPath = FPaths::GetPath(WorldPath) / SublevelName;
+        const FString WorldPath = World->GetOutermost()->GetName();
+        const FString CandidatePath = FPaths::GetPath(WorldPath) / SublevelName;
+        SublevelPath = SanitizeProjectRelativePath(CandidatePath);
+        if (SublevelPath.IsEmpty())
+        {
+            Subsystem->SendAutomationResponse(Socket, RequestId, false,
+                FString::Printf(TEXT("Invalid or unsafe sublevel path: %s"), *CandidatePath),
+                nullptr, TEXT("SECURITY_VIOLATION"));
+            return true;
+        }
     }

As per coding guidelines, Use SanitizeProjectRelativePath() to validate and sanitize all file paths to prevent directory traversal attacks.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_LevelStructureHandlers.cpp`
around lines 513 - 530, The synthesized SublevelPath that is built from
World->GetOutermost()->GetName() and SublevelName must also be validated with
SanitizeProjectRelativePath; after the branch that sets SublevelPath =
FPaths::GetPath(WorldPath) / SublevelName call SanitizeProjectRelativePath on
the result, check for an empty return, and if empty use
Subsystem->SendAutomationResponse(Socket, RequestId, false,
FString::Printf(TEXT("Invalid or unsafe sublevel path: %s"), *SublevelPath),
nullptr, TEXT("SECURITY_VIOLATION")) and return true; only replace SublevelPath
with the sanitized value when SanitizeProjectRelativePath succeeds.

492-567: ⚠️ Potential issue | 🔴 Critical

This now creates a dangling streaming reference instead of a sublevel.

The new flow never creates or validates the target map package before adding the streaming level to the current editor world. In the default-path case, create_sublevel can persist a reference that points at no level asset, and the validated parentLevel still is not used to choose which world gets modified. At minimum, reject non-existent SublevelPath values until the asset-creation flow is restored.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_LevelStructureHandlers.cpp`
around lines 492 - 567, The handler currently adds a ULevelStreamingDynamic
(StreamingLevel) pointing at SublevelPath without ensuring the target map
package exists and without using ParentLevel to select which world to modify,
producing dangling references; update the flow in the function handling
ParentLevel/SublevelPath so that you (1) resolve and validate the actual package
name with FPackageName::DoesPackageExist for SublevelPath (use the same
normalization used for ParentLevel), rejecting the request with a
SECURITY/LEVEL_NOT_FOUND response if the package doesn't exist, or (2) if
asset-creation is intended, create the map package before calling
World->AddStreamingLevel and McpSafeAssetSave; also ensure ParentLevel is used
to pick the correct World (instead of always using World) when adding the
streaming level. Use symbols: ParentLevel, SublevelPath,
SanitizeProjectRelativePath, FPackageName::DoesPackageExist, World (and
World->GetOutermost/GetMapName), StreamingLevel, World->AddStreamingLevel, and
McpSafeAssetSave to locate and fix the logic.

404-447: ⚠️ Potential issue | 🔴 Critical

Don't garbage-collect the save=false result.

Because bSaveSucceeded defaults to true, this teardown also runs when the caller explicitly asked not to save. The handler then reports success even though the new world/package were marked transient and collected.

Minimal fix
-    if (bSaveSucceeded && NewWorld)
+    if (bSave && bSaveSucceeded && NewWorld)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_LevelStructureHandlers.cpp`
around lines 404 - 447, The teardown currently gated only by bSaveSucceeded
causes worlds to be marked transient and GC'd even when the caller passed
save=false; update HandleCreateLevel to only run the destruction/GC block when
the save was actually requested and succeeded (e.g. add an explicit check for
the incoming save request flag alongside bSaveSucceeded: if (bSaveRequested &&
bSaveSucceeded && NewWorld) ), leaving NewWorld and Package untouched when save
was false, and keep the existing calls/flags (bIsTearingDown, bIsVisible,
UnRegisterTickFunction, SetFlags(RF_Transient), CollectGarbage(...)) inside that
guarded branch so CollectGarbage(GARBAGE_COLLECTION_KEEPFLAGS) is not called for
save=false.
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_SystemControlHandlers.cpp (1)

330-357: ⚠️ Potential issue | 🟠 Major

Restore assetPath sanitization before loading/exporting.

assetPath is now used raw in DoesAssetExist() and LoadAsset(). That drops the same SanitizeProjectRelativePath() guard the other asset handlers still use, so malformed roots/traversal-style inputs now reach the export path unchecked.

🛡️ Suggested fix
     if (ExportPath.IsEmpty()) {
       SendAutomationError(RequestingSocket, RequestId,
                           TEXT("exportPath is required for export"),
                           TEXT("INVALID_ARGUMENT"));
       return true;
     }
+
+    FString SanitizedAssetPath = SanitizeProjectRelativePath(AssetPath);
+    if (SanitizedAssetPath.IsEmpty()) {
+      SendAutomationError(RequestingSocket, RequestId,
+                          TEXT("Invalid assetPath: path traversal or invalid characters detected."),
+                          TEXT("INVALID_PATH"));
+      return true;
+    }

     FString SafeExportPath = SanitizeProjectFilePath(ExportPath);
     if (SafeExportPath.IsEmpty()) {
       SendAutomationError(RequestingSocket, RequestId,
                           FString::Printf(TEXT("Invalid or unsafe export path: %s"), *ExportPath),
@@
-    if (!UEditorAssetLibrary::DoesAssetExist(AssetPath)) {
+    if (!UEditorAssetLibrary::DoesAssetExist(SanitizedAssetPath)) {
       SendAutomationError(RequestingSocket, RequestId,
-                          FString::Printf(TEXT("Asset not found: %s"), *AssetPath),
+                          FString::Printf(TEXT("Asset not found: %s"), *SanitizedAssetPath),
                           TEXT("ASSET_NOT_FOUND"));
       return true;
     }
@@
-    UObject* Asset = UEditorAssetLibrary::LoadAsset(AssetPath);
+    UObject* Asset = UEditorAssetLibrary::LoadAsset(SanitizedAssetPath);
     if (!Asset) {
       SendAutomationError(RequestingSocket, RequestId,
-                          FString::Printf(TEXT("Failed to load asset: %s"), *AssetPath),
+                          FString::Printf(TEXT("Failed to load asset: %s"), *SanitizedAssetPath),
                           TEXT("LOAD_FAILED"));
       return true;
     }
@@
-    FString ExpectedExportPath = ExportDir / FPaths::GetBaseFilename(AssetPath) + TEXT(".") + Extension;
+    FString ExpectedExportPath = ExportDir / FPaths::GetBaseFilename(SanitizedAssetPath) + TEXT(".") + Extension;
@@
-      Result->SetStringField(TEXT("assetPath"), AssetPath);
+      Result->SetStringField(TEXT("assetPath"), SanitizedAssetPath);
@@
-      Result->SetStringField(TEXT("assetPath"), AssetPath);
+      Result->SetStringField(TEXT("assetPath"), SanitizedAssetPath);

As per coding guidelines, "Use SanitizeProjectRelativePath() to validate and sanitize all file paths to prevent directory traversal attacks."

Also applies to: 381-400, 428-493

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_SystemControlHandlers.cpp`
around lines 330 - 357, The assetPath is being used raw for DoesAssetExist() and
LoadAsset(), so restore sanitization by calling SanitizeProjectRelativePath() on
AssetPath and using the sanitized result (e.g., SafeAssetPath or
SafeAssetRelativePath) for existence checks and loading; if
SanitizeProjectRelativePath() returns empty treat it as an invalid/unsafe path
and return via SendAutomationError (same pattern used for ExportPath with
SanitizeProjectFilePath). Update the handlers referenced (where AssetPath is
read and passed to DoesAssetExist() and LoadAsset()) to use the sanitized
variable and apply the same check in the other affected sections (around the
other ranges noted) to prevent directory traversal.
♻️ Duplicate comments (1)
src/tools/handlers/blueprint-handlers.ts (1)

402-409: ⚠️ Potential issue | 🔴 Critical

reparent_blueprint is still TypeScript-only here.

This branch dispatches a new bridge action, but the matching C++ handler/registration is still missing from the plugin side, so the request will fall through to the unknown-action path at runtime. Please land the Unreal-side handler in Plugins/McpAutomationBridge/Source/ in the same PR before exposing this action.

Based on learnings, "Every TypeScript action must have a corresponding C++ handler implementation in the plugin" and "Implement Unreal side actions in the appropriate handler under Plugins/McpAutomationBridge/Source/ and register in UMcpAutomationBridgeSubsystem::InitializeHandlers()."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/handlers/blueprint-handlers.ts` around lines 402 - 409, The
TypeScript action 'reparent_blueprint' is being dispatched from
executeAutomationRequest in blueprint-handlers.ts but lacks a corresponding
Unreal C++ handler; implement a C++ handler in the plugin under
Plugins/McpAutomationBridge/Source/ (naming it to handle the
'reparent_blueprint' action), wire its logic to accept the same params
(blueprintPath, parentClass, timeoutMs), and register that handler in
UMcpAutomationBridgeSubsystem::InitializeHandlers() so the bridge recognizes and
routes 'reparent_blueprint' requests instead of falling through to the
unknown-action path.
🧹 Nitpick comments (5)
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_WidgetAuthoringHandlers.cpp (1)

1372-1430: Pull repeated payload parsing and widget validation into a shared helper function.

The nameslotName fallback, unique-name collision scan, and parent-panel validation pattern is duplicated across all ~30 widget authoring handlers (lines 523, 600, 677, 768, 849, 949, 1039, 1124, 1215, 1372, 1452, 1553, 1651, 1762, 1864, 1996, 2104, 2176, 2248, 2346, 2432, 2521, 2590, and more). Instead of fixing the same issues repeatedly across handlers, extract this into a helper that returns the parsed slot name and validated parent panel, then reuse it for all widget creation actions.

Per coding guidelines, also migrate from manual GetJsonStringField() calls to FJsonObjectConverter for structured JSON parsing.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_WidgetAuthoringHandlers.cpp`
around lines 1372 - 1430, Repeated payload parsing and widget validation (name
-> slotName fallback, unique-name collision scan, parent lookup/UPanelWidget
validation) is duplicated across many handlers (e.g., the handler around
UGridPanel creation using GetJsonStringField, LoadWidgetBlueprint and
SendAutomationError). Extract a shared helper (e.g., ResolveSlotAndParent(const
TSharedPtr<FJsonObject>& Payload, UWidgetBlueprint* WidgetBP, FString&
OutUniqueSlotName, UPanelWidget*& OutParentPanel, FString& OutErrorCode,
FString& OutErrorMessage)) that: parses the payload using FJsonObjectConverter
into a small struct instead of manual GetJsonStringField calls, applies the
name->slotName fallback, generates a collision-free UniqueName by checking
WidgetBP->WidgetTree->FindWidget, validates and casts the ParentSlot to
UPanelWidget, and returns structured error info so callers (the handlers that
call LoadWidgetBlueprint and create widgets) can call SendAutomationError when
the helper indicates failure; then replace the duplicated blocks in each widget
authoring handler to call this helper and use OutUniqueSlotName/OutParentPanel
for widget construction and attachment.
src/types/tool-interfaces.ts (1)

121-124: Expose the new reparent operation on IBlueprintTools too.

The PR adds a blueprint reparent action, but this public interface still has no typed entry point for it. That leaves tools.blueprintTools consumers out of sync with the MCP surface; add a method that mirrors the schema payload (blueprintPath, parentClass, async options).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/types/tool-interfaces.ts` around lines 121 - 124, Add a typed method for
the new reparent action to the IBlueprintTools interface: declare a method (e.g.
reparentBlueprint) on IBlueprintTools with signature reparentBlueprint(params: {
blueprintPath: string; parentClass: string; timeoutMs?: number;
waitForCompletion?: boolean; waitForCompletionTimeoutMs?: number }):
Promise<StandardActionResponse>; ensure you import/keep using
StandardActionResponse and place the method alongside other blueprint methods
(e.g., near addFunction/renameFunction/removeFunction) so consumers of
tools.blueprintTools have a matching typed entry point.
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpKeyValidation.cpp (1)

12-98: Replace the hardcoded key list with engine APIs for better forward compatibility.

This validator's hardcoded TSet<FName> will not reflect keys added to UE in future releases, causing it to reject valid inputs. Since this is editor-only code (K2Node validation), InputCore is available in the build configuration; use FKey(FName(*KeyName)).IsValid() or query EKeys::GetAllKeys() instead to validate against the runtime-registered key set.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpKeyValidation.cpp`
around lines 12 - 98, GetRegisteredKeyNames contains a hardcoded TSet<FName>
that will miss keys added by the engine; remove or stop using
GetRegisteredKeyNames and change IsKeyNameRegistered to validate against the
engine's registered keys instead (e.g. use FKey(FName(*KeyName)).IsValid() or
check membership in EKeys::GetAllKeys()). Update references to
GetRegisteredKeyNames, export no editor-only dependency issues, and ensure
IsKeyNameRegistered returns false for empty strings as before but otherwise
relies on the engine API for forward-compatible validation.
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_ControlHandlers.cpp (1)

3271-3587: Model read_log with structs and FJsonObjectConverter.

Lines 3271-3587 hand-parse every input and hand-serialize several response variants. This is already a lot of schema surface to keep in sync with the TypeScript side; request/response structs would make defaults and field drift much safer.

As per coding guidelines, plugins/McpAutomationBridge/**/*Handlers.cpp: Use FJsonObjectConverter for JSON parsing and struct serialization instead of manual JSON parsing.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_ControlHandlers.cpp`
around lines 3271 - 3587, The handler currently hand-parses request fields and
manually constructs JSON responses (see the parameter parsing block using
LogTypeArg, TailLines, Filter, Severity and the result construction via
Result->Set* and building OutputLines), which should be replaced with
struct-based (de)serialization using FJsonObjectConverter; create a Request
struct (fields: logType, tail, filter, severity) and Response structs (for file
listing, crash summary, crash_callstack, and normal log result) matching the
TypeScript schema, use FJsonObjectConverter::JsonObjectToUStruct to populate the
Request from Payload, and use FJsonObjectConverter::UStructToJsonObject (or
ToJsonObjectString then Parse) to populate the Result TSharedPtr<FJsonObject>
instead of manual SetStringField/SetNumberField calls, keeping the existing
logic for reading files (TargetFile, bIsCrashXml, RawContent) but replace all
manual JSON assembly with struct serialization and wiring the
SendAutomationResponse calls to return the serialized Result.
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp (1)

934-938: Consider typed payload structs for the new node-specific branches.

These additions introduce several small ad hoc schemas with repeated TryGet*Field parsing and custom required-field checks. A request struct per node family plus FJsonObjectConverter would make the TS/C++ contract much easier to keep in sync. As per coding guidelines, Use FJsonObjectConverter for JSON parsing and struct serialization instead of manual JSON parsing.

Also applies to: 1023-1034, 1056-1079, 1150-1152

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp`
around lines 934 - 938, Replace the ad-hoc JSON parsing around
Payload->TryGetStringField and ActionPath with typed request structs and
FJsonObjectConverter deserialization: define small FStructs for each node family
(e.g., FNodeActionRequest with FString InputActionPath and FString MemberName)
and use FJsonObjectConverter::JsonObjectToUStruct to populate them from the
incoming FJsonObject, then validate required fields on the struct (falling back
from InputActionPath to MemberName in the struct validation logic) instead of
repeating Payload->TryGetStringField checks; apply the same pattern to the other
branches flagged (lines ~1023-1034, 1056-1079, 1150-1152) so callers use the
typed structs and unified validation rather than manual field extraction.
🤖 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_BlueprintGraphHandlers.cpp`:
- Around line 1968-1978: Add defensive null checks before iterating and when
using each node: verify TargetGraph is non-null and TargetGraph->Nodes has
elements before entering the for loop, and inside the loop skip any null Node
pointers (e.g. if (Node == nullptr) continue;). Apply the same guard in the
other similar block around lines 1994-2000. This prevents dereferencing
stale/null UEdGraphNode* while still building results with
McpHandlerUtils::CreateResultObject(), M->Set... and
Matches.Add(MakeShared<FJsonValueObject>(M)).
- Around line 1072-1115: The handler creates and persists a node even when the
target subsystem class isn't resolved (EISubsysClass for
GetEnhancedInputSubsystem and the similar variable in GetWorldSubsystem),
leaving SubsystemClass unset; update the logic to fail fast: resolve the target
UClass (match fully-qualified path or use proper UObject/Class lookup) before
calling NewObject<UEdGraphNode> (or if you must create the node earlier,
immediately remove it from TargetGraph, call TargetGraph->RemoveNode(NewNode)
and undo/save appropriately), then call SendAutomationError(RequestingSocket,
RequestId, ...) and return true when class resolution fails; ensure you
reference GetSubsystemNodeClass and the "SubsystemClass" property when checking
and avoid persisting or returning success for unconfigured NewNode, and mark/
save the Blueprint only on success.
- Around line 967-994: The code assigns InputActionObj into an FObjectProperty
without validating the property's expected class or handling Blueprint asset
paths; update the EINode FObjectProperty branch (the CastField<FObjectProperty>
block in McpAutomationBridge_BlueprintGraphHandlers.cpp) to first check
Prop->PropertyClass is non-null and that
InputActionObj->GetClass()->IsChildOf(Prop->PropertyClass) (or
Prop->PropertyClass->IsChildOf(InputActionObj->GetClass()) as appropriate), and
only then call Prop->SetObjectPropertyValue; also add the same Blueprint
fallback used in the FClassProperty branch (iterate TObjectIterator<UBlueprint>
or resolve generated class equivalents) to resolve Blueprint asset paths to
their generated UClass/UObject before assignment so Blueprint-created
InputAction assets are handled correctly.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintHandlers.cpp`:
- Around line 4654-4658: The code defaults bSaveAfterCompile to true even when
callers send non-boolean values because TryGetBoolField silently leaves the
default on parse failure; instead, when
LocalPayload->HasField(TEXT("saveAfterCompile")) or HasField(TEXT("save")) is
true, explicitly validate the JSON field type (e.g., retrieve the field via
GetField/FindField and confirm EJson::Boolean) and only set bSaveAfterCompile
when the field is actually a boolean; if the field exists but is not a boolean,
reject the request (return an error path) following the same explicit
validation/error handling pattern used by the other mutating handlers in this
file rather than silently defaulting to persistence.
- Around line 5880-5888: The response currently hardcodes compiled=true, omits
the save outcome, and uses oldParent/newParent field names; update the payload
to return the actual reparent result contract by populating Result with the real
compile and save outcomes (e.g., use the actual compile flag variable instead of
true and include a saved/dirty status or save result field), include both
oldParentClass/newParentClass (use OldParentClass->GetName() and
NewParentClass->GetName()) and their paths (OldParentClass->GetPathName(),
NewParentClass->GetPathName()), and preserve any existing verification via
McpHandlerUtils::AddVerification before calling SendAutomationResponse so
callers can verify persistence.
- Around line 5869-5878: Replace the direct ParentClass assignment and manual
structural mark with the engine reparent helper: call
UBlueprintEditorLibrary::ReparentBlueprint(Blueprint, NewParentClass) instead of
assigning Blueprint->ParentClass and
FBlueprintEditorUtils::MarkBlueprintAsStructurallyModified(Blueprint); then
continue with FKismetEditorUtilities::CompileBlueprint(Blueprint) and
SaveLoadedAssetThrottled(Blueprint, -1.0, true). Ensure you remove the direct
assignment/MarkBlueprintAsStructurallyModified calls so the reparent helper can
perform validation and fix-ups.
- Around line 2761-2775: When a variablePinType is explicitly provided but
ResolveUClass(SubClassName) fails, do not silently set
PinType.PinSubCategoryObject to UObject::StaticClass(); instead call
SendAutomationError(RequestingSocket, RequestId, FString::Printf(TEXT("Could not
resolve class '%s'"), *SubClassName), TEXT("CLASS_NOT_FOUND")) and return true
(mirroring the variableType handling that checks FoundClass), and apply the same
change to the class-type handling block (the code around ResolveUClass,
PinType.PinSubCategoryObject, and the fallback to UObject::StaticClass()) so
explicit type resolution failures are reported rather than degraded.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_ConsoleCommandHandlers.cpp`:
- Around line 299-302: The C++ handler is now populating a per-command "results"
array via Result->SetArrayField(TEXT("results")), but the TypeScript helper that
parses batch_console_commands responses and the executeBatchConsoleCommands()
callsite still expect only counts and drop "results"; update the TS response
schema/parser used by batch_console_commands (and the
executeBatchConsoleCommands() return type) to include a results: any[] (or
typed) field and ensure the parser returns that array instead of discarding it
so callers receive the per-command outputs.
- Around line 112-159: ExecWithCapture currently ignores the bool returned by
GEngine->Exec (via FMcpOutputCapture) so callers (both the batch loop and
single-command path) think every non-blocked command succeeded; modify
ExecWithCapture to capture and return the handled status alongside the captured
lines (e.g., change signature to return a struct or pair containing
TArray<FString> Output and bool bHandled), update calls in the batch loop (where
failedCount is computed) to increment failedCount when bHandled is false, and
update the single-command path to treat bHandled==false as an unhandled/failed
command; update all call sites (references to ExecWithCapture, the batch logic
that inspects failedCount, and the single-command handling) to use the new
return shape so Exec()'s handled status is preserved and propagated.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_ControlHandlers.cpp`:
- Around line 3417-3428: The code currently reads the first MaxBytes of the file
into Bytes (using FileHandle->Read) which causes tail requests to return stale
data when files exceed 32MB; change the logic around FileHandle->Size(),
ReadSize and the read to instead read the last MaxBytes bytes: compute Offset =
FMath::Max<int64>(0, TotalSize - MaxBytes), set ReadSize = TotalSize - Offset,
call FileHandle->Seek(Offset) (or SetPosition) before
FileHandle->Read(Bytes.GetData(), ReadSize), then null-terminate and convert to
RawContent as before (affecting the block that fills Bytes, RawContent, and the
later tail handling around the same logic).
- Around line 3298-3355: The handler is returning absolute filesystem paths
(MainLog, BackupLog, CrashXml, LatestCrashDir, LogsDir, CrashesDir) in the JSON
(files[*].path, files[*].crashDir, logsDir, crashesDir and error messages);
change it to return only relative paths or sanitized filenames instead: compute
and store just the filename or path relative to LogsDir/CrashesDir (use
FPaths::GetCleanFilename or FPaths::MakePathRelativeTo/relative logic) when
building each TSharedPtr<FJsonObject> and when setting Result fields, and ensure
SendAutomationResponse/error messages do not include the full absolute paths;
update uses in the same function (the "all" handler) and the similar blocks
referenced (around the symbols handling file reads at lines noted) to uniformly
redact/convert absolute paths before adding to JSON.
- Around line 3195-3237: Capture the boolean return value from
GEditor->Exec(World, *Command, DirectCapture) (e.g., bool bHandled =
GEditor->Exec(...)) and use it when building the response instead of always
sending success=true; if bHandled is false, call SendAutomationResponse(Socket,
RequestId, false, ...) with an appropriate error message (similar to other
handlers) and include the Resp payload only on success/when useful; update any
local logic that assumes success (e.g., the Msg construction and success flag
passed to SendAutomationResponse) so SendAutomationResponse reflects the actual
command handling result.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_LevelStructureHandlers.cpp`:
- Around line 319-327: The current branch that checks bCreateWorldPartition
creates a non-World-Partition level and then sets
bWorldPartitionActuallyEnabled=false, which means bCreateWorldPartition is
advertised but not honored; update the logic so that if bCreateWorldPartition is
true and the code path cannot actually enable World Partition (as indicated by
HandleEnableWorldPartition() refusing later), the operation is rejected
up-front: in the function that handles level creation (references:
bCreateWorldPartition, NewWorld, AWorldSettings, bWorldPartitionActuallyEnabled,
and HandleEnableWorldPartition()), detect that World Partition cannot be fully
enabled and return failure (or clear bCreateWorldPartition and log an explicit
error) instead of returning success; ensure the early-return/failure path
propagates so callers know the requested option was not honored.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_WidgetAuthoringHandlers.cpp`:
- Around line 537-545: The code uniquifies the widget name into UniqueName but
continues to use the original SlotName when reporting/returning the created
widget, causing callers to receive the pre-collision name; update the SlotName
(or the value returned/reported) to the final UniqueName before calling
WidgetTree->ConstructWidget (e.g. assign SlotName = UniqueName immediately after
the uniquification loop and before UCanvasPanel* CanvasPanel =
WidgetBP->WidgetTree->ConstructWidget(...)), and apply the same change to the
other uniquified creation branches (the other ConstructWidget calls that perform
similar uniquification).
- Around line 705-717: The manual visibility mapping block should be replaced
with the existing GetVisibility helper to avoid missing cases (like
SelfHitTestInvisible); call GetVisibility with the Payload and the "visibility"
key to obtain an ESlateVisibility and pass that directly to VBox->SetVisibility
(remove the string comparisons and the manual mapping around GetJsonStringField
and use GetVisibility instead).

In `@src/tools/handlers/blueprint-handlers.ts`:
- Around line 547-562: The unwrap function currently descends whenever it sees a
result or data object and can strip legitimate blueprint fields; change unwrap
to only descend when the current object is actually a bridge envelope (check
that current.type === 'automation_response' or other explicit bridge marker)
before replacing current with current.result or current.data, preserving sibling
fields otherwise; update the same logic in the other occurrence (the duplicate
unwrap at 564-575) so both only unwrap when the envelope type matches the bridge
format.
- Around line 220-223: The rename branch calls executeAutomationRequest for
'blueprint_rename_event' but prefers name over path, causing wrong target
selection; update the resolution order to use blueprintPath || (argsRecord.path
as string) || argsTyped.name for both blueprintCandidates and requestedPath so
it matches the other branches (reference the executeAutomationRequest call,
'blueprint_rename_event', blueprintCandidates, requestedPath,
argsTyped.blueprintPath, argsRecord.path, argsTyped.name).

In `@src/tools/handlers/system-handlers.ts`:
- Around line 646-656: The TypeScript handler is calling
executeAutomationRequest with top-level action 'read_log', but the C++ bridge
only registers 'system_control' and dispatches sub-actions inside
HandleSystemControlAction; change the call in system-handlers.ts to send action:
'system_control' (instead of 'read_log') and include the sub-action/type (e.g.
'read_log' or 'get_logs') in the args payload following the same pattern used by
other system_control sub-actions, or alternatively add a new C++
InitializeHandlers() registration for 'read_log' that forwards to
HandleReadLog(); update the executeAutomationRequest invocation to match the
chosen pattern so the bridge can find and handle the request.

In `@src/utils/response-validator.ts`:
- Around line 99-106: The current heuristic inside
src/utils/response-validator.ts (within the isRecord(val) branch) treats any
small object with one transform-like key as a 3D vector and flattens it (e.g.,
val -> `[${x}, ${y}, ${z}]`), which loses other fields like name; tighten the
check by computing the intersection between Object.keys(val) and the known
transform keys ['x','y','z','pitch','yaw','roll'] and only treat val as a
vector/rotator if at least two transform keys are present AND there are no
additional non-transform keys (or alternatively require all keys to be a subset
of the transform set); update the conditional that currently uses keys.length <=
3 && keys.some(...) and keep the rest of the logic that builds x/y/z unchanged.

---

Outside diff comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_LevelStructureHandlers.cpp`:
- Around line 513-530: The synthesized SublevelPath that is built from
World->GetOutermost()->GetName() and SublevelName must also be validated with
SanitizeProjectRelativePath; after the branch that sets SublevelPath =
FPaths::GetPath(WorldPath) / SublevelName call SanitizeProjectRelativePath on
the result, check for an empty return, and if empty use
Subsystem->SendAutomationResponse(Socket, RequestId, false,
FString::Printf(TEXT("Invalid or unsafe sublevel path: %s"), *SublevelPath),
nullptr, TEXT("SECURITY_VIOLATION")) and return true; only replace SublevelPath
with the sanitized value when SanitizeProjectRelativePath succeeds.
- Around line 492-567: The handler currently adds a ULevelStreamingDynamic
(StreamingLevel) pointing at SublevelPath without ensuring the target map
package exists and without using ParentLevel to select which world to modify,
producing dangling references; update the flow in the function handling
ParentLevel/SublevelPath so that you (1) resolve and validate the actual package
name with FPackageName::DoesPackageExist for SublevelPath (use the same
normalization used for ParentLevel), rejecting the request with a
SECURITY/LEVEL_NOT_FOUND response if the package doesn't exist, or (2) if
asset-creation is intended, create the map package before calling
World->AddStreamingLevel and McpSafeAssetSave; also ensure ParentLevel is used
to pick the correct World (instead of always using World) when adding the
streaming level. Use symbols: ParentLevel, SublevelPath,
SanitizeProjectRelativePath, FPackageName::DoesPackageExist, World (and
World->GetOutermost/GetMapName), StreamingLevel, World->AddStreamingLevel, and
McpSafeAssetSave to locate and fix the logic.
- Around line 404-447: The teardown currently gated only by bSaveSucceeded
causes worlds to be marked transient and GC'd even when the caller passed
save=false; update HandleCreateLevel to only run the destruction/GC block when
the save was actually requested and succeeded (e.g. add an explicit check for
the incoming save request flag alongside bSaveSucceeded: if (bSaveRequested &&
bSaveSucceeded && NewWorld) ), leaving NewWorld and Package untouched when save
was false, and keep the existing calls/flags (bIsTearingDown, bIsVisible,
UnRegisterTickFunction, SetFlags(RF_Transient), CollectGarbage(...)) inside that
guarded branch so CollectGarbage(GARBAGE_COLLECTION_KEEPFLAGS) is not called for
save=false.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_SystemControlHandlers.cpp`:
- Around line 330-357: The assetPath is being used raw for DoesAssetExist() and
LoadAsset(), so restore sanitization by calling SanitizeProjectRelativePath() on
AssetPath and using the sanitized result (e.g., SafeAssetPath or
SafeAssetRelativePath) for existence checks and loading; if
SanitizeProjectRelativePath() returns empty treat it as an invalid/unsafe path
and return via SendAutomationError (same pattern used for ExportPath with
SanitizeProjectFilePath). Update the handlers referenced (where AssetPath is
read and passed to DoesAssetExist() and LoadAsset()) to use the sanitized
variable and apply the same check in the other affected sections (around the
other ranges noted) to prevent directory traversal.

---

Duplicate comments:
In `@src/tools/handlers/blueprint-handlers.ts`:
- Around line 402-409: The TypeScript action 'reparent_blueprint' is being
dispatched from executeAutomationRequest in blueprint-handlers.ts but lacks a
corresponding Unreal C++ handler; implement a C++ handler in the plugin under
Plugins/McpAutomationBridge/Source/ (naming it to handle the
'reparent_blueprint' action), wire its logic to accept the same params
(blueprintPath, parentClass, timeoutMs), and register that handler in
UMcpAutomationBridgeSubsystem::InitializeHandlers() so the bridge recognizes and
routes 'reparent_blueprint' requests instead of falling through to the
unknown-action path.

---

Nitpick comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp`:
- Around line 934-938: Replace the ad-hoc JSON parsing around
Payload->TryGetStringField and ActionPath with typed request structs and
FJsonObjectConverter deserialization: define small FStructs for each node family
(e.g., FNodeActionRequest with FString InputActionPath and FString MemberName)
and use FJsonObjectConverter::JsonObjectToUStruct to populate them from the
incoming FJsonObject, then validate required fields on the struct (falling back
from InputActionPath to MemberName in the struct validation logic) instead of
repeating Payload->TryGetStringField checks; apply the same pattern to the other
branches flagged (lines ~1023-1034, 1056-1079, 1150-1152) so callers use the
typed structs and unified validation rather than manual field extraction.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_ControlHandlers.cpp`:
- Around line 3271-3587: The handler currently hand-parses request fields and
manually constructs JSON responses (see the parameter parsing block using
LogTypeArg, TailLines, Filter, Severity and the result construction via
Result->Set* and building OutputLines), which should be replaced with
struct-based (de)serialization using FJsonObjectConverter; create a Request
struct (fields: logType, tail, filter, severity) and Response structs (for file
listing, crash summary, crash_callstack, and normal log result) matching the
TypeScript schema, use FJsonObjectConverter::JsonObjectToUStruct to populate the
Request from Payload, and use FJsonObjectConverter::UStructToJsonObject (or
ToJsonObjectString then Parse) to populate the Result TSharedPtr<FJsonObject>
instead of manual SetStringField/SetNumberField calls, keeping the existing
logic for reading files (TargetFile, bIsCrashXml, RawContent) but replace all
manual JSON assembly with struct serialization and wiring the
SendAutomationResponse calls to return the serialized Result.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_WidgetAuthoringHandlers.cpp`:
- Around line 1372-1430: Repeated payload parsing and widget validation (name ->
slotName fallback, unique-name collision scan, parent lookup/UPanelWidget
validation) is duplicated across many handlers (e.g., the handler around
UGridPanel creation using GetJsonStringField, LoadWidgetBlueprint and
SendAutomationError). Extract a shared helper (e.g., ResolveSlotAndParent(const
TSharedPtr<FJsonObject>& Payload, UWidgetBlueprint* WidgetBP, FString&
OutUniqueSlotName, UPanelWidget*& OutParentPanel, FString& OutErrorCode,
FString& OutErrorMessage)) that: parses the payload using FJsonObjectConverter
into a small struct instead of manual GetJsonStringField calls, applies the
name->slotName fallback, generates a collision-free UniqueName by checking
WidgetBP->WidgetTree->FindWidget, validates and casts the ParentSlot to
UPanelWidget, and returns structured error info so callers (the handlers that
call LoadWidgetBlueprint and create widgets) can call SendAutomationError when
the helper indicates failure; then replace the duplicated blocks in each widget
authoring handler to call this helper and use OutUniqueSlotName/OutParentPanel
for widget construction and attachment.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpKeyValidation.cpp`:
- Around line 12-98: GetRegisteredKeyNames contains a hardcoded TSet<FName> that
will miss keys added by the engine; remove or stop using GetRegisteredKeyNames
and change IsKeyNameRegistered to validate against the engine's registered keys
instead (e.g. use FKey(FName(*KeyName)).IsValid() or check membership in
EKeys::GetAllKeys()). Update references to GetRegisteredKeyNames, export no
editor-only dependency issues, and ensure IsKeyNameRegistered returns false for
empty strings as before but otherwise relies on the engine API for
forward-compatible validation.

In `@src/types/tool-interfaces.ts`:
- Around line 121-124: Add a typed method for the new reparent action to the
IBlueprintTools interface: declare a method (e.g. reparentBlueprint) on
IBlueprintTools with signature reparentBlueprint(params: { blueprintPath:
string; parentClass: string; timeoutMs?: number; waitForCompletion?: boolean;
waitForCompletionTimeoutMs?: number }): Promise<StandardActionResponse>; ensure
you import/keep using StandardActionResponse and place the method alongside
other blueprint methods (e.g., near addFunction/renameFunction/removeFunction)
so consumers of tools.blueprintTools have a matching typed entry point.
🪄 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: 9b2bdf03-a963-4939-be14-542be76591cc

📥 Commits

Reviewing files that changed from the base of the PR and between 28e1c4a and 80b5ae8.

📒 Files selected for processing (22)
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeHelpers.h
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintHandlers.cpp
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_ConsoleCommandHandlers.cpp
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_ControlHandlers.cpp
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_LevelStructureHandlers.cpp
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_SystemControlHandlers.cpp
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_WidgetAuthoringHandlers.cpp
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpHandlerUtils.cpp
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpKeyValidation.cpp
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpSafeOperations.h
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Public/McpAutomationBridgeSubsystem.h
  • src/tools/blueprint.ts
  • src/tools/consolidated-tool-definitions.ts
  • src/tools/consolidated-tool-handlers.ts
  • src/tools/handlers/blueprint-handlers.ts
  • src/tools/handlers/graph-handlers.ts
  • src/tools/handlers/system-handlers.ts
  • src/types/handler-types.ts
  • src/types/tool-interfaces.ts
  • src/utils/response-validator.ts
💤 Files with no reviewable changes (2)
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpSafeOperations.h
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeHelpers.h
✅ Files skipped from review due to trivial changes (3)
  • src/types/handler-types.ts
  • src/tools/handlers/graph-handlers.ts
  • plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PipelineHandlers.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/tools/consolidated-tool-definitions.ts

Comment on lines +967 to +994
UObject *InputActionObj = StaticLoadObject(
UObject::StaticClass(), nullptr, *ActionPath);
if (!InputActionObj) {
SendAutomationError(
RequestingSocket, RequestId,
FString::Printf(
TEXT("InputAction asset not found at '%s'. "
"Verify the path, e.g. /Game/Input/Actions/IA_Move."),
*ActionPath),
TEXT("ASSET_NOT_FOUND"));
return true;
}
// Create the node via NewObject (same as dynamic fallback but with
// InputAction set BEFORE AllocateDefaultPins so the ActionValue pin
// gets the correct type — Bool/Axis1D/Axis2D/Axis3D)
UEdGraphNode *EINode = NewObject<UEdGraphNode>(TargetGraph, EINodeClass);
if (!EINode) {
SendAutomationError(RequestingSocket, RequestId,
TEXT("Failed to instantiate K2Node_EnhancedInputAction."),
TEXT("CREATE_FAILED"));
return true;
}
// Set InputAction via FObjectProperty reflection — no typed header needed
if (FObjectProperty *Prop = CastField<FObjectProperty>(
EINodeClass->FindPropertyByName(TEXT("InputAction")))) {
Prop->SetObjectPropertyValue(
Prop->ContainerPtrToValuePtr<void>(EINode), InputActionObj);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

wc -l plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp

Repository: ChiR24/Unreal_mcp

Length of output: 175


🏁 Script executed:

sed -n '950,1010p' plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp

Repository: ChiR24/Unreal_mcp

Length of output: 2966


🏁 Script executed:

sed -n '1620,1680p' plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp

Repository: ChiR24/Unreal_mcp

Length of output: 3198


🏁 Script executed:

rg -n "PropertyClass|MetaClass" plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp -A 3 -B 3

Repository: ChiR24/Unreal_mcp

Length of output: 43


🏁 Script executed:

rg -n "FObjectProperty.*PropertyClass\|PropertyClass.*FObjectProperty" plugins/McpAutomationBridge/Source/ -A 5

Repository: ChiR24/Unreal_mcp

Length of output: 43


🏁 Script executed:

rg -n "IsA\|IsChildOf" plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp

Repository: ChiR24/Unreal_mcp

Length of output: 43


🏁 Script executed:

rg -n "BP_.*_C\|Blueprint" plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp | head -20

Repository: ChiR24/Unreal_mcp

Length of output: 43


🏁 Script executed:

sed -n '1600,1700p' plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp

Repository: ChiR24/Unreal_mcp

Length of output: 5275


🏁 Script executed:

rg -n "SendAutomationError" plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp | head -10

Repository: ChiR24/Unreal_mcp

Length of output: 482


🏁 Script executed:

rg -n "Cast\|IsA" plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/ -t cpp | grep -i "object\|asset" | head -15

Repository: ChiR24/Unreal_mcp

Length of output: 43


🏁 Script executed:

grep -n "cast branch\|Cast.*FClassProperty\|FClassProperty.*Cast" plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp

Repository: ChiR24/Unreal_mcp

Length of output: 390


🏁 Script executed:

sed -n '1090,1120p' plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp

Repository: ChiR24/Unreal_mcp

Length of output: 1672


🏁 Script executed:

sed -n '1175,1210p' plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp

Repository: ChiR24/Unreal_mcp

Length of output: 1789


🏁 Script executed:

rg -n "PropertyClass" plugins/McpAutomationBridge/Source/ -B 2 -A 2

Repository: ChiR24/Unreal_mcp

Length of output: 1663


🏁 Script executed:

grep -n "StaticLoadObject\|LoadClass" plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp

Repository: ChiR24/Unreal_mcp

Length of output: 353


🏁 Script executed:

sed -n '930,970p' plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp

Repository: ChiR24/Unreal_mcp

Length of output: 1857


Add PropertyClass validation before assigning typed object properties.

Both code sections load UObject values and assign them directly to typed FObjectProperty fields without validating that the loaded object matches the property's expected class. This allows incompatible assets to be assigned silently and will fail at runtime.

The pattern used in McpAutomationBridge_AIHandlers.cpp (lines 595, 694) shows the correct approach:

if (ObjProp && ObjProp->PropertyClass && ObjProp->PropertyClass->IsChildOf(UBehaviorTree::StaticClass()))

Additionally, the FClassProperty branch (lines 1635–1641) includes a fallback TObjectIterator loop to handle Blueprint asset paths, but the FObjectProperty branch (line 1650) lacks this fallback and only uses StaticLoadObject(), making Blueprint asset paths unresolvable in that path.

Apply the same validation pattern and Blueprint fallback to both sections before 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_BlueprintGraphHandlers.cpp`
around lines 967 - 994, The code assigns InputActionObj into an FObjectProperty
without validating the property's expected class or handling Blueprint asset
paths; update the EINode FObjectProperty branch (the CastField<FObjectProperty>
block in McpAutomationBridge_BlueprintGraphHandlers.cpp) to first check
Prop->PropertyClass is non-null and that
InputActionObj->GetClass()->IsChildOf(Prop->PropertyClass) (or
Prop->PropertyClass->IsChildOf(InputActionObj->GetClass()) as appropriate), and
only then call Prop->SetObjectPropertyValue; also add the same Blueprint
fallback used in the FClassProperty branch (iterate TObjectIterator<UBlueprint>
or resolve generated class equivalents) to resolve Blueprint asset paths to
their generated UClass/UObject before assignment so Blueprint-created
InputAction assets are handled correctly.

Comment on lines +1072 to +1115
UClass *EISubsysClass = nullptr;
for (TObjectIterator<UClass> It; It; ++It) {
if (It->GetName().Equals(TEXT("EnhancedInputLocalPlayerSubsystem"),
ESearchCase::IgnoreCase)) {
EISubsysClass = *It;
break;
}
}
UEdGraphNode *NewNode =
NewObject<UEdGraphNode>(TargetGraph, GetSubsystemNodeClass);
if (!NewNode) {
SendAutomationError(RequestingSocket, RequestId,
TEXT("Failed to create GetSubsystem node."),
TEXT("CREATE_FAILED"));
return true;
}
TargetGraph->AddNode(NewNode, false, false);
NewNode->CreateNewGuid();
if (EISubsysClass) {
FProperty *ClassProp =
GetSubsystemNodeClass->FindPropertyByName(TEXT("SubsystemClass"));
if (FClassProperty *CProp = CastField<FClassProperty>(ClassProp)) {
void *PropPtr = CProp->ContainerPtrToValuePtr<void>(NewNode);
CProp->SetPropertyValue(PropPtr, EISubsysClass);
}
}
NewNode->PostPlacedNewNode();
NewNode->AllocateDefaultPins();
// ReconstructNode forces pin regeneration now that SubsystemClass is set,
// ensuring ReturnValue is typed as EnhancedInputLocalPlayerSubsystem.
NewNode->ReconstructNode();
NewNode->NodePosX = X;
NewNode->NodePosY = Y;
FBlueprintEditorUtils::MarkBlueprintAsModified(Blueprint);
SaveLoadedAssetThrottled(Blueprint, -1.0, true);
TSharedPtr<FJsonObject> Result = McpHandlerUtils::CreateResultObject();
Result->SetStringField(TEXT("nodeId"), NewNode->NodeGuid.ToString());
Result->SetStringField(TEXT("nodeName"), NewNode->GetName());
Result->SetStringField(TEXT("nodeClass"), GetSubsystemNodeClass->GetName());
FText TitleText = NewNode->GetNodeTitle(ENodeTitleType::FullTitle);
Result->SetStringField(TEXT("nodeTitle"), TitleText.ToString());
if (EISubsysClass) {
Result->SetStringField(TEXT("subsystemClass"), EISubsysClass->GetPathName());
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp | sed -n '1050,1220p'

Repository: ChiR24/Unreal_mcp

Length of output: 9043


🏁 Script executed:

# Verify SaveLoadedAssetThrottled behavior and confirm it saves regardless of success/failure
rg -A 10 "SaveLoadedAssetThrottled" plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp | head -20

Repository: ChiR24/Unreal_mcp

Length of output: 901


🏁 Script executed:

# Check if there are any defensive measures elsewhere that might mitigate this issue
rg "SubsystemClass" plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp | head -20

Repository: ChiR24/Unreal_mcp

Length of output: 646


Fail hard when subsystem class resolution fails; don't persist unconfigured nodes.

Both GetEnhancedInputSubsystem (lines 1072–1119) and GetWorldSubsystem (lines 1150–1205) create and save the node even when the target class is missing. The class lookup only matches by short name (GetName().Equals()), so a fully-qualified class name cannot be resolved. When the class is not found, the node's SubsystemClass property remains unset, yet the node persists in the blueprint and a success response is sent. This should fail instead—either validate that the class exists before creating the node, or delete it and return an error if resolution fails.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp`
around lines 1072 - 1115, The handler creates and persists a node even when the
target subsystem class isn't resolved (EISubsysClass for
GetEnhancedInputSubsystem and the similar variable in GetWorldSubsystem),
leaving SubsystemClass unset; update the logic to fail fast: resolve the target
UClass (match fully-qualified path or use proper UObject/Class lookup) before
calling NewObject<UEdGraphNode> (or if you must create the node earlier,
immediately remove it from TargetGraph, call TargetGraph->RemoveNode(NewNode)
and undo/save appropriately), then call SendAutomationError(RequestingSocket,
RequestId, ...) and return true when class resolution fails; ensure you
reference GetSubsystemNodeClass and the "SubsystemClass" property when checking
and avoid persisting or returning success for unconfigured NewNode, and mark/
save the Blueprint only on success.

Comment on lines +1968 to +1978
for (UEdGraphNode *Node : TargetGraph->Nodes) {
FString Title = Node->GetNodeTitle(ENodeTitleType::ListView).ToString();
if (TitleFilter.IsEmpty() || Title.ToLower().Contains(TitleFilter)) {
TSharedPtr<FJsonObject> M = McpHandlerUtils::CreateResultObject();
M->SetStringField(TEXT("nodeId"), Node->NodeGuid.ToString());
M->SetStringField(TEXT("nodeName"), Node->GetName());
M->SetStringField(TEXT("nodeTitle"), Title);
M->SetNumberField(TEXT("x"), Node->NodePosX);
M->SetNumberField(TEXT("y"), Node->NodePosY);
Matches.Add(MakeShared<FJsonValueObject>(M));
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Null-check TargetGraph->Nodes in the new scan helpers.

The rest of this file treats graph-node arrays defensively, but these two new read-only paths dereference Node immediately. A stale/null slot would turn a diagnostic request into an editor crash.

Suggested guard
 for (UEdGraphNode *Node : TargetGraph->Nodes) {
+  if (!Node) {
+    continue;
+  }
   FString Title = Node->GetNodeTitle(ENodeTitleType::ListView).ToString();
   if (TitleFilter.IsEmpty() || Title.ToLower().Contains(TitleFilter)) {
     ...
   }
 }
 
 for (UEdGraphNode *Node : TargetGraph->Nodes) {
+  if (!Node) {
+    continue;
+  }
   for (UEdGraphPin *Pin : Node->Pins) {
     ...
   }
 }

Also applies to: 1994-2000

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintGraphHandlers.cpp`
around lines 1968 - 1978, Add defensive null checks before iterating and when
using each node: verify TargetGraph is non-null and TargetGraph->Nodes has
elements before entering the for loop, and inside the loop skip any null Node
pointers (e.g. if (Node == nullptr) continue;). Apply the same guard in the
other similar block around lines 1994-2000. This prevents dereferencing
stale/null UEdGraphNode* while still building results with
McpHandlerUtils::CreateResultObject(), M->Set... and
Matches.Add(MakeShared<FJsonValueObject>(M)).

Comment on lines +2761 to +2775
const TSharedPtr<FJsonObject> *PinTypeObj = nullptr;
if (LocalPayload->TryGetObjectField(TEXT("variablePinType"), PinTypeObj) && PinTypeObj) {
FString SubClassName;
if (!(*PinTypeObj)->TryGetStringField(TEXT("PinSubCategoryObject"), SubClassName) || SubClassName.IsEmpty()) {
(*PinTypeObj)->TryGetStringField(TEXT("objectClass"), SubClassName);
}
if (!SubClassName.IsEmpty()) {
if (UClass *SubClass = ResolveUClass(SubClassName)) {
PinType.PinSubCategoryObject = SubClass;
}
}
}
if (!PinType.PinSubCategoryObject.IsValid()) {
PinType.PinSubCategoryObject = UObject::StaticClass();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's see the full context around the flagged lines
wc -l "plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintHandlers.cpp"

Repository: ChiR24/Unreal_mcp

Length of output: 170


🏁 Script executed:

# Read the section around lines 2761-2795 to see the complete code
sed -n '2750,2800p' "plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintHandlers.cpp" | cat -n

Repository: ChiR24/Unreal_mcp

Length of output: 2818


🏁 Script executed:

# Also read the section around 2780-2794 to see what the "also applies to" refers to
sed -n '2770,2800p' "plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintHandlers.cpp" | cat -n

Repository: ChiR24/Unreal_mcp

Length of output: 1511


🏁 Script executed:

# Check if FJsonObjectConverter is used elsewhere in this file for similar operations
rg "FJsonObjectConverter" "plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintHandlers.cpp" -A 3

Repository: ChiR24/Unreal_mcp

Length of output: 411


🏁 Script executed:

# Check imports in the file to see what JSON handling utilities are already in use
head -n 100 "plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintHandlers.cpp" | grep -E "#include|using"

Repository: ChiR24/Unreal_mcp

Length of output: 1344


🏁 Script executed:

# Find the ResolveUClass function implementation
rg "ResolveUClass" "plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintHandlers.cpp" -B 3 -A 10 | head -80

Repository: ChiR24/Unreal_mcp

Length of output: 1474


🏁 Script executed:

# Check if there's error logging when class resolution fails
rg "ResolveUClass|NOT_FOUND|CLASS_NOT_FOUND|failed to resolve" "plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintHandlers.cpp" -i -B 2 -A 2

Repository: ChiR24/Unreal_mcp

Length of output: 8974


🏁 Script executed:

# Look at the overall handler context - find the function containing these lines
sed -n '2700,2761p' "plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintHandlers.cpp" | tail -70

Repository: ChiR24/Unreal_mcp

Length of output: 2950


🏁 Script executed:

# Check how other similar field parsing is handled - look for patterns with TryGetStringField and fallback
rg "TryGetStringField|TryGetObjectField" "plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintHandlers.cpp" -B 2 -A 5 | head -100

Repository: ChiR24/Unreal_mcp

Length of output: 4188


Ensure variablePinType resolution failures are reported, not silently degraded.

When variablePinType is explicitly provided but ResolveUClass() fails, the code silently falls back to UObject::StaticClass(). This is inconsistent with how the main variableType is handled: when main type resolution fails (lines 2827–2835), a CLASS_NOT_FOUND error is sent. Explicit type requests should fail, not degrade to a generic fallback.

Example: Lines 2761–2775 (object type)
if (UClass *SubClass = ResolveUClass(SubClassName)) {
  PinType.PinSubCategoryObject = SubClass;
}
// If ResolveUClass() returns nullptr, PinSubCategoryObject remains invalid
// and falls back to UObject::StaticClass() on line 2775
if (!PinType.PinSubCategoryObject.IsValid()) {
  PinType.PinSubCategoryObject = UObject::StaticClass();
}

Contrast with main type handling (lines 2827–2835):

UClass *FoundClass = ResolveUClass(VarType);
if (FoundClass) {
  PinType.PinSubCategoryObject = FoundClass;
} else {
  SendAutomationError(
      RequestingSocket, RequestId,
      FString::Printf(TEXT("Could not resolve class '%s'"), *VarType),
      TEXT("CLASS_NOT_FOUND"));
  return true;
}

Also applies to: 2780–2794 (class type).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintHandlers.cpp`
around lines 2761 - 2775, When a variablePinType is explicitly provided but
ResolveUClass(SubClassName) fails, do not silently set
PinType.PinSubCategoryObject to UObject::StaticClass(); instead call
SendAutomationError(RequestingSocket, RequestId, FString::Printf(TEXT("Could not
resolve class '%s'"), *SubClassName), TEXT("CLASS_NOT_FOUND")) and return true
(mirroring the variableType handling that checks FoundClass), and apply the same
change to the class-type handling block (the code around ResolveUClass,
PinType.PinSubCategoryObject, and the fallback to UObject::StaticClass()) so
explicit type resolution failures are reported rather than degraded.

Comment on lines +4654 to +4658
bool bSaveAfterCompile = true;
if (LocalPayload->HasField(TEXT("saveAfterCompile")))
LocalPayload->TryGetBoolField(TEXT("saveAfterCompile"),
bSaveAfterCompile);
LocalPayload->TryGetBoolField(TEXT("saveAfterCompile"), bSaveAfterCompile);
if (LocalPayload->HasField(TEXT("save")))
LocalPayload->TryGetBoolField(TEXT("save"), bSaveAfterCompile);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Reject invalid save flags instead of defaulting to persistence.

These TryGetBoolField() calls ignore parse failures. If a caller sends a non-boolean save/saveAfterCompile, bSaveAfterCompile stays true and the asset is still written to disk. This should follow the explicit validation pattern used by the other mutating handlers above.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_BlueprintHandlers.cpp`
around lines 4654 - 4658, The code defaults bSaveAfterCompile to true even when
callers send non-boolean values because TryGetBoolField silently leaves the
default on parse failure; instead, when
LocalPayload->HasField(TEXT("saveAfterCompile")) or HasField(TEXT("save")) is
true, explicitly validate the JSON field type (e.g., retrieve the field via
GetField/FindField and confirm EJson::Boolean) and only set bSaveAfterCompile
when the field is actually a boolean; if the field exists but is not a boolean,
reject the request (return an error path) following the same explicit
validation/error handling pattern used by the other mutating handlers in this
file rather than silently defaulting to persistence.

Comment on lines +705 to +717
// Apply visibility if provided
FString VisibilityStr = GetJsonStringField(Payload, TEXT("visibility"));
if (!VisibilityStr.IsEmpty())
{
if (VisibilityStr.Equals(TEXT("Collapsed"), ESearchCase::IgnoreCase))
VBox->SetVisibility(ESlateVisibility::Collapsed);
else if (VisibilityStr.Equals(TEXT("Hidden"), ESearchCase::IgnoreCase))
VBox->SetVisibility(ESlateVisibility::Hidden);
else if (VisibilityStr.Equals(TEXT("HitTestInvisible"), ESearchCase::IgnoreCase))
VBox->SetVisibility(ESlateVisibility::HitTestInvisible);
else
VBox->SetVisibility(ESlateVisibility::Visible);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
FILE="plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_WidgetAuthoringHandlers.cpp"
sed -n '301,320p;705,717p' "$FILE"

Repository: ChiR24/Unreal_mcp

Length of output: 1565


Use the existing GetVisibility() helper instead of duplicating logic.

The manual visibility mapping is missing the SelfHitTestInvisible case that the helper function already supports. A payload with visibility: "SelfHitTestInvisible" would incorrectly be set to Visible.

Proposed fix
         FString VisibilityStr = GetJsonStringField(Payload, TEXT("visibility"));
         if (!VisibilityStr.IsEmpty())
         {
-            if (VisibilityStr.Equals(TEXT("Collapsed"), ESearchCase::IgnoreCase))
-                VBox->SetVisibility(ESlateVisibility::Collapsed);
-            else if (VisibilityStr.Equals(TEXT("Hidden"), ESearchCase::IgnoreCase))
-                VBox->SetVisibility(ESlateVisibility::Hidden);
-            else if (VisibilityStr.Equals(TEXT("HitTestInvisible"), ESearchCase::IgnoreCase))
-                VBox->SetVisibility(ESlateVisibility::HitTestInvisible);
-            else
-                VBox->SetVisibility(ESlateVisibility::Visible);
+            VBox->SetVisibility(GetVisibility(VisibilityStr));
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Apply visibility if provided
FString VisibilityStr = GetJsonStringField(Payload, TEXT("visibility"));
if (!VisibilityStr.IsEmpty())
{
if (VisibilityStr.Equals(TEXT("Collapsed"), ESearchCase::IgnoreCase))
VBox->SetVisibility(ESlateVisibility::Collapsed);
else if (VisibilityStr.Equals(TEXT("Hidden"), ESearchCase::IgnoreCase))
VBox->SetVisibility(ESlateVisibility::Hidden);
else if (VisibilityStr.Equals(TEXT("HitTestInvisible"), ESearchCase::IgnoreCase))
VBox->SetVisibility(ESlateVisibility::HitTestInvisible);
else
VBox->SetVisibility(ESlateVisibility::Visible);
}
// Apply visibility if provided
FString VisibilityStr = GetJsonStringField(Payload, TEXT("visibility"));
if (!VisibilityStr.IsEmpty())
{
VBox->SetVisibility(GetVisibility(VisibilityStr));
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_WidgetAuthoringHandlers.cpp`
around lines 705 - 717, The manual visibility mapping block should be replaced
with the existing GetVisibility helper to avoid missing cases (like
SelfHitTestInvisible); call GetVisibility with the Payload and the "visibility"
key to obtain an ESlateVisibility and pass that directly to VBox->SetVisibility
(remove the string comparisons and the manual mapping around GetJsonStringField
and use GetVisibility instead).

Comment on lines +220 to +223
const res = await executeAutomationRequest(tools, 'blueprint_rename_event', {
blueprintCandidates: [argsTyped.blueprintPath || argsTyped.name || (argsRecord.path as string) || ''],
requestedPath: argsTyped.blueprintPath || argsTyped.name || (argsRecord.path as string) || '',
oldName: (argsRecord.oldName as string) ?? '',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Prefer the explicit path over name in rename_event.

Every other blueprint mutation branch resolves the target as blueprintPath || path || name. This one does blueprintPath || name || path, so a call that carries both fields can rename the event on the wrong asset.

♻️ Suggested fix
-        blueprintCandidates: [argsTyped.blueprintPath || argsTyped.name || (argsRecord.path as string) || ''],
-        requestedPath: argsTyped.blueprintPath || argsTyped.name || (argsRecord.path as string) || '',
+        blueprintCandidates: [argsTyped.blueprintPath || (argsRecord.path as string) || argsTyped.name || ''],
+        requestedPath: argsTyped.blueprintPath || (argsRecord.path as string) || argsTyped.name || '',
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/handlers/blueprint-handlers.ts` around lines 220 - 223, The rename
branch calls executeAutomationRequest for 'blueprint_rename_event' but prefers
name over path, causing wrong target selection; update the resolution order to
use blueprintPath || (argsRecord.path as string) || argsTyped.name for both
blueprintCandidates and requestedPath so it matches the other branches
(reference the executeAutomationRequest call, 'blueprint_rename_event',
blueprintCandidates, requestedPath, argsTyped.blueprintPath, argsRecord.path,
argsTyped.name).

Comment on lines +547 to +562
// Unwrap nested automation_response envelopes — the bridge sometimes wraps
// the real payload in { type: 'automation_response', result: { ... } }
const unwrap = (obj: Record<string, unknown>): Record<string, unknown> => {
let current = obj;
let depth = 0;
while (depth < 5) {
const inner = current.result ?? current.data;
if (inner && typeof inner === 'object' && !Array.isArray(inner)) {
current = inner as Record<string, unknown>;
depth++;
} else {
break;
}
}
return current;
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Only unwrap actual bridge envelopes.

The new loop keeps descending whenever the current object has a result or data object. That will also peel legitimate blueprint payloads that happen to expose a data/result field, dropping sibling fields from the returned blueprint.

🔧 Suggested fix
-    const unwrap = (obj: Record<string, unknown>): Record<string, unknown> => {
+    const isEnvelope = (value: Record<string, unknown>): boolean =>
+      value.type === 'automation_response' ||
+      'requestId' in value ||
+      'success' in value ||
+      'message' in value ||
+      'error' in value;
+
+    const unwrap = (obj: Record<string, unknown>): Record<string, unknown> => {
       let current = obj;
       let depth = 0;
-      while (depth < 5) {
+      while (depth < 5 && isEnvelope(current)) {
         const inner = current.result ?? current.data;
         if (inner && typeof inner === 'object' && !Array.isArray(inner)) {
           current = inner as Record<string, unknown>;
           depth++;

Also applies to: 564-575

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/handlers/blueprint-handlers.ts` around lines 547 - 562, The unwrap
function currently descends whenever it sees a result or data object and can
strip legitimate blueprint fields; change unwrap to only descend when the
current object is actually a bridge envelope (check that current.type ===
'automation_response' or other explicit bridge marker) before replacing current
with current.result or current.data, preserving sibling fields otherwise; update
the same logic in the other occurrence (the duplicate unwrap at 564-575) so both
only unwrap when the envelope type matches the bridge format.

Comment on lines 646 to +656
case 'read_log':
return cleanObject(await tools.logTools.readOutputLog(args as Record<string, unknown>));
case 'read_logs':
case 'get_log':
case 'get_logs': {
// All logType variants are routed through the C++ bridge, which locates
// the log file reliably via FPaths::ProjectSavedDir() regardless of the
// Node.js working directory or UE_PROJECT_PATH env var.
const res = await executeAutomationRequest(
tools, 'read_log', args,
'Automation bridge not available for log reading'
) as Record<string, unknown>;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Verify what executeAutomationRequest actually sends and whether read_log
# is registered as a top-level bridge action.
# Expected: either common-handlers wraps this as system_control, or the plugin
# registers a top-level read_log alias in InitializeHandlers().

echo "=== executeAutomationRequest implementation ==="
sed -n '1,220p' src/tools/handlers/common-handlers.ts

echo
echo "=== read_log registrations / dispatch ==="
rg -n -C3 'read_log|get_logs|HandleReadLog|InitializeHandlers|RegisterHandler' \
  src/tools \
  plugins/McpAutomationBridge/Source/McpAutomationBridge

Repository: ChiR24/Unreal_mcp

Length of output: 50374


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Searching for read_log registration in InitializeHandlers ==="
rg -n 'RegisterHandler.*read_log|RegisterHandler.*system_control' \
  plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeSubsystem.cpp

echo
echo "=== Checking ProcessRequest dispatch logic ==="
rg -n -A5 'ProcessRequest|AutomationHandlers\[' \
  plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeSubsystem.cpp \
  plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_ProcessRequest.cpp | head -80

echo
echo "=== Looking for HandleSystemControlAction registration ==="
rg -n 'RegisterHandler.*HandleSystemControlAction|TEXT.*system_control' \
  plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeSubsystem.cpp

Repository: ChiR24/Unreal_mcp

Length of output: 1172


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Checking sendAutomationRequest dispatch in ProcessRequest ==="
rg -n -B2 -A10 'sendAutomationRequest|AutomationHandlers.Find' \
  plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/ | head -100

echo
echo "=== Full system_control handler registration at line 757 ==="
sed -n '757,775p' plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeSubsystem.cpp

Repository: ChiR24/Unreal_mcp

Length of output: 3095


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Check all RegisterHandler calls to confirm no read_log at top level ==="
rg -n 'RegisterHandler.*TEXT' \
  plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeSubsystem.cpp | \
  grep -E '(read_log|get_log)' || echo "No read_log/get_log top-level registrations found"

echo
echo "=== Verify: does HandleSystemControlAction expect sub-action or direct action? ==="
sed -n '40,60p' plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_SystemControlHandlers.cpp

Repository: ChiR24/Unreal_mcp

Length of output: 926


Fix the action name sent to the C++ bridge.

The TypeScript code sends 'read_log' as the top-level action, but the C++ bridge only registers 'system_control' as a handler in InitializeHandlers(). The read_log logic is inside HandleSystemControlAction() as a sub-action check, not a top-level registered action. This will cause the request to fail with "handler not found."

Either:

  1. Register 'read_log' as a direct handler in C++ InitializeHandlers() that calls HandleReadLog(), or
  2. Send action: 'system_control' from TypeScript with the action type embedded in the payload/args (following the pattern for other system_control sub-actions)

Check the architecture pattern used by other system_control sub-actions to determine the correct approach.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/handlers/system-handlers.ts` around lines 646 - 656, The TypeScript
handler is calling executeAutomationRequest with top-level action 'read_log',
but the C++ bridge only registers 'system_control' and dispatches sub-actions
inside HandleSystemControlAction; change the call in system-handlers.ts to send
action: 'system_control' (instead of 'read_log') and include the sub-action/type
(e.g. 'read_log' or 'get_logs') in the args payload following the same pattern
used by other system_control sub-actions, or alternatively add a new C++
InitializeHandlers() registration for 'read_log' that forwards to
HandleReadLog(); update the executeAutomationRequest invocation to match the
chosen pattern so the bridge can find and handle the request.

Comment on lines 99 to 106
if (isRecord(val)) {
const keys = Object.keys(val);
// Check if it looks like a 3D vector/transform
if (keys.some(k => ['x', 'y', 'z', 'pitch', 'yaw', 'roll'].includes(k))) {
if (keys.length <= 3 && keys.some(k => ['x', 'y', 'z', 'pitch', 'yaw', 'roll'].includes(k))) {
const x = val.x ?? val.pitch ?? 0;
const y = val.y ?? val.yaw ?? 0;
const z = val.z ?? val.roll ?? 0;
return `[${x}, ${y}, ${z}]`;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Tighten the vector/rotator heuristic.

Line 102 still treats any small object containing x/pitch-style keys as a transform, so payloads like { x, name } get flattened to [x, 0, 0] and lose data in the summary.

🛠️ Suggested fix
-      if (keys.length <= 3 && keys.some(k => ['x', 'y', 'z', 'pitch', 'yaw', 'roll'].includes(k))) {
+      const vectorKeys = ['x', 'y', 'z'];
+      const rotatorKeys = ['pitch', 'yaw', 'roll'];
+      const isVector =
+        keys.length === 3 &&
+        vectorKeys.every(k => k in val) &&
+        keys.every(k => vectorKeys.includes(k));
+      const isRotator =
+        keys.length === 3 &&
+        rotatorKeys.every(k => k in val) &&
+        keys.every(k => rotatorKeys.includes(k));
+
+      if (isVector || isRotator) {
         const x = val.x ?? val.pitch ?? 0;
         const y = val.y ?? val.yaw ?? 0;
         const z = val.z ?? val.roll ?? 0;
         return `[${x}, ${y}, ${z}]`;
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/response-validator.ts` around lines 99 - 106, The current heuristic
inside src/utils/response-validator.ts (within the isRecord(val) branch) treats
any small object with one transform-like key as a 3D vector and flattens it
(e.g., val -> `[${x}, ${y}, ${z}]`), which loses other fields like name; tighten
the check by computing the intersection between Object.keys(val) and the known
transform keys ['x','y','z','pitch','yaw','roll'] and only treat val as a
vector/rotator if at least two transform keys are present AND there are no
additional non-transform keys (or alternatively require all keys to be a subset
of the transform set); update the conditional that currently uses keys.length <=
3 && keys.some(...) and keep the rest of the logic that builds x/y/z unchanged.

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

Labels

area/plugin area/server bug Something isn't working enhancement New feature or request javascript Pull requests that update javascript code size/xl

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants