diff --git a/TODO.txt b/TODO.txt index f011c141..c3a83af5 100644 --- a/TODO.txt +++ b/TODO.txt @@ -351,6 +351,8 @@ can we allow the terminal to stay so i can like run some git commands myself wit - 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 * add logging for all agent connect/disconnects in the terminal - 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 [sync] right now sync is running REALLY slow diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua index 1c581b26..98c3d7a0 100644 --- a/plugin/src/App/init.lua +++ b/plugin/src/App/init.lua @@ -49,6 +49,12 @@ local Theme = require(script.Theme) local Http = require(Packages.Http) local encodeService = require(Plugin.ChangeBatcher.encodeService) +local encodeProperty = require(Plugin.ChangeBatcher.encodeProperty) +local SHA1 = require(Plugin.SHA1) +local RbxDom = require(Packages.RbxDom) +local decodeValue = require(Plugin.Reconciler.decodeValue) +local getProperty = require(Plugin.Reconciler.getProperty) +local trueEquals = require(Plugin.Reconciler.trueEquals) local Page = require(script.Page) local Notifications = require(script.Components.Notifications) @@ -202,8 +208,8 @@ function App:init() if RunService:IsEdit() then self.mcpStream = McpStream.new({ - onSyncCommand = function(requestId) - return self:startMcpSync(requestId) + onSyncCommand = function(requestId, mode, overrides) + return self:startMcpSync(requestId, mode, overrides) end, getPluginConfig = function(key) return Settings:get(key) @@ -1117,7 +1123,10 @@ function App:endSession() Log.trace("Session terminated by user") end -function App:startMcpSync(requestId) +function App:startMcpSync(requestId, mode, overrides) + mode = mode or "standard" + overrides = overrides or {} + return Promise.new(function(resolve, _reject) if self.state.appStatus == AppStatus.Connected then resolve({ @@ -1192,13 +1201,71 @@ function App:startMcpSync(requestId) local patchTree = PatchTree.build(patch, instanceMap, { "Property", "Current", "Incoming" }, gitMetadata) mcpPatchTree = patchTree - local allPreSelected = true + -- Apply overrides: look up each override by id in the tree, verify, set selection + local overrideSelections = {} + for _, override in overrides do + local node = patchTree:getNode(override.id) + if not node or not node.patchType then + Log.trace("MCP sync: override id {} not found in patch tree, skipping", override.id) + continue + end + + local verified = true + + if override.studioHash and node.instance and node.instance:IsA("LuaSourceContainer") then + local source = node.instance.Source + local gitBlob = "blob " .. tostring(#source) .. "\0" .. source + local currentHash = SHA1(buffer.fromstring(gitBlob)) + if currentHash ~= override.studioHash then + Log.info( + "MCP sync: override {} studioHash mismatch (expected={}, got={})", + override.id, + override.studioHash, + currentHash + ) + verified = false + end + end + + if verified and override.expectedProperties and node.instance then + for propName, expectedEncoded in override.expectedProperties do + local decodeOk, expectedValue = decodeValue(expectedEncoded, instanceMap) + if not decodeOk then + Log.info( + "MCP sync: override {} could not decode expected value for {}", + override.id, + propName + ) + verified = false + break + end + local readOk, currentValue = getProperty(node.instance, propName) + if not readOk then + Log.info("MCP sync: override {} could not read property {}", override.id, propName) + verified = false + break + end + if not trueEquals(expectedValue, currentValue) then + Log.info("MCP sync: override {} property {} mismatch", override.id, propName) + verified = false + break + end + end + end + + if verified then + overrideSelections[override.id] = override.direction + end + end + + -- Compute resolution state: check defaults + overrides + local allResolved = true local hasSelectableNodes = false patchTree:forEach(function(node) if node.patchType then hasSelectableNodes = true - if node.defaultSelection == nil then - allPreSelected = false + if node.defaultSelection == nil and overrideSelections[node.id] == nil then + allResolved = false end end end) @@ -1212,14 +1279,55 @@ function App:startMcpSync(requestId) return "Accept" end + -- DRYRUN: return full change list without applying + if mode == "dryrun" then + Log.info("MCP sync: dryrun mode, returning changes without applying") + local changes = self:_buildMcpChangeList(patchTree, patch, instanceMap, nil, true) + resolveOnce({ + status = "dryrun", + changes = changes, + }) + return "Abort" + end + + -- FASTFAIL: if unresolved changes remain, fail immediately + if mode == "fastfail" and not allResolved then + Log.info("MCP sync: fastfail mode, unresolved changes exist") + local changes = self:_buildMcpChangeList(patchTree, patch, instanceMap, nil, true) + resolveOnce({ + status = "fastfail_unresolved", + changes = changes, + message = "Unresolved changes exist. Provide overrides or use standard mode for user review.", + }) + return "Abort" + end + + -- Merge overrides into default selections + local selections = PatchTree.buildInitialSelections(patchTree) + for nodeId, direction in overrideSelections do + selections[nodeId] = direction + end + + -- Check if all nodes are now resolved (defaults + overrides) + local allPreSelected = true + patchTree:forEach(function(node) + if node.patchType and selections[node.id] == nil then + allPreSelected = false + end + end) + + -- MANUAL: always show UI + if mode == "manual" then + allPreSelected = false + end + if allPreSelected then - Log.info("MCP sync: all changes pre-selected, fast-forwarding") - local selections = PatchTree.buildInitialSelections(patchTree) - local changes = self:_buildMcpChangeList(patchTree, selections) + Log.info("MCP sync: all changes resolved, fast-forwarding") + local changes = self:_buildMcpChangeList(patchTree, patch, instanceMap, selections, false) local autoSelectedIds = {} patchTree:forEach(function(node) - if node.patchType and node.defaultSelection and node.defaultSelection ~= "ignore" then + if node.patchType and selections[node.id] then autoSelectedIds[node.id] = true end end) @@ -1254,7 +1362,7 @@ function App:startMcpSync(requestId) local result = self.confirmationEvent:Wait() if result == "Abort" then - local changes = self:_buildMcpChangeList(patchTree, nil) + local changes = self:_buildMcpChangeList(patchTree, patch, instanceMap, nil, true) resolveOnce({ status = "rejected", changes = changes, @@ -1264,7 +1372,7 @@ function App:startMcpSync(requestId) end if type(result) == "table" and result.type == "Confirm" then - local changes = self:_buildMcpChangeList(patchTree, result.selections) + local changes = self:_buildMcpChangeList(patchTree, patch, instanceMap, result.selections, false) resolveOnce({ status = "success", changes = changes, @@ -1321,16 +1429,36 @@ function App:startMcpSync(requestId) end) end -function App:_buildMcpChangeList(patchTree, selections) +local SCRIPT_CLASS_SET = { + Script = true, + LocalScript = true, + ModuleScript = true, +} + +function App:_buildMcpChangeList(patchTree, patch, instanceMap, selections, includeAll) local changes = {} + local patchUpdatedLookup = nil + if patch and patch.updated then + patchUpdatedLookup = {} + for _, change in patch.updated do + patchUpdatedLookup[change.id] = change + end + end + patchTree:forEach(function(node) if not node.patchType then return end local direction = nil - if selections then + if includeAll then + if selections and selections[node.id] then + direction = selections[node.id] + elseif node.defaultSelection then + direction = node.defaultSelection + end + elseif selections then local sel = selections[node.id] if sel == "push" then direction = "push" @@ -1355,10 +1483,56 @@ function App:_buildMcpChangeList(patchTree, selections) end local path = table.concat(segments, "/") - table.insert(changes, { + + local entry: any = { path = path, - direction = direction, - }) + direction = direction or "unresolved", + id = node.id, + className = node.className, + patchType = node.patchType, + defaultSelection = node.defaultSelection, + } + + if node.instance and node.instance:IsA("LuaSourceContainer") then + local source = node.instance.Source + local gitBlob = "blob " .. tostring(#source) .. "\0" .. source + entry.studioHash = SHA1(buffer.fromstring(gitBlob)) + end + + if node.patchType == "Edit" and patchUpdatedLookup and node.instance then + local patchChange = patchUpdatedLookup[node.id] + if patchChange and patchChange.changedProperties then + local props = {} + local isScript = SCRIPT_CLASS_SET[node.className] + + for propName, incomingEncoded in patchChange.changedProperties do + if isScript and propName == "Source" then + continue + end + + local descriptor = RbxDom.findCanonicalPropertyDescriptor(node.className, propName) + if not descriptor then + continue + end + + local currentOk, currentEncoded = encodeProperty(node.instance, propName, descriptor) + if not currentOk then + continue + end + + props[propName] = { + current = currentEncoded, + incoming = incomingEncoded, + } + end + + if next(props) then + entry.properties = props + end + end + end + + table.insert(changes, entry) end) return changes diff --git a/plugin/src/McpStream.lua b/plugin/src/McpStream.lua index 614c7cc3..1a446bbd 100644 --- a/plugin/src/McpStream.lua +++ b/plugin/src/McpStream.lua @@ -94,9 +94,13 @@ function McpStream:_tryConnect() end if data.type == "sync" and data.requestId then - Log.info("MCP stream: received sync command (requestId={})", data.requestId) + Log.info( + "MCP stream: received sync command (requestId={}, mode={})", + data.requestId, + data.mode or "standard" + ) - local resultPromise = self._onSyncCommand(data.requestId) + local resultPromise = self._onSyncCommand(data.requestId, data.mode or "standard", data.overrides or {}) resultPromise :andThen(function(result) diff --git a/src/web/api.rs b/src/web/api.rs index 3c3d7f6b..8205d7e8 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -3518,6 +3518,42 @@ fn pick_script_path(instance: InstanceWithMeta<'_>) -> Option { .map(|path| path.to_owned()) } +/// Enrich each SyncChange with `fs_path` by looking up the instance Ref in the +/// RojoTree and reading its InstigatingSource path. +fn enrich_sync_changes_with_fs_path( + changes: &mut [super::mcp::SyncChange], + serve_session: &ServeSession, +) { + use crate::snapshot::InstigatingSource; + use std::str::FromStr; + + let tree = serve_session.tree(); + let project_path = serve_session.root_project().folder_location(); + + for change in changes.iter_mut() { + let ref_str = match &change.id { + Some(id) => id.as_str(), + None => continue, + }; + + let r = match rbx_dom_weak::types::Ref::from_str(ref_str) { + Ok(r) => r, + Err(_) => continue, + }; + + let meta = match tree.get_metadata(r) { + Some(m) => m, + None => continue, + }; + + if let Some(InstigatingSource::Path(path)) = &meta.instigating_source { + if let Ok(relative) = path.strip_prefix(project_path) { + change.fs_path = Some(relative.to_string_lossy().into_owned()); + } + } + } +} + // --------------------------------------------------------------------------- // MCP stream WebSocket: plugin connects here to receive sync commands // --------------------------------------------------------------------------- @@ -3701,7 +3737,11 @@ async fn handle_mcp_stream_connection( msg = websocket.next() => { match msg { Some(Ok(Message::Text(text))) => { - if let Ok(result) = serde_json::from_str::(&text) { + if let Ok(mut result) = serde_json::from_str::(&text) { + enrich_sync_changes_with_fs_path( + &mut result.changes, + &serve_session, + ); let mut slot = mcp_state .result_tx .lock() diff --git a/src/web/mcp.rs b/src/web/mcp.rs index 1970162e..ef0981e8 100644 --- a/src/web/mcp.rs +++ b/src/web/mcp.rs @@ -25,6 +25,18 @@ pub struct PluginConfig { pub place_id: Option, } +/// Agent-specified auto-accept directive for a single instance change. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct SyncOverride { + pub id: String, + pub direction: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub studio_hash: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub expected_properties: Option, +} + /// Command sent from the MCP handler to the plugin via the MCP stream WebSocket. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -32,14 +44,32 @@ pub struct McpSyncCommand { #[serde(rename = "type")] pub command_type: String, pub request_id: String, + #[serde(default)] + pub mode: String, + #[serde(default)] + pub overrides: Vec, } /// A single change entry in the sync result. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct SyncChange { pub path: String, pub direction: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub id: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub class_name: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub patch_type: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub studio_hash: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub default_selection: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub fs_path: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub properties: Option, } /// Result sent from the plugin back through the MCP stream WebSocket. @@ -229,11 +259,43 @@ fn handle_tools_list(id: Option) -> Response> { "tools": [ { "name": "atlas_sync", - "description": "Sync filesystem changes to Roblox Studio. Equivalent to clicking the Atlas sync button. If all changes are pre-selected by the git system, they are auto-accepted (fast-forward). Otherwise, the user is prompted to review changes in Studio and the call blocks until they accept or reject.", + "description": "Sync filesystem changes to Roblox Studio. Supports modes: 'standard' (default, auto-accepts if all git-resolved, else shows UI), 'manual' (always shows UI), 'fastfail' (fails immediately if unresolved changes exist), 'dryrun' (returns what would change without applying). Supports overrides to auto-accept specific changes by instance id with hash/property verification.", "inputSchema": { "type": "object", - "properties": {}, - "required": [] + "properties": { + "mode": { + "type": "string", + "enum": ["standard", "manual", "fastfail", "dryrun"], + "description": "Sync mode. 'standard' (default): auto-accept if all changes are git-resolved, otherwise show Studio UI. 'manual': always show Studio UI. 'fastfail': fail immediately if any changes are unresolved. 'dryrun': return what would change without applying." + }, + "overrides": { + "type": "array", + "description": "Auto-accept directives for specific instance changes, matched by id from a previous sync response.", + "items": { + "type": "object", + "properties": { + "id": { + "type": "string", + "description": "Server Ref (32-char hex) from a previous sync response." + }, + "direction": { + "type": "string", + "enum": ["push", "pull"], + "description": "Direction to accept: 'push' (Atlas to Studio) or 'pull' (Studio to Atlas)." + }, + "studioHash": { + "type": "string", + "description": "For scripts: SHA1 of git blob of current Studio Source. Override is rejected if hash doesn't match (concurrent edit detected)." + }, + "expectedProperties": { + "type": "object", + "description": "Map of property name to expected current Studio value (RbxDom encoded). Override is rejected if any value doesn't match." + } + }, + "required": ["id", "direction"] + } + } + } } } ] @@ -259,15 +321,32 @@ async fn handle_tools_call( return json_response(&resp, StatusCode::OK); } - handle_atlas_sync(id, mcp_state, active_api_connections).await + 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 } async fn handle_atlas_sync( id: Option, + arguments: Value, mcp_state: Arc, active_api_connections: Arc, ) -> Response> { - // Check if the plugin is connected via the regular API WebSocket (connected mode). + let mode = arguments + .get("mode") + .and_then(|v| v.as_str()) + .unwrap_or("standard") + .to_string(); + + let overrides: Vec = arguments + .get("overrides") + .and_then(|v| serde_json::from_value(v.clone()).ok()) + .unwrap_or_default(); + if active_api_connections.load(Ordering::Relaxed) > 0 { let config_info = { let config = mcp_state @@ -293,7 +372,6 @@ async fn handle_atlas_sync( ); } - // Check if a sync is already in progress (from another agent invocation). if mcp_state .sync_in_progress .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) @@ -306,7 +384,6 @@ async fn handle_atlas_sync( ); } - // Check if the plugin MCP stream is connected. if !mcp_state.plugin_stream_connected.load(Ordering::Relaxed) { mcp_state.sync_in_progress.store(false, Ordering::SeqCst); return tool_response( @@ -317,7 +394,6 @@ async fn handle_atlas_sync( ); } - // Create a oneshot channel for receiving the result from the plugin. let (result_tx, result_rx) = tokio::sync::oneshot::channel(); { let mut slot = mcp_state @@ -331,15 +407,15 @@ async fn handle_atlas_sync( let command = McpSyncCommand { command_type: "sync".to_string(), request_id: request_id.clone(), + mode, + overrides, }; - // Send the sync command to the plugin via the watch channel. if mcp_state.command_tx.send(Some(command)).is_err() { mcp_state.sync_in_progress.store(false, Ordering::SeqCst); return tool_response(id, true, "Failed to send sync command to the plugin."); } - // Await the plugin's result (blocks until plugin responds or disconnects). let result = match result_rx.await { Ok(result) => result, Err(_) => { @@ -354,15 +430,19 @@ async fn handle_atlas_sync( mcp_state.sync_in_progress.store(false, Ordering::SeqCst); - // Clear the command so the watch channel doesn't re-fire on reconnect. let _ = mcp_state.command_tx.send(None); - let is_error = result.status != "success" && result.status != "empty"; + let is_error = !matches!(result.status.as_str(), "success" | "empty" | "dryrun"); let mut text = match result.status.as_str() { "success" => "Sync completed successfully.".to_string(), "empty" => "No changes to sync. Studio is already up to date.".to_string(), "rejected" => "Sync was rejected by the user.".to_string(), + "dryrun" => "Dry run complete. No changes were applied.".to_string(), + "fastfail_unresolved" => { + "Sync failed: unresolved changes exist that require manual review or overrides." + .to_string() + } "already_connected" => "Atlas is already connected to Studio in live sync mode. \ All changes are syncing automatically." .to_string(), @@ -382,8 +462,31 @@ async fn handle_atlas_sync( }; text.push_str(&format!("\n\n{label}:")); for change in &result.changes { - text.push_str(&format!("\n- [{}] {}", change.direction, change.path)); + let fs_info = change + .fs_path + .as_deref() + .map(|p| format!(", fs={p}")) + .unwrap_or_default(); + let type_info = match (change.patch_type.as_deref(), change.class_name.as_deref()) { + (Some(pt), Some(cn)) => format!(" ({pt}, {cn}{fs_info})"), + (Some(pt), None) => format!(" ({pt}{fs_info})"), + _ if !fs_info.is_empty() => format!(" ({fs_info})"), + _ => String::new(), + }; + text.push_str(&format!( + "\n- [{}] {}{}", + change.direction, change.path, type_info + )); } + + let json_block = serde_json::json!({ + "status": result.status, + "changes": result.changes, + }); + text.push_str(&format!( + "\n\n\n{}\n", + serde_json::to_string(&json_block).unwrap_or_default() + )); } tool_response(id, is_error, &text) @@ -448,10 +551,13 @@ mod tests { let cmd = McpSyncCommand { command_type: "sync".to_string(), request_id: "abc-123".to_string(), + mode: "standard".to_string(), + overrides: vec![], }; let json = serde_json::to_value(&cmd).unwrap(); assert_eq!(json["type"], "sync"); assert_eq!(json["requestId"], "abc-123"); + assert_eq!(json["mode"], "standard"); } #[test] @@ -499,6 +605,7 @@ mod tests { two_way_sync: true, one_shot_sync: false, confirmation_behavior: "Always".to_string(), + place_id: None, }; let json = serde_json::to_value(&cfg).unwrap(); assert_eq!(json["twoWaySync"], true); @@ -549,6 +656,8 @@ mod tests { let cmd = McpSyncCommand { command_type: "sync".to_string(), request_id: "test-1".to_string(), + mode: "standard".to_string(), + overrides: vec![], }; state.command_tx.send(Some(cmd)).unwrap(); @@ -583,6 +692,7 @@ mod tests { two_way_sync: true, one_shot_sync: false, confirmation_behavior: "Initial".to_string(), + place_id: None, }; *state.plugin_config.lock().unwrap() = Some(cfg); @@ -652,12 +762,16 @@ mod tests { mod atlas_sync_guards { use super::*; + fn empty_args() -> Value { + serde_json::json!({}) + } + #[tokio::test] async fn rejects_when_api_connected() { let state = Arc::new(McpSyncState::new()); let conns = Arc::new(AtomicUsize::new(1)); - let resp = handle_atlas_sync(Some(Value::from(1)), state, conns).await; + let resp = handle_atlas_sync(Some(Value::from(1)), empty_args(), state, conns).await; let bytes = resp.into_body().collect().await.unwrap().to_bytes(); let json: Value = serde_json::from_slice(&bytes).unwrap(); @@ -673,10 +787,11 @@ mod tests { two_way_sync: true, one_shot_sync: false, confirmation_behavior: "Always".to_string(), + place_id: None, }); let conns = Arc::new(AtomicUsize::new(1)); - let resp = handle_atlas_sync(Some(Value::from(1)), state, conns).await; + let resp = handle_atlas_sync(Some(Value::from(1)), empty_args(), state, conns).await; let bytes = resp.into_body().collect().await.unwrap().to_bytes(); let json: Value = serde_json::from_slice(&bytes).unwrap(); @@ -691,7 +806,7 @@ mod tests { state.sync_in_progress.store(true, Ordering::SeqCst); let conns = Arc::new(AtomicUsize::new(0)); - let resp = handle_atlas_sync(Some(Value::from(2)), state, conns).await; + let resp = handle_atlas_sync(Some(Value::from(2)), empty_args(), state, conns).await; let bytes = resp.into_body().collect().await.unwrap().to_bytes(); let json: Value = serde_json::from_slice(&bytes).unwrap(); @@ -706,7 +821,8 @@ mod tests { state.plugin_stream_connected.store(false, Ordering::SeqCst); let conns = Arc::new(AtomicUsize::new(0)); - let resp = handle_atlas_sync(Some(Value::from(3)), state.clone(), conns).await; + let resp = + handle_atlas_sync(Some(Value::from(3)), empty_args(), state.clone(), conns).await; let bytes = resp.into_body().collect().await.unwrap().to_bytes(); let json: Value = serde_json::from_slice(&bytes).unwrap(); @@ -736,6 +852,7 @@ mod tests { changes: vec![SyncChange { path: "ServerScriptService/Main.server.luau".to_string(), direction: "push".to_string(), + ..Default::default() }], message: None, }; @@ -744,7 +861,8 @@ mod tests { tx.send(result).unwrap(); }); - let resp = handle_atlas_sync(Some(Value::from(5)), state.clone(), conns).await; + let resp = + handle_atlas_sync(Some(Value::from(5)), empty_args(), state.clone(), conns).await; let bytes = resp.into_body().collect().await.unwrap().to_bytes(); let json: Value = serde_json::from_slice(&bytes).unwrap(); @@ -774,10 +892,12 @@ mod tests { 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() }, ], message: Some("User rejected the sync changes.".to_string()), @@ -787,7 +907,7 @@ mod tests { tx.send(result).unwrap(); }); - let resp = handle_atlas_sync(Some(Value::from(6)), state, conns).await; + let resp = handle_atlas_sync(Some(Value::from(6)), empty_args(), state, conns).await; let bytes = resp.into_body().collect().await.unwrap().to_bytes(); let json: Value = serde_json::from_slice(&bytes).unwrap(); @@ -813,7 +933,8 @@ mod tests { let _tx = state2.result_tx.lock().unwrap().take(); }); - let resp = handle_atlas_sync(Some(Value::from(7)), state.clone(), conns).await; + let resp = + handle_atlas_sync(Some(Value::from(7)), empty_args(), state.clone(), conns).await; let bytes = resp.into_body().collect().await.unwrap().to_bytes(); let json: Value = serde_json::from_slice(&bytes).unwrap(); @@ -845,7 +966,7 @@ mod tests { .unwrap(); }); - let resp = handle_atlas_sync(Some(Value::from(8)), state, conns).await; + let resp = handle_atlas_sync(Some(Value::from(8)), empty_args(), state, conns).await; let bytes = resp.into_body().collect().await.unwrap().to_bytes(); let json: Value = serde_json::from_slice(&bytes).unwrap(); @@ -853,5 +974,423 @@ mod tests { let text = json["result"]["content"][0]["text"].as_str().unwrap(); assert!(text.contains("No changes to sync")); } + + #[tokio::test] + async fn dryrun_status_is_not_an_error() { + let state = Arc::new(McpSyncState::new()); + state.plugin_stream_connected.store(true, Ordering::SeqCst); + let conns = Arc::new(AtomicUsize::new(0)); + + 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 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(); + }); + + let args = serde_json::json!({"mode": "dryrun"}); + let resp = handle_atlas_sync(Some(Value::from(9)), args, state, conns).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("Dry run complete")); + assert!(text.contains("")); + } + + #[tokio::test] + async fn fastfail_unresolved_is_error() { + let state = Arc::new(McpSyncState::new()); + state.plugin_stream_connected.store(true, Ordering::SeqCst); + let conns = Arc::new(AtomicUsize::new(0)); + + 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 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(); + }); + + let args = serde_json::json!({"mode": "fastfail"}); + let resp = handle_atlas_sync(Some(Value::from(10)), args, state, conns).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("unresolved")); + } + + #[tokio::test] + async fn mode_and_overrides_forwarded_to_command() { + let state = Arc::new(McpSyncState::new()); + state.plugin_stream_connected.store(true, Ordering::SeqCst); + let conns = Arc::new(AtomicUsize::new(0)); + + 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(); + + 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")); + + 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(); + }); + + let args = serde_json::json!({ + "mode": "fastfail", + "overrides": [{ + "id": "aabbccdd", + "direction": "push", + "studioHash": "abc123" + }] + }); + let resp = handle_atlas_sync(Some(Value::from(11)), args, state, conns).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); + } + + #[tokio::test] + async fn mode_defaults_to_standard() { + let state = Arc::new(McpSyncState::new()); + state.plugin_stream_connected.store(true, Ordering::SeqCst); + let conns = Arc::new(AtomicUsize::new(0)); + + 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(); + + assert_eq!(cmd.mode, "standard"); + assert!(cmd.overrides.is_empty()); + + 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(); + }); + + let resp = handle_atlas_sync(Some(Value::from(12)), empty_args(), state, conns).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); + } + + #[tokio::test] + async fn enriched_changes_include_json_block() { + let state = Arc::new(McpSyncState::new()); + state.plugin_stream_connected.store(true, Ordering::SeqCst); + let conns = Arc::new(AtomicUsize::new(0)); + + 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 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!({ + "Anchored": { + "current": false, + "incoming": true + } + })), + }], + message: None, + }) + .unwrap(); + }); + + let resp = handle_atlas_sync(Some(Value::from(13)), empty_args(), state, conns).await; + let bytes = resp.into_body().collect().await.unwrap().to_bytes(); + let json: Value = serde_json::from_slice(&bytes).unwrap(); + + let text = json["result"]["content"][0]["text"].as_str().unwrap(); + assert!(text.contains("")); + assert!(text.contains("")); + assert!(text.contains("Edit")); + assert!(text.contains("Part")); + assert!(text.contains("fs=src/workspace/Part.model.json5")); + + 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["changes"][0]["id"], "00aabb"); + assert_eq!(inner["changes"][0]["className"], "Part"); + assert_eq!( + inner["changes"][0]["fsPath"], + "src/workspace/Part.model.json5" + ); + assert_eq!( + inner["changes"][0]["properties"]["Anchored"]["current"], + false + ); + assert_eq!( + inner["changes"][0]["properties"]["Anchored"]["incoming"], + true + ); + } + } + + // -- New wire type tests for added structs -------------------------------- + + mod new_wire_types { + use super::*; + + #[test] + fn sync_override_serializes_camel_case() { + let ov = SyncOverride { + id: "aabbccdd".to_string(), + direction: "push".to_string(), + studio_hash: Some("abc123".to_string()), + expected_properties: Some(serde_json::json!({"Anchored": true})), + }; + let json = serde_json::to_value(&ov).unwrap(); + assert_eq!(json["id"], "aabbccdd"); + assert_eq!(json["direction"], "push"); + assert_eq!(json["studioHash"], "abc123"); + assert_eq!(json["expectedProperties"]["Anchored"], true); + } + + #[test] + fn sync_override_defaults_optional_fields() { + let json = serde_json::json!({ + "id": "aabb", + "direction": "pull" + }); + let ov: SyncOverride = serde_json::from_value(json).unwrap(); + assert_eq!(ov.id, "aabb"); + assert_eq!(ov.direction, "pull"); + assert!(ov.studio_hash.is_none()); + assert!(ov.expected_properties.is_none()); + } + + #[test] + fn sync_override_omits_none_on_serialize() { + let ov = SyncOverride { + id: "aabb".to_string(), + direction: "push".to_string(), + studio_hash: None, + expected_properties: None, + }; + let json = serde_json::to_value(&ov).unwrap(); + assert!(json.get("studioHash").is_none()); + assert!(json.get("expectedProperties").is_none()); + } + + #[test] + fn mcp_sync_command_includes_mode_and_overrides() { + let cmd = McpSyncCommand { + command_type: "sync".to_string(), + request_id: "r1".to_string(), + mode: "dryrun".to_string(), + overrides: vec![SyncOverride { + id: "x".to_string(), + direction: "pull".to_string(), + studio_hash: None, + expected_properties: None, + }], + }; + let json = serde_json::to_value(&cmd).unwrap(); + assert_eq!(json["mode"], "dryrun"); + assert_eq!(json["overrides"][0]["id"], "x"); + } + + #[test] + fn mcp_sync_command_defaults_mode_and_overrides() { + let json = serde_json::json!({ + "type": "sync", + "requestId": "r1" + }); + let cmd: McpSyncCommand = serde_json::from_value(json).unwrap(); + assert_eq!(cmd.mode, ""); + assert!(cmd.overrides.is_empty()); + } + + #[test] + fn sync_change_serializes_all_fields() { + let change = SyncChange { + path: "Workspace/Part".to_string(), + direction: "push".to_string(), + id: Some("aabb".to_string()), + class_name: Some("Part".to_string()), + patch_type: Some("Edit".to_string()), + studio_hash: Some("hash123".to_string()), + default_selection: Some("push".to_string()), + fs_path: Some("src/workspace/Part.model.json5".to_string()), + properties: Some( + serde_json::json!({"Anchored": {"current": false, "incoming": true}}), + ), + }; + let json = serde_json::to_value(&change).unwrap(); + assert_eq!(json["id"], "aabb"); + assert_eq!(json["className"], "Part"); + assert_eq!(json["patchType"], "Edit"); + assert_eq!(json["studioHash"], "hash123"); + assert_eq!(json["defaultSelection"], "push"); + assert_eq!(json["fsPath"], "src/workspace/Part.model.json5"); + assert!(json["properties"]["Anchored"].is_object()); + } + + #[test] + fn sync_change_omits_none_fields() { + let change = SyncChange { + path: "Test".to_string(), + direction: "push".to_string(), + ..Default::default() + }; + let json = serde_json::to_value(&change).unwrap(); + assert!(json.get("id").is_none()); + assert!(json.get("className").is_none()); + assert!(json.get("patchType").is_none()); + assert!(json.get("studioHash").is_none()); + assert!(json.get("defaultSelection").is_none()); + assert!(json.get("fsPath").is_none()); + assert!(json.get("properties").is_none()); + } + + #[test] + fn enriched_sync_result_roundtrip() { + let result = McpSyncResult { + request_id: "r1".to_string(), + status: "dryrun".to_string(), + changes: vec![ + SyncChange { + path: "ServerScriptService/Main".to_string(), + direction: "push".to_string(), + id: Some("00aa".to_string()), + class_name: Some("Script".to_string()), + patch_type: Some("Edit".to_string()), + studio_hash: Some("hashval".to_string()), + default_selection: Some("push".to_string()), + fs_path: None, + properties: None, + }, + SyncChange { + path: "Workspace/Part".to_string(), + direction: "unresolved".to_string(), + id: Some("00bb".to_string()), + class_name: Some("Part".to_string()), + patch_type: Some("Edit".to_string()), + studio_hash: None, + default_selection: None, + fs_path: Some("src/workspace/Part.model.json5".to_string()), + properties: Some( + serde_json::json!({"Position": {"current": [0,0,0], "incoming": [1,2,3]}}), + ), + }, + ], + message: None, + }; + let serialized = serde_json::to_string(&result).unwrap(); + let deserialized: McpSyncResult = serde_json::from_str(&serialized).unwrap(); + assert_eq!(deserialized.changes.len(), 2); + assert_eq!( + deserialized.changes[0].studio_hash.as_deref(), + Some("hashval") + ); + assert_eq!( + deserialized.changes[1].fs_path.as_deref(), + Some("src/workspace/Part.model.json5") + ); + assert!(deserialized.changes[1].properties.is_some()); + } + } + + // -- Input schema tests --------------------------------------------------- + + mod input_schema_tests { + use super::*; + + #[test] + fn tools_list_has_mode_and_overrides_in_schema() { + 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 schema = &json["result"]["tools"][0]["inputSchema"]; + assert!(schema["properties"]["mode"].is_object()); + assert!(schema["properties"]["overrides"].is_object()); + + let mode_enum = schema["properties"]["mode"]["enum"].as_array().unwrap(); + let mode_strs: Vec<&str> = mode_enum.iter().filter_map(|v| v.as_str()).collect(); + assert!(mode_strs.contains(&"standard")); + assert!(mode_strs.contains(&"manual")); + assert!(mode_strs.contains(&"fastfail")); + assert!(mode_strs.contains(&"dryrun")); + + let override_items = &schema["properties"]["overrides"]["items"]; + assert_eq!(override_items["properties"]["id"]["type"], "string"); + assert!(override_items["required"] + .as_array() + .unwrap() + .contains(&Value::from("id"))); + assert!(override_items["required"] + .as_array() + .unwrap() + .contains(&Value::from("direction"))); + } } }