From 729ca26a7335cde8fe78ee9f1eda6a815a273d6e Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Thu, 17 Sep 2026 22:27:11 +0000 Subject: [PATCH] fix(surface): name the field a refused application request failed on Every reviewed-schema rejection said only that the request did not match. The adapter error now carries the decode detail, and the problem envelope repeats it to the caller. --- .../tracedecay-cli/src/tool_command/tests.rs | 2 +- .../src/application_surface.rs | 134 +++++++++++------- .../src/application_surface/git.rs | 56 +++++--- .../application_surface/configuration_wire.rs | 10 +- .../src/application_surface/dispatch.rs | 14 +- .../feedback_observation.rs | 2 +- .../src/application_surface/problems.rs | 28 ++-- .../src/application_surface/retained.rs | 2 +- .../src/application_surface/tests.rs | 16 +-- .../mcp/tools/handlers/application_surface.rs | 2 +- 10 files changed, 162 insertions(+), 104 deletions(-) diff --git a/crates/tracedecay-cli/src/tool_command/tests.rs b/crates/tracedecay-cli/src/tool_command/tests.rs index 3817bfaa00..93c237bb36 100644 --- a/crates/tracedecay-cli/src/tool_command/tests.rs +++ b/crates/tracedecay-cli/src/tool_command/tests.rs @@ -1646,7 +1646,7 @@ fn application_surface_rejects_invalid_output_formats() { .expect_err("format outside the schema must fail"); assert!(matches!( error, - ApplicationSurfaceAdapterError::InvalidSurfaceRequest + ApplicationSurfaceAdapterError::InvalidSurfaceRequest { .. } )); } } diff --git a/crates/tracedecay-daemon-protocol/src/application_surface.rs b/crates/tracedecay-daemon-protocol/src/application_surface.rs index 043fd3e246..8dcb58e38c 100644 --- a/crates/tracedecay-daemon-protocol/src/application_surface.rs +++ b/crates/tracedecay-daemon-protocol/src/application_surface.rs @@ -55,8 +55,10 @@ pub enum ApplicationSurfaceAdapterError { CatalogValidation(#[from] CatalogValidationError), #[error("application surface request handle is invalid")] InvalidRequestHandle, - #[error("application surface request does not match its reviewed schema")] - InvalidSurfaceRequest, + /// `detail` names the field or shape the reviewed schema refused. It + /// echoes only the caller's own request, never store or session content. + #[error("application surface request does not match its reviewed schema: {detail}")] + InvalidSurfaceRequest { detail: String }, #[error("owning daemon application service is unavailable")] DaemonUnavailable, /// No daemon accepted the connection after the transport's restart grace; @@ -69,6 +71,14 @@ pub enum ApplicationSurfaceAdapterError { UnknownOrNotAuthorized, } +impl ApplicationSurfaceAdapterError { + pub fn invalid_request(detail: impl std::fmt::Display) -> Self { + Self::InvalidSurfaceRequest { + detail: detail.to_string(), + } + } +} + /// Transport keys every surface adapter accepts but no reviewed application /// request schema declares. `format` selects the rendered output and /// `__mcp_request_id` carries protocol identity; both are stripped here so @@ -91,7 +101,9 @@ pub fn separate_application_tool_request( if let Some(format) = args.get("format") && !matches!(format.as_str(), Some("markdown" | "json")) { - return Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest); + return Err(ApplicationSurfaceAdapterError::invalid_request( + "`format` must be markdown or json", + )); } let requested_format = requested_output_format(&args); if let Some(object) = args.as_object_mut() { @@ -131,14 +143,26 @@ fn adapt_shipped_diagnostics_request( .unwrap_or("workspace") { "workspace" => serde_json::json!("workspace"), - "package" => return Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + "package" => { + return Err(ApplicationSurfaceAdapterError::invalid_request( + "`scope` package is not supported for diagnostics", + )); + } "file" => serde_json::json!({ "file": args .get("path") .and_then(Value::as_str) - .ok_or(ApplicationSurfaceAdapterError::InvalidSurfaceRequest)? + .ok_or_else(|| { + ApplicationSurfaceAdapterError::invalid_request( + "`path` is required when `scope` is file", + ) + })? }), - _ => return Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + other => { + return Err(ApplicationSurfaceAdapterError::invalid_request(format!( + "`scope` `{other}` is not one of workspace or file" + ))); + } }; let maximum_diagnostics = match args.get("maximum_diagnostics") { Some(maximum_diagnostics) => maximum_diagnostics.clone(), @@ -444,7 +468,7 @@ fn parse_native_integration_surface_request( operation: ApplicationSurfaceOperation, value: Value, ) -> Result { - let invalid = |_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest; + let invalid = ApplicationSurfaceAdapterError::invalid_request; match operation { ApplicationSurfaceOperation::NativeIntegrationStackSnapshot => { serde_json::from_value(value) @@ -496,7 +520,9 @@ fn parse_native_integration_surface_request( .map(NativeIntegrationSurfaceRequest::Worktree) .map_err(invalid) } - _ => Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + _ => Err(ApplicationSurfaceAdapterError::invalid_request( + "operation is not a native-integration surface", + )), } } @@ -516,13 +542,13 @@ pub fn parse_application_surface_request( } ApplicationSurfaceOperation::GitHubStackSignalExpand => serde_json::from_value(value) .map(ApplicationSurfaceRequest::GitHubStackSignalExpand) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::GitPreview => serde_json::from_value(value) .map(ApplicationSurfaceRequest::GitPreview) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::GitApply => serde_json::from_value(value) .map(ApplicationSurfaceRequest::GitApply) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::NativeIntegrationStackSnapshot | ApplicationSurfaceOperation::NativeIntegrationPreflight | ApplicationSurfaceOperation::NativeIntegrationApprove @@ -539,160 +565,160 @@ pub fn parse_application_surface_request( } ApplicationSurfaceOperation::TestResults => serde_json::from_value(value) .map(ApplicationSurfaceRequest::TestResults) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::CodeExactOccurrence => { serde_json::from_value::(value) .map(CallableCodeSurfaceRequest::ExactOccurrence) .map(ApplicationSurfaceRequest::CallableCode) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::CodePhraseSearch => { serde_json::from_value::(value) .map(CallableCodeSurfaceRequest::PhraseSearch) .map(ApplicationSurfaceRequest::CallableCode) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::CodeSymbolSearch => { serde_json::from_value::(value) .map(PrimitiveCodeSurfaceRequest::SymbolSearch) .map(ApplicationSurfaceRequest::PrimitiveCode) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::CodeSignatureSearch => { serde_json::from_value::(value) .map(PrimitiveCodeSurfaceRequest::SignatureSearch) .map(ApplicationSurfaceRequest::PrimitiveCode) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::CodeImplementations => { serde_json::from_value::(value) .map(PrimitiveCodeSurfaceRequest::Implementations) .map(ApplicationSurfaceRequest::PrimitiveCode) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::CodeTypeHierarchy => { serde_json::from_value::(value) .map(PrimitiveCodeSurfaceRequest::TypeHierarchy) .map(ApplicationSurfaceRequest::PrimitiveCode) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::CodeCallers => { serde_json::from_value::(value) .map(PrimitiveCodeSurfaceRequest::Callers) .map(ApplicationSurfaceRequest::PrimitiveCode) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::CodeCallees => { serde_json::from_value::(value) .map(CallableCodeSurfaceRequest::Callees) .map(ApplicationSurfaceRequest::CallableCode) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::CodeFacets => { serde_json::from_value::(value) .map(CallableCodeSurfaceRequest::Facets) .map(ApplicationSurfaceRequest::CallableCode) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::CodeTimeline => { serde_json::from_value::(value) .map(CallableCodeSurfaceRequest::Timeline) .map(ApplicationSurfaceRequest::CallableCode) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::CodeDeclaration => { serde_json::from_value::(value) .map(CallableCodeSurfaceRequest::Declaration) .map(ApplicationSurfaceRequest::CallableCode) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::CodeTypeDefinition => { serde_json::from_value::(value) .map(CallableCodeSurfaceRequest::TypeDefinition) .map(ApplicationSurfaceRequest::CallableCode) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::CodeReferences => { serde_json::from_value::(value) .map(CallableCodeSurfaceRequest::References) .map(ApplicationSurfaceRequest::CallableCode) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::SessionLookup => { serde_json::from_value::(value) .map(PrimitiveRequest::SessionLookup) .map(ApplicationSurfaceRequest::Primitive) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::QualifiedName => { serde_json::from_value::(value) .map(PrimitiveRequest::QualifiedName) .map(ApplicationSurfaceRequest::Primitive) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::CallChain => { serde_json::from_value::(value) .map(PrimitiveRequest::CallChain) .map(ApplicationSurfaceRequest::Primitive) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::FileDependents => { serde_json::from_value::(value) .map(PrimitiveRequest::FileDependents) .map(ApplicationSurfaceRequest::Primitive) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::SourceLines => { serde_json::from_value::(value) .map(PrimitiveRequest::SourceLines) .map(ApplicationSurfaceRequest::Primitive) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::SourceBody => { serde_json::from_value::(value) .map(PrimitiveRequest::SourceBody) .map(ApplicationSurfaceRequest::Primitive) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::SourceOutline => { serde_json::from_value::(value) .map(PrimitiveRequest::SourceOutline) .map(ApplicationSurfaceRequest::Primitive) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::ModuleApi => { serde_json::from_value::(value) .map(PrimitiveRequest::ModuleApi) .map(ApplicationSurfaceRequest::Primitive) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::HealthRead => { serde_json::from_value::(value) .map(PrimitiveRequest::HealthRead) .map(ApplicationSurfaceRequest::Primitive) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::HealthDelta => { serde_json::from_value::(value) .map(PrimitiveRequest::HealthDelta) .map(ApplicationSurfaceRequest::Primitive) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::StorageStatus => { serde_json::from_value::(value) .map(PrimitiveRequest::StorageStatus) .map(ApplicationSurfaceRequest::Primitive) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::DiagnosticsRead => { serde_json::from_value::(value) .map(PrimitiveRequest::DiagnosticsRead) .map(ApplicationSurfaceRequest::Primitive) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::ObservatoryRead => serde_json::from_value(value) .map(ApplicationSurfaceRequest::ObservatoryRead) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::ConfigurationList | ApplicationSurfaceOperation::ConfigurationGet | ApplicationSurfaceOperation::ConfigurationSet @@ -706,52 +732,52 @@ pub fn parse_application_surface_request( | ApplicationSurfaceOperation::ConfigurationAudit => { configuration_wire_request_from_invocation_payload(operation.as_str(), value) .map(ApplicationSurfaceRequest::Configuration) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } ApplicationSurfaceOperation::ContextScoutStatus => serde_json::from_value(value) .map(ContextScoutSurfaceRequestV1::Status) .map(ApplicationSurfaceRequest::ContextScout) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::ContextScoutRecent => serde_json::from_value(value) .map(ContextScoutSurfaceRequestV1::Recent) .map(ApplicationSurfaceRequest::ContextScout) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::ContextScoutExplain => serde_json::from_value(value) .map(ContextScoutSurfaceRequestV1::Explain) .map(ApplicationSurfaceRequest::ContextScout) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::ContextScoutCapability => serde_json::from_value(value) .map(ContextScoutSurfaceRequestV1::Capability) .map(ApplicationSurfaceRequest::ContextScout) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::ContextScoutBudget => serde_json::from_value(value) .map(ContextScoutSurfaceRequestV1::Budget) .map(ApplicationSurfaceRequest::ContextScout) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::ContextScoutPause => serde_json::from_value(value) .map(ContextScoutSurfaceRequestV1::Pause) .map(ApplicationSurfaceRequest::ContextScout) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::ContextScoutResume => serde_json::from_value(value) .map(ContextScoutSurfaceRequestV1::Resume) .map(ApplicationSurfaceRequest::ContextScout) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::ContextScoutCancel => serde_json::from_value(value) .map(ContextScoutSurfaceRequestV1::Cancel) .map(ApplicationSurfaceRequest::ContextScout) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::ContextScoutClaim => serde_json::from_value(value) .map(ContextScoutSurfaceRequestV1::Claim) .map(ApplicationSurfaceRequest::ContextScout) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::ContextScoutDelivery => serde_json::from_value(value) .map(ContextScoutSurfaceRequestV1::Delivery) .map(ApplicationSurfaceRequest::ContextScout) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::ContextScoutFeedback => serde_json::from_value(value) .map(ContextScoutSurfaceRequestV1::Feedback) .map(ApplicationSurfaceRequest::ContextScout) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), ApplicationSurfaceOperation::FeedbackDiagnostics | ApplicationSurfaceOperation::FeedbackGet | ApplicationSurfaceOperation::FeedbackExpand @@ -759,7 +785,7 @@ pub fn parse_application_surface_request( | ApplicationSurfaceOperation::FeedbackImpact | ApplicationSurfaceOperation::AffectedTests => { let request: FeedbackSurfaceRequest = serde_json::from_value(value) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest)?; + .map_err(ApplicationSurfaceAdapterError::invalid_request)?; Ok(ApplicationSurfaceRequest::Feedback( FeedbackSurfaceRequest::new(request.request_handle) .map_err(|_| ApplicationSurfaceAdapterError::InvalidRequestHandle)?, @@ -767,15 +793,15 @@ pub fn parse_application_surface_request( } ApplicationSurfaceOperation::FeedbackAdvisoryCycle => { let request: FeedbackAdvisoryCycleSurfaceRequestV1 = serde_json::from_value(value) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest)?; + .map_err(ApplicationSurfaceAdapterError::invalid_request)?; request .validate() - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest)?; + .map_err(ApplicationSurfaceAdapterError::invalid_request)?; Ok(ApplicationSurfaceRequest::FeedbackAdvisoryCycle(request)) } ApplicationSurfaceOperation::FeedbackProximity => serde_json::from_value(value) .map(ApplicationSurfaceRequest::FeedbackProximity) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .map_err(ApplicationSurfaceAdapterError::invalid_request), } } diff --git a/crates/tracedecay-daemon-protocol/src/application_surface/git.rs b/crates/tracedecay-daemon-protocol/src/application_surface/git.rs index 100cee4a0f..d151255693 100644 --- a/crates/tracedecay-daemon-protocol/src/application_surface/git.rs +++ b/crates/tracedecay-daemon-protocol/src/application_surface/git.rs @@ -14,28 +14,36 @@ pub(super) fn parse_git_read_surface_request( operation: ApplicationSurfaceOperation, value: Value, ) -> Result { - let object = value - .as_object() - .ok_or(ApplicationSurfaceAdapterError::InvalidSurfaceRequest)?; + let object = value.as_object().ok_or_else(|| { + ApplicationSurfaceAdapterError::invalid_request("request must be an object") + })?; let bounded_u64 = |name: &str, default: u64, maximum: u64| match object.get(name) { None => Ok(default), Some(value) => value .as_u64() .filter(|value| (1..=maximum).contains(value)) - .ok_or(ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .ok_or_else(|| { + ApplicationSurfaceAdapterError::invalid_request(format!( + "`{name}` must be an integer between 1 and {maximum}" + )) + }), }; let boolean = |name: &str, default: bool| match object.get(name) { None => Ok(default), - Some(value) => value - .as_bool() - .ok_or(ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + Some(value) => value.as_bool().ok_or_else(|| { + ApplicationSurfaceAdapterError::invalid_request(format!("`{name}` must be a boolean")) + }), }; let optional_string = |name: &str| match object.get(name) { None => Ok(None), Some(value) => value .as_str() .map(|value| Some(value.to_owned())) - .ok_or(ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + .ok_or_else(|| { + ApplicationSurfaceAdapterError::invalid_request(format!( + "`{name}` must be a string" + )) + }), }; let max_entries = bounded_u64( "max_entries", @@ -53,13 +61,17 @@ pub(super) fn parse_git_read_surface_request( .and_then(Value::as_str) .filter(|value| !value.is_empty() && value.trim() == *value) .map(str::to_owned) - .ok_or(ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .ok_or_else(|| { + ApplicationSurfaceAdapterError::invalid_request(format!( + "`{name}` is required and must be a non-empty trimmed string" + )) + }) }; let scope_name = match object.get("scope") { None => "working_tree", - Some(value) => value - .as_str() - .ok_or(ApplicationSurfaceAdapterError::InvalidSurfaceRequest)?, + Some(value) => value.as_str().ok_or_else(|| { + ApplicationSurfaceAdapterError::invalid_request("`scope` must be a string") + })?, }; let scope = |allow_commit_range: bool| match scope_name { "working_tree" if !object.contains_key("base") && !object.contains_key("head") => { @@ -70,11 +82,13 @@ pub(super) fn parse_git_read_surface_request( } "commit_range" if allow_commit_range => Ok(GitDiffScopeV1::CommitRange { base: GitOidV1::new(string("base")?) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest)?, + .map_err(ApplicationSurfaceAdapterError::invalid_request)?, head: GitOidV1::new(string("head")?) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest)?, + .map_err(ApplicationSurfaceAdapterError::invalid_request)?, }), - _ => Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + _ => Err(ApplicationSurfaceAdapterError::invalid_request(format!( + "`scope` `{scope_name}` is not one of working_tree, staged, or commit_range for this operation, or carries base/head outside commit_range" + ))), }; let request = match operation { ApplicationSurfaceOperation::GitStatus => GitReadRequestV1::Status, @@ -95,7 +109,11 @@ pub(super) fn parse_git_read_surface_request( scope: scope(false)?, daemon_binding: None, }, - _ => return Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + _ => { + return Err(ApplicationSurfaceAdapterError::invalid_request( + "operation is not a Git read", + )); + } }; let allowed = match operation { ApplicationSurfaceOperation::GitStatus => &["max_entries", "max_bytes"][..], @@ -116,8 +134,10 @@ pub(super) fn parse_git_read_surface_request( ApplicationSurfaceOperation::GitHunks => &["scope", "max_entries", "max_bytes"][..], _ => &[], }; - if object.keys().any(|key| !allowed.contains(&key.as_str())) { - return Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest); + if let Some(unknown) = object.keys().find(|key| !allowed.contains(&key.as_str())) { + return Err(ApplicationSurfaceAdapterError::invalid_request(format!( + "unknown field `{unknown}`" + ))); } Ok(GitReadSurfaceRequest { request, diff --git a/crates/tracedecay-daemon-service/src/application_surface/configuration_wire.rs b/crates/tracedecay-daemon-service/src/application_surface/configuration_wire.rs index 8d97761a56..0c8febfac5 100644 --- a/crates/tracedecay-daemon-service/src/application_surface/configuration_wire.rs +++ b/crates/tracedecay-daemon-service/src/application_surface/configuration_wire.rs @@ -63,11 +63,11 @@ pub(super) fn configuration_binding_has_schema( pub(super) fn configuration_invocation_payload( request: &tracedecay_contracts::ConfigurationWireRequestV1, ) -> Result { - let mut wire = serde_json::to_value(request) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest)?; - wire.get_mut("request") - .map(Value::take) - .ok_or(ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + let mut wire = + serde_json::to_value(request).map_err(ApplicationSurfaceAdapterError::invalid_request)?; + wire.get_mut("request").map(Value::take).ok_or_else(|| { + ApplicationSurfaceAdapterError::invalid_request("configuration wire request has no body") + }) } fn payload_decodes(payload: Option<&Value>) -> bool { diff --git a/crates/tracedecay-daemon-service/src/application_surface/dispatch.rs b/crates/tracedecay-daemon-service/src/application_surface/dispatch.rs index 12b9d674a0..e4ed2774d5 100644 --- a/crates/tracedecay-daemon-service/src/application_surface/dispatch.rs +++ b/crates/tracedecay-daemon-service/src/application_surface/dispatch.rs @@ -56,7 +56,9 @@ pub fn application_surface_dispatch_input_with_controls( requested_format: RequestedOutputFormat, ) -> Result, ApplicationSurfaceAdapterError> { if !request.matches(operation) { - return Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest); + return Err(ApplicationSurfaceAdapterError::invalid_request( + "request body does not belong to the addressed operation", + )); } Ok(DispatchInput { request_id, @@ -106,7 +108,7 @@ pub async fn execute_application_surface( .ok_or(ApplicationSurfaceAdapterError::UnknownOrNotAuthorized)?; ( i64::try_from(capability.deadline().maximum_millis()) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest)? + .map_err(ApplicationSurfaceAdapterError::invalid_request)? .saturating_mul(1_000), capability.cancellation().clone(), capability.terminal_states().clone(), @@ -143,7 +145,7 @@ pub async fn execute_application_surface( ApplicationSurfaceRequest::Feedback(request), ) => Some( serde_json::to_value(request) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest)?, + .map_err(ApplicationSurfaceAdapterError::invalid_request)?, ), _ => None, }; @@ -393,7 +395,11 @@ pub async fn execute_application_surface( }); let request = request .with_resolved_scope(resolved_scope) - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest)? + .map_err(|problem| { + ApplicationSurfaceAdapterError::invalid_request(format!( + "resolved scope was refused: {problem:?}" + )) + })? .with_delivery_route(delivery_route); let Some(executor) = executor else { return Ok(ApplicationSurfaceInvocationResult { diff --git a/crates/tracedecay-daemon-service/src/application_surface/feedback_observation.rs b/crates/tracedecay-daemon-service/src/application_surface/feedback_observation.rs index 6c33468cbe..49b221b265 100644 --- a/crates/tracedecay-daemon-service/src/application_surface/feedback_observation.rs +++ b/crates/tracedecay-daemon-service/src/application_surface/feedback_observation.rs @@ -186,7 +186,7 @@ pub(super) fn surface_rejection_metadata( FeedbackArgumentRejectionClassV1::InvalidShape, FeedbackOutcomeV1::Rejected, )), - ApplicationSurfaceAdapterError::InvalidSurfaceRequest => Some(( + ApplicationSurfaceAdapterError::InvalidSurfaceRequest { .. } => Some(( FeedbackRejectedArgumentV1::RequestBody, FeedbackArgumentRejectionClassV1::InvalidShape, FeedbackOutcomeV1::Rejected, diff --git a/crates/tracedecay-daemon-service/src/application_surface/problems.rs b/crates/tracedecay-daemon-service/src/application_surface/problems.rs index 81452decf3..351aacb6ad 100644 --- a/crates/tracedecay-daemon-service/src/application_surface/problems.rs +++ b/crates/tracedecay-daemon-service/src/application_surface/problems.rs @@ -53,6 +53,17 @@ pub(super) fn application_contract_error_response(error: ApplicationContractErro StatusCode::INTERNAL_SERVER_ERROR.into_response() } +fn invalid_surface_request_problem(message: String) -> ApplicationProblem { + ApplicationProblem::InvalidRequest { + diagnostic: SafeDiagnostic { + code: "application.surface.invalid_request".to_owned(), + message, + }, + retry: RetryDirective::Never, + legal_actions: Vec::new(), + } +} + pub(super) fn http_adapter_problem( contract: ResultContractRef, request_id: RequestId, @@ -62,16 +73,11 @@ pub(super) fn http_adapter_problem( ApplicationSurfaceAdapterError::UnknownOrNotAuthorized => { ApplicationProblem::not_found_or_not_authorized(RetryDirective::Never) } - ApplicationSurfaceAdapterError::InvalidRequestHandle - | ApplicationSurfaceAdapterError::InvalidSurfaceRequest => { - ApplicationProblem::InvalidRequest { - diagnostic: SafeDiagnostic { - code: "application.surface.invalid_request".to_owned(), - message: "The application request is invalid".to_owned(), - }, - retry: RetryDirective::Never, - legal_actions: Vec::new(), - } + ApplicationSurfaceAdapterError::InvalidRequestHandle => { + invalid_surface_request_problem("The application request is invalid".to_owned()) + } + ApplicationSurfaceAdapterError::InvalidSurfaceRequest { detail } => { + invalid_surface_request_problem(format!("The application request is invalid: {detail}")) } // Catalog composition is derived from `const` application specs, so // these failures are deterministic for the lifetime of the process. @@ -139,7 +145,7 @@ pub fn mcp_project_open_reset_refusal( pub(crate) fn current_micros() -> Result { tracedecay_contracts::clock::try_now_micros() - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } pub(super) fn invocation_problem( diff --git a/crates/tracedecay-daemon-service/src/application_surface/retained.rs b/crates/tracedecay-daemon-service/src/application_surface/retained.rs index 22197c750a..1ece683e02 100644 --- a/crates/tracedecay-daemon-service/src/application_surface/retained.rs +++ b/crates/tracedecay-daemon-service/src/application_surface/retained.rs @@ -364,7 +364,7 @@ pub(super) fn outcome_value( payload .map(serde_json::to_value) .transpose() - .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + .map_err(ApplicationSurfaceAdapterError::invalid_request) } Ok(match outcome { diff --git a/crates/tracedecay-daemon-service/src/application_surface/tests.rs b/crates/tracedecay-daemon-service/src/application_surface/tests.rs index e08e80d4fa..51188d4b35 100644 --- a/crates/tracedecay-daemon-service/src/application_surface/tests.rs +++ b/crates/tracedecay-daemon-service/src/application_surface/tests.rs @@ -603,7 +603,7 @@ fn git_read_parser_rejects_values_outside_the_catalog_schema() { ] { assert!(matches!( parse_application_surface_request(operation, args), - Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest { .. }) )); } } @@ -853,7 +853,7 @@ fn context_scout_controls_and_claims_preserve_the_exact_address() { ApplicationSurfaceOperation::ContextScoutPause, claim_body, ), - Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest { .. }) )); let work = serde_json::json!({ @@ -1383,14 +1383,14 @@ fn feedback_cycle_projections_require_the_canonical_handle() { assert!(matches!( parse_application_surface_request(operation, serde_json::json!({"node_id": "node"})), - Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest { .. }) )); assert!(matches!( parse_application_surface_request( operation, serde_json::json!({"files": ["src/lib.rs"]}) ), - Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest { .. }) )); assert!(matches!( parse_application_surface_request( @@ -1423,7 +1423,7 @@ fn explicit_feedback_cycle_accepts_only_a_document_uri() { "request_handle": "rh.client.selected" }), ), - Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest { .. }) )); } @@ -1443,7 +1443,7 @@ fn callable_code_page_is_transport_owned() { ); assert!(matches!( rejected, - Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest { .. }) )); let rejected = parse_application_surface_request( @@ -1460,7 +1460,7 @@ fn callable_code_page_is_transport_owned() { ); assert!(matches!( rejected, - Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest) + Err(ApplicationSurfaceAdapterError::InvalidSurfaceRequest { .. }) )); } @@ -2128,7 +2128,7 @@ async fn sse_resume_after_memory_restart_returns_canonical_expired_problem() { #[test] fn surface_rejection_metadata_distinguishes_invalid_input_from_authorization() { assert_eq!( - surface_rejection_metadata(&ApplicationSurfaceAdapterError::InvalidSurfaceRequest), + surface_rejection_metadata(&ApplicationSurfaceAdapterError::invalid_request("detail")), Some(( FeedbackRejectedArgumentV1::RequestBody, FeedbackArgumentRejectionClassV1::InvalidShape, diff --git a/crates/tracedecay/src/mcp/tools/handlers/application_surface.rs b/crates/tracedecay/src/mcp/tools/handlers/application_surface.rs index 0ce3e176e8..3c4896257b 100644 --- a/crates/tracedecay/src/mcp/tools/handlers/application_surface.rs +++ b/crates/tracedecay/src/mcp/tools/handlers/application_surface.rs @@ -237,7 +237,7 @@ fn application_surface_dispatch_error( AdapterError::UnknownOrNotAuthorized => { ("application_surface_not_found_or_not_authorized", false) } - AdapterError::InvalidRequestHandle | AdapterError::InvalidSurfaceRequest => { + AdapterError::InvalidRequestHandle | AdapterError::InvalidSurfaceRequest { .. } => { ("application_surface_invalid_request", false) } AdapterError::Catalog(_)