An MCP server gives an AI client a callable interface. Reviewing it means following the path from a tool description to the code that reads data, changes state, or authorizes a payment. I want the interface to explain that behavior accurately, including what happens when a request fails.
These examples come from two public projects: textokit-mcp and dotnet-mcp-orders-server. The pinned textokit revision uses JavaScript modules, not TypeScript; the orders server uses C#. This article is a source-based walkthrough. I inspected the referenced code and test definitions; I did not execute their tests for this article.
1. Names and input schemas must match the implementation
I start with the interface exposed to the client. Can a caller distinguish a lookup from a cancellation? Which arguments are required? Does the implementation enforce the constraints advertised by the schema?
In textokit-mcp, paid tool names and schemas are derived from the upstream OpenAPI document:
name: 'textokit_' + route.replace(/-/g, '_'), inputSchema: leanSchema(op.requestBody?.content?.['application/json']?.schema), Source: index.mjs, buildTools.
That transformation deserves attention. The same file removes top-level composition and conditional keywords in leanSchema. Its comment says cross-field rules remain upstream. I would therefore check the upstream rejection behavior before claiming that the advertised schema captures every rule. Publishing a schema and validating a request are separate responsibilities.
2. Descriptions should state the actual boundary
A useful description tells the client what a tool can do and what must already be true. It should avoid implying access, persistence, or authority that the implementation does not provide.
The orders server makes its example data explicit:
[McpServerTool(Name = "list_orders")] [Description("List the fictional orders in this server's memory.")] public Order[] ListOrders() => orders.List(); “Fictional” and “in this server's memory” are meaningful limits. A reader should not mistake this demonstration for a connection to a production order system. I also compare descriptions with failure paths: wording that promises cancellation would be misleading if the operation still needs an independently recorded approval.
3. Failures and timeouts need an understandable contract
I look for explicit failure responses and bounded network waits. A caller needs to distinguish an unsuccessful request from a successful result containing an error-shaped string.
Textokit builds MCP error results with:
const fail = (s) => ({ ...text(s), isError: true }); Its payment requests include:
signal: AbortSignal.timeout(TIMEOUT_MS), Source: index.mjs, requests and handler.
The timeout applies to individual fetch calls. It does not establish a single deadline for the entire payment flow, which can make an initial request and a signed retry. I would also inspect configuration validation and error sanitization: the handler forwards caught exception messages. A timeout response alone cannot establish whether a remote side effect occurred, so retry behavior needs its own review.
4. Secrets must stay out of results and logs
I trace credentials from configuration into signing or authentication, then inspect output paths. That includes startup failures and exception messages, not only successful responses.
Textokit reads its wallet credential from the environment:
const rawKey = process.env.TEXTOKIT_PRIVATE_KEY; Its startup log is:
console.error(`textokit-mcp ready: ${paid.length} paid tools + ${free.length} free; wallet ${account ? account.address : 'none (free tools only)'}; cap ${MAX_CENTS} cents per call`); That log references the public address rather than the private key. This observation does not prove that every dependency or failure path protects secrets. I would still check returned upstream bodies, signing errors, and captured logs with synthetic sentinel credentials. No such execution is claimed here.
5. Writes and spending need authority outside model arguments
For consequential tools, I ask where permission originates and what limits it. An argument saying “approved” is insufficient when the caller can supply that argument itself.
The orders approval store checks an action, an exact order, and an expiration:
return approval is not null && approval.Action == "cancel_order" && approval.OrderId == orderId && approval.ExpiresAt > DateTimeOffset.UtcNow; The implementation claims the approval file through a rename and deletes the claimed file afterward. The tool description explicitly says the tool cannot create approvals. Deployment permissions must preserve that separation.
Textokit's spending boundary is different: an externally configured wallet and per-call cap authorize automatic payment. It does not provide the orders server's independent, single-use approval workflow. I would document that distinction and review recipient restrictions and cumulative spending separately.
6. Untrusted inputs remain data
Tool arguments, retrieved text, and upstream responses should not acquire authority merely because they appear in a conversation. I follow each value into paths, requests, and state changes.
Before using an order ID in an approval filename, the orders implementation applies:
public static bool IsSafeId(string id) => !string.IsNullOrEmpty(id) && id.Length <= 64 && id.All(c => char.IsAsciiLetterOrDigit(c) || c == '-'); This restricts the filename component; it is not a general prompt-injection defense. The broader review asks whether external content can bypass approval, influence destinations, or become executable instructions. An allowlist at one boundary provides evidence about that boundary only.
7. Tests should check denied actions and unchanged state
I read assertions before summarizing test coverage. A test name is weaker evidence than a check that an unauthorized operation leaves the original state intact.
The orders tests include:
var before = tools.GetOrder("ORD-1001"); var error = Assert.Throws<McpException>(() => tools.CancelOrder("ORD-1001")); Assert.Contains("No order was changed", error.Message); Assert.Equal(before, tools.GetOrder("ORD-1001")); That file also defines cases for expired, mismatched, malformed, and consumed approvals. Textokit's smoke test exercises initialization and tool discovery; it does not establish payment correctness or secret protection.
A review report should distinguish source observations, executable test definitions, and observed execution results. Each finding needs a concrete consequence, supporting evidence, and a next action that fits the server's actual scope.
If you want this checklist applied to your server, I offer a fixed-price review in 48 hours on Upwork: https://www.upwork.com/services/product/2116921806127265220
