diff --git a/src/Netclaw.Cli.Tests/Mcp/McpCommandTests.cs b/src/Netclaw.Cli.Tests/Mcp/McpCommandTests.cs index e65dc2086..5869db920 100644 --- a/src/Netclaw.Cli.Tests/Mcp/McpCommandTests.cs +++ b/src/Netclaw.Cli.Tests/Mcp/McpCommandTests.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // // Copyright (C) 2026 - 2026 Petabridge, LLC // @@ -291,6 +291,92 @@ public async Task Add_CreatesApprovalPolicySectionWhenMissing() Assert.Equal("All", personal.GetProperty("McpServersMode").GetString()); } + // ── Add-time unconditional OAuth hint ── + // + // The daemon owns OAuth discovery (RFC 9728/8414, via McpOAuthClientRegistrar). + // The CLI does not probe the endpoint; it prints an unconditional hint for any + // HTTP/SSE server added without an explicit Authorization header. + + [Theory] + [InlineData("stdio")] + [InlineData("http-with-header")] + public async Task Add_DoesNotPrintOAuthHint_ForStdioOrExplicitAuthorizationHeader(string scenario) + { + var args = scenario is "stdio" + ? new[] { "mcp", "add", "--transport", "stdio", "local", "--", "npx", "-y", "@local/mcp" } + : new[] { "mcp", "add", "--transport", "http", "--header", "Authorization: Bearer test-token", "myapi", "https://api.example.com/mcp" }; + + var exitCode = await McpCommand.RunAsync(args, _paths, output: _output); + + Assert.Equal(0, exitCode); + + var output = _output.ToString(); + Assert.DoesNotContain("Next steps:", output); + Assert.DoesNotContain("netclaw mcp auth", output); + Assert.Contains("Next: run `netclaw mcp permissions`", output); + } + + [Fact] + public async Task Add_HttpServerWithoutAuthorizationHeader_PrintsUnconditionalAuthHint() + { + var args = new[] { "mcp", "add", "--transport", "http", "plain", "https://plain.example/mcp" }; + var exitCode = await McpCommand.RunAsync(args, _paths, output: _output); + + Assert.Equal(0, exitCode); + + var output = _output.ToString(); + Assert.Contains("Next steps:", output); + Assert.Contains("If this server requires OAuth, authorize first: netclaw mcp auth plain", output); + Assert.Contains("Then grant tools: netclaw mcp permissions", output); + } + + [Fact] + public async Task Add_WithAuthFlag_NoDaemon_PrintsFallbackHint() + { + var args = new[] { "mcp", "add", "--auth", "--transport", "http", "notion", "https://mcp.notion.com/mcp" }; + var exitCode = await McpCommand.RunAsync(args, _paths, output: _output); + + Assert.Equal(0, exitCode); + + var output = _output.ToString(); + Assert.Contains("Next steps:", output); + Assert.Contains("authorize first: netclaw mcp auth notion", output); + Assert.Contains("--auth: daemon API not available. Run `netclaw mcp auth notion` once the daemon is running.", output); + } + + [Fact] + public async Task Add_WithAuthFlag_DaemonRejects_PropagatesAuthErrorForAddedServer() + { + var args = new[] { "mcp", "add", "--auth", "--transport", "http", "notion", "https://mcp.notion.com/mcp" }; + var daemonApi = CreateDaemonApi(request => request.RequestUri!.AbsolutePath switch + { + "/api/mcp/oauth/start/notion" => new HttpResponseMessage(HttpStatusCode.Forbidden), + _ => new HttpResponseMessage(HttpStatusCode.NotFound), + }); + + var exitCode = await McpCommand.RunAsync( + args, _paths, daemonApi, output: _output); + + // The auth flow must target the added server ('notion'), not the '--auth' + // flag position — a wrong name would print "MCP server '--auth' not found." + Assert.Equal(1, exitCode); + Assert.Contains("HTTP 403 Forbidden", _output.ToString()); + Assert.Contains("notion", _output.ToString()); + } + + [Fact] + public async Task Add_WithAuthFlag_OnStdio_Ignored() + { + var args = new[] { "mcp", "add", "--auth", "--transport", "stdio", "local", "--", "npx", "-y", "@local/mcp" }; + var exitCode = await McpCommand.RunAsync(args, _paths, output: _output); + + Assert.Equal(0, exitCode); + + var output = _output.ToString(); + Assert.Contains("--auth ignored: OAuth is only for HTTP/SSE servers.", output); + Assert.Contains("netclaw mcp permissions", output); + } + [Fact] public async Task List_NoServers_ShowsEmptyMessage() { diff --git a/src/Netclaw.Cli/Mcp/McpCommand.cs b/src/Netclaw.Cli/Mcp/McpCommand.cs index 8319c0f05..8e1d43ae5 100644 --- a/src/Netclaw.Cli/Mcp/McpCommand.cs +++ b/src/Netclaw.Cli/Mcp/McpCommand.cs @@ -1,9 +1,10 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- using System.Diagnostics; +using System.Net.Http; using System.Net.Http.Json; using System.Net.Sockets; using System.Text; @@ -46,7 +47,7 @@ public static async Task RunAsync(string[] args, NetclawPaths paths, Daemon return subcommand switch { - "add" => RunAdd(args, paths, writer), + "add" => await RunAddAsync(args, paths, writer, daemonApi), "auth" => await RunAuthAsync(args, paths, daemonApi, writer), "list" => await RunListAsync(paths, daemonApi, writer), "get" => RunGet(args, paths, writer), @@ -60,15 +61,20 @@ public static async Task RunAsync(string[] args, NetclawPaths paths, Daemon }; } - internal static int RunAdd(string[] args, NetclawPaths paths, TextWriter writer) + internal static async Task RunAddAsync( + string[] args, + NetclawPaths paths, + TextWriter writer, + DaemonApi? daemonApi = null) { - // Parse: netclaw mcp add [--transport ] [--client-id ] [--scope ] [--env KEY=VALUE]... [--header "Key: Value"]... [--grant-all] [command/url] [-- args...] + // Parse: netclaw mcp add [--transport ] [--client-id ] [--scope ] [--env KEY=VALUE]... [--header "Key: Value"]... [--grant-all] [--auth] [command/url] [-- args...] string? transport = null; string? oauthClientId = null; string? oauthScope = null; var envVars = new Dictionary(); var headers = new Dictionary(); var grantAll = false; + var runAuth = false; string? commandOrUrl = null; string[]? commandArgs = null; @@ -96,6 +102,12 @@ internal static int RunAdd(string[] args, NetclawPaths paths, TextWriter writer) continue; } + if (args[i] == "--auth") + { + runAuth = true; + continue; + } + if (args[i] is "--transport" or "-t" && i + 1 < args.Length) { transport = args[++i]; @@ -239,7 +251,49 @@ internal static int RunAdd(string[] args, NetclawPaths paths, TextWriter writer) writer.WriteLine(" until you opt in via `netclaw mcp permissions`."); } writer.WriteLine("Approval defaults: Personal=Auto, Team=Approval, Public=Deny"); - writer.WriteLine($"Next: run `netclaw mcp permissions` to grant tools and adjust approvals for '{serverName.Value}'."); + + // The daemon owns OAuth discovery (RFC 9728/8414, via McpOAuthClientRegistrar). + // The CLI does not probe the endpoint, so it cannot know in advance whether a + // given HTTP/SSE server requires OAuth. Print the hint unconditionally for any + // HTTP/SSE server that has no explicit Authorization header: stdio servers run + // local commands and never use OAuth, and a server with a static Authorization + // header is already using its own credentials. + var hasAuthorizationHeader = headers.Keys.Any( + key => string.Equals(key, "Authorization", StringComparison.OrdinalIgnoreCase)); + var showOAuthHint = transport is not "stdio" && !hasAuthorizationHeader; + + if (showOAuthHint) + { + writer.WriteLine(); + writer.WriteLine("Next steps:"); + writer.WriteLine($" - If this server requires OAuth, authorize first: netclaw mcp auth {serverName.Value}"); + writer.WriteLine(" - Then grant tools: netclaw mcp permissions"); + } + else + { + writer.WriteLine($"Next: run `netclaw mcp permissions` to grant tools and adjust approvals for '{serverName.Value}'."); + } + + if (runAuth && transport is not "stdio") + { + if (daemonApi is null) + { + writer.WriteLine(); + writer.WriteLine("--auth: daemon API not available. Run `netclaw mcp auth " + + $"{serverName.Value}` once the daemon is running."); + } + else + { + writer.WriteLine(); + return await RunAuthAsync(["mcp", "auth", serverName.Value], paths, daemonApi, writer); + } + } + else if (runAuth && transport is "stdio") + { + writer.WriteLine(); + writer.WriteLine("--auth ignored: OAuth is only for HTTP/SSE servers."); + } + return 0; } @@ -1350,6 +1404,12 @@ private static int WriteHelp(TextWriter writer) writer.WriteLine(" --grant-all CI escape hatch. Skip the empty-grants writes and leave tool"); writer.WriteLine(" grants null (legacy \"all pass\" behavior). Approval defaults"); writer.WriteLine(" (Personal=Approval, Team=Approval, Public=Deny) are still written."); + writer.WriteLine(" --auth Start the OAuth flow immediately after adding (HTTP/SSE only)."); + writer.WriteLine(" --client-id Pre-registered OAuth client ID for servers that do not support"); + writer.WriteLine(" dynamic client registration."); + writer.WriteLine(); + writer.WriteLine("On add, HTTP/SSE servers without an Authorization header print a hint to run"); + writer.WriteLine("`netclaw mcp auth` first. The daemon detects OAuth requirements at auth time."); writer.WriteLine(); writer.WriteLine("Examples:"); writer.WriteLine(" netclaw mcp add --transport stdio memorizer -- npx -y @memorizer/mcp-server");