From fd49602adba72c604e0942ace42cc5f3a3d023b5 Mon Sep 17 00:00:00 2001 From: Joe Date: Sun, 1 Mar 2026 09:41:20 -0600 Subject: [PATCH] Implement get_script command for improved script retrieval in MCP - Introduced a new `get_script` command to allow agents to read script source code directly from Roblox Studio, enhancing conflict resolution during sync operations. - Updated the MCP stream handling to support the new command, including error handling for missing or invalid parameters. - Enhanced the App component to manage the `get_script` command and its response, ensuring proper integration with the existing MCP functionality. - Updated the TODO list to reflect the addition of the `get_script` command and its associated documentation requirements. These changes aim to improve the overall functionality and user experience of the MCP integration, particularly in script management. --- .../mcp_get_script_endpoint_4c4e80af.plan.md | 269 +++++++ ...sync_endpoint_enhancement_20ff55ce.plan.md | 379 ++++++++++ TODO.txt | 5 +- plugin/src/App/init.lua | 89 +++ plugin/src/McpStream.lua | 28 + src/web/api.rs | 220 +++++- src/web/mcp.rs | 700 +++++++++++++----- src/web/mod.rs | 4 +- 8 files changed, 1487 insertions(+), 207 deletions(-) create mode 100644 .cursor/plans/mcp_get_script_endpoint_4c4e80af.plan.md create mode 100644 .cursor/plans/mcp_sync_endpoint_enhancement_20ff55ce.plan.md diff --git a/.cursor/plans/mcp_get_script_endpoint_4c4e80af.plan.md b/.cursor/plans/mcp_get_script_endpoint_4c4e80af.plan.md new file mode 100644 index 00000000..69cb9ea7 --- /dev/null +++ b/.cursor/plans/mcp_get_script_endpoint_4c4e80af.plan.md @@ -0,0 +1,269 @@ +--- +name: MCP get_script endpoint +overview: Add a `get_script` MCP tool that reads script source from Studio. Accepts `id` (server Ref) or `fsPath` (resolved server-side to id via RojoTree). Plugin uses a shallow copy of instanceMap.fromIds retained from the last MCP sync for O(1) lookup. +todos: + - id: generalize-channels + content: Rename McpSyncState -> McpState, generalize command/result channels to serde_json::Value, rename sync_in_progress -> command_in_progress, update all references in mcp.rs, api.rs, mod.rs + status: completed + - id: rust-structs-and-tool + content: Add GetScriptCommand/GetScriptResult structs in mcp.rs, add get_script tool definition (with full workflow description) in handle_tools_list, add handle_get_script handler + status: completed + - id: api-resolution + content: "In api.rs: add command type routing in MCP stream handler (getScript bypasses active_api_connections guard), resolve fsPath -> id via tree for getScript commands, enrich result with id/fsPath" + status: completed + - id: plugin-mcpstream + content: "In McpStream.lua: add handler for data.type == 'getScript', forward to onGetScriptCommand callback" + status: completed + - id: plugin-retain-map + content: "In App/init.lua: at top of setConfirmCallback, shallow-copy instanceMap.fromIds into self._lastMcpFromIds (survives InstanceMap:stop())" + status: completed + - id: plugin-handler + content: "In App/init.lua: wire onGetScriptCommand, implement handleMcpGetScript with id-based lookup via self._lastMcpFromIds, source reading (regular + draft), SHA1 hash, include instancePath in response" + status: completed + - id: tests + content: Rust unit tests (serde, handler, guards, response formatting), api.rs resolution tests, plugin tests (id lookup via retained map, shallow copy survives stop(), source reading, hash) + status: completed +isProject: false +--- + +# MCP `get_script` Endpoint + +## Design + +The agent provides `id` (server Ref from a sync response) or `fsPath` (Atlas filesystem path). Both resolve to a server Ref which the plugin looks up via a shallow copy of `instanceMap.fromIds` retained from the last MCP sync. No raw instance paths, no QueryDescendants, no name ambiguity issues. + +This endpoint exists for conflict resolution: agent calls `atlas_sync`, gets a conflict response with `id` and `fsPath` per change, then calls `get_script` to read the Studio version of conflicting scripts. The `studioHash` in the response feeds back into subsequent sync override verification. + +```mermaid +sequenceDiagram + participant Agent + participant mcpRs as mcp.rs + participant apiRs as api.rs + participant Plugin as Plugin McpStream + + Agent->>mcpRs: get_script(id OR fsPath, fromDraft?) + mcpRs->>apiRs: command_tx (Value) + Note over apiRs: If fsPath: resolve to id via RojoTree + apiRs->>Plugin: {type: "getScript", requestId, id, fromDraft} + Plugin->>Plugin: self._lastMcpFromIds[id] (O(1)) + Plugin->>Plugin: Read Source (or draft), compute SHA1 + Plugin->>apiRs: {requestId, status, source, studioHash, className, instancePath} + Note over apiRs: Enrich with id, fsPath + apiRs->>mcpRs: result_tx (Value) + mcpRs->>Agent: Tool response with source + metadata +``` + + + +## Channel Generalization + +Change `McpSyncState` channels from typed structs to `serde_json::Value` so any command/result can flow through. This enables `get_script` and future endpoints without additional channel plumbing. + +- Rename `McpSyncState` -> `McpState` +- Rename `sync_in_progress` -> `command_in_progress` (any MCP command blocks others) +- `command_tx: watch::Sender>` (was `Option`) +- `result_tx: Mutex>>` (was `oneshot::Sender`) + +Each handler serializes its command struct to Value before sending, deserializes from Value on response. + +## Retained InstanceMap (Shallow Copy) + +**Critical:** `InstanceMap:stop()` iterates all entries and calls `removeInstance`, which sets `fromIds[id] = nil` for every entry. This is called by `ServeSession:__stopInternal()` during `endSession()`. Storing a reference to the InstanceMap object or its `fromIds` table gives an empty map after session teardown. + +**Solution:** At the top of `setConfirmCallback` (before any branch that could trigger session teardown), make a shallow copy of `fromIds`: + +```lua +serveSession:setConfirmCallback(function(instanceMap, patch, serverInfo) + local retainedFromIds = {} + for id, inst in instanceMap.fromIds do + retainedFromIds[id] = inst + end + self._lastMcpFromIds = retainedFromIds + + -- ... rest of existing callback logic +end) +``` + +This copy: + +- Survives `InstanceMap:stop()` because it's a separate table +- Is O(N) but fast (just string key -> Instance ref copies, typical project has a few thousand entries) +- Is replaced on the next MCP sync (each sync creates a fresh copy) +- Stale entries handled gracefully: if `instance.Parent == nil`, the instance was destroyed since the sync +- Cleared when MCP stream disconnects (`self._lastMcpFromIds = nil`) + +**Note:** The `already_connected` early return (line 1131-1138 in `startMcpSync`) does NOT create a ServeSession, so `_lastMcpFromIds` is not updated in that case -- it retains whatever value it had from the last actual sync. This is correct behavior. + +## get_script Input Schema + +```json +{ + "id": "00abcdef01234567890abcdef0123456", + "fsPath": "src/server/MyScript.server.luau", + "fromDraft": false +} +``` + +- `id`: Server Ref (32-char hex) from a previous sync response. Looked up via retained `_lastMcpFromIds` on plugin. +- `fsPath`: Atlas filesystem path relative to project root. Resolved server-side to `id` via RojoTree. +- Priority: `id` > `fsPath`. At least one required. +- `fromDraft`: If true, read unsaved editor content via `ScriptEditorService:GetEditorSource`. Default: false. + +## get_script Response + +``` +Script source for ServerScriptService/MyScript (Script): +Hash: a1b2c3d4e5... + + +{"status":"success","source":"print('hello')","studioHash":"a1b2c3...","className":"Script","instancePath":"ServerScriptService/MyScript","id":"00abcdef...","fsPath":"src/server/MyScript.server.luau","isDraft":false} + +``` + +- `instancePath`: Human-readable instance path derived from `instance:GetFullName()` on the plugin side, reformatted from `game.A.B.C` to `A/B/C`. Included for readability and logging; not used for any lookup. + +## Tool Description + +The `get_script` tool definition in `handle_tools_list` must include a comprehensive description that teaches the agent the full workflow: + +``` +Read a script's source code from Roblox Studio. The id and fsPath parameters come from +a previous atlas_sync response's change entries. + +If you haven't run atlas_sync yet, call atlas_sync(mode: "dryrun") first -- this +establishes the instance mapping without applying any changes, then you can call +get_script with the id or fsPath from that response. + +Conflict resolution workflow: +1. atlas_sync(mode: "fastfail") -- get changes with studioHash, fail if unresolved +2. get_script(id: "...") -- read the Studio version of a conflicting script +3. Merge the Studio source with your local changes, write to filesystem +4. atlas_sync(overrides: [{id, direction: "push", studioHash: "...from get_script response..."}]) + +The studioHash in the response is the SHA1 of the git blob format of the script's Source +property. Pass it as the studioHash in sync overrides to verify the script hasn't changed +between your get_script call and the retry sync. +``` + +## Server-Side Resolution (api.rs) + +When api.rs receives a `getScript` command from the channel: + +1. **If `fsPath` present** (and no `id`): `project_root.join(fsPath)` -> `tree.get_ids_at_path(&abs_path)` -> get first Ref -> verify exists and is a script class -> inject as `id` in the command forwarded to plugin. Store fsPath for response enrichment. +2. **If `id` present**: optionally verify it exists in the tree and is a script class. Pass through to plugin. Resolve fsPath from tree metadata for response enrichment. +3. **On response from plugin**: enrich with `id` (Ref hex) and `fsPath`. + +## Plugin-Side Resolution + +The plugin receives `{ type: "getScript", requestId, id, fromDraft }`: + +```lua +local fromIds = self._lastMcpFromIds +if not fromIds then + -- error: "No previous sync session. Run atlas_sync(mode: \"dryrun\") first to establish the instance mapping." +end + +local instance = fromIds[id] +if not instance then + -- error: "Instance not found by id. The sync session may be stale, run atlas_sync again." +end +if instance.Parent == nil then + -- error: "Instance was deleted since last sync. Run atlas_sync again." +end + +if not instance:IsA("LuaSourceContainer") then + -- error: "Instance is not a script (ClassName)" +end +``` + +After resolution: + +1. Read source: if `fromDraft`, use `ScriptEditorService:GetEditorSource(instance)`; else `instance.Source` +2. Compute studioHash: `SHA1(buffer.fromstring("blob " .. #source .. "\0" .. source))` +3. Build instancePath: `string.gsub(instance:GetFullName(), "^game%.", ""):gsub("%.", "/")` +4. Return `{ requestId, status = "success", source, studioHash, className = instance.ClassName, instancePath }` + +## Guards + +- `command_in_progress` prevents concurrent MCP commands (channel has one result slot) +- `plugin_stream_connected` required (otherwise no way to communicate with Studio) +- NOT blocked by `active_api_connections > 0`: In api.rs, the `active_api_connections` guard must only apply to `sync` commands, not `getScript`. The command type routing checks the `type` field before this guard. Reading a script is safe during live sync; if no retained map exists, the plugin returns a clear error. +- If `id` or `fsPath` resolves to a non-script class in the tree, return error immediately from api.rs without plugin round-trip + +## Files to Change + +### Rust: `[src/web/mcp.rs](src/web/mcp.rs)` + +- Rename `McpSyncState` -> `McpState`, generalize channels to `Value` +- Add `GetScriptCommand` struct: `{ request_id, id?, fs_path?, from_draft }` +- Add `GetScriptResult` struct: `{ status, source?, studio_hash?, class_name?, instance_path?, id?, fs_path?, is_draft?, message? }` +- Add `get_script` tool definition in `handle_tools_list` with full workflow description and inputSchema (id, fsPath, fromDraft) +- Add `handle_get_script` handler (parse params, validate at least one of id/fsPath, send command, format response) +- Update `handle_atlas_sync` to serialize/deserialize via Value +- Update all references from `McpSyncState` to `McpState` +- Tests: wire type serde, handler parse, response formatting + +### Rust: `[src/web/api.rs](src/web/api.rs)` + +- Update `McpSyncState` -> `McpState` references +- Update channel types to `Value` in MCP stream handler +- Add command type routing: check `type` field on received command Value BEFORE the `active_api_connections` guard. `sync` goes through the guard; `getScript` bypasses it. +- For `getScript`: resolve fsPath -> id via tree if needed, verify script class, forward to plugin with `id`, enrich result with id/fsPath on response +- For `sync`: existing behavior (forward, enrich fsPath on result) + +### Rust: `[src/web/mod.rs](src/web/mod.rs)` (if `McpSyncState` is referenced) + +- Update type references + +### Plugin: `[plugin/src/McpStream.lua](plugin/src/McpStream.lua)` + +- Add handler for `data.type == "getScript"` +- Call `self._onGetScriptCommand(data.requestId, data)`, expect Promise result +- Send JSON result back over WebSocket (same pattern as sync) + +### Plugin: `[plugin/src/App/init.lua](plugin/src/App/init.lua)` + +- **Shallow copy instanceMap.fromIds**: At the top of `setConfirmCallback` in `startMcpSync`, copy `instanceMap.fromIds` into `self._lastMcpFromIds`. This must happen before any branch (the copy survives `InstanceMap:stop()` which clears the original). +- Clear `self._lastMcpFromIds = nil` when MCP stream disconnects. +- Wire `onGetScriptCommand` callback in McpStream constructor. +- Add `handleMcpGetScript(requestId, params)` method: + - Lookup via `self._lastMcpFromIds[id]` + - Handle missing map ("Run atlas_sync first") + - Handle stale entries (not found, Parent == nil) + - LuaSourceContainer check + - Source reading (regular + draft via ScriptEditorService) + - SHA1 hash computation (using existing `SHA1.luau`) + - Build instancePath from `instance:GetFullName()` + - Return Promise resolving to result table + +## Test Plan + +### Rust unit tests (mcp.rs) + +- `GetScriptResult` serde round-trip with all fields (including `instancePath`) +- `handle_tools_list` includes `get_script` tool with correct inputSchema and workflow description +- `handle_get_script` returns error when no plugin stream connected +- `handle_get_script` returns error when command already in progress +- `handle_get_script` requires at least one of id/fsPath +- Response formatting: success case includes `` block, error case sets `isError` + +### Rust integration (api.rs) + +- `getScript` with `fsPath`: resolves via `get_ids_at_path`, injects `id`, forwards to plugin +- `getScript` with `id`: passes through, resolves `fsPath` for enrichment +- `getScript` with non-script `fsPath`: returns error before forwarding +- `getScript` with non-existent `fsPath`: returns error +- `getScript` bypasses `active_api_connections > 0` guard (command type routing) +- Result enrichment adds `id` and `fsPath` + +### Plugin tests + +- Shallow copy survives `InstanceMap:stop()`: copy retains entries after original is cleared +- Id lookup: found, not found, stale (Parent nil), no retained map +- Non-script instance: returns error +- Source reading: regular source, draft source via ScriptEditorService +- StudioHash: correct git blob format SHA1 +- instancePath: correctly derived from GetFullName +- Retained map cleared on MCP stream disconnect +- `already_connected` early return does not update retained map + diff --git a/.cursor/plans/mcp_sync_endpoint_enhancement_20ff55ce.plan.md b/.cursor/plans/mcp_sync_endpoint_enhancement_20ff55ce.plan.md new file mode 100644 index 00000000..ce38ec0b --- /dev/null +++ b/.cursor/plans/mcp_sync_endpoint_enhancement_20ff55ce.plan.md @@ -0,0 +1,379 @@ +--- +name: MCP sync endpoint enhancement +overview: Enhance the `atlas_sync` MCP tool to support modes (standard/manual/fastfail/dryrun), agent-specified override auto-accepts with hash + property verification, and enriched change metadata (studioHash, property before/after values, className, patchType, fsPath) in the response. +todos: + - id: rust-structs + content: Add SyncOverride struct (id-based matching, with expectedProperties), expand McpSyncCommand (mode, overrides), expand SyncChange (id, className, patchType, studioHash, defaultSelection, fsPath, properties) in mcp.rs + status: completed + - id: rust-handler + content: Update handle_atlas_sync to parse mode/overrides from params, include in command, handle new statuses (dryrun, fastfail_unresolved), emit structured JSON in response + status: completed + - id: rust-tool-schema + content: Update atlas_sync inputSchema in handle_tools_list with mode and overrides parameters + status: completed + - id: rust-api-enrich + content: In api.rs MCP stream handler, enrich changes with fsPath by looking up Ref in tree's InstigatingSource::Path after receiving plugin result. Handle Ref::from_str failures gracefully (plugin-generated GUIDs for removed nodes). + status: completed + - id: plugin-mcpstream + content: Update McpStream.lua to forward mode and overrides from sync command to onSyncCommand callback + status: completed + - id: plugin-startmcpsync + content: Update startMcpSync in App/init.lua to accept mode/overrides, implement mode logic (manual/fastfail/dryrun) and id-based override matching with studioHash + expectedProperties verification (decodeValue before trueEquals). Thread patch/instanceMap to _buildMcpChangeList. + status: completed + - id: plugin-changelist + content: Expand _buildMcpChangeList signature to receive patch + instanceMap. Encode properties using raw changedProperties (incoming) and encodeProperty via descriptor lookup (current). Skip Source for scripts. Handle encode failures gracefully. + status: completed + - id: rust-tests + content: Update existing mcp.rs tests and add new tests for modes, overrides, enriched SyncChange fields, and new statuses + status: completed + - id: test-suite + content: Full coverage test suite. Rust side - wire type serde for all new structs, mode passthrough, fsPath enrichment (valid Ref, invalid Ref, ProjectNode, missing instance), JSON response block parsing. Plugin side - _buildMcpChangeList property encoding (scripts skip Source, encode failures, multiple property types), override verification (studioHash match/mismatch, expectedProperties match/mismatch/decode, stale id), mode behavior (standard fast-forward, standard user-review, manual always-review, fastfail unresolved, fastfail all-resolved, dryrun returns without applying), end-to-end MCP stream forwarding of mode/overrides. + status: completed +isProject: false +--- + +# MCP Sync Endpoint Enhancement + +## Context + +The current `atlas_sync` tool takes no parameters and has one behavior: if all changes are git-auto-selected, fast-forward; otherwise show Studio UI and block. The agent has no way to control this, no way to auto-accept specific changes, and gets minimal metadata back. + +The enhanced endpoint enables an agent workflow like: + +1. `atlas_sync(mode: "fastfail")` -- get changes with hashes, fail if any are unresolved +2. `get_script(id)` -- read conflicting script from Studio (future endpoint, uses `id` from sync response) +3. Merge changes, write to filesystem +4. `atlas_sync(overrides: [{id, direction, studioHash}])` -- auto-accept with hash verification + +## New Input Schema + +```json +{ + "mode": "standard", + "overrides": [ + { + "id": "00abcdef01234567890abcdef0123456", + "direction": "push", + "studioHash": "a1b2c3...", + "expectedProperties": { + "Disabled": false + } + }, + { + "id": "00abcdef01234567890abcdef0123457", + "direction": "push", + "expectedProperties": { + "Position": {"Vector3": [0, 5, 0]}, + "Anchored": true + } + } + ] +} +``` + +- **mode**: `"standard"` (default) | `"manual"` | `"fastfail"` | `"dryrun"` +- **overrides**: array of auto-accept directives per instance: + - `id` (required): the server Ref (32-char hex) from a previous sync response. Matched directly via `patchTree:getNode(id)` -- O(1), no ambiguity possible. + - `direction` (required): `"push"` or `"pull"` + - `studioHash` (scripts only): SHA1 of git blob of Source, verifies Source hasn't changed since agent last inspected + - `expectedProperties` (optional): map of property name to expected current Studio value. Plugin verifies each against the live instance. Uses RbxDom encoded format (same as response). If any mismatch, the override is rejected and the change becomes unresolved. + +### Why `id` instead of `path` + +Instance names are not unique in Roblox -- a parent can have multiple children with the same name. Paths built from `instance.Name` segments (e.g. `Workspace/Part`) are ambiguous when duplicates exist. The server Ref (`id`) is a unique 128-bit identifier per instance, assigned by the server and round-tripped through all responses. Using it eliminates the path ambiguity problem entirely on both the response and override sides, with no need for path escaping or debugId disambiguation. + +The `path` field in responses remains for human readability (agents can display it, use it in logs). The `fsPath` field provides the unique Atlas filesystem path for file editing. Neither is used for override matching. + +## New Response Change Schema + +Currently each change is `{path, direction}`. Enhanced: + +```json +{ + "path": "ServerScriptService/MyScript", + "id": "00abcdef...", + "direction": "push", + "patchType": "Edit", + "className": "Script", + "studioHash": "a1b2c3...", + "defaultSelection": "push", + "fsPath": "src/server/MyScript.server.luau", + "properties": { + "Disabled": { + "current": false, + "incoming": true + } + } +} +``` + +For a non-script instance with property changes: + +```json +{ + "path": "Workspace/MyPart", + "id": "00abcdef...", + "direction": "push", + "patchType": "Edit", + "className": "Part", + "defaultSelection": "pull", + "fsPath": "src/workspace/MyPart.model.json5", + "properties": { + "Position": { + "current": {"Vector3": [0, 5, 0]}, + "incoming": {"Vector3": [10, 5, 0]} + }, + "Anchored": { + "current": false, + "incoming": true + } + } +} +``` + +- `studioHash`: SHA1 of `"blob \0"` (git blob format) of the script's Source in Studio. Only present for script classes. Used by agents for override verification. +- `fsPath`: Atlas filesystem path relative to project root. Enriched server-side from `InstigatingSource::Path`. Null for project-node instances or additions. +- `defaultSelection`: what git auto-selected (`"push"` / `"pull"` / null) +- `properties`: map of changed property name to `{current, incoming}` values in RbxDom encoded format. `current` is the live Studio value, `incoming` is the Atlas value. For scripts, Source is omitted from `properties` (use `studioHash` + `get_script` instead to avoid massive payloads). Only present for Edit patchType. + +## Mode Behavior (plugin confirm callback) + + +| Mode | Behavior | +| ---------- | ------------------------------------------------------------------------------------------------------------------------- | +| `standard` | Current: fast-forward if all pre-selected, otherwise show UI | +| `manual` | Always show UI, never auto-accept | +| `fastfail` | If any change has null `defaultSelection` AND no matching override, resolve immediately with `fastfail_unresolved` status | +| `dryrun` | Compute full change list with hashes, resolve immediately, abort session without applying | + + +## Override Matching and Verification (plugin side) + +For each override, the plugin: + +1. **Match by `id`**: Look up the PatchTree node via `patchTree:getNode(override.id)`. If not found (stale id from a previous session), skip the override. +2. **studioHash verification** (scripts): If `override.studioHash` is provided, compute SHA1 of git blob of `node.instance.Source` and compare. Mismatch = override rejected (someone changed the script since the agent last looked). +3. **expectedProperties verification** (any instance): If `override.expectedProperties` is provided, for each property in the map: + a. **Decode** the expected value via `decodeValue(expectedValue, instanceMap)` -- the override arrives as RbxDom-encoded JSON (e.g. `{"Vector3": [0, 5, 0]}`), which must be decoded to a live Roblox value before comparison. + b. Read the current Studio value via `getProperty(node.instance, propName)`. + c. Compare using `trueEquals(decodedExpected, currentValue)` -- fuzzy float comparison, EnumItem handling, etc. + d. Any mismatch = override rejected (someone changed a property since the agent last looked). +4. If all verifications pass, set the node's selection to `override.direction`, treating it as pre-selected for mode logic. +5. If verification fails, the node remains unresolved. In `fastfail` mode this means the sync fails immediately with the unresolved changes listed. In `standard` mode the user sees them in the Studio UI. + +This design ensures: no path ambiguity (id is unique), no path escaping needed, O(1) node lookup, and the agent never silently overwrites concurrent changes from other developers. + +## Files to Change + +### Rust: `[src/web/mcp.rs](src/web/mcp.rs)` + +- Add `SyncOverride` struct: `{id, direction, studio_hash?, expected_properties?}` +- Expand `McpSyncCommand`: add `mode: String`, `overrides: Vec` +- Expand `SyncChange`: add `id`, `class_name`, `patch_type`, `studio_hash`, `default_selection`, `fs_path`, `properties` (all optional) +- Update `atlas_sync` tool definition in `handle_tools_list` with new inputSchema +- Update `handle_atlas_sync` to parse mode + overrides from params and include them in the command +- Handle new statuses (`fastfail_unresolved`, `dryrun`) in response formatting +- Update `handle_atlas_sync` response text to include richer change info (structured JSON instead of plain text list) +- Update tests + +### Rust: `[src/web/api.rs](src/web/api.rs)` (MCP stream handler) + +- Serialize mode + overrides into the WebSocket command to the plugin +- After receiving `McpSyncResult`, enrich each change's `fs_path`: + - Attempt to parse `change.id` as `Ref` (32-char hex, via `Ref::from_str`). If parse fails (e.g. plugin-generated GUID for removed nodes), leave `fs_path = None` and continue. + - Look up `tree.get_instance(ref).metadata.instigating_source` + - If `InstigatingSource::Path(p)`, strip project root prefix, set `fs_path` + - If instance not found or source is `ProjectNode`, leave `fs_path = None` +- Forward enriched result via oneshot + +### Plugin: `[plugin/src/McpStream.lua](plugin/src/McpStream.lua)` + +- Forward `data.mode` and `data.overrides` from the sync command to `self._onSyncCommand(data.requestId, data.mode, data.overrides)` + +### Plugin: `[plugin/src/App/init.lua](plugin/src/App/init.lua)` + +`**startMcpSync(requestId, mode, overrides)`:** + +- Accept `mode` and `overrides` parameters +- In the confirm callback (which receives `instanceMap, patch, serverInfo`), after building PatchTree: + - Apply overrides: for each override, look up node by `patchTree:getNode(override.id)`, verify studioHash for scripts, verify expectedProperties via `decodeValue` + `trueEquals`, set selection + - `dryrun`: build full change list via `_buildMcpChangeList(patchTree, patch, instanceMap, nil, true)`, resolve with `status: "dryrun"`, return `"Abort"` + - `fastfail`: check if any node is unresolved after defaults + overrides; if so, resolve with `fastfail_unresolved` + full change list, return `"Abort"` + - `manual`: force into user-review flow (skip the `allPreSelected` fast-forward) + - `standard`: current behavior +- All calls to `_buildMcpChangeList` in this function must pass `patch` and `instanceMap` (available as closure variables from the confirm callback) + +`**_buildMcpChangeList(patchTree, patch, instanceMap, selections, includeAll)`:** + +Signature change: receives the original `patch` object and `instanceMap` in addition to the tree, because: + +- `node.changeList` stores **decoded** Roblox values (Vector3 userdata, EnumItem, etc.) that cannot be JSON-serialized +- We need the raw `patch.updated[].changedProperties` for incoming values (already in RbxDom-encoded wire format) +- We need `encodeProperty(instance, propName, descriptor)` to encode current Studio values into the same wire format +- Both require looking up the `RbxDom.findCanonicalPropertyDescriptor(instance.ClassName, propName)` for each property + +Implementation: + +- Add `includeAll` parameter (for dryrun/fastfail: include all changes regardless of selection) +- Add `className = node.className` +- Add `patchType = node.patchType` +- Add `id = node.id` (server Ref, for fsPath enrichment). Note: for Removed nodes, this may be a plugin-generated GUID rather than a server Ref -- the Rust `fsPath` enrichment must handle `Ref::from_str` parse failures gracefully (set `fsPath = null`). +- Add `defaultSelection = node.defaultSelection` +- For script instances (`node.instance:IsA("LuaSourceContainer")`): compute and add `studioHash` = SHA1 of git blob of `instance.Source` +- Add `properties` map for Edit nodes: build a `patchLookup` from `patch.updated` keyed by `change.id`, then for each property in `patchLookup[node.id].changedProperties`: + - **Skip** `Source` for script classes (handled by `studioHash` + future `get_script`) + - `incoming`: use the raw value from `changedProperties[propName]` (already RbxDom-encoded wire format, JSON-safe) + - `current`: look up descriptor via `RbxDom.findCanonicalPropertyDescriptor(node.className, propName)`, then call `encodeProperty(node.instance, propName, descriptor)` to get the RbxDom-encoded current value + - Store as `{current = encodedCurrent, incoming = encodedIncoming}` + - If encoding fails for either side, skip that property (don't crash the response) + +## New Statuses + + +| Status | When | isError | +| -------------------------------------------------------- | ---------------------------------------- | --------- | +| `dryrun` | Dryrun mode, returning what would happen | false | +| `fastfail_unresolved` | Fastfail mode, unresolved changes exist | true | +| (existing) `success`, `empty`, `rejected`, `error`, etc. | Unchanged | Unchanged | + + +## Response Format Enhancement + +Currently the MCP tool response is plain text. After this change, for all statuses that include changes (success, fastfail, dryrun, rejected), include a structured JSON block inside the text content so the agent can parse it: + +``` +Sync completed successfully. + +Changes: +- [push] ServerScriptService/MyScript (Edit, Script, fs=src/server/MyScript.server.luau) +- [pull] Workspace/MyPart (Edit, Part, fs=src/workspace/MyPart.model.json5) + + +{"status":"success","changes":[{"path":"ServerScriptService/MyScript","id":"00ab...","direction":"push","patchType":"Edit","className":"Script","studioHash":"abc123","defaultSelection":"push","fsPath":"src/server/MyScript.server.luau","properties":{"Disabled":{"current":false,"incoming":true}}},{"path":"Workspace/MyPart","id":"00cd...","direction":"pull","patchType":"Edit","className":"Part","defaultSelection":"pull","fsPath":"src/workspace/MyPart.model.json5","properties":{"Position":{"current":{"Vector3":[0,5,0]},"incoming":{"Vector3":[10,5,0]}}}}]} + +``` + +The `` block gives the agent machine-readable data while keeping the human-readable summary above it. Note: script Source is deliberately excluded from `properties` to avoid massive payloads -- agents use `studioHash` for verification and the future `get_script` tool to read actual source content when needed. + +## Test Suite + +Full coverage testing across both Rust and Lua, organized by layer. + +### Rust: `src/web/mcp.rs` unit tests + +**Wire type serde (extend existing `wire_types` module):** + +- `SyncOverride` serializes/deserializes with camelCase (`id`, `direction`, `studioHash`, `expectedProperties`) +- `SyncOverride` defaults: missing `studioHash` and `expectedProperties` deserialize to `None` +- `McpSyncCommand` serializes `mode` and `overrides` fields +- `SyncChange` serializes all new optional fields (`id`, `className`, `patchType`, `studioHash`, `defaultSelection`, `fsPath`, `properties`) +- `SyncChange` omits `None` optional fields (skip_serializing_if) +- `McpSyncResult` with enriched `SyncChange` entries round-trips correctly + +**Handler tests (extend existing `handler_tests` module):** + +- `handle_tools_list` returns updated inputSchema with `mode` and `overrides` +- `handle_atlas_sync` parses `mode` from params and passes through to command +- `handle_atlas_sync` parses `overrides` from params and passes through to command +- `handle_atlas_sync` defaults `mode` to `"standard"` when omitted +- `handle_atlas_sync` defaults `overrides` to empty array when omitted + +**Guard tests (extend existing `atlas_sync_guards` module):** + +- Success result with enriched changes includes `` block in response text +- Dryrun status maps to `isError: false` +- `fastfail_unresolved` status maps to `isError: true` +- Empty status still returns `isError: false` +- Rejected with enriched changes includes `` block + +### Rust: `src/web/api.rs` fsPath enrichment tests + +- Valid Ref resolves to `InstigatingSource::Path` -> fsPath is set (relative to project root) +- Valid Ref with `InstigatingSource::ProjectNode` -> fsPath is None +- Invalid Ref string (plugin GUID) -> fsPath is None, no panic +- Ref for instance not in tree -> fsPath is None +- Multiple changes: only those with valid paths get fsPath + +### Plugin: `_buildMcpChangeList` tests (`plugin/src/App/init.spec.lua` or dedicated file) + +**Basic enrichment:** + +- Edit node includes `id`, `className`, `patchType`, `defaultSelection` +- Add node includes `id`, `className`, `patchType = "Add"` +- Remove node includes `id`, `className`, `patchType = "Remove"` +- Script node includes `studioHash` (correct git blob SHA1) +- Non-script node has no `studioHash` + +**Property encoding:** + +- Edit node with `changedProperties` produces `properties` map with `{current, incoming}` in RbxDom-encoded format +- `Source` is skipped for script classes +- `Name` change appears in properties as `changedName` +- Failed encode for current value skips that property (no crash) +- Failed encode for incoming value skips that property (no crash) +- Multiple property types: Bool, Number, String, Vector3, Color3, Enum + +**includeAll parameter:** + +- `includeAll = false` with selections: only includes selected nodes +- `includeAll = false` without selections: includes all with defaultSelection or "push" +- `includeAll = true`: includes every node with patchType regardless of selection + +### Plugin: Override verification tests + +**id-based matching:** + +- Override with valid node id matches correctly +- Override with unknown id is silently skipped +- Multiple overrides match their respective nodes + +**studioHash verification:** + +- Matching hash: override accepted, node selection set +- Mismatching hash: override rejected, node stays unresolved +- No studioHash on override for script: override accepted (no verification) +- studioHash on non-script: ignored (only Source hashing applies to scripts) + +**expectedProperties verification:** + +- All properties match: override accepted +- One property mismatch: override rejected entirely +- Empty expectedProperties: override accepted (no verification) +- Property value decoded via `decodeValue` before `trueEquals` comparison +- Fuzzy float comparison works (e.g. `0.30000001` matches `0.3`) +- EnumItem comparison works (e.g. `{Enum = 2}` vs `Enum.Material.Plastic`) + +### Plugin: Mode behavior tests + +**standard mode:** + +- All pre-selected: fast-forwards, resolves `success`, returns Confirm with selections +- Some unresolved: shows Confirming UI, waits for user +- Empty patch: resolves `empty` + +**manual mode:** + +- All pre-selected: still shows Confirming UI (no fast-forward) +- User confirms: resolves `success` +- User aborts: resolves `rejected` + +**fastfail mode:** + +- All resolved (via defaults + overrides): fast-forwards, resolves `success` +- Some unresolved after defaults + overrides: resolves `fastfail_unresolved` immediately, returns `"Abort"`, includes full change list with `includeAll = true` +- Override verification failure makes node unresolved, triggers fastfail + +**dryrun mode:** + +- Resolves immediately with `status: "dryrun"`, returns `"Abort"` +- Change list includes all nodes with full metadata (properties, studioHash) +- Session is aborted without applying any changes +- Works with empty patch (resolves `empty`) + +### Plugin: McpStream forwarding tests + +- `{type: "sync", requestId, mode, overrides}` forwards all three fields to `onSyncCommand` +- Missing `mode` defaults to `"standard"` (or nil, handled by `startMcpSync`) +- Missing `overrides` defaults to empty table +- Result is JSON-encoded and sent back over WebSocket + diff --git a/TODO.txt b/TODO.txt index c3a83af5..0f8d8cb0 100644 --- a/TODO.txt +++ b/TODO.txt @@ -341,11 +341,10 @@ can we allow the terminal to stay so i can like run some git commands myself wit - we should steal this system fully: D:\UserGenerated\studio-rust-mcp-server\plugin\Utils\Paths.luau - in the sync feedback it gives, we should include Paths.GetRelativePath so the agent is able to find it via the read_script ALONGSIDE the current info we can give so it can also find it on the Atlas filesystem - investigate this further, because i just want to give the agent the best possible way to query roblox instances, and also atlas instances on the filesystem. if using this Paths module is an overstep and we can accomplish it with a fully atlas-based approach, that's fine - aslong as it works CONSISTENTLY and is simple. -* get_script_history PATH - - * port all of the basic MCP endpoints offered via https://github.com/Roblox/studio-rust-mcp-server - integrate them into how we've done our mcp in atlas - make sure to handle the placeid filtering like we already do when establishing the connection + - we've done some actual fixes over at D:\UserGenerated\studio-rust-mcp-server on these, we should probably pull those fixed versions! we don't need that whole studio selection system from over there, though. * syncback - invokes plugin based syncback - ensure this doesn't interrupt the connection of the mcp, since i think we shutdown the api? we should just not shutdown the api or something @@ -353,6 +352,8 @@ can we allow the terminal to stay so i can like run some git commands myself wit - also add logging for commands (including the json) it runs and the resulting json. try to keep this on one line please, json compact. * ensure all mcp route documentation is well formed and gives the agent knowledge on how to use this system to the fullest. - I did .md based docs over with my D:\UserGenerated\studio-rust-mcp-server codebase, we should probably switch to this as it's cleaner + - Instruct on good sync flow, with get_script merging. + - If user requests get_script and we havn't sync'd yet, just do a dryrun sync to get the instance id map going. [sync] right now sync is running REALLY slow diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua index 98c3d7a0..060de7f0 100644 --- a/plugin/src/App/init.lua +++ b/plugin/src/App/init.lua @@ -211,6 +211,9 @@ function App:init() onSyncCommand = function(requestId, mode, overrides) return self:startMcpSync(requestId, mode, overrides) end, + onGetScriptCommand = function(requestId, data) + return self:handleMcpGetScript(requestId, data) + end, getPluginConfig = function(key) return Settings:get(key) end, @@ -239,6 +242,7 @@ function App:willUnmount() if self.mcpStream then self.mcpStream:stop() self.mcpStream = nil + self._lastMcpFromIds = nil end end @@ -1186,6 +1190,12 @@ function App:startMcpSync(requestId, mode, overrides) serveSession:setConfirmCallback(function(instanceMap, patch, serverInfo) cachedServerInfo = serverInfo + local retainedFromIds = {} + for id, inst in instanceMap.fromIds do + retainedFromIds[id] = inst + end + self._lastMcpFromIds = retainedFromIds + PatchSet.removeDataModelName(patch, instanceMap) if PatchSet.isEmpty(patch) then @@ -1538,6 +1548,85 @@ function App:_buildMcpChangeList(patchTree, patch, instanceMap, selections, incl return changes end +function App:handleMcpGetScript(requestId, params) + local ScriptEditorService = game:GetService("ScriptEditorService") + + return Promise.resolve():andThen(function() + local fromIds = self._lastMcpFromIds + if not fromIds then + return { + requestId = requestId, + status = "error", + message = 'No previous sync session. Run atlas_sync(mode: "dryrun") first to establish the instance mapping.', + } + end + + local id = params.id + if not id then + return { + requestId = requestId, + status = "error", + message = "No id provided. The server should have resolved fsPath to an id.", + } + end + + local instance = fromIds[id] + if not instance then + return { + requestId = requestId, + status = "error", + message = "Instance not found by id. The sync session may be stale, run atlas_sync again.", + } + end + if instance.Parent == nil then + return { + requestId = requestId, + status = "error", + message = "Instance was deleted since last sync. Run atlas_sync again.", + } + end + + if not instance:IsA("LuaSourceContainer") then + return { + requestId = requestId, + status = "error", + message = `Instance is not a script (class: {instance.ClassName})`, + } + end + + local source + local isDraft = params.fromDraft == true + if isDraft then + local ok, draft = pcall(ScriptEditorService.GetEditorSource, ScriptEditorService, instance) + if ok then + source = draft + else + source = instance.Source + isDraft = false + end + else + source = instance.Source + end + + local gitBlob = "blob " .. tostring(#source) .. "\0" .. source + local studioHash = SHA1(buffer.fromstring(gitBlob)) + + local fullName = instance:GetFullName() + local instancePath = string.gsub(fullName, "^game%.", "") + instancePath = string.gsub(instancePath, "%.", "/") + + return { + requestId = requestId, + status = "success", + source = source, + studioHash = studioHash, + className = instance.ClassName, + instancePath = instancePath, + isDraft = isDraft, + } + end) +end + function App:render() local pluginName = "Atlas " .. Version.display(Config.version) diff --git a/plugin/src/McpStream.lua b/plugin/src/McpStream.lua index 1a446bbd..d93477cf 100644 --- a/plugin/src/McpStream.lua +++ b/plugin/src/McpStream.lua @@ -17,6 +17,7 @@ function McpStream.new(options) local self = setmetatable({}, McpStream) self._onSyncCommand = options.onSyncCommand + self._onGetScriptCommand = options.onGetScriptCommand self._getPluginConfig = options.getPluginConfig self._wsClient = nil self._running = false @@ -125,6 +126,33 @@ function McpStream:_tryConnect() end) end end) + elseif data.type == "getScript" and data.requestId then + Log.info("MCP stream: received getScript command (requestId={})", data.requestId) + + local resultPromise = self._onGetScriptCommand(data.requestId, data) + + resultPromise + :andThen(function(result) + if self._wsClient then + local json = HttpService:JSONEncode(result) + pcall(function() + self._wsClient:Send(json) + end) + end + end) + :catch(function(err) + Log.warn("MCP stream: getScript command failed: {}", tostring(err)) + if self._wsClient then + local errorResult = HttpService:JSONEncode({ + requestId = data.requestId, + status = "error", + message = tostring(err), + }) + pcall(function() + self._wsClient:Send(errorResult) + end) + end + end) end end) diff --git a/src/web/api.rs b/src/web/api.rs index 8205d7e8..597283e9 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -125,7 +125,7 @@ pub async fn call( serve_session: Arc, mut request: Request, syncback_signal: Arc, - mcp_state: Arc, + mcp_state: Arc, active_api_connections: Arc, ) -> Response> { let service = ApiService::new(serve_session); @@ -3554,6 +3554,115 @@ fn enrich_sync_changes_with_fs_path( } } +// --------------------------------------------------------------------------- +// MCP get_script resolution helpers +// --------------------------------------------------------------------------- + +struct GetScriptContext { + id: String, + fs_path: Option, +} + +fn resolve_get_script_command( + cmd: &serde_json::Value, + serve_session: &ServeSession, +) -> Result<(serde_json::Value, GetScriptContext), serde_json::Value> { + use crate::snapshot::InstigatingSource; + use std::str::FromStr; + + let request_id = cmd + .get("requestId") + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(); + let from_draft = cmd + .get("fromDraft") + .and_then(|v| v.as_bool()) + .unwrap_or(false); + let id_param = cmd.get("id").and_then(|v| v.as_str()); + let fs_path_param = cmd.get("fsPath").and_then(|v| v.as_str()); + + let tree = serve_session.tree(); + let project_path = serve_session.root_project().folder_location(); + + let (resolved_ref, resolved_id, resolved_fs_path) = if let Some(id_str) = id_param { + let r = rbx_dom_weak::types::Ref::from_str(id_str).map_err(|_| { + serde_json::json!({ + "requestId": request_id, + "status": "error", + "message": format!("Invalid id format: {id_str}"), + }) + })?; + + let fs_path = tree.get_metadata(r).and_then(|meta| { + if let Some(InstigatingSource::Path(path)) = &meta.instigating_source { + path.strip_prefix(project_path) + .ok() + .map(|rel| rel.to_string_lossy().into_owned()) + } else { + None + } + }); + + (r, id_str.to_string(), fs_path) + } else if let Some(fs_path_str) = fs_path_param { + let abs_path = project_path.join(fs_path_str); + let ids = tree.get_ids_at_path(&abs_path); + if ids.is_empty() { + return Err(serde_json::json!({ + "requestId": request_id, + "status": "error", + "message": format!("No instance found at fsPath: {fs_path_str}"), + })); + } + let r = ids[0]; + (r, r.to_string(), Some(fs_path_str.to_string())) + } else { + return Err(serde_json::json!({ + "requestId": request_id, + "status": "error", + "message": "Either id or fsPath must be provided.", + })); + }; + + if let Some(inst) = tree.get_instance(resolved_ref) { + let class = inst.class_name(); + if !matches!(class.as_ref(), "Script" | "LocalScript" | "ModuleScript") { + return Err(serde_json::json!({ + "requestId": request_id, + "status": "error", + "message": format!("Instance is not a script (class: {class})"), + })); + } + } + + let forwarded = serde_json::json!({ + "type": "getScript", + "requestId": request_id, + "id": resolved_id, + "fromDraft": from_draft, + }); + + let ctx = GetScriptContext { + id: resolved_id, + fs_path: resolved_fs_path, + }; + + Ok((forwarded, ctx)) +} + +fn enrich_get_script_result(result: &mut serde_json::Value, ctx: &GetScriptContext) { + if let Some(obj) = result.as_object_mut() { + obj.insert("id".to_string(), serde_json::Value::String(ctx.id.clone())); + if let Some(fs_path) = &ctx.fs_path { + obj.insert( + "fsPath".to_string(), + serde_json::Value::String(fs_path.clone()), + ); + } + } +} + // --------------------------------------------------------------------------- // MCP stream WebSocket: plugin connects here to receive sync commands // --------------------------------------------------------------------------- @@ -3561,7 +3670,7 @@ fn enrich_sync_changes_with_fs_path( async fn handle_mcp_stream_upgrade( request: &mut Request, serve_session: Arc, - mcp_state: Arc, + mcp_state: Arc, active_api_connections: Arc, ) -> Response> { use std::sync::atomic::Ordering; @@ -3602,10 +3711,10 @@ async fn handle_mcp_stream_upgrade( async fn handle_mcp_stream_connection( websocket: HyperWebsocket, serve_session: Arc, - mcp_state: Arc, + mcp_state: Arc, active_api_connections: Arc, ) -> anyhow::Result<()> { - use super::mcp::{McpSyncResult, PluginConfig}; + use super::mcp::PluginConfig; use std::sync::atomic::Ordering; let mut websocket = websocket.await?; @@ -3692,30 +3801,36 @@ async fn handle_mcp_stream_connection( } } + // Tracks context from the most recent outgoing command so we can enrich + // the plugin's response. For getScript: stores the resolved Ref + fsPath. + let mut pending_get_script_context: Option = None; + loop { tokio::select! { - // Watch for sync commands from the MCP handler. result = command_rx.changed() => { if result.is_err() { break; } let command = command_rx.borrow_and_update().clone(); - let Some(cmd) = command else { + let Some(cmd_value) = command else { continue; }; - // If we are in connected mode right now, immediately respond - // without forwarding to the plugin. - if active_api_connections.load(Ordering::Relaxed) > 0 { - let result = McpSyncResult { - request_id: cmd.request_id, - status: "already_connected".to_string(), - changes: vec![], - message: Some( - "Atlas is already connected in live sync mode.".to_string(), - ), - }; + let cmd_type = cmd_value + .get("type") + .and_then(|t| t.as_str()) + .unwrap_or(""); + + // For sync commands, block if a live API connection is active. + // getScript is a read-only operation that can proceed regardless. + if cmd_type == "sync" && active_api_connections.load(Ordering::Relaxed) > 0 { + let result = serde_json::json!({ + "requestId": cmd_value.get("requestId").and_then(|v| v.as_str()).unwrap_or(""), + "status": "already_connected", + "changes": [], + "message": "Atlas is already connected in live sync mode.", + }); let mut slot = mcp_state .result_tx .lock() @@ -3726,31 +3841,70 @@ async fn handle_mcp_stream_connection( continue; } - let json_cmd = serde_json::to_string(&cmd)?; + // For getScript, resolve fsPath -> id via tree before forwarding. + if cmd_type == "getScript" { + match resolve_get_script_command(&cmd_value, &serve_session) { + Ok((forwarded, ctx)) => { + pending_get_script_context = Some(ctx); + let json_cmd = serde_json::to_string(&forwarded) + .unwrap_or_else(|_| "{}".to_string()); + if websocket.send(Message::Text(json_cmd.into())).await.is_err() { + log::debug!("MCP stream: plugin disconnected while sending getScript"); + break; + } + continue; + } + Err(error_result) => { + let mut slot = mcp_state + .result_tx + .lock() + .unwrap_or_else(|e| e.into_inner()); + if let Some(tx) = slot.take() { + let _ = tx.send(error_result); + } + continue; + } + } + } + + let json_cmd = serde_json::to_string(&cmd_value)?; if websocket.send(Message::Text(json_cmd.into())).await.is_err() { log::debug!("MCP stream: plugin disconnected while sending command"); break; } } - // Handle incoming messages from the plugin. msg = websocket.next() => { match msg { Some(Ok(Message::Text(text))) => { - if let Ok(mut result) = serde_json::from_str::(&text) { - enrich_sync_changes_with_fs_path( - &mut result.changes, - &serve_session, - ); + if let Ok(mut result_value) = serde_json::from_str::(&text) { + // Enrich based on the command type that produced this result. + if let Some(ctx) = pending_get_script_context.take() { + enrich_get_script_result(&mut result_value, &ctx); + } else if let Some(changes) = result_value.get_mut("changes") { + if let Some(changes_arr) = changes.as_array_mut() { + let mut sync_changes: Vec = changes_arr + .iter() + .filter_map(|v| serde_json::from_value(v.clone()).ok()) + .collect(); + enrich_sync_changes_with_fs_path( + &mut sync_changes, + &serve_session, + ); + *changes = serde_json::to_value(&sync_changes) + .unwrap_or(serde_json::Value::Array(vec![])); + } + } + let mut slot = mcp_state .result_tx .lock() .unwrap_or_else(|e| e.into_inner()); if let Some(tx) = slot.take() { - let _ = tx.send(result); + let _ = tx.send(result_value); } } else { - log::debug!("MCP stream: ignoring non-result message from plugin"); + log::debug!("MCP stream: ignoring non-JSON message from plugin"); } } Some(Ok(Message::Close(_))) | None => { @@ -3773,7 +3927,6 @@ async fn handle_mcp_stream_connection( } } - // Clean up: mark disconnected, cancel any pending sync. mcp_state .plugin_stream_connected .store(false, Ordering::SeqCst); @@ -3785,19 +3938,18 @@ async fn handle_mcp_stream_connection( *slot = None; } - // If there's a pending sync request, send a disconnect error. { let mut slot = mcp_state .result_tx .lock() .unwrap_or_else(|e| e.into_inner()); if let Some(tx) = slot.take() { - let _ = tx.send(McpSyncResult { - request_id: String::new(), - status: "error".to_string(), - changes: vec![], - message: Some("Plugin MCP stream disconnected.".to_string()), - }); + let _ = tx.send(serde_json::json!({ + "requestId": "", + "status": "error", + "changes": [], + "message": "Plugin MCP stream disconnected.", + })); } } diff --git a/src/web/mcp.rs b/src/web/mcp.rs index ef0981e8..b3cdb945 100644 --- a/src/web/mcp.rs +++ b/src/web/mcp.rs @@ -84,22 +84,49 @@ pub struct McpSyncResult { pub message: Option, } -/// Shared state for MCP sync coordination between the agent-facing `/mcp` +/// Result for get_script, deserialized from the plugin's Value response. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GetScriptResult { + pub request_id: String, + pub status: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub source: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub studio_hash: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub class_name: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub instance_path: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub id: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub fs_path: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub is_draft: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub message: Option, +} + +/// Shared state for MCP coordination between the agent-facing `/mcp` /// endpoint and the plugin-facing `/api/mcp/stream` WebSocket. -pub struct McpSyncState { - pub sync_in_progress: AtomicBool, - pub command_tx: tokio::sync::watch::Sender>, - pub command_rx: tokio::sync::watch::Receiver>, - pub result_tx: Mutex>>, +/// +/// Channels carry `serde_json::Value` so any command/result type can flow +/// through (sync, getScript, future endpoints). +pub struct McpState { + pub command_in_progress: AtomicBool, + pub command_tx: tokio::sync::watch::Sender>, + pub command_rx: tokio::sync::watch::Receiver>, + pub result_tx: Mutex>>, pub plugin_stream_connected: AtomicBool, pub plugin_config: Mutex>, } -impl McpSyncState { +impl McpState { pub fn new() -> Self { let (command_tx, command_rx) = tokio::sync::watch::channel(None); Self { - sync_in_progress: AtomicBool::new(false), + command_in_progress: AtomicBool::new(false), command_tx, command_rx, result_tx: Mutex::new(None), @@ -181,7 +208,7 @@ fn json_response(body: &JsonRpcResponse, status: StatusCode) -> Response, - mcp_state: Arc, + mcp_state: Arc, active_api_connections: Arc, ) -> Response> { if request.method() != Method::POST { @@ -297,6 +324,27 @@ fn handle_tools_list(id: Option) -> Response> { } } } + }, + { + "name": "get_script", + "description": "Read a script's source code from Roblox Studio. The id and fsPath parameters come from a previous atlas_sync response's change entries.\n\nIf you haven't run atlas_sync yet, call atlas_sync(mode: \"dryrun\") first -- this establishes the instance mapping without applying any changes, then you can call get_script with the id or fsPath from that response.\n\nConflict resolution workflow:\n1. atlas_sync(mode: \"fastfail\") -- get changes with studioHash, fail if unresolved\n2. get_script(id: \"...\") -- read the Studio version of a conflicting script\n3. Merge the Studio source with your local changes, write to filesystem\n4. atlas_sync(overrides: [{id, direction: \"push\", studioHash: \"...from get_script response...\"}])\n\nThe studioHash in the response is the SHA1 of the git blob format of the script's Source property. Pass it as the studioHash in sync overrides to verify the script hasn't changed between your get_script call and the retry sync.", + "inputSchema": { + "type": "object", + "properties": { + "id": { + "type": "string", + "description": "Server Ref (32-char hex) from a previous atlas_sync response. Preferred over fsPath." + }, + "fsPath": { + "type": "string", + "description": "Atlas filesystem path relative to project root (e.g. 'src/server/MyScript.server.luau'). Resolved server-side to an instance id." + }, + "fromDraft": { + "type": "boolean", + "description": "If true, read unsaved editor content instead of the saved Source property. Default: false." + } + } + } } ] }); @@ -307,7 +355,7 @@ fn handle_tools_list(id: Option) -> Response> { async fn handle_tools_call( id: Option, params: Option, - mcp_state: Arc, + mcp_state: Arc, active_api_connections: Arc, ) -> Response> { let tool_name = params @@ -316,24 +364,26 @@ async fn handle_tools_call( .and_then(|n| n.as_str()) .unwrap_or(""); - if tool_name != "atlas_sync" { - let resp = JsonRpcResponse::error(id, -32602, format!("Unknown tool: {tool_name}")); - return json_response(&resp, StatusCode::OK); - } - let arguments = params .as_ref() .and_then(|p| p.get("arguments")) .cloned() .unwrap_or(Value::Object(Default::default())); - handle_atlas_sync(id, arguments, mcp_state, active_api_connections).await + match tool_name { + "atlas_sync" => handle_atlas_sync(id, arguments, mcp_state, active_api_connections).await, + "get_script" => handle_get_script(id, arguments, mcp_state).await, + _ => { + let resp = JsonRpcResponse::error(id, -32602, format!("Unknown tool: {tool_name}")); + json_response(&resp, StatusCode::OK) + } + } } async fn handle_atlas_sync( id: Option, arguments: Value, - mcp_state: Arc, + mcp_state: Arc, active_api_connections: Arc, ) -> Response> { let mode = arguments @@ -373,19 +423,19 @@ async fn handle_atlas_sync( } if mcp_state - .sync_in_progress + .command_in_progress .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) .is_err() { return tool_response( id, true, - "An atlas_sync operation is already in progress. Please wait for it to complete.", + "An MCP command is already in progress. Please wait for it to complete.", ); } if !mcp_state.plugin_stream_connected.load(Ordering::Relaxed) { - mcp_state.sync_in_progress.store(false, Ordering::SeqCst); + mcp_state.command_in_progress.store(false, Ordering::SeqCst); return tool_response( id, true, @@ -411,15 +461,16 @@ async fn handle_atlas_sync( overrides, }; - if mcp_state.command_tx.send(Some(command)).is_err() { - mcp_state.sync_in_progress.store(false, Ordering::SeqCst); + let command_value = serde_json::to_value(&command).unwrap_or(Value::Null); + if mcp_state.command_tx.send(Some(command_value)).is_err() { + mcp_state.command_in_progress.store(false, Ordering::SeqCst); return tool_response(id, true, "Failed to send sync command to the plugin."); } - let result = match result_rx.await { - Ok(result) => result, + let result_value = match result_rx.await { + Ok(v) => v, Err(_) => { - mcp_state.sync_in_progress.store(false, Ordering::SeqCst); + mcp_state.command_in_progress.store(false, Ordering::SeqCst); return tool_response( id, true, @@ -428,7 +479,19 @@ async fn handle_atlas_sync( } }; - mcp_state.sync_in_progress.store(false, Ordering::SeqCst); + let result: McpSyncResult = match serde_json::from_value(result_value) { + Ok(r) => r, + Err(e) => { + mcp_state.command_in_progress.store(false, Ordering::SeqCst); + return tool_response( + id, + true, + &format!("Failed to parse sync result from plugin: {e}"), + ); + } + }; + + mcp_state.command_in_progress.store(false, Ordering::SeqCst); let _ = mcp_state.command_tx.send(None); @@ -492,6 +555,121 @@ async fn handle_atlas_sync( tool_response(id, is_error, &text) } +async fn handle_get_script( + id: Option, + arguments: Value, + mcp_state: Arc, +) -> Response> { + if mcp_state + .command_in_progress + .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) + .is_err() + { + return tool_response( + id, + true, + "An MCP command is already in progress. Please wait for it to complete.", + ); + } + + if !mcp_state.plugin_stream_connected.load(Ordering::Relaxed) { + mcp_state.command_in_progress.store(false, Ordering::SeqCst); + return tool_response( + id, + true, + "No Roblox Studio plugin is connected to the MCP stream. \ + Make sure Studio is open with the Atlas plugin installed.", + ); + } + + let has_id = arguments.get("id").and_then(|v| v.as_str()).is_some(); + let has_fs_path = arguments.get("fsPath").and_then(|v| v.as_str()).is_some(); + if !has_id && !has_fs_path { + mcp_state.command_in_progress.store(false, Ordering::SeqCst); + return tool_response( + id, + true, + "Either 'id' or 'fsPath' must be provided. Both come from a previous atlas_sync response.", + ); + } + + let (result_tx, result_rx) = tokio::sync::oneshot::channel(); + { + let mut slot = mcp_state + .result_tx + .lock() + .unwrap_or_else(|e| e.into_inner()); + *slot = Some(result_tx); + } + + let request_id = uuid::Uuid::new_v4().to_string(); + let from_draft = arguments + .get("fromDraft") + .and_then(|v| v.as_bool()) + .unwrap_or(false); + + let command = serde_json::json!({ + "type": "getScript", + "requestId": request_id, + "id": arguments.get("id"), + "fsPath": arguments.get("fsPath"), + "fromDraft": from_draft, + }); + + if mcp_state.command_tx.send(Some(command)).is_err() { + mcp_state.command_in_progress.store(false, Ordering::SeqCst); + return tool_response(id, true, "Failed to send get_script command to the plugin."); + } + + let result_value = match result_rx.await { + Ok(v) => v, + Err(_) => { + mcp_state.command_in_progress.store(false, Ordering::SeqCst); + return tool_response( + id, + true, + "Plugin disconnected before completing the get_script operation.", + ); + } + }; + + mcp_state.command_in_progress.store(false, Ordering::SeqCst); + let _ = mcp_state.command_tx.send(None); + + let result: GetScriptResult = match serde_json::from_value(result_value) { + Ok(r) => r, + Err(e) => { + return tool_response( + id, + true, + &format!("Failed to parse get_script result from plugin: {e}"), + ); + } + }; + + let is_error = result.status != "success"; + + if is_error { + let msg = result.message.as_deref().unwrap_or("Unknown error"); + return tool_response(id, true, msg); + } + + let class_name = result.class_name.as_deref().unwrap_or("Script"); + let instance_path = result.instance_path.as_deref().unwrap_or("Unknown"); + let studio_hash = result.studio_hash.as_deref().unwrap_or(""); + + let mut text = + format!("Script source for {instance_path} ({class_name}):\nHash: {studio_hash}"); + + let json_block = serde_json::to_value(&result).unwrap_or(Value::Null); + text.push_str(&format!( + "\n\n\n{}\n", + serde_json::to_string(&json_block).unwrap_or_default() + )); + + tool_response(id, false, &text) +} + fn tool_response(id: Option, is_error: bool, text: &str) -> Response> { let result = serde_json::json!({ "content": [{ "type": "text", "text": text }], @@ -614,15 +792,15 @@ mod tests { } } - // -- McpSyncState tests --------------------------------------------------- + // -- McpState tests ------------------------------------------------------- mod state_tests { use super::*; #[test] fn new_state_starts_clean() { - let state = McpSyncState::new(); - assert!(!state.sync_in_progress.load(Ordering::Relaxed)); + let state = McpState::new(); + assert!(!state.command_in_progress.load(Ordering::Relaxed)); assert!(!state.plugin_stream_connected.load(Ordering::Relaxed)); assert!(state.plugin_config.lock().unwrap().is_none()); assert!(state.result_tx.lock().unwrap().is_none()); @@ -630,16 +808,16 @@ mod tests { } #[test] - fn sync_mutex_prevents_double_acquire() { - let state = McpSyncState::new(); - let first = state.sync_in_progress.compare_exchange( + fn command_mutex_prevents_double_acquire() { + let state = McpState::new(); + let first = state.command_in_progress.compare_exchange( false, true, Ordering::SeqCst, Ordering::SeqCst, ); assert!(first.is_ok()); - let second = state.sync_in_progress.compare_exchange( + let second = state.command_in_progress.compare_exchange( false, true, Ordering::SeqCst, @@ -649,45 +827,35 @@ mod tests { } #[test] - fn command_channel_delivers() { - let state = McpSyncState::new(); + fn command_channel_delivers_value() { + let state = McpState::new(); let mut rx = state.command_rx.clone(); - let cmd = McpSyncCommand { - command_type: "sync".to_string(), - request_id: "test-1".to_string(), - mode: "standard".to_string(), - overrides: vec![], - }; + let cmd = serde_json::json!({"type": "sync", "requestId": "test-1"}); state.command_tx.send(Some(cmd)).unwrap(); assert!(rx.has_changed().unwrap()); let received = rx.borrow_and_update().clone().unwrap(); - assert_eq!(received.request_id, "test-1"); + assert_eq!(received["requestId"], "test-1"); } #[test] - fn result_oneshot_delivers() { - let state = McpSyncState::new(); + fn result_oneshot_delivers_value() { + let state = McpState::new(); let (tx, rx) = tokio::sync::oneshot::channel(); *state.result_tx.lock().unwrap() = Some(tx); - let result = McpSyncResult { - request_id: "r1".to_string(), - status: "success".to_string(), - changes: vec![], - message: None, - }; + let result = serde_json::json!({"status": "success", "requestId": "r1"}); let sender = state.result_tx.lock().unwrap().take().unwrap(); sender.send(result).unwrap(); let received = rx.blocking_recv().unwrap(); - assert_eq!(received.status, "success"); + assert_eq!(received["status"], "success"); } #[test] fn plugin_config_cache() { - let state = McpSyncState::new(); + let state = McpState::new(); let cfg = PluginConfig { two_way_sync: true, one_shot_sync: false, @@ -729,9 +897,10 @@ mod tests { let bytes = rt.block_on(async { resp.into_body().collect().await.unwrap().to_bytes() }); let json: Value = serde_json::from_slice(&bytes).unwrap(); let tools = json["result"]["tools"].as_array().unwrap(); - assert_eq!(tools.len(), 1); + assert_eq!(tools.len(), 2); assert_eq!(tools[0]["name"], "atlas_sync"); assert!(tools[0]["inputSchema"].is_object()); + assert_eq!(tools[1]["name"], "get_script"); } #[test] @@ -768,7 +937,7 @@ mod tests { #[tokio::test] async fn rejects_when_api_connected() { - let state = Arc::new(McpSyncState::new()); + let state = Arc::new(McpState::new()); let conns = Arc::new(AtomicUsize::new(1)); let resp = handle_atlas_sync(Some(Value::from(1)), empty_args(), state, conns).await; @@ -782,7 +951,7 @@ mod tests { #[tokio::test] async fn rejects_when_api_connected_includes_config() { - let state = Arc::new(McpSyncState::new()); + let state = Arc::new(McpState::new()); *state.plugin_config.lock().unwrap() = Some(PluginConfig { two_way_sync: true, one_shot_sync: false, @@ -801,9 +970,9 @@ mod tests { } #[tokio::test] - async fn rejects_when_sync_in_progress() { - let state = Arc::new(McpSyncState::new()); - state.sync_in_progress.store(true, Ordering::SeqCst); + async fn rejects_when_command_in_progress() { + let state = Arc::new(McpState::new()); + state.command_in_progress.store(true, Ordering::SeqCst); let conns = Arc::new(AtomicUsize::new(0)); let resp = handle_atlas_sync(Some(Value::from(2)), empty_args(), state, conns).await; @@ -817,7 +986,7 @@ mod tests { #[tokio::test] async fn rejects_when_no_plugin_connected() { - let state = Arc::new(McpSyncState::new()); + let state = Arc::new(McpState::new()); state.plugin_stream_connected.store(false, Ordering::SeqCst); let conns = Arc::new(AtomicUsize::new(0)); @@ -830,32 +999,27 @@ mod tests { let text = json["result"]["content"][0]["text"].as_str().unwrap(); assert!(text.contains("No Roblox Studio plugin")); // Mutex should be released after rejection. - assert!(!state.sync_in_progress.load(Ordering::Relaxed)); + assert!(!state.command_in_progress.load(Ordering::Relaxed)); } #[tokio::test] async fn sends_command_and_returns_success_result() { - let state = Arc::new(McpSyncState::new()); + let state = Arc::new(McpState::new()); state.plugin_stream_connected.store(true, Ordering::SeqCst); let conns = Arc::new(AtomicUsize::new(0)); let state2 = Arc::clone(&state); - // Spawn a task that simulates the plugin responding. tokio::spawn(async move { let mut rx = state2.command_rx.clone(); rx.changed().await.unwrap(); let cmd = rx.borrow().clone().unwrap(); + let req_id = cmd["requestId"].as_str().unwrap_or(""); - let result = McpSyncResult { - request_id: cmd.request_id, - status: "success".to_string(), - changes: vec![SyncChange { - path: "ServerScriptService/Main.server.luau".to_string(), - direction: "push".to_string(), - ..Default::default() - }], - message: None, - }; + let result = serde_json::json!({ + "requestId": req_id, + "status": "success", + "changes": [{"path": "ServerScriptService/Main.server.luau", "direction": "push"}], + }); let tx = state2.result_tx.lock().unwrap().take().unwrap(); tx.send(result).unwrap(); @@ -870,12 +1034,12 @@ mod tests { let text = json["result"]["content"][0]["text"].as_str().unwrap(); assert!(text.contains("Sync completed successfully")); assert!(text.contains("[push] ServerScriptService/Main.server.luau")); - assert!(!state.sync_in_progress.load(Ordering::Relaxed)); + assert!(!state.command_in_progress.load(Ordering::Relaxed)); } #[tokio::test] async fn returns_rejection_with_presented_changes() { - let state = Arc::new(McpSyncState::new()); + let state = Arc::new(McpState::new()); state.plugin_stream_connected.store(true, Ordering::SeqCst); let conns = Arc::new(AtomicUsize::new(0)); @@ -884,24 +1048,17 @@ mod tests { let mut rx = state2.command_rx.clone(); rx.changed().await.unwrap(); let cmd = rx.borrow().clone().unwrap(); + let req_id = cmd["requestId"].as_str().unwrap_or(""); - let result = McpSyncResult { - request_id: cmd.request_id, - status: "rejected".to_string(), - changes: vec![ - SyncChange { - path: "ReplicatedStorage/Foo.luau".to_string(), - direction: "push".to_string(), - ..Default::default() - }, - SyncChange { - path: "Workspace/Bar.model.json5".to_string(), - direction: "pull".to_string(), - ..Default::default() - }, + let result = serde_json::json!({ + "requestId": req_id, + "status": "rejected", + "changes": [ + {"path": "ReplicatedStorage/Foo.luau", "direction": "push"}, + {"path": "Workspace/Bar.model.json5", "direction": "pull"}, ], - message: Some("User rejected the sync changes.".to_string()), - }; + "message": "User rejected the sync changes.", + }); let tx = state2.result_tx.lock().unwrap().take().unwrap(); tx.send(result).unwrap(); @@ -921,7 +1078,7 @@ mod tests { #[tokio::test] async fn handles_plugin_disconnect_during_sync() { - let state = Arc::new(McpSyncState::new()); + let state = Arc::new(McpState::new()); state.plugin_stream_connected.store(true, Ordering::SeqCst); let conns = Arc::new(AtomicUsize::new(0)); @@ -941,12 +1098,12 @@ mod tests { assert_eq!(json["result"]["isError"], true); let text = json["result"]["content"][0]["text"].as_str().unwrap(); assert!(text.contains("disconnected")); - assert!(!state.sync_in_progress.load(Ordering::Relaxed)); + assert!(!state.command_in_progress.load(Ordering::Relaxed)); } #[tokio::test] async fn empty_status_is_not_an_error() { - let state = Arc::new(McpSyncState::new()); + let state = Arc::new(McpState::new()); state.plugin_stream_connected.store(true, Ordering::SeqCst); let conns = Arc::new(AtomicUsize::new(0)); @@ -955,15 +1112,16 @@ mod tests { let mut rx = state2.command_rx.clone(); rx.changed().await.unwrap(); let cmd = rx.borrow().clone().unwrap(); + let req_id = cmd["requestId"].as_str().unwrap_or(""); + + let result = serde_json::json!({ + "requestId": req_id, + "status": "empty", + "changes": [], + }); let tx = state2.result_tx.lock().unwrap().take().unwrap(); - tx.send(McpSyncResult { - request_id: cmd.request_id, - status: "empty".to_string(), - changes: vec![], - message: None, - }) - .unwrap(); + tx.send(result).unwrap(); }); let resp = handle_atlas_sync(Some(Value::from(8)), empty_args(), state, conns).await; @@ -977,7 +1135,7 @@ mod tests { #[tokio::test] async fn dryrun_status_is_not_an_error() { - let state = Arc::new(McpSyncState::new()); + let state = Arc::new(McpState::new()); state.plugin_stream_connected.store(true, Ordering::SeqCst); let conns = Arc::new(AtomicUsize::new(0)); @@ -986,22 +1144,22 @@ mod tests { let mut rx = state2.command_rx.clone(); rx.changed().await.unwrap(); let cmd = rx.borrow().clone().unwrap(); + let req_id = cmd["requestId"].as_str().unwrap_or(""); + + let result = serde_json::json!({ + "requestId": req_id, + "status": "dryrun", + "changes": [{ + "path": "Workspace/Part", + "direction": "unresolved", + "id": "aabb", + "className": "Part", + "patchType": "Edit", + }], + }); let tx = state2.result_tx.lock().unwrap().take().unwrap(); - tx.send(McpSyncResult { - request_id: cmd.request_id, - status: "dryrun".to_string(), - changes: vec![SyncChange { - path: "Workspace/Part".to_string(), - direction: "unresolved".to_string(), - id: Some("aabb".to_string()), - class_name: Some("Part".to_string()), - patch_type: Some("Edit".to_string()), - ..Default::default() - }], - message: None, - }) - .unwrap(); + tx.send(result).unwrap(); }); let args = serde_json::json!({"mode": "dryrun"}); @@ -1017,7 +1175,7 @@ mod tests { #[tokio::test] async fn fastfail_unresolved_is_error() { - let state = Arc::new(McpSyncState::new()); + let state = Arc::new(McpState::new()); state.plugin_stream_connected.store(true, Ordering::SeqCst); let conns = Arc::new(AtomicUsize::new(0)); @@ -1026,19 +1184,20 @@ mod tests { let mut rx = state2.command_rx.clone(); rx.changed().await.unwrap(); let cmd = rx.borrow().clone().unwrap(); + let req_id = cmd["requestId"].as_str().unwrap_or(""); + + let result = serde_json::json!({ + "requestId": req_id, + "status": "fastfail_unresolved", + "changes": [{ + "path": "ServerScriptService/Script", + "direction": "unresolved", + }], + "message": "Unresolved changes exist.", + }); let tx = state2.result_tx.lock().unwrap().take().unwrap(); - tx.send(McpSyncResult { - request_id: cmd.request_id, - status: "fastfail_unresolved".to_string(), - changes: vec![SyncChange { - path: "ServerScriptService/Script".to_string(), - direction: "unresolved".to_string(), - ..Default::default() - }], - message: Some("Unresolved changes exist.".to_string()), - }) - .unwrap(); + tx.send(result).unwrap(); }); let args = serde_json::json!({"mode": "fastfail"}); @@ -1053,7 +1212,7 @@ mod tests { #[tokio::test] async fn mode_and_overrides_forwarded_to_command() { - let state = Arc::new(McpSyncState::new()); + let state = Arc::new(McpState::new()); state.plugin_stream_connected.store(true, Ordering::SeqCst); let conns = Arc::new(AtomicUsize::new(0)); @@ -1062,21 +1221,22 @@ mod tests { let mut rx = state2.command_rx.clone(); rx.changed().await.unwrap(); let cmd = rx.borrow().clone().unwrap(); + let req_id = cmd["requestId"].as_str().unwrap_or(""); - assert_eq!(cmd.mode, "fastfail"); - assert_eq!(cmd.overrides.len(), 1); - assert_eq!(cmd.overrides[0].id, "aabbccdd"); - assert_eq!(cmd.overrides[0].direction, "push"); - assert_eq!(cmd.overrides[0].studio_hash.as_deref(), Some("abc123")); + assert_eq!(cmd["mode"], "fastfail"); + assert_eq!(cmd["overrides"].as_array().unwrap().len(), 1); + assert_eq!(cmd["overrides"][0]["id"], "aabbccdd"); + assert_eq!(cmd["overrides"][0]["direction"], "push"); + assert_eq!(cmd["overrides"][0]["studioHash"], "abc123"); + + let result = serde_json::json!({ + "requestId": req_id, + "status": "success", + "changes": [], + }); let tx = state2.result_tx.lock().unwrap().take().unwrap(); - tx.send(McpSyncResult { - request_id: cmd.request_id, - status: "success".to_string(), - changes: vec![], - message: None, - }) - .unwrap(); + tx.send(result).unwrap(); }); let args = serde_json::json!({ @@ -1095,7 +1255,7 @@ mod tests { #[tokio::test] async fn mode_defaults_to_standard() { - let state = Arc::new(McpSyncState::new()); + let state = Arc::new(McpState::new()); state.plugin_stream_connected.store(true, Ordering::SeqCst); let conns = Arc::new(AtomicUsize::new(0)); @@ -1104,18 +1264,19 @@ mod tests { let mut rx = state2.command_rx.clone(); rx.changed().await.unwrap(); let cmd = rx.borrow().clone().unwrap(); + let req_id = cmd["requestId"].as_str().unwrap_or(""); - assert_eq!(cmd.mode, "standard"); - assert!(cmd.overrides.is_empty()); + assert_eq!(cmd["mode"], "standard"); + assert!(cmd["overrides"].as_array().unwrap().is_empty()); + + let result = serde_json::json!({ + "requestId": req_id, + "status": "empty", + "changes": [], + }); let tx = state2.result_tx.lock().unwrap().take().unwrap(); - tx.send(McpSyncResult { - request_id: cmd.request_id, - status: "empty".to_string(), - changes: vec![], - message: None, - }) - .unwrap(); + tx.send(result).unwrap(); }); let resp = handle_atlas_sync(Some(Value::from(12)), empty_args(), state, conns).await; @@ -1126,7 +1287,7 @@ mod tests { #[tokio::test] async fn enriched_changes_include_json_block() { - let state = Arc::new(McpSyncState::new()); + let state = Arc::new(McpState::new()); state.plugin_stream_connected.store(true, Ordering::SeqCst); let conns = Arc::new(AtomicUsize::new(0)); @@ -1135,30 +1296,30 @@ mod tests { let mut rx = state2.command_rx.clone(); rx.changed().await.unwrap(); let cmd = rx.borrow().clone().unwrap(); + let req_id = cmd["requestId"].as_str().unwrap_or(""); - let tx = state2.result_tx.lock().unwrap().take().unwrap(); - tx.send(McpSyncResult { - request_id: cmd.request_id, - status: "success".to_string(), - changes: vec![SyncChange { - path: "Workspace/Part".to_string(), - direction: "push".to_string(), - id: Some("00aabb".to_string()), - class_name: Some("Part".to_string()), - patch_type: Some("Edit".to_string()), - default_selection: Some("push".to_string()), - fs_path: Some("src/workspace/Part.model.json5".to_string()), - studio_hash: None, - properties: Some(serde_json::json!({ + let result = serde_json::json!({ + "requestId": req_id, + "status": "success", + "changes": [{ + "path": "Workspace/Part", + "direction": "push", + "id": "00aabb", + "className": "Part", + "patchType": "Edit", + "defaultSelection": "push", + "fsPath": "src/workspace/Part.model.json5", + "properties": { "Anchored": { "current": false, "incoming": true } - })), + } }], - message: None, - }) - .unwrap(); + }); + + let tx = state2.result_tx.lock().unwrap().take().unwrap(); + tx.send(result).unwrap(); }); let resp = handle_atlas_sync(Some(Value::from(13)), empty_args(), state, conns).await; @@ -1393,4 +1554,205 @@ mod tests { .contains(&Value::from("direction"))); } } + + // -- get_script tests ----------------------------------------------------- + + mod get_script_tests { + use super::*; + + #[test] + fn get_script_result_serde_roundtrip() { + let result = GetScriptResult { + request_id: "r1".to_string(), + status: "success".to_string(), + source: Some("print('hi')".to_string()), + studio_hash: Some("abc123".to_string()), + class_name: Some("Script".to_string()), + instance_path: Some("ServerScriptService/Main".to_string()), + id: Some("00aabb".to_string()), + fs_path: Some("src/server/Main.server.luau".to_string()), + is_draft: Some(false), + message: None, + }; + let json = serde_json::to_value(&result).unwrap(); + assert_eq!(json["requestId"], "r1"); + assert_eq!(json["source"], "print('hi')"); + assert_eq!(json["studioHash"], "abc123"); + assert_eq!(json["className"], "Script"); + assert_eq!(json["instancePath"], "ServerScriptService/Main"); + assert_eq!(json["isDraft"], false); + assert!(json.get("message").is_none()); + + let deserialized: GetScriptResult = serde_json::from_value(json).unwrap(); + assert_eq!(deserialized.source.as_deref(), Some("print('hi')")); + assert_eq!( + deserialized.fs_path.as_deref(), + Some("src/server/Main.server.luau") + ); + } + + #[test] + fn get_script_result_omits_none_fields() { + let result = GetScriptResult { + request_id: "r1".to_string(), + status: "error".to_string(), + source: None, + studio_hash: None, + class_name: None, + instance_path: None, + id: None, + fs_path: None, + is_draft: None, + message: Some("Not found".to_string()), + }; + let json = serde_json::to_value(&result).unwrap(); + assert!(json.get("source").is_none()); + assert!(json.get("studioHash").is_none()); + assert!(json.get("className").is_none()); + assert!(json.get("isDraft").is_none()); + assert_eq!(json["message"], "Not found"); + } + + #[test] + fn tools_list_includes_get_script() { + let resp = handle_tools_list(Some(Value::from(1))); + let rt = tokio::runtime::Runtime::new().unwrap(); + let bytes = rt.block_on(async { resp.into_body().collect().await.unwrap().to_bytes() }); + let json: Value = serde_json::from_slice(&bytes).unwrap(); + let tools = json["result"]["tools"].as_array().unwrap(); + assert_eq!(tools.len(), 2); + + let get_script = tools.iter().find(|t| t["name"] == "get_script").unwrap(); + assert!(get_script["description"] + .as_str() + .unwrap() + .contains("Conflict resolution")); + assert!(get_script["inputSchema"]["properties"]["id"].is_object()); + assert!(get_script["inputSchema"]["properties"]["fsPath"].is_object()); + assert!(get_script["inputSchema"]["properties"]["fromDraft"].is_object()); + } + + #[tokio::test] + async fn get_script_rejects_when_no_plugin() { + let state = Arc::new(McpState::new()); + state.plugin_stream_connected.store(false, Ordering::SeqCst); + + let args = serde_json::json!({"id": "aabb"}); + let resp = handle_get_script(Some(Value::from(1)), args, state).await; + let bytes = resp.into_body().collect().await.unwrap().to_bytes(); + let json: Value = serde_json::from_slice(&bytes).unwrap(); + + assert_eq!(json["result"]["isError"], true); + let text = json["result"]["content"][0]["text"].as_str().unwrap(); + assert!(text.contains("No Roblox Studio plugin")); + } + + #[tokio::test] + async fn get_script_rejects_when_command_in_progress() { + let state = Arc::new(McpState::new()); + state.command_in_progress.store(true, Ordering::SeqCst); + state.plugin_stream_connected.store(true, Ordering::SeqCst); + + let args = serde_json::json!({"id": "aabb"}); + let resp = handle_get_script(Some(Value::from(1)), args, state).await; + let bytes = resp.into_body().collect().await.unwrap().to_bytes(); + let json: Value = serde_json::from_slice(&bytes).unwrap(); + + assert_eq!(json["result"]["isError"], true); + let text = json["result"]["content"][0]["text"].as_str().unwrap(); + assert!(text.contains("already in progress")); + } + + #[tokio::test] + async fn get_script_requires_id_or_fspath() { + let state = Arc::new(McpState::new()); + state.plugin_stream_connected.store(true, Ordering::SeqCst); + + let args = serde_json::json!({"fromDraft": true}); + let resp = handle_get_script(Some(Value::from(1)), args, state).await; + let bytes = resp.into_body().collect().await.unwrap().to_bytes(); + let json: Value = serde_json::from_slice(&bytes).unwrap(); + + assert_eq!(json["result"]["isError"], true); + let text = json["result"]["content"][0]["text"].as_str().unwrap(); + assert!(text.contains("id") && text.contains("fsPath")); + } + + #[tokio::test] + async fn get_script_success_includes_json_block() { + let state = Arc::new(McpState::new()); + state.plugin_stream_connected.store(true, Ordering::SeqCst); + + let state2 = Arc::clone(&state); + tokio::spawn(async move { + let mut rx = state2.command_rx.clone(); + rx.changed().await.unwrap(); + let cmd = rx.borrow().clone().unwrap(); + let req_id = cmd["requestId"].as_str().unwrap_or(""); + + let result = serde_json::json!({ + "requestId": req_id, + "status": "success", + "source": "print('hello')", + "studioHash": "abc123def456", + "className": "Script", + "instancePath": "ServerScriptService/Main", + "isDraft": false, + }); + + let tx = state2.result_tx.lock().unwrap().take().unwrap(); + tx.send(result).unwrap(); + }); + + let args = serde_json::json!({"id": "00aabb"}); + let resp = handle_get_script(Some(Value::from(1)), args, state).await; + let bytes = resp.into_body().collect().await.unwrap().to_bytes(); + let json: Value = serde_json::from_slice(&bytes).unwrap(); + + assert_eq!(json["result"]["isError"], false); + let text = json["result"]["content"][0]["text"].as_str().unwrap(); + assert!(text.contains("ServerScriptService/Main")); + assert!(text.contains("abc123def456")); + assert!(text.contains("")); + assert!(text.contains("")); + + let json_start = text.find("\n").unwrap() + 7; + let json_end = text.find("\n").unwrap(); + let inner: Value = serde_json::from_str(&text[json_start..json_end]).unwrap(); + assert_eq!(inner["status"], "success"); + assert_eq!(inner["source"], "print('hello')"); + } + + #[tokio::test] + async fn get_script_error_from_plugin() { + let state = Arc::new(McpState::new()); + state.plugin_stream_connected.store(true, Ordering::SeqCst); + + let state2 = Arc::clone(&state); + tokio::spawn(async move { + let mut rx = state2.command_rx.clone(); + rx.changed().await.unwrap(); + let cmd = rx.borrow().clone().unwrap(); + let req_id = cmd["requestId"].as_str().unwrap_or(""); + + let result = serde_json::json!({ + "requestId": req_id, + "status": "error", + "message": "Instance not found by id.", + }); + + let tx = state2.result_tx.lock().unwrap().take().unwrap(); + tx.send(result).unwrap(); + }); + + let args = serde_json::json!({"id": "deadbeef"}); + let resp = handle_get_script(Some(Value::from(1)), args, state).await; + let bytes = resp.into_body().collect().await.unwrap().to_bytes(); + let json: Value = serde_json::from_slice(&bytes).unwrap(); + + assert_eq!(json["result"]["isError"], true); + let text = json["result"]["content"][0]["text"].as_str().unwrap(); + assert!(text.contains("Instance not found by id")); + } + } } diff --git a/src/web/mod.rs b/src/web/mod.rs index 884f5de8..790780d9 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -64,7 +64,7 @@ impl SyncbackSignal { pub struct LiveServer { serve_session: Arc, syncback_signal: Arc, - mcp_state: Arc, + mcp_state: Arc, active_api_connections: Arc, } @@ -73,7 +73,7 @@ impl LiveServer { LiveServer { serve_session, syncback_signal: Arc::new(SyncbackSignal::new()), - mcp_state: Arc::new(mcp::McpSyncState::new()), + mcp_state: Arc::new(mcp::McpState::new()), active_api_connections: Arc::new(AtomicUsize::new(0)), } }