feat: add UMaterialFunction support to material authoring handlers

[nekwo]

Summary

Adds full UMaterialFunction support to material authoring, enabling creation and editing of reusable material functions alongside materials. Also includes stability fixes for TObjectPtr compilation, plugin content mount paths, Blueprint CDO property mutation, and widget styling via ImportText.

Changes

• Add UMaterialFunction as a first-class host type in material authoring handlers (create, connect, disconnect, inspect nodes)
• Add graph traversal, node property editing, and batch operations for material expressions
• Switch nodeId from GUID to stable UObject name with collision handling
• Add set_style action with read/write modes using FProperty::ImportText_Direct for class-agnostic property mutation
• Fix set_property to resolve Blueprint CDOs via blueprintPath parameter
• Fix TObjectPtr compile errors in FindExpressionInArray template (indexed loops with static_cast)
• Fix plugin content mount points (e.g. /EterniaCore/) being clobbered with /Game/ prefix
• Add SanitizeProjectRelativePath to all widget template creation actions
• Add UE 5.0 compatibility for ImportText/ExportText via version macros
• Refactor synchronous FS and exec calls to async in pipeline handlers
• Address review feedback: Payload guards, JSON type coercion, field presence checks, AddVerification, break_connections error handling

Related Issues

Related to #347

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.5)
• Tested MCP client integration (client: Claude Desktop)
• 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

NOTE:
The refactoring is necessary for it to work or else it would be redundant to write another class just for Just material functions

All Devin review changes are included from previous pr

---

[github-actions]

📏 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]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Converts pipeline UBT discovery to async I/O; documents event-loop blocking sentinel; adds Material Function support and name-based node IDs; centralizes and tightens project-relative path validation via FPackageName; enables blueprint CDO targeting for inspect handlers; broad asset/widget/audio handler sanitizations and save-path safety.

Changes

Cohort / File(s)	Summary
Async Pipeline Discovery & Sentinel src/tools/handlers/pipeline-handlers.ts, .jules/sentinel.md	Replaced synchronous fs/exec calls with async fs.promises.* and a promisified exec (5s timeout); tightened executability checks; internal helpers converted to async; added sentinel entry documenting blocking sync Node calls.
Material Function & Graph Updates plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AssetWorkflowHandlers.cpp, plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_MaterialGraphHandlers.cpp, plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpVersionCompatibility.h, src/tools/handlers/material-authoring-handlers.ts, src/tools/consolidated-tool-definitions.ts	Added UMaterialFunction host handling, new MCP_GET_FUNCTION_EXPRESSIONS macro, switched graph nodeId responses to GetName(), expanded TS material-authoring schemas/actions and graph query/mutation handlers.
Path Validation & Sanitization plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeHelpers.h, plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpHandlerUtils.cpp, plugins/McpAutomationBridge/Source/.../McpAutomationBridge_AudioHandlers.cpp, plugins/McpAutomationBridge/Source/.../McpAutomationBridge_LevelStructureHandlers.cpp, plugins/McpAutomationBridge/Source/.../McpAutomationBridge_WidgetAuthoringHandlers.cpp	Require explicit /Game/, /Engine/, /Script/ roots or validate via FPackageName::IsValidLongPackageName; removed ad-hoc heuristics; centralized sanitization; early rejection of invalid paths and standardized use of McpSafeAssetSave with error handling.
Blueprint CDO / Inspect Changes plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp, src/tools/handlers/inspect-handlers.ts	get_property/set_property now accept optional blueprintPath (CDO targeting), require either objectPath or blueprintPath, prefer blueprint resolution and add CDO-specific error codes/behavior.
Widget & Audio Handler Refinements plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_WidgetAuthoringHandlers.cpp, plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AudioHandlers.cpp	Accept JSON path field, sanitize/validate creation directories, add reflection-based widget property read/write, require USoundWave for dialogue sound, and handle save failures explicitly with error returns.
Handlers & Utilities Adjustments plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpHandlerUtils.cpp, plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeHelpers.h	Normalized asset/path validation to engine package checks, expanded ResolveObjectFromPath to accept valid long package names, and include engine MountReason in warnings.
TS Tooling Docs & Schemas src/tools/consolidated-tool-definitions.ts, src/tools/handlers/inspect-handlers.ts	Added MATERIAL_AUTHORING_ACTIONS constant; clarified inspect docs for objectPath vs blueprintPath and nested struct dot-notation; updated schemas to include widget path and expanded material-authoring inputs/outputs.
Subsystem Header & Changelog plugins/McpAutomationBridge/Source/McpAutomationBridge/Public/McpAutomationBridgeSubsystem.h, CHANGELOG.md	Removed private HandleGetAssetGraph declaration; updated CHANGELOG entry documenting SanitizeProjectRelativePath behavior change.

Sequence Diagram(s)

sequenceDiagram
  participant Client as Tool Caller
  participant Handler as pipeline-handlers.ts
  participant FS as fs.promises (filesystem)
  participant Shell as execAsync (shell)
  participant UBT as UnrealBuildTool

  Client->>Handler: request pipeline task
  Handler->>FS: await readdir/access/readFile (candidate dirs/files)
  alt candidate found via filesystem
    FS-->>Handler: candidate path
  else fallback to PATH lookup
    Handler->>Shell: await execAsync('which/unreal*', timeout=5s)
    Shell-->>Handler: stdout match
  end
  Handler->>UBT: spawn/execute with found path
  UBT-->>Handler: result/exit
  Handler-->>Client: return result

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• feat: improve developer documentation, command safety, and blueprint node handling #37 — Modifies src/tools/handlers/pipeline-handlers.ts (UBT discovery and exec behavior); directly related to async/exec changes.
• feat(core): safe map transitions, path security hardening, fuzzy component resolution, and idempotent level ops #220 — Overlaps in McpAutomationBridge path-sanitization and handler validation changes; closely related to package-name validation edits.

Suggested labels

area/tools

Poem

🐰 I nudged a blocking call away,
swapped sync for awaits to save the day.
Materials and functions now have name,
CDOs and paths no longer play the same.
The event loop hops — hooray!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 76.74% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main feature: adding UMaterialFunction support to material authoring handlers, which is the central focus of the changeset.
Description check	✅ Passed	The description covers all required template sections with comprehensive details: clear summary, detailed change list, related issue reference, type of change checkboxes, testing information, and pre-merge checklist—all completed and well-documented.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

Let me scan the changed TypeScript files to identify which functions are missing docstrings.

---

🏁 Script executed:

#!/bin/bash
# Find TypeScript functions/exported symbols missing JSDoc comments in changed files
FILES=(
  "src/tools/handlers/material-authoring-handlers.ts"
  "src/tools/consolidated-tool-definitions.ts"
  "src/tools/handlers/inspect-handlers.ts"
  "src/tools/handlers/pipeline-handlers.ts"
)

for f in "${FILES[@]}"; do
  echo "=== $f ==="
  # Use python to parse and find functions/classes without preceding JSDoc
  python3 - "$f" <<'PYEOF'
import sys, re

filepath = sys.argv[1]
try:
    with open(filepath, 'r') as fh:
        lines = fh.readlines()
except FileNotFoundError:
    print(f"  [NOT FOUND]")
    sys.exit(0)

missing = []
for i, line in enumerate(lines):
    stripped = line.strip()
    # Match exported or top-level function/class/const arrow function declarations
    is_decl = bool(re.match(
        r'^(export\s+)?(async\s+)?function\s+\w+|'
        r'^(export\s+)?class\s+\w+|'
        r'^(export\s+)?(const|let|var)\s+\w+\s*=\s*(async\s+)?\(|'
        r'^(export\s+)?(const|let|var)\s+\w+\s*=\s*(async\s+)?function',
        stripped
    ))
    if is_decl:
        # Look back for a JSDoc comment (/** ... */)
        has_jsdoc = False
        for j in range(i-1, max(i-10, -1), -1):
            prev = lines[j].strip()
            if prev == '' or prev.startswith('//'):
                continue
            if prev.endswith('*/') or prev.startswith('/**') or prev.startswith('*'):
                has_jsdoc = True
            break
        if not has_jsdoc:
            name_match = re.search(r'(function|class|const|let|var)\s+(\w+)', stripped)
            name = name_match.group(2) if name_match else stripped[:60]
            missing.append((i+1, name, stripped[:80]))

if missing:
    for lineno, name, decl in missing:
        print(f"  Line {lineno}: {name} — {decl}")
else:
    print("  [all declarations have preceding doc comments or none found]")
PYEOF
  echo ""
done

Length of output: 5002

---

The previous script caught too many false positives (local const variables inside function bodies). Let me refine the search.

---

🏁 Script executed:

