[AI-1126] Phase D-c (CLI): materialize definition MCP allowlists — kcap-owned registry, capability strip, additive wire field

README.md

@@ -326,7 +326,7 @@ Requires `kcap login` **and a running daemon with this repo checked out** — th
 kcap mcp flow-result   # launched by the daemon — not meant to be run manually
 ```
 
-Stdio MCP server the **daemon injects into hosted review-flow reviewer sessions** (both Claude and Codex reviewers) as their only MCP server. It exposes a single tool, **`submit_review_result`** (`round_token`, `kind: "findings" | "clean"`, `findings`), which posts the reviewer's result to the Capacitor server — the reliable alternative to the transcript-marker fallback. It is deliberately separate from `kcap mcp flows` so an unattended reviewer can never start a nested flow, and it reads its identity from daemon-provided environment (`KCAP_FLOW_AGENT_ID`); run manually it just exits with an explanation. There is nothing to register or configure.
+Stdio MCP server the **daemon injects into hosted review-flow reviewer sessions** (both Claude and Codex reviewers). It exposes a single tool, **`submit_review_result`** (`round_token`, `kind: "findings" | "clean"`, `findings`), which posts the reviewer's result to the Capacitor server — the reliable alternative to the transcript-marker fallback. It is deliberately separate from `kcap mcp flows` so an unattended reviewer can never start a nested flow, and it reads its identity from daemon-provided environment (`KCAP_FLOW_AGENT_ID`); run manually it just exits with an explanation. It's not necessarily the only server a reviewer gets, though: the flow definition's `mcp:` allowlist can additionally grant kcap-owned context servers (e.g. `kcap-sessions`), resolved against the same built-in registry — unknown names are skipped and any flow-starting server is always stripped regardless of listing, so a reviewer still can't start a nested flow. There is nothing to register or configure.
 
 ### Memory MCP server (for agents)
 

src/Capacitor.Cli.Core/KcapMcpRegistry.cs

@@ -0,0 +1,27 @@
+namespace Capacitor.Cli.Core;
+
+/// <summary>AI-1126 D-c: the kcap-owned MCP server registry — the ONLY source a flow
+/// definition's mcp: allowlist resolves against (never user config). StartsFlows marks
+/// servers that can start flows; they are stripped from every allowlist regardless of
+/// listing (the recursion guard — the server strips too, this is the authoritative layer).
+/// Ids are canonical lower-case; Resolve is case-insensitive so casing can't dodge the strip.</summary>
+public sealed record KcapMcpServerDescriptor(string Id, string[] Args, bool StartsFlows);
+
+public static class KcapMcpRegistry {
+    static readonly Dictionary<string, KcapMcpServerDescriptor> Entries = new(StringComparer.OrdinalIgnoreCase) {
+        ["kcap-review"]   = new("kcap-review",   ["mcp", "review"],   false),
+        ["kcap-sessions"] = new("kcap-sessions", ["mcp", "sessions"], false),
+        ["kcap-memory"]   = new("kcap-memory",   ["mcp", "memory"],   false),
+        ["kcap-flows"]    = new("kcap-flows",    ["mcp", "flows"],    true),
+    };
+
+    /// <summary>Resolves an allowlist entry to its descriptor. Case-insensitive, trims
+    /// surrounding whitespace. A null or blank name — e.g. a wire-deserialized allowlist
+    /// element — returns null rather than throwing, so callers can route it through the
+    /// same unknown-name skip path as any other unresolvable name.</summary>
+    public static KcapMcpServerDescriptor? Resolve(string? name) {
+        if (string.IsNullOrWhiteSpace(name)) return null;
+
+        return Entries.TryGetValue(name.Trim(), out var d) ? d : null;
+    }
+}

src/Capacitor.Cli.Core/Models.cs

@@ -814,7 +814,12 @@ public readonly record struct LaunchAgentCommand(
         // a mismatch (e.g. a different machine, where the path doesn't resolve) skips the sync.
         // Appended last as an optional field so the SignalR positional binding stays wire-compatible
         // with older daemons/servers.
-        string?           SyncFromRepoRoot = null
+        string?           SyncFromRepoRoot = null,
+        // AI-1126 D-c: for a review-flow launch, the flow definition's MCP allowlist — server-owned
+        // names the daemon resolves against the kcap-owned KcapMcpRegistry and materializes into the
+        // launcher's MCP config (flow-starting servers are stripped regardless of listing). Appended
+        // last, same wire-compat rule as SyncFromRepoRoot above.
+        string[]?         McpAllowlist = null
     );
 
 /// <summary>

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs

@@ -359,7 +359,9 @@ async Task HandleLaunchAgent(LaunchAgentCommand cmd) {
                 ReviewLaunch: isReview && cmd.Review is { } reviewArgs
                     ? await ReviewLaunchBuilder.BuildAsync(cmd.Vendor, _config.CapacitorPath, _config.ServerUrl ?? "", reviewArgs.Owner, reviewArgs.Repo, reviewArgs.PrNumber)
                     : null
-            );
+            ) {
+                McpAllowlist = cmd.McpAllowlist
+            };
 
             try {
                 launcher.Prepare(launcherCtx);

src/Capacitor.Cli.Daemon/Services/ClaudeLauncher.cs

@@ -130,7 +130,13 @@ public LaunchArgs BuildArgs(LauncherContext ctx) {
     /// kcap-flow-result submission server, or the empty map when the daemon has no server
     /// URL / kcap path (zero servers is the recursion-safe default). Built via JsonNode
     /// string casts — JsonValue.Create / collection expressions lower to generic Add<T>
-    /// and trip NativeAOT (IL3050).</summary>
+    /// and trip NativeAOT (IL3050).
+    /// AI-1126 D-c: on the non-empty path, also materializes the flow definition's
+    /// <see cref="LauncherContext.McpAllowlist"/> into the same mcpServers object — each name
+    /// resolved against the kcap-owned <see cref="KcapMcpRegistry"/> (never ambient user
+    /// config), unknown names skipped, flow-starting servers stripped regardless of listing.
+    /// Allowlist servers get KCAP_URL only — never KCAP_FLOW_AGENT_ID, which is exclusive to
+    /// the flow-result submission channel.</summary>
     string BuildReviewFlowMcpConfig(LauncherContext ctx) {
         if (string.IsNullOrWhiteSpace(config.ServerUrl) || string.IsNullOrWhiteSpace(config.CapacitorPath)) return EmptyMcpConfig;
 
@@ -147,7 +153,36 @@ string BuildReviewFlowMcpConfig(LauncherContext ctx) {
             }
         };
 
-        return new JsonObject { ["mcpServers"] = new JsonObject { ["kcap-flow-result"] = server } }.ToJsonString();
+        var mcpServers = new JsonObject { ["kcap-flow-result"] = server };
+
+        foreach (var name in ctx.McpAllowlist ?? []) {
+            var descriptor = KcapMcpRegistry.Resolve(name);
+
+            if (descriptor is null) {
+                // name may be null here — a wire-deserialized allowlist element — so log it
+                // defensively rather than let a null flow into the formatter unlabeled.
+                LogAllowlistEntryUnknown(name ?? "(null)", ctx.AgentId);
+                continue;
+            }
+
+            if (descriptor.StartsFlows) {
+                LogAllowlistEntryStripped(name, ctx.AgentId);
+                continue;
+            }
+
+            var entryArgs = new JsonArray();
+            foreach (var a in descriptor.Args) entryArgs.Add((JsonNode?)a);
+
+            mcpServers[descriptor.Id] = new JsonObject {
+                ["command"] = (JsonNode?)config.CapacitorPath,
+                ["args"]    = entryArgs,
+                ["env"]     = new JsonObject {
+                    ["KCAP_URL"] = (JsonNode?)config.ServerUrl
+                }
+            };
+        }
+
+        return new JsonObject { ["mcpServers"] = mcpServers }.ToJsonString();
     }
 
     /// Claude needs no mandatory daemon-level flags (cwd is set via forkpty chdir), so a
@@ -450,4 +485,10 @@ static void WriteMcpConfig(string sourceRepoPath, string worktreePath) {
 
     [LoggerMessage(Level = LogLevel.Warning, Message = "Failed to remove mcp config for agent {AgentId}")]
     partial void LogCleanupMcpConfigFailed(Exception ex, string agentId);
+
+    [LoggerMessage(Level = LogLevel.Warning, Message = "MCP allowlist entry '{Name}' is not a kcap-owned server — skipping (agent {AgentId})")]
+    partial void LogAllowlistEntryUnknown(string name, string agentId);
+
+    [LoggerMessage(Level = LogLevel.Warning, Message = "MCP allowlist entry '{Name}' can start flows — stripped (agent {AgentId})")]
+    partial void LogAllowlistEntryStripped(string name, string agentId);
 }

src/Capacitor.Cli.Daemon/Services/CodexLauncher.cs

@@ -93,6 +93,7 @@ public LaunchArgs BuildArgs(LauncherContext ctx) {
             args.Add("-c");
             args.Add("mcp_servers={}");
             AddFlowResultServer(args, ctx);
+            AddAllowlistServers(args, ctx);
         }
 
         AddModelArg(args, ctx.Model);
@@ -131,6 +132,44 @@ void AddFlowResultServer(List<string> args, LauncherContext ctx) {
         args.Add($"mcp_servers.{name}.env={{KCAP_URL={TomlString(config.ServerUrl)},KCAP_FLOW_AGENT_ID={TomlString(ctx.AgentId)}}}");
     }
 
+    /// <summary>AI-1126 D-c: materializes the flow definition's <see cref="LauncherContext.McpAllowlist"/>
+    /// as additional dotted overrides, in the same clear-then-whitelist style as
+    /// <see cref="AddFlowResultServer"/>. Each name resolves against the kcap-owned
+    /// <see cref="KcapMcpRegistry"/> — never ambient user config — unknown names are skipped,
+    /// and any flow-starting server is stripped regardless of listing (the recursion guard).
+    /// Allowlist servers get KCAP_URL only — never KCAP_FLOW_AGENT_ID, which is exclusive to
+    /// the flow-result submission channel. Skipped (same as the flow-result server) when the
+    /// daemon has no server URL or kcap path configured.</summary>
+    void AddAllowlistServers(List<string> args, LauncherContext ctx) {
+        if (string.IsNullOrWhiteSpace(config.ServerUrl) || string.IsNullOrWhiteSpace(config.CapacitorPath)) return;
+
+        foreach (var name in ctx.McpAllowlist ?? []) {
+            var descriptor = KcapMcpRegistry.Resolve(name);
+
+            if (descriptor is null) {
+                // name may be null here — a wire-deserialized allowlist element — so log it
+                // defensively rather than let a null flow into the formatter unlabeled.
+                LogAllowlistEntryUnknown(name ?? "(null)", ctx.AgentId);
+                continue;
+            }
+
+            if (descriptor.StartsFlows) {
+                LogAllowlistEntryStripped(name, ctx.AgentId);
+                continue;
+            }
+
+            var id       = descriptor.Id;
+            var argsList = string.Join(",", descriptor.Args.Select(TomlString));
+
+            args.Add("-c");
+            args.Add($"mcp_servers.{id}.command={TomlString(config.CapacitorPath)}");
+            args.Add("-c");
+            args.Add($"mcp_servers.{id}.args=[{argsList}]");
+            args.Add("-c");
+            args.Add($"mcp_servers.{id}.env={{KCAP_URL={TomlString(config.ServerUrl)}}}");
+        }
+    }
+
     /// Append `-m <model>` unless the model is empty or the "default" no-override sentinel.
     /// "default" is the sentinel from the flow/agent dispatch; passing it as `-m default` is
     /// rejected by Codex on a ChatGPT account ("The 'default' model is not supported when using
@@ -263,4 +302,10 @@ static bool HooksInstalledIn(string hooksPath) {
 
     [LoggerMessage(Level = LogLevel.Debug, Message = "Tools array of length {Count} ignored for vendor=codex (no allowlist concept) — agent {AgentId}")]
     partial void LogToolsIgnoredForCodex(string agentId, int count);
+
+    [LoggerMessage(Level = LogLevel.Warning, Message = "MCP allowlist entry '{Name}' is not a kcap-owned server — skipping (agent {AgentId})")]
+    partial void LogAllowlistEntryUnknown(string name, string agentId);
+
+    [LoggerMessage(Level = LogLevel.Warning, Message = "MCP allowlist entry '{Name}' can start flows — stripped (agent {AgentId})")]
+    partial void LogAllowlistEntryStripped(string name, string agentId);
 }

src/Capacitor.Cli.Daemon/Services/IHostedAgentLauncher.cs

@@ -81,6 +81,13 @@ internal sealed record LauncherContext(
     /// <summary>Owned worktree (daemon-created) vs borrowed cwd (the user's own checkout).
     /// Borrowed-cwd launches skip repo-mutating <c>Prepare()</c> steps.</summary>
     public WorkLocation Work { get; init; } = WorkLocation.OwnedWorktree;
+
+    /// <summary>AI-1126 D-c: the review-flow definition's MCP allowlist, carried verbatim from
+    /// <see cref="Capacitor.Cli.Core.LaunchAgentCommand.McpAllowlist"/>. Launchers resolve each
+    /// name against the kcap-owned MCP registry and materialize matching servers into the vendor's
+    /// MCP config, stripping any flow-starting server regardless of listing. Null/local-spawn
+    /// launches (e.g. <c>kcap run-agent</c>) never set this.</summary>
+    public string[]? McpAllowlist { get; init; }
 }
 
 internal readonly record struct LaunchArgs(string[] Args, string? McpConfigPath);

test/Capacitor.Cli.Tests.Unit/ClaudeLauncherReviewFlowTests.cs

@@ -3,6 +3,7 @@
 using Capacitor.Cli.Daemon;
 using Capacitor.Cli.Daemon.Services;
 using Microsoft.Extensions.Logging.Abstractions;
+using TUnit.Assertions.Enums;
 
 namespace Capacitor.Cli.Tests.Unit;
 
@@ -96,4 +97,108 @@ public async Task Non_review_flow_launch_is_unchanged_no_bypass_or_strict_mcp()
     public async Task Claude_launcher_supports_unattended() {
         await Assert.That(NewLauncher().SupportsUnattended).IsTrue();
     }
+
+    // === AI-1126 D-c: definition MCP allowlist materialization ===
+
+    [Test]
+    public async Task ReviewFlow_with_allowlist_merges_servers_into_mcp_config() {
+        var ctx = NewCtx(isReviewFlow: true) with { McpAllowlist = ["kcap-sessions"] };
+        var args = NewLauncher(serverUrl: "https://t.example", capacitorPath: "/opt/kcap").BuildArgs(ctx).Args;
+
+        await Assert.That(args).Contains("--strict-mcp-config");
+        await Assert.That(args).Contains("--disallowedTools");
+        var dIdx = Array.IndexOf(args, "--disallowedTools");
+        await Assert.That(args[dIdx + 1]).IsEqualTo("Agent");
+
+        var cfgIndex = Array.IndexOf(args, "--mcp-config");
+        var servers  = JsonNode.Parse(args[cfgIndex + 1])!.AsObject()["mcpServers"]!.AsObject();
+
+        await Assert.That(servers.Count).IsEqualTo(2);
+
+        var flowResult = servers["kcap-flow-result"]!.AsObject();
+        await Assert.That(flowResult["env"]!["KCAP_FLOW_AGENT_ID"]!.GetValue<string>()).IsEqualTo("a-1");
+
+        var sessions = servers["kcap-sessions"]!.AsObject();
+        await Assert.That(sessions["command"]!.GetValue<string>()).IsEqualTo("/opt/kcap");
+        await Assert.That(sessions["args"]![0]!.GetValue<string>()).IsEqualTo("mcp");
+        await Assert.That(sessions["args"]![1]!.GetValue<string>()).IsEqualTo("sessions");
+        await Assert.That(sessions["env"]!["KCAP_URL"]!.GetValue<string>()).IsEqualTo("https://t.example");
+        await Assert.That(sessions["env"]!.AsObject().ContainsKey("KCAP_FLOW_AGENT_ID")).IsFalse();
+    }
+
+    [Test]
+    public async Task ReviewFlow_allowlist_strips_flow_starting_server_any_case() {
+        var ctx = NewCtx(isReviewFlow: true) with { McpAllowlist = ["KCAP-Flows", "kcap-sessions"] };
+        var args = NewLauncher(serverUrl: "https://t.example", capacitorPath: "/opt/kcap").BuildArgs(ctx).Args;
+
+        var cfgIndex = Array.IndexOf(args, "--mcp-config");
+        var servers  = JsonNode.Parse(args[cfgIndex + 1])!.AsObject()["mcpServers"]!.AsObject();
+
+        await Assert.That(servers.Count).IsEqualTo(2);
+        await Assert.That(servers.ContainsKey("kcap-flow-result")).IsTrue();
+        await Assert.That(servers.ContainsKey("kcap-sessions")).IsTrue();
+        await Assert.That(servers.ContainsKey("kcap-flows")).IsFalse();
+    }
+
+    [Test]
+    public async Task ReviewFlow_allowlist_skips_unknown_names() {
+        var ctxWithAllowlist = NewCtx(isReviewFlow: true) with { McpAllowlist = ["not-a-server"] };
+        var ctxWithout       = NewCtx(isReviewFlow: true);
+        var launcher         = NewLauncher(serverUrl: "https://t.example", capacitorPath: "/opt/kcap");
+
+        var argsWithAllowlist = launcher.BuildArgs(ctxWithAllowlist).Args;
+        var argsWithout       = launcher.BuildArgs(ctxWithout).Args;
+
+        var cfgIndexWith    = Array.IndexOf(argsWithAllowlist, "--mcp-config");
+        var cfgIndexWithout = Array.IndexOf(argsWithout, "--mcp-config");
+
+        await Assert.That(argsWithAllowlist[cfgIndexWith + 1]).IsEqualTo(argsWithout[cfgIndexWithout + 1]);
+    }
+
+    [Test]
+    public async Task ReviewFlow_without_allowlist_args_byte_identical_to_today() {
+        var args = NewLauncher(serverUrl: "https://t.example", capacitorPath: "/opt/kcap")
+            .BuildArgs(NewCtx(isReviewFlow: true)).Args;
+
+        const string expectedMcpConfig =
+            """{"mcpServers":{"kcap-flow-result":{"command":"/opt/kcap","args":["mcp","flow-result"],"env":{"KCAP_URL":"https://t.example","KCAP_FLOW_AGENT_ID":"a-1"}}}}""";
+
+        string[] expected = [
+            "--permission-mode", "bypassPermissions",
+            "--strict-mcp-config",
+            "--mcp-config", expectedMcpConfig,
+            "--disallowedTools", "Agent",
+            "--model", "sonnet",
+            "--", "review this"
+        ];
+
+        await Assert.That(args).IsEquivalentTo(expected, CollectionOrdering.Matching);
+    }
+
+    [Test]
+    public async Task ReviewFlow_allowlist_with_null_element_skips_it_without_throwing() {
+        // A wire-deserialized allowlist can contain a null element (e.g. a malformed JSON
+        // array entry). It must flow through the existing unknown-name skip path rather
+        // than throw a NullReferenceException out of the launch path.
+        var ctx = NewCtx(isReviewFlow: true) with { McpAllowlist = [null!, "kcap-sessions"] };
+
+        var args = NewLauncher(serverUrl: "https://t.example", capacitorPath: "/opt/kcap").BuildArgs(ctx).Args;
+
+        var cfgIndex = Array.IndexOf(args, "--mcp-config");
+        var servers  = JsonNode.Parse(args[cfgIndex + 1])!.AsObject()["mcpServers"]!.AsObject();
+
+        await Assert.That(servers.Count).IsEqualTo(2);
+        await Assert.That(servers.ContainsKey("kcap-flow-result")).IsTrue();
+        await Assert.That(servers.ContainsKey("kcap-sessions")).IsTrue();
+    }
+
+    [Test]
+    public async Task ReviewFlow_empty_allowlist_args_byte_identical_to_null_allowlist() {
+        var launcher = NewLauncher(serverUrl: "https://t.example", capacitorPath: "/opt/kcap");
+
+        var argsNull  = launcher.BuildArgs(NewCtx(isReviewFlow: true) with { McpAllowlist = null }).Args;
+        var argsEmpty = launcher.BuildArgs(NewCtx(isReviewFlow: true) with { McpAllowlist = [] }).Args;
+
+        await Assert.That(argsEmpty).IsEquivalentTo(argsNull, CollectionOrdering.Matching);
+    }
 }