feat(mcp): integrate devbench

[alandtse]

Registers Open Shaders' tools into the standalone devbench MCP/REST test bench over its cross-plugin C-ABI. Purely additive and gated.

What's here

• DevBenchBridge — registers an openshaders.feature tool (list / toggle) into devbench via DevBenchAPI (vendored devbench-api vcpkg overlay port → DevBench::API). Namespaced to avoid collisions in devbench's shared registry. All exceptions are contained at the C-ABI boundary.
• Build wiring — gated behind option(DEVBENCH_BRIDGE ON): ships by default (CI exercises it), -DDEVBENCH_BRIDGE=OFF drops the dependency and compiles DevBenchBridge.cpp to an empty Install(). The overlay port self-resolves under cmake/ports/devbench-api (portfile fetches the MIT API source — no source copy).

Validation

Confirmed live: openshaders.feature appears in devbench's /api/tools and is callable over both MCP and REST — a toggle round-trip flipped a CS feature's state in-game.

Scope

Bridge only. This does not touch the embedded RemoteControl server or add console capture (devbench owns capture). Further tools (feature set/reset, shader-cache, capture, abtest) and shader-recompile events register the same way as they're added.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added DevBench host integration with connection status monitoring in plugin settings
  • Shader compilation events now reported to external DevBench host for monitoring

• Refactor

  • Replaced in-process remote control server with external DevBench bridge integration
  • Simplified remote feature management by delegating to external DevBench host

[coderabbitai]

Note

Reviews paused

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

Use the following commands to manage reviews:

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

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR replaces the in-process MCP server architecture with a DevBench host bridge. It introduces a new vcpkg port for devbench-api, integrates it conditionally via CMake options and features, implements a C-ABI-safe bridge registering openshaders tools into DevBench, refactors RemoteControl to a devbench status panel, emits shader events to DevBench, and removes the cpp-mcp submodule.

Changes

DevBench Bridge & VCpkg Port Integration

Layer / File(s)	Summary
DevBench vcpkg port definition cmake/ports/devbench-api/vcpkg.json, cmake/ports/devbench-api/portfile.cmake, cmake/ports/devbench-api/devbench-api-config.cmake, cmake/ports/devbench-api/README.md	New vcpkg manifest and portfile pin alandtse/devbench, download and install DevBenchAPI.h/.cpp and CMake config, define DevBench::API IMPORTED target publishing include and source paths, and document vcpkg consumption and MIT license.
CMake build system and preset wiring CMakeLists.txt, vcpkg.json, CMakeUserPresets.json.template	Add DEVBENCH_BRIDGE CMake option (default ON); append devbench-bridge vcpkg feature when enabled; conditionally find_package(devbench-api CONFIG), link DevBench::API, and define DEVBENCH_BRIDGE_ENABLED; enable the option in the build preset.
RemoteControl public API surface reduction src/Features/RemoteControl.h	Remove MCP-related includes and forward declarations; drop feature metadata overrides; remove Load(), Reset(), settings persistence methods, and Settings struct; rewrite GetFeatureSummary() to describe devbench bridge; convert ctor/dtor to inline defaults.
DevBenchBridge core infrastructure src/Features/RemoteControl/DevBenchBridge.h, src/Features/RemoteControl/DevBenchBridge.cpp	Declare public Install() function; implement RunHandler() JSON dispatcher with single response and exception safety; implement RunReadOnMainThread() marshaling reads to SKSE task queue with 5-second timeout and error responses.
openshaders.feature tool handler src/Features/RemoteControl/DevBenchBridge.cpp	Implement feature list/get/set/reset/toggle actions with computed restartFields from boot-vs-live settings comparison, main-thread queued mutations, openshaders.feature.changed events, VR-incompatibility rejection, and queued metadata responses.
openshaders.inspect, shadercache, capture, settings handlers src/Features/RemoteControl/DevBenchBridge.cpp	Implement inspect (state/frame-stamp/VR-flag and shadercache counters), shadercache (clear/deleteDisk), capture (renderdoc/screenshot), and settings (save/load/reset) with parameter validation, main-thread marshalling, exception handling, and JSON responses.
DevBenchBridge::Install() registration src/Features/RemoteControl/DevBenchBridge.cpp	Implement enabled-build registration of feature/inspect/shadercache/capture/settings tools with stable JSON descriptors and handler entrypoints, and disabled-build no-op when DEVBENCH_BRIDGE_ENABLED not defined.
RemoteControl UI refactor and startup integration src/Features/RemoteControl.cpp	Add ReadDevBenchPort() JSON helper; call DevBenchBridge::Install() from DataLoaded(); replace MCP server UI with devbench bridge status panel showing host presence, bound port (refreshed periodically), tool namespaces, install guidance, and bridge-disabled messaging.
ShaderCache DevBench event emission src/ShaderCache.cpp	Add compile-time guarded DevBenchAPI.h include; emit openshaders.shaderRecompiled events with task counts and duration in CompilationSet::Complete when interface available.
Git submodule configuration changes .gitmodules	Remove extern/cpp-mcp submodule entry; add branch = optiscaler-build to extern/FidelityFX-SDK submodule.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