#!/bin/bash
# Find TOP-LEVEL function/class/exported arrow-function declarations missing JSDoc
FILES=(
  "src/tools/handlers/material-authoring-handlers.ts"
  "src/tools/consolidated-tool-definitions.ts"
  "src/tools/handlers/inspect-handlers.ts"
  "src/tools/handlers/pipeline-handlers.ts"
)

for f in "${FILES[@]}"; do
  echo "=== $f ==="
  python3 - "$f" <<'PYEOF'
import sys, re

filepath = sys.argv[1]
try:
    with open(filepath, 'r') as fh:
        lines = fh.readlines()
except FileNotFoundError:
    print("  [NOT FOUND]")
    sys.exit(0)

# Only match lines that start at column 0 (top-level declarations)
TOP_LEVEL = re.compile(
    r'^(export\s+)?(async\s+)?function\s+\w+|'
    r'^(export\s+)?class\s+\w+|'
    r'^(export\s+)(const|let|var)\s+\w+\s*[=:]'  # exported const/let at top level
)

missing = []
for i, line in enumerate(lines):
    raw = line  # preserve leading whitespace to check indentation
    stripped = line.strip()
    # Must start at column 0 (no indentation)
    if raw and raw[0] in (' ', '\t'):
        continue
    if not TOP_LEVEL.match(stripped):
        continue

    # Look backward up to 15 lines for a JSDoc block
    has_jsdoc = False
    for j in range(i - 1, max(i - 15, -1), -1):
        prev = lines[j].strip()
        if prev == '':
            continue
        # JSDoc end marker
        if prev.endswith('*/'):
            has_jsdoc = True
            break
        # Part of a JSDoc block
        if prev.startswith('*') or prev.startswith('/**'):
            has_jsdoc = True
            break
        # Something else (a decorator, another statement)
        break

    if not has_jsdoc:
        name_match = re.search(r'(?:function|class|const|let|var)\s+(\w+)', stripped)
        name = name_match.group(1) if name_match else '(unknown)'
        missing.append((i + 1, name, stripped[:90]))

if missing:
    for lineno, name, decl in missing:
        print(f"  Line {lineno:4d}: {name}")
        print(f"           {decl}")
else:
    print("  ✅ No top-level declarations missing docstrings found")
PYEOF
  echo ""
done

