mirror of
https://github.com/rojo-rbx/rojo.git
synced 2026-10-22 21:41:37 +00:00
Enhance MCP stream handling and update TODO list
- Added place ID validation in the MCP stream connection to ensure only whitelisted IDs are accepted, improving security and control over connections. - Updated the PluginConfig structure to include an optional place ID field, allowing for better configuration management. - Refined the TODO list to include new tasks related to hash-based comparisons and terminal command execution, enhancing project tracking. These changes aim to strengthen the MCP integration and improve the overall functionality of the plugin.
This commit is contained in:
parent
0f5cabb22a
commit
581aeaeffa
4 changed files with 77 additions and 23 deletions
23
TODO.txt
23
TODO.txt
|
|
@ -316,6 +316,7 @@ we can utilize the git hashes we also use for auto select, too. we use the same
|
||||||
because have already loaded it, we can detect this.
|
because have already loaded it, we can detect this.
|
||||||
this is an optimization to avoid writing 100,000 files or something on larger projects when file bytes match perfectly, or directories verifiably already exist.
|
this is an optimization to avoid writing 100,000 files or something on larger projects when file bytes match perfectly, or directories verifiably already exist.
|
||||||
make sure this is applied to plugin-based syncback, too.
|
make sure this is applied to plugin-based syncback, too.
|
||||||
|
can we do hash based comparisons, too? it seems most effective for actual file changes.. but im not sure..
|
||||||
|
|
||||||
[fix dedup]
|
[fix dedup]
|
||||||
dedup is broken due to relying on sort order
|
dedup is broken due to relying on sort order
|
||||||
|
|
@ -323,26 +324,8 @@ recreate broken state by forcing random order in all matching dedup systems of t
|
||||||
if it cannot match correctly with randomization, then it is inherently broken
|
if it cannot match correctly with randomization, then it is inherently broken
|
||||||
in all 3 matchers, simply randomize the order of the children - this will break the tests.
|
in all 3 matchers, simply randomize the order of the children - this will break the tests.
|
||||||
|
|
||||||
[mcp]
|
[syncback via extension]
|
||||||
* atlas_sync
|
can we allow the terminal to stay so i can like run some git commands myself without having to make a whole new terminal?
|
||||||
- Basically equiv to git fast-forward merge
|
|
||||||
- Atlas plugin automatically connects (tries every 5s if nothing there, do not spam errors) via https://create.roblox.com/docs/reference/engine/classes/HttpService#CreateWebStreamClient
|
|
||||||
- Atlas serve automatically hosts a http based mcp server (NOT stdio based!)
|
|
||||||
- Decide whether it's easier to make a separate server from the api.rs file, or integrate it - integrating would be cleaner, but im not sure if it would support without a massive amount of changes
|
|
||||||
- When an agent invokes atlas_sync, it invokes Atlas sync from Roblox in the plugin as if the user clicked the sync button
|
|
||||||
- Must be on a mutex, so multiple agents cannot invoke it at the same time. Just refuses if it's currently happening
|
|
||||||
- Also it needs to refuse it if we're actively connected on the regular api layer too, not just the mcp based command.
|
|
||||||
- If we are connected already, respond to the agent with a proper notice. Make sure if we are in connected-mode where all things are syncing automatically all the time, we tell the agent. We need to tell the agent the configuration of the lua plugin probably?
|
|
||||||
- If we are in connected-mode we will need to figure out a way to await syncing to finish to ensure state is good?
|
|
||||||
- Upon the plugin connecting, if there are NO changes (Always/Initial) it should just immediately accept
|
|
||||||
- If all changes are pre-selected, e.g. from our git pre selection system, then we "fast-forward" by clicking accept immediately.
|
|
||||||
- If there are actually changes, the agent should hang while waiting for the user to either accept/deny them
|
|
||||||
- The output of the mcp request needs to conclude that it was a success and what changes were synced AND if it was rejected + what changes were presented to the user to sync.
|
|
||||||
- Let's just give a list of file paths on the actual disk (relative to project so like ReplicatedStorage/SomeFile.luau or ReplicatedStorage/SomeModel.json5 etc) ?
|
|
||||||
- Needs to include the DIRECTION, too.
|
|
||||||
- I'm not sure what is the most value on this, but we absolutely need to give the agent a response on what was actually synced, ideally what the content was(?), and the directions of sync.
|
|
||||||
- Does nothing if we are running in connected-mode (already syncing)
|
|
||||||
- Agent should await the completion of this sync before proceeding, this mcp command needs to yield and give the response as described above.
|
|
||||||
|
|
||||||
[mcp]
|
[mcp]
|
||||||
* syncback
|
* syncback
|
||||||
|
|
|
||||||
|
|
@ -78,6 +78,7 @@ function McpStream:_tryConnect()
|
||||||
twoWaySync = self._getPluginConfig("twoWaySync"),
|
twoWaySync = self._getPluginConfig("twoWaySync"),
|
||||||
oneShotSync = self._getPluginConfig("oneShotSync"),
|
oneShotSync = self._getPluginConfig("oneShotSync"),
|
||||||
confirmationBehavior = self._getPluginConfig("confirmationBehavior"),
|
confirmationBehavior = self._getPluginConfig("confirmationBehavior"),
|
||||||
|
placeId = game.PlaceId,
|
||||||
})
|
})
|
||||||
pcall(function()
|
pcall(function()
|
||||||
wsClient:Send(greeting)
|
wsClient:Send(greeting)
|
||||||
|
|
|
||||||
|
|
@ -137,7 +137,13 @@ pub async fn call(
|
||||||
}
|
}
|
||||||
(&Method::GET, "/api/mcp/stream") => {
|
(&Method::GET, "/api/mcp/stream") => {
|
||||||
if is_upgrade_request(&request) {
|
if is_upgrade_request(&request) {
|
||||||
handle_mcp_stream_upgrade(&mut request, mcp_state, active_api_connections).await
|
handle_mcp_stream_upgrade(
|
||||||
|
&mut request,
|
||||||
|
Arc::clone(&service.serve_session),
|
||||||
|
mcp_state,
|
||||||
|
active_api_connections,
|
||||||
|
)
|
||||||
|
.await
|
||||||
} else {
|
} else {
|
||||||
msgpack(
|
msgpack(
|
||||||
ErrorResponse::bad_request(
|
ErrorResponse::bad_request(
|
||||||
|
|
@ -3518,6 +3524,7 @@ fn pick_script_path(instance: InstanceWithMeta<'_>) -> Option<PathBuf> {
|
||||||
|
|
||||||
async fn handle_mcp_stream_upgrade(
|
async fn handle_mcp_stream_upgrade(
|
||||||
request: &mut Request<Incoming>,
|
request: &mut Request<Incoming>,
|
||||||
|
serve_session: Arc<ServeSession>,
|
||||||
mcp_state: Arc<super::mcp::McpSyncState>,
|
mcp_state: Arc<super::mcp::McpSyncState>,
|
||||||
active_api_connections: Arc<std::sync::atomic::AtomicUsize>,
|
active_api_connections: Arc<std::sync::atomic::AtomicUsize>,
|
||||||
) -> Response<Full<Bytes>> {
|
) -> Response<Full<Bytes>> {
|
||||||
|
|
@ -3541,8 +3548,13 @@ async fn handle_mcp_stream_upgrade(
|
||||||
};
|
};
|
||||||
|
|
||||||
tokio::spawn(async move {
|
tokio::spawn(async move {
|
||||||
if let Err(e) =
|
if let Err(e) = handle_mcp_stream_connection(
|
||||||
handle_mcp_stream_connection(websocket, mcp_state, active_api_connections).await
|
websocket,
|
||||||
|
serve_session,
|
||||||
|
mcp_state,
|
||||||
|
active_api_connections,
|
||||||
|
)
|
||||||
|
.await
|
||||||
{
|
{
|
||||||
log::error!("MCP stream WebSocket error: {e}");
|
log::error!("MCP stream WebSocket error: {e}");
|
||||||
}
|
}
|
||||||
|
|
@ -3553,6 +3565,7 @@ async fn handle_mcp_stream_upgrade(
|
||||||
|
|
||||||
async fn handle_mcp_stream_connection(
|
async fn handle_mcp_stream_connection(
|
||||||
websocket: HyperWebsocket,
|
websocket: HyperWebsocket,
|
||||||
|
serve_session: Arc<ServeSession>,
|
||||||
mcp_state: Arc<super::mcp::McpSyncState>,
|
mcp_state: Arc<super::mcp::McpSyncState>,
|
||||||
active_api_connections: Arc<std::sync::atomic::AtomicUsize>,
|
active_api_connections: Arc<std::sync::atomic::AtomicUsize>,
|
||||||
) -> anyhow::Result<()> {
|
) -> anyhow::Result<()> {
|
||||||
|
|
@ -3577,6 +3590,61 @@ async fn handle_mcp_stream_connection(
|
||||||
if let Ok(greeting) = serde_json::from_str::<serde_json::Value>(&text) {
|
if let Ok(greeting) = serde_json::from_str::<serde_json::Value>(&text) {
|
||||||
if greeting.get("type").and_then(|t| t.as_str()) == Some("hello") {
|
if greeting.get("type").and_then(|t| t.as_str()) == Some("hello") {
|
||||||
if let Ok(config) = serde_json::from_value::<PluginConfig>(greeting.clone()) {
|
if let Ok(config) = serde_json::from_value::<PluginConfig>(greeting.clone()) {
|
||||||
|
// Validate place ID against the project's servePlaceIds / blockedPlaceIds.
|
||||||
|
if let Some(place_id_f64) = config.place_id {
|
||||||
|
let place_id = place_id_f64 as u64;
|
||||||
|
if let Some(expected) = serve_session.serve_place_ids() {
|
||||||
|
if !expected.contains(&place_id) {
|
||||||
|
log::trace!(
|
||||||
|
"MCP stream rejected: place ID {} not in servePlaceIds",
|
||||||
|
place_id
|
||||||
|
);
|
||||||
|
let reject = serde_json::json!({
|
||||||
|
"type": "rejected",
|
||||||
|
"reason": format!(
|
||||||
|
"Place ID {} is not in the servePlaceIds whitelist",
|
||||||
|
place_id
|
||||||
|
),
|
||||||
|
});
|
||||||
|
let _ = websocket
|
||||||
|
.send(Message::Text(
|
||||||
|
serde_json::to_string(&reject).unwrap().into(),
|
||||||
|
))
|
||||||
|
.await;
|
||||||
|
let _ = websocket.send(Message::Close(None)).await;
|
||||||
|
mcp_state
|
||||||
|
.plugin_stream_connected
|
||||||
|
.store(false, Ordering::SeqCst);
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if let Some(blocked) = serve_session.blocked_place_ids() {
|
||||||
|
if blocked.contains(&place_id) {
|
||||||
|
log::trace!(
|
||||||
|
"MCP stream rejected: place ID {} is in blockedPlaceIds",
|
||||||
|
place_id
|
||||||
|
);
|
||||||
|
let reject = serde_json::json!({
|
||||||
|
"type": "rejected",
|
||||||
|
"reason": format!(
|
||||||
|
"Place ID {} is in the blockedPlaceIds list",
|
||||||
|
place_id
|
||||||
|
),
|
||||||
|
});
|
||||||
|
let _ = websocket
|
||||||
|
.send(Message::Text(
|
||||||
|
serde_json::to_string(&reject).unwrap().into(),
|
||||||
|
))
|
||||||
|
.await;
|
||||||
|
let _ = websocket.send(Message::Close(None)).await;
|
||||||
|
mcp_state
|
||||||
|
.plugin_stream_connected
|
||||||
|
.store(false, Ordering::SeqCst);
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
let mut slot = mcp_state
|
let mut slot = mcp_state
|
||||||
.plugin_config
|
.plugin_config
|
||||||
.lock()
|
.lock()
|
||||||
|
|
|
||||||
|
|
@ -21,6 +21,8 @@ pub struct PluginConfig {
|
||||||
pub one_shot_sync: bool,
|
pub one_shot_sync: bool,
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub confirmation_behavior: String,
|
pub confirmation_behavior: String,
|
||||||
|
#[serde(default)]
|
||||||
|
pub place_id: Option<f64>,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// 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.
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue