From 7604916e9c85b2492cfbcf36bdd32f5380caffc6 Mon Sep 17 00:00:00 2001 From: yyjeqhc <1772413353@qq.com> Date: Tue, 25 Aug 2026 23:48:38 +0800 Subject: [PATCH 1/2] Piggyback Session message resolution on MCP calls --- src/mcp.rs | 131 ++++++++++++++++++++++++ src/mcp_tests/http_transport.rs | 119 +++++++++++++++++++++ src/mcp_tests/protocol.rs | 22 ++++ src/mcp_tests/tools.rs | 66 ++++++++++++ src/tool_runtime/kernel.rs | 73 +++++++++++++ src/tool_runtime/sessions/events.rs | 20 ++-- src/tool_runtime/sessions/messages.rs | 23 +++++ src/tool_runtime/sessions/mod.rs | 10 +- src/tool_runtime/sessions/model.rs | 11 ++ src/tool_runtime/sessions/store.rs | 67 ++++++++++++ src/tool_runtime/sessions/tests.rs | 103 +++++++++++++++++++ src/tool_runtime/startup_brief.rs | 1 + src/tool_runtime/tests/startup_brief.rs | 9 ++ 13 files changed, 645 insertions(+), 10 deletions(-) diff --git a/src/mcp.rs b/src/mcp.rs index d730ea27..455da7e4 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -531,6 +531,26 @@ fn add_stateless_workflow_recorder_metadata(payload: &mut Value, model_surface: "description": "MCP wrapper metadata only. ACK means the current model context still remembers the referenced open Session message. Repeat ACK ids on subsequent calls while remembered. If omitted later, unresolved ACK-required guidance may be returned again. ACK does not resolve the message." }), ); + properties.insert( + crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_FIELD.to_string(), + json!({ + "type": "object", + "description": "MCP wrapper metadata only. After one non-todo message in recording_session_id is already handled, resolve it and attach bounded resolution text on this same WebCodex call instead of making a separate resolve call. For requires_ack guidance, include the same message_id in ack_session_message_ids on this request. The target is always the exact recording Session and this object is removed before concrete tool parsing. Do not use it to predict whether the current tool call will succeed; todo completion still uses complete_session_message.", + "properties": { + "message_id": { + "type": "string", + "pattern": "^wc_msg_[A-Za-z0-9_]+$" + }, + "resolution": { + "type": "string", + "minLength": 1, + "maxLength": crate::tool_runtime::sessions::MAX_MESSAGE_RESOLUTION_CHARS + } + }, + "required": ["message_id", "resolution"], + "additionalProperties": false + }), + ); if matches!(model_surface, ModelSurface::FullOperatorRuntime) { properties.insert( crate::tool_runtime::sessions::TOOL_CALL_ACK_SESSION_CONTEXT_REVISION_FIELD @@ -3122,6 +3142,56 @@ async fn handle_mcp_request_with_lifecycle( } else { Vec::new() }; + let session_message_resolution = if stateless_2026 { + if let Some(arguments) = params.arguments.as_object_mut() { + arguments.remove( + crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_INTERNAL_FIELD, + ); + } + match strip_stateless_session_message_resolution(&mut params.arguments) { + Ok(value) => value, + Err(message) => { + if let Some(lc) = lifecycle.as_deref() { + lc.dispatch_failed("invalid_arguments"); + lc.dispatch_finished(false, Some(false), "invalid_arguments"); + } + if let (Some(slot), Some(timer)) = ( + model_ergonomics_out.as_deref_mut(), + pre_kernel_model_ergonomics.take(), + ) { + *slot = Some( + timer + .finish() + .record_for_pre_result_failure("invalid_arguments"), + ); + } + return McpOutcome::BadRequest(rpc_error(id, -32602, message)); + } + } + } else { + None + }; + if session_message_resolution.is_some() && session_id.is_none() { + let message = format!( + "field '{}' requires '{}' for the exact target Workflow Session", + crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_FIELD, + crate::tool_runtime::sessions::TOOL_CALL_RECORDING_SESSION_ID_FIELD, + ); + if let Some(lc) = lifecycle.as_deref() { + lc.dispatch_failed("invalid_arguments"); + lc.dispatch_finished(false, Some(false), "invalid_arguments"); + } + return McpOutcome::BadRequest(rpc_error(id, -32602, message)); + } + if let (Some(arguments), Some(resolution)) = + (params.arguments.as_object_mut(), session_message_resolution) + { + arguments.insert( + crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_INTERNAL_FIELD + .to_string(), + json!(resolution), + ); + } if !ack_session_message_ids.is_empty() { if let Some(arguments) = params.arguments.as_object_mut() { arguments.insert( @@ -3618,6 +3688,67 @@ fn strip_stateless_ack_session_message_ids(arguments: &mut Value) -> Result Result, String> { + let Some(object) = arguments.as_object_mut() else { + return Ok(None); + }; + let Some(value) = + object.remove(crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_FIELD) + else { + return Ok(None); + }; + let Value::Object(mut fields) = value else { + return Err(format!( + "field '{}' must be an object with message_id and resolution", + crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_FIELD + )); + }; + if fields.len() != 2 || !fields.contains_key("message_id") || !fields.contains_key("resolution") + { + return Err(format!( + "field '{}' accepts exactly message_id and resolution", + crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_FIELD + )); + } + let Some(Value::String(message_id)) = fields.remove("message_id") else { + return Err("session_message_resolution.message_id must be a wc_msg_* string".to_string()); + }; + let message_id = message_id.trim().to_string(); + let valid_message_id = message_id.strip_prefix("wc_msg_").is_some_and(|suffix| { + !suffix.is_empty() + && suffix + .as_bytes() + .iter() + .all(|byte| byte.is_ascii_alphanumeric() || *byte == b'_') + }); + if !valid_message_id { + return Err( + "session_message_resolution.message_id must be a valid wc_msg_* id".to_string(), + ); + } + let Some(Value::String(resolution)) = fields.remove("resolution") else { + return Err("session_message_resolution.resolution must be a string".to_string()); + }; + let resolution = resolution.trim().to_string(); + if resolution.is_empty() { + return Err("session_message_resolution.resolution must not be empty".to_string()); + } + if resolution.chars().count() > crate::tool_runtime::sessions::MAX_MESSAGE_RESOLUTION_CHARS { + return Err(format!( + "session_message_resolution.resolution exceeds {} chars", + crate::tool_runtime::sessions::MAX_MESSAGE_RESOLUTION_CHARS + )); + } + Ok(Some( + crate::tool_runtime::sessions::ToolCallSessionMessageResolution { + message_id, + resolution, + }, + )) +} + fn strip_stateless_ack_session_context_revision(arguments: &mut Value) -> Option { arguments .as_object_mut()? diff --git a/src/mcp_tests/http_transport.rs b/src/mcp_tests/http_transport.rs index 17df7f0e..1fbbe0f5 100644 --- a/src/mcp_tests/http_transport.rs +++ b/src/mcp_tests/http_transport.rs @@ -897,6 +897,123 @@ async fn http_mcp_2026_request_scoped_ack_redelivers_until_durable_resolution() ); assert!(second_stored[0].first_ack_observed_at.is_some()); + let (status, third_post_body) = stateless_2026_tool_call( + &service, + "secret", + 229, + "post_session_message", + json!({ + "session_id": session_id, + "kind": "guidance", + "priority": "high", + "requires_ack": true, + "message": "Resolve this without a dedicated resolve tool call." + }), + None, + ) + .await; + assert_eq!(status, StatusCode::OK, "{third_post_body}"); + let third_message_id = stateless_tool_output(&third_post_body)["message_id"] + .as_str() + .unwrap() + .to_string(); + let resolution_text = "handled through ordinary list_tools wrapper metadata"; + + let missing_ack_args = with_mcp_recording_session( + json!({ + crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_FIELD: { + "message_id": third_message_id, + "resolution": resolution_text + } + }), + &session_id, + ); + let (status, missing_ack_body) = stateless_2026_tool_call( + &service, + "secret", + 230, + "list_tools", + missing_ack_args, + None, + ) + .await; + assert_eq!(status, StatusCode::OK, "{missing_ack_body}"); + assert_eq!( + stateless_tool_output(&missing_ack_body)["error_kind"], + "invalid_session_message" + ); + let still_open = runtime + .sessions + .list_messages( + &session_id, + crate::tool_runtime::sessions::ListSessionMessagesFilter { + message_id: Some(third_message_id.clone()), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!( + still_open[0].status, + crate::tool_runtime::sessions::SessionMessageStatus::Open + ); + + let mut piggyback_args = with_mcp_recording_session( + json!({ + crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_FIELD: { + "message_id": third_message_id, + "resolution": resolution_text + } + }), + &session_id, + ); + piggyback_args.as_object_mut().unwrap().insert( + crate::tool_runtime::sessions::TOOL_CALL_ACK_SESSION_MESSAGE_IDS_FIELD.to_string(), + json!([third_message_id]), + ); + let (status, piggyback_body) = stateless_2026_tool_call( + &service, + "secret", + 231, + "list_tools", + piggyback_args.clone(), + None, + ) + .await; + assert_eq!(status, StatusCode::OK, "{piggyback_body}"); + assert_eq!(piggyback_body["result"]["isError"], false); + let piggyback_output = stateless_tool_output(&piggyback_body); + assert_eq!( + piggyback_output["session_attention"]["ack"]["accepted_count"], + 1 + ); + assert!(piggyback_output["session_attention"]["messages"] + .as_array() + .unwrap() + .is_empty()); + let piggyback_stored = runtime + .sessions + .list_messages( + &session_id, + crate::tool_runtime::sessions::ListSessionMessagesFilter { + message_id: Some(third_message_id.clone()), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!( + piggyback_stored[0].status, + crate::tool_runtime::sessions::SessionMessageStatus::Resolved + ); + assert_eq!( + piggyback_stored[0].resolution.as_deref(), + Some(resolution_text) + ); + + let (status, replay_body) = + stateless_2026_tool_call(&service, "secret", 232, "list_tools", piggyback_args, None).await; + assert_eq!(status, StatusCode::OK, "{replay_body}"); + assert_eq!(replay_body["result"]["isError"], false); + let audit = serde_json::to_string( &runtime .sessions @@ -907,6 +1024,8 @@ async fn http_mcp_2026_request_scoped_ack_redelivers_until_durable_resolution() .unwrap(); assert!(!audit.contains("ack_session_message_ids")); assert!(!audit.contains("__webcodex_stateless_ack_session_message_ids")); + assert!(!audit.contains("__webcodex_stateless_session_message_resolution")); + assert!(!audit.contains("handled through ordinary list_tools wrapper metadata")); } #[tokio::test] diff --git a/src/mcp_tests/protocol.rs b/src/mcp_tests/protocol.rs index 09add173..7c2e5a62 100644 --- a/src/mcp_tests/protocol.rs +++ b/src/mcp_tests/protocol.rs @@ -208,6 +208,21 @@ async fn mcp_stateless_tools_list_uses_2026_result_shape() { let description = ack["description"].as_str().unwrap(); assert!(description.contains("current model context still remembers")); assert!(description.contains("ACK does not resolve")); + let resolution = &read_files["inputSchema"]["properties"]["session_message_resolution"]; + assert_eq!(resolution["type"], "object"); + assert_eq!( + resolution["properties"]["message_id"]["pattern"], + "^wc_msg_[A-Za-z0-9_]+$" + ); + assert_eq!(resolution["properties"]["resolution"]["minLength"], 1); + assert_eq!( + resolution["properties"]["resolution"]["maxLength"], + crate::tool_runtime::sessions::MAX_MESSAGE_RESOLUTION_CHARS + ); + let resolution_description = resolution["description"].as_str().unwrap(); + assert!(resolution_description.contains("same WebCodex call")); + assert!(resolution_description.contains("recording_session_id")); + assert!(resolution_description.contains("complete_session_message")); let context_ack = &read_files["inputSchema"]["properties"]["ack_session_context_revision"]; assert_eq!(context_ack["type"], "integer"); @@ -242,6 +257,13 @@ async fn mcp_legacy_tools_list_omits_2026_only_result_fields() { .all(|tool| tool["inputSchema"]["properties"] .get("ack_session_message_ids") .is_none())); + assert!(value["result"]["tools"] + .as_array() + .unwrap() + .iter() + .all(|tool| tool["inputSchema"]["properties"] + .get("session_message_resolution") + .is_none())); assert!(value["result"]["tools"] .as_array() .unwrap() diff --git a/src/mcp_tests/tools.rs b/src/mcp_tests/tools.rs index c0663752..21b770ba 100644 --- a/src/mcp_tests/tools.rs +++ b/src/mcp_tests/tools.rs @@ -192,6 +192,8 @@ fn stateless_workflow_recorder_metadata_does_not_expand_connector_or_generic_too .contains_key(crate::tool_runtime::sessions::TOOL_CALL_RECORDING_SESSION_ID_FIELD)); assert!(!generic_properties .contains_key(crate::tool_runtime::sessions::TOOL_CALL_ACK_SESSION_MESSAGE_IDS_FIELD)); + assert!(!generic_properties + .contains_key(crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_FIELD)); assert!(!generic_properties .contains_key(crate::tool_runtime::sessions::TOOL_CALL_ACK_SESSION_CONTEXT_REVISION_FIELD)); } @@ -239,6 +241,70 @@ fn stateless_ack_wrapper_normalizes_and_is_removed_before_concrete_tool_parsing( assert!(strip_stateless_ack_session_message_ids(&mut oversized).is_err()); } +#[test] +fn stateless_message_resolution_wrapper_is_validated_and_removed_before_concrete_parsing() { + let mut arguments = json!({ + crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_FIELD: { + "message_id": "wc_msg_beta", + "resolution": " handled in the current model turn " + } + }); + let resolution = strip_stateless_session_message_resolution(&mut arguments) + .unwrap() + .expect("message resolution wrapper"); + assert_eq!(resolution.message_id, "wc_msg_beta"); + assert_eq!(resolution.resolution, "handled in the current model turn"); + assert!(arguments + .get(crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_FIELD) + .is_none()); + + arguments.as_object_mut().unwrap().insert( + crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_INTERNAL_FIELD + .to_string(), + json!(resolution), + ); + let recorder = + crate::tool_runtime::sessions::ToolCallRecorderMetadata::from_arguments(&arguments); + assert_eq!( + recorder + .session_message_resolution + .as_ref() + .map(|value| value.message_id.as_str()), + Some("wc_msg_beta") + ); + let concrete = crate::tool_runtime::sessions::strip_tool_call_expectation_metadata(arguments); + assert!(concrete + .get(crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_INTERNAL_FIELD) + .is_none()); + crate::tool_runtime::ToolCall::from_tool_name("list_tools", concrete) + .expect("message resolution wrapper metadata must be gone before concrete parsing"); + + for malformed in [ + json!({ + crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_FIELD: { + "message_id": "not-a-message-id", + "resolution": "handled" + } + }), + json!({ + crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_FIELD: { + "message_id": "wc_msg_beta", + "resolution": " " + } + }), + json!({ + crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_FIELD: { + "message_id": "wc_msg_beta", + "resolution": "handled", + "extra": true + } + }), + ] { + let mut malformed = malformed; + assert!(strip_stateless_session_message_resolution(&mut malformed).is_err()); + } +} + #[test] fn stateless_context_revision_ack_is_request_scoped_and_removed_before_parsing() { let mut arguments = json!({ diff --git a/src/tool_runtime/kernel.rs b/src/tool_runtime/kernel.rs index 2ff58b42..f8c6521a 100644 --- a/src/tool_runtime/kernel.rs +++ b/src/tool_runtime/kernel.rs @@ -207,6 +207,20 @@ impl ToolRuntime { &recorder_metadata.ack_session_message_ids, ) }); + let session_message_resolution = (context.transport == ToolTransport::Mcp) + .then(|| recorder_metadata.session_message_resolution.clone()) + .flatten(); + if session_message_resolution.is_some() && context.session_id.is_none() { + return ToolCallOutcome { + success: false, + result: None, + error_status: Some(ToolCallErrorStatus::InvalidArguments { + message: "session_message_resolution requires recording_session_id".to_string(), + }), + project: None, + model_ergonomics: None, + }; + } if collaboration_session_tool(&request.tool_name) { if let (Some(recorder_session_id), Some(target_session_id)) = ( context.session_id, @@ -602,6 +616,65 @@ impl ToolRuntime { }; } }; + if let (Some(session_id), Some(message_resolution)) = + (context.session_id, session_message_resolution.as_ref()) + { + let current_request_acknowledged = outer_ack_observation + .as_ref() + .is_some_and(|ack| ack.accepted_ids.contains(&message_resolution.message_id)); + if let Err(error) = self.sessions.resolve_message_from_wrapper( + session_id, + &message_resolution.message_id, + message_resolution.resolution.clone(), + current_request_acknowledged, + ) { + let mut result = session_context::session_message_error_result( + session_id, + Some(&message_resolution.message_id), + error, + ); + super::dispatch::decorate_structured_execution_prestart_denial( + &request.tool_name, + &mut result, + "session_message_resolution_failed", + ); + let recording = self.sessions.record_model_facing_tool_call_finished( + session_event, + false, + &result.output, + result.error.as_deref(), + Some("session_message_resolution_failed"), + ); + super::add_session_telemetry_hint( + &mut result, + &self.sessions, + session_id, + recording.as_ref().map(|recorded| recorded.event_id.clone()), + ); + if let Some(recorded) = recording.as_ref() { + if session_context::add_session_context_continuity(&mut result, recorded) { + self.add_session_history_recovery(&mut result, recorded, context.auth) + .await; + } + } + session_context::add_session_attention_projection( + &mut result, + &self.sessions, + session_id, + outer_ack_observation + .as_ref() + .expect("authorized outer recorder must have ACK observation"), + recorder_ack_requested, + ); + return ToolCallOutcome { + success: false, + result: Some(result), + error_status: None, + project: None, + model_ergonomics: None, + }; + } + } if let ToolCall::ImportConversationFilesToProject { trusted_mcp_host_file_import, .. diff --git a/src/tool_runtime/sessions/events.rs b/src/tool_runtime/sessions/events.rs index c3bafcea..a4564ff6 100644 --- a/src/tool_runtime/sessions/events.rs +++ b/src/tool_runtime/sessions/events.rs @@ -14,14 +14,15 @@ use serde_json::{json, Value}; use super::model::{ PersistentShellEventEvidence, SessionContextRevisionAck, SessionEvent, ToolCallExpectation, - ToolCallRecorderMetadata, MAX_OBSERVED_PATHS_PER_EVENT, MAX_VALIDATION_EXCERPT_CHARS, - SESSION_ID_PREFIX, TOOL_ASSERTION_NAME_FIELD, + ToolCallRecorderMetadata, ToolCallSessionMessageResolution, MAX_OBSERVED_PATHS_PER_EVENT, + MAX_VALIDATION_EXCERPT_CHARS, SESSION_ID_PREFIX, TOOL_ASSERTION_NAME_FIELD, TOOL_CALL_ACK_SESSION_CONTEXT_REVISION_INTERNAL_FIELD, TOOL_CALL_ACK_SESSION_MESSAGE_IDS_INTERNAL_FIELD, TOOL_CALL_EXPECTATION_METADATA_FIELDS, - TOOL_CALL_RECORDING_SESSION_ID_FIELD, TOOL_EXPECTATION_RESULT_MATCHED, - TOOL_EXPECTATION_RESULT_MISMATCH, TOOL_EXPECTATION_RESULT_NONE, - TOOL_EXPECTATION_RESULT_UNEXPECTED_FAILURE, TOOL_EXPECTATION_RESULT_UNEXPECTED_SUCCESS, - TOOL_EXPECTED_FAILURE_FIELD, TOOL_EXPECTED_FAILURE_KIND_FIELD, + TOOL_CALL_RECORDING_SESSION_ID_FIELD, TOOL_CALL_SESSION_MESSAGE_RESOLUTION_INTERNAL_FIELD, + TOOL_EXPECTATION_RESULT_MATCHED, TOOL_EXPECTATION_RESULT_MISMATCH, + TOOL_EXPECTATION_RESULT_NONE, TOOL_EXPECTATION_RESULT_UNEXPECTED_FAILURE, + TOOL_EXPECTATION_RESULT_UNEXPECTED_SUCCESS, TOOL_EXPECTED_FAILURE_FIELD, + TOOL_EXPECTED_FAILURE_KIND_FIELD, }; use super::util::redact_and_bound_value; use super::util::{bound_summary_string, validation_excerpt}; @@ -57,6 +58,12 @@ impl ToolCallRecorderMetadata { .collect() }) .unwrap_or_default(), + session_message_resolution: arguments + .as_object() + .and_then(|obj| obj.get(TOOL_CALL_SESSION_MESSAGE_RESOLUTION_INTERNAL_FIELD)) + .and_then(|value| { + serde_json::from_value::(value.clone()).ok() + }), ack_session_context_revision: if context_continuity_capable { match arguments .as_object() @@ -131,6 +138,7 @@ pub(crate) fn strip_tool_call_expectation_metadata(arguments: Value) -> Value { } obj.remove(TOOL_CALL_ACK_SESSION_MESSAGE_IDS_INTERNAL_FIELD); obj.remove(TOOL_CALL_ACK_SESSION_CONTEXT_REVISION_INTERNAL_FIELD); + obj.remove(TOOL_CALL_SESSION_MESSAGE_RESOLUTION_INTERNAL_FIELD); Value::Object(obj) } diff --git a/src/tool_runtime/sessions/messages.rs b/src/tool_runtime/sessions/messages.rs index 5958b695..84efad24 100644 --- a/src/tool_runtime/sessions/messages.rs +++ b/src/tool_runtime/sessions/messages.rs @@ -189,6 +189,29 @@ impl SessionStore { Ok(message) } + pub(crate) fn resolve_message_from_wrapper( + &self, + session_id: &str, + message_id: &str, + resolution: String, + current_request_acknowledged: bool, + ) -> Result { + let (message, changed) = { + let mut inner = self.inner.lock().expect("session store mutex poisoned"); + inner.resolve_message_from_wrapper( + session_id, + message_id, + resolution, + current_request_acknowledged, + )? + }; + self.persist_after_mutation(); + if changed { + self.notify_message_observation(); + } + Ok(message) + } + pub(crate) fn complete_message( &self, input: CompleteSessionMessageInput, diff --git a/src/tool_runtime/sessions/mod.rs b/src/tool_runtime/sessions/mod.rs index 326134a5..79d2f85d 100644 --- a/src/tool_runtime/sessions/mod.rs +++ b/src/tool_runtime/sessions/mod.rs @@ -48,13 +48,15 @@ pub(crate) use model::{ SessionExecutionContextUpdateError, SessionGuardDenial, SessionGuards, SessionLifecycle, SessionLifecycleDenial, SessionMessage, SessionMessageError, SessionMessageKind, SessionMessageObservationError, SessionMessagePriority, SessionMessageStatus, SessionSummary, - SessionTransport, ToolCallRecorderMetadata, ToolCallStart, DEFAULT_MAX_EVENTS_PER_SESSION, - DEFAULT_MAX_SESSIONS, MAX_CODING_INSTRUCTION_CHARS, MAX_MESSAGE_COMPLETION_KEY_CHARS, - MAX_MESSAGE_LIST_LIMIT, MAX_TOOL_CALL_ACK_MESSAGE_IDS, - SESSION_INBOX_HIGH_GUIDANCE_ATTENTION_INSTRUCTION, + SessionTransport, ToolCallRecorderMetadata, ToolCallSessionMessageResolution, ToolCallStart, + DEFAULT_MAX_EVENTS_PER_SESSION, DEFAULT_MAX_SESSIONS, MAX_CODING_INSTRUCTION_CHARS, + MAX_MESSAGE_COMPLETION_KEY_CHARS, MAX_MESSAGE_LIST_LIMIT, MAX_MESSAGE_RESOLUTION_CHARS, + MAX_TOOL_CALL_ACK_MESSAGE_IDS, SESSION_INBOX_HIGH_GUIDANCE_ATTENTION_INSTRUCTION, SESSION_INBOX_HIGH_GUIDANCE_ATTENTION_REASON, TOOL_CALL_ACK_SESSION_CONTEXT_REVISION_FIELD, TOOL_CALL_ACK_SESSION_CONTEXT_REVISION_INTERNAL_FIELD, TOOL_CALL_ACK_SESSION_MESSAGE_IDS_FIELD, TOOL_CALL_ACK_SESSION_MESSAGE_IDS_INTERNAL_FIELD, TOOL_CALL_RECORDING_SESSION_ID_FIELD, + TOOL_CALL_SESSION_MESSAGE_RESOLUTION_FIELD, + TOOL_CALL_SESSION_MESSAGE_RESOLUTION_INTERNAL_FIELD, TOOL_EXPECTATION_RESULT_UNEXPECTED_FAILURE, }; pub(crate) use store::SessionStore; diff --git a/src/tool_runtime/sessions/model.rs b/src/tool_runtime/sessions/model.rs index 3ae706ef..8cebd24f 100644 --- a/src/tool_runtime/sessions/model.rs +++ b/src/tool_runtime/sessions/model.rs @@ -59,6 +59,9 @@ pub(crate) const TOOL_CALL_RECORDING_SESSION_ID_FIELD: &str = "recording_session pub(crate) const TOOL_CALL_ACK_SESSION_MESSAGE_IDS_FIELD: &str = "ack_session_message_ids"; pub(crate) const TOOL_CALL_ACK_SESSION_MESSAGE_IDS_INTERNAL_FIELD: &str = "__webcodex_stateless_ack_session_message_ids"; +pub(crate) const TOOL_CALL_SESSION_MESSAGE_RESOLUTION_FIELD: &str = "session_message_resolution"; +pub(crate) const TOOL_CALL_SESSION_MESSAGE_RESOLUTION_INTERNAL_FIELD: &str = + "__webcodex_stateless_session_message_resolution"; pub(crate) const TOOL_CALL_ACK_SESSION_CONTEXT_REVISION_FIELD: &str = "ack_session_context_revision"; pub(crate) const TOOL_CALL_ACK_SESSION_CONTEXT_REVISION_INTERNAL_FIELD: &str = @@ -787,6 +790,13 @@ pub(crate) struct ToolCallExpectation { pub(crate) assertion_name: Option, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub(crate) struct ToolCallSessionMessageResolution { + pub(crate) message_id: String, + pub(crate) resolution: String, +} + #[derive(Debug, Clone, Default, PartialEq, Eq)] pub(crate) struct ToolCallRecorderMetadata { /// Explicit generic wrapper recorder provenance. It is internal metadata, @@ -794,6 +804,7 @@ pub(crate) struct ToolCallRecorderMetadata { pub(crate) recording_session_id: Option, pub(crate) expectation: ToolCallExpectation, pub(crate) ack_session_message_ids: Vec, + pub(crate) session_message_resolution: Option, pub(crate) ack_session_context_revision: SessionContextRevisionAck, } diff --git a/src/tool_runtime/sessions/store.rs b/src/tool_runtime/sessions/store.rs index 2fff89de..9fdc66db 100644 --- a/src/tool_runtime/sessions/store.rs +++ b/src/tool_runtime/sessions/store.rs @@ -3270,6 +3270,73 @@ impl SessionStoreInner { Ok((message.clone(), changed)) } + pub(super) fn resolve_message_from_wrapper( + &mut self, + session_id: &str, + message_id: &str, + resolution: String, + current_request_acknowledged: bool, + ) -> Result<(SessionMessage, bool), SessionMessageError> { + self.touch(session_id); + let Some(stored) = self.sessions.get_mut(session_id) else { + return Err(SessionMessageError::UnknownSession); + }; + let lifecycle = stored.lifecycle(); + if !lifecycle.allows_mutation() { + return Err(SessionMessageError::SessionClosed { lifecycle }); + } + let resolution = validate_resolution_text(resolution)?; + if resolution.is_empty() { + return Err(SessionMessageError::InvalidInput( + "session_message_resolution.resolution must not be empty".to_string(), + )); + } + let record = stored + .hot_mut() + .expect("active session message mutation must stay hot"); + let Some(message_index) = record + .messages + .iter() + .position(|message| message.message_id == message_id) + else { + return Err(SessionMessageError::UnknownMessage); + }; + let snapshot = record.messages[message_index].as_ref().clone(); + if snapshot.closure_kind.is_some() { + return Err(SessionMessageError::MessageNotOpen); + } + if snapshot.kind == super::model::SessionMessageKind::Todo { + return Err(SessionMessageError::InvalidInput( + "todo messages require complete_session_message rather than session_message_resolution" + .to_string(), + )); + } + if snapshot.status == SessionMessageStatus::Resolved { + if snapshot.resolution.as_deref() == Some(resolution.as_str()) { + return Ok((snapshot, false)); + } + return Err(SessionMessageError::IdempotencyConflict); + } + if snapshot.requires_ack && !current_request_acknowledged { + return Err(SessionMessageError::InvalidInput( + "requires_ack guidance must be acknowledged on the same request before wrapper resolution" + .to_string(), + )); + } + + let revision = Self::next_message_observation_revision(record)?; + let now = now_ts(); + let message = Arc::make_mut(&mut record.messages[message_index]); + message.status = SessionMessageStatus::Resolved; + message.resolved_at = Some(now); + message.resolution = Some(resolution); + record.updated_at = now; + record + .message_observation_revisions + .insert(message.message_id.clone(), revision); + Ok((message.clone(), true)) + } + pub(super) fn complete_message( &mut self, input: CompleteSessionMessageInput, diff --git a/src/tool_runtime/sessions/tests.rs b/src/tool_runtime/sessions/tests.rs index 54782f04..75aeef99 100644 --- a/src/tool_runtime/sessions/tests.rs +++ b/src/tool_runtime/sessions/tests.rs @@ -3533,6 +3533,109 @@ fn session_message_create_list_and_resolve_contract() { assert!(open.is_empty()); } +#[test] +fn wrapper_resolution_requires_ack_rejects_todo_and_replays_idempotently() { + let store = SessionStore::default(); + let session = store.start_session(None, None); + let guidance = store + .post_message_with_ack( + PostSessionMessageInput { + session_id: session.session_id.clone(), + kind: SessionMessageKind::Guidance, + message: "apply the reviewed direction".to_string(), + tags: Vec::new(), + reply_to: None, + priority: SessionMessagePriority::High, + }, + true, + ) + .unwrap(); + + let missing_ack = store.resolve_message_from_wrapper( + &session.session_id, + &guidance.message_id, + "handled".to_string(), + false, + ); + assert!(matches!( + missing_ack, + Err(SessionMessageError::InvalidInput(_)) + )); + let still_open = store + .list_messages( + &session.session_id, + ListSessionMessagesFilter { + message_id: Some(guidance.message_id.clone()), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!(still_open[0].status, SessionMessageStatus::Open); + + let resolved = store + .resolve_message_from_wrapper( + &session.session_id, + &guidance.message_id, + "handled".to_string(), + true, + ) + .unwrap(); + assert_eq!(resolved.status, SessionMessageStatus::Resolved); + assert_eq!(resolved.resolution.as_deref(), Some("handled")); + let resolved_at = resolved.resolved_at; + + let replay = store + .resolve_message_from_wrapper( + &session.session_id, + &guidance.message_id, + "handled".to_string(), + false, + ) + .unwrap(); + assert_eq!(replay.resolved_at, resolved_at); + let conflict = store.resolve_message_from_wrapper( + &session.session_id, + &guidance.message_id, + "different completion".to_string(), + true, + ); + assert!(matches!( + conflict, + Err(SessionMessageError::IdempotencyConflict) + )); + + let todo = store + .post_message(PostSessionMessageInput { + session_id: session.session_id.clone(), + kind: SessionMessageKind::Todo, + message: "finish the delegated task".to_string(), + tags: Vec::new(), + reply_to: None, + priority: SessionMessagePriority::Normal, + }) + .unwrap(); + let todo_resolution = store.resolve_message_from_wrapper( + &session.session_id, + &todo.message_id, + "done".to_string(), + true, + ); + assert!(matches!( + todo_resolution, + Err(SessionMessageError::InvalidInput(_)) + )); + let retained_todo = store + .list_messages( + &session.session_id, + ListSessionMessagesFilter { + message_id: Some(todo.message_id), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!(retained_todo[0].status, SessionMessageStatus::Open); +} + #[test] fn read_only_guards_block_write_and_shell_classifications() { let store = SessionStore::default(); diff --git a/src/tool_runtime/startup_brief.rs b/src/tool_runtime/startup_brief.rs index 2a339751..3aa6da1b 100644 --- a/src/tool_runtime/startup_brief.rs +++ b/src/tool_runtime/startup_brief.rs @@ -58,6 +58,7 @@ pub(crate) fn builtin_coding_workflow_projection() -> Value { "session_context_ack": "Schema has ack_session_context_revision: copy latest returned session_context_revision exactly; never increment/derive. No returned revision: keep ACK. If unavailable/unknown, omit. Missing/stale ACK is nonblocking.", "session_recording": "After work_on_project creates or continues an execution Workflow Session, when a later WebCodex schema exposes recording_session_id, keep passing that execution/recording Session as recording_session_id. It is recorder provenance/context only: a concrete business session_id may target a different Session, and recording_session_id grants no business authority.", "session_message_ack": "When session_attention returns open requires_ack guidance still present in the model context, keep echoing those message ids in ack_session_message_ids on later calls. This ACK is request-scoped model-context proof only: it does not resolve messages, grant authority, or gate execution; missing/stale ACK remains nonblocking.", + "session_message_resolution": "After a non-todo Session message is already handled, a stateless MCP schema may expose session_message_resolution. Piggyback {message_id, resolution} on the next ordinary WebCodex call together with recording_session_id instead of making a separate resolve call. For requires_ack guidance, echo the same id in ack_session_message_ids on that request. The target is always the exact recording Session; the wrapper field is removed before concrete tool parsing. Do not use it to predict the success of the current tool call. Todo completion still uses complete_session_message.", "normal_closeout": "Normal success: finish_coding_task(summary_only=true); full closeout only for unresolved validation/evidence or handoff/debug detail." }, "roles": { diff --git a/src/tool_runtime/tests/startup_brief.rs b/src/tool_runtime/tests/startup_brief.rs index fe3d3849..db9fa182 100644 --- a/src/tool_runtime/tests/startup_brief.rs +++ b/src/tool_runtime/tests/startup_brief.rs @@ -120,6 +120,15 @@ fn assert_builtin_workflow(output: &Value) { assert!(message_ack_guidance.contains("grant authority")); assert!(message_ack_guidance.contains("gate execution")); assert!(message_ack_guidance.contains("missing/stale ACK remains nonblocking")); + let message_resolution_guidance = workflow["model_protocol"]["session_message_resolution"] + .as_str() + .expect("Session message resolution guidance"); + assert!(message_resolution_guidance.contains("session_message_resolution")); + assert!(message_resolution_guidance.contains("next ordinary WebCodex call")); + assert!(message_resolution_guidance.contains("recording_session_id")); + assert!(message_resolution_guidance.contains("ack_session_message_ids")); + assert!(message_resolution_guidance.contains("Do not use it to predict")); + assert!(message_resolution_guidance.contains("complete_session_message")); let closeout_guidance = workflow["model_protocol"]["normal_closeout"] .as_str() .expect("normal closeout guidance"); From 56a6933171f64959f58d8990ab5453e2f0db0d8c Mon Sep 17 00:00:00 2001 From: yyjeqhc <1772413353@qq.com> Date: Wed, 26 Aug 2026 00:11:30 +0800 Subject: [PATCH 2/2] Preserve Session resolution scope authority --- src/tool_runtime/kernel.rs | 158 +++++++++++++++++++++++++++++++++++-- 1 file changed, 151 insertions(+), 7 deletions(-) diff --git a/src/tool_runtime/kernel.rs b/src/tool_runtime/kernel.rs index f8c6521a..fd48bd9c 100644 --- a/src/tool_runtime/kernel.rs +++ b/src/tool_runtime/kernel.rs @@ -136,6 +136,20 @@ pub(crate) fn check_runtime_tool_scope( } } +fn check_session_message_resolution_scope( + auth: Option<&AuthContext>, + requested: bool, +) -> Result<(), ToolCallErrorStatus> { + if requested { + // Piggyback resolution is the same business mutation as the dedicated + // Session tool. Reuse its canonical scope policy so a caller cannot + // acquire Session-closure authority from an unrelated main tool scope. + check_runtime_tool_scope(auth, "resolve_session_message") + } else { + Ok(()) + } +} + impl ToolRuntime { pub(crate) async fn call_tool_with_context( &self, @@ -200,13 +214,6 @@ impl ToolRuntime { } } let recorder_ack_requested = !recorder_metadata.ack_session_message_ids.is_empty(); - let outer_ack_observation = context.session_id.map(|recorder_session_id| { - session_context::observe_session_attention_acks( - &self.sessions, - recorder_session_id, - &recorder_metadata.ack_session_message_ids, - ) - }); let session_message_resolution = (context.transport == ToolTransport::Mcp) .then(|| recorder_metadata.session_message_resolution.clone()) .flatten(); @@ -221,6 +228,25 @@ impl ToolRuntime { model_ergonomics: None, }; } + if let Err(error_status) = check_session_message_resolution_scope( + context.auth, + session_message_resolution.is_some(), + ) { + return ToolCallOutcome { + success: false, + result: None, + error_status: Some(error_status), + project: None, + model_ergonomics: None, + }; + } + let outer_ack_observation = context.session_id.map(|recorder_session_id| { + session_context::observe_session_attention_acks( + &self.sessions, + recorder_session_id, + &recorder_metadata.ack_session_message_ids, + ) + }); if collaboration_session_tool(&request.tool_name) { if let (Some(recorder_session_id), Some(target_session_id)) = ( context.session_id, @@ -1042,6 +1068,124 @@ mod tests { assert!(!serialized.contains("secret-content")); } + #[tokio::test] + async fn piggyback_resolution_cannot_inherit_main_tool_scope() { + let runtime = test_runtime(); + let auth = oauth(&["project:read"]); + let fingerprint = crate::tool_runtime::workflow_session_authority_fingerprint(Some(&auth)) + .expect("OAuth test authority must have a stable identity"); + let session = runtime + .sessions + .start_session_with_options( + crate::tool_runtime::sessions::SessionCreateOptions::new( + None, + Some("piggyback scope fence".to_string()), + crate::tool_runtime::SessionMode::Normal, + crate::tool_runtime::sessions::SessionGuards::default(), + ) + .with_owner_authority_fingerprint(Some(fingerprint)), + ) + .unwrap(); + let message = runtime + .sessions + .post_message_with_ack( + crate::tool_runtime::sessions::PostSessionMessageInput { + session_id: session.session_id.clone(), + kind: crate::tool_runtime::sessions::SessionMessageKind::Note, + message: "close only with Session mutation authority".to_string(), + tags: Vec::new(), + reply_to: None, + priority: crate::tool_runtime::sessions::SessionMessagePriority::Normal, + }, + false, + ) + .unwrap(); + let mut arguments = json!({ + "project": "demo", + "path": "README.md" + }); + arguments.as_object_mut().unwrap().insert( + crate::tool_runtime::sessions::TOOL_CALL_SESSION_MESSAGE_RESOLUTION_INTERNAL_FIELD + .to_string(), + json!({ + "message_id": message.message_id, + "resolution": "handled" + }), + ); + + let outcome = runtime + .call_tool_with_context( + ToolCallRequest { + tool_name: "read_file".to_string(), + arguments, + }, + ToolCallContext { + transport: ToolTransport::Mcp, + session_id: Some(&session.session_id), + auth: Some(&auth), + window: None, + record_oauth_scope_denials: false, + host_file_import_trust: HostFileImportTrust::Untrusted, + }, + ) + .await; + + assert_eq!( + outcome.error_status, + Some(ToolCallErrorStatus::InsufficientScope { + required_scope: Some(crate::auth::SCOPE_RUNTIME_READ), + description: "missing required scope: runtime:read".to_string(), + }) + ); + assert!(outcome.result.is_none()); + let retained = runtime + .sessions + .list_messages( + &session.session_id, + crate::tool_runtime::sessions::ListSessionMessagesFilter { + message_id: Some(message.message_id), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!(retained.len(), 1); + assert_eq!( + retained[0].status, + crate::tool_runtime::sessions::SessionMessageStatus::Open, + "scope denial must happen before the piggyback closure mutation" + ); + assert!(retained[0].resolution.is_none()); + } + + #[test] + fn session_message_resolution_reuses_dedicated_resolve_scope() { + let project_read_only = oauth(&["project:read"]); + assert_eq!( + check_runtime_tool_scope(Some(&project_read_only), "read_file"), + Ok(()), + "main project read authority must remain independent" + ); + assert_eq!( + check_session_message_resolution_scope(Some(&project_read_only), true), + Err(ToolCallErrorStatus::InsufficientScope { + required_scope: Some(crate::auth::SCOPE_RUNTIME_READ), + description: "missing required scope: runtime:read".to_string(), + }), + "piggyback resolution must not inherit the main tool scope" + ); + assert_eq!( + check_session_message_resolution_scope(Some(&project_read_only), false), + Ok(()), + "ordinary calls without resolution keep their existing scope contract" + ); + let runtime_read = oauth(&["runtime:read"]); + assert_eq!( + check_session_message_resolution_scope(Some(&runtime_read), true), + Ok(()), + "piggyback resolution must track the dedicated resolve tool policy" + ); + } + #[test] fn coding_agent_tools_require_independent_execution_scope() { for insufficient in [