# Also check all other TS handler files for completeness
echo "=== All other src/tools/handlers/*.ts files ==="
for f in src/tools/handlers/*.ts; do
  # skip already checked
  case "$f" in
    src/tools/handlers/material-authoring-handlers.ts) continue;;
    src/tools/handlers/inspect-handlers.ts) continue;;
    src/tools/handlers/pipeline-handlers.ts) continue;;
  esac
  python3 - "$f" <<'PYEOF'
import sys, re

filepath = sys.argv[1]
try:
    with open(filepath, 'r') as fh:
        lines = fh.readlines()
except FileNotFoundError:
    sys.exit(0)

TOP_LEVEL = re.compile(
    r'^(export\s+)?(async\s+)?function\s+\w+|'
    r'^(export\s+)?class\s+\w+|'
    r'^(export\s+)(const|let|var)\s+\w+\s*[=:]'
)

missing = []
for i, line in enumerate(lines):
    raw = line
    stripped = line.strip()
    if raw and raw[0] in (' ', '\t'):
        continue
    if not TOP_LEVEL.match(stripped):
        continue
    has_jsdoc = False
    for j in range(i - 1, max(i - 15, -1), -1):
        prev = lines[j].strip()
        if prev == '':
            continue
        if prev.endswith('*/') or prev.startswith('*') or prev.startswith('/**'):
            has_jsdoc = True
            break
        break
    if not has_jsdoc:
        name_match = re.search(r'(?:function|class|const|let|var)\s+(\w+)', stripped)
        name = name_match.group(1) if name_match else '(unknown)'
        missing.append((i + 1, name))

if missing:
    print(f"  {filepath}:")
    for lineno, name in missing:
        print(f"    Line {lineno:4d}: {name}")
PYEOF
done

Length of output: 3352

---

@nekwo — Good news: none of the files changed in this PR are responsible for the docstring coverage gap. All four modified TypeScript files (material-authoring-handlers.ts, consolidated-tool-definitions.ts, inspect-handlers.ts, pipeline-handlers.ts) have complete top-level docstring coverage.

The 76.19% figure comes entirely from pre-existing functions in untouched handler files. Here's the breakdown of where coverage is missing:

File	Missing docstrings on
actor-handlers.ts	handleActorTools (Line 465)
ai-handlers.ts	getTimeoutMs (Line 22)
animation-handlers.ts	handleAnimationTools (Line 31)
argument-helper.ts	createNormalizedArgs, normalizeArgsInternal (Lines 51, 122)
asset-handlers.ts	handleAssetTools (Line 123)
audio-handlers.ts	setSoundMixClassOverride, clearSoundMixClassOverride, setBaseSoundMix (Lines 39, 52, 65)
blueprint-handlers.ts	hasBlueprintPathTraversal, handleBlueprintTools, handleBlueprintGet (Lines 22, 27, 491)
effect-handlers.ts	ensureActionAndSubAction, handleEffectTools (Lines 6, 24)
graph-handlers.ts	handleGraphTools, handleBlueprintGraph, handleNiagaraGraph, handleMaterialGraph, handleBehaviorTree (Lines 74–215)
sequence-handlers.ts	normalizeSequencePath, markSequenceCreated, markSequenceDeleted, getMessageString, handleSequenceTools (Lines 19–52)
system-handlers.ts	handleSystemTools, handleConsoleCommand (Lines 25, 718)
… and ~20 more getTimeoutMs helpers in various handlers

To push coverage above 80%, you'd need to add JSDoc comments to roughly 10–15 of the more prominent functions above (the handle*Tools and utility functions are the highest-value targets). These are all pre-existing gaps unrelated to this PR's changes.

---

🧠 Learnings used

Learnt from: Fl0p
Repo: ChiR24/Unreal_mcp PR: 342
File: plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_SystemControlHandlers.cpp:602-606
Timestamp: 2026-04-07T19:02:45.303Z
Learning: In the ChiR24/Unreal_mcp repository, `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_SystemControlHandlers.cpp` is upstream handler code marked as DO NOT TOUCH within the team's contribution scope. Changes to this file (including the `execute_python` handler and Python execution wrapper) are pre-existing upstream code and should not be flagged in PR reviews.

---

Learnt from: kalihman
Repo: ChiR24/Unreal_mcp PR: 308
File: tests/integration.mjs:79-84
Timestamp: 2026-03-23T00:28:50.183Z
Learning: In ChiR24/Unreal_mcp, the test runner (`tests/test-runner.mjs`) only supports keyword-based pass/fail matching on response text. It has no mechanism for structured assertions on response body fields (e.g., checking returned asset names, counts, or classNames). Adding such capability requires changes to the test framework itself and is out of scope for feature/fix PRs.

---

Learnt from: nekwo
Repo: ChiR24/Unreal_mcp PR: 354
File: src/tools/consolidated-tool-definitions.ts:899-899
Timestamp: 2026-04-16T04:38:33.843Z
Learning: In ChiR24/Unreal_mcp (`src/tools/consolidated-tool-definitions.ts` and `src/tools/handlers/material-authoring-handlers.ts`), the `additionalOutputs` field name for custom material expression nodes is intentional and must not be renamed to `outputs`. The C++ handler in `McpAutomationBridge_AssetWorkflowHandlers.cpp` reads the JSON key as `"additionalOutputs"` (line 3984), so any rename would silently break the wire protocol.

---

Learnt from: kalihman
Repo: ChiR24/Unreal_mcp PR: 0
File: :0-0
Timestamp: 2026-03-23T05:43:45.224Z
Learning: In ChiR24/Unreal_mcp (asset-handlers.ts), wrapping bridge/handler errors inside `ResponseFactory.success()` instead of `ResponseFactory.error()` is a pre-existing pattern shared across all `manage_asset` action branches (import, delete, list, search_assets, etc.). Fixing it for a single action would be inconsistent; the error propagation pattern should be addressed holistically in a dedicated refactor PR.

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-04T13:12:36.657Z
Learning: Ensure 100% TypeScript and C++ coverage for all tools; no incomplete stubs or 'Not Implemented' placeholders

[nekwo]

they are unrelated but i can still do em if you want @ChiR24 and if its not too much trouble for my agent

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

src/tools/handlers/material-authoring-handlers.ts (1)

77-93: ⚠️ Potential issue | 🟡 Minor

Normalize fallback path inputs too, not just full-path aliases.

materialPath/instancePath is normalized, but the name + path fallback still forwards raw path. That makes behavior format-dependent for /Content/... and backslash inputs.

💡 Suggested fix

@@
-          path = extractOptionalString(params, 'path') ?? '/Game/Materials';
+          path = normalizeAssetPath(extractOptionalString(params, 'path') ?? '/Game/Materials');
@@
-          path = extractOptionalString(params, 'path') ?? '/Game/Materials';
+          path = normalizeAssetPath(extractOptionalString(params, 'path') ?? '/Game/Materials');

As per coding guidelines "Preserve path normalization. Prefer /Game/... asset paths and do not add new code that depends on /Content/... input staying unnormalized."

Also applies to: 714-732

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/tools/handlers/material-authoring-handlers.ts` around lines 77 - 93, The
fallback branch currently takes raw path input from normalizeArgs into path (via
extractOptionalString) without normalizing, causing inconsistent handling
compared to the materialPath branch; update the fallback to run the resolved
path through normalizeAssetPath (same normalizer used for parseMaterialPath)
before assigning to path so inputs like "/Content/..." or backslashes are
converted to "/Game/..." form; specifically modify the fallback that uses
normalizeArgs, extractString, and extractOptionalString to call
normalizeAssetPath(...) on the extracted path (and default '/Game/Materials' as
needed) so both branches produce normalized asset paths.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/tools/handlers/material-authoring-handlers.ts`:
- Around line 77-93: The fallback branch currently takes raw path input from
normalizeArgs into path (via extractOptionalString) without normalizing, causing
inconsistent handling compared to the materialPath branch; update the fallback
to run the resolved path through normalizeAssetPath (same normalizer used for
parseMaterialPath) before assigning to path so inputs like "/Content/..." or
backslashes are converted to "/Game/..." form; specifically modify the fallback
that uses normalizeArgs, extractString, and extractOptionalString to call
normalizeAssetPath(...) on the extracted path (and default '/Game/Materials' as
needed) so both branches produce normalized asset paths.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 916e548a-5695-4c05-9d35-8ab3ce3a16e8

📥 Commits

Reviewing files that changed from the base of the PR and between 6029950 and c49dabf.

📒 Files selected for processing (2)

• src/tools/handlers/inspect-handlers.ts
• src/tools/handlers/material-authoring-handlers.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/tools/handlers/inspect-handlers.ts

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

src/tools/handlers/material-authoring-handlers.ts (2)

418-424: Consider filtering passthrough args to avoid key collisions.

The ...args spread (line 423) occurs after explicit keys like subAction and assetPath. If the caller's args contain these keys, they would override the handler's explicit values, potentially causing unexpected behavior.

Safer alternative using destructuring

+        // Extract only additional passthrough keys, excluding those already set
+        const { assetPath: _, materialPath: __, x: _x, y: _y, ...extraParams } = rawArgs ?? {};
+
         const res = (await executeAutomationRequest(tools, TOOL_ACTIONS.MANAGE_MATERIAL_AUTHORING, {
           subAction: action,
           assetPath,
           x,
           y,
-          ...args, // Pass through any additional params
+          ...extraParams, // Pass through only extra params
         })) as AutomationResponse;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/tools/handlers/material-authoring-handlers.ts` around lines 418 - 424,
The spread of caller-provided args into the executeAutomationRequest payload can
overwrite explicit keys (subAction, assetPath, x, y); fix this in the handler by
filtering/omitting those reserved keys from the args object before spreading
into the request (e.g., derive a safeArgs that excludes subAction, assetPath, x,
y) and pass ...safeArgs to executeAutomationRequest (referencing
executeAutomationRequest and TOOL_ACTIONS.MANAGE_MATERIAL_AUTHORING to locate
the call).

---

1147-1180: Consider extracting shared validation logic.

The inputs and additionalOutputs validation (lines 1147-1180) is nearly identical to lines 477-510 in add_custom_expression. Extracting this to a helper function would reduce duplication and ensure consistent validation.

Example helper extraction

function validateCustomExpressionArrays(
  actionName: string,
  inputs: unknown,
  additionalOutputs: unknown
): { error?: Record<string, unknown>; validInputs: unknown[]; validOutputs: unknown[] } {
  // Shared validation logic for inputs and additionalOutputs
  // Returns error response or validated arrays
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/tools/handlers/material-authoring-handlers.ts` around lines 1147 - 1180,
Extract the duplicated validation for inputs and additionalOutputs used in
add_custom_expression and update_custom_expression into a single helper function
(e.g. validateCustomExpressionArrays) that accepts the action name and the two
values and returns either a standardized error ResponseFactory payload or the
validated arrays; replace the inline checks in both functions (the loops that
validate item object shape, name presence, and optional type string) with calls
to this helper, ensure the helper uses the same error codes ('INVALID_INPUTS' /
'INVALID_OUTPUTS') and message formats (including array index in messages) so
behavior is identical, and update both add_custom_expression and
update_custom_expression to early-return on the helper's error result and
otherwise use the validated arrays it returns.

plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp (1)

163-195: Extract the Blueprint→CDO resolution branch before it drifts.

The Blueprint load / GeneratedClass / CDO / error-code sequence is now duplicated across both property handlers, with another near-copy in HandleInspectCdoAction. A small shared resolver would make future fixes land once instead of three times.

Also applies to: 437-468

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp`
around lines 163 - 195, Extract the repeated Blueprint→GeneratedClass→CDO lookup
and error handling (the sequence using LoadBlueprintAsset, checking
Blueprint->GeneratedClass, calling GetDefaultObject and sending
SendAutomationError with "BLUEPRINT_NOT_FOUND"/"CDO_NOT_FOUND") into a single
helper function (e.g., ResolveBlueprintToCDO or GetRootObjectFromBlueprintPath)
that returns a bool and fills out an output UObject* RootObject and FString
ObjectPath (or signals failure by internally calling SendAutomationError with
the same messages it currently emits). Replace the duplicated blocks in the
property handlers and in HandleInspectCdoAction to call this new helper, use its
returned RootObject/ObjectPath, and keep existing behavior/return values
unchanged.

🤖 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_PropertyHandlers.cpp`:
- Around line 163-195: The current branch that loads a Blueprint via
LoadBlueprintAsset then sets RootObject = GeneratedClass->GetDefaultObject()
misses Blueprint-added component templates, causing nested paths like
"MyAddedComponent.RelativeLocation" to fail; modify the Blueprint-handling
blocks in McpAutomationBridge_PropertyHandlers (both around the
LoadBlueprintAsset/RootObject logic and the similar block at lines ~437-468) to
first parse the first path segment and attempt the same SCS-template lookup used
by FindCdoComponent in HandleInspectCdoAction (i.e., resolve the component name
against the Blueprint's SimpleConstructionScript/USCS_Node::ComponentTemplate)
and if found use that ComponentTemplate as the RootObject for subsequent
property resolution, otherwise fall back to the existing
GeneratedClass->GetDefaultObject() nested-property resolution so
set_object_property/get_object_property will succeed for BP-added components.

---

Nitpick comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp`:
- Around line 163-195: Extract the repeated Blueprint→GeneratedClass→CDO lookup
and error handling (the sequence using LoadBlueprintAsset, checking
Blueprint->GeneratedClass, calling GetDefaultObject and sending
SendAutomationError with "BLUEPRINT_NOT_FOUND"/"CDO_NOT_FOUND") into a single
helper function (e.g., ResolveBlueprintToCDO or GetRootObjectFromBlueprintPath)
that returns a bool and fills out an output UObject* RootObject and FString
ObjectPath (or signals failure by internally calling SendAutomationError with
the same messages it currently emits). Replace the duplicated blocks in the
property handlers and in HandleInspectCdoAction to call this new helper, use its
returned RootObject/ObjectPath, and keep existing behavior/return values
unchanged.

In `@src/tools/handlers/material-authoring-handlers.ts`:
- Around line 418-424: The spread of caller-provided args into the
executeAutomationRequest payload can overwrite explicit keys (subAction,
assetPath, x, y); fix this in the handler by filtering/omitting those reserved
keys from the args object before spreading into the request (e.g., derive a
safeArgs that excludes subAction, assetPath, x, y) and pass ...safeArgs to
executeAutomationRequest (referencing executeAutomationRequest and
TOOL_ACTIONS.MANAGE_MATERIAL_AUTHORING to locate the call).
- Around line 1147-1180: Extract the duplicated validation for inputs and
additionalOutputs used in add_custom_expression and update_custom_expression
into a single helper function (e.g. validateCustomExpressionArrays) that accepts
the action name and the two values and returns either a standardized error
ResponseFactory payload or the validated arrays; replace the inline checks in
both functions (the loops that validate item object shape, name presence, and
optional type string) with calls to this helper, ensure the helper uses the same
error codes ('INVALID_INPUTS' / 'INVALID_OUTPUTS') and message formats
(including array index in messages) so behavior is identical, and update both
add_custom_expression and update_custom_expression to early-return on the
helper's error result and otherwise use the validated arrays it returns.

🪄 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: 4232c2a6-e0ec-46af-afb7-21796bc2e13b

📥 Commits

Reviewing files that changed from the base of the PR and between c49dabf and 8c46c03.

📒 Files selected for processing (2)

• plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp
• src/tools/handlers/material-authoring-handlers.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major

blueprintPath still misses Blueprint-added component templates.

These branches always bind RootObject to GeneratedClass->GetDefaultObject(). That covers class defaults and native components, but nested paths like MyAddedComponent.RelativeLocation still resolve against the actor CDO, while Blueprint-added components only exist on USCS_Node::ComponentTemplate. In practice, valid Blueprint default paths for BP-added components will still come back as PROPERTY_NOT_FOUND in both set_object_property and get_object_property.

Please route the first path segment through the same SCS-template lookup pattern used by FindCdoComponent() in HandleInspectCdoAction before falling back to normal nested-property resolution.

Also applies to: 437-468

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp`
around lines 163 - 195, The current branch that loads a Blueprint via
LoadBlueprintAsset then sets RootObject = GeneratedClass->GetDefaultObject()
misses Blueprint-added component templates, causing nested paths like
"MyAddedComponent.RelativeLocation" to fail; modify the Blueprint-handling
blocks in McpAutomationBridge_PropertyHandlers (both around the
LoadBlueprintAsset/RootObject logic and the similar block at lines ~437-468) to
first parse the first path segment and attempt the same SCS-template lookup used
by FindCdoComponent in HandleInspectCdoAction (i.e., resolve the component name
against the Blueprint's SimpleConstructionScript/USCS_Node::ComponentTemplate)
and if found use that ComponentTemplate as the RootObject for subsequent
property resolution, otherwise fall back to the existing
GeneratedClass->GetDefaultObject() nested-property resolution so
set_object_property/get_object_property will succeed for BP-added components.

[nekwo]

This finding is valid but out of scope for this PR. Here's why:

The limitation is real — MyBPComponent.RelativeLocation on a CDO won't find BP-added components because they live on SCS node templates, not on the CDO. But:

This is a pre-existing limitation of the property resolution system, not something introduced by the blueprintPath PR
FindCdoComponent is part of inspect_cdo, a completely separate action with its own code path
Integrating SCS-template lookup into the generic set_property/get_property property resolution would require modifying ResolveNestedPropertyPath or adding a Blueprint-aware wrapper around it — it's used in 20+ call sites across the codebase
The blueprintPath we added correctly handles all CDO-level properties and native component properties, which is the common case
This should be tracked as a follow-up enhancement, not a fix for the current PR.

[coderabbitai]

@nekwo, that's a fair and well-reasoned call. The scope argument holds — ResolveNestedPropertyPath being shared across 20+ call sites makes a Blueprint-aware wrapper a meaningful refactor that deserves its own PR with proper testing.

Would you like me to open a GitHub issue to track this as a follow-up enhancement? I can capture:

• The exact limitation (BP-added component templates on USCS_Node::ComponentTemplate are not reachable via blueprintPath + dot-notation in set_property/get_property)
• The proposed fix direction (SCS-template lookup as a pre-step before ResolveNestedPropertyPath, similar to FindCdoComponent)
• Links back to this PR for context

( ˶ᵔ ᵕ ᵔ˶ )

---

✏️ Learnings added

Learnt from: nekwo
Repo: ChiR24/Unreal_mcp PR: 354
File: plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp:163-195
Timestamp: 2026-04-16T05:12:57.655Z
Learning: In ChiR24/Unreal_mcp (`plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp`), when `blueprintPath` is provided to `HandleSetObjectProperty`/`HandleGetObjectProperty`, `RootObject` is intentionally bound to `GeneratedClass->GetDefaultObject()` (CDO). This means nested dot-notation paths targeting Blueprint-added component templates (e.g., `MyBPComponent.RelativeLocation`) will return `PROPERTY_NOT_FOUND` because those components only exist on `USCS_Node::ComponentTemplate`, not on the CDO. This is a known pre-existing limitation of the `ResolveNestedPropertyPath` system (used in 20+ call sites), not introduced by the `blueprintPath` feature. A follow-up enhancement should add a Blueprint-aware SCS-template lookup step (similar to `FindCdoComponent` in `HandleInspectCdoAction`) before falling back to standard nested-property resolution. Do not flag this gap as a regression in PR `#354`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

🧠 Learnings used

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/*Handlers.cpp : Do not use absolute Windows paths in handler code - use relative paths or UE's path resolution utilities

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/*Handlers.cpp : Use `nullptr` for path-based object lookups instead of the deprecated `ANY_PACKAGE` constant

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/*Handlers.cpp : Use `SCS->CreateNode()` with proper SCS ownership for component creation in UE 5.7+ instead of relying on Blueprint ownership

---

Learnt from: nekwo
Repo: ChiR24/Unreal_mcp PR: 354
File: plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp:490-501
Timestamp: 2026-04-16T04:58:59.272Z
Learning: In ChiR24/Unreal_mcp (`src/tools/handlers/inspect-handlers.ts` and `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp`), `CDO_NOT_FOUND` is intentionally treated as a member of the "not found" error family in the `inspect_cdo` path (Blueprint CDO couldn't be loaded — semantically a not-found condition). `CDO_TRANSFORM` is intentionally NOT given special-case handling in TypeScript; it is an invalid-operation error (object found, but runtime transform access is forbidden on a CDO) and propagates correctly with its original error code and message via the default `cleanObject(res)` path. Do not flag the absence of a CDO_TRANSFORM branch alongside OBJECT_NOT_FOUND/BLUEPRINT_NOT_FOUND as a gap.

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T19:14:22.831Z
Learning: Applies to Plugins/McpAutomationBridge/Source/**/*.{cpp,h} : Implement Unreal side actions in the appropriate handler under `Plugins/McpAutomationBridge/Source/` and register in `UMcpAutomationBridgeSubsystem::InitializeHandlers()`.

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/*Handlers.cpp : All handler code is automatically dispatched to the game thread by the subsystem - do not add additional thread dispatch logic

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/*Handlers.cpp : Use `FJsonObjectConverter` for JSON parsing and struct serialization instead of manual JSON parsing

---

Learnt from: kalihman
Repo: ChiR24/Unreal_mcp PR: 311
File: plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AssetQueryHandlers.cpp:332-340
Timestamp: 2026-03-28T22:04:49.643Z
Learning: In ChiR24/Unreal_mcp (`plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AssetQueryHandlers.cpp`), base-class short-name aliases (`SoundBase`, `Texture`, `DataAsset`) in the `ClassMappings` table are intentionally left without forcing `bRecursiveClasses=true`. They are designed to be used in combination with the caller-supplied `"recursiveClasses": true` parameter. Silently forcing recursive class matching based on class name heuristics would break the exact-match contract for callers who want to query only exact instances of a base class.

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/McpAutomationBridge{Subsystem.h,_ProcessRequest.cpp} : Declare new handler in `McpAutomationBridgeSubsystem.h` and register it in the `InitializeHandlers()` method when adding a handler

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T19:14:22.831Z
Learning: Applies to Plugins/McpAutomationBridge/Source/**/*.{cpp,h} : Do not introduce new `ANY_PACKAGE` usage; use modern lookup patterns such as `nullptr` where required by newer UE versions.

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T19:14:22.831Z
Learning: Applies to Plugins/McpAutomationBridge/Source/**/*.{cpp,h} : For Blueprint component templates, let SCS own nodes and templates through `CreateNode()` and `AddNode()` patterns.

