From 275cf6d799903e0f2702cac407566b9f95aa51fd Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Tue, 19 May 2026 21:37:19 -0700 Subject: [PATCH 01/17] feat(remote-control): add MCP server core feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new "Remote Control" CORE feature that hosts an embedded Model Context Protocol (MCP) server inside CommunityShaders.dll so AI assistants (Claude Code, Cursor, Continue, etc.) can query and mutate runtime state for A/B testing and performance investigation. Server is off by default and binds to 127.0.0.1 — opt-in via the Settings UI. The ImGui panel exposes a "Copy MCP client config to clipboard" button that emits a Streamable-HTTP (MCP 2025-03-26) config snippet ready to paste into a host's mcpServers settings. Initial slice ships a single bootstrap tool (get_state) so the end-to-end wiring can be validated; subsequent commits will add list_features, get/set/toggle/reset_feature, run_abtest, capture_renderdoc, and capture_screenshot. Library choice: hkr04/cpp-mcp pinned to a0eb22c9 (280 stars, MIT, 2025-03-26 spec compliant including Streamable HTTP). Vendored as a submodule under extern/cpp-mcp — same pattern as FidelityFX-SDK and Streamline — because cpp-mcp has no install rules (upstream PR #12 still open). Only the server-side translation units are compiled; the bundled stdio/SSE *client* implementations are intentionally omitted. The library vendors its own nlohmann_json (3.11.3) which has a different ABI namespace tag than vcpkg's (3.12.0) and would cause LNK2001 on mcp::server::set_capabilities / register_tool if cpp-mcp linked against the vendored copy and consumers against vcpkg. cmake/cpp-mcp.cmake patches mcp_message.h at configure time to use from vcpkg, writing the patched header to a build-tree mirror so the submodule stays clean. cpp-mcp's vendored cpp-httplib pulls in , but Skyrim's PCH transitively brings the legacy via . Define _WINSOCKAPI_ on the cpp-mcp target (PUBLIC) so consumers and the PCH skip the legacy header. Wiring: * extern/cpp-mcp submodule pinned to hkr04/cpp-mcp@a0eb22c9 * cmake/cpp-mcp.cmake builds the static library target * features/Remote Control/CORE marker + versioned ini * src/Features/RemoteControl.{h,cpp} (Feature subclass) * registered in globals::features and Feature::GetFeatureList Verified end-to-end: CommunityShaders.dll builds clean, grows ~500 KB, deploys to SE+VR. Off-by-default toggle exposed in the Core Features → Utility group of the in-game menu. Co-Authored-By: Claude Opus 4.7 --- .gitmodules | 3 + CMakeLists.txt | 2 + cmake/cpp-mcp.cmake | 96 ++++++++ extern/cpp-mcp | 1 + features/Remote Control/CORE | 0 .../Shaders/Features/RemoteControl.ini | 2 + src/Feature.cpp | 2 + src/Features/RemoteControl.cpp | 222 ++++++++++++++++++ src/Features/RemoteControl.h | 80 +++++++ src/Globals.cpp | 2 + src/Globals.h | 2 + 11 files changed, 412 insertions(+) create mode 100644 cmake/cpp-mcp.cmake create mode 160000 extern/cpp-mcp create mode 100644 features/Remote Control/CORE create mode 100644 features/Remote Control/Shaders/Features/RemoteControl.ini create mode 100644 src/Features/RemoteControl.cpp create mode 100644 src/Features/RemoteControl.h diff --git a/.gitmodules b/.gitmodules index db0fe8c6f7..7bed141dcb 100644 --- a/.gitmodules +++ b/.gitmodules @@ -8,3 +8,6 @@ path = extern/FidelityFX-SDK url = https://github.com/alandtse/FidelityFX-SDK-DX11 branch = optiscaler-build +[submodule "extern/cpp-mcp"] + path = extern/cpp-mcp + url = https://github.com/hkr04/cpp-mcp.git diff --git a/CMakeLists.txt b/CMakeLists.txt index 2350126edc..d225f0eac3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -101,6 +101,7 @@ add_subdirectory(${CMAKE_SOURCE_DIR}/cmake/Streamline) find_path(DETOURS_INCLUDE_DIRS "detours/detours.h") find_library(DETOURS_LIBRARY detours REQUIRED) include(FidelityFX-SDK) +include(cpp-mcp) target_compile_definitions( ${PROJECT_NAME} @@ -144,6 +145,7 @@ target_link_libraries( unordered_dense::unordered_dense efsw::efsw Tracy::TracyClient + cpp-mcp Streamline d3d12.lib Microsoft::DirectX-Headers diff --git a/cmake/cpp-mcp.cmake b/cmake/cpp-mcp.cmake new file mode 100644 index 0000000000..b2b9b393c9 --- /dev/null +++ b/cmake/cpp-mcp.cmake @@ -0,0 +1,96 @@ +# Build cpp-mcp (https://github.com/hkr04/cpp-mcp) from its vendored +# submodule as a static library target. Upstream has no install rules +# (PR #12 still open), so we drive its build ourselves — same pattern +# we use for FidelityFX-SDK and Streamline. +# +# Only the server-side translation units are compiled; the bundled +# stdio/SSE *client* implementations are intentionally omitted because +# we are exclusively a server. +# +# nlohmann_json ABI alignment: +# cpp-mcp vendors nlohmann_json 3.11.3 in extern/cpp-mcp/common/json.hpp, +# while vcpkg ships 3.12.0. Both versions wrap their public API in an +# ABI-versioned inline namespace (`nlohmann::json_abi_v3_11_3` vs +# `nlohmann::json_abi_v3_12_0`), so even though both files share the +# same include guard (INCLUDE_NLOHMANN_JSON_HPP_), the symbol names +# differ. If cpp-mcp's own TUs picked up the vendored copy and our +# consumers picked up vcpkg's, `mcp::server::set_capabilities` and +# `register_tool` would link-fail (LNK2001) with two different +# ABI-tagged signatures. +# +# Fix: patch mcp_message.h at configure time to use +# `#include ` instead of `#include "json.hpp"`. +# The patched copy is written to a build-tree mirror; the submodule +# stays clean. Both cpp-mcp's own compilation and every consumer +# then resolve to vcpkg's 3.12.0 → single ABI namespace, symbols +# match, linker happy. + +set(CPP_MCP_DIR "${CMAKE_SOURCE_DIR}/extern/cpp-mcp") +set(CPP_MCP_PATCHED_INC "${CMAKE_BINARY_DIR}/cpp-mcp-patched/include") + +if(NOT EXISTS "${CPP_MCP_DIR}/src/mcp_server.cpp") + message(FATAL_ERROR + "cpp-mcp submodule missing. Run:\n" + " git submodule update --init --recursive extern/cpp-mcp") +endif() + +find_package(Threads REQUIRED) +find_package(nlohmann_json CONFIG REQUIRED) + +# Patch mcp_message.h to use vcpkg nlohmann_json (see header comment). +# All other cpp-mcp headers are copied verbatim into the patched mirror +# so they live next to the patched header and find each other. +file(MAKE_DIRECTORY "${CPP_MCP_PATCHED_INC}") +file(GLOB _cpp_mcp_headers "${CPP_MCP_DIR}/include/*.h") +foreach(_hdr IN LISTS _cpp_mcp_headers) + get_filename_component(_name "${_hdr}" NAME) + file(READ "${_hdr}" _content) + if(_name STREQUAL "mcp_message.h") + string(REPLACE + "#include \"json.hpp\"" + "#include " + _content "${_content}") + endif() + file(WRITE "${CPP_MCP_PATCHED_INC}/${_name}" "${_content}") +endforeach() + +add_library(cpp-mcp STATIC + "${CPP_MCP_DIR}/src/mcp_message.cpp" + "${CPP_MCP_DIR}/src/mcp_resource.cpp" + "${CPP_MCP_DIR}/src/mcp_server.cpp" + "${CPP_MCP_DIR}/src/mcp_tool.cpp" +) + +# Order matters: patched mirror first so its mcp_message.h wins over the +# submodule's. `common/` is still needed for httplib.h (no ABI issue +# there — it's not shared with any vcpkg dep). +target_include_directories(cpp-mcp + PUBLIC "${CPP_MCP_PATCHED_INC}" + "${CPP_MCP_DIR}/common" +) + +target_compile_features(cpp-mcp PUBLIC cxx_std_17) + +target_compile_definitions(cpp-mcp PUBLIC + MCP_MAX_SESSIONS=10 + MCP_SESSION_TIMEOUT=30 + # cpp-mcp's vendored cpp-httplib pulls in . Skyrim/CLib's + # transitive defaults to the legacy , which + # conflicts (redefinition of sockaddr, WSAData, etc.). Tell Windows + # headers to skip the legacy winsock so winsock2.h is the only one + # in the build. PUBLIC so it propagates to every TU that links + # cpp-mcp (including the PCH compilation of CommunityShaders). + _WINSOCKAPI_ +) + +target_link_libraries(cpp-mcp PUBLIC + Threads::Threads + nlohmann_json::nlohmann_json +) + +if(MSVC) + target_compile_options(cpp-mcp PRIVATE /utf-8 /bigobj /W0) + target_compile_definitions(cpp-mcp PRIVATE _CRT_SECURE_NO_WARNINGS) +endif() + +set_target_properties(cpp-mcp PROPERTIES FOLDER "extern") diff --git a/extern/cpp-mcp b/extern/cpp-mcp new file mode 160000 index 0000000000..a0eb22c98d --- /dev/null +++ b/extern/cpp-mcp @@ -0,0 +1 @@ +Subproject commit a0eb22c98dbd8ce8b3ef69679310c1a038905c08 diff --git a/features/Remote Control/CORE b/features/Remote Control/CORE new file mode 100644 index 0000000000..e69de29bb2 diff --git a/features/Remote Control/Shaders/Features/RemoteControl.ini b/features/Remote Control/Shaders/Features/RemoteControl.ini new file mode 100644 index 0000000000..000b60a568 --- /dev/null +++ b/features/Remote Control/Shaders/Features/RemoteControl.ini @@ -0,0 +1,2 @@ +[Info] +Version = 1-0-0 diff --git a/src/Feature.cpp b/src/Feature.cpp index c6c0df6fe8..5264802f5a 100644 --- a/src/Feature.cpp +++ b/src/Feature.cpp @@ -18,6 +18,7 @@ #include "Features/LightLimitFix.h" #include "Features/LinearLighting.h" #include "Features/PerformanceOverlay.h" +#include "Features/RemoteControl.h" #include "Features/RenderDoc.h" #include "Features/ScreenSpaceGI.h" #include "Features/ScreenSpaceShadows.h" @@ -240,6 +241,7 @@ const std::vector& Feature::GetFeatureList() &globals::features::extendedTranslucency, &globals::features::upscaling, &globals::features::renderDoc, + &globals::features::remoteControl, &globals::features::weatherEditor, &globals::features::screenshotFeature, &globals::features::linearLighting, diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp new file mode 100644 index 0000000000..3aa8066a76 --- /dev/null +++ b/src/Features/RemoteControl.cpp @@ -0,0 +1,222 @@ +// Remote Control feature: hosts an in-process Model Context Protocol (MCP) +// server inside CommunityShaders.dll, letting AI assistants query and mutate +// runtime state for A/B testing. Off by default and loopback-only. +// +// Transport: HTTP+SSE (Streamable HTTP, MCP 2025-03-26). +// Endpoint: http://:/mcp (modern, single endpoint) +// http://:/sse (legacy SSE, also exposed by cpp-mcp) + +#include "Features/RemoteControl.h" + +#include "Globals.h" +#include "State.h" + +#include +#include + +#include +#include + +// cpp-mcp headers. Kept inside the .cpp only so the vendored httplib/json +// in extern/cpp-mcp/common don't leak into other translation units. +#include "mcp_server.h" +#include "mcp_tool.h" + +RemoteControl* RemoteControl::GetSingleton() +{ + return &globals::features::remoteControl; +} + +RemoteControl::RemoteControl() = default; + +RemoteControl::~RemoteControl() +{ + StopServer(); +} + +void RemoteControl::Load() +{ + // Settings have already been read in by the time Load() fires. + if (settings.enabled) { + StartServer(); + } +} + +void RemoteControl::Reset() +{ + // No per-frame state to reset. +} + +void RemoteControl::LoadSettings(json& o_json) +{ + settings.enabled = o_json.value("enabled", false); + settings.port = o_json.value("port", 8910); + settings.bindAddress = o_json.value("bindAddress", std::string("127.0.0.1")); +} + +void RemoteControl::SaveSettings(json& o_json) +{ + o_json["enabled"] = settings.enabled; + o_json["port"] = settings.port; + o_json["bindAddress"] = settings.bindAddress; +} + +void RemoteControl::RestoreDefaultSettings() +{ + settings = Settings{}; +} + +void RemoteControl::DrawSettings() +{ + ImGui::TextWrapped( + "Exposes Community Shaders over Model Context Protocol (MCP) so AI " + "assistants such as Claude Code can drive A/B testing, toggle " + "features, and trigger captures. Off by default. Bound to 127.0.0.1 " + "unless explicitly changed."); + ImGui::Spacing(); + + const bool wasEnabled = settings.enabled; + if (ImGui::Checkbox("Enable MCP server", &settings.enabled)) { + if (settings.enabled && !wasEnabled) { + StartServer(); + } else if (!settings.enabled && wasEnabled) { + StopServer(); + } + } + + // Port + bind address can only be edited while the server is stopped. + ImGui::BeginDisabled(IsRunning()); + ImGui::InputInt("Port", &settings.port); + settings.port = std::clamp(settings.port, 1024, 65535); + ImGui::InputText("Bind address", &settings.bindAddress); + ImGui::EndDisabled(); + if (IsRunning()) { + ImGui::SameLine(); + ImGui::TextDisabled("(stop the server to edit)"); + } + + if (!lastError.empty()) { + ImGui::TextColored(ImVec4(1.0f, 0.5f, 0.4f, 1.0f), + "Server error: %s", lastError.c_str()); + } + + if (IsRunning()) { + ImGui::TextColored(ImVec4(0.4f, 0.9f, 0.5f, 1.0f), + "Listening on %s:%d", settings.bindAddress.c_str(), activePort); + } + + ImGui::Separator(); + ImGui::Text("Connect from an MCP client (Claude Code, Cursor, etc.):"); + + if (ImGui::Button("Copy MCP client config to clipboard")) { + ImGui::SetClipboardText(BuildClientConfig().c_str()); + } + ImGui::SameLine(); + ImGui::TextDisabled("(?)"); + if (ImGui::IsItemHovered()) { + ImGui::SetTooltip( + "Paste the JSON into your Claude Code settings under " + "\"mcpServers\". Other MCP hosts (Cursor, Continue) accept the " + "same shape."); + } + + if (ImGui::CollapsingHeader("Config preview")) { + const auto preview = BuildClientConfig(); + ImGui::PushTextWrapPos(); + ImGui::TextUnformatted(preview.c_str()); + ImGui::PopTextWrapPos(); + } +} + +std::string RemoteControl::BuildClientConfig() const +{ + // Streamable HTTP transport per the MCP 2025-03-26 spec. Same shape works + // for Claude Code, Cursor, Continue, and other MCP hosts. + const json cfg = { + { "mcpServers", + { { "community-shaders", + { + { "type", "http" }, + { "url", std::format("http://{}:{}/mcp", + settings.bindAddress, settings.port) }, + } } } } + }; + return cfg.dump(4); +} + +void RemoteControl::StartServer() +{ + if (server) { + return; + } + lastError.clear(); + + try { + mcp::server::configuration cfg; + cfg.host = settings.bindAddress; + cfg.port = settings.port; + cfg.name = "Community Shaders"; + cfg.version = "0.1.0"; + + server = std::make_unique(cfg); + server->set_server_info(cfg.name, cfg.version); + server->set_capabilities({ { "tools", mcp::json::object() } }); + server->set_instructions( + "This server exposes the Skyrim Community Shaders plugin. " + "Use the tools to inspect engine state for performance " + "investigation and A/B testing of graphics features."); + + RegisterTools(); + + if (!server->start(false)) { // false = non-blocking + throw std::runtime_error("server.start() returned false"); + } + activePort = settings.port; + logger::info("Remote Control: MCP server listening on {}:{}", + settings.bindAddress, activePort); + } catch (const std::exception& e) { + lastError = e.what(); + logger::error("Remote Control: failed to start MCP server: {}", + e.what()); + server.reset(); + activePort = 0; + } +} + +void RemoteControl::StopServer() +{ + if (!server) { + return; + } + try { + server->stop(); + } catch (...) { + // best-effort on shutdown + } + server.reset(); + activePort = 0; + logger::info("Remote Control: MCP server stopped"); +} + +void RemoteControl::RegisterTools() +{ + // Bootstrap tool — proves the wire-up is working end-to-end. + // Subsequent commits will add: list_features, get_feature, set_feature, + // toggle_feature, run_abtest, capture_renderdoc, capture_screenshot, etc. + const auto stateTool = mcp::tool_builder("get_state") + .with_description( + "Return Community Shaders runtime state: " + "frame counter, plugin version, VR mode.") + .build(); + server->register_tool(stateTool, + [](const mcp::json& /*params*/, const std::string& /*session_id*/) -> mcp::json { + const uint frames = globals::state ? globals::state->frameCount : 0; + const bool vr = REL::Module::IsVR(); + const std::string payload = std::format( + R"({{"frame_count":{},"vr":{},"plugin":"CommunityShaders"}})", + frames, vr ? "true" : "false"); + return mcp::json::array({ mcp::json{ + { "type", "text" }, + { "text", payload } } }); + }); +} diff --git a/src/Features/RemoteControl.h b/src/Features/RemoteControl.h new file mode 100644 index 0000000000..7850573331 --- /dev/null +++ b/src/Features/RemoteControl.h @@ -0,0 +1,80 @@ +#pragma once + +#include "Feature.h" + +#include +#include +#include + +using json = nlohmann::json; + +// Forward declare cpp-mcp types so we don't leak its vendored +// httplib / json headers into consumers of this header. +namespace mcp +{ + class server; +} + +class RemoteControl : public Feature +{ +public: + static RemoteControl* GetSingleton(); + + // Feature overrides — see Feature.h for contracts. + std::string GetName() override { return "Remote Control"; } + std::string GetShortName() override { return "RemoteControl"; } + std::string_view GetCategory() const override { return FeatureCategories::kUtility; } + bool IsCore() const override { return true; } + bool IsInMenu() const override { return true; } + bool SupportsVR() override { return true; } + std::string_view GetShaderDefineName() override { return ""; } + bool HasShaderDefine(RE::BSShader::Type) override { return false; } + + std::pair> GetFeatureSummary() override + { + return { + "Expose Community Shaders to AI assistants over Model Context Protocol (MCP).", + { + "Loopback-only JSON-RPC server, off by default", + "Pair with Claude Code / Cursor / Continue for A/B testing", + "One-click clipboard copy of MCP client config", + } + }; + } + + // Lifecycle + void Load() override; + void Reset() override; + + // Settings persistence + void DrawSettings() override; + void RestoreDefaultSettings() override; + void LoadSettings(json& o_json) override; + void SaveSettings(json& o_json) override; + + struct Settings + { + bool enabled = false; // opt-in + int port = 8910; // arbitrary high port + std::string bindAddress = "127.0.0.1"; // loopback by default + } settings; + + RemoteControl(); + ~RemoteControl(); + + RemoteControl(const RemoteControl&) = delete; + RemoteControl& operator=(const RemoteControl&) = delete; + RemoteControl(RemoteControl&&) = delete; + RemoteControl& operator=(RemoteControl&&) = delete; + +private: + void StartServer(); + void StopServer(); + bool IsRunning() const noexcept { return server != nullptr; } + std::string BuildClientConfig() const; + void RegisterTools(); + + std::unique_ptr server; + int activePort = 0; + std::string lastError; +}; diff --git a/src/Globals.cpp b/src/Globals.cpp index 850d0f9dce..d216a05b74 100644 --- a/src/Globals.cpp +++ b/src/Globals.cpp @@ -17,6 +17,7 @@ #include "Features/LightLimitFix.h" #include "Features/LinearLighting.h" #include "Features/PerformanceOverlay.h" +#include "Features/RemoteControl.h" #include "Features/RenderDoc.h" #include "Features/ScreenSpaceGI.h" #include "Features/ScreenSpaceShadows.h" @@ -85,6 +86,7 @@ namespace globals Upscaling upscaling{}; HDRDisplay hdrDisplay{}; RenderDoc renderDoc{}; + RemoteControl remoteControl{}; ScreenshotFeature screenshotFeature{}; WeatherEditor weatherEditor{}; ExponentialHeightFog exponentialHeightFog{}; diff --git a/src/Globals.h b/src/Globals.h index a5e262a768..4556d5ab18 100644 --- a/src/Globals.h +++ b/src/Globals.h @@ -41,6 +41,7 @@ class State; class Deferred; struct TruePBR; class RenderDoc; +class RemoteControl; class Menu; namespace SIE @@ -92,6 +93,7 @@ namespace globals extern Upscaling upscaling; extern HDRDisplay hdrDisplay; extern RenderDoc renderDoc; + extern RemoteControl remoteControl; extern ScreenshotFeature screenshotFeature; extern WeatherEditor weatherEditor; extern ExponentialHeightFog exponentialHeightFog; From eaa6877967a461b8e969824fea2b0c8bf56f8450 Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Tue, 19 May 2026 21:59:09 -0700 Subject: [PATCH 02/17] feat(remote-control): add list_features tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enumerate all graphics features over MCP. Returns a JSON array with one object per feature including: name, shortName, loaded, version, category, isCore, supportsVR, inMenu. Read-only — iterates Feature::GetFeatureList() without mutation. No render-thread synchronization required. Refactored RegisterTools() into per-tool helpers (RegisterGetStateTool, RegisterListFeaturesTool) so subsequent commits can drop in additional tools without ballooning a single function. Pulled the MCP content envelope into a TextResult() helper shared across tools. Co-Authored-By: Claude Opus 4.7 --- src/Features/RemoteControl.cpp | 67 +++++++++++++++++++++++++++------- src/Features/RemoteControl.h | 2 + 2 files changed, 55 insertions(+), 14 deletions(-) diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index 3aa8066a76..c7baac01e4 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -198,25 +198,64 @@ void RemoteControl::StopServer() logger::info("Remote Control: MCP server stopped"); } +// Helper: wrap a payload string in the MCP tool-result content envelope +// (an array of typed content items). Tools return application data as the +// "text" field of a single content item; consumers typically parse it as +// JSON. +static mcp::json TextResult(std::string text) +{ + return mcp::json::array({ mcp::json{ + { "type", "text" }, + { "text", std::move(text) } } }); +} + void RemoteControl::RegisterTools() { - // Bootstrap tool — proves the wire-up is working end-to-end. - // Subsequent commits will add: list_features, get_feature, set_feature, - // toggle_feature, run_abtest, capture_renderdoc, capture_screenshot, etc. - const auto stateTool = mcp::tool_builder("get_state") - .with_description( - "Return Community Shaders runtime state: " - "frame counter, plugin version, VR mode.") - .build(); - server->register_tool(stateTool, + RegisterGetStateTool(); + RegisterListFeaturesTool(); +} + +void RemoteControl::RegisterGetStateTool() +{ + const auto tool = mcp::tool_builder("get_state") + .with_description( + "Return Community Shaders runtime state: " + "frame counter, plugin version, VR mode.") + .build(); + server->register_tool(tool, [](const mcp::json& /*params*/, const std::string& /*session_id*/) -> mcp::json { const uint frames = globals::state ? globals::state->frameCount : 0; const bool vr = REL::Module::IsVR(); - const std::string payload = std::format( + return TextResult(std::format( R"({{"frame_count":{},"vr":{},"plugin":"CommunityShaders"}})", - frames, vr ? "true" : "false"); - return mcp::json::array({ mcp::json{ - { "type", "text" }, - { "text", payload } } }); + frames, vr ? "true" : "false")); + }); +} + +void RemoteControl::RegisterListFeaturesTool() +{ + const auto tool = mcp::tool_builder("list_features") + .with_description( + "Enumerate Community Shaders graphics features. " + "Returns a JSON array with one entry per feature: " + "name, shortName, loaded, version, category, " + "isCore, supportsVR.") + .build(); + server->register_tool(tool, + [](const mcp::json& /*params*/, const std::string& /*session_id*/) -> mcp::json { + mcp::json features = mcp::json::array(); + for (auto* f : Feature::GetFeatureList()) { + features.push_back({ + { "name", f->GetName() }, + { "shortName", f->GetShortName() }, + { "loaded", f->loaded }, + { "version", f->version }, + { "category", std::string(f->GetCategory()) }, + { "isCore", f->IsCore() }, + { "supportsVR", f->SupportsVR() }, + { "inMenu", f->IsInMenu() }, + }); + } + return TextResult(features.dump()); }); } diff --git a/src/Features/RemoteControl.h b/src/Features/RemoteControl.h index 7850573331..ca25dc3875 100644 --- a/src/Features/RemoteControl.h +++ b/src/Features/RemoteControl.h @@ -73,6 +73,8 @@ class RemoteControl : public Feature bool IsRunning() const noexcept { return server != nullptr; } std::string BuildClientConfig() const; void RegisterTools(); + void RegisterGetStateTool(); + void RegisterListFeaturesTool(); std::unique_ptr server; int activePort = 0; From 8abac4ca356f493d7c68b5894566bc54999a18d5 Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Tue, 19 May 2026 22:01:40 -0700 Subject: [PATCH 03/17] feat(remote-control): add get_feature_settings tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Return the JSON settings blob for a single feature by shortName. Round-trips through Feature::SaveSettings(), so what comes out is exactly what the on-disk config encodes — the same shape that set_feature_settings (next commit) will accept back. Read-only. No mutation, no render-thread sync required. Also introduces ErrorResult() — a small helper that emits a single-text-item content envelope with {"error": "...", ...}. Conventions for all future tools: * success → {"...": ...} (tool-specific shape) * failure → {"error": "", "": ...} Callers can always JSON.parse() the text field. Co-Authored-By: Claude Opus 4.7 --- src/Features/RemoteControl.cpp | 48 ++++++++++++++++++++++++++++++++++ src/Features/RemoteControl.h | 1 + 2 files changed, 49 insertions(+) diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index c7baac01e4..62ef91c4a3 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -209,10 +209,25 @@ static mcp::json TextResult(std::string text) { "text", std::move(text) } } }); } +// Helper: emit an error result. Convention: a single text content item +// containing a JSON object with "error" + optional context fields, so +// callers always get parseable JSON whether the call succeeded or not. +static mcp::json ErrorResult(std::string_view message, mcp::json context = {}) +{ + mcp::json obj = { { "error", message } }; + if (!context.is_null()) { + obj.update(context); + } + return mcp::json::array({ mcp::json{ + { "type", "text" }, + { "text", obj.dump() } } }); +} + void RemoteControl::RegisterTools() { RegisterGetStateTool(); RegisterListFeaturesTool(); + RegisterGetFeatureSettingsTool(); } void RemoteControl::RegisterGetStateTool() @@ -259,3 +274,36 @@ void RemoteControl::RegisterListFeaturesTool() return TextResult(features.dump()); }); } + +void RemoteControl::RegisterGetFeatureSettingsTool() +{ + const auto tool = mcp::tool_builder("get_feature_settings") + .with_description( + "Return the current JSON settings blob for a " + "single feature. Use list_features to discover " + "shortNames. The exact schema is feature-specific " + "— it mirrors what Feature::SaveSettings emits to " + "the on-disk config and what set_feature_settings " + "expects back.") + .with_string_param("shortName", + "Feature shortName as returned by list_features.") + .build(); + server->register_tool(tool, + [](const mcp::json& params, const std::string& /*session_id*/) -> mcp::json { + const std::string shortName = params.value("shortName", std::string{}); + if (shortName.empty()) { + return ErrorResult("missing required parameter 'shortName'"); + } + auto* feature = Feature::FindFeatureByShortName(shortName); + if (!feature) { + return ErrorResult("feature not found or not loaded", + { { "shortName", shortName } }); + } + // SaveSettings() uses nlohmann::json (unordered map). Keep the + // intermediate value as plain json and re-emit as a string so + // we don't have to round-trip through mcp::json's ordered map. + ::json blob; + feature->SaveSettings(blob); + return TextResult(blob.dump()); + }); +} diff --git a/src/Features/RemoteControl.h b/src/Features/RemoteControl.h index ca25dc3875..34893fc5ca 100644 --- a/src/Features/RemoteControl.h +++ b/src/Features/RemoteControl.h @@ -75,6 +75,7 @@ class RemoteControl : public Feature void RegisterTools(); void RegisterGetStateTool(); void RegisterListFeaturesTool(); + void RegisterGetFeatureSettingsTool(); std::unique_ptr server; int activePort = 0; From 2bc690ae3d6ad15c8961a0d08ccd93ba5aac43bb Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Tue, 19 May 2026 22:03:59 -0700 Subject: [PATCH 04/17] feat(remote-control): add toggle_feature tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enable or disable a graphics feature at runtime by flipping Feature::loaded. ForEachLoadedFeature skips features whose loaded flag is false, so every per-frame entrypoint (Prepass, Reflections, Reset, etc.) becomes a no-op for that feature on the next frame. GPU resources allocated during SetupResources() are NOT freed — the goal is fast A/B perf/quality comparisons, not memory reclaim. Re-toggling restores rendering with whatever state the feature had mid-flight. Atomic single-bool write — no command queue required. The listener thread sets `loaded`, the render thread reads it on its next pass. A torn read isn't possible (single byte), and the worst case from a race is "one extra frame of new state" which is the desired outcome anyway. Walks GetFeatureList() directly instead of FindFeatureByShortName() because the latter filters on loaded==true (built for "find an active feature") — we need to be able to re-enable disabled ones. Co-Authored-By: Claude Opus 4.7 --- src/Features/RemoteControl.cpp | 54 ++++++++++++++++++++++++++++++++++ src/Features/RemoteControl.h | 1 + 2 files changed, 55 insertions(+) diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index 62ef91c4a3..e207b11c48 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -228,6 +228,7 @@ void RemoteControl::RegisterTools() RegisterGetStateTool(); RegisterListFeaturesTool(); RegisterGetFeatureSettingsTool(); + RegisterToggleFeatureTool(); } void RemoteControl::RegisterGetStateTool() @@ -275,6 +276,59 @@ void RemoteControl::RegisterListFeaturesTool() }); } +void RemoteControl::RegisterToggleFeatureTool() +{ + const auto tool = mcp::tool_builder("toggle_feature") + .with_description( + "Enable or disable a feature at runtime by " + "flipping its 'loaded' flag. Disabled features " + "are skipped by Feature::ForEachLoadedFeature so " + "their per-frame rendering work doesn't run. GPU " + "resources are NOT freed — this is for A/B " + "perf/quality comparisons, not memory reclaim. " + "Reverting only requires another toggle_feature " + "call.") + .with_string_param("shortName", + "Feature shortName as returned by list_features.") + .with_boolean_param("enabled", + "true to load (run), false to skip.") + .build(); + server->register_tool(tool, + [](const mcp::json& params, const std::string& /*session_id*/) -> mcp::json { + const std::string shortName = params.value("shortName", std::string{}); + if (shortName.empty()) { + return ErrorResult("missing required parameter 'shortName'"); + } + if (!params.contains("enabled") || !params["enabled"].is_boolean()) { + return ErrorResult("missing required boolean parameter 'enabled'"); + } + const bool desired = params["enabled"].get(); + // FindFeatureByShortName filters on `loaded == true`, so it won't + // help us re-enable a feature. Walk the full list ourselves. + Feature* target = nullptr; + for (auto* f : Feature::GetFeatureList()) { + if (f->GetShortName() == shortName) { + target = f; + break; + } + } + if (!target) { + return ErrorResult("feature not found", + { { "shortName", shortName } }); + } + const bool previous = target->loaded; + target->loaded = desired; + logger::info("Remote Control: toggle_feature({}, {}) (was {})", + shortName, desired, previous); + return TextResult(mcp::json({ + { "shortName", shortName }, + { "previous", previous }, + { "current", desired }, + }) + .dump()); + }); +} + void RemoteControl::RegisterGetFeatureSettingsTool() { const auto tool = mcp::tool_builder("get_feature_settings") diff --git a/src/Features/RemoteControl.h b/src/Features/RemoteControl.h index 34893fc5ca..57ef5eab2e 100644 --- a/src/Features/RemoteControl.h +++ b/src/Features/RemoteControl.h @@ -76,6 +76,7 @@ class RemoteControl : public Feature void RegisterGetStateTool(); void RegisterListFeaturesTool(); void RegisterGetFeatureSettingsTool(); + void RegisterToggleFeatureTool(); std::unique_ptr server; int activePort = 0; From 89d1dedde397463e0cad5f5bf0defbb9e2b74fd5 Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Tue, 19 May 2026 22:24:19 -0700 Subject: [PATCH 05/17] feat(remote-control): add set_feature_settings tool Replace a feature's settings with a supplied JSON blob via the existing Feature::LoadSettings(json&) path. Round-trips through dump/parse to bridge cpp-mcp's ordered_json to plain nlohmann::json expected by the feature interface. Handler runs on the cpp-mcp listener thread. Most features only deserialize into member variables here (same pattern the ImGui menu uses), and a few set a recompileFlag that the render loop consumes on the next frame (e.g. ScreenSpaceGI, DynamicCubemaps). Both patterns are safe at this thread boundary. Settings whose LoadSettings synchronously reallocates GPU resources would race the renderer; none in-tree currently do, but documented in the tool description as a follow-up to tighten with a command queue if a future feature lands one. LightLimitFix is a known no-override case: set_feature_settings will return applied=true but the feature's runtime state won't change because LightLimitFix doesn't implement LoadSettings. Tracked separately as a feature-side fix. Co-Authored-By: Claude Opus 4.7 --- src/Features/RemoteControl.cpp | 65 ++++++++++++++++++++++++++++++++++ src/Features/RemoteControl.h | 1 + 2 files changed, 66 insertions(+) diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index e207b11c48..a5bcaa0ca0 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -229,6 +229,7 @@ void RemoteControl::RegisterTools() RegisterListFeaturesTool(); RegisterGetFeatureSettingsTool(); RegisterToggleFeatureTool(); + RegisterSetFeatureSettingsTool(); } void RemoteControl::RegisterGetStateTool() @@ -329,6 +330,70 @@ void RemoteControl::RegisterToggleFeatureTool() }); } +void RemoteControl::RegisterSetFeatureSettingsTool() +{ + // Empty schema for the "settings" object means "any shape" — the actual + // schema is feature-specific and discoverable via get_feature_settings. + const auto tool = mcp::tool_builder("set_feature_settings") + .with_description( + "Replace a feature's settings with the supplied " + "JSON blob. Schema is feature-specific; use " + "get_feature_settings to fetch the current " + "shape, mutate the fields you care about, then " + "send the whole object back. Changes take " + "effect on the next frame.\n\n" + "CAVEAT: handler runs on the cpp-mcp listener " + "thread, NOT the render thread. Settings that " + "merely deserialize into member variables are " + "safe (the same pattern the ImGui menu uses on " + "the input thread). Settings whose LoadSettings " + "synchronously rebuilds GPU resources may " + "race the renderer — to be tightened with a " + "command queue in a follow-up commit.") + .with_string_param("shortName", + "Feature shortName as returned by list_features.") + .with_object_param("settings", + "Settings blob as returned by get_feature_settings.", + mcp::json::object()) + .build(); + server->register_tool(tool, + [](const mcp::json& params, const std::string& /*session_id*/) -> mcp::json { + const std::string shortName = params.value("shortName", std::string{}); + if (shortName.empty()) { + return ErrorResult("missing required parameter 'shortName'"); + } + if (!params.contains("settings") || !params["settings"].is_object()) { + return ErrorResult("missing required object parameter 'settings'"); + } + auto* feature = Feature::FindFeatureByShortName(shortName); + if (!feature) { + return ErrorResult("feature not found or not loaded", + { { "shortName", shortName } }); + } + // Round-trip through dump/parse to convert from cpp-mcp's + // ordered_json (params) to the feature's plain nlohmann::json. + ::json blob; + try { + blob = ::json::parse(params["settings"].dump()); + } catch (const std::exception& e) { + return ErrorResult("settings is not valid JSON", + { { "detail", e.what() } }); + } + try { + feature->LoadSettings(blob); + } catch (const std::exception& e) { + return ErrorResult("LoadSettings threw", + { { "shortName", shortName }, { "detail", e.what() } }); + } + logger::info("Remote Control: set_feature_settings({})", shortName); + return TextResult(mcp::json({ + { "shortName", shortName }, + { "applied", true }, + }) + .dump()); + }); +} + void RemoteControl::RegisterGetFeatureSettingsTool() { const auto tool = mcp::tool_builder("get_feature_settings") diff --git a/src/Features/RemoteControl.h b/src/Features/RemoteControl.h index 57ef5eab2e..902319a60c 100644 --- a/src/Features/RemoteControl.h +++ b/src/Features/RemoteControl.h @@ -76,6 +76,7 @@ class RemoteControl : public Feature void RegisterGetStateTool(); void RegisterListFeaturesTool(); void RegisterGetFeatureSettingsTool(); + void RegisterSetFeatureSettingsTool(); void RegisterToggleFeatureTool(); std::unique_ptr server; From 4e2b3d6db703fe77e9ed779e6e7d61c949beffff Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Tue, 19 May 2026 22:25:32 -0700 Subject: [PATCH 06/17] feat(remote-control): add reset_feature_settings tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restore a feature to its built-in defaults via Feature::RestoreDefaultSettings(). Semantically distinct from set_feature_settings({}) — RestoreDefaultSettings is per-feature reset logic (may release or recreate state that an empty-blob LoadSettings would not touch). Useful as the "B" side of A/B comparisons: snapshot with get_feature_settings, reset, observe, then restore with set_feature_settings. Co-Authored-By: Claude Opus 4.7 --- src/Features/RemoteControl.cpp | 47 ++++++++++++++++++++++++++++++++++ src/Features/RemoteControl.h | 1 + 2 files changed, 48 insertions(+) diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index a5bcaa0ca0..0c0b131d63 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -230,6 +230,7 @@ void RemoteControl::RegisterTools() RegisterGetFeatureSettingsTool(); RegisterToggleFeatureTool(); RegisterSetFeatureSettingsTool(); + RegisterResetFeatureSettingsTool(); } void RemoteControl::RegisterGetStateTool() @@ -330,6 +331,52 @@ void RemoteControl::RegisterToggleFeatureTool() }); } +void RemoteControl::RegisterResetFeatureSettingsTool() +{ + const auto tool = mcp::tool_builder("reset_feature_settings") + .with_description( + "Restore a feature's settings to their built-in " + "defaults via Feature::RestoreDefaultSettings(). " + "Distinct from set_feature_settings({}) because " + "RestoreDefaultSettings is feature-specific reset " + "logic (may release/recreate per-feature state in " + "ways LoadSettings({}) does not).\n\n" + "Useful as the 'B' side of an A/B test: capture " + "current settings with get_feature_settings, run " + "reset_feature_settings, observe via tracy + a " + "renderdoc capture, then call set_feature_settings " + "with the captured blob to return to the user's " + "configuration. Same listener-thread caveats as " + "set_feature_settings.") + .with_string_param("shortName", + "Feature shortName as returned by list_features.") + .build(); + server->register_tool(tool, + [](const mcp::json& params, const std::string& /*session_id*/) -> mcp::json { + const std::string shortName = params.value("shortName", std::string{}); + if (shortName.empty()) { + return ErrorResult("missing required parameter 'shortName'"); + } + auto* feature = Feature::FindFeatureByShortName(shortName); + if (!feature) { + return ErrorResult("feature not found or not loaded", + { { "shortName", shortName } }); + } + try { + feature->RestoreDefaultSettings(); + } catch (const std::exception& e) { + return ErrorResult("RestoreDefaultSettings threw", + { { "shortName", shortName }, { "detail", e.what() } }); + } + logger::info("Remote Control: reset_feature_settings({})", shortName); + return TextResult(mcp::json({ + { "shortName", shortName }, + { "reset", true }, + }) + .dump()); + }); +} + void RemoteControl::RegisterSetFeatureSettingsTool() { // Empty schema for the "settings" object means "any shape" — the actual diff --git a/src/Features/RemoteControl.h b/src/Features/RemoteControl.h index 902319a60c..243e2b4a5f 100644 --- a/src/Features/RemoteControl.h +++ b/src/Features/RemoteControl.h @@ -77,6 +77,7 @@ class RemoteControl : public Feature void RegisterListFeaturesTool(); void RegisterGetFeatureSettingsTool(); void RegisterSetFeatureSettingsTool(); + void RegisterResetFeatureSettingsTool(); void RegisterToggleFeatureTool(); std::unique_ptr server; From 43fdba78d28a84e4701728c4d521915f87f322c5 Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Tue, 19 May 2026 22:26:33 -0700 Subject: [PATCH 07/17] feat(remote-control): add console tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Singular MCP tool for the entire console concern. Queues the command onto the main game thread via SKSE::GetTaskInterface()-> AddTask so RE::Console::ExecuteCommand runs on the next tick, honoring Skyrim's script-VM main-thread requirement. Fire-and-forget by design: RE::Console::ExecuteCommand is void and RE::ConsoleLog is a shared sink with no command-to-output correlation; many useful commands (tcl, tfc, tg, tm…) are silent. Scraping the log would produce misleading positives often enough to be worse than not exposing it. To verify state changes, callers should poll get_state.frame_count past the returned enqueued_at_frame and observe via tracy/renderdoc/screenshot side channels. Response shape: { "queued": true, "command": "…", "enqueued_at_frame": N } Following the agentic-renderdoc philosophy of fewer, semantically rich tools, this is one tool rather than e.g. execute_console, read_console_log, get_console_history. Future console-related capabilities get added as optional parameters on this same tool, not as new top-level tools. Co-Authored-By: Claude Opus 4.7 --- src/Features/RemoteControl.cpp | 67 ++++++++++++++++++++++++++++++++++ src/Features/RemoteControl.h | 1 + 2 files changed, 68 insertions(+) diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index 0c0b131d63..bfe2384c00 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -231,6 +231,7 @@ void RemoteControl::RegisterTools() RegisterToggleFeatureTool(); RegisterSetFeatureSettingsTool(); RegisterResetFeatureSettingsTool(); + RegisterConsoleTool(); } void RemoteControl::RegisterGetStateTool() @@ -331,6 +332,72 @@ void RemoteControl::RegisterToggleFeatureTool() }); } +void RemoteControl::RegisterConsoleTool() +{ + // Singular tool for the entire console concern. Future console-related + // capabilities (history readout, command lookup, etc.) get added as + // optional parameters / additional response fields here rather than as + // separate tools — per the "fewer, semantically rich tools" philosophy. + const auto tool = mcp::tool_builder("console") + .with_description( + "Execute a Skyrim console command. Fire-and-forget: " + "the command is queued onto the main game thread via " + "SKSE's TaskInterface and runs on the next tick. " + "Returns immediately with the frame counter at the " + "moment of enqueue.\n\n" + "RE::Console::ExecuteCommand is `void` — there is " + "no per-command return value. RE::ConsoleLog is a " + "shared sink (engine + every SKSE plugin) with no " + "command-to-output correlation, and many useful " + "commands are silent (tcl, tfc, tg, tm, tlb…), so " + "scraping console output is unreliable and " + "intentionally NOT exposed.\n\n" + "To verify a state change, poll get_state until " + "frame_count > enqueued_at_frame (at least one tick " + "elapsed), then observe via side channels: tracy " + "captures for perf-affecting changes, " + "capture(kind='renderdoc'|'screenshot') for visual " + "confirmation, or future feature-specific get_* " + "tools that read RE:: state directly.\n\n" + "Common A/B-relevant commands:\n" + " tcl — toggle player collision\n" + " tfc [1] — free camera (1 = pause game)\n" + " tg — toggle grass\n" + " tm — toggle menus / HUD\n" + " tll <0..15> — toggle land LOD level\n" + " setweather — force weather (persistent)\n" + " fw — force weather (temporary)\n" + " coc — teleport to cell\n" + " set timescale to N — game-time multiplier\n") + .with_string_param("command", + "The console command, exactly as typed after the ~ key.") + .build(); + server->register_tool(tool, + [](const mcp::json& params, const std::string& /*session_id*/) -> mcp::json { + std::string command = params.value("command", std::string{}); + if (command.empty()) { + return ErrorResult("missing required parameter 'command'"); + } + auto* task = SKSE::GetTaskInterface(); + if (!task) { + return ErrorResult("SKSE TaskInterface unavailable"); + } + const uint enqueuedFrame = globals::state ? globals::state->frameCount : 0; + // Capture by value so the string outlives this lambda's scope. + task->AddTask([command]() { + RE::Console::ExecuteCommand(command.c_str()); + }); + logger::info("Remote Control: console({}) queued at frame {}", + command, enqueuedFrame); + return TextResult(mcp::json({ + { "queued", true }, + { "command", std::move(command) }, + { "enqueued_at_frame", enqueuedFrame }, + }) + .dump()); + }); +} + void RemoteControl::RegisterResetFeatureSettingsTool() { const auto tool = mcp::tool_builder("reset_feature_settings") diff --git a/src/Features/RemoteControl.h b/src/Features/RemoteControl.h index 243e2b4a5f..f3bfa1e9f3 100644 --- a/src/Features/RemoteControl.h +++ b/src/Features/RemoteControl.h @@ -79,6 +79,7 @@ class RemoteControl : public Feature void RegisterSetFeatureSettingsTool(); void RegisterResetFeatureSettingsTool(); void RegisterToggleFeatureTool(); + void RegisterConsoleTool(); std::unique_ptr server; int activePort = 0; From 390b25a4fa18c92eb0e4747956c2c94e166a8b79 Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Tue, 19 May 2026 22:27:36 -0700 Subject: [PATCH 08/17] feat(remote-control): add capture tool (kind-dispatched) Single MCP tool for all frame-capture flavors, kind-dispatched. Per the agentic-renderdoc design philosophy, one rich tool beats two parallel tools (capture_renderdoc, capture_screenshot) that would each need their own description and increase agent decision cost. Future kinds (tracy snapshot, video clip) extend the same tool. Supported kinds: - renderdoc: wraps existing RenderDoc::TriggerCapture / TriggerMultiFrameCapture. Honors `frames` (1-120, default 1). Returns 400-equivalent if RenderDoc feature isn't loaded or in-app DLL isn't attached. - screenshot: signals ScreenshotFeature::captureRequested atomic (existing lossless non-blocking path). `frames` ignored. Fire-and-forget; no artifact path is returned. RenderDoc captures land in its configured dir; screenshots in Screenshots/. Callers must observe via the filesystem (or, for renderdoc, attach the UI). Co-Authored-By: Claude Opus 4.7 --- src/Features/RemoteControl.cpp | 98 ++++++++++++++++++++++++++++++++++ src/Features/RemoteControl.h | 1 + 2 files changed, 99 insertions(+) diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index bfe2384c00..5d2b966bfe 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -232,6 +232,7 @@ void RemoteControl::RegisterTools() RegisterSetFeatureSettingsTool(); RegisterResetFeatureSettingsTool(); RegisterConsoleTool(); + RegisterCaptureTool(); } void RemoteControl::RegisterGetStateTool() @@ -332,6 +333,103 @@ void RemoteControl::RegisterToggleFeatureTool() }); } +void RemoteControl::RegisterCaptureTool() +{ + // One tool for all frame-capture kinds, kind-dispatched. Adding new + // capture types later (e.g. tracy snapshot, video clip) extends this + // tool's `kind` enum rather than spawning new top-level tools. + const auto tool = mcp::tool_builder("capture") + .with_description( + "Trigger a frame capture on the next render. Kind-" + "dispatched so all capture flavors live behind one " + "tool — see the agentic-renderdoc design notes.\n\n" + "Supported kinds:\n" + " renderdoc — RenderDoc multi-frame capture via " + "the in-application API. Honors the `frames` " + "parameter (default 1, max 120). RenderDoc must " + "be attached or the in-app DLL loaded; check " + "list_features for RenderDoc loaded=true. Output " + "lands in RenderDoc's configured captures dir.\n" + " screenshot — Lossless screenshot via the " + "Screenshot feature's non-blocking capture path. " + "The `frames` parameter is ignored. Output lands " + "in the game's Screenshots/ folder.\n\n" + "Fire-and-forget: the trigger flag is set " + "immediately and the render loop consumes it on " + "the next frame. No artifact path is returned " + "synchronously — for renderdoc, inspect the " + "captures directory; for screenshots, watch the " + "Screenshots folder.") + .with_string_param("kind", + "'renderdoc' or 'screenshot'.") + .with_number_param("frames", + "RenderDoc only: number of consecutive frames to " + "capture (1-120). Default 1. Ignored for " + "screenshot.", + /*required=*/false) + .build(); + server->register_tool(tool, + [](const mcp::json& params, const std::string& /*session_id*/) -> mcp::json { + const std::string kind = params.value("kind", std::string{}); + if (kind.empty()) { + return ErrorResult("missing required parameter 'kind'"); + } + const uint enqueuedFrame = globals::state ? globals::state->frameCount : 0; + + if (kind == "renderdoc") { + auto* renderDoc = &globals::features::renderDoc; + if (!renderDoc->loaded) { + return ErrorResult("RenderDoc feature is not loaded", + { { "hint", "list_features shows RenderDoc.loaded" } }); + } + if (!renderDoc->IsAvailable()) { + return ErrorResult( + "RenderDoc API not available — attach RenderDoc or " + "load the in-app DLL"); + } + uint32_t frameCount = 1; + if (params.contains("frames") && params["frames"].is_number()) { + const auto raw = params["frames"].get(); + frameCount = static_cast(std::clamp(raw, 1, 120)); + } + if (frameCount == 1) { + renderDoc->TriggerCapture(); + } else { + renderDoc->TriggerMultiFrameCapture(frameCount); + } + logger::info("Remote Control: capture(renderdoc, {}) at frame {}", + frameCount, enqueuedFrame); + return TextResult(mcp::json({ + { "queued", true }, + { "kind", "renderdoc" }, + { "frames", frameCount }, + { "enqueued_at_frame", enqueuedFrame }, + }) + .dump()); + } + + if (kind == "screenshot") { + auto* shot = &globals::features::screenshotFeature; + if (!shot->loaded) { + return ErrorResult("Screenshot feature is not loaded"); + } + shot->captureRequested.store(true, std::memory_order_release); + logger::info("Remote Control: capture(screenshot) at frame {}", + enqueuedFrame); + return TextResult(mcp::json({ + { "queued", true }, + { "kind", "screenshot" }, + { "enqueued_at_frame", enqueuedFrame }, + }) + .dump()); + } + + return ErrorResult("unknown kind", + { { "kind", kind }, + { "supported", mcp::json::array({ "renderdoc", "screenshot" }) } }); + }); +} + void RemoteControl::RegisterConsoleTool() { // Singular tool for the entire console concern. Future console-related diff --git a/src/Features/RemoteControl.h b/src/Features/RemoteControl.h index f3bfa1e9f3..c24d51cef6 100644 --- a/src/Features/RemoteControl.h +++ b/src/Features/RemoteControl.h @@ -80,6 +80,7 @@ class RemoteControl : public Feature void RegisterResetFeatureSettingsTool(); void RegisterToggleFeatureTool(); void RegisterConsoleTool(); + void RegisterCaptureTool(); std::unique_ptr server; int activePort = 0; From aac30ad4e6e79cf8ac39cd5623c772ff0632aecb Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Tue, 19 May 2026 22:33:01 -0700 Subject: [PATCH 09/17] feat(remote-control): add connected-clients ImGui table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In-game visibility for who's currently driving the MCP server. Sortable ImGui table showing session id, connect time, idle time, request count, and last tool invoked. Tracked entirely in-process — no MCP tool exposes this; it's a human-debugging surface. Bookkeeping is updated on every tool handler invocation (lambda captures `this` and calls RecordToolCall(session_id, toolName) before the existing body) and on cpp-mcp's session-cleanup callback when a client disconnects. A std::mutex protects the session map across the listener and main threads. Per-session force-disconnect is intentionally not surfaced — cpp-mcp's public API doesn't expose it cleanly. The settings UI documents that toggling "Enable MCP server" off/on serves as the panic-button "kick everyone". Co-Authored-By: Claude Opus 4.7 --- src/Features/RemoteControl.cpp | 179 +++++++++++++++++++++++++++++++-- src/Features/RemoteControl.h | 32 ++++++ 2 files changed, 203 insertions(+), 8 deletions(-) diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index 5d2b966bfe..925345fdb3 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -126,6 +126,129 @@ void RemoteControl::DrawSettings() ImGui::TextUnformatted(preview.c_str()); ImGui::PopTextWrapPos(); } + + ImGui::Separator(); + DrawClientsTable(); +} + +void RemoteControl::DrawClientsTable() +{ + // Snapshot under the lock to keep the listener-thread updates from + // racing the draw. The snapshot is small (a handful of sessions at most). + std::vector rows; + { + std::lock_guard lock(sessionMutex); + rows.reserve(sessions.size()); + for (const auto& [_, info] : sessions) { + rows.push_back(info); + } + } + + const std::string headerLabel = std::format("Connected clients ({})##rc-clients", rows.size()); + if (!ImGui::CollapsingHeader(headerLabel.c_str(), ImGuiTreeNodeFlags_DefaultOpen)) { + return; + } + + if (!IsRunning()) { + ImGui::TextDisabled("Server not running."); + return; + } + if (rows.empty()) { + ImGui::TextDisabled( + "No clients connected. Paste the config above into " + "your MCP host and run a tool to populate this table."); + return; + } + + constexpr ImGuiTableFlags flags = ImGuiTableFlags_Borders | ImGuiTableFlags_Resizable | + ImGuiTableFlags_RowBg | ImGuiTableFlags_Sortable | + ImGuiTableFlags_SortMulti | ImGuiTableFlags_ScrollY; + + enum ColumnId : ImGuiID + { + ColSession = 0, + ColConnected, + ColIdle, + ColRequests, + ColLastTool, + }; + + if (ImGui::BeginTable("##rc-clients-table", 5, flags, ImVec2(0.0f, 120.0f))) { + ImGui::TableSetupColumn("Session", ImGuiTableColumnFlags_DefaultSort, 0.0f, ColSession); + ImGui::TableSetupColumn("Connected", 0, 0.0f, ColConnected); + ImGui::TableSetupColumn("Idle for", 0, 0.0f, ColIdle); + ImGui::TableSetupColumn("Requests", 0, 0.0f, ColRequests); + ImGui::TableSetupColumn("Last tool", 0, 0.0f, ColLastTool); + ImGui::TableSetupScrollFreeze(0, 1); + ImGui::TableHeadersRow(); + + if (auto* sortSpecs = ImGui::TableGetSortSpecs(); sortSpecs && sortSpecs->SpecsCount > 0) { + std::sort(rows.begin(), rows.end(), + [&](const SessionInfo& a, const SessionInfo& b) { + for (int i = 0; i < sortSpecs->SpecsCount; ++i) { + const auto& spec = sortSpecs->Specs[i]; + const bool desc = spec.SortDirection == ImGuiSortDirection_Descending; + int cmp = 0; + switch (static_cast(spec.ColumnUserID)) { + case ColSession: + cmp = a.id.compare(b.id); + break; + case ColConnected: + cmp = a.connected < b.connected ? -1 : (a.connected > b.connected ? 1 : 0); + break; + case ColIdle: + cmp = a.lastSeen < b.lastSeen ? -1 : (a.lastSeen > b.lastSeen ? 1 : 0); + break; + case ColRequests: + cmp = a.requestCount < b.requestCount ? -1 : (a.requestCount > b.requestCount ? 1 : 0); + break; + case ColLastTool: + cmp = a.lastTool.compare(b.lastTool); + break; + } + if (cmp != 0) { + return desc ? cmp > 0 : cmp < 0; + } + } + return false; + }); + } + + const auto now = std::chrono::system_clock::now(); + const auto formatRelative = [](std::chrono::seconds sec) -> std::string { + const auto s = sec.count(); + if (s < 60) { + return std::format("{}s ago", s); + } + if (s < 3600) { + return std::format("{}m {}s ago", s / 60, s % 60); + } + return std::format("{}h {}m ago", s / 3600, (s % 3600) / 60); + }; + + for (const auto& info : rows) { + ImGui::TableNextRow(); + const auto connectedSec = std::chrono::duration_cast(now - info.connected); + const auto idleSec = std::chrono::duration_cast(now - info.lastSeen); + + ImGui::TableSetColumnIndex(0); + ImGui::TextUnformatted(info.id.c_str()); + ImGui::TableSetColumnIndex(1); + ImGui::TextUnformatted(formatRelative(connectedSec).c_str()); + ImGui::TableSetColumnIndex(2); + ImGui::TextUnformatted(formatRelative(idleSec).c_str()); + ImGui::TableSetColumnIndex(3); + ImGui::Text("%llu", static_cast(info.requestCount)); + ImGui::TableSetColumnIndex(4); + ImGui::TextUnformatted(info.lastTool.empty() ? "(none)" : info.lastTool.c_str()); + } + + ImGui::EndTable(); + } + + ImGui::TextDisabled( + "To force-disconnect all clients, toggle 'Enable MCP server' off and back on. " + "Per-session kick is not exposed by cpp-mcp's public API."); } std::string RemoteControl::BuildClientConfig() const @@ -168,6 +291,14 @@ void RemoteControl::StartServer() RegisterTools(); + // Drop a session from the bookkeeping map on disconnect. cpp-mcp + // dispatches this from its listener thread when the SSE/HTTP + // connection tears down. + server->register_session_cleanup("remote-control-session-tracker", + [this](const std::string& sessionId) { + DropSession(sessionId); + }); + if (!server->start(false)) { // false = non-blocking throw std::runtime_error("server.start() returned false"); } @@ -195,9 +326,33 @@ void RemoteControl::StopServer() } server.reset(); activePort = 0; + { + std::lock_guard lock(sessionMutex); + sessions.clear(); + } logger::info("Remote Control: MCP server stopped"); } +void RemoteControl::RecordToolCall(const std::string& sessionId, const std::string& toolName) +{ + const auto now = std::chrono::system_clock::now(); + std::lock_guard lock(sessionMutex); + auto& info = sessions[sessionId]; + if (info.requestCount == 0) { + info.id = sessionId; + info.connected = now; + } + info.lastSeen = now; + info.requestCount += 1; + info.lastTool = toolName; +} + +void RemoteControl::DropSession(const std::string& sessionId) +{ + std::lock_guard lock(sessionMutex); + sessions.erase(sessionId); +} + // Helper: wrap a payload string in the MCP tool-result content envelope // (an array of typed content items). Tools return application data as the // "text" field of a single content item; consumers typically parse it as @@ -243,7 +398,8 @@ void RemoteControl::RegisterGetStateTool() "frame counter, plugin version, VR mode.") .build(); server->register_tool(tool, - [](const mcp::json& /*params*/, const std::string& /*session_id*/) -> mcp::json { + [this](const mcp::json& /*params*/, const std::string& session_id) -> mcp::json { + RecordToolCall(session_id, "get_state"); const uint frames = globals::state ? globals::state->frameCount : 0; const bool vr = REL::Module::IsVR(); return TextResult(std::format( @@ -262,7 +418,8 @@ void RemoteControl::RegisterListFeaturesTool() "isCore, supportsVR.") .build(); server->register_tool(tool, - [](const mcp::json& /*params*/, const std::string& /*session_id*/) -> mcp::json { + [this](const mcp::json& /*params*/, const std::string& session_id) -> mcp::json { + RecordToolCall(session_id, "list_features"); mcp::json features = mcp::json::array(); for (auto* f : Feature::GetFeatureList()) { features.push_back({ @@ -298,7 +455,8 @@ void RemoteControl::RegisterToggleFeatureTool() "true to load (run), false to skip.") .build(); server->register_tool(tool, - [](const mcp::json& params, const std::string& /*session_id*/) -> mcp::json { + [this](const mcp::json& params, const std::string& session_id) -> mcp::json { + RecordToolCall(session_id, "toggle_feature"); const std::string shortName = params.value("shortName", std::string{}); if (shortName.empty()) { return ErrorResult("missing required parameter 'shortName'"); @@ -369,7 +527,8 @@ void RemoteControl::RegisterCaptureTool() /*required=*/false) .build(); server->register_tool(tool, - [](const mcp::json& params, const std::string& /*session_id*/) -> mcp::json { + [this](const mcp::json& params, const std::string& session_id) -> mcp::json { + RecordToolCall(session_id, "capture"); const std::string kind = params.value("kind", std::string{}); if (kind.empty()) { return ErrorResult("missing required parameter 'kind'"); @@ -471,7 +630,8 @@ void RemoteControl::RegisterConsoleTool() "The console command, exactly as typed after the ~ key.") .build(); server->register_tool(tool, - [](const mcp::json& params, const std::string& /*session_id*/) -> mcp::json { + [this](const mcp::json& params, const std::string& session_id) -> mcp::json { + RecordToolCall(session_id, "console"); std::string command = params.value("command", std::string{}); if (command.empty()) { return ErrorResult("missing required parameter 'command'"); @@ -517,7 +677,8 @@ void RemoteControl::RegisterResetFeatureSettingsTool() "Feature shortName as returned by list_features.") .build(); server->register_tool(tool, - [](const mcp::json& params, const std::string& /*session_id*/) -> mcp::json { + [this](const mcp::json& params, const std::string& session_id) -> mcp::json { + RecordToolCall(session_id, "reset_feature_settings"); const std::string shortName = params.value("shortName", std::string{}); if (shortName.empty()) { return ErrorResult("missing required parameter 'shortName'"); @@ -569,7 +730,8 @@ void RemoteControl::RegisterSetFeatureSettingsTool() mcp::json::object()) .build(); server->register_tool(tool, - [](const mcp::json& params, const std::string& /*session_id*/) -> mcp::json { + [this](const mcp::json& params, const std::string& session_id) -> mcp::json { + RecordToolCall(session_id, "set_feature_settings"); const std::string shortName = params.value("shortName", std::string{}); if (shortName.empty()) { return ErrorResult("missing required parameter 'shortName'"); @@ -620,7 +782,8 @@ void RemoteControl::RegisterGetFeatureSettingsTool() "Feature shortName as returned by list_features.") .build(); server->register_tool(tool, - [](const mcp::json& params, const std::string& /*session_id*/) -> mcp::json { + [this](const mcp::json& params, const std::string& session_id) -> mcp::json { + RecordToolCall(session_id, "get_feature_settings"); const std::string shortName = params.value("shortName", std::string{}); if (shortName.empty()) { return ErrorResult("missing required parameter 'shortName'"); diff --git a/src/Features/RemoteControl.h b/src/Features/RemoteControl.h index c24d51cef6..4cfd95a126 100644 --- a/src/Features/RemoteControl.h +++ b/src/Features/RemoteControl.h @@ -2,9 +2,13 @@ #include "Feature.h" +#include +#include #include +#include #include #include +#include using json = nlohmann::json; @@ -13,6 +17,11 @@ using json = nlohmann::json; namespace mcp { class server; + struct tool; + // cpp-mcp's tool_handler is std::function + // but json there is ordered_json, which we can't forward-declare cleanly. + // We type-erase via std::function in the tool-tracking layer and + // keep the real signature local to the .cpp file. } class RemoteControl : public Feature @@ -67,6 +76,18 @@ class RemoteControl : public Feature RemoteControl(RemoteControl&&) = delete; RemoteControl& operator=(RemoteControl&&) = delete; + // Session bookkeeping for the ImGui "Connected clients" table. + // Updated on every tool invocation (listener thread) and on session + // cleanup (cpp-mcp callback). Read from the main thread when drawing. + struct SessionInfo + { + std::string id; + std::chrono::system_clock::time_point connected; + std::chrono::system_clock::time_point lastSeen; + uint64_t requestCount = 0; + std::string lastTool; + }; + private: void StartServer(); void StopServer(); @@ -82,7 +103,18 @@ class RemoteControl : public Feature void RegisterConsoleTool(); void RegisterCaptureTool(); + // Records a tool invocation against the per-session table. + // Safe to call from the cpp-mcp listener thread. + void RecordToolCall(const std::string& sessionId, const std::string& toolName); + // Drops a session from the table on disconnect. + void DropSession(const std::string& sessionId); + // Draws the connected-clients ImGui table. + void DrawClientsTable(); + std::unique_ptr server; int activePort = 0; std::string lastError; + + mutable std::mutex sessionMutex; + std::unordered_map sessions; }; From 2dc74ff62f146c14d811c9ff3e556e6e6f32ad68 Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Tue, 19 May 2026 22:34:28 -0700 Subject: [PATCH 10/17] feat(remote-control): add abtest tool (action-dispatched) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Single MCP tool driving the full A/B testing lifecycle in features/Performance Overlay/ABTesting. Action-dispatched per the agentic-renderdoc design: one rich tool instead of separate start_abtest / stop_abtest / get_abtest_results / clear_snapshots / set_interval tools. Actions: status — return enabled, usingTestConfig, interval, hasCachedSnapshots start — Enable() the rotation. Optional `interval` parameter (seconds) applied first if provided. stop — Disable(). Snapshots retained. clear — ClearCachedSnapshots(). diff — return the per-setting USER vs TEST diff list so callers know exactly what the rotation is toggling. Setup of the TEST config itself remains in the Performance Overlay UI — this tool drives lifecycle only, not test-config authoring. Co-Authored-By: Claude Opus 4.7 --- src/Features/RemoteControl.cpp | 109 +++++++++++++++++++++++++++++++++ src/Features/RemoteControl.h | 1 + 2 files changed, 110 insertions(+) diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index 925345fdb3..5e74cf5267 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -8,6 +8,7 @@ #include "Features/RemoteControl.h" +#include "Features/PerformanceOverlay/ABTesting/ABTesting.h" #include "Globals.h" #include "State.h" @@ -388,6 +389,7 @@ void RemoteControl::RegisterTools() RegisterResetFeatureSettingsTool(); RegisterConsoleTool(); RegisterCaptureTool(); + RegisterAbtestTool(); } void RemoteControl::RegisterGetStateTool() @@ -491,6 +493,113 @@ void RemoteControl::RegisterToggleFeatureTool() }); } +void RemoteControl::RegisterAbtestTool() +{ + // Single tool for the entire A/B testing lifecycle. Action-dispatched + // rather than spawning start_abtest / stop_abtest / get_abtest_results + // / clear_abtest_snapshots / set_abtest_interval — fewer richer tools. + const auto tool = mcp::tool_builder("abtest") + .with_description( + "Drive the built-in A/B testing harness " + "(features/Performance Overlay/ABTesting). The " + "harness rotates between a USER configuration " + "(your current settings) and a TEST configuration " + "(typically a preset under test) on a fixed " + "interval, snapshots both in memory to avoid disk " + "I/O during swaps, and aggregates per-variant " + "frame timing so you can compare quality and perf.\n\n" + "Actions:\n" + " status — return enabled, usingTestConfig, " + "interval, hasCachedSnapshots.\n" + " start — Enable() the manager (begin rotating). " + "Optional `interval` parameter (seconds) is applied " + "first if provided.\n" + " stop — Disable() the manager. Snapshots are " + "retained.\n" + " clear — ClearCachedSnapshots(). Use to reset " + "before a fresh comparison.\n" + " diff — return the per-key diff list " + "(GetConfigDiffEntries) so callers know which " + "settings the rotation is actually toggling.\n\n" + "Setup of the TEST config itself lives in the " + "Performance Overlay UI — this tool only drives " + "the lifecycle, not the test-config authoring.") + .with_string_param("action", + "'status', 'start', 'stop', 'clear', or 'diff'.") + .with_number_param("interval", + "Seconds per variant when action='start'. " + "Default 0 (no change).", + /*required=*/false) + .build(); + server->register_tool(tool, + [this](const mcp::json& params, const std::string& session_id) -> mcp::json { + RecordToolCall(session_id, "abtest"); + const std::string action = params.value("action", std::string{}); + if (action.empty()) { + return ErrorResult("missing required parameter 'action'"); + } + auto* mgr = ABTestingManager::GetSingleton(); + if (!mgr) { + return ErrorResult("ABTestingManager singleton unavailable"); + } + + const auto statusBlob = [&]() { + return mcp::json({ + { "enabled", mgr->IsEnabled() }, + { "usingTestConfig", mgr->IsUsingTestConfig() }, + { "interval", mgr->GetTestInterval() }, + { "hasCachedSnapshots", mgr->HasCachedSnapshots() }, + }); + }; + + if (action == "status") { + return TextResult(statusBlob().dump()); + } + if (action == "start") { + if (params.contains("interval") && params["interval"].is_number()) { + const auto secs = params["interval"].get(); + if (secs > 0) { + mgr->SetTestInterval(static_cast(secs)); + } + } + mgr->Enable(); + logger::info("Remote Control: abtest(start)"); + return TextResult(statusBlob().dump()); + } + if (action == "stop") { + mgr->Disable(); + logger::info("Remote Control: abtest(stop)"); + return TextResult(statusBlob().dump()); + } + if (action == "clear") { + mgr->ClearCachedSnapshots(); + logger::info("Remote Control: abtest(clear)"); + return TextResult(statusBlob().dump()); + } + if (action == "diff") { + mcp::json entries = mcp::json::array(); + for (const auto& entry : mgr->GetConfigDiffEntries()) { + // SettingsDiffEntry uses generic a/b labels (see + // Utils/FileSystem.h). For A/B testing semantics here, + // `a` is USER and `b` is TEST. + entries.push_back({ + { "path", entry.path }, + { "userValue", entry.aValue }, + { "testValue", entry.bValue }, + }); + } + return TextResult(mcp::json({ + { "hasCachedSnapshots", mgr->HasCachedSnapshots() }, + { "entries", std::move(entries) }, + }) + .dump()); + } + return ErrorResult("unknown action", + { { "action", action }, + { "supported", mcp::json::array({ "status", "start", "stop", "clear", "diff" }) } }); + }); +} + void RemoteControl::RegisterCaptureTool() { // One tool for all frame-capture kinds, kind-dispatched. Adding new diff --git a/src/Features/RemoteControl.h b/src/Features/RemoteControl.h index 4cfd95a126..887335cd88 100644 --- a/src/Features/RemoteControl.h +++ b/src/Features/RemoteControl.h @@ -102,6 +102,7 @@ class RemoteControl : public Feature void RegisterToggleFeatureTool(); void RegisterConsoleTool(); void RegisterCaptureTool(); + void RegisterAbtestTool(); // Records a tool invocation against the per-session table. // Safe to call from the cpp-mcp listener thread. From 94e99dff6f6f27f82daf6d65d490375e73320b1a Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Tue, 19 May 2026 22:37:30 -0700 Subject: [PATCH 11/17] fix(remote-control): add feature includes for capture tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The capture tool dereferences globals::features::renderDoc and ::screenshotFeature, which were only forward-declared via Globals.h. The previous commit (118e2e564) compiled because nothing yet touched those pointers — adding the tool exposed the missing includes. Pull RenderDoc.h and ScreenshotFeature.h so the member calls (IsAvailable, TriggerCapture, TriggerMultiFrameCapture, captureRequested.store) resolve. Folding this into the capture commit would require an interactive rebase (against project guidance); leaving as a separate fix so each commit stays self-contained and bisect-correct from this point forward. Co-Authored-By: Claude Opus 4.7 --- src/Features/RemoteControl.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index 5e74cf5267..8b814ba351 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -9,6 +9,8 @@ #include "Features/RemoteControl.h" #include "Features/PerformanceOverlay/ABTesting/ABTesting.h" +#include "Features/RenderDoc.h" +#include "Features/ScreenshotFeature.h" #include "Globals.h" #include "State.h" From d47e4053529aaaf570cd06cd04e61f8d4abb9446 Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Tue, 19 May 2026 22:44:28 -0700 Subject: [PATCH 12/17] refactor(remote-control): consolidate to 5 tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the agentic-renderdoc design (https://github.com/alandtse/ agentic-renderdoc#why-this-design), "fewer, semantically rich tools outperform expansive tool suites" — the tool description IS the prompt engineering and each tool's surface should encode operational expertise rather than expose one-method-per-call. We weren't shipped, so collapse the 9-tool surface to 5: inspect(kind) — non-feature engine reads (currently kind='state'; extensible) feature(action, ...) — all feature ops: list / get / set / reset / toggle console(command) — unchanged capture(kind, frames?) — unchanged abtest(action, interval?) — unchanged Removed tools (folded into the new two): get_state → inspect(kind='state') list_features → feature(action='list') get_feature_settings → feature(action='get', shortName) set_feature_settings → feature(action='set', shortName, settings) reset_feature_settings → feature(action='reset', shortName) toggle_feature → feature(action='toggle', shortName, enabled) The two new tool descriptions are deliberately rich (per the philosophy): each documents every action's params, the timing / threading semantics, the A/B testing flow as a worked example, and the known gotchas (LightLimitFix's missing SaveSettings/ LoadSettings override means get returns null and set is a silent no-op; toggle keeps GPU resources alive). Shared helpers (EngineStateBlob, FeatureEntry) are factored as static-file-locals so the same blobs can be reused if we add future tools that surface the same data in different contexts. The ImGui session-tracking table continues to work — handlers still call RecordToolCall(session_id, "") inline; only the recorded names change. Co-Authored-By: Claude Opus 4.7 --- src/Features/RemoteControl.cpp | 460 +++++++++++++++++---------------- src/Features/RemoteControl.h | 8 +- 2 files changed, 234 insertions(+), 234 deletions(-) diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index 8b814ba351..24f64e5598 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -383,115 +383,265 @@ static mcp::json ErrorResult(std::string_view message, mcp::json context = {}) void RemoteControl::RegisterTools() { - RegisterGetStateTool(); - RegisterListFeaturesTool(); - RegisterGetFeatureSettingsTool(); - RegisterToggleFeatureTool(); - RegisterSetFeatureSettingsTool(); - RegisterResetFeatureSettingsTool(); - RegisterConsoleTool(); - RegisterCaptureTool(); - RegisterAbtestTool(); + // Five tools, each semantically rich. Reads vs writes vs lifecycles are + // separated by tool; within each tool, kind/action discriminates the + // specific operation. See agentic-renderdoc's "Why this design" notes — + // fewer rich tools outperform expansive suites because the agent reads + // fewer descriptions and each description carries the operational + // expertise (timing, gotchas, verification routes). + RegisterInspectTool(); // reads (non-feature engine state) + RegisterFeatureTool(); // all feature ops (list/get/set/reset/toggle) + RegisterConsoleTool(); // Skyrim console passthrough + RegisterCaptureTool(); // frame capture (renderdoc/screenshot) + RegisterAbtestTool(); // A/B test lifecycle } -void RemoteControl::RegisterGetStateTool() +// Helper used by both inspect(kind="state") and (potentially) future tools. +static mcp::json EngineStateBlob() { - const auto tool = mcp::tool_builder("get_state") - .with_description( - "Return Community Shaders runtime state: " - "frame counter, plugin version, VR mode.") - .build(); - server->register_tool(tool, - [this](const mcp::json& /*params*/, const std::string& session_id) -> mcp::json { - RecordToolCall(session_id, "get_state"); - const uint frames = globals::state ? globals::state->frameCount : 0; - const bool vr = REL::Module::IsVR(); - return TextResult(std::format( - R"({{"frame_count":{},"vr":{},"plugin":"CommunityShaders"}})", - frames, vr ? "true" : "false")); - }); + const uint frames = globals::state ? globals::state->frameCount : 0; + const bool vr = REL::Module::IsVR(); + return mcp::json({ + { "plugin", "CommunityShaders" }, + { "frame_count", frames }, + { "vr", vr }, + }); } -void RemoteControl::RegisterListFeaturesTool() +// Helper used by feature(action="list") to build one entry per feature. +static mcp::json FeatureEntry(Feature* f) { - const auto tool = mcp::tool_builder("list_features") + return mcp::json({ + { "name", f->GetName() }, + { "shortName", f->GetShortName() }, + { "loaded", f->loaded }, + { "version", f->version }, + { "category", std::string(f->GetCategory()) }, + { "isCore", f->IsCore() }, + { "supportsVR", f->SupportsVR() }, + { "inMenu", f->IsInMenu() }, + }); +} + +void RemoteControl::RegisterInspectTool() +{ + // Single read endpoint for non-feature engine state. Kind-discriminated + // so future engine reads (weather, cell, player, render targets) extend + // the same tool rather than spawning new top-level reads. For feature + // reads (list, get settings), use the `feature` tool with the + // corresponding action. + const auto tool = mcp::tool_builder("inspect") .with_description( - "Enumerate Community Shaders graphics features. " - "Returns a JSON array with one entry per feature: " - "name, shortName, loaded, version, category, " - "isCore, supportsVR.") + "Read non-feature engine state. Kind-dispatched; the " + "response is always a JSON object delivered as the " + "text of a single content item.\n\n" + "Kinds:\n" + " state — { plugin, frame_count, vr }. Frame counter " + "monotonically increases each render tick; use as a " + "ground truth for verifying that deferred operations " + "(see `console`) have had time to run.\n\n" + "For feature reads (enumerate / settings), use the " + "`feature` tool with action='list' or 'get'.") + .with_string_param("kind", + "Currently 'state'. New kinds will be added here " + "rather than as new tools.") .build(); server->register_tool(tool, - [this](const mcp::json& /*params*/, const std::string& session_id) -> mcp::json { - RecordToolCall(session_id, "list_features"); - mcp::json features = mcp::json::array(); - for (auto* f : Feature::GetFeatureList()) { - features.push_back({ - { "name", f->GetName() }, - { "shortName", f->GetShortName() }, - { "loaded", f->loaded }, - { "version", f->version }, - { "category", std::string(f->GetCategory()) }, - { "isCore", f->IsCore() }, - { "supportsVR", f->SupportsVR() }, - { "inMenu", f->IsInMenu() }, - }); + [this](const mcp::json& params, const std::string& session_id) -> mcp::json { + RecordToolCall(session_id, "inspect"); + const std::string kind = params.value("kind", std::string{}); + if (kind.empty()) { + return ErrorResult("missing required parameter 'kind'"); + } + if (kind == "state") { + return TextResult(EngineStateBlob().dump()); } - return TextResult(features.dump()); + return ErrorResult("unknown kind", + { { "kind", kind }, + { "supported", mcp::json::array({ "state" }) } }); }); } -void RemoteControl::RegisterToggleFeatureTool() +void RemoteControl::RegisterFeatureTool() { - const auto tool = mcp::tool_builder("toggle_feature") + // One tool for all graphics-feature operations. Action-dispatched so the + // agent has a single description that documents the full feature + // vocabulary plus the gotchas across all five operations (silent no-op + // for missing overrides, listener-thread caveats, etc). + const auto tool = mcp::tool_builder("feature") .with_description( - "Enable or disable a feature at runtime by " - "flipping its 'loaded' flag. Disabled features " - "are skipped by Feature::ForEachLoadedFeature so " - "their per-frame rendering work doesn't run. GPU " - "resources are NOT freed — this is for A/B " - "perf/quality comparisons, not memory reclaim. " - "Reverting only requires another toggle_feature " - "call.") + "All graphics-feature operations — enumerate, " + "inspect settings, mutate settings, restore defaults, " + "toggle on/off. Action-dispatched; each action takes " + "the parameters listed below.\n\n" + "Actions:\n" + " list — no other params. Returns a JSON array; " + "each entry has { name, shortName, loaded, version, " + "category, isCore, supportsVR, inMenu }.\n" + " get — params: shortName. Returns the " + "Feature::SaveSettings(json) blob. May return null " + "if the feature has no SaveSettings/LoadSettings " + "override (e.g. LightLimitFix); set/reset will " + "silently no-op for these.\n" + " set — params: shortName, settings (object). " + "Calls Feature::LoadSettings on the listener thread. " + "Safe for value-assigning LoadSettings (the common " + "case) and for features that flip a recompileFlag " + "(ScreenSpaceGI, DynamicCubemaps) — the render loop " + "picks them up on the next frame. Settings that " + "synchronously rebuild GPU resources would race; " + "none in-tree currently do.\n" + " reset — params: shortName. Calls " + "Feature::RestoreDefaultSettings(). Distinct from " + "set({}) because RestoreDefaultSettings is " + "feature-specific reset logic (may release/recreate " + "state).\n" + " toggle — params: shortName, enabled (boolean). " + "Flips Feature::loaded. Disabled features are " + "skipped by ForEachLoadedFeature so their per-frame " + "rendering work doesn't run. GPU resources allocated " + "in SetupResources are NOT freed — A/B perf/quality, " + "not memory reclaim.\n\n" + "A/B testing pattern:\n" + " 1. feature(action='get', shortName='Skylighting') → snapshot\n" + " 2. feature(action='reset', shortName='Skylighting') → defaults\n" + " 3. capture + tracy capture → measure\n" + " 4. feature(action='set', shortName='Skylighting', settings=) → restore\n\n" + "Gotchas:\n" + " • Some features have no SaveSettings/LoadSettings " + "override. `get` returns null; `set` and `reset` " + "claim success but don't change anything. Confirmed " + "case: LightLimitFix.\n" + " • toggle keeps GPU resources alive. If a feature " + "still affects rendering after `enabled=false`, it " + "has a hook that isn't gated on `loaded` — file an " + "issue with the shortName.") + .with_string_param("action", + "One of: 'list', 'get', 'set', 'reset', 'toggle'.") .with_string_param("shortName", - "Feature shortName as returned by list_features.") + "Required for all actions except 'list'. From the " + "list response.", + /*required=*/false) + .with_object_param("settings", + "Required for action='set'. Shape that matches what " + "action='get' returned for the same feature.", + mcp::json::object(), + /*required=*/false) .with_boolean_param("enabled", - "true to load (run), false to skip.") + "Required for action='toggle'.", + /*required=*/false) .build(); server->register_tool(tool, [this](const mcp::json& params, const std::string& session_id) -> mcp::json { - RecordToolCall(session_id, "toggle_feature"); + RecordToolCall(session_id, "feature"); + const std::string action = params.value("action", std::string{}); + if (action.empty()) { + return ErrorResult("missing required parameter 'action'"); + } + + if (action == "list") { + mcp::json features = mcp::json::array(); + for (auto* f : Feature::GetFeatureList()) { + features.push_back(FeatureEntry(f)); + } + return TextResult(features.dump()); + } + const std::string shortName = params.value("shortName", std::string{}); if (shortName.empty()) { - return ErrorResult("missing required parameter 'shortName'"); - } - if (!params.contains("enabled") || !params["enabled"].is_boolean()) { - return ErrorResult("missing required boolean parameter 'enabled'"); + return ErrorResult("missing required parameter 'shortName'", + { { "action", action } }); } - const bool desired = params["enabled"].get(); - // FindFeatureByShortName filters on `loaded == true`, so it won't - // help us re-enable a feature. Walk the full list ourselves. - Feature* target = nullptr; - for (auto* f : Feature::GetFeatureList()) { - if (f->GetShortName() == shortName) { - target = f; - break; + + if (action == "toggle") { + if (!params.contains("enabled") || !params["enabled"].is_boolean()) { + return ErrorResult("missing required boolean parameter 'enabled'"); + } + const bool desired = params["enabled"].get(); + // FindFeatureByShortName filters on loaded==true so it can't + // help re-enable; walk the full list ourselves. + Feature* target = nullptr; + for (auto* f : Feature::GetFeatureList()) { + if (f->GetShortName() == shortName) { + target = f; + break; + } + } + if (!target) { + return ErrorResult("feature not found", + { { "shortName", shortName } }); } + const bool previous = target->loaded; + target->loaded = desired; + logger::info("Remote Control: feature(toggle, {}, {}) (was {})", + shortName, desired, previous); + return TextResult(mcp::json({ + { "action", "toggle" }, + { "shortName", shortName }, + { "previous", previous }, + { "current", desired }, + }) + .dump()); } - if (!target) { - return ErrorResult("feature not found", + + auto* feature = Feature::FindFeatureByShortName(shortName); + if (!feature) { + return ErrorResult("feature not found or not loaded", { { "shortName", shortName } }); } - const bool previous = target->loaded; - target->loaded = desired; - logger::info("Remote Control: toggle_feature({}, {}) (was {})", - shortName, desired, previous); - return TextResult(mcp::json({ - { "shortName", shortName }, - { "previous", previous }, - { "current", desired }, - }) - .dump()); + + if (action == "get") { + // SaveSettings uses nlohmann::json (unordered). Keep the + // intermediate value as plain json and dump as a string so + // we don't have to round-trip through mcp::json's ordered map. + ::json blob; + feature->SaveSettings(blob); + return TextResult(blob.dump()); + } + if (action == "set") { + if (!params.contains("settings") || !params["settings"].is_object()) { + return ErrorResult("missing required object parameter 'settings'"); + } + ::json blob; + try { + blob = ::json::parse(params["settings"].dump()); + } catch (const std::exception& e) { + return ErrorResult("settings is not valid JSON", + { { "detail", e.what() } }); + } + try { + feature->LoadSettings(blob); + } catch (const std::exception& e) { + return ErrorResult("LoadSettings threw", + { { "shortName", shortName }, { "detail", e.what() } }); + } + logger::info("Remote Control: feature(set, {})", shortName); + return TextResult(mcp::json({ + { "action", "set" }, + { "shortName", shortName }, + { "applied", true }, + }) + .dump()); + } + if (action == "reset") { + try { + feature->RestoreDefaultSettings(); + } catch (const std::exception& e) { + return ErrorResult("RestoreDefaultSettings threw", + { { "shortName", shortName }, { "detail", e.what() } }); + } + logger::info("Remote Control: feature(reset, {})", shortName); + return TextResult(mcp::json({ + { "action", "reset" }, + { "shortName", shortName }, + { "reset", true }, + }) + .dump()); + } + + return ErrorResult("unknown action", + { { "action", action }, + { "supported", mcp::json::array({ "list", "get", "set", "reset", "toggle" }) } }); }); } @@ -766,149 +916,3 @@ void RemoteControl::RegisterConsoleTool() .dump()); }); } - -void RemoteControl::RegisterResetFeatureSettingsTool() -{ - const auto tool = mcp::tool_builder("reset_feature_settings") - .with_description( - "Restore a feature's settings to their built-in " - "defaults via Feature::RestoreDefaultSettings(). " - "Distinct from set_feature_settings({}) because " - "RestoreDefaultSettings is feature-specific reset " - "logic (may release/recreate per-feature state in " - "ways LoadSettings({}) does not).\n\n" - "Useful as the 'B' side of an A/B test: capture " - "current settings with get_feature_settings, run " - "reset_feature_settings, observe via tracy + a " - "renderdoc capture, then call set_feature_settings " - "with the captured blob to return to the user's " - "configuration. Same listener-thread caveats as " - "set_feature_settings.") - .with_string_param("shortName", - "Feature shortName as returned by list_features.") - .build(); - server->register_tool(tool, - [this](const mcp::json& params, const std::string& session_id) -> mcp::json { - RecordToolCall(session_id, "reset_feature_settings"); - const std::string shortName = params.value("shortName", std::string{}); - if (shortName.empty()) { - return ErrorResult("missing required parameter 'shortName'"); - } - auto* feature = Feature::FindFeatureByShortName(shortName); - if (!feature) { - return ErrorResult("feature not found or not loaded", - { { "shortName", shortName } }); - } - try { - feature->RestoreDefaultSettings(); - } catch (const std::exception& e) { - return ErrorResult("RestoreDefaultSettings threw", - { { "shortName", shortName }, { "detail", e.what() } }); - } - logger::info("Remote Control: reset_feature_settings({})", shortName); - return TextResult(mcp::json({ - { "shortName", shortName }, - { "reset", true }, - }) - .dump()); - }); -} - -void RemoteControl::RegisterSetFeatureSettingsTool() -{ - // Empty schema for the "settings" object means "any shape" — the actual - // schema is feature-specific and discoverable via get_feature_settings. - const auto tool = mcp::tool_builder("set_feature_settings") - .with_description( - "Replace a feature's settings with the supplied " - "JSON blob. Schema is feature-specific; use " - "get_feature_settings to fetch the current " - "shape, mutate the fields you care about, then " - "send the whole object back. Changes take " - "effect on the next frame.\n\n" - "CAVEAT: handler runs on the cpp-mcp listener " - "thread, NOT the render thread. Settings that " - "merely deserialize into member variables are " - "safe (the same pattern the ImGui menu uses on " - "the input thread). Settings whose LoadSettings " - "synchronously rebuilds GPU resources may " - "race the renderer — to be tightened with a " - "command queue in a follow-up commit.") - .with_string_param("shortName", - "Feature shortName as returned by list_features.") - .with_object_param("settings", - "Settings blob as returned by get_feature_settings.", - mcp::json::object()) - .build(); - server->register_tool(tool, - [this](const mcp::json& params, const std::string& session_id) -> mcp::json { - RecordToolCall(session_id, "set_feature_settings"); - const std::string shortName = params.value("shortName", std::string{}); - if (shortName.empty()) { - return ErrorResult("missing required parameter 'shortName'"); - } - if (!params.contains("settings") || !params["settings"].is_object()) { - return ErrorResult("missing required object parameter 'settings'"); - } - auto* feature = Feature::FindFeatureByShortName(shortName); - if (!feature) { - return ErrorResult("feature not found or not loaded", - { { "shortName", shortName } }); - } - // Round-trip through dump/parse to convert from cpp-mcp's - // ordered_json (params) to the feature's plain nlohmann::json. - ::json blob; - try { - blob = ::json::parse(params["settings"].dump()); - } catch (const std::exception& e) { - return ErrorResult("settings is not valid JSON", - { { "detail", e.what() } }); - } - try { - feature->LoadSettings(blob); - } catch (const std::exception& e) { - return ErrorResult("LoadSettings threw", - { { "shortName", shortName }, { "detail", e.what() } }); - } - logger::info("Remote Control: set_feature_settings({})", shortName); - return TextResult(mcp::json({ - { "shortName", shortName }, - { "applied", true }, - }) - .dump()); - }); -} - -void RemoteControl::RegisterGetFeatureSettingsTool() -{ - const auto tool = mcp::tool_builder("get_feature_settings") - .with_description( - "Return the current JSON settings blob for a " - "single feature. Use list_features to discover " - "shortNames. The exact schema is feature-specific " - "— it mirrors what Feature::SaveSettings emits to " - "the on-disk config and what set_feature_settings " - "expects back.") - .with_string_param("shortName", - "Feature shortName as returned by list_features.") - .build(); - server->register_tool(tool, - [this](const mcp::json& params, const std::string& session_id) -> mcp::json { - RecordToolCall(session_id, "get_feature_settings"); - const std::string shortName = params.value("shortName", std::string{}); - if (shortName.empty()) { - return ErrorResult("missing required parameter 'shortName'"); - } - auto* feature = Feature::FindFeatureByShortName(shortName); - if (!feature) { - return ErrorResult("feature not found or not loaded", - { { "shortName", shortName } }); - } - // SaveSettings() uses nlohmann::json (unordered map). Keep the - // intermediate value as plain json and re-emit as a string so - // we don't have to round-trip through mcp::json's ordered map. - ::json blob; - feature->SaveSettings(blob); - return TextResult(blob.dump()); - }); -} diff --git a/src/Features/RemoteControl.h b/src/Features/RemoteControl.h index 887335cd88..1ab0edcff5 100644 --- a/src/Features/RemoteControl.h +++ b/src/Features/RemoteControl.h @@ -94,12 +94,8 @@ class RemoteControl : public Feature bool IsRunning() const noexcept { return server != nullptr; } std::string BuildClientConfig() const; void RegisterTools(); - void RegisterGetStateTool(); - void RegisterListFeaturesTool(); - void RegisterGetFeatureSettingsTool(); - void RegisterSetFeatureSettingsTool(); - void RegisterResetFeatureSettingsTool(); - void RegisterToggleFeatureTool(); + void RegisterInspectTool(); + void RegisterFeatureTool(); void RegisterConsoleTool(); void RegisterCaptureTool(); void RegisterAbtestTool(); From 675e8a31d12e4bf4bac8cfd8d7a62f391994ce59 Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Wed, 20 May 2026 00:28:03 -0700 Subject: [PATCH 13/17] fix(remote-control): enforce loopback-only bind + clamp port Addresses CodeRabbit Major finding on PR #27: persisted bindAddress and port were applied verbatim, so a config edit could expose the control endpoint off-host (the surface advertises loopback-only). - Centralize validation in IsLoopbackAddress / NormalizeBindAddress / ClampPort helpers in an anonymous namespace. - Apply in LoadSettings so stale/malicious config is corrected at load with a warning. - Re-apply in StartServer immediately before the bind so UI edits or hot-reload can't slip a non-loopback host through. Co-Authored-By: Claude Opus 4.7 --- src/Features/RemoteControl.cpp | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index 24f64e5598..29ec9cc0a4 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -25,6 +25,29 @@ #include "mcp_server.h" #include "mcp_tool.h" +namespace +{ + // The control endpoint is intentionally loopback-only — exposing it off-host + // would let any networked client toggle features and dispatch captures. + bool IsLoopbackAddress(const std::string& host) + { + return host == "127.0.0.1" || host == "::1" || host == "localhost"; + } + + void NormalizeBindAddress(std::string& host) + { + if (!IsLoopbackAddress(host)) { + logger::warn("Remote Control: non-loopback bindAddress '{}' rejected; forcing 127.0.0.1", host); + host = "127.0.0.1"; + } + } + + int ClampPort(int port) + { + return std::clamp(port, 1024, 65535); + } +} + RemoteControl* RemoteControl::GetSingleton() { return &globals::features::remoteControl; @@ -53,8 +76,9 @@ void RemoteControl::Reset() void RemoteControl::LoadSettings(json& o_json) { settings.enabled = o_json.value("enabled", false); - settings.port = o_json.value("port", 8910); + settings.port = ClampPort(o_json.value("port", 8910)); settings.bindAddress = o_json.value("bindAddress", std::string("127.0.0.1")); + NormalizeBindAddress(settings.bindAddress); } void RemoteControl::SaveSettings(json& o_json) @@ -278,6 +302,11 @@ void RemoteControl::StartServer() lastError.clear(); try { + // Re-validate at bind time — settings may have been touched via the UI + // or hot-reload since LoadSettings ran. + NormalizeBindAddress(settings.bindAddress); + settings.port = ClampPort(settings.port); + mcp::server::configuration cfg; cfg.host = settings.bindAddress; cfg.port = settings.port; From 4684e2c14d9f4c3b96069341453bf3b601c90b1d Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Wed, 20 May 2026 01:40:34 -0700 Subject: [PATCH 14/17] fix(remote-control): clarify loopback-only UI + guard cmake patch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot findings on PR #27: - DrawSettings panel said the server was bound to 127.0.0.1 "unless explicitly changed", which is no longer true now that NormalizeBindAddress forces non-loopback values back. Reword to "loopback-only — any non-loopback bind address is rejected at load and bind time" so the UI matches the actual policy. - cmake/cpp-mcp.cmake's mcp_message.h patch silently no-op'd if the expected `#include "json.hpp"` ever disappeared upstream, letting the nlohmann_json ABI mismatch silently regress. Verify the substring exists before the REPLACE and emit a FATAL_ERROR pointing at the rationale in the header comment. Co-Authored-By: Claude Opus 4.7 --- cmake/cpp-mcp.cmake | 11 +++++++++++ src/Features/RemoteControl.cpp | 5 +++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/cmake/cpp-mcp.cmake b/cmake/cpp-mcp.cmake index b2b9b393c9..e53ed17a72 100644 --- a/cmake/cpp-mcp.cmake +++ b/cmake/cpp-mcp.cmake @@ -46,6 +46,17 @@ foreach(_hdr IN LISTS _cpp_mcp_headers) get_filename_component(_name "${_hdr}" NAME) file(READ "${_hdr}" _content) if(_name STREQUAL "mcp_message.h") + # Fail fast if the expected include vanishes upstream — otherwise the + # ABI mismatch would silently come back and only surface as an LNK2001 + # well into the link step. + string(FIND "${_content}" "#include \"json.hpp\"" _json_inc_pos) + if(_json_inc_pos EQUAL -1) + message(FATAL_ERROR + "cpp-mcp: expected `#include \"json.hpp\"` in mcp_message.h " + "but did not find it. Upstream may have changed the include; " + "review cmake/cpp-mcp.cmake and adjust the patch (see header " + "comment for the ABI-alignment rationale).") + endif() string(REPLACE "#include \"json.hpp\"" "#include " diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index 29ec9cc0a4..79037f35fb 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -98,8 +98,9 @@ void RemoteControl::DrawSettings() ImGui::TextWrapped( "Exposes Community Shaders over Model Context Protocol (MCP) so AI " "assistants such as Claude Code can drive A/B testing, toggle " - "features, and trigger captures. Off by default. Bound to 127.0.0.1 " - "unless explicitly changed."); + "features, and trigger captures. Off by default. The endpoint is " + "loopback-only — any non-loopback bind address is rejected at load " + "and bind time."); ImGui::Spacing(); const bool wasEnabled = settings.enabled; From a8a2d963cc384685423b02d393be671139b84370 Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Wed, 20 May 2026 22:19:11 -0700 Subject: [PATCH 15/17] fix(remote-control): race-free frameCount + IPv6 URL + tool docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three Copilot findings on PR #27: - State::frameCount is non-atomic and incremented on the render thread, but RemoteControl reads it from the MCP listener thread (inspect kind=state, capture queue, console tick). That's a data race / UB. Add State::frameCountAtomic, a thread-safe mirror written immediately after the existing `frameCount++` on the render thread; route the off-thread reads in RemoteControl through it. The hot path (ScreenSpaceGI, Streamline, Upscaling) keeps using the non-atomic field — no behavior change there. - BuildClientConfig formatted the URL as `http://{host}:{port}/mcp`, which is invalid for the IPv6 loopback "::1" because the URL authority requires bracketed IPv6 literals (RFC 3986 §3.2.2). Detect ':' in bindAddress and wrap it. - The console tool description told agents to "poll get_state", but the tool is `inspect(kind='state')`. Update the description to match what's actually registered. Co-Authored-By: Claude Opus 4.7 --- src/Features/RemoteControl.cpp | 16 ++++++++++------ src/State.cpp | 2 ++ src/State.h | 4 ++++ 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index 79037f35fb..caab2cb654 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -283,13 +283,17 @@ std::string RemoteControl::BuildClientConfig() const { // Streamable HTTP transport per the MCP 2025-03-26 spec. Same shape works // for Claude Code, Cursor, Continue, and other MCP hosts. + // IPv6 literals must be bracketed in a URL authority (RFC 3986 §3.2.2), + // so the IPv6 loopback "::1" becomes "[::1]". IPv4 / hostnames pass + // through verbatim. + const std::string hostInUrl = (settings.bindAddress.find(':') != std::string::npos) ? "[" + settings.bindAddress + "]" : settings.bindAddress; const json cfg = { { "mcpServers", { { "community-shaders", { { "type", "http" }, { "url", std::format("http://{}:{}/mcp", - settings.bindAddress, settings.port) }, + hostInUrl, settings.port) }, } } } } }; return cfg.dump(4); @@ -429,7 +433,7 @@ void RemoteControl::RegisterTools() // Helper used by both inspect(kind="state") and (potentially) future tools. static mcp::json EngineStateBlob() { - const uint frames = globals::state ? globals::state->frameCount : 0; + const uint frames = globals::state ? globals::state->frameCountAtomic.load(std::memory_order_relaxed) : 0u; const bool vr = REL::Module::IsVR(); return mcp::json({ { "plugin", "CommunityShaders" }, @@ -824,7 +828,7 @@ void RemoteControl::RegisterCaptureTool() if (kind.empty()) { return ErrorResult("missing required parameter 'kind'"); } - const uint enqueuedFrame = globals::state ? globals::state->frameCount : 0; + const uint enqueuedFrame = globals::state ? globals::state->frameCountAtomic.load(std::memory_order_relaxed) : 0u; if (kind == "renderdoc") { auto* renderDoc = &globals::features::renderDoc; @@ -900,8 +904,8 @@ void RemoteControl::RegisterConsoleTool() "commands are silent (tcl, tfc, tg, tm, tlb…), so " "scraping console output is unreliable and " "intentionally NOT exposed.\n\n" - "To verify a state change, poll get_state until " - "frame_count > enqueued_at_frame (at least one tick " + "To verify a state change, poll inspect(kind='state') " + "until frame_count > enqueued_at_frame (at least one tick " "elapsed), then observe via side channels: tracy " "captures for perf-affecting changes, " "capture(kind='renderdoc'|'screenshot') for visual " @@ -931,7 +935,7 @@ void RemoteControl::RegisterConsoleTool() if (!task) { return ErrorResult("SKSE TaskInterface unavailable"); } - const uint enqueuedFrame = globals::state ? globals::state->frameCount : 0; + const uint enqueuedFrame = globals::state ? globals::state->frameCountAtomic.load(std::memory_order_relaxed) : 0u; // Capture by value so the string outlives this lambda's scope. task->AddTask([command]() { RE::Console::ExecuteCommand(command.c_str()); diff --git a/src/State.cpp b/src/State.cpp index 681a6996cc..b9ebfdd05f 100644 --- a/src/State.cpp +++ b/src/State.cpp @@ -186,6 +186,8 @@ void State::Reset() lastVertexDescriptor = 0; std::memset(&permutationDataPrevious, 0xFF, sizeof(PermutationCB)); frameCount++; + // Publish for off-thread readers (e.g. the MCP listener thread). + frameCountAtomic.store(frameCount, std::memory_order_relaxed); if (auto* imageSpaceManager = RE::ImageSpaceManager::GetSingleton()) { GET_INSTANCE_MEMBER(BSImagespaceShaderApplyReflections, imageSpaceManager); diff --git a/src/State.h b/src/State.h index fdddd770fa..5d0987e0c2 100644 --- a/src/State.h +++ b/src/State.h @@ -268,6 +268,10 @@ class State Util::FrameChecker frameChecker; uint frameCount = 0; + // Thread-safe mirror of frameCount maintained by the render thread. + // Off-thread readers (MCP listener, future telemetry) must read this + // instead of touching frameCount directly to avoid a data race. + std::atomic frameCountAtomic{ 0 }; // Skyrim constants float2 screenSize = {}; From f969628a9910b696a801ce276b82ea6497c52f35 Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Thu, 21 May 2026 22:21:19 -0700 Subject: [PATCH 16/17] fix(remote-control): add explicit STL headers; CONFIGURE_DEPENDS on header GLOB Address remaining Copilot review comments on PR #27: - RemoteControl.cpp uses std::clamp/std::sort (algorithm) and std::vector (vector) directly. Include the headers explicitly instead of relying on transitive includes from imgui/cpp-mcp, which could break on PCH/toolchain changes. - cmake/cpp-mcp.cmake's file(GLOB) for cpp-mcp headers now uses CONFIGURE_DEPENDS so adding/removing a header in the submodule triggers a CMake reconfigure on the next build. Otherwise the patched header mirror could go stale across submodule updates with no obvious failure. --- cmake/cpp-mcp.cmake | 2 +- src/Features/RemoteControl.cpp | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/cmake/cpp-mcp.cmake b/cmake/cpp-mcp.cmake index e53ed17a72..e218957891 100644 --- a/cmake/cpp-mcp.cmake +++ b/cmake/cpp-mcp.cmake @@ -41,7 +41,7 @@ find_package(nlohmann_json CONFIG REQUIRED) # All other cpp-mcp headers are copied verbatim into the patched mirror # so they live next to the patched header and find each other. file(MAKE_DIRECTORY "${CPP_MCP_PATCHED_INC}") -file(GLOB _cpp_mcp_headers "${CPP_MCP_DIR}/include/*.h") +file(GLOB _cpp_mcp_headers CONFIGURE_DEPENDS "${CPP_MCP_DIR}/include/*.h") foreach(_hdr IN LISTS _cpp_mcp_headers) get_filename_component(_name "${_hdr}" NAME) file(READ "${_hdr}" _content) diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index caab2cb654..7ee901b981 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -17,8 +17,10 @@ #include #include +#include #include #include +#include // cpp-mcp headers. Kept inside the .cpp only so the vendored httplib/json // in extern/cpp-mcp/common don't leak into other translation units. From 65b75457b3479b41f1ed72a07c84062989020b68 Mon Sep 17 00:00:00 2001 From: Alan Tse Date: Thu, 21 May 2026 22:33:24 -0700 Subject: [PATCH 17/17] fix(remote-control): marshal mutations to main thread; tighten loopback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address follow-up review comments on PR #27: Thread safety — route Feature/ABTesting mutations onto the main thread via SKSE::GetTaskInterface(), mirroring the existing console tool. Direct writes from the MCP listener thread race with the render loop reading the same state: - feature(action='toggle') now AddTask()s the Feature::loaded write; response includes 'previous', 'requested', and 'enqueued_at_frame'. - feature(action='set') and feature(action='reset') now AddTask() the LoadSettings / RestoreDefaultSettings call. LoadSettings can touch UI/render-thread-visible state inside features; doing it off-thread was a real race. - abtest(action='start'/'stop'/'clear') now AddTask() the Enable/Disable/Clear calls. ABTestingManager::Enable swaps configs via State::Load and Menu::Load, which expect main-thread access. - abtest(action='status') remains synchronous — it only reads. Tool return contract changes: mutations now respond with { queued: true, enqueued_at_frame: N } instead of synchronous applied/reset flags. Callers wanting confirmation should re-query with the corresponding read action. Security — IsLoopbackAddress() no longer treats "localhost" as loopback. On Windows the hosts file or a hijacked resolver can map "localhost" to a non-loopback address, which would silently expose the control endpoint off-host. Only literal 127.0.0.1 / ::1 pass. Docs — capture tool hints now point at feature(action='list') instead of the non-existent list_features tool. The RemoteControl.h forward-declare comment no longer references a "tool-tracking layer" the class never had; it now explains that mcp::json (ordered_json alias) is what blocks moving registration into the header. --- src/Features/RemoteControl.cpp | 132 +++++++++++++++++++++++++-------- src/Features/RemoteControl.h | 7 +- 2 files changed, 105 insertions(+), 34 deletions(-) diff --git a/src/Features/RemoteControl.cpp b/src/Features/RemoteControl.cpp index 7ee901b981..bda02d65e2 100644 --- a/src/Features/RemoteControl.cpp +++ b/src/Features/RemoteControl.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -31,9 +32,12 @@ namespace { // The control endpoint is intentionally loopback-only — exposing it off-host // would let any networked client toggle features and dispatch captures. + // Only accept literal loopback IPs: on Windows the hosts file (or a + // hijacked resolver) can map "localhost" to a routable address, which would + // silently break the loopback-only contract. bool IsLoopbackAddress(const std::string& host) { - return host == "127.0.0.1" || host == "::1" || host == "localhost"; + return host == "127.0.0.1" || host == "::1"; } void NormalizeBindAddress(std::string& host) @@ -607,15 +611,29 @@ void RemoteControl::RegisterFeatureTool() return ErrorResult("feature not found", { { "shortName", shortName } }); } + // Marshal the write onto the main/render thread. Feature::loaded + // is read every frame by Feature::ForEachLoadedFeature without + // synchronization, so writing it directly from the MCP listener + // thread is a data race. AddTask runs the closure on the next + // tick. + auto* task = SKSE::GetTaskInterface(); + if (!task) { + return ErrorResult("SKSE TaskInterface unavailable"); + } const bool previous = target->loaded; - target->loaded = desired; - logger::info("Remote Control: feature(toggle, {}, {}) (was {})", - shortName, desired, previous); + const uint enqueuedFrame = globals::state ? globals::state->frameCountAtomic.load(std::memory_order_relaxed) : 0u; + task->AddTask([target, desired, shortName]() { + target->loaded = desired; + logger::info("Remote Control: feature(toggle, {}, {}) applied", + shortName, desired); + }); return TextResult(mcp::json({ { "action", "toggle" }, { "shortName", shortName }, { "previous", previous }, - { "current", desired }, + { "requested", desired }, + { "queued", true }, + { "enqueued_at_frame", enqueuedFrame }, }) .dump()); } @@ -645,32 +663,54 @@ void RemoteControl::RegisterFeatureTool() return ErrorResult("settings is not valid JSON", { { "detail", e.what() } }); } - try { - feature->LoadSettings(blob); - } catch (const std::exception& e) { - return ErrorResult("LoadSettings threw", - { { "shortName", shortName }, { "detail", e.what() } }); + // Marshal LoadSettings onto the main thread. Many features + // mutate UI/render-thread-visible state inside LoadSettings + // (palettes, cached textures, settings JSON read elsewhere), + // so calling it from the MCP listener thread is racy. + auto* task = SKSE::GetTaskInterface(); + if (!task) { + return ErrorResult("SKSE TaskInterface unavailable"); } - logger::info("Remote Control: feature(set, {})", shortName); + const uint enqueuedFrame = globals::state ? globals::state->frameCountAtomic.load(std::memory_order_relaxed) : 0u; + task->AddTask([feature, blob, shortName]() mutable { + try { + feature->LoadSettings(blob); + logger::info("Remote Control: feature(set, {}) applied", shortName); + } catch (const std::exception& e) { + logger::error("Remote Control: feature(set, {}) LoadSettings threw: {}", + shortName, e.what()); + } + }); return TextResult(mcp::json({ { "action", "set" }, { "shortName", shortName }, - { "applied", true }, + { "queued", true }, + { "enqueued_at_frame", enqueuedFrame }, }) .dump()); } if (action == "reset") { - try { - feature->RestoreDefaultSettings(); - } catch (const std::exception& e) { - return ErrorResult("RestoreDefaultSettings threw", - { { "shortName", shortName }, { "detail", e.what() } }); + // Same marshaling rationale as feature(set): RestoreDefaultSettings + // touches state that the render/UI threads read concurrently. + auto* task = SKSE::GetTaskInterface(); + if (!task) { + return ErrorResult("SKSE TaskInterface unavailable"); } - logger::info("Remote Control: feature(reset, {})", shortName); + const uint enqueuedFrame = globals::state ? globals::state->frameCountAtomic.load(std::memory_order_relaxed) : 0u; + task->AddTask([feature, shortName]() { + try { + feature->RestoreDefaultSettings(); + logger::info("Remote Control: feature(reset, {}) applied", shortName); + } catch (const std::exception& e) { + logger::error("Remote Control: feature(reset, {}) RestoreDefaultSettings threw: {}", + shortName, e.what()); + } + }); return TextResult(mcp::json({ { "action", "reset" }, { "shortName", shortName }, - { "reset", true }, + { "queued", true }, + { "enqueued_at_frame", enqueuedFrame }, }) .dump()); } @@ -741,28 +781,58 @@ void RemoteControl::RegisterAbtestTool() }; if (action == "status") { + // Read-only — safe from the listener thread; the only state we + // touch is the manager's atomic-ish status getters. return TextResult(statusBlob().dump()); } + + // Lifecycle actions (start/stop/clear) marshal onto the main thread: + // Enable/Disable swap configs via State::Load → JSON, and Menu::Load + // touches settings the menu/render thread also reads. Doing that + // from the listener thread is a race against the next frame's UI. + auto* task = SKSE::GetTaskInterface(); + if (!task) { + return ErrorResult("SKSE TaskInterface unavailable"); + } + const uint enqueuedFrame = globals::state ? globals::state->frameCountAtomic.load(std::memory_order_relaxed) : 0u; + const auto queuedResult = [&](const std::string& act) { + auto blob = statusBlob(); + blob["action"] = act; + blob["queued"] = true; + blob["enqueued_at_frame"] = enqueuedFrame; + return TextResult(blob.dump()); + }; + if (action == "start") { + std::optional interval; if (params.contains("interval") && params["interval"].is_number()) { const auto secs = params["interval"].get(); if (secs > 0) { - mgr->SetTestInterval(static_cast(secs)); + interval = static_cast(secs); } } - mgr->Enable(); - logger::info("Remote Control: abtest(start)"); - return TextResult(statusBlob().dump()); + task->AddTask([mgr, interval]() { + if (interval) { + mgr->SetTestInterval(*interval); + } + mgr->Enable(); + logger::info("Remote Control: abtest(start) applied"); + }); + return queuedResult("start"); } if (action == "stop") { - mgr->Disable(); - logger::info("Remote Control: abtest(stop)"); - return TextResult(statusBlob().dump()); + task->AddTask([mgr]() { + mgr->Disable(); + logger::info("Remote Control: abtest(stop) applied"); + }); + return queuedResult("stop"); } if (action == "clear") { - mgr->ClearCachedSnapshots(); - logger::info("Remote Control: abtest(clear)"); - return TextResult(statusBlob().dump()); + task->AddTask([mgr]() { + mgr->ClearCachedSnapshots(); + logger::info("Remote Control: abtest(clear) applied"); + }); + return queuedResult("clear"); } if (action == "diff") { mcp::json entries = mcp::json::array(); @@ -803,7 +873,7 @@ void RemoteControl::RegisterCaptureTool() "the in-application API. Honors the `frames` " "parameter (default 1, max 120). RenderDoc must " "be attached or the in-app DLL loaded; check " - "list_features for RenderDoc loaded=true. Output " + "feature(action='list') for RenderDoc loaded=true. Output " "lands in RenderDoc's configured captures dir.\n" " screenshot — Lossless screenshot via the " "Screenshot feature's non-blocking capture path. " @@ -836,7 +906,7 @@ void RemoteControl::RegisterCaptureTool() auto* renderDoc = &globals::features::renderDoc; if (!renderDoc->loaded) { return ErrorResult("RenderDoc feature is not loaded", - { { "hint", "list_features shows RenderDoc.loaded" } }); + { { "hint", "feature(action='list') shows RenderDoc.loaded" } }); } if (!renderDoc->IsAvailable()) { return ErrorResult( diff --git a/src/Features/RemoteControl.h b/src/Features/RemoteControl.h index 1ab0edcff5..0c88ad8107 100644 --- a/src/Features/RemoteControl.h +++ b/src/Features/RemoteControl.h @@ -19,9 +19,10 @@ namespace mcp class server; struct tool; // cpp-mcp's tool_handler is std::function - // but json there is ordered_json, which we can't forward-declare cleanly. - // We type-erase via std::function in the tool-tracking layer and - // keep the real signature local to the .cpp file. + // where `json` is an alias for ordered_json — that can't be forward-declared + // cleanly without dragging the full vendored nlohmann/json header into this + // public header. Tool registration therefore stays in the .cpp where the + // real signature is in scope; only opaque pointers are exposed here. } class RemoteControl : public Feature