fix(mcp): conform read-only observation tools to NL-DSL-LLM - #97
Conversation
There was a problem hiding this comment.
Validator approval after policy checks for exact head 5cb7cd6c175c4d7c69ef8a30737ce7a678ab45e0.
Ticket: ticket-078
Correlation ID: local-semcod-monag-pr-97-ticket-078
Model: zai/glm-5.3
Reviewed diff chunks: 5
Advisory LLM verdict: APPROVE
Advisory summary: Reviewed all 5 diff chunk(s). Chunk 1/5 adds ticket metadata, intent, a CLI dsl subcommand delegating to nl_contract with structured JSON output and correct exit codes, and refactors Query/parse_dsl for strict validation with new parameters. Required checks (governance x2, onedev/local-verify) all pass per protected assessment. | Chunk 2/5 refactors the MCP adapter onto a shared validated observation contract: new execute_dsl/nl_ask/describe_grammar tools, schema://current resource, strict DSL parsing (duplicate tokens, malformed values, nonfinite numbers rejected), and unified error envelopes with correct isError. All required checks (onedev/local-verify, governance/enforce, governance/remote lifecycle) pass per the protected assessment; PR reports full 362-test suite green. No blocking code or security issues found in this chunk. | Chunk 3 of 5 shows a well-structured closed observation contract (nl_contract.py) with strict schema validation before DSL dispatch, proper error envelopes, and hardened MCP/REST handlers (params type check, request size limits, allow-listed fields, correct MIME types). The advise tool now routes through the DSL executor as claimed. All required checks passed. | Chunk adds an HTTP endpoint delegating to the shared nl_contract executor with proper status mapping (200/400/500) and error envelope handling, plus a comprehensive conformance test suite covering schema parity, validation-before-dispatch, provider opt-out, provenance retention, and cross-interface (MCP/HTTP/CLI) contract consistency. Required checks all pass. | Final chunk contains integration test coverage only: REST query/DSL endpoints (validation, locale fast path, 500 envelope on scanner failure), CLI envelope behavior for ask vs legacy query, and real stdio MCP transport tests covering schema resource, execute_dsl success and monag_query isError. All required protected checks (governance / remote lifecycle, governance / enforce, onedev/local-verify) passed. No security or blocking issues found in the visible code.
Advisory findings: none
The LLM output above is advisory and was not used as the approval trust root.
Semantic review prerequisite: not_required; policy 676cb4516bbfed2a000e40b9b1b6e4a430ecc761ec546aeb53d721a1905cfdd7.
Actual PR impact radar
Exact range: 327c8196adbb4f6e01755023b212478a83e06095...5cb7cd6c175c4d7c69ef8a30737ce7a678ab45e0
Change digest: dde642266ab5bb35487b06476473911bd17585853515d24f4231b714fbab95d5
Score: 80/100 (L), estimated 124 min, split recommended: true
Affected services/components: repository-wide/unclassified
Machine-readable radar JSONL and SVG
{"actual_change":{"additions":706,"base_sha":"327c8196adbb4f6e01755023b212478a83e06095","binary_files":0,"categories":{"code":6,"configuration":1,"docs":1,"tests":1},"change_digest":"dde642266ab5bb35487b06476473911bd17585853515d24f4231b714fbab95d5","comparison":"327c8196adbb4f6e01755023b212478a83e06095...5cb7cd6c175c4d7c69ef8a30737ce7a678ab45e0","deletions":132,"file_count":9,"files":["project/ticket-078/README.md","project/ticket-078/intent.json","src/monag/cli.py","src/monag/dsl.py","src/monag/dsl_llm.py","src/monag/mcp.py","src/monag/nl_contract.py","src/monag/panel.py","tests/test_nl_contract.py"],"head_sha":"5cb7cd6c175c4d7c69ef8a30737ce7a678ab45e0","service_count":0,"services":[]},"assessment_mode":"observed-pr","axes":{"coupling":5,"delivery":4,"scope":5,"uncertainty":3,"validation":3},"complexity":"L","confidence":0.9,"diagnostics":["RADAR-ACCEPTANCE-MISSING","RADAR-BUDGET-EXCEEDED"],"estimate":{"budget_minutes":30,"minutes":124,"within_budget":false},"impact":{"components":["command","project","query","src/monag","success","tests","wellmanifest"],"files":["command/result","project/ticket-078/README.md","project/ticket-078/intent.json","query/DSL/schema","src/monag/cli.py","src/monag/dsl.py","src/monag/dsl_llm.py","src/monag/mcp.py","src/monag/nl_contract.py","src/monag/panel.py","success/error","tests/test_nl_contract.py","wellmanifest/nl-dsl-llm"],"public_interfaces":["query/DSL/schema"],"runtime_dependencies":1},"schema":"subactor.ticket-radar/v1","score":80,"split":{"parts":[{"estimated_minutes":20,"name":"Define contract and acceptance boundary","scope":["query/DSL/schema"]},{"estimated_minutes":14,"name":"Implement command","scope":["command"]},{"estimated_minutes":14,"name":"Implement project","scope":["project"]},{"estimated_minutes":14,"name":"Implement query","scope":["query"]},{"estimated_minutes":14,"name":"Implement src/monag","scope":["src/monag"]},{"estimated_minutes":14,"name":"Implement success","scope":["success"]},{"estimated_minutes":15,"name":"Validate and project to trackers","scope":["tests","planfile","github/gitlab/jira projections"]}],"reason":"estimated_minutes_exceed_budget","recommended":true},"standards":[{"id":"wellmanifest/dsl","revision":"6c60fc4e0dd1f1bb74f46a7745e28019908d1203","version":"0.1.0-dev"},{"id":"wellmanifest/ticket-lifecycle","revision":"5bf581907a87b46a13a73e6c033d3abe4d9a306f","version":"0.1.0-dev"},{"id":"wellmanifest/git-lifecycle","revision":"7d77d4b7af57e69bc75c3a0290b3a4805c5c4438","version":"0.2.0-dev"},{"id":"wellmanifest/logs","revision":"48c284ef7a069055c0bcb6b900147ce5e65f8b43","version":"0.3.0"}],"ticket_ref":"ticket-078"}<svg xmlns="http://www.w3.org/2000/svg" width="128" height="128" viewBox="0 0 128 128" role="img"><title>ticket-078: fix(mcp): conform read-only observation tools to NL-DSL-LLM</title><rect width="128" height="128" rx="12" fill="#f8fafc"/><g stroke-width="1"><polygon points="64,55 72,61 69,71 59,71 56,61" fill="none" stroke="#d7dde5"/><polygon points="64,47 80,59 74,78 54,78 48,59" fill="none" stroke="#d7dde5"/><polygon points="64,38 89,56 79,85 49,85 39,56" fill="none" stroke="#d7dde5"/><polygon points="64,30 97,53 84,92 44,92 31,53" fill="none" stroke="#d7dde5"/><polygon points="64,21 105,51 89,99 39,99 23,51" fill="none" stroke="#d7dde5"/><line x1="64" y1="64" x2="64" y2="21" stroke="#aab4c0"/><line x1="64" y1="64" x2="105" y2="51" stroke="#aab4c0"/><line x1="64" y1="64" x2="89" y2="99" stroke="#aab4c0"/><line x1="64" y1="64" x2="39" y2="99" stroke="#aab4c0"/><line x1="64" y1="64" x2="23" y2="51" stroke="#aab4c0"/></g><polygon points="64,21 105,51 79,85 49,85 31,53" fill="#fb923c" fill-opacity="0.45" stroke="#c2410c" stroke-width="2"/><circle cx="64" cy="64" r="3" fill="#c2410c"/><g font-family="sans-serif" font-size="7" fill="#334155"><text x="64" y="11" text-anchor="middle">SCO</text><text x="114" y="48" text-anchor="middle">COU</text><text x="95" y="107" text-anchor="middle">UNC</text><text x="33" y="107" text-anchor="middle">VAL</text><text x="14" y="48" text-anchor="middle">DEL</text></g><text x="64" y="124" text-anchor="middle" font-family="sans-serif" font-size="8" fill="#0f172a">L · 124m</text></svg>DECISION D-078-4767
TICKET ticket-078
HEAD_SHA 5cb7cd6c175c4d7c69ef8a30737ce7a678ab45e0
CORRELATION_ID local-semcod-monag-pr-97-ticket-078
ACTOR agent:ifuri-validator-agent[bot]
APPLIED_RULE P-CORE-015
INPUT author_login = "tom-sapletta-com"
INPUT observed_checks = ["governance / remote lifecycle=PASS","governance / enforce=PASS","onedev/local-verify=PASS"]
INPUT required_checks = ["onedev/local-verify","governance / remote lifecycle","governance / enforce"]
INPUT required_checks_source = "protected registry (env/request)"
INPUT reviewer_login = "ifuri-validator-agent[bot]"
INPUT semantic_review_assessment = {"schema":"subactor.validator/semantic-review-assessment/v1","subject":{"repository":"semcod/monag","pull_request":97,"head_sha":"5cb7cd6c175c4d7c69ef8a30737ce7a678ab45e0","base_sha":"327c8196adbb4f6e01755023b212478a83e06095","diff_sha256":"466348e1e9528261445b8fda26b61ef290b0db5bf575558639c98673cb50f551"},"policy":{"policy_schema":"subactor.validator/semantic-review-policy/v1","policy_version":1,"policy_sha256":"676cb4516bbfed2a000e40b9b1b6e4a430ecc761ec546aeb53d721a1905cfdd7","required":false,"critical_paths":[],"observed_paths":["project/ticket-078/README.md","project/ticket-078/intent.json","src/monag/cli.py","src/monag/dsl.py","src/monag/dsl_llm.py","src/monag/mcp.py","src/monag/nl_contract.py","src/monag/panel.py","tests/test_nl_contract.py"]},"grounding":"full-diff-not-per-finding-proof","execution_authority":false,"status":"not_required","reason":null,"review_sha256":null,"unresolved":[]}
INPUT superseded_checks = []
INPUT ticket_radar_receipt = {"schema":"subactor.ticket-radar/v1","base_sha":"327c8196adbb4f6e01755023b212478a83e06095","head_sha":"5cb7cd6c175c4d7c69ef8a30737ce7a678ab45e0","change_digest":"dde642266ab5bb35487b06476473911bd17585853515d24f4231b714fbab95d5","score":80,"complexity":"L","estimated_minutes":124,"split_recommended":true,"services":[],"authority":"ADVISORY","promotion":"FORBIDDEN"}
VERDICT APPROVE AUTHORITY DETERMINISTIC
REJECTED REQUEST_CHANGES BECAUSE NO_UNSAFE_CHANGE_REASON_FOUND
ADVISORY llm_verdict = "APPROVE" MODEL "zai/glm-5.3"
ASSERT VERDICT_AUTHORITY != "ADVISORY"
MONAG's MCP query adapter returned Markdown without provenance and marked unrecognized queries as successful. It also lacked the standard tool names and schema resource, while the advisory tool bypassed the DSL executor.
Add
execute_dsl,nl_ask,describe_grammarandschema://current, preserving existingmonag_*text tools with structured results and correctisError. A shared closed observation schema, validator and result adapter serve MCP,monag --json ask/dsl, and versioned REST query/DSL/schema endpoints. Unknown, duplicate, malformed and nonfinite DSL parameters fail before provider or scanner dispatch. Advisory observations now use that executor; the legacy audit issue limit is forwarded correctly.This adopts the read-only observation interface of wellmanifest/nl-dsl-llm at
2040efe37b9eb898350f3fec0285a2d4e69d4e34. Mutation commands and MCP client registration are outside the claim. No new runtime dependencies or changes to concurrent governance adoption.Validation: final full application suite 362 tests and 30 subtests passed, including 12 conformance tests. Rejected LLM translation also retains attempted-provider provenance. Real stdio, HTTP and CLI tests cover success/error envelopes, provider opt-out, strict rejection and legacy tools. Independent JSON Schema validation against the pinned normative command/result schemas passed. Changed-source Ruff and governance passed.
Native Planfile PLF-028; delivery ticket-078. Closes #95. Publication requires exact-head protected checks and independent Validator approval.