---

Learnt from: nekwo
Repo: ChiR24/Unreal_mcp PR: 354
File: src/tools/handlers/material-authoring-handlers.ts:33-53
Timestamp: 2026-04-16T03:05:23.176Z
Learning: In ChiR24/Unreal_mcp (`src/tools/handlers/material-authoring-handlers.ts`), the local `normalizeAssetPath()` helper is intentionally separate from the shared `normalizePath()` utility. Its purpose is limited to `/Content/` → `/Game/` remapping and backslash normalisation for material handler paths. Plugin mount-point clobbering protection (e.g., `/EterniaCore/`, `/ShooterCore/`) is enforced on the C++ side in `SanitizeProjectRelativePath` via `FPackageName::IsValidLongPackageName`. Do not suggest replacing `normalizeAssetPath()` with the shared utility or assume that the TypeScript layer is responsible for plugin mount-point validation.

---

Learnt from: nekwo
Repo: ChiR24/Unreal_mcp PR: 354
File: src/tools/consolidated-tool-definitions.ts:899-899
Timestamp: 2026-04-16T04:38:33.843Z
Learning: In ChiR24/Unreal_mcp (`src/tools/consolidated-tool-definitions.ts` and `src/tools/handlers/material-authoring-handlers.ts`), the `additionalOutputs` field name for custom material expression nodes is intentional and must not be renamed to `outputs`. The C++ handler in `McpAutomationBridge_AssetWorkflowHandlers.cpp` reads the JSON key as `"additionalOutputs"` (line 3984), so any rename would silently break the wire protocol.

