[Sirajmx]

The defect

REST has POST /api/v1/orders to turn a booked deal into an order, but there was no MCP equivalent. A buyer or operator working purely over MCP could discover, quote, book, and distribute a deal, but had no tool to create the order that follows — the flow dead-ended at "distributed."

Already filed as AI-11 (High priority — "major incoherence"), reported independently of this fix.

The fix

Added create_order to mcp_server.py, calling the existing order_service.create_order(deal_id, quote_id, metadata) — no new service logic was needed, since the REST router itself is a thin wrapper around the same function. metadata is accepted as a JSON object string (matching the established convention for dict-typed MCP tool parameters elsewhere in this file, e.g. create_package's product_ids). Not operator-gated, matching REST's own _get_optional_api_key_record (optional, not required) and the sibling list_orders tool, unlike transition_order which is a genuinely admin action.

Also regenerated docs/reference/mcp-tools.md (auto-generated, drift-guarded by test_docs_inventory_drift.py — a real CI check this PR's create_order addition would otherwise fail).

Verification

New tests in tests/unit/test_mcp_server.py (TestCreateOrder, 3 tests): creates and persists an order, parses JSON metadata, and works correctly with no arguments at all (both deal_id/quote_id are optional). Negative control: broke the deal_id passthrough — exactly the one test asserting it failed, the other two (unrelated to deal_id) correctly still passed.

Full unit suite: 1524 passed (1521 baseline + 3 new), 0 failures. Integration suite: 39 passed, 28 skipped, 0 failures. ruff check/ruff format --check: clean.

Live, end-to-end: called create_order over a real MCP session with deal_id/metadata, then confirmed the exact same order via GET /api/v1/orders (REST) — real cross-surface persistence, not a tool-local mock.

A note on scope, not a defect

The tool's own docstring initially read "...persist its state machine (AI-11)" — caught and removed before this draft, since that's a public-facing tool description (shown to any MCP client, and now baked into the committed docs/reference/mcp-tools.md), not an internal comment. Different treatment than the CHANGELOG entry, which keeps the reference — a changelog is expected to carry tracking references, a tool description read by an end user or LLM client isn't.

[atc964]

Clean mirror, and delegating to order_service.create_order rather than reimplementing means this inherits #71's 404 fix for free. Docs regeneration is correct.

parsed_metadata = json.loads(metadata) returns whatever the JSON says, but REST types it metadata: Optional[dict]. So {"metadata": "spring"} is a 422 over HTTP and a stored string over MCP. That string later reaches order_meta.update(proposed) in apply_change_request, an uncaught AttributeError on a str, so an approved change request against that order 500s with no API path to repair it.

Required before merge:

1. Reject non-dict metadata after parsing. if parsed_metadata is not None and not isinstance(parsed_metadata, dict), return the error shape REST's 422 produces. Full schema validation is not necessary.
2. A test passing metadata='"spring"' asserting it is rejected rather than stored.

Follow-up, not blocking: the PR body cites create_package's product_ids as precedent. That parameter does not exist. The convention claim is right, the actual precedents are update_rate_card(entries) and bulk_deal_operations(operations).

[Sirajmx]

@atc964 Confirmed — a bare '"spring"' or '[1, 2]' parses fine via json.loads but isn't a dict, and REST's Optional[dict] typing would 422 on the same input this tool silently accepted.

Pushed 8629b84: rejects non-dict metadata up front with an invalid_metadata error before it ever reaches order_service.create_order, matching REST's error shape (skipped full schema validation as you said isn't necessary — just the type gate). Added two tests (bare string, array) confirming both are rejected and never reach storage; reverted the check and confirmed both fail as expected, then restored it.

Also noted the create_package/product_ids precedent citation in the PR body doesn't hold up (that parameter doesn't exist) — the real precedents are update_rate_card(entries) and bulk_deal_operations(operations), which is what this follows.

Full suite: 1565 passed, 28 skipped. Ready for another look.

[atc964]

The isinstance gate is right and sits before order_service.create_order, the string and array rejection tests assert set_order is never awaited, and malformed JSON behavior is unchanged from the first commit. Approving.

One correction for the record, no code change needed: this does not actually match REST's 422 shape. There is no custom RequestValidationError handler, so REST rejects a non-dict metadata with FastAPI's default Pydantic error list (detail as a list with type dict_type), not a detail object. What your check does match is this codebase's service-error envelope, the same {"detail": {"error", "message"}} that _service_json emits when order_service raises HTTPException, which is the more useful consistency for MCP callers anyway. Worth fixing the comment above the check so the next reader is not misled.

Optional polish: wrap the json.loads in a try/except returning an invalid_metadata error too, so '{bad' gets the same friendly envelope instead of a generic tool error.