From 07032df0378fef239cfd038aede4b955234c9d40 Mon Sep 17 00:00:00 2001 From: afjcjsbx Date: Fri, 24 Apr 2026 10:32:55 +0200 Subject: [PATCH] fix(mcp): normalize local command paths and document env-file usage --- README.md | 3 +- cmd/picoclaw/internal/mcp/add.go | 16 ++++++++- cmd/picoclaw/internal/mcp/command_test.go | 40 +++++++++++++++++++++++ docs/project/README.it.md | 3 +- docs/reference/mcp-cli.md | 26 +++++++++++---- 5 files changed, 78 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 9668fcf34..73cc877fa 100644 --- a/README.md +++ b/README.md @@ -581,7 +581,8 @@ picoclaw mcp test filesystem `picoclaw mcp` is a configuration manager: it updates `config.json` under `tools.mcp.servers`, but it does not keep the server process running itself. -Use `picoclaw mcp edit` when you need advanced fields such as `headers`, `env_file`, or `deferred`. +Use `picoclaw mcp edit` when you need advanced fields that are not covered by `picoclaw mcp add`. +For example, `picoclaw mcp add` supports `--deferred` and `--env-file`, while `picoclaw mcp edit` is still useful for direct JSON editing and uncommon MCP settings. For full MCP configuration (stdio, SSE, HTTP transports, Tool Discovery), see [Tools Configuration - MCP](docs/reference/tools_configuration.md#mcp-tool). For CLI usage and examples, see [MCP Server CLI](docs/reference/mcp-cli.md). diff --git a/cmd/picoclaw/internal/mcp/add.go b/cmd/picoclaw/internal/mcp/add.go index 87917095e..8ad68571f 100644 --- a/cmd/picoclaw/internal/mcp/add.go +++ b/cmd/picoclaw/internal/mcp/add.go @@ -12,6 +12,7 @@ import ( type addOptions struct { Env []string + EnvFile string Headers []string Transport string Force bool @@ -70,7 +71,8 @@ func newAddCommand() *cobra.Command { } flags := cmd.Flags() - flags.StringArrayP("env", "e", nil, "Environment variable in KEY=value format (repeatable)") + flags.StringArrayP("env", "e", nil, "Environment variable in KEY=value format (repeatable, saved to config)") + flags.String("env-file", "", "Path to an env file for stdio servers (recommended for secrets)") flags.StringArrayP("header", "H", nil, "HTTP header in 'Name: Value' or 'Name=Value' format (repeatable)") flags.StringP("transport", "t", "stdio", "Transport type: stdio, http, or sse") flags.BoolP("force", "f", false, "Overwrite an existing server without prompting") @@ -119,8 +121,16 @@ func parseAddArgs(args []string) (addOptions, string, string, []string, bool, er } i++ opts.Env = append(opts.Env, args[i]) + case arg == "--env-file": + if i+1 >= len(args) { + return addOptions{}, "", "", nil, false, fmt.Errorf("missing value for %s", arg) + } + i++ + opts.EnvFile = args[i] case strings.HasPrefix(arg, "--env="): opts.Env = append(opts.Env, strings.TrimPrefix(arg, "--env=")) + case strings.HasPrefix(arg, "--env-file="): + opts.EnvFile = strings.TrimPrefix(arg, "--env-file=") case arg == "--header" || arg == "-H": if i+1 >= len(args) { return addOptions{}, "", "", nil, false, fmt.Errorf("missing value for %s", arg) @@ -193,6 +203,9 @@ func buildServerConfig(target string, args []string, opts addOptions) (config.MC if len(env) > 0 { return config.MCPServerConfig{}, fmt.Errorf("--env can only be used with stdio transport") } + if strings.TrimSpace(opts.EnvFile) != "" { + return config.MCPServerConfig{}, fmt.Errorf("--env-file can only be used with stdio transport") + } if len(args) > 0 { return config.MCPServerConfig{}, fmt.Errorf("%s transport does not accept command arguments", transport) } @@ -230,6 +243,7 @@ func buildServerConfig(target string, args []string, opts addOptions) (config.MC server.Command = command server.Args = commandArgs server.Env = env + server.EnvFile = strings.TrimSpace(opts.EnvFile) return server, nil } diff --git a/cmd/picoclaw/internal/mcp/command_test.go b/cmd/picoclaw/internal/mcp/command_test.go index 9f8d822e4..be1c9763e 100644 --- a/cmd/picoclaw/internal/mcp/command_test.go +++ b/cmd/picoclaw/internal/mcp/command_test.go @@ -139,6 +139,46 @@ func TestMCPAddSupportsExplicitStdioCommandAfterSeparator(t *testing.T) { assert.Equal(t, map[string]string{"AIRTABLE_API_KEY": "YOUR_KEY"}, server.Env) } +func TestMCPAddSupportsEnvFileForStdio(t *testing.T) { + configPath := setupMCPConfigEnv(t) + + cmd := NewMCPCommand() + _, err := executeCommand(cmd, []string{ + "add", + "--env-file", + ".env.mcp", + "filesystem", + "npx", + "-y", + "@modelcontextprotocol/server-filesystem", + }, "") + require.NoError(t, err) + + cfg := readMCPConfig(t, configPath) + server := cfg.Tools.MCP.Servers["filesystem"] + assert.Equal(t, "stdio", server.Type) + assert.Equal(t, "npx", server.Command) + assert.Equal(t, []string{"-y", "@modelcontextprotocol/server-filesystem"}, server.Args) + assert.Equal(t, ".env.mcp", server.EnvFile) +} + +func TestMCPAddRejectsEnvFileForHTTP(t *testing.T) { + setupMCPConfigEnv(t) + + cmd := NewMCPCommand() + _, err := executeCommand(cmd, []string{ + "add", + "--transport", + "http", + "--env-file", + ".env.mcp", + "context7", + "https://mcp.context7.com/mcp", + }, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "--env-file can only be used with stdio transport") +} + func TestMCPAddRejectsNonExecutableLocalCommand(t *testing.T) { setupMCPConfigEnv(t) diff --git a/docs/project/README.it.md b/docs/project/README.it.md index cd7ecac7d..b3db6fece 100644 --- a/docs/project/README.it.md +++ b/docs/project/README.it.md @@ -564,7 +564,8 @@ picoclaw mcp test filesystem `picoclaw mcp` agisce come configuration manager: aggiorna `config.json` sotto `tools.mcp.servers`, ma non mantiene in esecuzione il processo del server. -Usa `picoclaw mcp edit` quando ti servono campi avanzati come `headers`, `env_file` o `deferred`. +Usa `picoclaw mcp edit` quando ti servono campi avanzati che non sono coperti da `picoclaw mcp add`. +Per esempio, `picoclaw mcp add` supporta `--deferred` e `--env-file`, mentre `picoclaw mcp edit` resta utile per modifiche JSON dirette e opzioni MCP meno comuni. Per la configurazione MCP completa (trasporti stdio, SSE, HTTP, Tool Discovery), vedi [Configurazione degli Strumenti - MCP](../reference/tools_configuration.md#mcp-tool). Per la reference della CLI, vedi [MCP Server CLI](../reference/mcp-cli.md). diff --git a/docs/reference/mcp-cli.md b/docs/reference/mcp-cli.md index 77291b140..18b2b4c1c 100644 --- a/docs/reference/mcp-cli.md +++ b/docs/reference/mcp-cli.md @@ -36,12 +36,18 @@ Add a stdio server via `npx`: picoclaw mcp add filesystem -- npx -y @modelcontextprotocol/server-filesystem /tmp ``` -Add a stdio server with environment variables: +Add a stdio server with environment variables saved in config: ```bash picoclaw mcp add github --env GITHUB_PERSONAL_ACCESS_TOKEN=ghp_xxx -- npx -y @modelcontextprotocol/server-github ``` +Add a stdio server using an env file for secrets: + +```bash +picoclaw mcp add github --env-file .env.github -- npx -y @modelcontextprotocol/server-github +``` + Add a remote HTTP server: ```bash @@ -108,7 +114,8 @@ Supported flags: | Flag | Meaning | |------|---------| -| `--env`, `-e` | Add a stdio environment variable in `KEY=value` format. Repeatable. | +| `--env`, `-e` | Add a stdio environment variable in `KEY=value` format. Repeatable. Values are saved to config. | +| `--env-file` | Attach an env file path to a stdio server. Recommended for secrets you do not want stored inline in `config.json`. | | `--header`, `-H` | Add an HTTP header in `Name: Value` or `Name=Value` format. Repeatable. | | `--transport`, `-t` | Transport type: `stdio` (default), `http`, or `sse`. | | `--force`, `-f` | Overwrite an existing server entry without confirmation. | @@ -131,6 +138,11 @@ Parsing behavior: - use the `--` separator when the stdio command itself has arguments that may look like PicoClaw CLI flags - without `--`, PicoClaw treats the first two non-flag tokens as `` and `` +Secret handling: + +- `--env KEY=value` stores the resolved value directly in `config.json` +- use `--env-file` instead when the value is sensitive and should stay outside the main config file + Example: ```bash @@ -182,6 +194,7 @@ For `stdio`: - `` is treated as the command - `[args...]` are stored in `args` - `--env` is supported +- `--env-file` is supported and stored in `env_file` - `--header` is rejected - `-- [args...]` is supported and recommended for unambiguous parsing @@ -190,6 +203,7 @@ For `http` / `sse`: - `` must be a valid URL - extra command args are rejected - `--env` is rejected +- `--env-file` is rejected - `--header` is supported and stored in `headers` Overwrite behavior: @@ -322,9 +336,7 @@ picoclaw mcp edit This opens the config file in the editor pointed to by `$EDITOR`. -Use it when you need to configure MCP fields that are not exposed directly by `picoclaw mcp add`, such as: - -- `env_file` +Use it when you need to configure MCP fields that are not exposed directly by `picoclaw mcp add`. If `$EDITOR` is not set, the command fails with an explicit error. @@ -337,10 +349,10 @@ For common cases: 3. Check all servers at a glance with `picoclaw mcp list --status`. 4. Start PicoClaw normally so the configured MCP server is loaded by the host. -For advanced cases (e.g. `env_file`): +For advanced cases: 1. Add the base entry with `picoclaw mcp add`. -2. Run `picoclaw mcp edit` to fill in `env_file` or other fields not exposed as CLI flags. +2. Run `picoclaw mcp edit` to fill in fields that are not exposed as CLI flags. 3. Run `picoclaw mcp show ` to confirm the final configuration and tool list. ## Related Docs