[nekwo]

yes go ahead

[coderabbitai]

🧩 Analysis chain

@nekwo, the issue is created! Here's a summary of what was captured:

• Title: feat: add SCS-template lookup to set_property/get_property for Blueprint-added components
• Body: Documents the limitation (BP-added components on USCS_Node::ComponentTemplate unreachable via blueprintPath + dot-notation), backlinks to PR #354 and this comment, and outlines the proposed fix direction (parse first path segment → FindCdoComponent-style SCS walk → use template as RootObject if found, else fall back to CDO).
• Assignee: @nekwo

---

🏁 Script executed:

gh issue create \
  --repo ChiR24/Unreal_mcp \
  --title "feat: add SCS-template lookup to set_property/get_property for Blueprint-added components" \
  --body "## Summary

When \`blueprintPath\` is supplied to \`HandleSetObjectProperty\` / \`HandleGetObjectProperty\`, \`RootObject\` is bound to \`GeneratedClass->GetDefaultObject()\` (CDO). This correctly handles all CDO-level properties and native component properties, but dot-notation paths targeting **Blueprint-added component templates** (e.g. \`MyBPComponent.RelativeLocation\`) still return \`PROPERTY_NOT_FOUND\` because those components live on \`USCS_Node::ComponentTemplate\`, not on the CDO.

## Background

Identified during review of PR `#354` (feat: add UMaterialFunction support to material authoring handlers) — see discussion at https://github.com/ChiR24/Unreal_mcp/pull/354#discussion_r3090893183.

This is a **pre-existing limitation** of the shared \`ResolveNestedPropertyPath\` system (used in 20+ call sites), not a regression introduced by `#354`. It was explicitly deferred as out-of-scope for that PR.

## Proposed Enhancement

In \`McpAutomationBridge_PropertyHandlers.cpp\`, after a Blueprint CDO is resolved via \`blueprintPath\`:

1. Parse the **first path segment** of a dot-notation \`propertyName\`.
2. Attempt the same SCS-template lookup used by \`FindCdoComponent()\` in \`HandleInspectCdoAction\` — walk \`Blueprint->SimpleConstructionScript->GetAllNodes()\` (and parent BP chain) for a matching \`USCS_Node::ComponentTemplate\`.
3. If a template is found, use it as the \`RootObject\` for the remaining property path resolution.
4. Otherwise fall back to the existing CDO-based \`ResolveNestedPropertyPath\` call.

This applies to both \`HandleSetObjectProperty\` (around lines 163–195) and \`HandleGetObjectProperty\` (around lines 437–468) in \`plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp\`.

## Files Affected

- \`plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp\`

## Requested by

`@nekwo`" \
  --assignee nekwo

Length of output: 76

---

🏁 Script executed:

gh issue create \
  --repo ChiR24/Unreal_mcp \
  --title "feat: add SCS-template lookup to set_property/get_property for Blueprint-added components" \
  --body "## Summary

When \`blueprintPath\` is supplied to \`HandleSetObjectProperty\` / \`HandleGetObjectProperty\`, \`RootObject\` is bound to \`GeneratedClass->GetDefaultObject()\` (CDO). This correctly handles all CDO-level properties and native component properties, but dot-notation paths targeting **Blueprint-added component templates** (e.g. \`MyBPComponent.RelativeLocation\`) still return \`PROPERTY_NOT_FOUND\` because those components live on \`USCS_Node::ComponentTemplate\`, not on the CDO.

## Background

Identified during review of PR `#354` (feat: add UMaterialFunction support to material authoring handlers) — see discussion at https://github.com/ChiR24/Unreal_mcp/pull/354#discussion_r3090893183.

This is a **pre-existing limitation** of the shared \`ResolveNestedPropertyPath\` system (used in 20+ call sites), not a regression introduced by `#354`. It was explicitly deferred as out-of-scope for that PR.

## Proposed Enhancement

In \`McpAutomationBridge_PropertyHandlers.cpp\`, after a Blueprint CDO is resolved via \`blueprintPath\`:

1. Parse the **first path segment** of a dot-notation \`propertyName\`.
2. Attempt the same SCS-template lookup used by \`FindCdoComponent()\` in \`HandleInspectCdoAction\` — walk \`Blueprint->SimpleConstructionScript->GetAllNodes()\` (and parent BP chain) for a matching \`USCS_Node::ComponentTemplate\`.
3. If a template is found, use it as the \`RootObject\` for the remaining property path resolution.
4. Otherwise fall back to the existing CDO-based \`ResolveNestedPropertyPath\` call.

This applies to both \`HandleSetObjectProperty\` (around lines 163–195) and \`HandleGetObjectProperty\` (around lines 437–468) in \`plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp\`.

## Files Affected

- \`plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp\`

## Requested by

`@nekwo`"

Length of output: 47

---

@nekwo, the follow-up issue has been opened: #357

It captures:

• The limitation (BP-added component templates on USCS_Node::ComponentTemplate not reachable via blueprintPath + dot-notation in set_property/get_property)
• Context that it's pre-existing and not a regression from PR #354
• The proposed fix direction (parse first path segment → FindCdoComponent-style SCS walk → use template as RootObject, else fall back to CDO)
• Backlinks to this PR and comment

---

🧠 Learnings used

Learnt from: nekwo
Repo: ChiR24/Unreal_mcp PR: 354
File: plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp:163-195
Timestamp: 2026-04-16T05:12:57.655Z
Learning: In ChiR24/Unreal_mcp (`plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp`), when `blueprintPath` is provided to `HandleSetObjectProperty`/`HandleGetObjectProperty`, `RootObject` is intentionally bound to `GeneratedClass->GetDefaultObject()` (CDO). This means nested dot-notation paths targeting Blueprint-added component templates (e.g., `MyBPComponent.RelativeLocation`) will return `PROPERTY_NOT_FOUND` because those components only exist on `USCS_Node::ComponentTemplate`, not on the CDO. This is a known pre-existing limitation of the `ResolveNestedPropertyPath` system (used in 20+ call sites), not introduced by the `blueprintPath` feature. A follow-up enhancement should add a Blueprint-aware SCS-template lookup step (similar to `FindCdoComponent` in `HandleInspectCdoAction`) before falling back to standard nested-property resolution. Do not flag this gap as a regression in PR `#354`.

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/*Handlers.cpp : Do not use absolute Windows paths in handler code - use relative paths or UE's path resolution utilities

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/*Handlers.cpp : Use `nullptr` for path-based object lookups instead of the deprecated `ANY_PACKAGE` constant

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/*Handlers.cpp : Use `SCS->CreateNode()` with proper SCS ownership for component creation in UE 5.7+ instead of relying on Blueprint ownership

---

Learnt from: nekwo
Repo: ChiR24/Unreal_mcp PR: 354
File: plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp:490-501
Timestamp: 2026-04-16T04:58:59.272Z
Learning: In ChiR24/Unreal_mcp (`src/tools/handlers/inspect-handlers.ts` and `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp`), `CDO_NOT_FOUND` is intentionally treated as a member of the "not found" error family in the `inspect_cdo` path (Blueprint CDO couldn't be loaded — semantically a not-found condition). `CDO_TRANSFORM` is intentionally NOT given special-case handling in TypeScript; it is an invalid-operation error (object found, but runtime transform access is forbidden on a CDO) and propagates correctly with its original error code and message via the default `cleanObject(res)` path. Do not flag the absence of a CDO_TRANSFORM branch alongside OBJECT_NOT_FOUND/BLUEPRINT_NOT_FOUND as a gap.

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T19:14:22.831Z
Learning: Applies to Plugins/McpAutomationBridge/Source/**/*.{cpp,h} : Implement Unreal side actions in the appropriate handler under `Plugins/McpAutomationBridge/Source/` and register in `UMcpAutomationBridgeSubsystem::InitializeHandlers()`.

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/*Handlers.cpp : All handler code is automatically dispatched to the game thread by the subsystem - do not add additional thread dispatch logic

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/*Handlers.cpp : Use `FJsonObjectConverter` for JSON parsing and struct serialization instead of manual JSON parsing

---

Learnt from: kalihman
Repo: ChiR24/Unreal_mcp PR: 311
File: plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AssetQueryHandlers.cpp:332-340
Timestamp: 2026-03-28T22:04:49.643Z
Learning: In ChiR24/Unreal_mcp (`plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AssetQueryHandlers.cpp`), base-class short-name aliases (`SoundBase`, `Texture`, `DataAsset`) in the `ClassMappings` table are intentionally left without forcing `bRecursiveClasses=true`. They are designed to be used in combination with the caller-supplied `"recursiveClasses": true` parameter. Silently forcing recursive class matching based on class name heuristics would break the exact-match contract for callers who want to query only exact instances of a base class.

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/McpAutomationBridge{Subsystem.h,_ProcessRequest.cpp} : Declare new handler in `McpAutomationBridgeSubsystem.h` and register it in the `InitializeHandlers()` method when adding a handler

---

Learnt from: nekwo
Repo: ChiR24/Unreal_mcp PR: 354
File: src/tools/consolidated-tool-definitions.ts:899-899
Timestamp: 2026-04-16T04:38:33.843Z
Learning: In ChiR24/Unreal_mcp (`src/tools/consolidated-tool-definitions.ts` and `src/tools/handlers/material-authoring-handlers.ts`), the `additionalOutputs` field name for custom material expression nodes is intentional and must not be renamed to `outputs`. The C++ handler in `McpAutomationBridge_AssetWorkflowHandlers.cpp` reads the JSON key as `"additionalOutputs"` (line 3984), so any rename would silently break the wire protocol.

---

Learnt from: kalihman
Repo: ChiR24/Unreal_mcp PR: 0
File: :0-0
Timestamp: 2026-03-23T05:43:45.224Z
Learning: In ChiR24/Unreal_mcp (asset-handlers.ts), wrapping bridge/handler errors inside `ResponseFactory.success()` instead of `ResponseFactory.error()` is a pre-existing pattern shared across all `manage_asset` action branches (import, delete, list, search_assets, etc.). Fixing it for a single action would be inconsistent; the error propagation pattern should be addressed holistically in a dedicated refactor PR.

---

Learnt from: Fl0p
Repo: ChiR24/Unreal_mcp PR: 342
File: plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_SystemControlHandlers.cpp:602-606
Timestamp: 2026-04-07T19:02:45.303Z
Learning: In the ChiR24/Unreal_mcp repository, `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_SystemControlHandlers.cpp` is upstream handler code marked as DO NOT TOUCH within the team's contribution scope. Changes to this file (including the `execute_python` handler and Python execution wrapper) are pre-existing upstream code and should not be flagged in PR reviews.

---

Learnt from: Fl0p
Repo: ChiR24/Unreal_mcp PR: 342
File: plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeSubsystem.cpp:412-424
Timestamp: 2026-04-07T19:03:49.111Z
Learning: In ChiR24/Unreal_mcp, `TargetSocket == nullptr` in `UMcpAutomationBridgeSubsystem::SendAutomationResponse` (plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeSubsystem.cpp) is a deliberate transport convention: HTTP-originated requests always pass nullptr, while WebSocket handlers always supply a valid socket. The double-check `!TargetSocket.IsValid() && NativeTransport` is intentional and correct — do not flag it as an ambiguous discriminator or suggest replacing it with HasPendingRequest().

---

Learnt from: nekwo
Repo: ChiR24/Unreal_mcp PR: 354
File: src/tools/handlers/material-authoring-handlers.ts:33-53
Timestamp: 2026-04-16T03:05:23.176Z
Learning: In ChiR24/Unreal_mcp (`src/tools/handlers/material-authoring-handlers.ts`), the local `normalizeAssetPath()` helper is intentionally separate from the shared `normalizePath()` utility. Its purpose is limited to `/Content/` → `/Game/` remapping and backslash normalisation for material handler paths. Plugin mount-point clobbering protection (e.g., `/EterniaCore/`, `/ShooterCore/`) is enforced on the C++ side in `SanitizeProjectRelativePath` via `FPackageName::IsValidLongPackageName`. Do not suggest replacing `normalizeAssetPath()` with the shared utility or assume that the TypeScript layer is responsible for plugin mount-point validation.

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T19:14:22.831Z
Learning: Applies to Plugins/McpAutomationBridge/Source/**/*.{cpp,h} : For Blueprint component templates, let SCS own nodes and templates through `CreateNode()` and `AddNode()` patterns.