• alandtse/open-shaders#67: This PR implements the DevBench bridge functionality (DevBenchBridge::Install, openshaders.* tool handlers, shader-recompile events, inspect/capture/settings handlers) fulfilling the architecture and requirements described in the issue.

Possibly related PRs

• alandtse/open-shaders#27: This PR removes the MCP-based RemoteControl feature (and its supporting cpp-mcp CMake build integration) that was added in #27, replacing it with the DevBench bridge implementation.
• alandtse/open-shaders#62: Both PRs modify RemoteControl.cpp shadercache observability—this PR replaces the in-process MCP server's inspect(kind="shadercache") with the DevBench bridge's equivalent inspection handler.

Poem

🐰 Thumpthumps of code refactored bright,
From MCP's maze to DevBench's light,
New vcpkg port and bridge now stand,
With tools registered, safe and planned,
Shaders emit their stories true—
The integration's complete and new! 🎨✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title 'feat(mcp): integrate devbench' is vague and misleading—it references 'mcp' but the PR actually replaces the in-process MCP server with a devbench bridge, making the title's use of 'mcp' confusing.	Revise the title to accurately reflect the primary change, such as 'feat: integrate devbench bridge for remote tool registration' or 'feat: replace MCP server with devbench bridge integration'.
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.32% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/devbench-bridge

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

No actionable suggestions for changed features.

[Copilot]

Pull request overview

This PR adds devbench integration for Community Shaders tooling and extends the existing RemoteControl console tool with gated console-output capture.

Changes:

• Adds a DevBenchBridge that registers a basic feature tool with devbench through the devbench C-ABI.
• Adds ConsoleLogCapture and wires it into the RemoteControl console tool for opt-in captured output reads.
• Adds a devbench-api vcpkg overlay port and build-system wiring.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
vcpkg.json	Adds devbench-api as a dependency.
src/XSEPlugin.cpp	Installs the devbench bridge during kDataLoaded.
src/Features/RemoteControl.cpp	Installs console capture and adds console read/capture parameters.
src/DevBenchBridge.h	Declares the devbench bridge entry point.
src/DevBenchBridge.cpp	Implements devbench feature tool registration and handling.
src/ConsoleLogCapture.h	Declares gated console-log capture APIs.
src/ConsoleLogCapture.cpp	Implements the ConsoleLog::VPrint detour and capture buffer.
CMakeLists.txt	Finds and links DevBench::API and defines bridge enablement.
cmake/ports/devbench-api/vcpkg.json	Defines the overlay port metadata.
cmake/ports/devbench-api/README.md	Documents consuming the devbench API overlay port.
cmake/ports/devbench-api/portfile.cmake	Fetches and installs the devbench API source/header/license.
cmake/ports/devbench-api/devbench-api-config.cmake	Defines the imported DevBench::API target.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmake/ports/devbench-api/README.md`:
- Around line 36-48: The README's "Interim: local overlay" section incorrectly
says the portfile's REF/SHA512 are placeholders; instead, update that text to
state that portfile.cmake currently pins a specific REF and SHA512 (referencing
the REF/SHA512 entries in portfile.cmake) and clarify the proper consumer
options: either use the pinned port as-is, override via an overlay by pointing
"overlay-ports" to a local checkout, or update the portfile's REF/SHA512 when
publishing a new GitHub release; remove the line suggesting consumers must fill
in values that are already populated.

In `@src/DevBenchBridge.cpp`:
- Around line 25-66: FeatureToolHandler currently risks letting C++ exceptions
escape the C-ABI boundary because only json::parse is guarded; wrap the entire
body of FeatureToolHandler (everything that uses args, calls args.value,
Feature::GetFeatureList, Feature::FindFeatureByShortName,
SKSE::GetTaskInterface, etc.) in a try/catch(...) block, and in the catch
construct a JSON error payload (e.g. { "error":"internal exception" , "details":
"<optional safe message>" }) and call a_write(a_sink, dumped.c_str()) to return
it; ensure the catch does not rethrow and that a_write is always called so no
exceptions propagate across the C ABI.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a1673e95-0733-4553-8188-bf93250808f1

📥 Commits

Reviewing files that changed from the base of the PR and between 05d10c3 and 1057866.

📒 Files selected for processing (12)

• CMakeLists.txt
• cmake/ports/devbench-api/README.md
• cmake/ports/devbench-api/devbench-api-config.cmake
• cmake/ports/devbench-api/portfile.cmake
• cmake/ports/devbench-api/vcpkg.json
• src/ConsoleLogCapture.cpp
• src/ConsoleLogCapture.h
• src/DevBenchBridge.cpp
• src/DevBenchBridge.h
• src/Features/RemoteControl.cpp
• src/XSEPlugin.cpp
• vcpkg.json

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

[coderabbitai]

🧹 Nitpick comments (1)

src/DevBenchBridge.cpp (1)

19-72: 💤 Low value

Consider documenting the threading model and loaded field semantics.

The toggle path directly sets target->loaded = desired on the main thread. This works because SKSE's TaskInterface queues the lambda to the main thread where game/feature state is safely accessed.

If loaded state changes ever require additional setup/teardown (e.g., shader recompilation, resource allocation), this direct assignment may need to invoke a method like Feature::SetEnabled() instead. Current implementation appears correct for hot-toggle scenarios.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/DevBenchBridge.cpp` around lines 19 - 72, The toggle path in
BuildFeatureResult directly mutates Feature::loaded on the main thread via
SKSE::GetTaskInterface()->AddTask; document the threading model and semantics of
the loaded field (that it must only be touched on the main thread) and, if
enabling/disabling requires more than a plain flag flip (teardown/setup, shader
recompiles, resource allocation), replace direct assignment target->loaded =
desired with a safe API such as Feature::SetEnabled(bool) (or implement
SetEnabled) that runs required work on the main thread and emits the same
openshaders.feature.changed event via
DevBenchAPI::GetDevBenchInterface001()->EmitEvent inside the task lambda; also
add a brief comment in BuildFeatureResult near the task->AddTask call describing
this threading contract.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/DevBenchBridge.cpp`:
- Around line 19-72: The toggle path in BuildFeatureResult directly mutates
Feature::loaded on the main thread via SKSE::GetTaskInterface()->AddTask;
document the threading model and semantics of the loaded field (that it must
only be touched on the main thread) and, if enabling/disabling requires more
than a plain flag flip (teardown/setup, shader recompiles, resource allocation),
replace direct assignment target->loaded = desired with a safe API such as
Feature::SetEnabled(bool) (or implement SetEnabled) that runs required work on
the main thread and emits the same openshaders.feature.changed event via
DevBenchAPI::GetDevBenchInterface001()->EmitEvent inside the task lambda; also
add a brief comment in BuildFeatureResult near the task->AddTask call describing
this threading contract.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 01f62260-094b-4fec-a998-7aed453ca076

📥 Commits

Reviewing files that changed from the base of the PR and between 1057866 and d7aa0a9.

📒 Files selected for processing (7)

• CMakeLists.txt
• CMakeUserPresets.json.template
• cmake/ports/devbench-api/README.md
• src/DevBenchBridge.cpp
• src/DevBenchBridge.h
• src/XSEPlugin.cpp
• vcpkg.json

✅ Files skipped from review due to trivial changes (2)

• CMakeUserPresets.json.template
• cmake/ports/devbench-api/README.md

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

• src/XSEPlugin.cpp
• src/DevBenchBridge.h

[alandtse]

Consider throughout whether we need to mention Open Shaders or can leave it out of the text/comments. Also check repo standards re comments and code churn.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[coderabbitai]

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)

src/Features/RemoteControl/DevBenchBridge.cpp (1)

82-87: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Catch non-std::exception failures in the main-thread trampoline.

If read() throws anything outside std::exception, the exception escapes on the game thread and the promise never gets a value, so a handled bridge request degrades into a crash/timeout path.

Proposed fix

 		task->AddTask([prom, read = std::move(a_read)]() {
 			try {
 				prom->set_value(read());
 			} catch (const std::exception& e) {
 				prom->set_value(json{ { "error", "read threw on main thread" }, { "detail", e.what() } });
+			} catch (...) {
+				prom->set_value(json{ { "error", "read threw on main thread" }, { "detail", "unknown error" } });
 			}
 		});

As per coding guidelines, "Include proper resource management and graceful degradation in error handling".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Features/RemoteControl/DevBenchBridge.cpp` around lines 82 - 87, The
trampoline lambda passed to task->AddTask currently only catches std::exception,
so non-std exceptions thrown by read() escape and leave prom unset; add a
catch(...) handler after the existing catch(const std::exception& e) in the
lambda (the closure that captures prom and read) that calls prom->set_value(...)
with a json object indicating a non-std exception (e.g., { "error", "read threw
non-std exception" } and optionally a generic "detail" string), ensuring the
promise is always fulfilled even for non-std throws.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/Features/RemoteControl/DevBenchBridge.cpp`:
- Around line 236-243: The request handler currently queues set/reset mutations
with task->AddTask and only looks up the target via
Feature::FindFeatureByShortName inside that fire-and-forget task, causing
callers to get { "queued": true } even when shortName is invalid or the feature
is unloaded; change the flow to preflight the lookup on the main thread (call
Feature::FindFeatureByShortName before calling task->AddTask) and return an
error JSON when the lookup fails, or alternatively invoke a synchronous helper
that performs the mutation on the main thread and returns success/error JSON
(use the same LoadSettings/apply logic inside that helper) so that set/reset
only acknowledge queued=true when the target feature was validated successfully.
- Around line 369-383: The current use of RunReadOnMainThread around
globals::features::renderDoc allows the helper to time out while the lambda is
still pending, so TriggerCapture/TriggerMultiFrameCapture may run after a
timeout; change this to a pure queued/write path (replace the
RunReadOnMainThread call with the project's queued/write helper) or modify the
helper call so the lambda is only executed when the main thread confirmed
execution will occur (i.e., no timeout). Concretely, ensure the code that
invokes renderDoc->TriggerCapture and renderDoc->TriggerMultiFrameCapture runs
only via the queued/write-on-main-thread API (so the returned status matches
actual execution) and apply the same change to the similar handler that starts
at lines 387-394.

---

Outside diff comments:
In `@src/Features/RemoteControl/DevBenchBridge.cpp`:
- Around line 82-87: The trampoline lambda passed to task->AddTask currently
only catches std::exception, so non-std exceptions thrown by read() escape and
leave prom unset; add a catch(...) handler after the existing catch(const
std::exception& e) in the lambda (the closure that captures prom and read) that
calls prom->set_value(...) with a json object indicating a non-std exception
(e.g., { "error", "read threw non-std exception" } and optionally a generic
"detail" string), ensuring the promise is always fulfilled even for non-std
throws.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a43979d4-5b2d-4300-9ea0-e6428f6f00b8

📥 Commits

Reviewing files that changed from the base of the PR and between 68e6502 and 4da6fc8.

📒 Files selected for processing (8)

• CMakeLists.txt
• cmake/ports/devbench-api/README.md
• cmake/ports/devbench-api/portfile.cmake
• src/Features/RemoteControl.cpp
• src/Features/RemoteControl.h
• src/Features/RemoteControl/DevBenchBridge.cpp
• src/Features/RemoteControl/DevBenchBridge.h
• vcpkg.json

💤 Files with no reviewable changes (1)

• src/Features/RemoteControl/DevBenchBridge.h

✅ Files skipped from review due to trivial changes (1)

• cmake/ports/devbench-api/README.md

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

• vcpkg.json
• cmake/ports/devbench-api/portfile.cmake
• CMakeLists.txt

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.