diff --git a/src/Capacitor.Cli/Commands/McpReviewServer.cs b/src/Capacitor.Cli/Commands/McpReviewServer.cs index 750a2b9b2..365b0e393 100644 --- a/src/Capacitor.Cli/Commands/McpReviewServer.cs +++ b/src/Capacitor.Cli/Commands/McpReviewServer.cs @@ -395,7 +395,12 @@ record McpTool(string Name, string Description, McpInputSchema InputSchema); record McpInputSchema(string Type, Dictionary Properties, string[] Required); -record McpSchemaProperty(string Type, string Description); +// Items was added when the work-items server declared the first `array`-typed properties in any of +// these MCP servers, and an array with no `items` is incomplete JSON Schema — a strict client can +// reject it, and a model has to guess the element type. Optional and trailing, so every existing +// `new("string", "…")` call is unchanged, and omitted from the wire entirely when null +// (DefaultIgnoreCondition = WhenWritingNull below). +record McpSchemaProperty(string Type, string Description, McpSchemaProperty? Items = null); record McpToolCallResult(McpContentItem[] Content, bool? IsError = null); diff --git a/src/Capacitor.Cli/Commands/McpWorkItemsServer.cs b/src/Capacitor.Cli/Commands/McpWorkItemsServer.cs index 083777300..d7bbb1d6f 100644 --- a/src/Capacitor.Cli/Commands/McpWorkItemsServer.cs +++ b/src/Capacitor.Cli/Commands/McpWorkItemsServer.cs @@ -129,6 +129,21 @@ string baseUrl using var httpResponse = toolName switch { "declare_work_item" => await SendWithRefreshRetryAsync(client, baseUrl, c => c.PostAsync($"{baseUrl}/api/work-items/declare", ToJsonContent(BuildDeclareBody(arguments)))), "get_session_work_items" => await SendWithRefreshRetryAsync(client, baseUrl, c => c.GetAsync(BuildSessionUrl(baseUrl, arguments))), + + // The declared breakdown/relation surface. Every id is a + // REQUIRED argument here, unlike session_id: there is no ambient "current work item" + // to fall back to, and guessing one would attach the wrong graph edge. + "declare_work_breakdown" => await SendWithRefreshRetryAsync(client, baseUrl, c => c.PostAsync( + ItemUrl(baseUrl, arguments, "parent_id", "breakdown"), ToJsonContent(BuildBreakdownBody(arguments)))), + "retract_work_breakdown" => await SendWithRefreshRetryAsync(client, baseUrl, c => c.PostAsync( + ItemUrl(baseUrl, arguments, "parent_id", "breakdown/retract"), ToJsonContent(BuildBreakdownBody(arguments)))), + "declare_work_relation" => await SendWithRefreshRetryAsync(client, baseUrl, c => c.PostAsync( + ItemUrl(baseUrl, arguments, "from_id", "relations"), ToJsonContent(BuildRelationBody(arguments)))), + "retract_work_relation" => await SendWithRefreshRetryAsync(client, baseUrl, c => c.PostAsync( + ItemUrl(baseUrl, arguments, "from_id", "relations/retract"), ToJsonContent(BuildRelationBody(arguments)))), + "get_work_item_topology" => await SendWithRefreshRetryAsync(client, baseUrl, c => c.GetAsync( + ItemUrl(baseUrl, arguments, "work_item_id", "topology"))), + _ => throw new ArgumentException($"Unknown tool: {toolName}") }; @@ -227,6 +242,124 @@ internal static JsonObject BuildDeclareBody(JsonObject? args) { internal static string BuildSessionUrl(string baseUrl, JsonObject? args) => $"{baseUrl}/api/work-items/session/{Uri.EscapeDataString(ResolveSessionId(args))}"; + /// + /// Builds a work-item-scoped URL, reading a REQUIRED id from . + /// Required with no fallback, deliberately: can default to the + /// ambient session because "the session I am running in" is unambiguous, whereas there is no + /// ambient work item — a default here would silently attach the wrong edge of the graph. + /// Escaped, so an id containing a slash or a percent cannot walk out of its path segment and hit + /// a different route. + /// + internal static string ItemUrl(string baseUrl, JsonObject? args, string idKey, string suffix) { + var id = RequireString(args, idKey); + + // Escaping alone is NOT sufficient containment. `.` is unreserved in RFC 3986, so + // EscapeDataString leaves it untouched, and a dot segment is then removed by URI + // normalization before the request is sent: an id of "." collapses + // /api/work-items/./breakdown to /api/work-items/breakdown, and ".." reaches + // /api/breakdown — a different route entirely, whose response would be attributed to the + // id the caller passed. + // + // Exactly these two, and no more (review correction): "..." and longer runs are ordinary + // path segments, not dot segments, so rejecting them would refuse an id the server might + // accept. A caller-supplied "%2e" is inert — EscapeDataString escapes the '%' itself to + // "%252e", and standard URI processing does not decode twice. + if (id is "." or "..") throw new ArgumentException($"'{idKey}' is not a valid work item id."); + + return $"{baseUrl}/api/work-items/{Uri.EscapeDataString(id)}/{suffix}"; + } + + /// Reads a required non-blank string argument, throwing the clean tool-error shape when + /// it is absent, null, blank, or the wrong JSON type. A whitespace-only id is rejected here + /// rather than escaped into a URL that would 404 for an unrelated-looking reason. + internal static string RequireString(JsonObject? args, string key) { + var node = args?[key]; + + if (node is null) throw new ArgumentException($"'{key}' is required."); + + // Shape-tested rather than try/catch (review finding): a bare catch would report an + // unrelated failure as a type error. + if (node is not JsonValue jsonValue || !jsonValue.TryGetValue(out var value)) + throw new ArgumentException($"'{key}' must be a string."); + + if (string.IsNullOrWhiteSpace(value)) throw new ArgumentException($"'{key}' must not be blank."); + + return value; + } + + // Server-side validation is NOT duplicated here — same reasoning as BuildDeclareBody's note. The + // rules the server owns (cross-repo edges, unknown/deleted ids, a parent listed among its own + // parts, self-relations, an empty parts list, the relation_kind vocabulary) all surface as coded + // 4xx bodies through HandleToolCallAsync. What IS validated locally is SHAPE: a present-but- + // wrong-typed argument must fail loudly rather than be dropped, because a silently omitted + // part_ids turns a malformed declare into a differently-shaped request whose rejection reads as + // if the caller had sent nothing. + internal static JsonObject BuildBreakdownBody(JsonObject? args) { + var body = new JsonObject(); + + // Presence, not truthiness (review finding): `{"part_ids": null}` is a PRESENT wrong shape, + // and the `is { } node` form treated it as absence — silently omitting it and turning a + // malformed declare into a differently-shaped request. Explicit null now fails like any other + // wrong type. + if (args is not null && args.TryGetPropertyValue("part_ids", out var node)) { + if (node is null) throw new ArgumentException("'part_ids' must be an array of strings, not null."); + + body["part_ids"] = ReadStringArray(node, "part_ids"); + } + + return body; + } + + internal static JsonObject BuildRelationBody(JsonObject? args) { + var body = new JsonObject(); + + // to_id and relation_kind are left to the server to require and to interpret: it owns the + // vocabulary and the structural rules, and a coded 400 naming the real reason beats a guess + // made here. Every SUPPLIED string is forwarded verbatim, including "" (review finding): the + // previous `is { Length: > 0 }` form dropped an explicit empty string, so the caller got the + // server's "required" error instead of its more useful "invalid value" one. Absence stays + // absence; a present non-string still fails locally, as shape validation should. + CopySuppliedString(args, "to_id", body); + CopySuppliedString(args, "relation_kind", body); + + return body; + } + + /// Copies a string argument into the request body if the caller SUPPLIED the key at + /// all. An empty string is a supplied value and is forwarded; an explicit null or a non-string is + /// a wrong shape and throws; an absent key is left absent so the server's own "required" error + /// surfaces rather than a local guess. + static void CopySuppliedString(JsonObject? args, string key, JsonObject body) { + if (args is null || !args.TryGetPropertyValue(key, out var node)) return; + + if (node is null) throw new ArgumentException($"'{key}' must be a string, not null."); + + if (node is not JsonValue value || !value.TryGetValue(out var text)) + throw new ArgumentException($"'{key}' must be a string."); + + body[key] = text; + } + + /// Reads a JSON array of non-blank strings. Any other present shape — a bare string, an + /// object, an array holding a number or a blank — throws, so a malformed argument surfaces as a + /// validation error instead of being partially dropped. + internal static JsonArray ReadStringArray(JsonNode node, string key) { + if (node is not JsonArray array) throw new ArgumentException($"'{key}' must be an array of strings."); + + var result = new JsonArray(); + + foreach (var element in array) { + if (element is not JsonValue elementValue || !elementValue.TryGetValue(out var value)) + throw new ArgumentException($"'{key}' must contain only strings."); + + if (string.IsNullOrWhiteSpace(value)) throw new ArgumentException($"'{key}' must not contain blank entries."); + + result.Add(value); + } + + return result; + } + /// Decodes the JSON-RPC method field, returning null for a present but /// wrong-shaped value (e.g. an object) instead of throwing — a malformed request must yield /// an invalid-request response, never terminate the stdio loop. @@ -313,6 +446,52 @@ internal static McpTool[] BuildToolsList() => [ "List the work items the current session is attached to.", new("object", new() { ["session_id"] = new("string", "Session id to look up. Defaults to the current kcap-hooked session (KCAP_SESSION_ID) when omitted.") - }, [])) + }, [])), + + // The declared work-breakdown / relation surface. NOTE: no tool + // here accepts `source` or `declared_by`. The server resolves both from the authenticated + // caller and rejects a `source` of "user" outright, so exposing either would be an argument + // the server ignores at best and a spoofing surface at worst. + new("declare_work_breakdown", + "Declare that a work item is broken down into parts (sub-items). Idempotent: re-declaring an " + + "existing part is accepted and reported as existing rather than created. A part can have at " + + "most one parent, and all items must live in the same repository.", + new("object", new() { + ["parent_id"] = new("string", "The work item being broken down."), + ["part_ids"] = new("array", "Work item ids that are parts of the parent.", new("string", "A work item id.")) + }, ["parent_id", "part_ids"])), + + new("retract_work_breakdown", + "Retract a previously declared breakdown, detaching the named parts from the parent.", + new("object", new() { + ["parent_id"] = new("string", "The work item whose breakdown is being retracted."), + ["part_ids"] = new("array", "Work item ids to detach from the parent.", new("string", "A work item id.")) + }, ["parent_id", "part_ids"])), + + new("declare_work_relation", + "Declare a dependency between two work items: 'blocks' means from_id blocks to_id, " + + "'blocked_by' means from_id is blocked by to_id. Both items must live in the same repository, " + + "and an item cannot relate to itself.", + new("object", new() { + ["from_id"] = new("string", "The work item the relation starts from."), + ["to_id"] = new("string", "The work item on the other end of the relation."), + ["relation_kind"] = new("string", "Either 'blocks' or 'blocked_by'.") + }, ["from_id", "to_id", "relation_kind"])), + + new("retract_work_relation", + "Retract a previously declared dependency between two work items.", + new("object", new() { + ["from_id"] = new("string", "The work item the relation starts from."), + ["to_id"] = new("string", "The work item on the other end of the relation."), + ["relation_kind"] = new("string", "Either 'blocks' or 'blocked_by'.") + }, ["from_id", "to_id", "relation_kind"])), + + new("get_work_item_topology", + "Read a work item's declared breakdown and relations — its parent, parts, and dependencies. " + + "Scoped to what the caller can see, so items you have no access to are absent rather than hidden " + + "placeholders.", + new("object", new() { + ["work_item_id"] = new("string", "The work item whose topology to read.") + }, ["work_item_id"])) ]; } diff --git a/test/Capacitor.Cli.Tests.Unit/McpWorkItemsServerTests.cs b/test/Capacitor.Cli.Tests.Unit/McpWorkItemsServerTests.cs index 21e8e41dc..7c9b8a729 100644 --- a/test/Capacitor.Cli.Tests.Unit/McpWorkItemsServerTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/McpWorkItemsServerTests.cs @@ -152,10 +152,326 @@ public async Task Declare_body_rejects_out_of_range_pr_number() { } [Test] - public async Task Tools_list_has_two_tools() { + public async Task Tools_list_exposes_the_declare_and_breakdown_surface() { var tools = McpWorkItemsServer.BuildToolsList(); - await Assert.That(tools.Length).IsEqualTo(2); - await Assert.That(tools.Select(t => t.Name).ToArray()).IsEquivalentTo(new[] { "declare_work_item", "get_session_work_items" }); + await Assert.That(tools.Select(t => t.Name).ToArray()).IsEquivalentTo(new[] { + "declare_work_item", "get_session_work_items", + "declare_work_breakdown", "retract_work_breakdown", + "declare_work_relation", "retract_work_relation", + "get_work_item_topology" + }); + } + + // ── declared breakdown + relations ─────────────────────────────────────── + + [Test] + public async Task No_tool_advertises_a_server_owned_source_or_declared_by_argument() { + // Named "advertises", not "accepts" (review correction): without additionalProperties:false a + // JSON Schema does not forbid a caller supplying these keys, so this test proves only that we + // never invite them. The guarantee that they are never FORWARDED is a property of the body + // builders, asserted separately below. + var tools = McpWorkItemsServer.BuildToolsList(); + + // Non-vacuity only: without this the loop body would never run if BuildToolsList returned + // empty and the test would pass having checked nothing. Deliberately NOT an exact count + // (review finding) — Tools_list_exposes_the_declare_and_breakdown_surface already pins the + // exact set, and duplicating it here would make an unrelated new tool fail this test too. + await Assert.That(tools.Length).IsGreaterThan(0); + + foreach (var tool in tools) { + await Assert.That(tool.InputSchema.Properties.Keys).DoesNotContain("source") + .Because($"{tool.Name} must not advertise a server-owned field"); + await Assert.That(tool.InputSchema.Properties.Keys).DoesNotContain("declared_by") + .Because($"{tool.Name} must not advertise a server-owned field"); + } + } + + [Test] + public async Task Body_builders_never_forward_a_caller_supplied_source_or_declared_by() { + // THIS is the real guarantee: the builders whitelist their output, so even a caller that + // ignores the schema and sends these keys cannot get them onto the wire. The server resolves + // both from the authenticated identity and rejects a source of "user" outright. + const string spoofed = """ + {"parent_id":"p1","part_ids":["a"],"to_id":"b","relation_kind":"blocks", + "source":"user","declared_by":"someone-else"} + """; + + var breakdown = McpWorkItemsServer.BuildBreakdownBody(Args(spoofed)); + var relation = McpWorkItemsServer.BuildRelationBody(Args(spoofed)); + + foreach (var body in new[] { breakdown, relation }) { + await Assert.That(body.ContainsKey("source")).IsFalse(); + await Assert.That(body.ContainsKey("declared_by")).IsFalse(); + } + + // Precondition: the bodies are not empty for an unrelated reason, so the absences above mean + // "filtered out" rather than "nothing was built". + await Assert.That(breakdown.ContainsKey("part_ids")).IsTrue(); + await Assert.That(relation.ContainsKey("to_id")).IsTrue(); + } + + [Test] + public async Task Every_breakdown_tool_declares_its_ids_required() { + // Unlike session_id, these ids have no ambient fallback — a schema that marked them optional + // would invite a call with no id at all. + var byName = McpWorkItemsServer.BuildToolsList().ToDictionary(t => t.Name); + + await Assert.That(byName["declare_work_breakdown"].InputSchema.Required).IsEquivalentTo(new[] { "parent_id", "part_ids" }); + await Assert.That(byName["retract_work_breakdown"].InputSchema.Required).IsEquivalentTo(new[] { "parent_id", "part_ids" }); + await Assert.That(byName["declare_work_relation"].InputSchema.Required).IsEquivalentTo(new[] { "from_id", "to_id", "relation_kind" }); + await Assert.That(byName["retract_work_relation"].InputSchema.Required).IsEquivalentTo(new[] { "from_id", "to_id", "relation_kind" }); + await Assert.That(byName["get_work_item_topology"].InputSchema.Required).IsEquivalentTo(new[] { "work_item_id" }); + } + + [Test] + public async Task Array_properties_declare_their_element_type() { + // An `array` with no `items` is incomplete JSON Schema: a strict client can reject it and a + // model has to guess the element type. + foreach (var tool in McpWorkItemsServer.BuildToolsList()) { + foreach (var (name, property) in tool.InputSchema.Properties) { + if (property.Type != "array") continue; + + await Assert.That(property.Items).IsNotNull() + .Because($"{tool.Name}.{name} is an array and must declare items"); + await Assert.That(property.Items!.Type).IsEqualTo("string"); + } + } + } + + [Test] + public async Task Item_url_builds_the_route_and_escapes_the_id() { + var url = McpWorkItemsServer.ItemUrl("http://x", Args("""{"parent_id":"wi 1/2"}"""), "parent_id", "breakdown"); + + // The escape is what stops an id containing a slash from walking out of its path segment into + // a different route. + await Assert.That(url).IsEqualTo("http://x/api/work-items/wi%201%2F2/breakdown"); + } + + [Test] + public async Task Item_url_rejects_a_missing_blank_or_wrong_typed_id() { + var missing = Assert.Throws( + () => McpWorkItemsServer.ItemUrl("http://x", new JsonObject(), "parent_id", "breakdown")); + await Assert.That(missing!.Message).Contains("parent_id"); + + var blank = Assert.Throws( + () => McpWorkItemsServer.ItemUrl("http://x", Args("""{"parent_id":" "}"""), "parent_id", "breakdown")); + await Assert.That(blank!.Message).Contains("blank"); + + var wrongType = Assert.Throws( + () => McpWorkItemsServer.ItemUrl("http://x", Args("""{"parent_id":42}"""), "parent_id", "breakdown")); + await Assert.That(wrongType!.Message).Contains("string"); + } + + [Test] + public async Task Breakdown_body_carries_part_ids() { + var body = McpWorkItemsServer.BuildBreakdownBody(Args("""{"parent_id":"p1","part_ids":["a","b"]}""")); + + // parent_id rides the URL, not the body — sending it twice invites the two copies to diverge. + await Assert.That(body["parent_id"]).IsNull(); + await Assert.That(body["part_ids"]!.AsArray().Select(n => n!.GetValue()).ToArray()) + .IsEquivalentTo(new[] { "a", "b" }); + } + + [Test] + public async Task Breakdown_body_rejects_a_wrong_shaped_part_ids_instead_of_dropping_it() { + // Silently omitting a malformed part_ids would turn a bad declare into a differently-shaped + // request whose rejection reads as though the caller had sent nothing. + var notArray = Assert.Throws( + () => McpWorkItemsServer.BuildBreakdownBody(Args("""{"parent_id":"p1","part_ids":"a"}"""))); + await Assert.That(notArray!.Message).Contains("array"); + + var notStrings = Assert.Throws( + () => McpWorkItemsServer.BuildBreakdownBody(Args("""{"parent_id":"p1","part_ids":[1,2]}"""))); + await Assert.That(notStrings!.Message).Contains("strings"); + + var blankEntry = Assert.Throws( + () => McpWorkItemsServer.BuildBreakdownBody(Args("""{"parent_id":"p1","part_ids":["a"," "]}"""))); + await Assert.That(blankEntry!.Message).Contains("blank"); + } + + [Test] + public async Task Breakdown_body_leaves_an_absent_part_ids_to_the_server_to_reject() { + // Deliberate pass-through: the server owns the "empty parts" rule and names it in a coded 400. + var body = McpWorkItemsServer.BuildBreakdownBody(Args("""{"parent_id":"p1"}""")); + + await Assert.That(body["part_ids"]).IsNull(); + } + + [Test] + public async Task Relation_body_carries_to_id_and_relation_kind() { + var body = McpWorkItemsServer.BuildRelationBody(Args("""{"from_id":"a","to_id":"b","relation_kind":"blocks"}""")); + + await Assert.That(body["from_id"]).IsNull(); // rides the URL + await Assert.That(body["to_id"]!.GetValue()).IsEqualTo("b"); + await Assert.That(body["relation_kind"]!.GetValue()).IsEqualTo("blocks"); + } + + [Test] + public async Task Breakdown_body_rejects_an_explicit_null_part_ids() { + // A PRESENT null is a wrong shape, not absence. An earlier revision's `is { } node` form read + // it as absence and silently omitted the field (review finding). + var ex = Assert.Throws( + () => McpWorkItemsServer.BuildBreakdownBody(Args("""{"parent_id":"p1","part_ids":null}"""))); + + await Assert.That(ex!.Message).Contains("part_ids"); + } + + [Test] + public async Task Relation_body_forwards_an_explicitly_empty_string_rather_than_dropping_it() { + // Dropping "" would make the server answer "relation_kind is required" when the caller DID + // supply it — the wrong diagnosis. Forwarding it gets the server's invalid-value error, which + // is the one that helps. + var body = McpWorkItemsServer.BuildRelationBody(Args("""{"from_id":"a","to_id":"","relation_kind":""}""")); + + await Assert.That(body.ContainsKey("to_id")).IsTrue(); + await Assert.That(body["to_id"]!.GetValue()).IsEqualTo(""); + await Assert.That(body["relation_kind"]!.GetValue()).IsEqualTo(""); + } + + [Test] + public async Task Relation_body_leaves_absent_keys_absent_and_rejects_present_wrong_types() { + var absent = McpWorkItemsServer.BuildRelationBody(Args("""{"from_id":"a"}""")); + await Assert.That(absent.ContainsKey("to_id")).IsFalse(); + await Assert.That(absent.ContainsKey("relation_kind")).IsFalse(); + + var nullKind = Assert.Throws( + () => McpWorkItemsServer.BuildRelationBody(Args("""{"to_id":"b","relation_kind":null}"""))); + await Assert.That(nullKind!.Message).Contains("relation_kind"); + + var numericKind = Assert.Throws( + () => McpWorkItemsServer.BuildRelationBody(Args("""{"to_id":"b","relation_kind":7}"""))); + await Assert.That(numericKind!.Message).Contains("relation_kind"); + } + + [Test] + public async Task Item_url_rejects_dot_segment_ids_that_escaping_alone_would_not_contain() { + // `.` is unreserved in RFC 3986, so EscapeDataString leaves it alone and URI normalization + // then REMOVES the segment: "." would reach /api/work-items/breakdown and ".." would reach + // /api/breakdown — a different route whose response would be attributed to the id passed. + // The slash test does not cover this, because the hazard is normalization, not escaping. + foreach (var id in new[] { ".", ".." }) { + var ex = Assert.Throws( + () => McpWorkItemsServer.ItemUrl("http://x", Args($$"""{"parent_id":"{{id}}"}"""), "parent_id", "breakdown")); + + await Assert.That(ex!.Message).Contains("parent_id").Because($"id {id} must be rejected"); + } + + // And ONLY those two (review correction — an earlier revision rejected any all-dot id, and + // this test pinned that over-broad behaviour). "..." is an ordinary path segment, not a dot + // segment, so refusing it would reject an id the server might accept. + var threeDots = McpWorkItemsServer.ItemUrl("http://x", Args("""{"parent_id":"..."}"""), "parent_id", "breakdown"); + await Assert.That(threeDots).IsEqualTo("http://x/api/work-items/.../breakdown"); + + // A dot INSIDE an otherwise-real id is fine too — the guard must not ban the character. + var ok = McpWorkItemsServer.ItemUrl("http://x", Args("""{"parent_id":"wi.1"}"""), "parent_id", "breakdown"); + await Assert.That(ok).IsEqualTo("http://x/api/work-items/wi.1/breakdown"); + } + + [Test] + public async Task Relation_body_does_not_enumerate_the_relation_kind_vocabulary() { + // The server owns the vocabulary. Passing an unknown kind through means the caller gets the + // server's coded rejection naming the real reason, rather than a client-side guess that could + // drift from the server as kinds are added. + var body = McpWorkItemsServer.BuildRelationBody(Args("""{"from_id":"a","to_id":"b","relation_kind":"depends_on"}""")); + + await Assert.That(body["relation_kind"]!.GetValue()).IsEqualTo("depends_on"); + } + + // ── dispatch: the route/method/body pairing itself ──────────────────────── + + /// + /// Review finding: the helper tests above verify URL and body construction INDEPENDENTLY, so none + /// of them would catch a switch entry that used GET instead of POST, picked the wrong suffix, or + /// paired a route with the wrong body builder. These drive the real dispatch through a fake + /// transport and assert what would actually go on the wire. + /// + sealed class CapturingHandler : HttpMessageHandler { + public HttpMethod? Method { get; private set; } + public string? Url { get; private set; } + public string? Body { get; private set; } + public int Calls { get; private set; } + + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken ct) { + Calls++; + Method = request.Method; + Url = request.RequestUri?.ToString(); + Body = request.Content is null ? null : await request.Content.ReadAsStringAsync(ct); + + return new HttpResponseMessage(System.Net.HttpStatusCode.OK) { Content = new StringContent("{}") }; + } + } + + static async Task DispatchAsync(string toolName, string argsJson) { + var handler = new CapturingHandler(); + using var client = new HttpClient(handler); + + // Built as nodes rather than an interpolated raw string: the trailing brace run in a + // hand-written JSON-RPC envelope collides with raw-string interpolation delimiters. + var request = new JsonObject { + ["params"] = new JsonObject { + ["name"] = toolName, + ["arguments"] = JsonNode.Parse(argsJson) + } + }; + + await McpWorkItemsServer.HandleToolCallAsync(JsonValue.Create(1)!, request, client, "http://x"); + + return handler; + } + + [Test] + public async Task Dispatch_declare_work_breakdown_posts_part_ids_to_the_breakdown_route() { + var h = await DispatchAsync("declare_work_breakdown", """{"parent_id":"p1","part_ids":["a","b"]}"""); + + await Assert.That(h.Calls).IsEqualTo(1); + await Assert.That(h.Method).IsEqualTo(HttpMethod.Post); + await Assert.That(h.Url).IsEqualTo("http://x/api/work-items/p1/breakdown"); + await Assert.That(h.Body).IsEqualTo("""{"part_ids":["a","b"]}"""); + } + + [Test] + public async Task Dispatch_retract_work_breakdown_targets_the_retract_route_with_the_same_body() { + var h = await DispatchAsync("retract_work_breakdown", """{"parent_id":"p1","part_ids":["a"]}"""); + + await Assert.That(h.Method).IsEqualTo(HttpMethod.Post); + await Assert.That(h.Url).IsEqualTo("http://x/api/work-items/p1/breakdown/retract"); + await Assert.That(h.Body).IsEqualTo("""{"part_ids":["a"]}"""); + } + + [Test] + public async Task Dispatch_declare_work_relation_posts_the_relation_body_not_the_breakdown_body() { + var h = await DispatchAsync("declare_work_relation", """{"from_id":"a","to_id":"b","relation_kind":"blocks"}"""); + + await Assert.That(h.Method).IsEqualTo(HttpMethod.Post); + await Assert.That(h.Url).IsEqualTo("http://x/api/work-items/a/relations"); + await Assert.That(h.Body).IsEqualTo("""{"to_id":"b","relation_kind":"blocks"}"""); + } + + [Test] + public async Task Dispatch_retract_work_relation_targets_the_relation_retract_route() { + var h = await DispatchAsync("retract_work_relation", """{"from_id":"a","to_id":"b","relation_kind":"blocked_by"}"""); + + await Assert.That(h.Method).IsEqualTo(HttpMethod.Post); + await Assert.That(h.Url).IsEqualTo("http://x/api/work-items/a/relations/retract"); + await Assert.That(h.Body).IsEqualTo("""{"to_id":"b","relation_kind":"blocked_by"}"""); + } + + [Test] + public async Task Dispatch_get_work_item_topology_is_a_GET_with_no_body() { + var h = await DispatchAsync("get_work_item_topology", """{"work_item_id":"wi-1"}"""); + + await Assert.That(h.Method).IsEqualTo(HttpMethod.Get); + await Assert.That(h.Url).IsEqualTo("http://x/api/work-items/wi-1/topology"); + await Assert.That(h.Body).IsNull(); + } + + [Test] + public async Task Dispatch_of_a_breakdown_tool_with_a_missing_id_never_reaches_the_network() { + // The local validation must fail BEFORE a request is built — otherwise a malformed call would + // hit some other route and the error would describe the wrong thing. + var h = await DispatchAsync("declare_work_breakdown", """{"part_ids":["a"]}"""); + + await Assert.That(h.Calls).IsEqualTo(0); } }