[nekwo]

looks like you opened to 356 and 357 by accident??

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
gh issue view 356 --repo ChiR24/Unreal_mcp --json number,title,body,state,url 2>&1
echo "---"
gh issue view 357 --repo ChiR24/Unreal_mcp --json number,title,body,state,url 2>&1

Length of output: 4231

---

Confirmed — both #356 and #357 are identical. Closing #357 as the duplicate now.

---

🏁 Script executed:

#!/bin/bash
gh issue close 357 --repo ChiR24/Unreal_mcp --comment "Closing as duplicate of `#356` — created twice due to a script retry."

Length of output: 60

---

🏁 Script executed:

#!/bin/bash
gh issue close 357 --repo ChiR24/Unreal_mcp

Length of output: 60

---

@nekwo, apologies for the duplicate — the script ran twice due to a retry when the first attempt didn't return clear output. Unfortunately I don't have sufficient permissions to close issues on this repo directly.

Could you please manually close #357 and keep #356 as the canonical tracking issue? The URL is: #357

---

🧠 Learnings used

Learnt from: nekwo
Repo: ChiR24/Unreal_mcp PR: 354
File: plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp:163-195
Timestamp: 2026-04-16T05:12:57.655Z
Learning: In ChiR24/Unreal_mcp (`plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp`), when `blueprintPath` is provided to `HandleSetObjectProperty`/`HandleGetObjectProperty`, `RootObject` is intentionally bound to `GeneratedClass->GetDefaultObject()` (CDO). This means nested dot-notation paths targeting Blueprint-added component templates (e.g., `MyBPComponent.RelativeLocation`) will return `PROPERTY_NOT_FOUND` because those components only exist on `USCS_Node::ComponentTemplate`, not on the CDO. This is a known pre-existing limitation of the `ResolveNestedPropertyPath` system (used in 20+ call sites), not introduced by the `blueprintPath` feature. A follow-up enhancement should add a Blueprint-aware SCS-template lookup step (similar to `FindCdoComponent` in `HandleInspectCdoAction`) before falling back to standard nested-property resolution. Do not flag this gap as a regression in PR `#354`.

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/*Handlers.cpp : Do not use absolute Windows paths in handler code - use relative paths or UE's path resolution utilities

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/*Handlers.cpp : Use `nullptr` for path-based object lookups instead of the deprecated `ANY_PACKAGE` constant

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/*Handlers.cpp : Use `SCS->CreateNode()` with proper SCS ownership for component creation in UE 5.7+ instead of relying on Blueprint ownership

---

Learnt from: nekwo
Repo: ChiR24/Unreal_mcp PR: 354
File: plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp:490-501
Timestamp: 2026-04-16T04:58:59.272Z
Learning: In ChiR24/Unreal_mcp (`src/tools/handlers/inspect-handlers.ts` and `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_PropertyHandlers.cpp`), `CDO_NOT_FOUND` is intentionally treated as a member of the "not found" error family in the `inspect_cdo` path (Blueprint CDO couldn't be loaded — semantically a not-found condition). `CDO_TRANSFORM` is intentionally NOT given special-case handling in TypeScript; it is an invalid-operation error (object found, but runtime transform access is forbidden on a CDO) and propagates correctly with its original error code and message via the default `cleanObject(res)` path. Do not flag the absence of a CDO_TRANSFORM branch alongside OBJECT_NOT_FOUND/BLUEPRINT_NOT_FOUND as a gap.

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T19:14:22.831Z
Learning: Applies to Plugins/McpAutomationBridge/Source/**/*.{cpp,h} : Implement Unreal side actions in the appropriate handler under `Plugins/McpAutomationBridge/Source/` and register in `UMcpAutomationBridgeSubsystem::InitializeHandlers()`.

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/*Handlers.cpp : All handler code is automatically dispatched to the game thread by the subsystem - do not add additional thread dispatch logic

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/*Handlers.cpp : Use `FJsonObjectConverter` for JSON parsing and struct serialization instead of manual JSON parsing

---

Learnt from: kalihman
Repo: ChiR24/Unreal_mcp PR: 311
File: plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AssetQueryHandlers.cpp:332-340
Timestamp: 2026-03-28T22:04:49.643Z
Learning: In ChiR24/Unreal_mcp (`plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AssetQueryHandlers.cpp`), base-class short-name aliases (`SoundBase`, `Texture`, `DataAsset`) in the `ClassMappings` table are intentionally left without forcing `bRecursiveClasses=true`. They are designed to be used in combination with the caller-supplied `"recursiveClasses": true` parameter. Silently forcing recursive class matching based on class name heuristics would break the exact-match contract for callers who want to query only exact instances of a base class.

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: plugins/McpAutomationBridge/AGENTS.md:0-0
Timestamp: 2026-02-21T17:23:38.940Z
Learning: Applies to plugins/McpAutomationBridge/**/McpAutomationBridge{Subsystem.h,_ProcessRequest.cpp} : Declare new handler in `McpAutomationBridgeSubsystem.h` and register it in the `InitializeHandlers()` method when adding a handler

