Skip to content

feat(mcp): read tools for customers, orders, payments and balances - #1216

Merged
mforce merged 9 commits into
mainfrom
feat/808-mcp-commerce-reads
Oct 11, 2026
Merged

mforce merged 9 commits into
mainfrom
feat/808-mcp-commerce-reads

Conversation

@mforce

@mforce mforce commented Oct 11, 2026 •

Copy link
Copy Markdown
Owner

Closes #808
Closes #810

Four read tools let a connected app answer sales questions: who the customers are, what was sold, what was paid, and who still owes money. Every amount arrives as a decimal string in its currency's precision, so a model never sees minor units. The PR also finishes #810: a tool can mirror several REST routes, and every tool's scope is checked.

Tools

Tool Role policy Scope Bound Mirrors
cluckwork_list_customers SalesFlow farm:read 1 to 100 per page, paged in the query ListCustomers
cluckwork_list_sales_orders SalesFlow farm:read 1 to 100 per page, paged in the query ListSalesOrders, GetSalesOrder
cluckwork_list_order_payments SalesAccess farm:read 1 to 100 per page, one order's payments paged in memory ListOrderPayments
cluckwork_list_customer_balances SalesAccess farm:read 1 to 100 per page, paged in the query ListCustomerBalances

Each tool is ReadOnly = true, constructs McpCallContext, and returns McpPage<T> built by McpPage<T>.ReadAsync.

  • The order list decides the money tier live. As GET /sales does (Sales: the Orders list cannot show which orders are unpaid #769), it asks AuthPolicies.SalesAccess through IAuthorizationService on each call. A Worker gets each order with outstanding: null, and unpaid: true is refused. McpCallContext now exposes the request's ClaimsPrincipal for this.
  • One order tool covers list and detail. Each row carries its lines, so the tool mirrors both ListSalesOrders and GetSalesOrder.
  • Payments check the order first. As the route does, the tool refuses an order id the farm cannot see. An order refuses overpayment, so its payment rows stay few; the tool pages them in memory, and a ponytail: comment names that ceiling.
  • Balances take their currency from farm settings. CustomerBalance has no currency, so the tool reads IFarmModule.GetSettingsAsync, as PaymentEndpoints does. If the settings are missing, the call fails instead of falling back to two decimals.

Contract and repository

  • ICommerceModule.ListCustomerBalancesAsync(int limit, int offset, ct) returns one page of CustomerBalance.
  • PaymentRepository now builds balances as one SQL query: two grouped subqueries joined by customer, ordered by outstanding descending, then customer id. The old code loaded both grouped lists and joined them in memory. The REST route keeps its unpaged method, which uses the same query and is now ordered too.

Free text left out (#792)

These fields are not in any tool result:

  • CustomerDetails: Phone, Email, Address, Note.
  • SalesOrderDetails: VoidReason, DiscountReasonNote.
  • PaymentDetails: ReferenceNumber, Note, VoidReason.

Phone and address are free text with only a length limit, and a Worker can set them. #792 allows names but no other text, so they stay out. Email goes with them. The tools return customer names, grade names, the order reference (SO- plus eight hex digits, set by code) and enum values.

Route and scope parity (#810)

  • [MirrorsRoute] takes several names. McpRouteParityTests checks each named route separately. The tool's roles must sit inside the intersection, and every name must resolve.
  • [MirrorsNoRoute] is not added. Every tool here has a REST twin.
  • Scope walk (from the 2026-10-10 comment on MCP slice 7: [MirrorsRoute] parity guard — tool RBAC cannot drift from its route #810). The MCP slice 3: map /mcp, wire RBAC and tools/list filtering #806 gate walk now requires exactly one scope policy: farm:read for a ReadOnly tool and daily-entries:write for any other. RealTree_EveryOAuthScopeIsRequiredByATool fails if any scope in OAuthScopes.All is required by no tool. It reads the scopes from the policies' OAuthScopeRequirement, not from their names, and it reads the policies on the tool's class as well as on the method, as the SDK does. HiddenTool_IsRefused_WhenCalledAnyway now also calls the real cluckwork_record_daily_entry with a farm:read-only token, which is refused. The tools/list theory already shows that token gets no write tool.

The proof mutation. /api/v1/customers restricted to AdminOnly in Program.cs, nothing under Mcp/ changed:

Failed McpRouteParityTests.RealTree_EveryToolMirrorsARoute_AndAdmitsNoRoleItDenies
Cluckwork.Api.Mcp.CommerceTools.ListCustomersAsync admits Sales, which ListCustomers denies
Cluckwork.Api.Mcp.CommerceTools.ListCustomersAsync admits Worker, which ListCustomers denies
Cluckwork.Api.Mcp.CommerceTools.ListCustomerBalancesAsync admits Sales, which ListCustomerBalances denies

The balance route lives under the same group, so its tool is named too.

The parity guard compares declared policies only. Changing McpCallContext's principal, or making an authorization decision inside a tool body, is a review obligation; the guard does not prove it.

Guards

  • Row 28. McpToolSurfaceTests.RealTree_ToolRepliesCarryNoMinorUnits walks every tool's reply type, read and write, the way the serializer does. It fails on any field whose name contains MinorUnit. That covers the amount and the exponent, including fields inside a nested Money.
  • Row 31. McpCommerceToolsTests.EveryCommerceTool_PagesWithCompletenessAndContinuation checks all four tools.
  • Row 31b. BalancePage_ReadsOnlyThePageFromTheDatabase counts the rows each data reader returned, using ReaderRowCounts from MCP slice 4: read tools — flocks, stock, egg lots, daily entries #807, now shared in McpConnection. BalancePages_OrderByOutstanding_ThenCustomerId pages three at a time through 24 tied balances against real Postgres, which pushes Postgres into its unstable top-N sort. Only the customer-id tie-break keeps every customer, once, in order.
  • Row 26. CrossFarmBearer_ReadsNoneOfTheOtherFarmsSales.
  • Decide how farm-controlled free text is presented to a model before the MCP tool surface exists #792. The reply-text list covers the new fields. Amounts_AreDecimalsInTheirCurrencysPrecision plants a note and a reference and checks that no reply contains them.

Mutations

Each mutant below is a row in tools/oauth/mutation-check.sh, run with MUTANT_FILTER. The baseline and the restore run were green (415/415 for the #808 rows, 444/444 for the #810 rows).

Mutant What it breaks Test that fails
reply-minor-units a long? OutstandingMinorUnits on SalesOrderRow RealTree_ToolRepliesCarryNoMinorUnits
write-reply-minor-units a long TotalMinorUnits on the write tool's RecordedDailyEntry RealTree_ToolRepliesCarryNoMinorUnits
balances-paged-in-memory the balance query loads the book, then pages BalancePage_ReadsOnlyThePageFromTheDatabase (row 31 stays green)
balances-no-tie-break no customer id in the balance ordering BalancePages_OrderByOutstanding_ThenCustomerId
money-tier-ignored the order list always shows what is owed Worker_ListsOrdersWithoutWhatTheyOwe_AndCannotFilterByIt
payments-wider-than-route the payments tool on SalesFlow RealTree_EveryToolMirrorsARoute_AndAdmitsNoRoleItDenies
customer-note-returned a Note on CustomerRow RealTree_ReadToolRepliesReturnOnlyReviewedText
balance-currency-assumed balances rendered with two decimals Amounts_AreDecimalsInTheirCurrencysPrecision
customers-route-admin-only /api/v1/customers on AdminOnly, nothing under Mcp/ RealTree_EveryToolMirrorsARoute_AndAdmitsNoRoleItDenies
parity-first-route-only the parity walk checks only the first named route ToolBreakingParity_IsAFinding
scope-unmatched-to-readonly the gate walk no longer ties scope to ReadOnly ToolMissingAGate_IsAGateFinding
write-tool-on-read-scope cluckwork_record_daily_entry on farm:read RealTree_EveryToolCarriesARolePolicyAndAScopePolicy
write-scope-unused the same edit, from the other side RealTree_EveryOAuthScopeIsRequiredByATool
scope-use-method-only the scope-use check ignores the tool's class ScopeDeclaredOnTheToolClass_CountsAsRequired

The #808 rows ran against a 415-test baseline, and the #810 rows against 444 after merging main. After the review fixes, reply-minor-units, write-reply-minor-units and scope-use-method-only ran against 445, with baseline and restore green. A first #808 run left its mutations applied, because the two new files were missing from the script's restore list. That is fixed, and the rerun above restored each file between mutants.

Verification

Local, filtered runs: the Application architecture and CommerceModuleTests suites (391 passed). Integration suites: Mcp.*, PaymentsTests, RoleMatrixTests, AdminGatingTests and BodyReadingEndpointTests (173 passed before the #810 changes). After merging main, the parity, surface, authorization and endpoint suites passed (89 tests).

User-facing docs (GLOSSARY, Help) belong to #811, and the tools are listed there in a comment.

The balance read now runs as one SQL query that joins two grouped
subqueries, largest outstanding first with the customer id breaking ties.
A paged overload on ICommerceModule reads only the rows it returns, for the
MCP balance tool (#808).
Amounts cross as McpMoney, a decimal string in the currency's precision.
The order list asks the SalesAccess policy live for what an order still
owes. Guard row 28 walks every reply for minor units.
Three ties came back in customer-id order without the tie-break, because
Postgres groups in that order. Twenty-four ties read three at a time make
it use its unstable top-N sort, so dropping the tie-break now fails.
…-reads

# Conflicts:
#	tests/Cluckwork.Api.IntegrationTests/Mcp/McpAuthorizationTests.cs
[MirrorsRoute] takes several route names, and a tool's roles must sit inside
their intersection. The gate walk now requires exactly one scope policy,
farm:read for a read tool and daily-entries:write for a write, and every
OAuth scope must be required by some tool (#810).
…el scopes

The money guard now covers the write tool's reply as well as the reads.
The scope-use check reads policies from the tool's declaring class, as
the SDK and the gate walk do. Both gaps were found in a Codex review.
@mforce

mforce commented Oct 11, 2026

Copy link
Copy Markdown
Owner Author

Live MCP run against a running stack

I ran the full procedure in tools/verify/features/mcp.md against 40a0fe38 on an isolated stack, using real SDK clients and the browser consent flow. Steps 1 to 12 all passed. That includes the step-up server check, the redaction positive control, the write-tool smoke test and both protocol versions. These checks cover the four new tools:

Check Result
Owner, Manager, Sales Each tool's reply matched the same caller's REST reply field for field. Customers: 102 rows over 15 pages. Orders: every filter matched REST. Payments: voided ones included and marked. Balances: no duplicates or gaps across pages, largest outstanding first, equal to GET /customers/balances.
Money USD gives 40.00. On a JPY farm the reply is "23040" and the SPA shows ¥23,040. On a KWD farm it is "23.040" and the SPA shows KWD 23.040. Every amount equals the REST minor units scaled to the currency.
Free text 11 markers were seeded through REST: customer phone, email, address and note; order discount note and void reason; payment reference, note and void reason. Each marked row appears in the MCP replies, and the marker appears 0 times. Customer names appear.
Worker outstanding is null on all 19 orders. unpaid: true is refused, as on REST. Payments and balances get -32600; REST answers 403.
ReadOnly No commerce tool in tools/list. Every call gets -32600; REST answers 403.
Foreign and missing ids Another farm's order gives the same message as a missing one; REST answers 404. A foreign customerId filter returns an empty page, like REST.

The default farm's currency is locked once it has sales (Account.CurrencyLocked). So JPY and KWD ran on two newly provisioned farms, set through the Settings page.

consent 1280 light
consent 1280 dark
JPY farm sales in the SPA
KWD farm sales in the SPA

These findings do not block this PR:

@mforce

mforce commented Oct 11, 2026

Copy link
Copy Markdown
Owner Author

A Codex review at 40a0fe38 found no product defect and two guard gaps. Both are fixed in 6a1b11ff, with no product code changed.

  • F1 (P2), the money guard checked only read replies. RealTree_ToolRepliesCarryNoMinorUnits now walks every tool method, the write tool included, and still requires McpMoney.Amount to appear. New mutant write-reply-minor-units adds long TotalMinorUnits to RecordedDailyEntry: killed, RecordedDailyEntry.TotalMinorUnits.
  • F2 (P3), the scope-use check skipped class-level policies. RealTree_EveryOAuthScopeIsRequiredByATool now reads the declaring class's attributes too, as the SDK and GateFindings do. ScopeDeclaredOnTheToolClass_CountsAsRequired covers a tool whose write scope sits only on its class. New mutant scope-use-method-only drops the class attributes: killed, Collections differ.
  • Exposed principal. The PR body now says the parity guard compares declared policies only. Changing McpCallContext's principal, or deciding authorization inside a tool body, is a review obligation.

tools/oauth/mutation-check.sh with MUTANT_FILTER for reply-minor-units, write-reply-minor-units and scope-use-method-only: baseline green 445/445, all three killed, restore green 445/445.

@mforce
mforce marked this pull request as ready for review October 11, 2026 05:25
@mforce
mforce merged commit ce3d5cb into main Oct 11, 2026
21 of 22 checks passed
@mforce
mforce deleted the feat/808-mcp-commerce-reads branch October 11, 2026 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant