diff --git a/.config/lychee.toml b/.config/lychee.toml index 99784d28..6657066f 100644 --- a/.config/lychee.toml +++ b/.config/lychee.toml @@ -1,2 +1,3 @@ exclude_loopback = true no_progress = true +exclude_path = ["(^|/)node_modules/"] diff --git a/.config/mise/tasks/github/workflows/ci/rust-test b/.config/mise/tasks/github/workflows/ci/rust-test index 7b77a9e6..20e5c248 100755 --- a/.config/mise/tasks/github/workflows/ci/rust-test +++ b/.config/mise/tasks/github/workflows/ci/rust-test @@ -1,6 +1,14 @@ #!/usr/bin/env bash -#MISE description="Run cargo test across the Rust workspace (reads TROGON_ATLAS_TEST_NATS_URL and TROGON_ATLAS_EXTRA_FIXTURE_DIRS from the environment)" +#MISE description="Run cargo test across the Rust workspace (reads TROGON_ATLAS_TEST_NATS_URL and TROGON_ATLAS_EXTRA_FIXTURE_DIRS from the environment; derives DOCKER_HOST from the active Docker context when unset)" set -euo pipefail +if [[ -z "${DOCKER_HOST:-}" ]] && command -v docker >/dev/null 2>&1; then + context_host="$(docker context inspect --format '{{.Endpoints.docker.Host}}' 2>/dev/null || true)" + if [[ "$context_host" == unix:* ]]; then + socket_path="$(sed -E 's#^unix:/*##' <<<"$context_host")" + export DOCKER_HOST="unix:///${socket_path}" + fi +fi + cd "$MISE_PROJECT_ROOT/rsworkspace" exec cargo test --workspace diff --git a/AGENTS.md b/AGENTS.md index 84b9b27b..40413033 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,3 +27,8 @@ Documentation is a valid first implementation of a workflow. Extend the recorded procedure as repairs reveal repeated steps, then move deterministic steps into maintained, tested tools. Do not leave the only reusable knowledge in a chat, memory entry, session note, or temporary script. + +## Pull request titles + +The SemConv check accepts only `feat`, `fix` and `chore`. Title documentation +changes `chore(docs): `, not `docs: `. diff --git a/docs/how-to/repair-validation-findings.md b/docs/how-to/repair-validation-findings.md index d01b55d7..22d84cf5 100644 --- a/docs/how-to/repair-validation-findings.md +++ b/docs/how-to/repair-validation-findings.md @@ -113,8 +113,8 @@ If a contract needs a new version, follow the ## Preview and apply the guarded change For a related set of edits, use `batch_mutate_json`. The outer MCP argument -is `request_json`, a JSON-encoded string containing a proto3 JSON -`BatchMutateRequest`. Inside that request, use `validateOnly: true` for the +is `request_json`, a proto3 JSON `BatchMutateRequest` object (a JSON-encoded +string is also accepted). Inside that request, use `validateOnly: true` for the preview and `ifMatch` on each existing-entity put. Each `entity` is an `Entity` wrapper, such as `{ "storyboard": }`. Use `createOnly: true` for an intentional new entity. diff --git a/rsworkspace/crates/trogon-atlas-mcp/src/server.rs b/rsworkspace/crates/trogon-atlas-mcp/src/server.rs index 0d58489a..addee78c 100644 --- a/rsworkspace/crates/trogon-atlas-mcp/src/server.rs +++ b/rsworkspace/crates/trogon-atlas-mcp/src/server.rs @@ -1802,19 +1802,20 @@ impl McpServer { &self, Parameters(p): Parameters, ) -> Result { - if p.entity_json.len() > MAX_JSON_BYTES { + let entity_json = p.entity_json.to_json_text(); + if entity_json.len() > MAX_JSON_BYTES { return Err(rmcp::ErrorData::invalid_params( format!( "entity_json exceeds the 4 MiB limit ({} bytes)", - p.entity_json.len() + entity_json.len() ), None, )); } let mut client = self.client(); - let entity: pb::Entity = tenant_types_for(&mut client, &json_type_scope(&p.entity_json)) + let entity: pb::Entity = tenant_types_for(&mut client, &json_type_scope(&entity_json)) .await? - .message_from_json("trogonatlas.eventmodel.v1alpha1.Entity", &p.entity_json) + .message_from_json("trogonatlas.eventmodel.v1alpha1.Entity", &entity_json) .map_err(|e| rmcp::ErrorData::invalid_params(e.to_string(), None))?; let operation_id = resolve_operation_id(p.operation_id, p.validate_only)?; let req = pb::PutEntityRequest { @@ -1849,22 +1850,23 @@ impl McpServer { &self, Parameters(p): Parameters, ) -> Result { - if p.request_json.len() > MAX_JSON_BYTES { + let request_json = p.request_json.to_json_text(); + if request_json.len() > MAX_JSON_BYTES { return Err(rmcp::ErrorData::invalid_params( format!( "request_json exceeds the 4 MiB limit ({} bytes)", - p.request_json.len() + request_json.len() ), None, )); } let mut client = self.client(); let req: pb::BatchMutateRequest = - tenant_types_for(&mut client, &json_type_scope(&p.request_json)) + tenant_types_for(&mut client, &json_type_scope(&request_json)) .await? .message_from_json( "trogonatlas.api.eventmodel.v1alpha1.BatchMutateRequest", - &p.request_json, + &request_json, ) .map_err(|e| rmcp::ErrorData::invalid_params(e.to_string(), None))?; self.ensure_can_mutate(&mut client, req.validate_only) diff --git a/rsworkspace/crates/trogon-atlas-mcp/src/tools.rs b/rsworkspace/crates/trogon-atlas-mcp/src/tools.rs index 46bbe550..62d4bb9f 100644 --- a/rsworkspace/crates/trogon-atlas-mcp/src/tools.rs +++ b/rsworkspace/crates/trogon-atlas-mcp/src/tools.rs @@ -583,13 +583,40 @@ pub struct PutEntityParams { pub operation_id: Option, } +#[derive(Debug, Deserialize)] +#[serde(transparent)] +pub struct JsonPayload(serde_json::Value); + +impl schemars::JsonSchema for JsonPayload { + fn schema_name() -> std::borrow::Cow<'static, str> { + "JsonPayload".into() + } + + fn json_schema(_: &mut schemars::SchemaGenerator) -> schemars::Schema { + schemars::json_schema!({ "type": ["object", "string"] }) + } + + fn inline_schema() -> bool { + true + } +} + +impl JsonPayload { + pub fn to_json_text(&self) -> String { + match &self.0 { + serde_json::Value::String(text) => text.clone(), + other => other.to_string(), + } + } +} + #[derive(Debug, Deserialize, schemars::JsonSchema)] #[serde(deny_unknown_fields)] pub struct PutEntityJsonParams { #[schemars( - description = "proto3 JSON of an Entity message, e.g. {\"event\": {\"id\": {\"namespace\": \"shop\", \"slug\": \"order-placed\", \"version\": 1}, \"title\": \"Order placed\"}}" + description = "proto3 JSON of an Entity message as a JSON object (a JSON-encoded string is also accepted), e.g. {\"event\": {\"id\": {\"namespace\": \"shop\", \"slug\": \"order-placed\", \"version\": 1}, \"title\": \"Order placed\"}}" )] - pub entity_json: String, + pub entity_json: JsonPayload, #[serde(default)] pub create_only: bool, #[serde(default)] @@ -611,8 +638,10 @@ pub struct PutEntityJsonParams { #[derive(Debug, Deserialize, schemars::JsonSchema)] #[serde(deny_unknown_fields)] pub struct BatchMutateJsonParams { - #[schemars(description = "proto3 JSON of a BatchMutateRequest message")] - pub request_json: String, + #[schemars( + description = "proto3 JSON of a BatchMutateRequest message as a JSON object (a JSON-encoded string is also accepted)" + )] + pub request_json: JsonPayload, } #[derive(Debug, Deserialize, schemars::JsonSchema)] @@ -976,6 +1005,57 @@ mod deny_unknown_params_tests { assert!(ok.is_ok(), "{ok:?}"); } + #[test] + fn put_entity_json_accepts_an_object_or_a_json_string() { + let entity = serde_json::json!({"event": {"title": "Order placed"}}); + let from_object = serde_json::from_value::( + serde_json::json!({ "entity_json": entity }), + ) + .unwrap(); + let from_string = serde_json::from_value::( + serde_json::json!({ "entity_json": entity.to_string() }), + ) + .unwrap(); + let parse = |p: &PutEntityJsonParams| { + serde_json::from_str::(&p.entity_json.to_json_text()).unwrap() + }; + assert_eq!(parse(&from_object), entity); + assert_eq!(parse(&from_string), entity); + } + + #[test] + fn batch_mutate_json_accepts_an_object_or_a_json_string() { + let request = serde_json::json!({"validateOnly": true, "ops": []}); + let from_object = serde_json::from_value::( + serde_json::json!({ "request_json": request }), + ) + .unwrap(); + let from_string = serde_json::from_value::( + serde_json::json!({ "request_json": request.to_string() }), + ) + .unwrap(); + let parse = |p: &BatchMutateJsonParams| { + serde_json::from_str::(&p.request_json.to_json_text()).unwrap() + }; + assert_eq!(parse(&from_object), request); + assert_eq!(parse(&from_string), request); + } + + #[test] + fn json_payload_arguments_advertise_object_and_string() { + for (schema, field) in [ + (schemars::schema_for!(PutEntityJsonParams), "entity_json"), + (schemars::schema_for!(BatchMutateJsonParams), "request_json"), + ] { + let types = &schema.as_value()["properties"][field]["type"]; + assert_eq!( + *types, + serde_json::json!(["object", "string"]), + "{schema:?}" + ); + } + } + #[test] fn apply_manifests_params_reject_unknown_field() { let err = serde_json::from_value::(serde_json::json!({ diff --git a/rsworkspace/crates/trogon-atlas-mcp/tests/operation_id_mcp_integration.rs b/rsworkspace/crates/trogon-atlas-mcp/tests/operation_id_mcp_integration.rs index 92f37156..001ecf9d 100644 --- a/rsworkspace/crates/trogon-atlas-mcp/tests/operation_id_mcp_integration.rs +++ b/rsworkspace/crates/trogon-atlas-mcp/tests/operation_id_mcp_integration.rs @@ -141,6 +141,26 @@ async fn mcp_put_entity_json_with_operation_id_is_applied_and_retrievable_via_ge mcp_service.cancel().await.unwrap(); } +#[tokio::test(flavor = "multi_thread")] +async fn mcp_put_entity_json_accepts_the_entity_as_a_json_object() { + let (client, mcp_service, _grpc_endpoint) = start_mcp_client().await; + + let entity: serde_json::Value = + serde_json::from_str(&event_json("mcp-obj", "created", 1, "Created")).unwrap(); + let put_result = client + .call_tool( + rmcp::model::CallToolRequestParams::new("put_entity_json").with_arguments(args_obj(&[ + ("entity_json", entity), + ("create_only", serde_json::json!(true)), + ])), + ) + .await + .unwrap(); + assert!(!put_result.is_error.unwrap_or(false), "{put_result:?}"); + + mcp_service.cancel().await.unwrap(); +} + #[tokio::test(flavor = "multi_thread")] async fn mcp_get_operation_reports_unspecified_for_an_id_never_claimed() { let (client, mcp_service, _grpc_endpoint) = start_mcp_client().await; diff --git a/rsworkspace/crates/trogon-atlas-server/src/validation.rs b/rsworkspace/crates/trogon-atlas-server/src/validation.rs index 7a8c9665..2ebd969a 100644 --- a/rsworkspace/crates/trogon-atlas-server/src/validation.rs +++ b/rsworkspace/crates/trogon-atlas-server/src/validation.rs @@ -4447,21 +4447,37 @@ pub fn validate_model(model: &pb::EventModel, all: &[StoredEntity]) -> Vec Some(( + "declares no entry; say which read model is observed and by whom (human persona+ui or automation processor)", + "entry", + )), + Some(entry) => match (entry.read_model.is_some(), entry.observer.is_some()) { + (false, false) => Some(( + "entry names neither a read model nor an observer; say which read model is observed and by whom (human persona+ui or automation processor)", + "entry", + )), + (false, true) => Some(( + "entry names an observer but no read model it observes; say which read model is observed", + "entry.read_model", + )), + (true, false) => Some(( + "entry names a read model but no observer; say who observes it (human persona+ui or automation processor)", + "entry.observer", + )), + (true, true) => None, + }, + }; + if let Some((detail, subject_field)) = missing_entry { issues.push(pb::ValidationIssue { severity: pb::validation_issue::Severity::Warning as i32, code: "STORYBOARD_MISSING_ENTRY".into(), message: format!( - "storyboard {}/{}@{} declares no entry; say which read model is observed and by whom (human persona+ui or automation processor)", + "storyboard {}/{}@{} {detail}", sb_ref_id.namespace, sb_ref_id.slug, sb_ref_id.version, ), subject: sb_subject.clone(), - subject_field: "entry".into(), + subject_field: subject_field.into(), ..Default::default() }); } @@ -10775,6 +10791,89 @@ mod previously_untested_rules { } } + #[test] + fn storyboard_entry_without_read_model_names_the_read_model() { + let ns = "sb"; + let mut sb = sb_missing_entry_and_outcome(ns, "story"); + if let Some(pb::entity::Kind::Storyboard(s)) = sb.kind.as_mut() { + s.entry = Some(pb::StoryboardEntry { + read_model: None, + observer: Some(pb::storyboard_entry::Observer::Human( + pb::storyboard_entry::HumanObserver::default(), + )), + }); + } + let issues = validate_model(&model_with_storyboard_member(ns, "story"), &[stored(sb)]); + let hits: Vec<_> = issues + .iter() + .filter(|i| i.code == "STORYBOARD_MISSING_ENTRY") + .collect(); + assert_eq!(hits.len(), 1, "{issues:?}"); + assert_eq!(hits[0].subject_field, "entry.read_model"); + assert!( + hits[0].message.contains("no read model"), + "{}", + hits[0].message + ); + assert!( + !hits[0].message.contains("declares no entry"), + "{}", + hits[0].message + ); + } + + fn entry_issue(entry: pb::StoryboardEntry) -> Vec { + let ns = "sb"; + let mut sb = sb_missing_entry_and_outcome(ns, "story"); + if let Some(pb::entity::Kind::Storyboard(s)) = sb.kind.as_mut() { + s.entry = Some(entry); + } + validate_model(&model_with_storyboard_member(ns, "story"), &[stored(sb)]) + .into_iter() + .filter(|i| i.code == "STORYBOARD_MISSING_ENTRY") + .collect() + } + + #[test] + fn storyboard_empty_entry_names_neither_read_model_nor_observer() { + let hits = entry_issue(pb::StoryboardEntry::default()); + assert_eq!(hits.len(), 1, "{hits:?}"); + assert_eq!(hits[0].subject_field, "entry"); + assert!( + hits[0] + .message + .contains("neither a read model nor an observer"), + "{}", + hits[0].message + ); + } + + #[test] + fn storyboard_entry_without_observer_names_the_observer() { + let hits = entry_issue(pb::StoryboardEntry { + read_model: Some(pb::ReadModelEdge::default()), + observer: None, + }); + assert_eq!(hits.len(), 1, "{hits:?}"); + assert_eq!(hits[0].subject_field, "entry.observer"); + assert!( + hits[0].message.contains("no observer"), + "{}", + hits[0].message + ); + } + + #[test] + fn storyboard_entry_with_read_model_and_observer_is_clean() { + let hits = entry_issue(pb::StoryboardEntry { + read_model: Some(pb::ReadModelEdge::default()), + observer: Some(pb::storyboard_entry::Observer::Human( + pb::storyboard_entry::HumanObserver::default(), + )), + }); + assert!(hits.is_empty(), "{hits:?}"); + } + #[test] fn storyboard_missing_entry_warns() { let ns = "sb"; @@ -10788,6 +10887,16 @@ mod previously_untested_rules { 1, "{issues:?}", ); + let hits: Vec<_> = issues + .iter() + .filter(|i| i.code == "STORYBOARD_MISSING_ENTRY") + .collect(); + assert_eq!(hits[0].subject_field, "entry"); + assert!( + hits[0].message.contains("declares no entry"), + "{}", + hits[0].message + ); } #[test]