---

Learnt from: Fl0p
Repo: ChiR24/Unreal_mcp PR: 342
File: plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_SystemControlHandlers.cpp:602-606
Timestamp: 2026-04-07T19:02:45.303Z
Learning: In the ChiR24/Unreal_mcp repository, `plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_SystemControlHandlers.cpp` is upstream handler code marked as DO NOT TOUCH within the team's contribution scope. Changes to this file (including the `execute_python` handler and Python execution wrapper) are pre-existing upstream code and should not be flagged in PR reviews.

---

Learnt from: kalihman
Repo: ChiR24/Unreal_mcp PR: 0
File: :0-0
Timestamp: 2026-03-23T05:43:45.224Z
Learning: In ChiR24/Unreal_mcp (asset-handlers.ts), wrapping bridge/handler errors inside `ResponseFactory.success()` instead of `ResponseFactory.error()` is a pre-existing pattern shared across all `manage_asset` action branches (import, delete, list, search_assets, etc.). Fixing it for a single action would be inconsistent; the error propagation pattern should be addressed holistically in a dedicated refactor PR.

---

Learnt from: nekwo
Repo: ChiR24/Unreal_mcp PR: 354
File: src/tools/consolidated-tool-definitions.ts:899-899
Timestamp: 2026-04-16T04:38:33.843Z
Learning: In ChiR24/Unreal_mcp (`src/tools/consolidated-tool-definitions.ts` and `src/tools/handlers/material-authoring-handlers.ts`), the `additionalOutputs` field name for custom material expression nodes is intentional and must not be renamed to `outputs`. The C++ handler in `McpAutomationBridge_AssetWorkflowHandlers.cpp` reads the JSON key as `"additionalOutputs"` (line 3984), so any rename would silently break the wire protocol.

---

Learnt from: Fl0p
Repo: ChiR24/Unreal_mcp PR: 342
File: plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeSubsystem.cpp:412-424
Timestamp: 2026-04-07T19:03:49.111Z
Learning: In ChiR24/Unreal_mcp, `TargetSocket == nullptr` in `UMcpAutomationBridgeSubsystem::SendAutomationResponse` (plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridgeSubsystem.cpp) is a deliberate transport convention: HTTP-originated requests always pass nullptr, while WebSocket handlers always supply a valid socket. The double-check `!TargetSocket.IsValid() && NativeTransport` is intentional and correct — do not flag it as an ambiguous discriminator or suggest replacing it with HasPendingRequest().

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T19:14:22.831Z
Learning: This is a dual-process system: the TypeScript MCP server lives in `src/`, and the Unreal Editor bridge plugin lives in `Plugins/McpAutomationBridge/`. Typical flow is schema validation -> tool registry dispatch -> domain handler -> `executeAutomationRequest()` -> WebSocket bridge -> C++ subsystem -> game-thread handler.

---

Learnt from: CR
Repo: ChiR24/Unreal_mcp PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T19:14:22.831Z
Learning: Applies to Plugins/McpAutomationBridge/Source/**/*.{cpp,h} : Do not introduce new `ANY_PACKAGE` usage; use modern lookup patterns such as `nullptr` where required by newer UE versions.

---

Learnt from: nekwo
Repo: ChiR24/Unreal_mcp PR: 354
File: src/tools/handlers/material-authoring-handlers.ts:33-53
Timestamp: 2026-04-16T03:05:23.176Z
Learning: In ChiR24/Unreal_mcp (`src/tools/handlers/material-authoring-handlers.ts`), the local `normalizeAssetPath()` helper is intentionally separate from the shared `normalizePath()` utility. Its purpose is limited to `/Content/` → `/Game/` remapping and backslash normalisation for material handler paths. Plugin mount-point clobbering protection (e.g., `/EterniaCore/`, `/ShooterCore/`) is enforced on the C++ side in `SanitizeProjectRelativePath` via `FPackageName::IsValidLongPackageName`. Do not suggest replacing `normalizeAssetPath()` with the shared utility or assume that the TypeScript layer is responsible for plugin mount-point validation.

[nekwo]

i don't have that kind of access thats for @chi354 or you to do

any other requested change? that seems to be it

[coderabbitai]

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

[ChiR24]

they are unrelated but i can still do em if you want @ChiR24 and if its not too much trouble for my agent

Appreciate the offer! Please don't add them here though – let's keep this PR clean and focused on the material-function work.

[coderabbitai]

Actionable comments posted: 1

🤖 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/consolidated-tool-definitions.ts`:
- Line 1517: Update the description for the sourcePin property (symbol:
sourcePin) to indicate it accepts either a numeric index (as a string) or a
named output rather than only a numeric index; keep the schema type as 'string'
and phrase the description like "Source output pin index or name (numeric index
passed as a string) — selects which output of a multi-output node to connect."
This clarifies that callers may pass either an index-or-name and aligns with the
C++ bridge behavior.

🪄 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: 25635250-4668-4a3f-ad2c-88783392588c

📥 Commits

Reviewing files that changed from the base of the PR and between 8c46c03 and 3c9cd5c.

📒 Files selected for processing (4)

• CHANGELOG.md
• plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AssetWorkflowHandlers.cpp
• plugins/McpAutomationBridge/Source/McpAutomationBridge/Public/McpAutomationBridgeSubsystem.h
• src/tools/consolidated-tool-definitions.ts

💤 Files with no reviewable changes (1)

• plugins/McpAutomationBridge/Source/McpAutomationBridge/Public/McpAutomationBridgeSubsystem.h

🚧 Files skipped from review as they are similar to previous changes (2)

• CHANGELOG.md
• plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AssetWorkflowHandlers.cpp

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Document sourcePin as index-or-name, not numeric-only.

The schema type is correct, but this description now implies callers may only pass a numeric output index. The bridge also accepts named outputs, so this will misdocument valid requests for multi-output custom expressions/material functions.

Suggested wording

-        sourcePin: { type: 'string', description: 'Source output pin index (numeric). Selects which output of a multi-output node to connect.' },
+        sourcePin: { type: 'string', description: 'Source output pin selector. Accepts either a numeric string (output index) or an output name for multi-output nodes.' },

Based on learnings, sourcePin is intentionally declared as type: 'string' because the C++ side accepts either a numeric string or an output name.

📝 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

	sourcePin: { type: 'string', description: 'Source output pin index (numeric). Selects which output of a multi-output node to connect.' },
	sourcePin: { type: 'string', description: 'Source output pin selector. Accepts either a numeric string (output index) or an output name for multi-output nodes.' },

🤖 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 1517, Update the
description for the sourcePin property (symbol: sourcePin) to indicate it
accepts either a numeric index (as a string) or a named output rather than only
a numeric index; keep the schema type as 'string' and phrase the description
like "Source output pin index or name (numeric index passed as a string) —
selects which output of a multi-output node to connect." This clarifies that
callers may pass either an index-or-name and aligns with the C++ bridge
behavior.

[devin-ai-integration]

Devin Review found 2 new potential issues.

View 29 additional findings in Devin Review.

[devin-ai-integration]

🟡 FindBlueprintNormalizedPath fails to strip object-path suffix for inputs without path separators

The reordering of suffix-stripping before root-normalization in FindBlueprintNormalizedPath introduces a behavioral regression for dotted inputs that lack / separators (e.g., BP_Test.BP_Test). The object-path stripping logic at McpAutomationBridgeHelpers.h:3031-3023 requires BeforeDot.FindLastChar('/', LastSlashIdx) to succeed with LastSlashIdx > 0. For an input like BP_Test.BP_Test, BeforeDot is BP_Test which contains no /, so the condition fails and the suffix is never stripped. After root normalization, the path becomes /Game/BP_Test.BP_Test instead of the previous /Game/BP_Test. While UEditorAssetLibrary::DoesAssetExist may handle both formats, OutNormalized is set to the un-stripped object-path form, and downstream callers that expect a clean package path (e.g., for constructing _C class paths) will get a malformed result.

Prompt for agents

The problem is in FindBlueprintNormalizedPath (McpAutomationBridgeHelpers.h around line 3000-3040). The old code prepended /Game/ before stripping .ObjectName suffixes, so the stripping logic (which relies on finding a / in the BeforeDot portion) always worked. The new code strips suffixes first, then normalizes roots. But for inputs like BP_Test.BP_Test (no slash), the suffix stripping fails because BeforeDot has no / character.

Possible fix: After the dot-suffix stripping block but before the mount-point normalization block, add a fallback that handles the case where the input contains a dot but no slash. For example, if CheckPath has no / and contains a dot where AfterDot == everything before the dot (i.e., it looks like Package.Package), strip the suffix portion. Alternatively, move the /Game/ prepend before the dot stripping as the old code did, but only for paths that dont start with / (bare names).

---

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

[devin-ai-integration]

🟡 Material expression nodeId changed from stable GUID to session-local object name without versioned API indicator

Multiple locations in McpAutomationBridge_MaterialGraphHandlers.cpp changed the nodeId response field from MaterialExpressionGuid.ToString() (a persistent, globally-unique identifier) to GetName() (an instance name like MaterialExpressionAdd_0 that is only unique within the owning material and can change across save/reload cycles). While FindExpressionByIdOrNameOrIndex still supports both GUID and name lookups, any MCP client that persists or compares nodeId values across sessions (e.g., storing graph layouts, undo stacks, or build scripts) will silently get different identifiers. The file header comment at line 23 still documents GUID string (MaterialExpressionGuid) as the first identification method, which is now inconsistent with what's returned.

---

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

[nekwo]

they are unrelated but i can still do em if you want @ChiR24 and if its not too much trouble for my agent

Appreciate the offer! Please don't add them here though – let's keep this PR clean and focused on the material-function work.

gotcha lmk if it doesn't work on a previous unreal ver

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (1)

plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AudioHandlers.cpp (1)

2287-2291: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

create_dialogue_wave can still reject a valid USoundWave on short-name collisions.

Line 2287 still resolves through ResolveSoundAsset(...), which searches both cues and waves by name; if a cue with the same short name is returned first, the cast fails and you emit INVALID_ARGUMENT even when a valid wave exists.

Suggested fix

-  USoundWave *SoundWave = Cast<USoundWave>(ResolveSoundAsset(SoundPath));
+  USoundWave *SoundWave = nullptr;
+  if (UEditorAssetLibrary::DoesAssetExist(SoundPath)) {
+    SoundWave = Cast<USoundWave>(UEditorAssetLibrary::LoadAsset(SoundPath));
+  } else if (!SoundPath.Contains(TEXT("/"))) {
+    // Resolve by short name, but restrict class query to USoundWave only.
+    SoundWave = ResolveSoundWaveAsset(SoundPath);
+  }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AudioHandlers.cpp`
around lines 2287 - 2291, The code in create_dialogue_wave uses
ResolveSoundAsset(...) and immediately casts to USoundWave, causing
INVALID_ARGUMENT when ResolveSoundAsset returns a SoundCue with the same short
name; change the logic in create_dialogue_wave to, after the initial
Cast<USoundWave>(ResolveSoundAsset(SoundPath)) fails, attempt to explicitly
locate/load a USoundWave asset by the given SoundPath/short name (for example
calling LoadObject<USoundWave> or querying the AssetRegistry for a USoundWave
with that name) before sending SendAutomationError, and only emit the
INVALID_ARGUMENT if no USoundWave can be found; keep references to
ResolveSoundAsset, USoundWave, SendAutomationError and
RequestingSocket/RequestId to locate and update the failing branch.

