feat(bridge): expose the in-flight automation action for external monitoring#503
feat(bridge): expose the in-flight automation action for external monitoring#503SoloGorilla wants to merge 2 commits into
Conversation
…itoring Adds a thread-safe accessor, McpAutomationBridge::GetInFlightAction(), that reports which automation action ProcessAutomationRequest is currently executing. It is set around the handler dispatch and cleared on every exit path; the guarding lock is held only for the brief set/get, never during the handler, so an external observer (e.g. an off-thread watchdog) can read the in-flight action even while a handler blocks the game thread — which is exactly when attribution is most useful (it lets a stall be blamed on the specific tool that caused it). The getter is exported (MCPAUTOMATIONBRIDGE_API) for cross-module use; no behavior changes for existing callers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis change adds a thread-safe in-flight automation action tracker in McpAutomationBridge and exposes it through ChangesIn-flight Action Tracking
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Core/Requests/McpAutomationBridge_ProcessRequest.cpp`:
- Around line 12-31: `GetInFlightAction()` is only defined in the .cpp, so other
modules still can’t call it despite the export macro. Add a public declaration
for `McpAutomationBridge::GetInFlightAction()` in the appropriate header used by
external modules, and keep the existing `.cpp` definition in
`McpAutomationBridge_ProcessRequest` aligned with that declaration so
cross-module access works.
🪄 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: 35839912-70c1-4f9b-8733-4e6d62aa96df
📒 Files selected for processing (1)
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Core/Requests/McpAutomationBridge_ProcessRequest.cpp
| // Publish the currently-executing automation action so external tooling (e.g. an off-thread watchdog) can attribute | ||
| // a game-thread stall to the tool that was in flight. The lock is held ONLY for the brief set/get — never during | ||
| // the handler — so a reader can observe the action off-thread even while a handler blocks the game thread. The | ||
| // getter is exported so another module can link it. | ||
| namespace McpAutomationBridge | ||
| { | ||
| static FCriticalSection GInFlightActionCS; | ||
| static FString GInFlightAction; | ||
| static void SetInFlightAction(const FString& InAction) | ||
| { | ||
| FScopeLock Lock(&GInFlightActionCS); | ||
| GInFlightAction = InAction; | ||
| } | ||
| MCPAUTOMATIONBRIDGE_API FString GetInFlightAction() | ||
| { | ||
| FScopeLock Lock(&GInFlightActionCS); | ||
| return GInFlightAction; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether GetInFlightAction is declared in a header.
rg -n "GetInFlightAction" --type=cpp --type=h -g '*.h'Repository: ChiR24/Unreal_mcp
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Files mentioning InFlightAction =="
rg -n "InFlightAction|GetInFlightAction|SetInFlightAction" .
echo
echo "== Relevant headers under the plugin =="
git ls-files 'plugins/McpAutomationBridge/**/*.h'
echo
echo "== Public API declarations for this symbol =="
rg -n "MCPAUTOMATIONBRIDGE_API.*GetInFlightAction|GetInFlightAction" plugins/McpAutomationBridge --glob '*.h'
echo
echo "== Current file context =="
sed -n '1,180p' plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Core/Requests/McpAutomationBridge_ProcessRequest.cppRepository: ChiR24/Unreal_mcp
Length of output: 27512
Expose GetInFlightAction() in a public header
MCPAUTOMATIONBRIDGE_API on the .cpp definition isn’t enough for other modules to call it; add a public declaration if cross-module access is intended.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Core/Requests/McpAutomationBridge_ProcessRequest.cpp`
around lines 12 - 31, `GetInFlightAction()` is only defined in the .cpp, so
other modules still can’t call it despite the export macro. Add a public
declaration for `McpAutomationBridge::GetInFlightAction()` in the appropriate
header used by external modules, and keep the existing `.cpp` definition in
`McpAutomationBridge_ProcessRequest` aligned with that declaration so
cross-module access works.
Review follow-up: MCPAUTOMATIONBRIDGE_API on the .cpp definition alone is not callable from other modules without a matching declaration. Declare McpAutomationBridge::GetInFlightAction() in the public McpAutomationBridgeSubsystem.h so external consumers link against it without re-declaring the extern themselves.
|
Addressed: |
|
@coderabbitai review |
✅ Action performedReview finished.
|
What
Adds a thread-safe accessor,
McpAutomationBridge::GetInFlightAction(), that reports which automation actionProcessAutomationRequestis currently executing.Why
Automation handlers run synchronously on the game thread. If a handler blocks it (a modal, or a long synchronous editor op), external tooling — e.g. an off-thread watchdog — can detect that the game thread stalled, but has no way to know which tool caused it. This accessor closes that gap: it lets a stall be attributed to the specific in-flight action.
How
The action is stored in a
namespace-scopedFStringguarded by anFCriticalSection. It is set right before dispatch and cleared on every exit path (including early returns and exceptions). The lock is held only for the brief set/get, never during the handler, so a reader can observe the action off-thread even while a handler is blocking the game thread — which is exactly when attribution matters most.The getter is exported (
MCPAUTOMATIONBRIDGE_API) for cross-module use. No behavior changes for existing callers; the setter/getter are additive.Verification
Built against UE 5.8 (
Result: Succeeded, 0 errors) and smoke-tested live: an off-thread watchdog readingGetInFlightAction()correctly named the in-flight tool while a handler was deliberately blocking the game thread.