Enhance MCP sync functionality and improve documentation

- Updated the TODO list to include a task for ensuring well-formed MCP route documentation, enhancing agent usability.
- Modified the MCP stream handling to support additional parameters for sync commands, including mode and overrides for better control during sync operations.
- Introduced a new function to enrich sync changes with filesystem paths, improving traceability of changes.
- Updated API documentation to reflect new sync modes and override capabilities, providing clearer guidance for users.

These changes aim to improve the overall functionality and user experience of the MCP integration and sync process.
This commit is contained in:
Joe 2026-03-01 09:19:41 -06:00
parent 5d86b1ebdf
commit 9aa3145b06
5 changed files with 801 additions and 42 deletions

View file

@ -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 - 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 * 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. - 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] [sync]
right now sync is running REALLY slow right now sync is running REALLY slow

View file

@ -49,6 +49,12 @@ local Theme = require(script.Theme)
local Http = require(Packages.Http) local Http = require(Packages.Http)
local encodeService = require(Plugin.ChangeBatcher.encodeService) 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 Page = require(script.Page)
local Notifications = require(script.Components.Notifications) local Notifications = require(script.Components.Notifications)
@ -202,8 +208,8 @@ function App:init()
if RunService:IsEdit() then if RunService:IsEdit() then
self.mcpStream = McpStream.new({ self.mcpStream = McpStream.new({
onSyncCommand = function(requestId) onSyncCommand = function(requestId, mode, overrides)
return self:startMcpSync(requestId) return self:startMcpSync(requestId, mode, overrides)
end, end,
getPluginConfig = function(key) getPluginConfig = function(key)
return Settings:get(key) return Settings:get(key)
@ -1117,7 +1123,10 @@ function App:endSession()
Log.trace("Session terminated by user") Log.trace("Session terminated by user")
end end
function App:startMcpSync(requestId) function App:startMcpSync(requestId, mode, overrides)
mode = mode or "standard"
overrides = overrides or {}
return Promise.new(function(resolve, _reject) return Promise.new(function(resolve, _reject)
if self.state.appStatus == AppStatus.Connected then if self.state.appStatus == AppStatus.Connected then
resolve({ resolve({
@ -1192,13 +1201,71 @@ function App:startMcpSync(requestId)
local patchTree = PatchTree.build(patch, instanceMap, { "Property", "Current", "Incoming" }, gitMetadata) local patchTree = PatchTree.build(patch, instanceMap, { "Property", "Current", "Incoming" }, gitMetadata)
mcpPatchTree = patchTree 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 local hasSelectableNodes = false
patchTree:forEach(function(node) patchTree:forEach(function(node)
if node.patchType then if node.patchType then
hasSelectableNodes = true hasSelectableNodes = true
if node.defaultSelection == nil then if node.defaultSelection == nil and overrideSelections[node.id] == nil then
allPreSelected = false allResolved = false
end end
end end
end) end)
@ -1212,14 +1279,55 @@ function App:startMcpSync(requestId)
return "Accept" return "Accept"
end 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 if allPreSelected then
Log.info("MCP sync: all changes pre-selected, fast-forwarding") Log.info("MCP sync: all changes resolved, fast-forwarding")
local selections = PatchTree.buildInitialSelections(patchTree) local changes = self:_buildMcpChangeList(patchTree, patch, instanceMap, selections, false)
local changes = self:_buildMcpChangeList(patchTree, selections)
local autoSelectedIds = {} local autoSelectedIds = {}
patchTree:forEach(function(node) 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 autoSelectedIds[node.id] = true
end end
end) end)
@ -1254,7 +1362,7 @@ function App:startMcpSync(requestId)
local result = self.confirmationEvent:Wait() local result = self.confirmationEvent:Wait()
if result == "Abort" then if result == "Abort" then
local changes = self:_buildMcpChangeList(patchTree, nil) local changes = self:_buildMcpChangeList(patchTree, patch, instanceMap, nil, true)
resolveOnce({ resolveOnce({
status = "rejected", status = "rejected",
changes = changes, changes = changes,
@ -1264,7 +1372,7 @@ function App:startMcpSync(requestId)
end end
if type(result) == "table" and result.type == "Confirm" then 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({ resolveOnce({
status = "success", status = "success",
changes = changes, changes = changes,
@ -1321,16 +1429,36 @@ function App:startMcpSync(requestId)
end) end)
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 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) patchTree:forEach(function(node)
if not node.patchType then if not node.patchType then
return return
end end
local direction = nil 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] local sel = selections[node.id]
if sel == "push" then if sel == "push" then
direction = "push" direction = "push"
@ -1355,10 +1483,56 @@ function App:_buildMcpChangeList(patchTree, selections)
end end
local path = table.concat(segments, "/") local path = table.concat(segments, "/")
table.insert(changes, {
local entry: any = {
path = path, 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) end)
return changes return changes

View file

@ -94,9 +94,13 @@ function McpStream:_tryConnect()
end end
if data.type == "sync" and data.requestId then 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 resultPromise
:andThen(function(result) :andThen(function(result)

View file

@ -3518,6 +3518,42 @@ fn pick_script_path(instance: InstanceWithMeta<'_>) -> Option<PathBuf> {
.map(|path| path.to_owned()) .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 // MCP stream WebSocket: plugin connects here to receive sync commands
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
@ -3701,7 +3737,11 @@ async fn handle_mcp_stream_connection(
msg = websocket.next() => { msg = websocket.next() => {
match msg { match msg {
Some(Ok(Message::Text(text))) => { Some(Ok(Message::Text(text))) => {
if let Ok(result) = serde_json::from_str::<McpSyncResult>(&text) { if let Ok(mut result) = serde_json::from_str::<McpSyncResult>(&text) {
enrich_sync_changes_with_fs_path(
&mut result.changes,
&serve_session,
);
let mut slot = mcp_state let mut slot = mcp_state
.result_tx .result_tx
.lock() .lock()

View file

@ -25,6 +25,18 @@ pub struct PluginConfig {
pub place_id: Option<f64>, pub place_id: Option<f64>,
} }
/// 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<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub expected_properties: Option<Value>,
}
/// Command sent from the MCP handler to the plugin via the MCP stream WebSocket. /// Command sent from the MCP handler to the plugin via the MCP stream WebSocket.
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")] #[serde(rename_all = "camelCase")]
@ -32,14 +44,32 @@ pub struct McpSyncCommand {
#[serde(rename = "type")] #[serde(rename = "type")]
pub command_type: String, pub command_type: String,
pub request_id: String, pub request_id: String,
#[serde(default)]
pub mode: String,
#[serde(default)]
pub overrides: Vec<SyncOverride>,
} }
/// A single change entry in the sync result. /// A single change entry in the sync result.
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone, Default, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")] #[serde(rename_all = "camelCase")]
pub struct SyncChange { pub struct SyncChange {
pub path: String, pub path: String,
pub direction: String, pub direction: String,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub id: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub class_name: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub patch_type: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub studio_hash: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub default_selection: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub fs_path: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub properties: Option<Value>,
} }
/// Result sent from the plugin back through the MCP stream WebSocket. /// Result sent from the plugin back through the MCP stream WebSocket.
@ -229,11 +259,43 @@ fn handle_tools_list(id: Option<Value>) -> Response<Full<Bytes>> {
"tools": [ "tools": [
{ {
"name": "atlas_sync", "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": { "inputSchema": {
"type": "object", "type": "object",
"properties": {}, "properties": {
"required": [] "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); 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( async fn handle_atlas_sync(
id: Option<Value>, id: Option<Value>,
arguments: Value,
mcp_state: Arc<McpSyncState>, mcp_state: Arc<McpSyncState>,
active_api_connections: Arc<std::sync::atomic::AtomicUsize>, active_api_connections: Arc<std::sync::atomic::AtomicUsize>,
) -> Response<Full<Bytes>> { ) -> Response<Full<Bytes>> {
// 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<SyncOverride> = arguments
.get("overrides")
.and_then(|v| serde_json::from_value(v.clone()).ok())
.unwrap_or_default();
if active_api_connections.load(Ordering::Relaxed) > 0 { if active_api_connections.load(Ordering::Relaxed) > 0 {
let config_info = { let config_info = {
let config = mcp_state 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 if mcp_state
.sync_in_progress .sync_in_progress
.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) .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) { if !mcp_state.plugin_stream_connected.load(Ordering::Relaxed) {
mcp_state.sync_in_progress.store(false, Ordering::SeqCst); mcp_state.sync_in_progress.store(false, Ordering::SeqCst);
return tool_response( 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 (result_tx, result_rx) = tokio::sync::oneshot::channel();
{ {
let mut slot = mcp_state let mut slot = mcp_state
@ -331,15 +407,15 @@ async fn handle_atlas_sync(
let command = McpSyncCommand { let command = McpSyncCommand {
command_type: "sync".to_string(), command_type: "sync".to_string(),
request_id: request_id.clone(), 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() { if mcp_state.command_tx.send(Some(command)).is_err() {
mcp_state.sync_in_progress.store(false, Ordering::SeqCst); mcp_state.sync_in_progress.store(false, Ordering::SeqCst);
return tool_response(id, true, "Failed to send sync command to the plugin."); 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 { let result = match result_rx.await {
Ok(result) => result, Ok(result) => result,
Err(_) => { Err(_) => {
@ -354,15 +430,19 @@ async fn handle_atlas_sync(
mcp_state.sync_in_progress.store(false, Ordering::SeqCst); 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 _ = 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() { let mut text = match result.status.as_str() {
"success" => "Sync completed successfully.".to_string(), "success" => "Sync completed successfully.".to_string(),
"empty" => "No changes to sync. Studio is already up to date.".to_string(), "empty" => "No changes to sync. Studio is already up to date.".to_string(),
"rejected" => "Sync was rejected by the user.".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. \ "already_connected" => "Atlas is already connected to Studio in live sync mode. \
All changes are syncing automatically." All changes are syncing automatically."
.to_string(), .to_string(),
@ -382,8 +462,31 @@ async fn handle_atlas_sync(
}; };
text.push_str(&format!("\n\n{label}:")); text.push_str(&format!("\n\n{label}:"));
for change in &result.changes { 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<json>\n{}\n</json>",
serde_json::to_string(&json_block).unwrap_or_default()
));
} }
tool_response(id, is_error, &text) tool_response(id, is_error, &text)
@ -448,10 +551,13 @@ mod tests {
let cmd = McpSyncCommand { let cmd = McpSyncCommand {
command_type: "sync".to_string(), command_type: "sync".to_string(),
request_id: "abc-123".to_string(), request_id: "abc-123".to_string(),
mode: "standard".to_string(),
overrides: vec![],
}; };
let json = serde_json::to_value(&cmd).unwrap(); let json = serde_json::to_value(&cmd).unwrap();
assert_eq!(json["type"], "sync"); assert_eq!(json["type"], "sync");
assert_eq!(json["requestId"], "abc-123"); assert_eq!(json["requestId"], "abc-123");
assert_eq!(json["mode"], "standard");
} }
#[test] #[test]
@ -499,6 +605,7 @@ mod tests {
two_way_sync: true, two_way_sync: true,
one_shot_sync: false, one_shot_sync: false,
confirmation_behavior: "Always".to_string(), confirmation_behavior: "Always".to_string(),
place_id: None,
}; };
let json = serde_json::to_value(&cfg).unwrap(); let json = serde_json::to_value(&cfg).unwrap();
assert_eq!(json["twoWaySync"], true); assert_eq!(json["twoWaySync"], true);
@ -549,6 +656,8 @@ mod tests {
let cmd = McpSyncCommand { let cmd = McpSyncCommand {
command_type: "sync".to_string(), command_type: "sync".to_string(),
request_id: "test-1".to_string(), request_id: "test-1".to_string(),
mode: "standard".to_string(),
overrides: vec![],
}; };
state.command_tx.send(Some(cmd)).unwrap(); state.command_tx.send(Some(cmd)).unwrap();
@ -583,6 +692,7 @@ mod tests {
two_way_sync: true, two_way_sync: true,
one_shot_sync: false, one_shot_sync: false,
confirmation_behavior: "Initial".to_string(), confirmation_behavior: "Initial".to_string(),
place_id: None,
}; };
*state.plugin_config.lock().unwrap() = Some(cfg); *state.plugin_config.lock().unwrap() = Some(cfg);
@ -652,12 +762,16 @@ mod tests {
mod atlas_sync_guards { mod atlas_sync_guards {
use super::*; use super::*;
fn empty_args() -> Value {
serde_json::json!({})
}
#[tokio::test] #[tokio::test]
async fn rejects_when_api_connected() { async fn rejects_when_api_connected() {
let state = Arc::new(McpSyncState::new()); let state = Arc::new(McpSyncState::new());
let conns = Arc::new(AtomicUsize::new(1)); 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 bytes = resp.into_body().collect().await.unwrap().to_bytes();
let json: Value = serde_json::from_slice(&bytes).unwrap(); let json: Value = serde_json::from_slice(&bytes).unwrap();
@ -673,10 +787,11 @@ mod tests {
two_way_sync: true, two_way_sync: true,
one_shot_sync: false, one_shot_sync: false,
confirmation_behavior: "Always".to_string(), confirmation_behavior: "Always".to_string(),
place_id: None,
}); });
let conns = Arc::new(AtomicUsize::new(1)); 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 bytes = resp.into_body().collect().await.unwrap().to_bytes();
let json: Value = serde_json::from_slice(&bytes).unwrap(); let json: Value = serde_json::from_slice(&bytes).unwrap();
@ -691,7 +806,7 @@ mod tests {
state.sync_in_progress.store(true, Ordering::SeqCst); state.sync_in_progress.store(true, Ordering::SeqCst);
let conns = Arc::new(AtomicUsize::new(0)); 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 bytes = resp.into_body().collect().await.unwrap().to_bytes();
let json: Value = serde_json::from_slice(&bytes).unwrap(); let json: Value = serde_json::from_slice(&bytes).unwrap();
@ -706,7 +821,8 @@ mod tests {
state.plugin_stream_connected.store(false, Ordering::SeqCst); state.plugin_stream_connected.store(false, Ordering::SeqCst);
let conns = Arc::new(AtomicUsize::new(0)); 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 bytes = resp.into_body().collect().await.unwrap().to_bytes();
let json: Value = serde_json::from_slice(&bytes).unwrap(); let json: Value = serde_json::from_slice(&bytes).unwrap();
@ -736,6 +852,7 @@ mod tests {
changes: vec![SyncChange { changes: vec![SyncChange {
path: "ServerScriptService/Main.server.luau".to_string(), path: "ServerScriptService/Main.server.luau".to_string(),
direction: "push".to_string(), direction: "push".to_string(),
..Default::default()
}], }],
message: None, message: None,
}; };
@ -744,7 +861,8 @@ mod tests {
tx.send(result).unwrap(); 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 bytes = resp.into_body().collect().await.unwrap().to_bytes();
let json: Value = serde_json::from_slice(&bytes).unwrap(); let json: Value = serde_json::from_slice(&bytes).unwrap();
@ -774,10 +892,12 @@ mod tests {
SyncChange { SyncChange {
path: "ReplicatedStorage/Foo.luau".to_string(), path: "ReplicatedStorage/Foo.luau".to_string(),
direction: "push".to_string(), direction: "push".to_string(),
..Default::default()
}, },
SyncChange { SyncChange {
path: "Workspace/Bar.model.json5".to_string(), path: "Workspace/Bar.model.json5".to_string(),
direction: "pull".to_string(), direction: "pull".to_string(),
..Default::default()
}, },
], ],
message: Some("User rejected the sync changes.".to_string()), message: Some("User rejected the sync changes.".to_string()),
@ -787,7 +907,7 @@ mod tests {
tx.send(result).unwrap(); 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 bytes = resp.into_body().collect().await.unwrap().to_bytes();
let json: Value = serde_json::from_slice(&bytes).unwrap(); let json: Value = serde_json::from_slice(&bytes).unwrap();
@ -813,7 +933,8 @@ mod tests {
let _tx = state2.result_tx.lock().unwrap().take(); 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 bytes = resp.into_body().collect().await.unwrap().to_bytes();
let json: Value = serde_json::from_slice(&bytes).unwrap(); let json: Value = serde_json::from_slice(&bytes).unwrap();
@ -845,7 +966,7 @@ mod tests {
.unwrap(); .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 bytes = resp.into_body().collect().await.unwrap().to_bytes();
let json: Value = serde_json::from_slice(&bytes).unwrap(); 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(); let text = json["result"]["content"][0]["text"].as_str().unwrap();
assert!(text.contains("No changes to sync")); 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("<json>"));
}
#[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("<json>"));
assert!(text.contains("</json>"));
assert!(text.contains("Edit"));
assert!(text.contains("Part"));
assert!(text.contains("fs=src/workspace/Part.model.json5"));
let json_start = text.find("<json>\n").unwrap() + 7;
let json_end = text.find("\n</json>").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")));
}
} }
} }