🤖 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_WidgetAuthoringHandlers.cpp`:
- Around line 3551-3554: In the set_style write-mode handler,
McpSafeAssetSave(WidgetBP) is called but its boolean return value is ignored;
capture the return value, and if it returns false handle the failure by
emitting/logging an error (include WidgetBP name/context), ensure the caller is
informed the save failed (do not report success), and keep the package dirty
(WidgetBP->MarkPackageDirty() already called) so it can be retried; update the
handler around McpSafeAssetSave, WidgetBP, and the surrounding write-mode
result/reporting logic to use the save result to determine success vs failure.

In `@src/tools/handlers/material-authoring-handlers.ts`:
- Around line 528-530: The code is incorrectly renaming the wire field
additionalOutputs to outputs (assigning payload.outputs = additionalOutputs),
breaking the tool-definition/bridge contract; change those assignments to
preserve the original field name by setting payload.additionalOutputs =
additionalOutputs instead; update both occurrences that reference
payload.outputs (the one shown and the similar block around the second
occurrence) so the payload keeps additionalOutputs exactly as received.

---

Duplicate comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AudioHandlers.cpp`:
- Around line 2287-2291: The code in create_dialogue_wave uses
ResolveSoundAsset(...) and immediately casts to USoundWave, causing
INVALID_ARGUMENT when ResolveSoundAsset returns a SoundCue with the same short
name; change the logic in create_dialogue_wave to, after the initial
Cast<USoundWave>(ResolveSoundAsset(SoundPath)) fails, attempt to explicitly
locate/load a USoundWave asset by the given SoundPath/short name (for example
calling LoadObject<USoundWave> or querying the AssetRegistry for a USoundWave
with that name) before sending SendAutomationError, and only emit the
INVALID_ARGUMENT if no USoundWave can be found; keep references to
ResolveSoundAsset, USoundWave, SendAutomationError and
RequestingSocket/RequestId to locate and update the failing branch.

🪄 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: ab4f2144-385a-42d2-af5c-526f5693f824

📥 Commits

Reviewing files that changed from the base of the PR and between 3c9cd5c and d7b629b.

📒 Files selected for processing (4)

• plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AssetWorkflowHandlers.cpp
• plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AudioHandlers.cpp
• plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_WidgetAuthoringHandlers.cpp
• src/tools/handlers/material-authoring-handlers.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_AssetWorkflowHandlers.cpp

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle save failure in set_style write mode

Line 3553 calls McpSafeAssetSave(WidgetBP) but ignores its return value, so the handler can report success even when the asset was not persisted.

💡 Suggested fix

-                WidgetBP->MarkPackageDirty();
-                McpSafeAssetSave(WidgetBP);
+                WidgetBP->MarkPackageDirty();
+                if (!McpSafeAssetSave(WidgetBP))
+                {
+                    SendAutomationError(RequestingSocket, RequestId,
+                        FString::Printf(TEXT("Failed to save widget blueprint after setting property '%s'"), *PropertyName),
+                        TEXT("SAVE_FAILED"));
+                    return true;
+                }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/McpAutomationBridge_WidgetAuthoringHandlers.cpp`
around lines 3551 - 3554, In the set_style write-mode handler,
McpSafeAssetSave(WidgetBP) is called but its boolean return value is ignored;
capture the return value, and if it returns false handle the failure by
emitting/logging an error (include WidgetBP name/context), ensure the caller is
informed the save failed (do not report success), and keep the package dirty
(WidgetBP->MarkPackageDirty() already called) so it can be retried; update the
handler around McpSafeAssetSave, WidgetBP, and the surrounding write-mode
result/reporting logic to use the save result to determine success vs failure.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep the wire field name as additionalOutputs in both custom-expression payloads.

These two assignments currently rename additionalOutputs to outputs, which breaks the tool-definition/bridge contract and can silently drop output updates.

Suggested fix

- if (additionalOutputs != null) {
-   payload.outputs = additionalOutputs;
- }
+ if (additionalOutputs != null) {
+   payload.additionalOutputs = additionalOutputs;
+ }

- if (hasAdditionalOutputs) payload.outputs = additionalOutputs;
+ if (hasAdditionalOutputs) payload.additionalOutputs = additionalOutputs;

Based on learnings: “the JSON field name for custom material expression nodes must remain additionalOutputs and must not be renamed to outputs … changing it would silently break interoperability across the tool-definition/handler boundary.”

Also applies to: 1197-1199

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/tools/handlers/material-authoring-handlers.ts` around lines 528 - 530,
The code is incorrectly renaming the wire field additionalOutputs to outputs
(assigning payload.outputs = additionalOutputs), breaking the
tool-definition/bridge contract; change those assignments to preserve the
original field name by setting payload.additionalOutputs = additionalOutputs
instead; update both occurrences that reference payload.outputs (the one shown
and the similar block around the second occurrence) so the payload keeps
additionalOutputs exactly as received.

[devin-ai-integration]

Devin Review found 1 new potential issue.

View 30 additional findings in Devin Review.

[devin-ai-integration]

🟡 Inconsistent indentation in add_custom_expression causes additionalOutputs mapping to appear out of scope

In the add_custom_expression case, lines 1527–1529 have zero-level indentation while surrounding code uses 8-space indentation. While JavaScript/TypeScript doesn't use indentation for scoping (so this is not a runtime error), the visual mismatch makes the code appear to be outside its containing case block, which can easily lead to maintenance errors.

Code in question

        if (inputs != null) {
          payload.inputs = inputs;
        }
if (additionalOutputs != null) {
  payload.outputs = additionalOutputs;
}

Similarly, lines 1185–1186 in update_custom_expression have 2-space indentation instead of the surrounding 8-space style.

(Refers to lines 1527-1529)

---

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