fix(plugin): plan-panel buffer + TCP server use-after-free (live-session reliability fixes)#232
fix(plugin): plan-panel buffer + TCP server use-after-free (live-session reliability fixes)#232zajalist wants to merge 16 commits into
Conversation
…onstruction Root cause: HaybaMCPMainPanel constructs sub-panels lazily on first tab visit. The Plan panel set Module->PlanPanel only when the user clicked the Plan tab. If hayba_propose_plan arrived BEFORE that first visit, the AsyncTask game-thread marshal pinned a null weak ref and silently dropped the plan — but the handler still returned received:true, step_count:N. Agents got a false success signal; users saw an empty Plan tab. Fix: - Extract FHaybaPlanStep into Public/HaybaMCPPlanTypes.h so the module can buffer it without including the private panel header. - Add a thread-safe pending-plan buffer on FHaybaMCPModule (StashPendingPlan / ConsumePendingPlan / HasPendingPlan). - HandleProposePlan now: always stashes, then tries to push to a live panel; uses TPromise/TFuture to find out (on the TCP worker thread) whether the panel actually existed, and reports that back as panel_visible in the response. Adds buffered + hint fields too. - SHaybaMCPMainPanel, when constructing the Plan tab, consumes any buffered plan and calls LoadPlan immediately so plans proposed before first tab visit become visible the moment the tab opens. The agent now gets an honest response telling it whether the user can see the plan right now or has to open the Plan tab. The plan is never lost regardless of timing.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughBuffers proposed plans until the Plan panel exists, introduces a game-thread MPSC dispatcher, centralizes per-connection TCP ownership, extends actor placement with landscape snapping and validators, adds tooling/startup probes, hardens idle/wait logic, and migrates AsyncTask(GameThread) callsites to the dispatcher. ChangesPending Plan Buffering System
Game-Thread Dispatcher (HaybaThreading)
TCP Connection Ownership Refactor
MCP Tools, Startup Probe, and Python Wrapper
Actor Spawn & Transform Engine Changes
Validator Rules & Hooks
UI Rename, Idle Hardening & Callsite Migrations
Sequence DiagramsequenceDiagram
participant CommandHandler
participant Module
participant GameThread
participant PlanPanel
CommandHandler->>Module: StashPendingPlan(steps, awaitSecs)
CommandHandler->>GameThread: ExecuteOnGameThread(delivery lambda)
GameThread->>PlanPanel: If exists -> LoadPlan(steps, awaitSecs)
GameThread->>Module: ConsumePendingPlan()
GameThread-->>CommandHandler: Resolve/timeout -> panel_visible / buffered
PlanPanel->>Module: ConsumePendingPlan() on creation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
…dow) SMultiColumnTableRow has a protected `Box` member. UE 5.7 promotes C4458 (declaration hides class member) to error in plugin builds, so the local `TSharedRef<SHorizontalBox> Box = ...` in the validator panel column factory broke the build. Rename to ActionBox. No behavior change — the local was scoped to the lambda and only used to compose the action-column row.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPCommandHandler.cpp`:
- Around line 447-471: The code blocks the game thread by dispatching an
AsyncTask to ENamedThreads::GameThread and then calling PanelOpenFuture.WaitFor;
instead, in HandleProposePlan detect if you are already on the game thread
(IsInGameThread) and run the delivery inline (call Panel->LoadPlan(...) and
M2->ConsumePendingPlan(...) and set PanelOpenPromise accordingly), and only
dispatch an AsyncTask when off the game thread; remove or avoid waiting on
PanelOpenFuture.WaitFor on the game thread—return immediately (or use an
asynchronous callback) so the dispatched lambda can run without being blocked.
Ensure you update/resolve PanelOpenPromise and PanelOpenFuture usage
consistently and reference AsyncTask, Panel->LoadPlan, M2->ConsumePendingPlan,
PanelOpenPromise, PanelOpenFuture, and HandleProposePlan when making the change.
In `@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPModule.cpp`:
- Around line 430-435: Clamp the incoming AwaitSecs in
FHaybaMCPModule::StashPendingPlan before storing it so negative or huge values
can't be buffered; replace the direct assignment PendingPlanAwaitSecs =
AwaitSecs with a clamped value (e.g. use FMath::Clamp(AwaitSecs, 0,
kMaxPendingAwaitSecs) or a constexpr/const int32 MAX_PENDING_AWAIT_SECS like
300), keeping the FScopeLock and assignments to PendingPlanSteps and
bPendingPlanConsumed unchanged and referencing PendingPlanAwaitSecs,
PendingPlanSteps, bPendingPlanConsumed, and PendingPlanLock.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 96afee21-8fe5-4914-8c9e-16d56117face
📒 Files selected for processing (6)
unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPCommandHandler.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPMainPanel.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPModule.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPPlanPanel.hunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Public/HaybaMCPModule.hunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Public/HaybaMCPPlanTypes.h
| void FHaybaMCPModule::StashPendingPlan(const TArray<FHaybaPlanStep>& Steps, int32 AwaitSecs) | ||
| { | ||
| FScopeLock Lock(&PendingPlanLock); | ||
| PendingPlanSteps = Steps; | ||
| PendingPlanAwaitSecs = AwaitSecs; | ||
| bPendingPlanConsumed = false; |
There was a problem hiding this comment.
Clamp AwaitSecs before storing pending plans.
Line 434 stores untrusted AwaitSecs directly. Negative/huge values can propagate into LoadPlan and break expected timeout semantics. Clamp once at stash-time so buffered state stays valid.
Suggested patch
void FHaybaMCPModule::StashPendingPlan(const TArray<FHaybaPlanStep>& Steps, int32 AwaitSecs)
{
FScopeLock Lock(&PendingPlanLock);
PendingPlanSteps = Steps;
- PendingPlanAwaitSecs = AwaitSecs;
+ PendingPlanAwaitSecs = FMath::Clamp(AwaitSecs, 0, 24 * 60 * 60);
bPendingPlanConsumed = false;
}📝 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.
| void FHaybaMCPModule::StashPendingPlan(const TArray<FHaybaPlanStep>& Steps, int32 AwaitSecs) | |
| { | |
| FScopeLock Lock(&PendingPlanLock); | |
| PendingPlanSteps = Steps; | |
| PendingPlanAwaitSecs = AwaitSecs; | |
| bPendingPlanConsumed = false; | |
| void FHaybaMCPModule::StashPendingPlan(const TArray<FHaybaPlanStep>& Steps, int32 AwaitSecs) | |
| { | |
| FScopeLock Lock(&PendingPlanLock); | |
| PendingPlanSteps = Steps; | |
| PendingPlanAwaitSecs = FMath::Clamp(AwaitSecs, 0, 24 * 60 * 60); | |
| bPendingPlanConsumed = false; | |
| } |
🤖 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 `@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPModule.cpp`
around lines 430 - 435, Clamp the incoming AwaitSecs in
FHaybaMCPModule::StashPendingPlan before storing it so negative or huge values
can't be buffered; replace the direct assignment PendingPlanAwaitSecs =
AwaitSecs with a clamped value (e.g. use FMath::Clamp(AwaitSecs, 0,
kMaxPendingAwaitSecs) or a constexpr/const int32 MAX_PENDING_AWAIT_SECS like
300), keeping the FScopeLock and assignments to PendingPlanSteps and
bPendingPlanConsumed unchanged and referencing PendingPlanAwaitSecs,
PendingPlanSteps, bPendingPlanConsumed, and PendingPlanLock.
…ter-free in TCP server
Root cause of UE EXCEPTION_ACCESS_VIOLATION at HaybaMCPTcpServer.cpp:123
during landscape_import session on 2026-05-23:
- HandleClientConnection (background worker) read a request, dispatched
AsyncTask(GameThread, [..., ClientSocket /* raw ptr */])
- Game thread did the heavy World->SpawnActor<ALandscape> work
- Meanwhile client disconnected (or the OS reset the socket)
- Worker observed disconnect, called DestroySocket(ClientSocket)
- Game thread eventually returned, called SendMessage on the freed
pointer -> access violation, hard crash
PR #229 made SpawnActor itself game-thread-safe, but the TCP server
captured the FSocket raw pointer into the response lambda with no
liveness guard. The slow landscape_import widened the race window
from negligible to catastrophic.
Fix: wrap FSocket* in struct FHaybaMCPConnection (POD-ish, owns the
socket, destructor closes + destroys exactly once). The worker holds a
TSharedPtr, every game-thread response lambda captures its own copy by
value. The socket survives until the last ref drops. Adds a bAlive
flag + per-connection SendLock so:
- SendMessage no-ops cleanly if the worker has marked the conn dead
while the lambda was running (e.g., client gone)
- concurrent game-thread response lambdas for the same connection
cannot interleave their header+body writes
ReadMessage/SendMessage now take const TSharedPtr<FHaybaMCPConnection>&
instead of FSocket*. Header layout unchanged: 4-byte big-endian length
prefix + JSON body.
No behavioral change for the happy path. The crash path becomes a
verbose log line + early return.
zajalist
changed the title
This session burned ~15 min chasing "why isn't hayba_propose_plan a tool?" when the answer was that Claude Code launches the MCP server from a stale dist build that pre-dated 6 hours of merged changes. The agent had no way to know — listTools just returned the older set silently. Now main() prints a startup banner to stderr (which Claude Code shows in the /mcp panel and MCP server logs): [hayba-mcp] build: 2026-05-23T08:58:47.349Z And if any .ts file under src/ is newer than the running dist/, it prints a STALE BUILD warning with the offending file path and the exact rebuild command: [hayba-mcp] STALE BUILD — source is 12.4 min newer than dist. [hayba-mcp] newest src: D:\Hackathons\...\src\tools\index.ts [hayba-mcp] fix: cd mcp-tools/hayba-mcp && npm run build:server, then /mcp reconnect Probe is best-effort — if it throws (e.g. cannot stat src/, which happens in some packaged-binary scenarios) it logs the probe failure and continues. Never fails startup. Skips node_modules and __tests__ subtrees so changes to vendored deps or test files do not spuriously trip the warning. Hooked in before registerTools so the warning lands BEFORE any tool registration log noise, making it visible at the top of the MCP log.
…yba_invoke for them
When toolRouting is 'deferred' (the default), the gamma-hybrid router
captures every server.tool(...) registration into a map but only
re-registers the ALWAYS_ON_META subset with MCP. Tools outside that
set are reachable only via hayba_invoke or hayba_pack_load.
hayba_propose_plan was NOT in ALWAYS_ON_META, which meant:
- Plan Mode is on by default for fresh sessions
- The UE plugin's destructive-op gate rejects spawn / delete /
set_property until the agent calls hayba_propose_plan and the
user approves
- But the agent had no direct tool — it had to fall back to
hayba_invoke({name: "hayba_propose_plan", ...})
- For a workflow that is, by definition, on the critical path for
any scene authoring, this is gratuitous friction
Add hayba_propose_plan + hayba_mark_plan_step (the natural pair —
streams progress back to the Plan tab as each step completes) to the
always-on set and wire passthrough registrations.
Smoke test (node dist/index.js + tools/list): 24 -> 26 tools, both
present, no other surface change.
…h them
Companion to the ALWAYS_ON_META promotion in the prior commit:
hayba_invoke routes via the schema registry (getRawShape) which is
populated by reg(name, shape, cost, returns) in recordEagerSchemas.
Without an entry here, hayba_invoke({name: "hayba_propose_plan", ...})
returns {ok:false, error:{kind:"unknown_tool"}} even though the tool
is in the captured map.
Add reg() entries for both Plan Mode meta tools so:
- Operators who keep them off the always-on surface can still reach
them via hayba_invoke (the documented fallback for unloaded packs).
- get_tool_signature("hayba_propose_plan") returns a real shape
instead of having to fall through to the legacy sidecar.
…pt multi-statement scripts Three connected MCP bugs surfaced during the 2026-05-23 Palestine scene session, all blocking the same flow: "spawn this mesh into the scene". A. actor_spawn rejected mesh asset paths Agents naturally call actor_spawn(class_path="/Game/.../SM_Foo") and get "class not found" because LoadClass<AActor> only accepts UClass paths. They then fall back to python_run + EditorAssetLibrary, which is the wrong tool for scene composition. Fix (HaybaMCPActorHandler.cpp): if class_path doesn't resolve to a UClass, try LoadObject<UObject>. If it's a UStaticMesh, spawn an AStaticMeshActor + SetStaticMesh + SetMobility(Movable). If it's a USkeletalMesh, spawn an ASkeletalMeshActor + SetSkeletalMeshAsset. Otherwise return a clear error naming the asset's actual class. B. python_run was hard-wired to single-statement mode Cmd.ExecutionMode was EPythonCommandExecutionMode::ExecuteStatement, which compiles in `single` mode. Anything beyond a one-liner fails with "SyntaxError: multiple statements found while compiling a single statement". The print-redirect wrapper from PR #228 is itself multi-line, so EVERY user script that went through it failed parse silently. Fix (HaybaMCPPythonHandler.cpp): switch to EPythonCommandExecutionMode::ExecuteFile. Multi-statement scripts now run as-is. C. python_run wrapper triggered the tier-3 classifier The wrapper used exec(compile(_hayba_user_src, ...)) which contains "compile(" — flagged as Tier 3 (filesystem/subprocess). Any user script with a custom snap_z or anything legitimate was blocked without an unsafe override, for no real reason: there is no fs or subprocess in the wrapper. Fix (python-run.ts): drop the compile() wrapper entirely. ExecuteFile now handles the multi-line preamble natively, and a plain exec(_hayba_user_src, _hayba_user_globals) gives the user the isolated globals dict with print pre-bound. Wrapper is now Tier-2 (mutation) — appropriate for an editor-mutating tool. Net effect: actor_spawn with a mesh path Just Works, no python_run needed for the common scene-composition case. When python_run IS needed, multi-line scripts execute and benign scripts no longer trip Tier 3.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@mcp-tools/hayba-mcp/src/tools/python/python-run.ts`:
- Line 57: Update the test snapshot in python-run.test.ts to match the new
wrapper output from wrapScriptForPrintRedirect: replace the old expected string
containing exec(compile(...)) with the new exec(_hayba_user_src,
_hayba_user_globals) invocation so the snapshot matches the emitted wrapper;
ensure the snapshot text exactly matches the wrapper produced by
wrapScriptForPrintRedirect.
In
`@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPPythonHandler.cpp`:
- Around line 78-87: The change sets FPythonCommandEx.ExecutionMode to
EPythonCommandExecutionMode::ExecuteFile which allows the Command string to be
treated as a filename (with args) as well as inline code; update
HaybaMCPPythonHandler::Run to validate/restrict the incoming script before
assigning Cmd.Command and Cmd.ExecutionMode to ensure only literal code is
accepted (or explicitly detect and reject file-path forms) so
ExecPythonCommandEx cannot be used to bypass Tier 3 restrictions—add checks for
newline/semicolon patterns, disallow path separators and common file extensions,
or require an explicit “literal” flag, and fail early with a clear error when
the script appears to be a file reference.
In `@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPTcpServer.h`:
- Around line 23-35: The bAlive flag in FHaybaMCPConnection is concurrently
accessed from the worker thread (HandleClientConnection) and game-thread lambdas
(SendMessage), causing a data race; change the member type from plain bool
bAlive to an atomic type (e.g., std::atomic<bool> or Unreal's TAtomic<bool>) in
struct FHaybaMCPConnection and update all uses (reads/writes) in
HandleClientConnection and SendMessage to operate on the atomic (keeping
SendLock for serializing writes), ensuring atomic load/store semantics so
visibility and thread-safety are guaranteed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35fbbcdd-a015-4efd-a8fc-0cb44b1d3bd5
📒 Files selected for processing (9)
mcp-tools/hayba-mcp/src/index.tsmcp-tools/hayba-mcp/src/tools/index.tsmcp-tools/hayba-mcp/src/tools/python/python-run.tsmcp-tools/hayba-mcp/src/tools/routing/register.tsunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPTcpServer.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPTcpServer.hunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/Slate/SHaybaValidatorPanel.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPActorHandler.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPPythonHandler.cpp
| '_hayba_user_globals = {"__name__": "__main__", "print": _hayba_print, "unreal": _hayba_unreal}', | ||
| `_hayba_user_src = """${escaped}"""`, | ||
| 'exec(compile(_hayba_user_src, "<python_run>", "exec"), _hayba_user_globals)', | ||
| 'exec(_hayba_user_src, _hayba_user_globals)', |
There was a problem hiding this comment.
Update the python-run snapshot expectation to match the new wrapper output.
wrapScriptForPrintRedirect now emits direct exec(_hayba_user_src, _hayba_user_globals), but the existing snapshot in mcp-tools/hayba-mcp/src/tools/python/python-run.test.ts still expects exec(compile(...)), so that test will drift/fail.
✅ Suggested test expectation update
- 'exec(compile(_hayba_user_src, "<python_run>", "exec"), _hayba_user_globals)',
+ 'exec(_hayba_user_src, _hayba_user_globals)',🤖 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 `@mcp-tools/hayba-mcp/src/tools/python/python-run.ts` at line 57, Update the
test snapshot in python-run.test.ts to match the new wrapper output from
wrapScriptForPrintRedirect: replace the old expected string containing
exec(compile(...)) with the new exec(_hayba_user_src, _hayba_user_globals)
invocation so the snapshot matches the emitted wrapper; ensure the snapshot text
exactly matches the wrapper produced by wrapScriptForPrintRedirect.
| // Execute. ExecuteFile (not ExecuteStatement) so the script can contain | ||
| // multiple top-level statements — imports, function defs, loops, etc. | ||
| // ExecuteStatement compiles in `single` mode, which raises | ||
| // SyntaxError: multiple statements found while compiling a single statement | ||
| // on the second line of any non-trivial script. Confirmed in the | ||
| // 2026-05-23 Palestine scene session: every print-redirect wrapper | ||
| // ever shipped was unusable because the wrapper itself is multi-line. | ||
| FPythonCommandEx Cmd; | ||
| Cmd.Command = Code; | ||
| Cmd.ExecutionMode = EPythonCommandExecutionMode::ExecuteStatement; | ||
| Cmd.ExecutionMode = EPythonCommandExecutionMode::ExecuteFile; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Unreal Engine EPythonCommandExecutionMode ExecuteFile vs ExecuteStatement behavior and security implications
💡 Result:
In Unreal Engine, EPythonCommandExecutionMode defines how Python commands are processed by the PythonScriptPlugin [1][2]. The two primary modes for execution are ExecuteFile and ExecuteStatement [1][3]. ExecuteFile: This mode treats the provided input as a file path or a script to be executed as a file [1]. It allows for the execution of complex scripts containing multiple statements and supports optional arguments [1]. This is the standard approach for running standalone Python scripts or automation tasks within the engine [1]. ExecuteStatement: This mode executes the provided input as a single Python statement [1]. It is designed for quick, inline commands and will print the result of the statement [1]. It cannot be used to run files [1]. Security Implications: Both modes inherently allow for arbitrary code execution within the context of the Unreal Editor process [1][4]. Because the Python environment in Unreal Engine has access to the engine's API (the unreal module) and the underlying operating system, any script executed via these modes runs with the same privileges as the Unreal Editor itself [5][4]. 1. Trust Boundary: There is no built-in sandbox for Python execution in Unreal Engine [4]. If an application or plugin accepts user-provided input (e.g., from a network request, a file, or an LLM) and passes it to ExecuteFile or ExecuteStatement, it creates a critical security vulnerability [6][4]. An attacker can use this to execute arbitrary OS commands, access or modify files, or exfiltrate sensitive data [7][6][8]. 2. Input Sanitization: Because these modes are designed to execute code, they do not perform sanitization [4]. Developers must treat any input passed to these functions as untrusted [4]. If dynamic execution is required, it should be strictly gated, and input should be validated against a strict allowlist rather than attempting to sanitize or blacklist dangerous commands [7][4]. 3. Principle of Least Privilege: Since the Python script runs with the privileges of the user running the Unreal Editor, any compromise of the Python execution environment can lead to a full compromise of the user's session or machine [6][8]. Avoid executing Python scripts from untrusted sources or external files that could be modified by other processes [6][4].
Citations:
- 1: https://dev.epicgames.com/documentation/en-us/unreal-engine/python-api/class/PythonCommandExecutionMode?application_version=5.7
- 2: https://dev.epicgames.com/documentation/en-us/unreal-engine/API/Plugins/PythonScriptPlugin
- 3: https://dev.epicgames.com/documentation/unreal-engine/API/Plugins/PythonScriptPlugin/EPythonCommandExecutionMode?lang=en-US
- 4: https://dev.to/mspro3210/when-prompts-become-shells-the-tool-registry-is-the-attack-surface-52n6
- 5: https://www.youtube.com/watch?v=voGBWQ-Gi6I
- 6: https://www.sentinelone.com/vulnerability-database/cve-2026-25632/
- 7: https://dev.to/kyb8801/sympyparseexpr-will-run-ossystem-if-you-let-it-heres-the-ast-gate-that-stopped-me-from-1nbe
- 8: Bug: Shell command injection via single-quote breakout in MCP config JSON interpolation e2b-dev/E2B#1154
🏁 Script executed:
#!/bin/bash
# Search for ExecuteFile usage patterns and documentation in the codebase
rg -n -C5 -g '*.h' -g '*.cpp' 'EPythonCommandExecutionMode::ExecuteFile'
# Look for any existing documentation or comments about execution modes
rg -n -C3 'ExecuteStatement|ExecuteFile' --type cpp | head -50Repository: zajalist/hayba
Length of output: 3851
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPPythonHandler.cpp"
# Show surrounding context for lines 60-120 (includes how Code is built and any tier/security gating)
nl -ba "$FILE" | sed -n '40,140p'
# Find where the handler gets/builds `Code` and whether it’s user-provided / tier-gated
rg -n "Code\\b|ExecutionMode|ExecuteStatement|ExecuteFile|tier|security|untrusted|allowlist|trust" "$FILE"
# Check whether ExecuteStatement is used anywhere else
rg -n "EPythonCommandExecutionMode::ExecuteStatement" unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit || trueRepository: zajalist/hayba
Length of output: 101
🌐 Web query:
Unreal Engine PythonScriptPlugin FPythonCommandEx ExecPythonCommandEx ExecuteFile ExecuteStatement documentation
💡 Result:
In Unreal Engine’s PythonScriptPlugin (C++ API), the key type is FPythonCommandEx. It carries the command string and metadata for how it should be executed: literal Python code vs a file, and what kind of execution mode to use. FPythonCommandEx includes fields like Command, CommandResult, ExecutionMode, FileExecutionScope, Flags, and LogOutput [1]. 1) FPythonCommandEx (what it is) - Purpose: “Extended information when executing Python commands” [1]. - Relevant fields: - Command (FString): “The command to run. This may be literal Python code, or a file (with optional arguments) to run.” [1] - CommandResult (FString): result of running the command [1] - ExecutionMode (EPythonCommandExecutionMode): controls how to execute the command [1] - FileExecutionScope (EPythonFileExecutionScope): controls scope when executing Python files [1] - Flags (EPythonCommandFlags): flags controlling how the command should be run [1] - LogOutput (TArray): log output captured during execution [1] 2) Execution modes you asked about (ExecuteFile / ExecuteStatement) Epic’s public API names correspond to “execution mode” strings that distinguish running a file vs a single statement. From Unreal’s Python command execution mode enum (Python API documentation): - EXECUTE_FILE: “Execute the Python command as a file… either a literal Python script containing multiple statements, or a file with optional arguments.” [2] - EXECUTE_STATEMENT: “Execute the Python command as a single statement… This mode cannot run files.” [2] - EVALUATE_STATEMENT: evaluate a single statement and return the result; cannot run files [2] So, mapping to your terms: - ExecuteFile corresponds to EXECUTE_FILE behavior (run a Python script/file, supporting multiple statements / optional args) [2]. - ExecuteStatement corresponds to EXECUTE_STATEMENT behavior (single statement only; cannot run files) [2]. 3) How ExecPythonCommandEx is intended to be used (high-level) Although I couldn’t access a non-403 page for the exact IPythonScriptPlugin::ExecPythonCommandEx function documentation in this research run, the usage pattern is shown in an Unreal-side example that distinguishes ExecuteStatement vs file-like execution by setting FPythonCommandEx.ExecutionMode and FPythonCommandEx.Command, then calling ExecPythonCommandEx [3]. In that example: - Ex.ExecutionMode is set to EPythonCommandExecutionMode::ExecuteStatement for single-statement execution. - Ex.Command is set to the text to execute. - ExecPythonCommandEx(Ex) is called; Ex.CommandResult would hold the output/result (consistent with FPythonCommandEx’s documented fields) [1][3]. 4) PythonScriptPlugin documentation entry point Epic’s plugin API documentation lists FPythonCommandEx and related types under the PythonScriptPlugin API reference [4]. Practical recommendation (based on the above) - If you want to run another script (i.e., file) from Unreal’s embedded Python execution, use the mode equivalent to EXECUTE_FILE / ExecuteFile, and put the script path (optionally with arguments) into FPythonCommandEx.Command [1][2]. - If you only need to run one statement/expression (and you’re not pointing at a file), use ExecuteStatement / EXECUTE_STATEMENT [2].
Citations:
- 1: https://dev.epicgames.com/documentation/en-us/unreal-engine/API/Plugins/PythonScriptPlugin/FPythonCommandEx
- 2: https://dev.epicgames.com/documentation/en-us/unreal-engine/python-api/class/PythonCommandExecutionMode?application_version=5.7
- 3: https://minimaleffort.tech/unreal-python-custom-script-editor/
- 4: https://dev.epicgames.com/documentation/en-us/unreal-engine/API/Plugins/PythonScriptPlugin
🌐 Web query:
site:dev.epicgames.com Unreal PythonScriptPlugin executes arbitrary code no sandbox security implications ExecPythonCommandEx
💡 Result:
Answer: Yes—Unreal’s PythonScriptPlugin “ExecPythonCommandEx” (exposed as PythonScriptLibrary.execute_python_command_ex in the Python API) is an arbitrary-Python-code execution capability by design, and therefore it has clear security implications if an attacker can influence the command/script content. The plugin provides execution-mode and file-scope options, but these are not a sandbox/safety boundary. Grounded evidence: - The Unreal Python API explicitly documents that execute_python_command_ex “Execute[s] the given Python command” and that the input “may be literal Python code, or a file (with optional arguments) that you want to run.” [1] - The API further shows that execution_mode is about how the command is executed (e.g., as a file vs a single statement), and not about restricting capabilities. For example, EXECUTE_FILE “allows you to execute either a literal Python script containing multiple statements, or a file with optional arguments.” [2] - For Python file execution, the only “scope” described is whether imports/changes are isolated via unique locals/globals (PRIVATE) vs sharing console locals/globals (PUBLIC). This is about environment behavior/side effects, not sandboxing or permission restriction. [3] - Internally, FPythonCommandEx includes fields for “Command” (the command to run) plus “ExecutionMode”, “FileExecutionScope”, “Flags”, and captured “LogOutput”, reinforcing that the mechanism is a general-purpose “run this Python command” feature. [4] Security implications (practical): - If any code path allows untrusted data to reach execute_python_command_ex/ExecPythonCommandEx (e.g., via remote requests, editing assets that embed script paths/arguments, malicious plugins, compromised user accounts, or insecure editor automation), the attacker can run arbitrary Python code in the context of the Unreal Editor/build environment where Python is enabled. - Because the design is “execute Python code/files,” you should treat it like remote code execution (RCE) within the process; the provided “private” file scope helps avoid contaminating the caller’s interpreter state but does not prevent destructive actions (file I/O, process spawning, network calls, etc.) that Python itself can perform. Mitigations: - Do not expose ExecPythonCommandEx / execute_python_command_ex to untrusted inputs. - Limit who can run editor scripting and lock down the content/scripts/paths that can be referenced. - If you must accept user-provided scripts, run them only in a controlled environment where OS/container permissions and network access are restricted; do not rely on PRIVATE/Public scope as a security boundary (it’s only about locals/globals isolation). [3] Notes about the specific phrasing “Unreal PythonScriptPlugin executes arbitrary code no sandbox security implications ExecPythonCommandEx”: - The sources above do not use the word “sandbox,” but they do establish that the API is a general “execute literal Python code or run a file” facility with scope controls that are not capability restrictions. [1][2][3]
Citations:
- 1: https://dev.epicgames.com/documentation/en-us/unreal-engine/python-api/class/PythonScriptLibrary?application_version=4.27
- 2: https://dev.epicgames.com/documentation/en-us/unreal-engine/API/Plugins/PythonScriptPlugin/FPythonCommandEx
- 3: https://dev.epicgames.com/documentation/en-us/unreal-engine/API/Plugins/PythonScriptPlugin
- 4: https://dev.epicgames.com/documentation/en-us/unreal-engine/BlueprintAPI/Python/Execution/ExecutePythonCommand_Advanced
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPPythonHandler.cpp"
# Print lines 60-110 with numbers using awk (no nl dependency)
awk 'NR>=60 && NR<=110 {printf "%d:%s\n", NR, $0}' "$FILE"
# Trace where `Code` is defined/assigned and whether it is derived from request input + any tier/trust checks
rg -n "Code\\b|Cmd\\.Command|ExecutionMode|tier|security|untrusted|allowlist|trust" "$FILE"
# Find any nearby documentation/comments about input validation or tiers in this handler
rg -n -C3 "tier|security|untrusted|allowlist|trust|policy|sanitize|validate" "$FILE" || trueRepository: zajalist/hayba
Length of output: 2732
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPPythonHandler.cpp"
# Show the full handler function(s) and enum/classify function with enough context
awk 'NR>=1 && NR<=140 {printf "%d:%s\n", NR, $0}' "$FILE"
# Specifically look for FPythonCommandEx fields that affect behavior/security
rg -n "FPythonCommandEx|FileExecutionScope|EPythonFileExecutionScope|Flags|EPythonCommandFlags|ExecutionMode|Cmd\." "$FILE"
# Show how `Code` is populated from the request payload
rg -n "TryGetStringField\\(TEXT\\(\"script\"\\)|\\bCode\\b\\s*=|ClassifyScript\\(" "$FILE"Repository: zajalist/hayba
Length of output: 4288
Confirm EPythonCommandExecutionMode::ExecuteFile is appropriate for multi-statement scripts and review the security/behavior change
ExecuteFile matches Epic’s PythonScriptPlugin semantics for running multi-statement Python (vs ExecuteStatement, which is single-statement/inline and can’t run files).
However, this switch also changes the input interpretation surface: FPythonCommandEx.Command may be treated as either literal Python code or a file (with optional arguments) depending on execution mode, and ExecPythonCommandEx provides no sandbox—tier gating here is only a heuristic on the script string. In HaybaMCPPythonHandler::Run (HaybaMCPToolkit/Private/handlers/HaybaMCPPythonHandler.cpp, reading script then setting Cmd.Command = Code + ExecutionMode = ExecuteFile), consider validating/restricting script to literal code only (or otherwise ensuring file-path execution can’t bypass the Tier 3 block).
🤖 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
`@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPPythonHandler.cpp`
around lines 78 - 87, The change sets FPythonCommandEx.ExecutionMode to
EPythonCommandExecutionMode::ExecuteFile which allows the Command string to be
treated as a filename (with args) as well as inline code; update
HaybaMCPPythonHandler::Run to validate/restrict the incoming script before
assigning Cmd.Command and Cmd.ExecutionMode to ensure only literal code is
accepted (or explicitly detect and reject file-path forms) so
ExecPythonCommandEx cannot be used to bypass Tier 3 restrictions—add checks for
newline/semicolon patterns, disallow path separators and common file extensions,
or require an explicit “literal” flag, and fail early with a clear error when
the script appears to be a file reference.
| struct FHaybaMCPConnection | ||
| { | ||
| FSocket* Socket = nullptr; | ||
| FCriticalSection SendLock; // serialize concurrent writes from | ||
| // multiple game-thread response lambdas | ||
| bool bAlive = true; // flipped to false on disconnect | ||
|
|
||
| explicit FHaybaMCPConnection(FSocket* InSocket) : Socket(InSocket) {} | ||
| ~FHaybaMCPConnection(); // closes + destroys Socket exactly once | ||
|
|
||
| FHaybaMCPConnection(const FHaybaMCPConnection&) = delete; | ||
| FHaybaMCPConnection& operator=(const FHaybaMCPConnection&) = delete; | ||
| }; |
There was a problem hiding this comment.
Data race on bAlive: use atomic type for thread-safe flag.
bAlive is written by the worker thread (in HandleClientConnection on disconnect) and read/written by game-thread lambdas (in SendMessage). A plain bool accessed concurrently from multiple threads without synchronization is undefined behavior in C++.
Use std::atomic<bool> (or Unreal's TAtomic<bool>) to ensure visibility and atomicity across threads.
🔒 Proposed fix
+#include <atomic>
+
struct FHaybaMCPConnection
{
FSocket* Socket = nullptr;
FCriticalSection SendLock; // serialize concurrent writes from
// multiple game-thread response lambdas
- bool bAlive = true; // flipped to false on disconnect
+ std::atomic<bool> bAlive{true}; // flipped to false on disconnect
explicit FHaybaMCPConnection(FSocket* InSocket) : Socket(InSocket) {}
~FHaybaMCPConnection(); // closes + destroys Socket exactly onceNote: With std::atomic<bool>, reads and writes in HandleClientConnection and SendMessage will be thread-safe. The existing SendLock remains necessary to serialize the header+body write sequence.
🤖 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 `@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPTcpServer.h`
around lines 23 - 35, The bAlive flag in FHaybaMCPConnection is concurrently
accessed from the worker thread (HandleClientConnection) and game-thread lambdas
(SendMessage), causing a data race; change the member type from plain bool
bAlive to an atomic type (e.g., std::atomic<bool> or Unreal's TAtomic<bool>) in
struct FHaybaMCPConnection and update all uses (reads/writes) in
HandleClientConnection and SendMessage to operate on the atomic (keeping
SendLock for serializing writes), ensuring atomic load/store semantics so
visibility and thread-safety are guaranteed.
…g patches dont crash A second EXCEPTION_ACCESS_VIOLATION hit during the 2026-05-23 Palestine session, this time at HaybaMCPTcpServer.cpp:166 inside ReadMessage, reading address 0xffffffffffffffff (canonical freed-pointer canary). Plugin DLL was _patch_0 (Live Coding). Root cause: my prior PR #232 fix (commit 3) changed ReadMessage from bool ReadMessage(FSocket* Socket, FString& OutMessage) to bool ReadMessage(const TSharedPtr<FHaybaMCPConnection>& Conn, FString&) Live Coding can hot-patch function BODIES but cannot safely change function SIGNATURES. Stale worker threads (compiled against the old FSocket* signature) called the patched function expecting a TSharedPtr parameter, reinterpreted the old FSocket* bytes as a TSharedPtr, deref the garbage vtable, crash. Fix: keep ReadMessage / SendMessage / HandleClientConnection signatures exactly as they always were (raw FSocket* params). Move the lifetime state into an internal lookup table on the server (TMap<FSocket*, TSharedPtr<FHaybaMCPConnection>>). The strong ref is held by the table plus pinned by every game-thread response lambda; the FSocket cannot be freed until both drop. The original UAF fix from PR #232 commit 3 is preserved. This shape is hot-patchable: function signatures never change, only function bodies and the connection-table fields are added (Live Coding handles header field additions on the module-level singleton fine). Structural changes like signature swaps now require a full rebuild; this is documented in-line so the next person to touch this file sees the constraint before making the same mistake.
zajalist
force-pushed
the
fix/plan-panel-buffered-delivery
branch
from
c03d0db to
b2d35b9
Compare
… so agents stop reaching for python_run
The user, watching the agent fight scene-composition for 10 minutes:
"why are you using python instead of the mcp tools directly". They
were right — actor_spawn had no snap-to-landscape semantics, so every
prop placement either guessed Z (and floated or buried) or fell back
to python_run with a manual unreal.SystemLibrary.line_trace_single.
Three connected changes:
A. Plugin: SnapZToLandscape() helper + snap_to_landscape param
- HaybaMCPActorHandler.cpp adds a private helper that line-traces
against ECC_WorldStatic and checks the hit actor is an
ALandscapeProxy (so we don't snap onto a random mesh).
- actor_spawn and actor_transform both accept snap_to_landscape:bool
and z_offset:number. When set, the handler overrides the supplied
Z with the landscape hit + offset.
- Response includes snapped_to_landscape:true + snapped_z so the
agent can read back what actually happened.
B. TS schemas updated
- actor-spawn.ts and actor-transform.ts schemas describe the new
params. recordEagerSchemas in tools/index.ts updated so
get_tool_signature returns them too.
- class_path now describes the dual UClass/StaticMesh acceptance
(the prior fix in this PR).
C. Validator rule: actor_spawn_not_on_landscape
- rules.ts adds the rule (severity: warning).
- tool-hooks.ts adds evaluateActorSpawnNotOnLandscape: fires when
class_path starts with /Game/ (i.e. a mesh asset, not a UClass)
AND the agent did not pass snap_to_landscape:true AND the result
did not report snapped_to_landscape:true. Surfaces in the
validator history + the UE plugin's Validator panel.
- actor-spawn.ts handler now wraps its result with
runAfterTool/attachFindingsToValue (same pattern as
execute-pcg-graph.ts and asset-retriever/browse.ts).
Lesson preserved as a memory pointer: when an agent reaches for
python_run during scene composition, the right fix is to extend the
native actor_/asset_ tool — not to make python_run faster.
Requires plugin rebuild for the C++ side; the TS side picks up on
/mcp reconnect.
… does not strip it The actor_spawn / actor_transform handlers had snap_to_landscape on their per-handler Zod schema (actor-spawn.ts, actor-transform.ts) but the MCP SDK validates against the shape passed to server.tool() in tools/index.ts — and THAT shape did not list the new param. SDK silently stripped snap_to_landscape and z_offset from incoming params, so the handler always saw the old shape, never invoked the snap, and the validator rule fired falsely (rule sees args without snap_to_landscape:true). Two schemas in two places drifted apart on the first add. Sync both: server.tool() shape for actor_spawn and actor_transform now declare snap_to_landscape:boolean + z_offset:number. The per-handler schema in actor-spawn.ts already had it; this is the wire-side mirror the SDK enforces. No behavior change for callers that already only passed the old shape.
…state never crashes UE
Crash at HaybaMCPIdleHandler.cpp:171 during the 2026-05-23 session:
EXCEPTION_ACCESS_VIOLATION reading 0xff... inside S->DoneEvent->Trigger().
The FWaitState* S parameter pointed to memory poisoned with 0xff (the
canonical pattern-fill for a TSharedPtr-released allocation). Most
likely cause: Live Coding patching the plugin while a ticker delegate
was still queued from the previous binary, so the delegate referenced
a SharedState whose layout no longer matched the new code.
The poll function now validates the state pointer at the top of each
tick before dereferencing anything:
- !S returns false (defensive).
- FMath::IsFinite(S->T0Seconds) is false when the double slot is
pattern-filled with 0xff (interprets as NaN). Live state always
has a real timestamp set in Handle() before the ticker registers.
- Subsystems.Num() out of [0, 32] is the corrupt-TArray probe — if
Num() reads garbage, the for-range would crash on Num iterations
of memcpy on bad pointers.
- The FEvent* is checked against UINTPTR_MAX and high-byte 0xFFFF
patterns before any virtual call, so a freed vtable can never be
invoked.
Each check is O(1) and the cumulative cost per tick is < 1 us — cheap
insurance against an entire class of Live Coding / pool-reuse UAFs in
the same family as the TCP server crash from PR #232 commit 3.
No behavioral change for healthy state. Stale ticks now log a warning
and return false (unregister) instead of crashing UE.
…enter actors actually snap
In the 2026-05-23 batch snap, 6 of 10 actors snapped successfully but
4 stayed at their original Z. The 4 failures were near other recently
spawned actors (or were the actor being snapped itself) — the original
LineTraceSingle hit the OTHER actors collision first, saw "not a
LandscapeProxy", and bailed with no snap.
Two-part fix:
- LineTraceMulti instead of Single: iterate all hits and pick the
first LandscapeProxy hit. Now a pillar that has another pillar
stacked above it (e.g. the tree directly above where a pillar
will be placed) still snaps correctly.
- Optional IgnoredActor param threaded through Spawn/Transform call
sites. The actor being snapped is excluded from the trace so its
own collision can never intercept its own snap probe — fixes the
"snap a new spawn, trace hits the new spawn itself" failure mode.
No API change for callers — IgnoredActor defaults to nullptr.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPTcpServer.cpp (1)
215-247:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard
ReadMessageagainstFSocket::RecvreturningtruewithBytesRead == 0Epic’s
FSocket::Recvdocumentation indicatestruedoesn’t guarantee bytes were read (BytesRead == 0can occur). Since both the 4-byte header and message body loops only advance onBytesRead > 0, this code can stall/spin if that happens. Add aBytesRead == 0disconnect guard in both loops (the file doesn’t show any non-blocking/pending-data handling for the accepted client socket prior toReadMessage).Defensive fix
while (HeaderBytesRead < 4) { int32 BytesRead = 0; if (!Socket->Recv(Header + HeaderBytesRead, 4 - HeaderBytesRead, BytesRead)) { return false; } + if (BytesRead == 0) + { + return false; // Graceful close / no progress + } HeaderBytesRead += BytesRead; } ... while (TotalBytesRead < static_cast<int32>(MessageLength)) { int32 BytesRead = 0; if (!Socket->Recv(Buffer.GetData() + TotalBytesRead, MessageLength - TotalBytesRead, BytesRead)) { return false; } + if (BytesRead == 0) + { + return false; // Graceful close / no progress + } TotalBytesRead += BytesRead; }🤖 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 `@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPTcpServer.cpp` around lines 215 - 247, The ReadMessage implementation must guard against FSocket::Recv returning true but setting BytesRead == 0 to avoid a spin; inside both loops that call Socket->Recv (the header loop that updates HeaderBytesRead using Header and the body loop that updates TotalBytesRead using Buffer), detect if BytesRead == 0 after a successful Recv and treat it as a disconnect/fatal read (return false or close the socket), rather than just continuing — update the code paths around HeaderBytesRead, TotalBytesRead, Socket->Recv, Header and Buffer to return false when BytesRead == 0.
♻️ Duplicate comments (1)
unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPTcpServer.h (1)
25-36:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winData race on
bAliveremains unfixed.This was flagged in a previous review:
bAliveis written by the worker thread and read by game-thread lambdas without synchronization. A plainboolaccessed concurrently is undefined behavior in C++.Use
std::atomic<bool>orTAtomic<bool>to ensure thread-safe visibility.🔒 Proposed fix
+#include <atomic> + struct FHaybaMCPConnection { FSocket* Socket = nullptr; FCriticalSection SendLock; - bool bAlive = true; + std::atomic<bool> bAlive{true}; explicit FHaybaMCPConnection(FSocket* InSocket) : Socket(InSocket) {}🤖 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 `@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPTcpServer.h` around lines 25 - 36, FHaybaMCPConnection’s bAlive is currently a plain bool causing a data race between the worker thread and game-thread lambdas; change bAlive to a thread-safe atomic (e.g., std::atomic<bool> or UE’s TAtomic<bool>) and update all accesses in FHaybaMCPConnection, its destructor ~FHaybaMCPConnection, and any lambdas or methods that read/write bAlive to use atomic load/store (or relaxed/acquire-release as appropriate) instead of direct reads/writes to ensure safe visibility across threads while keeping SendLock for compound operations.
🤖 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 `@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPTcpServer.cpp`:
- Around line 162-170: The bAlive flag on the connection is accessed from
multiple threads without synchronization; change the connection class's bAlive
member to an atomic type (e.g., FThreadSafeBool or TAtomic<bool>) in the header,
then update all places that read or write ConnRef->bAlive (the worker loop that
checks bIsRunning && ConnRef.IsValid() && ConnRef->bAlive, the write after
ReadMessage fails, and the game-thread checks before/after acquiring SendLock)
to use the atomic's load/store methods (or operator= / implicit bool for
FThreadSafeBool) so reads/writes are thread-safe; ensure UnregisterConn and any
other code paths that relied on non-atomic bAlive use the new atomic API.
---
Outside diff comments:
In `@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPTcpServer.cpp`:
- Around line 215-247: The ReadMessage implementation must guard against
FSocket::Recv returning true but setting BytesRead == 0 to avoid a spin; inside
both loops that call Socket->Recv (the header loop that updates HeaderBytesRead
using Header and the body loop that updates TotalBytesRead using Buffer), detect
if BytesRead == 0 after a successful Recv and treat it as a disconnect/fatal
read (return false or close the socket), rather than just continuing — update
the code paths around HeaderBytesRead, TotalBytesRead, Socket->Recv, Header and
Buffer to return false when BytesRead == 0.
---
Duplicate comments:
In `@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPTcpServer.h`:
- Around line 25-36: FHaybaMCPConnection’s bAlive is currently a plain bool
causing a data race between the worker thread and game-thread lambdas; change
bAlive to a thread-safe atomic (e.g., std::atomic<bool> or UE’s TAtomic<bool>)
and update all accesses in FHaybaMCPConnection, its destructor
~FHaybaMCPConnection, and any lambdas or methods that read/write bAlive to use
atomic load/store (or relaxed/acquire-release as appropriate) instead of direct
reads/writes to ensure safe visibility across threads while keeping SendLock for
compound operations.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: deaa14c0-c7a7-4b2f-9563-79bcffd22075
📒 Files selected for processing (9)
mcp-tools/hayba-mcp/src/tools/actor/actor-spawn.tsmcp-tools/hayba-mcp/src/tools/actor/actor-transform.tsmcp-tools/hayba-mcp/src/tools/index.tsmcp-tools/hayba-mcp/src/validator/rules.tsmcp-tools/hayba-mcp/src/validator/tool-hooks.tsunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPTcpServer.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPTcpServer.hunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPActorHandler.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPIdleHandler.cpp
| while (bIsRunning && ConnRef.IsValid() && ConnRef->bAlive) | ||
| { | ||
| FString Message; | ||
| if (!ReadMessage(ClientSocket, Message)) | ||
| { | ||
| const int32 NewCount = ClientCount.Decrement(); | ||
| UE_LOG(LogHaybaMCPTCP, Log, TEXT("Client disconnected (active: %d)"), NewCount); | ||
| ClientSocket->Close(); | ||
| ISocketSubsystem::Get(PLATFORM_SOCKETSUBSYSTEM)->DestroySocket(ClientSocket); | ||
| ConnRef->bAlive = false; | ||
| UnregisterConn(ClientSocket); |
There was a problem hiding this comment.
Data race on bAlive flag requires atomic access.
bAlive is read and written from multiple threads without synchronization:
- Worker thread writes at line 169 without any lock
- Game thread reads at lines 191, 259 before acquiring
SendLock - Worker thread reads at lines 162, 210
This is undefined behavior in C++. On ARM or with aggressive compiler optimizations, the game thread may see stale values, potentially defeating the use-after-free protection this PR introduces.
Proposed fix: Use FThreadSafeBool or TAtomic<bool>
In the header, change bAlive declaration:
-bool bAlive = true;
+TAtomic<bool> bAlive{true};Then update reads/writes (example for line 169):
-ConnRef->bAlive = false;
+ConnRef->bAlive.Store(false);And reads (example for line 162):
-while (bIsRunning && ConnRef.IsValid() && ConnRef->bAlive)
+while (bIsRunning && ConnRef.IsValid() && ConnRef->bAlive.Load())Alternatively, use FThreadSafeBool which wraps the atomic operations.
🤖 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 `@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPTcpServer.cpp`
around lines 162 - 170, The bAlive flag on the connection is accessed from
multiple threads without synchronization; change the connection class's bAlive
member to an atomic type (e.g., FThreadSafeBool or TAtomic<bool>) in the header,
then update all places that read or write ConnRef->bAlive (the worker loop that
checks bIsRunning && ConnRef.IsValid() && ConnRef->bAlive, the write after
ReadMessage fails, and the game-thread checks before/after acquiring SendLock)
to use the atomic's load/store methods (or operator= / implicit bool for
FThreadSafeBool) so reads/writes are thread-safe; ensure UnregisterConn and any
other code paths that relied on non-atomic bAlive use the new atomic API.
… + dev-ref cleanup
Plugin: serialize per-connection TCP commands so heavy game-thread ops
do not re-enter the task graph (crashes with
Assertion failed: ++Queue(QueueIndex).RecursionGuard == 1 in
TaskGraph.cpp:689). Each TCP message now blocks the worker on a future
until its game-thread task finishes before reading the next message.
Pipelines no longer pile up.
Validator: two new rules surfaced during the same scene session:
- actor_snap_to_landscape_silently_failed: warns when the agent
passed snap_to_landscape:true but the response shows the trace
missed (no snapped_to_landscape:true in result). This is the
floating CorbelSadness / buried LargeRock case.
- actor_tilted_but_not_buried: warns when a tilted prop snapped
flat onto the surface without a burial z_offset, so it reads as
balanced mid-fall instead of fallen.
All validator hints stripped of dev-internal references (no more PR
numbers, commit hashes, postmortem dates) — hints are now actionable
from inside the editor without needing GitHub access. Audience is
the operator of the editor, not the plugin author.
…ncTask+Wait Same TaskGraph.cpp:689 assert (++Queue(QueueIndex).RecursionGuard == 1) fired on hayba_propose_plan when the function ran on the game thread (every TCP command path under the serialized worker reaches this case). The handler unconditionally pushed AsyncTask(GameThread, ...) then TFuture::WaitFor — pushing to the game-thread queue while already inside a game-thread queue task is what triggers the RecursionGuard assert. Same pattern HaybaMCPLegacyHandler::RunOnGameThread uses: if IsInGameThread(), run the body inline. Otherwise marshal + wait. Off-thread path (test harness, etc.) keeps the original TPromise + WaitFor(2s) shape so that surface still works.
…re-entry crash class
Three sessions in May 2026 crashed UE with the same assert:
Assertion failed: ++Queue(QueueIndex).RecursionGuard == 1
TaskGraph.cpp:689
The pattern: a handler invoked via AsyncTask(GameThread, ...) calls
AsyncTask(GameThread, ...) again internally — re-entrant push into
UE's TaskGraph queue while it's already being processed. UE asserts
and the editor crashes.
Per-handler IsInGameThread() guards (HandleProposePlan, RunOnGameThread)
were whack-a-mole — they fix specific call sites but the next handler
that adds an AsyncTask(GameThread, ...) line silently regresses. The
proper fix is architectural: route ALL game-thread marshaling through
one dispatcher that uses a non-TaskGraph mechanism.
This commit:
- Adds HaybaMCPThreading::ExecuteOnGameThread (fire-and-forget) and
HaybaMCPThreading::RunOnGameThreadAndWait (blocking) in
Public/HaybaMCPThreading.h. Backed by an MPSC TQueue drained from
FCoreDelegates::OnEndFrame — fires OUTSIDE any TaskGraph queue-
processing window, so handlers can call back into the helper as
many times as they want without re-entry.
- Both helpers run inline if IsInGameThread() — no queueing, no
latency, no possibility of re-entry. Centralises the pattern that
was duplicated in RunOnGameThread / HandleProposePlan.
- FHaybaMCPModule::StartupModule/ShutdownModule wire up the
dispatcher lifecycle. Final Drain in Shutdown so anything queued
during teardown still runs.
- Replaces every AsyncTask(ENamedThreads::GameThread, ...) in the
plugin with one of the helpers. Touched: HaybaMCPTcpServer
(per-message dispatch, was TPromise+Wait), HaybaMCPCommandHandler
(7 sites — notification, scene-map push, validation push, memory
push, snapshot-state probe, tool-stream begin-turn,
ui_tool_stream record, post-handler tool-stream record),
HaybaMCPLegacyHandler::RunOnGameThread (replaces its hand-rolled
AsyncTask+FEvent+Wait), HaybaMCPIdleHandler (ticker registration),
HaybaMCPRenderHandler (render dispatch), HaybaMCPModule
(OnToolCallRecorded broadcast), SSliverDetailPanel (HTTP response).
- HaybaMCPTcpServer keeps its AsyncTask(AnyBackgroundThreadNormalTask)
for spawning worker threads — that's onto the background pool,
not GameThread, and is the ONE place using ::AsyncTask is right.
CI lint:
- mcp-tools/hayba-mcp/scripts/check-no-raw-asynctask-gamethread.mjs
flags any AsyncTask(ENamedThreads::GameThread, ...) outside the
dispatcher's own implementation. Allowlist is one entry
(HaybaMCPThreading.cpp); add to it only with documented reason in
the file itself.
- Wired into .github/workflows/ci.yml as a step after the existing
legacy-wrapper lint, so regressions are caught at PR time.
- Skips block comments and Doxygen `*` continuations so commentary
referencing the forbidden pattern is fine.
Net effect: the RecursionGuard crash family cannot recur in plugin
code. The IsInGameThread sprinkle in RunOnGameThread + HandleProposePlan
collapses to the single check inside the helpers.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPIdleHandler.cpp (1)
290-305:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix invalid pointer argument in ticker lambda (compile blocker).
Line 304 passes
&SharedState.Get()toPollOnce, which is the wrong expression.PollOnceexpectsFWaitState*; passSharedState.Get()directly.🔧 Proposed fix
SharedState->TickHandle = FTSTicker::GetCoreTicker().AddTicker( - FTickerDelegate::CreateLambda([SharedState](float Dt) { return PollOnce(Dt, &SharedState.Get()); }), + FTickerDelegate::CreateLambda([SharedState](float Dt) { return PollOnce(Dt, SharedState.Get()); }), POLL_INTERVAL_SECONDS);#!/bin/bash # Verify no invalid address-of-Get() pointer passing remains in C++ sources. rg -nP --type=cpp '&\s*[A-Za-z_][A-Za-z0-9_]*\.Get\(\)'Expected result: no matches for this pattern in production C++ sources.
🤖 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 `@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPIdleHandler.cpp` around lines 290 - 305, The ticker lambda is passing an invalid pointer by taking the address of SharedState.Get() when calling PollOnce; change the call inside the FTickerDelegate lambda to pass SharedState.Get() (an FWaitState*) directly instead of &SharedState.Get(). Locate the lambda created with FTSTicker::GetCoreTicker().AddTicker / FTickerDelegate::CreateLambda where SharedState is captured, and replace the argument to PollOnce so PollOnce(Dt, SharedState.Get()) is used; ensure SharedState, PollOnce, BusyOnEntry and IsBusy usages remain unchanged and that the corrected call compiles.unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPModule.cpp (1)
422-428:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCapturing
thisin fire-and-forget lambda risks dangling pointer.
RecordToolCallcapturesthisin a lambda queued for deferred game-thread execution. If the module is unloaded before the lambda runs (e.g., during editor shutdown race),thisbecomes dangling. Consider capturing a weak pointer or checking module availability inside the lambda.Proposed safer pattern
- HaybaThreading::ExecuteOnGameThread([this, Rec]() + HaybaThreading::ExecuteOnGameThread([Rec]() { - OnToolCallRecorded.Broadcast(Rec); + if (FHaybaMCPModule* M = FModuleManager::GetModulePtr<FHaybaMCPModule>("HaybaMCPToolkit")) + { + M->OnToolCallRecorded.Broadcast(Rec); + } });🤖 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 `@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPModule.cpp` around lines 422 - 428, RecordToolCall currently captures `this` in the fire-and-forget lambda passed to HaybaThreading::ExecuteOnGameThread which can dangle if the module is unloaded; change the lambda to capture a weak reference to the module (e.g., TWeakPtr/TWeakObjectPtr or std::weak_ptr depending on your module type) instead of `this`, capture `Rec` by value, and inside the lambda resolve/check the weak pointer is still valid before calling OnToolCallRecorded.Broadcast(Rec) so you avoid invoking Broadcast on a destroyed object.
🤖 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 `@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPThreading.cpp`:
- Around line 35-41: The loop over Batch calls each TFunction<void()>& Work
directly but the comment says to "Catch broadly" so wrap the Work() invocation
in a try/catch (catch (...)) inside the loop that catches any exception or
assertion from a single closure and prevents it from aborting the rest of the
Batch; inside the catch, log the failure (using the module/class logger or
UE_LOG/ensure/ensureAlwaysMsgf consistent with HaybaMCPThreading.cpp
conventions) and continue to the next Work without rethrowing.
---
Outside diff comments:
In
`@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPIdleHandler.cpp`:
- Around line 290-305: The ticker lambda is passing an invalid pointer by taking
the address of SharedState.Get() when calling PollOnce; change the call inside
the FTickerDelegate lambda to pass SharedState.Get() (an FWaitState*) directly
instead of &SharedState.Get(). Locate the lambda created with
FTSTicker::GetCoreTicker().AddTicker / FTickerDelegate::CreateLambda where
SharedState is captured, and replace the argument to PollOnce so PollOnce(Dt,
SharedState.Get()) is used; ensure SharedState, PollOnce, BusyOnEntry and IsBusy
usages remain unchanged and that the corrected call compiles.
In `@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPModule.cpp`:
- Around line 422-428: RecordToolCall currently captures `this` in the
fire-and-forget lambda passed to HaybaThreading::ExecuteOnGameThread which can
dangle if the module is unloaded; change the lambda to capture a weak reference
to the module (e.g., TWeakPtr/TWeakObjectPtr or std::weak_ptr depending on your
module type) instead of `this`, capture `Rec` by value, and inside the lambda
resolve/check the weak pointer is still valid before calling
OnToolCallRecorded.Broadcast(Rec) so you avoid invoking Broadcast on a destroyed
object.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1bd9a307-bbdd-4044-8784-c0ca6739877e
📒 Files selected for processing (15)
.github/workflows/ci.ymlmcp-tools/hayba-mcp/package.jsonmcp-tools/hayba-mcp/scripts/check-no-raw-asynctask-gamethread.mjsmcp-tools/hayba-mcp/src/validator/rules.tsmcp-tools/hayba-mcp/src/validator/tool-hooks.tsunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPCommandHandler.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPModule.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPTcpServer.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPThreading.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/Slivers/SSliverDetailPanel.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPActorHandler.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPIdleHandler.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPLegacyHandler.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/handlers/HaybaMCPRenderHandler.cppunreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Public/HaybaMCPThreading.h
✅ Files skipped from review due to trivial changes (1)
- mcp-tools/hayba-mcp/scripts/check-no-raw-asynctask-gamethread.mjs
| for (TFunction<void()>& Work : Batch) | ||
| { | ||
| // Each closure is independent — one throwing or asserting | ||
| // must not skip the rest. Catch broadly; UE's editor host | ||
| // tolerates this pattern in tick-driven code. | ||
| Work(); | ||
| } |
There was a problem hiding this comment.
Missing exception handling contradicts the comment.
The comment at lines 37-39 states that closures throwing or asserting "must not skip the rest" and that "Catch broadly" is appropriate, but the code calls Work() directly without any try-catch. If a closure throws, subsequent closures in the batch will be skipped.
Consider wrapping Work() in a try-catch if you intend to be resilient to individual closure failures:
Proposed fix
for (TFunction<void()>& Work : Batch)
{
// Each closure is independent — one throwing or asserting
// must not skip the rest. Catch broadly; UE's editor host
// tolerates this pattern in tick-driven code.
- Work();
+ try
+ {
+ Work();
+ }
+ catch (...)
+ {
+ // Swallow — continue with remaining closures.
+ }
}🤖 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 `@unreal/HaybaMCPToolkit/Source/HaybaMCPToolkit/Private/HaybaMCPThreading.cpp`
around lines 35 - 41, The loop over Batch calls each TFunction<void()>& Work
directly but the comment says to "Catch broadly" so wrap the Work() invocation
in a try/catch (catch (...)) inside the loop that catches any exception or
assertion from a single closure and prevents it from aborting the rest of the
Batch; inside the catch, log the failure (using the module/class logger or
UE_LOG/ensure/ensureAlwaysMsgf consistent with HaybaMCPThreading.cpp
conventions) and continue to the next Work without rethrowing.
…andscape source) The 2026-05-23 scene session crashed UE during PCG Generate after assigning a fresh landscape material. Density: PointsPerSquaredMeter ~0.001 across a 4 km² landscape via PCGGetLandscapeSettings(bUnbounded=true) yielded ~4000 shrub points × 3 mesh variants + ~320 tree points × 4 variants. Combined with concurrent shader compilation for the new sand material, UE crashed with EXCEPTION_ACCESS_VIOLATION reading 0xff... inside UnrealEditor-Core — no plugin frame in the callstack. The rule fires after hayba_execute_pcg_graph / pcg_execute_graph with a hint about lowering density, scoping to a small PCGVolume, or waiting for shaders to settle. Catalog-only for now (no evaluator); will wire an evaluator that inspects the executed graph's sampler nodes + the linked LandscapeProxy area when we have a path to read graph contents back from UE.
1 participant
Two C++ plugin reliability bugs surfaced by the 2026-05-23 Palestine scene build session. Both ship together because they need the same rebuild cycle.
#1 — Plan-panel buffered delivery (commit
3862a59+be2b29f)hayba_propose_planreturnedreceived: true, step_count: 9from the MCP side, but the UE Plan tab showed "No plan proposed yet."Root cause:
HaybaMCPMainPanelconstructs sub-panels lazily on first tab visit.Module->PlanPanelwas only set when the user clicked the Plan tab. Ifhayba_propose_planarrived before that first visit, the AsyncTask game-thread marshal inHandleProposePlanpinned a null weak ref and silently dropped the plan — but the handler still returned success.Fix:
Public/HaybaMCPPlanTypes.h(new) — extractFHaybaPlanStepso the module can buffer it without including the private panel header.FHaybaMCPModule— thread-safe pending-plan buffer (StashPendingPlan/ConsumePendingPlan/HasPendingPlan),FCriticalSection-guarded.HandleProposePlan— always stashes first, tries to push to a live panel; uses aTPromise<bool>so the response honestly reportspanel_visible: bool+buffered: bool+ ahint.SHaybaMCPMainPanel::ConstructBody(Plan case) — consumes any buffered plan on first construction.Also includes the
Box → ActionBoxrename inSHaybaValidatorPanel.cpp(C4458 shadow againstSMultiColumnTableRow::Boxwas a warning-as-error in UE 5.7 plugin builds).#2 — TCP server use-after-free (commit
e39ef01)UE crashed with
EXCEPTION_ACCESS_VIOLATIONatHaybaMCPTcpServer.cpp:123(SendMessage(ClientSocket, ResponseString)) during the same session, when I tried to dispatchlandscape_importover direct TCP.Root cause:
HandleClientConnection(background worker) reads a request and dispatchesAsyncTask(GameThread, [..., ClientSocket /* raw ptr */]).World->SpawnActor<ALandscape>work (now game-thread-safe thanks to PR fix(plugin): game-thread marshal for world-mutating legacy handlers + describe_assets #229).DestroySocket(ClientSocket).SendMessageon the freed pointer → access violation, hard crash.PR #229 made the SpawnActor itself safe, but the TCP server captured the raw
FSocket*into the response lambda with no liveness guard. The slow landscape_import widened the race window from negligible to catastrophic.Fix:
FHaybaMCPConnectionstruct (POD-ish, owns the socket, destructor closes + destroys it exactly once).TSharedPtr<FHaybaMCPConnection>. Every game-thread response lambda captures its own copy by value. Socket survives until the last ref drops.bAliveflag + per-connectionSendLock: SendMessage no-ops cleanly when the worker has already marked the conn dead; concurrent response lambdas can not interleave their header+body writes.ReadMessage/SendMessagenow takeconst TSharedPtr<FHaybaMCPConnection>&instead ofFSocket*. Wire format unchanged: 4-byte big-endian length prefix + JSON body.No behavioral change on the happy path. The crash path becomes a verbose log line + early return.
Requires plugin rebuild
Both are C++ plugin changes. Use Live Coding (Ctrl+Alt+F11 in editor) for a fast pickup, or close UE and rebuild for a clean apply. Live Coding sometimes misses header-shape changes (the plan-panel fix moved
FHaybaPlanStepinto a new public header) — ifhayba_propose_planstill returns the old{received, step_count}shape after Live Coding, do a full rebuild.Testing
LogHaybaMCPTCP: Verbose: SendMessage: header write failed; marking conn dead, no crash.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores