diff --git a/changelog.d/mcp-write-wait.changed.md b/changelog.d/mcp-write-wait.changed.md new file mode 100644 index 000000000..59453931e --- /dev/null +++ b/changelog.d/mcp-write-wait.changed.md @@ -0,0 +1,9 @@ +- An MCP write tool now waits 30 seconds for its result instead of 60, below the + MCP TypeScript SDK's default 60 second request timeout, so a client on that + default receives the `outcome_unknown` answer instead of timing out first. The + write itself still runs to completion. The server instructions now say that a + missing response is also an unknown outcome and how to reconcile it, and that + `graphs` needs a `graph_list` permit for a credential that is not an identity. + See [MCP][mcp-write-wait-guide]. + +[mcp-write-wait-guide]: ../docs/user/operations/mcp.md diff --git a/crates/omnigraph-server/src/mcp.rs b/crates/omnigraph-server/src/mcp.rs index 6877b88a3..0bfeac3ab 100644 --- a/crates/omnigraph-server/src/mcp.rs +++ b/crates/omnigraph-server/src/mcp.rs @@ -38,8 +38,13 @@ const RESULT_BYTES: usize = 1024 * 1024; const READ_DEADLINE: Duration = Duration::from_secs(30); /// How long a caller waits for a write. The write itself is owned by the /// server and continues past this; the caller then learns its outcome is -/// unknown. -const WRITE_DEADLINE: Duration = Duration::from_secs(60); +/// unknown. It stays below common client request timeouts (the MCP +/// TypeScript SDK defaults to 60 seconds), so a client on such a default +/// receives that answer instead of giving up first. +const WRITE_DEADLINE: Duration = READ_DEADLINE; +/// The MCP TypeScript SDK's default request timeout. +#[cfg(test)] +const COMMON_CLIENT_TIMEOUT: Duration = Duration::from_secs(60); const RESOURCE_METADATA_PATH: &str = "/.well-known/oauth-protected-resource"; /// Environment equivalent of `--mcp-allow-adhoc`. @@ -328,7 +333,7 @@ impl Refusal { /// the write completed: the outcome is unknown and must be reconciled, never /// replayed blindly. async fn tool_result(self, write: bool) -> CallToolResult { - let unknown = "The write keeps running and its outcome is unknown. Read the target and reconcile before replaying it."; + let unknown = "The write keeps running and its outcome is unknown. Before replaying it, read the target (`read` or `snapshot`, comparing `graph_commit_id`) or list the branch with `commits_list`, and replay a `mutate` with `if_graph_commit`."; match self { Refusal::Unauthenticated => failure( "unauthenticated", @@ -1398,7 +1403,7 @@ fn spec(settings: &McpSettings, name: &str) -> Option<&'static ToolSpec> { fn instructions(settings: &McpSettings) -> String { let mut text = String::from( "OmniGraph serves typed property graphs. Every call runs under the applied policy for your credential; a listed tool or graph can still be refused.\n\ - 1. Call `graphs` for graph ids.\n\ + 1. Call `graphs` for graph ids. With a credential that is not an identity, it needs a `graph_list` permit, which a server without a cluster policy refuses; then ask the user for the graph id.\n\ 2. Call `schema` for the graph (or read `omnigraph://graphs/{graph}/schema`) before writing a query: it declares the types, properties, keys and edges a query may name.\n\ 3. Prefer a stored read: `queries` lists them with typed parameters, and `query` runs one.\n", ); @@ -1409,7 +1414,7 @@ fn instructions(settings: &McpSettings) -> String { } if settings.allow_writes { text.push_str( - "Writes: `mutate`, `load` and the `branches_*` tools each publish at most one commit. For read-modify-write, pass your read's `graph_commit_id` to `mutate` as `if_graph_commit`; a 412 means nothing was written. The server never retries a write. A result with `outcome_unknown` means the write may or may not have happened: read the target and reconcile before replaying. Schema and deployment changes are operator-owned and not available here.\n", + "Writes: `mutate`, `load` and the `branches_*` tools each publish at most one commit. For read-modify-write, pass your read's `graph_commit_id` to `mutate` as `if_graph_commit`; a 412 means nothing was written. The server never retries a write. A result with `outcome_unknown`, or no response at all (a client timeout, a cancellation or a dropped connection), means the write may or may not have happened: read the target or list its commits and reconcile before replaying, and replay a `mutate` with `if_graph_commit`. Schema and deployment changes are operator-owned and not available here.\n", ); } text.push_str( @@ -1889,6 +1894,9 @@ mod tests { #[tokio::test] async fn a_write_whose_wait_ends_first_reports_an_unknown_outcome() { + // The server's answer must arrive before a client on a common default + // timeout gives up and never sees `outcome_unknown`. + assert!(WRITE_DEADLINE < COMMON_CLIENT_TIMEOUT); for refusal in [Refusal::Cancelled, Refusal::Deadline] { let result = refusal.tool_result(true).await; assert_eq!(result.is_error, Some(true)); diff --git a/docs/user/operations/mcp.md b/docs/user/operations/mcp.md index 5aeca433e..f97039544 100644 --- a/docs/user/operations/mcp.md +++ b/docs/user/operations/mcp.md @@ -93,6 +93,11 @@ for a deployment whose policy was written with an LLM caller in mind. | `commits_changes` | `graph`, `commit_id`, optional `kind`, `type`, `op`, `page_token`, `limit` | `GET /graphs/{graph}/commits/{commit_id}/changes` | `read` | | `changes_poll` | `graph`, optional `branch`, `cursor`, `start`, `kind`, `type`, `op`, `page_token`, `limit` | `GET /graphs/{graph}/changes` | `read` | +`graph_list` is a server-scoped action, and a server without an applied cluster +policy denies every server-scoped action. So with a static or other non-identity +token, `graphs` returns 403 until the cluster policy permits `graph_list`; the +other tools still work with a graph id the user supplies. + The server's `initialize` instructions tell a model to call `graphs`, then `schema`, then a stored read with `query` or, when enabled, its own GQ read with `read`. @@ -121,10 +126,16 @@ Write tools follow one contract: `if_graph_commit`. If the branch has moved, the call fails with 412 and nothing is written. - **No retries.** The server never replays a tool call. -- **Unknown outcomes are reported.** A write runs to completion even when the - caller stops waiting. A write still running after 60 seconds, a cancelled write, - or a write whose completion the server cannot confirm returns `isError` with - `"outcome_unknown": true`. Read the target and reconcile before replaying it. +- **A lost response is an unknown outcome.** A write runs to completion even when + the caller stops waiting. A write still running after 30 seconds, or one whose + completion the server cannot confirm, returns `isError` with + `"outcome_unknown": true`. A write the client cancelled, timed out or lost the + connection for returns nothing, and means the same. Before replaying it, read + the target with `read` or `snapshot` and compare `graph_commit_id`, or list the + branch with `commits_list`; replay a `mutate` with `if_graph_commit`, so a write + that did land fails with 412 instead of applying twice. The 30 second wait is + below the MCP TypeScript SDK's default 60 second request timeout; a client with + a shorter timeout loses the answer. - **Size.** The whole call, including `load` data, fits in the 64 KiB request. Use `POST /load/ndjson` for larger loads. @@ -166,7 +177,7 @@ A write adds `"outcome_unknown": true` to either shape when it may have happened | Request body | 64 KiB | | Complete tool result, including the text copy | 1 MiB | | Wait for one read | 30 seconds | -| Wait for one write | 60 seconds; the write continues past it | +| Wait for one write | 30 seconds; the write continues past it | | Concurrent tool executions | 16 | A cancelled or expired call stops the caller's wait. The execution keeps its