diff --git a/CLAUDE.md b/CLAUDE.md
index cd1c5f5..8787ede 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -88,7 +88,7 @@ sluice policy remove <id>
 sluice policy import <path.toml>    # seed DB from TOML (merge semantics)
 sluice policy export                # dump current rules as TOML
 
-sluice mcp add <name> --command <cmd> [--transport stdio|http|websocket] [--args "a,b"] [--env "K=V"] [--timeout 120]
+sluice mcp add <name> --command <cmd> [--transport stdio|http|websocket] [--args "a,b"] [--env "K=V"] [--header "K=V"] [--timeout 120]
 sluice mcp list
 sluice mcp remove <name>
 sluice mcp                          # start MCP gateway
@@ -128,7 +128,9 @@ docker exec openclaw openclaw mcp set sluice '{"url":"http://sluice:3000/mcp"}'
 
 For the hostname `sluice` to resolve inside OpenClaw, the compose file pins sluice's IP on the internal network (172.30.0.2) and adds an `extra_hosts` entry on tun2proxy (which OpenClaw shares). Docker's embedded DNS (127.0.0.11) is not reachable from OpenClaw because its DNS is routed through the TUN device. The `/etc/hosts` entry bypasses DNS entirely.
 
-When new MCP upstreams are added to sluice via `sluice mcp add`, restart sluice so the gateway picks them up. OpenClaw does not need to be restarted - its connection to sluice:3000/mcp remains valid and it re-queries the tool list on subsequent agent runs.
+MCP upstreams can be managed via `sluice mcp add|list|remove`, the REST API (`/api/mcp/upstreams`), or the Telegram bot (`/mcp add|list|remove`). All three paths write to the same SQLite store. After any addition or removal, restart sluice so the gateway re-reads the upstream set. OpenClaw does not need to be restarted: its connection to `sluice:3000/mcp` is registered once at sluice startup (via `WireMCPGateway`, which patches `mcp.servers.sluice = {url: ...}` in the agent's openclaw.json) and stays valid across sluice restarts. The agent re-queries the tool list on subsequent agent runs.
+
+The Telegram `/mcp add` path auto-deletes the chat message because `--env KEY=VAL` pairs may contain secrets (use `KEY=vault:name` to keep the plaintext out of the SQLite store and `/mcp list` output entirely).
 
 ## Policy Store
 
@@ -260,7 +262,7 @@ Action names operators commonly grep for: `tool_call` (MCP tool call policy verd
 
 ### MCP gateway
 
-Three upstream transports: stdio (child processes), Streamable HTTP, WebSocket. All satisfy `MCPUpstream` interface. Tools namespaced as `<upstream>__<tool>`. Policy evaluation: deny/allow/ask priority. `ContentInspector` blocks arguments and redacts responses using regex (JSON parsed before matching to prevent unicode escape bypass). Per-upstream timeouts (default 120s).
+Three upstream transports: stdio (child processes), Streamable HTTP, WebSocket. All satisfy `MCPUpstream` interface. Tools namespaced as `<upstream>__<tool>`. Policy evaluation: deny/allow/ask priority. `ContentInspector` blocks arguments and redacts responses using regex (JSON parsed before matching to prevent unicode escape bypass). Per-upstream timeout defaults are defined by the `mcp.DefaultTimeoutSec` constant (120s) shared across packages that need the fallback. `internal/store.AddMCPUpstream` duplicates the literal 120 because `internal/store` is imported by `internal/mcp` and cannot import it back (circular). A comment in `store.go` flags the duplicate so the two stay in sync.
 
 `MCPHTTPHandler` serves `POST /mcp` and `DELETE /mcp` on port 3000 (alongside `/healthz`). Session tracking via `Mcp-Session-Id` header. SSE response support.
 
diff --git a/README.md b/README.md
index db93907..65e4ff3 100644
--- a/README.md
+++ b/README.md
@@ -285,9 +285,14 @@ Manage sluice from your phone. Approve connections and tool calls, add credentia
 | `/policy deny <dest>` | Add deny rule |
 | `/cred add <name> [--env-var VAR]` | Add credential (value sent as next message, auto-deleted) |
 | `/cred rotate <name>` | Replace credential, hot-reload OpenClaw |
+| `/mcp list` | List registered MCP upstreams |
+| `/mcp add <name> --command <cmd> [flags]` | Register a new MCP upstream (stdio/http/websocket, see `/help`; chat message auto-deleted because `--env` may carry secrets) |
+| `/mcp remove <name>` | Remove an MCP upstream |
 | `/status` | Proxy stats and pending approvals |
 | `/audit recent [N]` | Last N audit entries |
 
+`/mcp add` only writes to the SQLite store. The MCP gateway builds its upstream set at startup, so restart sluice (`docker compose restart sluice`) for a new or removed upstream to take effect. The agent's connection to `sluice:3000/mcp` survives the restart.
+
 ### HTTP Webhooks
 
 REST API on port 3000 for programmatic approval integration. `GET /api/approvals` lists pending requests, `POST /api/approvals/{id}/resolve` resolves them. Use this to build custom approval UIs or integrate with existing workflows.
diff --git a/cmd/sluice/flagutil.go b/cmd/sluice/flagutil.go
index a1bb819..e930f5a 100644
--- a/cmd/sluice/flagutil.go
+++ b/cmd/sluice/flagutil.go
@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
+
+	"github.com/nemirovsky/sluice/internal/flagutil"
 )
 
 // parsePortsList parses a comma-separated string of port numbers into a
@@ -55,69 +57,9 @@ func parseProtocolsList(s string) ([]string, error) {
 	return protocols, nil
 }
 
-// reorderFlagsBeforePositional returns a copy of args with all flag
-// arguments moved before any positional arguments, so that Go's stdlib
-// flag parser (which stops at the first non-flag) still sees every flag.
-//
-// The FlagSet is consulted to determine which flags take a value and
-// therefore consume the following arg. "--flag=value" form is left as
-// a single token. "--" is treated as a terminator; everything after it
-// is positional, preserving the stdlib convention.
-//
-// Example: ["github", "--command", "https://x", "--timeout", "60"]
-// becomes  ["--command", "https://x", "--timeout", "60", "github"]
-//
-// Flags defined as bool do not consume the following arg. Everything
-// else (string, int, Func, Var) is assumed to.
+// reorderFlagsBeforePositional is a thin alias over flagutil.ReorderFlagsBeforePositional
+// so existing cmd/sluice callers (and their many tests) keep the old signature
+// without churn. New code should import internal/flagutil directly.
 func reorderFlagsBeforePositional(args []string, fs *flag.FlagSet) []string {
-	var flagArgs, positional []string
-	i := 0
-	for i < len(args) {
-		a := args[i]
-		if a == "--" {
-			// Terminator: everything after is positional, flag parsing
-			// should still see "--" to stop.
-			flagArgs = append(flagArgs, a)
-			positional = append(positional, args[i+1:]...)
-			break
-		}
-		if !strings.HasPrefix(a, "-") || a == "-" {
-			positional = append(positional, a)
-			i++
-			continue
-		}
-		flagArgs = append(flagArgs, a)
-		// --flag=value form: value is in the same arg.
-		if strings.Contains(a, "=") {
-			i++
-			continue
-		}
-		// Otherwise the next arg is the value for non-bool flags.
-		name := strings.TrimLeft(a, "-")
-		if isValueFlag(fs, name) && i+1 < len(args) {
-			flagArgs = append(flagArgs, args[i+1])
-			i += 2
-			continue
-		}
-		i++
-	}
-	return append(flagArgs, positional...)
-}
-
-// isValueFlag reports whether the named flag consumes the next argument
-// as its value. Bool flags do not; everything else does.
-func isValueFlag(fs *flag.FlagSet, name string) bool {
-	f := fs.Lookup(name)
-	if f == nil {
-		// Unknown flag. Assume it takes a value so we don't accidentally
-		// slurp something that might be a positional arg. fs.Parse will
-		// then surface the real error.
-		return true
-	}
-	// The stdlib flag package exposes bool flags via an IsBoolFlag method
-	// on the Value. Non-bool flags don't implement this.
-	if bf, ok := f.Value.(interface{ IsBoolFlag() bool }); ok && bf.IsBoolFlag() {
-		return false
-	}
-	return true
+	return flagutil.ReorderFlagsBeforePositional(args, fs)
 }
diff --git a/cmd/sluice/mcp.go b/cmd/sluice/mcp.go
index 4f485b9..a98be24 100644
--- a/cmd/sluice/mcp.go
+++ b/cmd/sluice/mcp.go
@@ -261,12 +261,12 @@ func handleMCPAdd(args []string) error {
 	command := fs.String("command", "", "command to run (stdio) or URL (http/websocket)")
 	argsStr := fs.String("args", "", "comma-separated arguments for the command")
 	envStr := fs.String("env", "", "comma-separated KEY=VAL environment variables (VAL may be vault:<name> for the whole value, or contain {vault:<name>} substrings for templated substitution)")
-	timeout := fs.Int("timeout", 120, "upstream timeout in seconds")
+	timeout := fs.Int("timeout", mcp.DefaultTimeoutSec, "upstream timeout in seconds")
 	transport := fs.String("transport", "stdio", "transport type: stdio, http, or websocket")
 	headers := make(map[string]string)
 	fs.Func("header", "HTTP header to send on every request to an http upstream (repeatable, format: KEY=VAL; VAL may be vault:<name> for the whole value, or contain {vault:<name>} substrings for templated substitution, e.g. \"Authorization=Bearer {vault:github_pat}\")", func(s string) error {
 		parts := strings.SplitN(s, "=", 2)
-		if len(parts) != 2 {
+		if len(parts) != 2 || parts[0] == "" {
 			return fmt.Errorf("invalid header format %q (expected KEY=VAL)", s)
 		}
 		headers[parts[0]] = parts[1]
@@ -291,6 +291,10 @@ func handleMCPAdd(args []string) error {
 		return fmt.Errorf("invalid transport %q: must be stdio, http, or websocket", *transport)
 	}
 
+	if *timeout <= 0 {
+		return fmt.Errorf("invalid --timeout %d: must be a positive integer (seconds)", *timeout)
+	}
+
 	if len(headers) > 0 && *transport != "http" {
 		return fmt.Errorf("--header is only valid for --transport http")
 	}
@@ -304,7 +308,7 @@ func handleMCPAdd(args []string) error {
 	if *envStr != "" {
 		for _, kv := range strings.Split(*envStr, ",") {
 			parts := strings.SplitN(kv, "=", 2)
-			if len(parts) != 2 {
+			if len(parts) != 2 || parts[0] == "" {
 				return fmt.Errorf("invalid env format %q (expected KEY=VAL)", kv)
 			}
 			env[parts[0]] = parts[1]
@@ -391,7 +395,7 @@ func handleMCPList(args []string) error {
 			headersStr = " headers=" + strings.Join(pairs, ",")
 		}
 		timeoutStr := ""
-		if u.TimeoutSec != 120 {
+		if u.TimeoutSec != mcp.DefaultTimeoutSec {
 			timeoutStr = fmt.Sprintf(" timeout=%ds", u.TimeoutSec)
 		}
 		fmt.Printf("[%d] %s command=%s%s%s%s%s%s\n", u.ID, u.Name, u.Command, transportStr, argsStr, envStr, headersStr, timeoutStr)
diff --git a/cmd/sluice/mcp_test.go b/cmd/sluice/mcp_test.go
index 2d3b164..d7f9ab4 100644
--- a/cmd/sluice/mcp_test.go
+++ b/cmd/sluice/mcp_test.go
@@ -6,6 +6,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/nemirovsky/sluice/internal/mcp"
@@ -374,6 +375,56 @@ func TestHandleMCPAddInvalidName(t *testing.T) {
 	}
 }
 
+// TestHandleMCPAddInvalidTimeout verifies that --timeout <= 0 is rejected
+// rather than being silently persisted. Previously the CLI accepted any
+// integer (including 0 and negatives), but the runtime constructors fall
+// back to DefaultTimeoutSec when TimeoutSec <= 0, so the persisted store
+// value diverged from the actual runtime behavior. Telegram /mcp add has
+// always rejected these values, this test locks in symmetry.
+func TestHandleMCPAddInvalidTimeout(t *testing.T) {
+	cases := []struct {
+		name    string
+		timeout string
+	}{
+		{"zero", "0"},
+		{"negative", "-5"},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			dir := t.TempDir()
+			dbPath := filepath.Join(dir, "test.db")
+
+			err := handleMCPAdd([]string{
+				"--db", dbPath,
+				"--command", "server",
+				"--timeout", tc.timeout,
+				"ups",
+			})
+			if err == nil {
+				t.Fatalf("expected error for --timeout %s", tc.timeout)
+			}
+			if !strings.Contains(err.Error(), "must be a positive integer") {
+				t.Errorf("error %q should mention 'must be a positive integer'", err)
+			}
+
+			// Nothing should be persisted when validation fails.
+			db, err := store.New(dbPath)
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer func() { _ = db.Close() }()
+			upstreams, err := db.ListMCPUpstreams()
+			if err != nil {
+				t.Fatal(err)
+			}
+			if len(upstreams) != 0 {
+				t.Errorf("expected 0 upstreams after invalid --timeout, got %d", len(upstreams))
+			}
+		})
+	}
+}
+
 // TestHandleMCPRemove verifies removing an upstream by name.
 func TestHandleMCPRemove(t *testing.T) {
 	dir := t.TempDir()
@@ -648,18 +699,41 @@ func TestHandleMCPGatewayInvalidChatID(t *testing.T) {
 }
 
 // TestHandleMCPAddEnvInvalidFormat verifies that bad env format is rejected.
+// Covers both missing "=" (BADFORMAT) and empty-key (=VALUE) cases. The CLI
+// rejects empty keys to match the Telegram handler's behavior so both entry
+// points reject the same malformed inputs.
 func TestHandleMCPAddEnvInvalidFormat(t *testing.T) {
 	dir := t.TempDir()
-	dbPath := filepath.Join(dir, "test.db")
 
-	err := handleMCPAdd([]string{
-		"--db", dbPath,
-		"--command", "server",
-		"--env", "BADFORMAT",
-		"myserver",
-	})
-	if err == nil {
-		t.Fatal("expected error for invalid env format")
+	cases := []struct {
+		label string
+		flag  string
+		value string
+	}{
+		{"env missing equals", "--env", "BADFORMAT"},
+		{"env empty key", "--env", "=VALUE"},
+		{"header missing equals", "--header", "BADFORMAT"},
+		{"header empty key", "--header", "=VALUE"},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.label, func(t *testing.T) {
+			dbPath := filepath.Join(dir, tc.label+".db")
+			args := []string{
+				"--db", dbPath,
+				"--command", "https://example.com/mcp",
+				"--transport", "http",
+				tc.flag, tc.value,
+				"myserver",
+			}
+			// The "http" transport is set so that --header is accepted for
+			// the header cases. The env cases do not require it, but sharing
+			// one arg set keeps the table loop uniform.
+			err := handleMCPAdd(args)
+			if err == nil {
+				t.Fatalf("expected error for %s %q", tc.flag, tc.value)
+			}
+		})
 	}
 }
 
diff --git a/docs/plans/20260408-mcp-upstream-commands.md b/docs/plans/20260408-mcp-upstream-commands.md
deleted file mode 100644
index ba74d8b..0000000
--- a/docs/plans/20260408-mcp-upstream-commands.md
+++ /dev/null
@@ -1,118 +0,0 @@
-# MCP Upstream CRUD Commands
-
-## Overview
-
-Add MCP upstream management to the Telegram bot and verify full coverage across all interfaces (CLI, HTTP API, Telegram). Currently:
-
-| Operation | CLI | HTTP API | Telegram |
-|-----------|-----|----------|----------|
-| List | `sluice mcp list` | `GET /api/mcp/upstreams` | missing |
-| Add | `sluice mcp add` | `POST /api/mcp/upstreams` | missing |
-| Remove | `sluice mcp remove` | `DELETE /api/mcp/upstreams/{name}` | missing |
-
-After this plan, Telegram will have `/mcp list`, `/mcp add`, and `/mcp remove`.
-
-## Context
-
-- Telegram commands: `internal/telegram/commands.go` (CommandHandler, Handle dispatch)
-- HTTP API: `internal/api/server.go` (MCP upstream handlers)
-- API spec: `internal/api/api.gen.go` (generated from OpenAPI)
-- CLI: `cmd/sluice/mcp.go` (mcp subcommand)
-- MCP store: `internal/store/store.go` (AddMCPUpstream, ListMCPUpstreams, RemoveMCPUpstream)
-- MCP gateway: `internal/mcp/gateway.go` (upstream lifecycle)
-- Auto-injection: `cmd/sluice/main.go` (MCP config write to shared volume)
-
-## Development Approach
-
-- **Testing approach**: Regular (code first, then tests)
-- Complete each task fully before moving to the next
-- CRITICAL: every task MUST include new/updated tests
-- CRITICAL: all tests must pass before starting next task
-
-## Testing Strategy
-
-- Unit tests for dispatch and subcommand parsing: `internal/telegram/commands_test.go`
-- Unit tests for nil-store guard behavior: `internal/telegram/commands_test.go`
-- Unit tests for MCP re-injection trigger: `internal/telegram/commands_test.go`
-- Existing store and API tests cover persistence layer
-
-## Progress Tracking
-
-- Mark completed items with `[x]` immediately when done
-- Add newly discovered tasks with + prefix
-
-## Implementation Steps
-
-### Task 1: Add /mcp dispatch and /mcp list
-
-**Files:**
-- Modify: `internal/telegram/commands.go`
-- Test: `internal/telegram/commands_test.go`
-
-- [ ] Add `case "mcp"` to Handle() dispatch
-- [ ] Add nil-store guard (check `h.store != nil` like handleCred does)
-- [ ] Implement `handleMCP(args)` with subcommand routing: list, add, remove
-- [ ] `/mcp list` - list registered upstreams with name, transport, command
-- [ ] Format list output for Telegram readability (not JSON)
-- [ ] Write tests for dispatch, nil-store guard, and list subcommand
-- [ ] Run tests - must pass before next task
-
-### Task 2: Add /mcp add with flag parsing
-
-**Files:**
-- Modify: `internal/telegram/commands.go`
-- Test: `internal/telegram/commands_test.go`
-
-- [ ] `/mcp add <name> --command <cmd>` - add stdio upstream (parse flags from args)
-- [ ] `/mcp add <name> --command <url> --transport http` - add HTTP/WebSocket upstream (use `--command` for URLs, matching CLI convention)
-- [ ] Support optional flags: `--transport`, `--args`, `--env`, `--timeout`
-- [ ] Note: `--env` flag may contain secrets. Delete the Telegram message after processing (same treatment as `/cred add`)
-- [ ] Write tests for add subcommand with various flag combinations
-- [ ] Run tests - must pass before next task
-
-### Task 3: Add /mcp remove with auto-injection
-
-**Files:**
-- Modify: `internal/telegram/commands.go`
-- Test: `internal/telegram/commands_test.go`
-
-- [ ] `/mcp remove <name>` - remove upstream by name
-- [ ] After add/remove: trigger MCP config re-injection into agent container. Add `mcpDir` field to `CommandHandler` (or accept it via constructor) so the handler can call the same MCP config write logic used in `cmd/sluice/main.go`
-- [ ] Write tests for remove subcommand and re-injection trigger
-- [ ] Run tests - must pass before next task
-
-### Task 4: Add /mcp to Telegram command menu
-
-**Files:**
-- Modify: `internal/telegram/approval.go`
-- Test: `internal/telegram/approval_test.go`
-
-- [ ] Add `{Command: "mcp", Description: "Manage MCP upstreams"}` to registerCommands
-- [ ] Update help output to include MCP section
-- [ ] Write tests for updated help and command registration
-- [ ] Run tests - must pass before next task
-
-### Task 5: Verify acceptance criteria
-
-- [ ] Verify `/mcp list` in Telegram shows upstreams
-- [ ] Verify `/mcp add` creates upstream and triggers auto-injection
-- [ ] Verify `/mcp remove` removes upstream
-- [ ] Run full test suite: `go test ./... -v -timeout 30s`
-
-### Task 6: [Final] Update documentation
-
-- [ ] Update CLAUDE.md CLI subcommands section if needed
-- [ ] Move this plan to `docs/plans/completed/`
-
-## Post-Completion
-
-**Manual verification:**
-- Deploy to knuth
-- Add an MCP upstream via Telegram: `/mcp add test-server --command "echo hello"`
-- Verify it appears in `/mcp list`
-- Verify `mcp-servers.json` is written to shared volume
-- Verify OpenClaw discovers the new MCP server
-- Remove it: `/mcp remove test-server`
-
-**Future work:**
-- Update/edit support for MCP upstreams (CLI, API, Telegram). Users can remove+add for now.
diff --git a/docs/plans/completed/20260408-mcp-upstream-commands.md b/docs/plans/completed/20260408-mcp-upstream-commands.md
new file mode 100644
index 0000000..06ef91d
--- /dev/null
+++ b/docs/plans/completed/20260408-mcp-upstream-commands.md
@@ -0,0 +1,118 @@
+# MCP Upstream CRUD Commands
+
+## Overview
+
+Add MCP upstream management to the Telegram bot and verify full coverage across all interfaces (CLI, HTTP API, Telegram). Currently:
+
+| Operation | CLI | HTTP API | Telegram |
+|-----------|-----|----------|----------|
+| List | `sluice mcp list` | `GET /api/mcp/upstreams` | missing |
+| Add | `sluice mcp add` | `POST /api/mcp/upstreams` | missing |
+| Remove | `sluice mcp remove` | `DELETE /api/mcp/upstreams/{name}` | missing |
+
+After this plan, Telegram will have `/mcp list`, `/mcp add`, and `/mcp remove`.
+
+## Context
+
+- Telegram commands: `internal/telegram/commands.go` (CommandHandler, Handle dispatch)
+- HTTP API: `internal/api/server.go` (MCP upstream handlers)
+- API spec: `internal/api/api.gen.go` (generated from OpenAPI)
+- CLI: `cmd/sluice/mcp.go` (mcp subcommand)
+- MCP store: `internal/store/store.go` (AddMCPUpstream, ListMCPUpstreams, RemoveMCPUpstream)
+- MCP gateway: `internal/mcp/gateway.go` (upstream lifecycle)
+- Auto-injection: `cmd/sluice/main.go` (MCP config write to shared volume)
+
+## Development Approach
+
+- **Testing approach**: Regular (code first, then tests)
+- Complete each task fully before moving to the next
+- CRITICAL: every task MUST include new/updated tests
+- CRITICAL: all tests must pass before starting next task
+
+## Testing Strategy
+
+- Unit tests for dispatch and subcommand parsing: `internal/telegram/commands_test.go`
+- Unit tests for nil-store guard behavior: `internal/telegram/commands_test.go`
+- Unit tests for MCP re-injection trigger: `internal/telegram/commands_test.go`
+- Existing store and API tests cover persistence layer
+
+## Progress Tracking
+
+- Mark completed items with `[x]` immediately when done
+- Add newly discovered tasks with + prefix
+
+## Implementation Steps
+
+### Task 1: Add /mcp dispatch and /mcp list
+
+**Files:**
+- Modify: `internal/telegram/commands.go`
+- Test: `internal/telegram/commands_test.go`
+
+- [x] Add `case "mcp"` to Handle() dispatch
+- [x] Add nil-store guard (check `h.store != nil` like handleCred does)
+- [x] Implement `handleMCP(args)` with subcommand routing: list, add, remove
+- [x] `/mcp list` - list registered upstreams with name, transport, command
+- [x] Format list output for Telegram readability (not JSON)
+- [x] Write tests for dispatch, nil-store guard, and list subcommand
+- [x] Run tests - must pass before next task
+
+### Task 2: Add /mcp add with flag parsing
+
+**Files:**
+- Modify: `internal/telegram/commands.go`
+- Test: `internal/telegram/commands_test.go`
+
+- [x] `/mcp add <name> --command <cmd>` - add stdio upstream (parse flags from args)
+- [x] `/mcp add <name> --command <url> --transport http` - add HTTP/WebSocket upstream (use `--command` for URLs, matching CLI convention)
+- [x] Support optional flags: `--transport`, `--args`, `--env`, `--timeout`
+- [x] Note: `--env` flag may contain secrets. Delete the Telegram message after processing (same treatment as `/cred add`)
+- [x] Write tests for add subcommand with various flag combinations
+- [x] Run tests - must pass before next task
+
+### Task 3: Add /mcp remove with auto-injection
+
+**Files:**
+- Modify: `internal/telegram/commands.go`
+- Test: `internal/telegram/commands_test.go`
+
+- [x] `/mcp remove <name>` - remove upstream by name
+- [x] Re-injection via `WireMCPGateway` was implemented and then removed during iter-1 code review. Sluice multiplexes every upstream behind a single agent-side entry (`mcp.servers.sluice = {url: http://sluice:3000/mcp}`) which is wired once at sluice startup and never changes on mutation. Re-calling `WireMCPGateway("sluice", url)` after /mcp add would not surface the new upstream to the agent (the gateway reads the upstream set from SQLite at startup) but would trigger an agent gateway restart. The agreed UX is: store mutation succeeds, response instructs the operator to restart sluice, no container-side RPC is issued on /mcp add or /mcp remove.
+- [x] Write tests for remove subcommand and regression guards that `WireMCPGateway` is NOT called on /mcp add or /mcp remove (see `TestHandleMCPAddDoesNotCallContainerManager` and `TestHandleMCPRemoveDoesNotCallContainerManager`).
+- [x] Run tests - must pass before next task
+
+### Task 4: Add /mcp to Telegram command menu
+
+**Files:**
+- Modify: `internal/telegram/approval.go`
+- Test: `internal/telegram/approval_test.go`
+
+- [x] Add `{Command: "mcp", Description: "Manage MCP upstreams"}` to registerCommands
+- [x] Update help output to include MCP section
+- [x] Write tests for updated help and command registration
+- [x] Run tests - must pass before next task
+
+### Task 5: Verify acceptance criteria
+
+- [x] Verify `/mcp list` in Telegram shows upstreams (verified via unit tests: `TestHandleMCPListEmpty`, `TestHandleMCPListWithUpstreams`, `TestHandleMCPListEscapesHTML`, `TestHandleMessageMCPListNotDeleted`)
+- [x] Verify `/mcp add` creates upstream (verified via unit tests: `TestHandleMCPAddStdio`, `TestHandleMCPAddWithArgsAndEnv`, `TestHandleMCPAddHTTPTransport`, `TestHandleMCPAddWebSocketTransport`, `TestHandleMCPAddDoesNotCallContainerManager`, `TestHandleMessageMCPAddDeletesMessage`)
+- [x] Verify `/mcp remove` removes upstream (verified via unit tests: `TestHandleMCPRemove`, `TestHandleMCPRemoveNotFound`, `TestHandleMCPRemoveStrayPositional`, `TestHandleMCPRemoveDoesNotCallContainerManager`)
+- [x] Run full test suite: `go test ./... -v -timeout 30s` -- all 2389 tests passing across 12 packages
+
+### Task 6: [Final] Update documentation
+
+- [x] Update CLAUDE.md: iter 2 added `--header "K=V"` to the documented `sluice mcp add` flag list (CLI Subcommands section) and expanded the MCP Gateway Setup section to cover all three management surfaces (CLI, REST, Telegram) plus the `mcp.servers.sluice={url}` wiring and the sluice-restart requirement for upstream mutations.
+- [x] Move this plan to `docs/plans/completed/`
+
+## Post-Completion
+
+**Manual verification:**
+- Deploy to a live sluice stack
+- Add an MCP upstream via Telegram: `/mcp add test-server --command "echo hello"`
+- Verify it appears in `/mcp list`
+- Restart sluice (the gateway builds its upstream set at startup)
+- Re-run an agent session and confirm the new tools are exposed under the `test-server__*` namespace
+- Remove it: `/mcp remove test-server`, restart sluice, confirm the tools are gone
+
+**Future work:**
+- Update/edit support for MCP upstreams (CLI, API, Telegram). Users can remove+add for now.
diff --git a/internal/flagutil/flagutil.go b/internal/flagutil/flagutil.go
new file mode 100644
index 0000000..9bfeb4e
--- /dev/null
+++ b/internal/flagutil/flagutil.go
@@ -0,0 +1,78 @@
+// Package flagutil provides shared helpers for Go's stdlib flag package. It
+// is used by both the CLI (cmd/sluice) and the Telegram command handler
+// (internal/telegram) so the two surfaces accept the same flag shapes and
+// behave identically on edge cases like the "--" terminator and single-dash
+// short flags.
+package flagutil
+
+import (
+	"flag"
+	"strings"
+)
+
+// ReorderFlagsBeforePositional returns a copy of args with all flag
+// arguments moved before any positional arguments, so that Go's stdlib
+// flag parser (which stops at the first non-flag) still sees every flag.
+//
+// The FlagSet is consulted to determine which flags take a value and
+// therefore consume the following arg. "--flag=value" form is left as
+// a single token. "--" is treated as a terminator: everything after it
+// is positional, preserving the stdlib convention.
+//
+// Example: ["github", "--command", "https://x", "--timeout", "60"]
+// becomes  ["--command", "https://x", "--timeout", "60", "github"]
+//
+// Flags defined as bool do not consume the following arg. Everything
+// else (string, int, Func, Var) is assumed to.
+func ReorderFlagsBeforePositional(args []string, fs *flag.FlagSet) []string {
+	var flagArgs, positional []string
+	i := 0
+	for i < len(args) {
+		a := args[i]
+		if a == "--" {
+			// Terminator: everything after is positional, flag parsing
+			// should still see "--" to stop.
+			flagArgs = append(flagArgs, a)
+			positional = append(positional, args[i+1:]...)
+			break
+		}
+		if !strings.HasPrefix(a, "-") || a == "-" {
+			positional = append(positional, a)
+			i++
+			continue
+		}
+		flagArgs = append(flagArgs, a)
+		// --flag=value form: value is in the same arg.
+		if strings.Contains(a, "=") {
+			i++
+			continue
+		}
+		// Otherwise the next arg is the value for non-bool flags.
+		name := strings.TrimLeft(a, "-")
+		if isValueFlag(fs, name) && i+1 < len(args) {
+			flagArgs = append(flagArgs, args[i+1])
+			i += 2
+			continue
+		}
+		i++
+	}
+	return append(flagArgs, positional...)
+}
+
+// isValueFlag reports whether the named flag consumes the next argument
+// as its value. Bool flags do not; everything else does.
+func isValueFlag(fs *flag.FlagSet, name string) bool {
+	f := fs.Lookup(name)
+	if f == nil {
+		// Unknown flag. Assume it takes a value so we don't accidentally
+		// slurp something that might be a positional arg. fs.Parse will
+		// then surface the real error.
+		return true
+	}
+	// The stdlib flag package exposes bool flags via an IsBoolFlag method
+	// on the Value. Non-bool flags don't implement this.
+	if bf, ok := f.Value.(interface{ IsBoolFlag() bool }); ok && bf.IsBoolFlag() {
+		return false
+	}
+	return true
+}
diff --git a/internal/flagutil/flagutil_test.go b/internal/flagutil/flagutil_test.go
new file mode 100644
index 0000000..cd951e0
--- /dev/null
+++ b/internal/flagutil/flagutil_test.go
@@ -0,0 +1,136 @@
+package flagutil
+
+import (
+	"flag"
+	"reflect"
+	"testing"
+)
+
+func newTestFlagSet() *flag.FlagSet {
+	fs := flag.NewFlagSet("test", flag.ContinueOnError)
+	fs.String("command", "", "")
+	fs.String("transport", "stdio", "")
+	fs.Int("timeout", 120, "")
+	fs.Bool("verbose", false, "")
+	fs.Func("header", "", func(string) error { return nil })
+	return fs
+}
+
+func TestReorderFlagsBeforePositional_NameFirst(t *testing.T) {
+	fs := newTestFlagSet()
+	in := []string{"github", "--command", "https://x", "--transport", "http"}
+	want := []string{"--command", "https://x", "--transport", "http", "github"}
+	got := ReorderFlagsBeforePositional(in, fs)
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+func TestReorderFlagsBeforePositional_NameLast(t *testing.T) {
+	fs := newTestFlagSet()
+	in := []string{"--command", "https://x", "github"}
+	want := []string{"--command", "https://x", "github"}
+	got := ReorderFlagsBeforePositional(in, fs)
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+func TestReorderFlagsBeforePositional_NameInMiddle(t *testing.T) {
+	fs := newTestFlagSet()
+	in := []string{"--command", "https://x", "github", "--transport", "http"}
+	want := []string{"--command", "https://x", "--transport", "http", "github"}
+	got := ReorderFlagsBeforePositional(in, fs)
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+func TestReorderFlagsBeforePositional_EqualsForm(t *testing.T) {
+	fs := newTestFlagSet()
+	in := []string{"github", "--command=https://x", "--timeout=60"}
+	want := []string{"--command=https://x", "--timeout=60", "github"}
+	got := ReorderFlagsBeforePositional(in, fs)
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+func TestReorderFlagsBeforePositional_BoolFlag(t *testing.T) {
+	fs := newTestFlagSet()
+	in := []string{"github", "--verbose", "--command", "https://x"}
+	want := []string{"--verbose", "--command", "https://x", "github"}
+	got := ReorderFlagsBeforePositional(in, fs)
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+func TestReorderFlagsBeforePositional_RepeatableFunc(t *testing.T) {
+	fs := newTestFlagSet()
+	in := []string{"github", "--header", "A=1", "--header", "B=2"}
+	want := []string{"--header", "A=1", "--header", "B=2", "github"}
+	got := ReorderFlagsBeforePositional(in, fs)
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+func TestReorderFlagsBeforePositional_Terminator(t *testing.T) {
+	fs := newTestFlagSet()
+	in := []string{"--command", "X", "--", "--not-a-flag", "github"}
+	want := []string{"--command", "X", "--", "--not-a-flag", "github"}
+	got := ReorderFlagsBeforePositional(in, fs)
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+func TestReorderFlagsBeforePositional_SingleDashFlag(t *testing.T) {
+	// Single-dash short flags (e.g. "-command") should be treated as flags,
+	// matching Go's stdlib flag parser which accepts both "-foo" and "--foo".
+	fs := newTestFlagSet()
+	in := []string{"github", "-command", "https://x"}
+	want := []string{"-command", "https://x", "github"}
+	got := ReorderFlagsBeforePositional(in, fs)
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+func TestReorderFlagsBeforePositional_BareDash(t *testing.T) {
+	// A bare "-" (often stdin) is positional, not a flag.
+	fs := newTestFlagSet()
+	in := []string{"-", "--command", "X"}
+	want := []string{"--command", "X", "-"}
+	got := ReorderFlagsBeforePositional(in, fs)
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+func TestReorderFlagsBeforePositional_Empty(t *testing.T) {
+	fs := newTestFlagSet()
+	got := ReorderFlagsBeforePositional(nil, fs)
+	if len(got) != 0 {
+		t.Errorf("expected empty, got %v", got)
+	}
+}
+
+// TestReorderFlagsBeforePositional_UnknownFlag exercises the isValueFlag
+// f == nil branch. When the reorderer encounters a flag that is not
+// registered on the FlagSet (e.g. a typo like --cmmand or an arg that
+// happens to look like a flag), it conservatively assumes the flag
+// consumes the next arg. This keeps the original token adjacency so that
+// fs.Parse can later produce a precise "flag provided but not defined"
+// error, rather than mis-splitting the stream and surfacing a cryptic
+// positional-arg error instead.
+func TestReorderFlagsBeforePositional_UnknownFlag(t *testing.T) {
+	fs := newTestFlagSet()
+	in := []string{"github", "--unknown", "val", "--command", "https://x"}
+	want := []string{"--unknown", "val", "--command", "https://x", "github"}
+	got := ReorderFlagsBeforePositional(in, fs)
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
diff --git a/internal/mcp/gateway.go b/internal/mcp/gateway.go
index fb70982..f1b4b52 100644
--- a/internal/mcp/gateway.go
+++ b/internal/mcp/gateway.go
@@ -64,7 +64,7 @@ func NewGateway(cfg GatewayConfig) (*Gateway, error) {
 		credResolver:  cfg.CredentialResolver,
 	}
 	if gw.timeoutSec == 0 {
-		gw.timeoutSec = 120
+		gw.timeoutSec = DefaultTimeoutSec
 	}
 	if gw.policy == nil {
 		// nil rules cannot fail compilation, so error is always nil here.
diff --git a/internal/mcp/upstream.go b/internal/mcp/upstream.go
index 142130c..108a43b 100644
--- a/internal/mcp/upstream.go
+++ b/internal/mcp/upstream.go
@@ -22,6 +22,11 @@ const (
 	TransportWS    = "websocket" // WebSocket client
 )
 
+// DefaultTimeoutSec is the default per-call timeout (in seconds) applied when
+// an upstream entry does not specify its own. Exported so CLI and Telegram
+// handlers can stay in sync with the gateway default.
+const DefaultTimeoutSec = 120
+
 // vaultPrefix marks env values that should be resolved from the vault.
 const vaultPrefix = "vault:"
 
@@ -165,7 +170,7 @@ type Upstream struct {
 	timeout  time.Duration
 }
 
-const defaultUpstreamTimeout = 120 * time.Second
+const defaultUpstreamTimeout = DefaultTimeoutSec * time.Second
 
 var validUpstreamName = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_-]*$`)
 
diff --git a/internal/store/store.go b/internal/store/store.go
index 35cff75..64aaedc 100644
--- a/internal/store/store.go
+++ b/internal/store/store.go
@@ -858,12 +858,17 @@ func (s *Store) AddMCPUpstream(name, command string, opts MCPUpstreamOpts) (int6
 	}
 	timeoutSec := opts.TimeoutSec
 	if timeoutSec == 0 {
+		// Keep in sync with internal/mcp.DefaultTimeoutSec. We cannot import
+		// internal/mcp here because internal/mcp already imports internal/store,
+		// which would introduce a dependency cycle.
 		timeoutSec = 120
 	}
 	transport := opts.Transport
 	if transport == "" {
 		transport = "stdio"
 	}
+	// Keep the transport list in sync with internal/mcp.ValidTransport and the
+	// Telegram /mcp add handler in internal/telegram/commands.go.
 	validTransports := map[string]bool{"stdio": true, "http": true, "websocket": true}
 	if !validTransports[transport] {
 		return 0, fmt.Errorf("invalid transport %q: must be stdio, http, or websocket", transport)
diff --git a/internal/telegram/approval.go b/internal/telegram/approval.go
index a4a799b..65500f7 100644
--- a/internal/telegram/approval.go
+++ b/internal/telegram/approval.go
@@ -220,6 +220,7 @@ func (tc *TelegramChannel) registerCommands() {
 		{Command: "status", Description: "Show proxy status"},
 		{Command: "policy", Description: "Manage policy rules"},
 		{Command: "cred", Description: "Manage credentials"},
+		{Command: "mcp", Description: "Manage MCP upstreams"},
 		{Command: "audit", Description: "Show audit log entries"},
 		{Command: "start", Description: "Show welcome message"},
 		{Command: "help", Description: "Show available commands"},
@@ -385,22 +386,21 @@ func (tc *TelegramChannel) handleMessage(msg *tgbotapi.Message) {
 		return
 	}
 
-	// Delete messages that contain credential values before processing
-	// to minimize exposure in chat history.
-	if cmd.Name == "cred" && len(cmd.Args) >= 1 &&
-		(cmd.Args[0] == "add" || cmd.Args[0] == "rotate") {
+	// Delete messages that may contain secrets before processing to minimize
+	// exposure in chat history. /cred add and /cred rotate always carry a
+	// credential value. /mcp add may carry secrets in --env KEY=VAL pairs.
+	isSensitive := containsSensitiveArgs(cmd)
+	if isSensitive {
 		del := tgbotapi.NewDeleteMessage(msg.Chat.ID, msg.MessageID)
 		if _, err := tc.api.Request(del); err != nil {
-			log.Printf("failed to delete credential message: %s", sanitizeError(err))
+			log.Printf("failed to delete sensitive command message: %s", sanitizeError(err))
 		}
 	}
 
 	// Forward as channel.Command (non-blocking, drop if full).
-	// Skip cred add/rotate commands to avoid forwarding plaintext
-	// credential values through the command channel.
-	isSensitiveCred := cmd.Name == "cred" && len(cmd.Args) >= 1 &&
-		(cmd.Args[0] == "add" || cmd.Args[0] == "rotate")
-	if !isSensitiveCred {
+	// Skip sensitive commands to avoid forwarding plaintext secrets
+	// through the command channel.
+	if !isSensitive {
 		select {
 		case tc.cmdCh <- channel.Command{
 			Name:        cmd.Name,
@@ -430,6 +430,14 @@ func (tc *TelegramChannel) handleMessage(msg *tgbotapi.Message) {
 			_, size := utf8.DecodeRuneInString(response[cut:])
 			cut += size
 		}
+		// Back up to the last newline before the cut. Every line our
+		// formatters emit is self-contained (open tags closed on the
+		// same line), so a newline boundary guarantees we don't split a
+		// <code> or <b> tag and confuse Telegram's HTML parser. Fall
+		// back to rune-level truncation when no newline exists.
+		if nl := strings.LastIndexByte(response[:cut], '\n'); nl >= 0 {
+			cut = nl
+		}
 		response = response[:cut] + "\n\n(truncated)"
 	}
 
@@ -447,3 +455,23 @@ func (tc *TelegramChannel) handleMessage(msg *tgbotapi.Message) {
 		log.Printf("telegram send error: %s", sanitizeError(err))
 	}
 }
+
+// containsSensitiveArgs reports whether a parsed command likely contains
+// secrets in its argument list (credential values, --env KEY=VAL pairs, etc).
+// Matching commands have their chat message deleted and skip forwarding to the
+// external command channel so the plaintext never leaves the bot goroutine.
+func containsSensitiveArgs(cmd *Command) bool {
+	if cmd == nil || len(cmd.Args) == 0 {
+		return false
+	}
+	switch cmd.Name {
+	case "cred":
+		// /cred add <name> <value>... and /cred rotate <name> <value>...
+		// both carry the secret in positional args.
+		return cmd.Args[0] == "add" || cmd.Args[0] == "rotate"
+	case "mcp":
+		// /mcp add may carry secrets via --env KEY=VAL.
+		return cmd.Args[0] == "add"
+	}
+	return false
+}
diff --git a/internal/telegram/approval_test.go b/internal/telegram/approval_test.go
index f67f113..50f4eca 100644
--- a/internal/telegram/approval_test.go
+++ b/internal/telegram/approval_test.go
@@ -7,6 +7,7 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"strconv"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -48,11 +49,12 @@ type tgResponse struct {
 type mockTelegramAPI struct {
 	server *httptest.Server
 
-	mu           sync.Mutex
-	sentMessages []tgbotapi.MessageConfig
-	editedMsgs   []tgbotapi.EditMessageTextConfig
-	callbacks    []tgbotapi.CallbackConfig
-	deletedMsgs  []tgbotapi.DeleteMessageConfig
+	mu             sync.Mutex
+	sentMessages   []tgbotapi.MessageConfig
+	editedMsgs     []tgbotapi.EditMessageTextConfig
+	callbacks      []tgbotapi.CallbackConfig
+	deletedMsgs    []tgbotapi.DeleteMessageConfig
+	setCommandsRaw string // raw JSON payload from the last setMyCommands call
 
 	nextMsgID int
 	updates   chan []tgbotapi.Update
@@ -141,6 +143,14 @@ func newMockTelegramAPI(t *testing.T) *mockTelegramAPI {
 				_ = json.NewEncoder(w).Encode(tgResponse{OK: true, Result: json.RawMessage(`[]`)})
 			}
 
+		case "setMyCommands":
+			_ = r.ParseForm()
+			raw := r.FormValue("commands")
+			m.mu.Lock()
+			m.setCommandsRaw = raw
+			m.mu.Unlock()
+			_ = json.NewEncoder(w).Encode(tgResponse{OK: true, Result: json.RawMessage(`true`)})
+
 		default:
 			_ = json.NewEncoder(w).Encode(tgResponse{OK: true, Result: json.RawMessage(`true`)})
 		}
@@ -185,6 +195,12 @@ func (m *mockTelegramAPI) getDeletedMessages() []tgbotapi.DeleteMessageConfig {
 	return out
 }
 
+func (m *mockTelegramAPI) getSetCommandsRaw() string {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	return m.setCommandsRaw
+}
+
 func escapeJSON(s string) string {
 	b, _ := json.Marshal(s)
 	// Strip surrounding quotes.
@@ -546,6 +562,55 @@ func TestCancelApprovalShowsShutdownReason(t *testing.T) {
 
 // --- Start/Stop lifecycle tests ---
 
+// TestRegisterCommands verifies that Start registers the bot command menu
+// with the expected entries (order preserved), including /mcp for MCP
+// upstream management.
+func TestRegisterCommands(t *testing.T) {
+	mock := newMockTelegramAPI(t)
+	s := newTestStore(t)
+	tc := newTestTelegramChannel(t, mock, s)
+
+	// Start calls registerCommands synchronously before returning, so the mock
+	// payload is available immediately with no polling needed.
+	if err := tc.Start(); err != nil {
+		t.Fatalf("Start: %v", err)
+	}
+	t.Cleanup(tc.Stop)
+
+	raw := mock.getSetCommandsRaw()
+	if raw == "" {
+		t.Fatal("setMyCommands was not called by Start()")
+	}
+
+	var cmds []tgbotapi.BotCommand
+	if err := json.Unmarshal([]byte(raw), &cmds); err != nil {
+		t.Fatalf("unmarshal commands payload: %v (raw=%q)", err, raw)
+	}
+
+	// Order matches the Telegram menu grouping convention: status first, then
+	// mutation groups, then meta commands.
+	want := []tgbotapi.BotCommand{
+		{Command: "status", Description: "Show proxy status"},
+		{Command: "policy", Description: "Manage policy rules"},
+		{Command: "cred", Description: "Manage credentials"},
+		{Command: "mcp", Description: "Manage MCP upstreams"},
+		{Command: "audit", Description: "Show audit log entries"},
+		{Command: "start", Description: "Show welcome message"},
+		{Command: "help", Description: "Show available commands"},
+	}
+	if len(cmds) != len(want) {
+		t.Fatalf("got %d commands, want %d (unexpected extras?): %+v", len(cmds), len(want), cmds)
+	}
+	for i, w := range want {
+		if cmds[i].Command != w.Command {
+			t.Errorf("cmds[%d].Command = %q, want %q", i, cmds[i].Command, w.Command)
+		}
+		if cmds[i].Description != w.Description {
+			t.Errorf("cmds[%d].Description = %q, want %q", i, cmds[i].Description, w.Description)
+		}
+	}
+}
+
 func TestStartStop(t *testing.T) {
 	mock := newMockTelegramAPI(t)
 	s := newTestStore(t)
@@ -1204,6 +1269,117 @@ func TestHandleMessageCredRotateDeletesMessage(t *testing.T) {
 	}
 }
 
+func TestHandleMessageMCPAddDeletesMessage(t *testing.T) {
+	mock := newMockTelegramAPI(t)
+	s := newTestStore(t)
+	tc := newTestTelegramChannel(t, mock, s)
+
+	// /mcp add may carry secrets via --env KEY=VAL so the chat message
+	// should be deleted the same way /cred add is.
+	tc.handleMessage(&tgbotapi.Message{
+		MessageID: 700,
+		Chat:      &tgbotapi.Chat{ID: 12345},
+		Text:      "/mcp add github --command npx --env GITHUB_PAT=super-secret",
+	})
+
+	time.Sleep(100 * time.Millisecond)
+	if len(mock.getDeletedMessages()) == 0 {
+		t.Error("mcp add message should be deleted for security")
+	}
+	// The plaintext --env value must not leak via the external command
+	// channel. handleMessage routes sensitive commands to the internal
+	// CommandHandler only.
+	if len(tc.cmdCh) != 0 {
+		t.Errorf("/mcp add must not forward to cmdCh (got %d entries, risks leaking --env secrets)", len(tc.cmdCh))
+	}
+}
+
+func TestHandleMessageMCPListNotDeleted(t *testing.T) {
+	mock := newMockTelegramAPI(t)
+	s := newTestStore(t)
+	tc := newTestTelegramChannel(t, mock, s)
+
+	// /mcp list never carries secrets so the message should not be deleted.
+	tc.handleMessage(&tgbotapi.Message{
+		MessageID: 701,
+		Chat:      &tgbotapi.Chat{ID: 12345},
+		Text:      "/mcp list",
+	})
+
+	time.Sleep(100 * time.Millisecond)
+	if len(mock.getDeletedMessages()) != 0 {
+		t.Error("non-sensitive /mcp list message should not be deleted")
+	}
+}
+
+// TestHandleMessageMCPListTruncation seeds enough upstreams to push /mcp
+// list past the 4000-rune telegram limit and verifies the truncated
+// response keeps <code> and <b> balanced. An unbalanced tag would make
+// Telegram reject the message under HTML parse mode.
+func TestHandleMessageMCPListTruncation(t *testing.T) {
+	mock := newMockTelegramAPI(t)
+	s := newTestStore(t)
+	// Seed 200 upstreams so the output comfortably exceeds telegramMaxMessage.
+	for i := 0; i < 200; i++ {
+		name := "upstream_" + strconv.Itoa(i)
+		if _, err := s.AddMCPUpstream(name, "npx", store.MCPUpstreamOpts{
+			Transport: "stdio",
+			Args:      []string{"--arg", "padding-to-ensure-overflow"},
+		}); err != nil {
+			t.Fatal(err)
+		}
+	}
+	tc := newTestTelegramChannel(t, mock, s)
+
+	tc.handleMessage(&tgbotapi.Message{
+		MessageID: 800,
+		Chat:      &tgbotapi.Chat{ID: 12345},
+		Text:      "/mcp list",
+	})
+
+	time.Sleep(100 * time.Millisecond)
+	msgs := mock.getSentMessages()
+	if len(msgs) == 0 {
+		t.Fatal("expected sendMessage to be called")
+	}
+	text := msgs[len(msgs)-1].Text
+	if !strings.Contains(text, "(truncated)") {
+		t.Errorf("expected truncation marker, got first 200 chars: %q", text[:min(200, len(text))])
+	}
+	if opens, closes := strings.Count(text, "<code>"), strings.Count(text, "</code>"); opens != closes {
+		t.Errorf("truncated output breaks <code> balance: %d open, %d close", opens, closes)
+	}
+	if opens, closes := strings.Count(text, "<b>"), strings.Count(text, "</b>"); opens != closes {
+		t.Errorf("truncated output breaks <b> balance: %d open, %d close", opens, closes)
+	}
+}
+
+func TestContainsSensitiveArgs(t *testing.T) {
+	tests := []struct {
+		name string
+		cmd  *Command
+		want bool
+	}{
+		{"nil command", nil, false},
+		{"empty args", &Command{Name: "cred"}, false},
+		{"cred add", &Command{Name: "cred", Args: []string{"add", "name", "secret"}}, true},
+		{"cred rotate", &Command{Name: "cred", Args: []string{"rotate", "name", "secret"}}, true},
+		{"cred list", &Command{Name: "cred", Args: []string{"list"}}, false},
+		{"cred remove", &Command{Name: "cred", Args: []string{"remove", "name"}}, false},
+		{"mcp add", &Command{Name: "mcp", Args: []string{"add", "name", "--command", "cmd"}}, true},
+		{"mcp list", &Command{Name: "mcp", Args: []string{"list"}}, false},
+		{"mcp remove", &Command{Name: "mcp", Args: []string{"remove", "name"}}, false},
+		{"policy show", &Command{Name: "policy", Args: []string{"show"}}, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := containsSensitiveArgs(tt.cmd); got != tt.want {
+				t.Errorf("containsSensitiveArgs(%+v) = %v, want %v", tt.cmd, got, tt.want)
+			}
+		})
+	}
+}
+
 func TestHandleMessageForwardsToCommandChannel(t *testing.T) {
 	mock := newMockTelegramAPI(t)
 	s := newTestStore(t)
@@ -1309,7 +1485,7 @@ func TestCredAddWithContainerManager(t *testing.T) {
 	if !strings.Contains(result, "Added credential") {
 		t.Errorf("expected add confirmation, got: %s", result)
 	}
-	if mgr.injectCalled {
+	if mgr.injectCalledSafe() {
 		t.Error("InjectEnvVars should not be called when no env_var bindings exist")
 	}
 }
@@ -1337,10 +1513,10 @@ func TestCredAddWithContainerManagerAndEnvVar(t *testing.T) {
 		t.Errorf("should indicate env vars updated, got: %s", result)
 	}
 
-	if !mgr.injectCalled {
+	if !mgr.injectCalledSafe() {
 		t.Error("InjectEnvVars should have been called")
 	}
-	if _, ok := mgr.injectEnv["OPENAI_API_KEY"]; !ok {
+	if _, ok := mgr.injectEnvSafe()["OPENAI_API_KEY"]; !ok {
 		t.Error("InjectEnvVars should include OPENAI_API_KEY")
 	}
 }
@@ -1369,10 +1545,10 @@ func TestCredRemoveWithContainerManager(t *testing.T) {
 		t.Errorf("expected remove confirmation, got: %s", result)
 	}
 
-	if !mgr.injectCalled {
+	if !mgr.injectCalledSafe() {
 		t.Error("InjectEnvVars should have been called after remove")
 	}
-	if v, ok := mgr.injectEnv["TEST_API_KEY"]; !ok || v != "" {
+	if v, ok := mgr.injectEnvSafe()["TEST_API_KEY"]; !ok || v != "" {
 		t.Errorf("removed env var should be empty, got: %q (exists=%v)", v, ok)
 	}
 }
@@ -1399,7 +1575,7 @@ func TestCredRotateWithContainerManager(t *testing.T) {
 	if !strings.Contains(result, "Rotated credential") {
 		t.Errorf("expected rotate confirmation, got: %s", result)
 	}
-	if !mgr.injectCalled {
+	if !mgr.injectCalledSafe() {
 		t.Error("InjectEnvVars should have been called after rotate")
 	}
 }
@@ -1815,29 +1991,75 @@ func TestRebuildResolverEmptyBindings(t *testing.T) {
 
 // --- mockContainerMgr ---
 
+// mockContainerMgr is a concurrency-safe stub ContainerManager used by the
+// Telegram tests. All state fields are guarded by mu because command handlers
+// may run on background goroutines (e.g. the telegram update loop) while the
+// test asserts; the mutex prevents data races flagged by -race and gives tests
+// deterministic reads.
 type mockContainerMgr struct {
+	mu            sync.Mutex
 	injectCalled  bool
 	injectEnv     map[string]string
 	injectErr     error
 	restartCalled bool
 	restartErr    error
+	// wireCalled tracks calls to WireMCPGateway. The MCP upstream mutation
+	// path must NOT invoke it (sluice URL is wired once at startup and does
+	// not change on /mcp add or /mcp remove), so tests assert wireCalled
+	// remains false after those operations.
+	wireCalled bool
 }
 
 func (m *mockContainerMgr) InjectEnvVars(_ context.Context, envMap map[string]string, _ bool) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	m.injectCalled = true
 	m.injectEnv = envMap
 	return m.injectErr
 }
 
 func (m *mockContainerMgr) RestartWithEnv(_ context.Context, _ map[string]string) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	m.restartCalled = true
 	return m.restartErr
 }
 
 func (m *mockContainerMgr) WireMCPGateway(_ context.Context, _, _ string) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	m.wireCalled = true
 	return nil
 }
 
+// injectCalledSafe, injectEnvSafe, and wireCalledSafe are read accessors that
+// lock mu so tests can observe state after handlers complete without tripping
+// the race detector.
+func (m *mockContainerMgr) injectCalledSafe() bool {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	return m.injectCalled
+}
+
+func (m *mockContainerMgr) injectEnvSafe() map[string]string {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if m.injectEnv == nil {
+		return nil
+	}
+	out := make(map[string]string, len(m.injectEnv))
+	for k, v := range m.injectEnv {
+		out[k] = v
+	}
+	return out
+}
+
+func (m *mockContainerMgr) wireCalledSafe() bool {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	return m.wireCalled
+}
+
 func (m *mockContainerMgr) Status(_ context.Context) (container.ContainerStatus, error) {
 	return container.ContainerStatus{Running: true}, nil
 }
diff --git a/internal/telegram/commands.go b/internal/telegram/commands.go
index 9ef3ff0..c2e64cd 100644
--- a/internal/telegram/commands.go
+++ b/internal/telegram/commands.go
@@ -2,10 +2,13 @@ package telegram
 
 import (
 	"context"
+	"flag"
 	"fmt"
+	"io"
 	"log"
 	"os"
 	"regexp"
+	"sort"
 	"strconv"
 	"strings"
 	"sync"
@@ -14,6 +17,8 @@ import (
 
 	"github.com/nemirovsky/sluice/internal/channel"
 	"github.com/nemirovsky/sluice/internal/container"
+	"github.com/nemirovsky/sluice/internal/flagutil"
+	"github.com/nemirovsky/sluice/internal/mcp"
 	"github.com/nemirovsky/sluice/internal/policy"
 	"github.com/nemirovsky/sluice/internal/store"
 	"github.com/nemirovsky/sluice/internal/vault"
@@ -27,13 +32,19 @@ type Command struct {
 
 // ParseCommand parses a Telegram message into a Command.
 // Returns nil if the message is not a command (doesn't start with /).
+//
+// Tokenization respects double and single quotes so operators can pass
+// values that contain spaces or commas via the documented forms such as
+// /mcp add myserver --args "a,b" --env "K=V,K=V" --header "Authorization=Bearer tok".
+// Without quote-aware splitting, the args "a,b" token would split into two
+// fields and drop the quoting, corrupting the downstream flag parse.
 func ParseCommand(text string) *Command {
 	text = strings.TrimSpace(text)
 	if !strings.HasPrefix(text, "/") {
 		return nil
 	}
-	parts := strings.Fields(text)
-	if len(parts) == 0 {
+	parts, err := tokenizeShellish(text)
+	if err != nil || len(parts) == 0 {
 		return nil
 	}
 	name := strings.TrimPrefix(parts[0], "/")
@@ -47,6 +58,69 @@ func ParseCommand(text string) *Command {
 	}
 }
 
+// tokenizeShellish splits a command-line style string into tokens, respecting
+// double and single quotes. Backslash escapes the next character inside or
+// outside double quotes. Single quotes are literal (no escapes). This mirrors
+// the subset of POSIX shell tokenization that Telegram users are likely to
+// reach for (quoted CSV values, quoted bearer tokens with spaces) without
+// pulling in a full shell parser dependency.
+//
+// Returns an error if a quote is left unterminated so ParseCommand can treat
+// malformed input as not-a-command rather than silently dropping quotes.
+func tokenizeShellish(text string) ([]string, error) {
+	var tokens []string
+	var cur strings.Builder
+	inToken := false
+	quote := byte(0) // 0 = unquoted, '"' or '\'' = inside that quote style
+	for i := 0; i < len(text); i++ {
+		c := text[i]
+		if quote == 0 {
+			switch {
+			case c == ' ' || c == '\t' || c == '\n' || c == '\r':
+				if inToken {
+					tokens = append(tokens, cur.String())
+					cur.Reset()
+					inToken = false
+				}
+			case c == '"' || c == '\'':
+				quote = c
+				inToken = true
+			case c == '\\' && i+1 < len(text):
+				i++
+				cur.WriteByte(text[i])
+				inToken = true
+			default:
+				cur.WriteByte(c)
+				inToken = true
+			}
+			continue
+		}
+		// Inside a quoted run.
+		if c == quote {
+			quote = 0
+			continue
+		}
+		if quote == '"' && c == '\\' && i+1 < len(text) {
+			next := text[i+1]
+			// Only a handful of escapes are meaningful inside double quotes
+			// (matching POSIX). For other chars the backslash is preserved.
+			if next == '"' || next == '\\' || next == '$' || next == '`' || next == '\n' {
+				i++
+				cur.WriteByte(next)
+				continue
+			}
+		}
+		cur.WriteByte(c)
+	}
+	if quote != 0 {
+		return nil, fmt.Errorf("unterminated quote %c", quote)
+	}
+	if inToken {
+		tokens = append(tokens, cur.String())
+	}
+	return tokens, nil
+}
+
 // CommandHandler holds the dependencies needed by command handlers.
 type CommandHandler struct {
 	engine              *atomic.Pointer[policy.Engine]
@@ -172,6 +246,8 @@ func (h *CommandHandler) Handle(cmd *Command) string {
 		return h.handlePolicy(cmd.Args)
 	case "cred":
 		return h.handleCred(cmd.Args)
+	case "mcp":
+		return h.handleMCP(cmd.Args)
 	case "status":
 		return h.handleStatus()
 	case "audit":
@@ -660,6 +736,300 @@ func (h *CommandHandler) credMutationComplete(msg string, removedEnvVars ...stri
 	return msg + "\nAgent env vars updated."
 }
 
+// handleMCP dispatches /mcp subcommands.
+func (h *CommandHandler) handleMCP(args []string) string {
+	// Guard first so that when the store is not configured, operators get
+	// a clear diagnostic even if they called /mcp with no args. Emitting
+	// the usage banner in that state would advertise commands that will
+	// all subsequently fail.
+	if h.store == nil {
+		return "MCP management is not available (policy store not configured)."
+	}
+	if len(args) == 0 {
+		return "Usage: /mcp list | " + mcpAddUsage + " | /mcp remove <name>"
+	}
+	switch args[0] {
+	case "list":
+		return h.mcpList()
+	case "add":
+		return h.mcpAdd(args[1:])
+	case "remove":
+		return h.mcpRemove(args[1:])
+	default:
+		return fmt.Sprintf("Unknown mcp subcommand: %s", args[0])
+	}
+}
+
+// mcpAddUsage is the usage string returned when /mcp add is called with
+// missing required flags or no positional name.
+//
+// Limitations operators should be aware of:
+//   - --args and --env are comma-separated so individual values cannot contain
+//     commas. For env values that need commas, set them via config TOML or CLI.
+//   - --header is repeatable (pass the flag once per KEY=VAL pair) so commas
+//     in header values are not a problem.
+//   - --env values are rendered verbatim in /mcp list output. Use the vault:
+//     indirection (e.g. GITHUB_PAT=vault:github_pat) to keep secrets out of
+//     the SQLite store and list responses.
+const mcpAddUsage = "Usage: /mcp add <name> --command <cmd> [--transport stdio|http|websocket] [--args \"a,b\"] [--env \"K=V,K=V\"] [--header \"K=V\" ...] [--timeout 120]"
+
+// mcpAdd registers a new MCP upstream from /mcp add arguments.
+//
+// Adding an upstream to the store is NOT enough for the running MCP gateway to
+// pick it up: the gateway builds its upstream set at startup and does not
+// hot-reload. After /mcp add the operator must restart sluice for the new
+// upstream to take effect. The agent container's openclaw.json always points
+// at sluice-as-a-whole (mcp.servers.sluice = {url: http://sluice:3000/mcp}),
+// which is wired once at sluice startup. There is no per-upstream entry for
+// the agent to re-read, so no gateway-level RPC is issued on /mcp add.
+func (h *CommandHandler) mcpAdd(args []string) string {
+	opts, errMsg := parseMCPAddFlags(args)
+	if errMsg != "" {
+		return errMsg
+	}
+
+	// reloadMu is not held here: MCP upstream changes do not recompile the
+	// policy engine or the binding resolver, and the running gateway is not
+	// hot-reloaded (the operator must restart sluice for the new upstream to
+	// take effect, see the function doc above).
+	id, err := h.store.AddMCPUpstream(opts.name, opts.command, store.MCPUpstreamOpts{
+		Args:       opts.cmdArgs,
+		Env:        opts.env,
+		Headers:    opts.headers,
+		TimeoutSec: opts.timeout,
+		Transport:  opts.transport,
+	})
+	if err != nil {
+		return fmt.Sprintf("Failed to add MCP upstream: %v", err)
+	}
+
+	return fmt.Sprintf(
+		"Added MCP upstream [%d] %s (%s)\nRestart sluice for the new upstream to take effect.",
+		id, htmlCode(opts.name), htmlCode(opts.transport),
+	)
+}
+
+// mcpAddOpts holds the fully parsed and validated arguments for /mcp add.
+type mcpAddOpts struct {
+	name      string
+	command   string
+	transport string
+	cmdArgs   []string
+	env       map[string]string
+	headers   map[string]string
+	timeout   int
+}
+
+// parseMCPAddFlags parses /mcp add arguments into mcpAddOpts. Returns either a
+// populated mcpAddOpts with empty errMsg, or a zero mcpAddOpts and a
+// user-facing error message for Telegram.
+func parseMCPAddFlags(args []string) (mcpAddOpts, string) {
+	if len(args) == 0 {
+		return mcpAddOpts{}, mcpAddUsage
+	}
+
+	fs := flag.NewFlagSet("mcp add", flag.ContinueOnError)
+	// Suppress stdlib's default error-to-stderr: errors surface via Parse
+	// and we translate them into Telegram responses.
+	fs.SetOutput(io.Discard)
+	command := fs.String("command", "", "command or URL")
+	transport := fs.String("transport", mcp.TransportStdio, "transport type")
+	argsStr := fs.String("args", "", "comma-separated args")
+	envStr := fs.String("env", "", "comma-separated KEY=VAL env pairs")
+	headers := make(map[string]string)
+	fs.Func("header", "KEY=VAL HTTP header, repeatable (http transport only)", func(s string) error {
+		parts := strings.SplitN(s, "=", 2)
+		if len(parts) != 2 || parts[0] == "" {
+			return fmt.Errorf("invalid --header %q: expected KEY=VAL", s)
+		}
+		headers[parts[0]] = parts[1]
+		return nil
+	})
+	timeout := fs.Int("timeout", mcp.DefaultTimeoutSec, "per-call timeout in seconds")
+	if err := fs.Parse(flagutil.ReorderFlagsBeforePositional(args, fs)); err != nil {
+		return mcpAddOpts{}, fmt.Sprintf("Invalid argument: %s\n%s", err, mcpAddUsage)
+	}
+
+	positional := fs.Args()
+	if len(positional) == 0 {
+		return mcpAddOpts{}, mcpAddUsage
+	}
+	if len(positional) > 1 {
+		return mcpAddOpts{}, fmt.Sprintf("Unexpected argument %q.\n%s", positional[1], mcpAddUsage)
+	}
+	name := positional[0]
+
+	if *command == "" {
+		return mcpAddOpts{}, mcpAddUsage
+	}
+
+	if err := mcp.ValidateUpstreamName(name); err != nil {
+		return mcpAddOpts{}, fmt.Sprintf("Invalid upstream name: %v", err)
+	}
+
+	if !mcp.ValidTransport(*transport) {
+		// Keep the transport list in sync with internal/store.Store.AddMCPUpstream
+		// and internal/mcp.ValidTransport.
+		return mcpAddOpts{}, fmt.Sprintf("Invalid transport %q: must be stdio, http, or websocket", *transport)
+	}
+
+	if *timeout <= 0 {
+		return mcpAddOpts{}, fmt.Sprintf("Invalid --timeout %d: must be a positive integer (seconds)", *timeout)
+	}
+
+	var cmdArgs []string
+	if *argsStr != "" {
+		cmdArgs = strings.Split(*argsStr, ",")
+	}
+
+	env, errMsg := parseCSVKeyValues(*envStr, "--env")
+	if errMsg != "" {
+		return mcpAddOpts{}, errMsg
+	}
+
+	if len(headers) > 0 && *transport != mcp.TransportHTTP {
+		return mcpAddOpts{}, fmt.Sprintf("--header is only valid for --transport %s", mcp.TransportHTTP)
+	}
+
+	return mcpAddOpts{
+		name:      name,
+		command:   *command,
+		transport: *transport,
+		cmdArgs:   cmdArgs,
+		env:       env,
+		headers:   headers,
+		timeout:   *timeout,
+	}, ""
+}
+
+// parseCSVKeyValues parses "K=V[,K=V,...]" into a map. Returns a user-facing
+// error message via the second return (empty on success).
+func parseCSVKeyValues(csv, flagName string) (map[string]string, string) {
+	if csv == "" {
+		return nil, ""
+	}
+	out := make(map[string]string)
+	for _, kv := range strings.Split(csv, ",") {
+		parts := strings.SplitN(kv, "=", 2)
+		if len(parts) != 2 || parts[0] == "" {
+			return nil, fmt.Sprintf("Invalid %s %q: expected KEY=VAL[,KEY=VAL,...]", flagName, csv)
+		}
+		out[parts[0]] = parts[1]
+	}
+	return out, ""
+}
+
+// mcpRemove removes an MCP upstream by name. Returns a human-readable message
+// for Telegram. Like /mcp add, a sluice restart is required for the running
+// gateway to pick up the removal (see mcpAdd for the rationale).
+func (h *CommandHandler) mcpRemove(args []string) string {
+	if len(args) == 0 {
+		return "Usage: /mcp remove <name>"
+	}
+	name := args[0]
+	if len(args) > 1 {
+		return fmt.Sprintf("Unexpected argument %q.\nUsage: /mcp remove <name>", args[1])
+	}
+
+	// reloadMu is not held here for the same reason as mcpAdd: MCP upstream
+	// changes do not recompile the policy engine or the binding resolver.
+	deleted, err := h.store.RemoveMCPUpstream(name)
+	if err != nil {
+		return fmt.Sprintf("Failed to remove MCP upstream: %v", err)
+	}
+	if !deleted {
+		return fmt.Sprintf("No MCP upstream named %s", htmlCode(name))
+	}
+
+	return fmt.Sprintf(
+		"Removed MCP upstream %s\nRestart sluice for the removal to take effect.",
+		htmlCode(name),
+	)
+}
+
+// mcpList renders all registered MCP upstreams for Telegram display.
+func (h *CommandHandler) mcpList() string {
+	upstreams, err := h.store.ListMCPUpstreams()
+	if err != nil {
+		return fmt.Sprintf("Failed to list MCP upstreams: %v", err)
+	}
+	if len(upstreams) == 0 {
+		return "No MCP upstreams registered."
+	}
+
+	var b strings.Builder
+	b.WriteString("<b>MCP upstreams</b>\n")
+	for _, u := range upstreams {
+		transport := u.Transport
+		if transport == "" {
+			transport = mcp.TransportStdio
+		}
+		fmt.Fprintf(&b, "[%d] %s (%s)\n  command: %s\n",
+			u.ID, htmlCode(u.Name), htmlCode(transport), htmlCode(u.Command))
+		if len(u.Args) > 0 {
+			fmt.Fprintf(&b, "  args: %s\n", htmlCode(strings.Join(u.Args, " ")))
+		}
+		if line := sortedKVLineRedacted(u.Env); line != "" {
+			fmt.Fprintf(&b, "  env: %s\n", htmlCode(line))
+		}
+		if line := sortedKVLineRedacted(u.Headers); line != "" {
+			fmt.Fprintf(&b, "  headers: %s\n", htmlCode(line))
+		}
+		if u.TimeoutSec != 0 && u.TimeoutSec != mcp.DefaultTimeoutSec {
+			fmt.Fprintf(&b, "  timeout: %ds\n", u.TimeoutSec)
+		}
+	}
+	return b.String()
+}
+
+// sortedKVLineRedacted renders a map as "k1=v1, k2=v2" with keys sorted and
+// values masked unless they are whole-value vault indirections.
+//
+// Values that are exactly "vault:<name>" are safe to surface because they are
+// pointers to credentials rather than the credentials themselves. Any other
+// value is replaced with "****" so raw env values and header values added
+// through Telegram, CLI, or the REST API cannot be retrieved by reading chat
+// history. Operators who need to audit the raw stored values can query the
+// SQLite store directly.
+//
+// Template forms like "Bearer {vault:github_pat}" are also masked, because
+// the literal prefix ("Bearer ", etc.) can still carry human-chosen content
+// that the operator did not intend for chat display. The goal is to default
+// toward redaction and let vault indirection be the explicit opt-in for
+// surfacing a value.
+//
+// Returns "" when the map is empty so callers can skip the row entirely.
+func sortedKVLineRedacted(m map[string]string) string {
+	if len(m) == 0 {
+		return ""
+	}
+	keys := make([]string, 0, len(m))
+	for k := range m {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	pairs := make([]string, 0, len(keys))
+	for _, k := range keys {
+		pairs = append(pairs, k+"="+maskUpstreamValue(m[k]))
+	}
+	return strings.Join(pairs, ", ")
+}
+
+// maskUpstreamValue returns v when it is a whole-value vault indirection
+// ("vault:<name>" with a non-empty name) and "****" otherwise. The empty
+// string passes through unchanged so operators can distinguish "KEY=" (set
+// but empty) from "KEY=****" (set to a real, masked value).
+func maskUpstreamValue(v string) string {
+	if v == "" {
+		return ""
+	}
+	const prefix = "vault:"
+	if strings.HasPrefix(v, prefix) && len(v) > len(prefix) && !strings.ContainsAny(v[len(prefix):], " \t") {
+		return v
+	}
+	return "****"
+}
+
 func (h *CommandHandler) handleStatus() string {
 	snap := h.engine.Load().Snapshot()
 	var b strings.Builder
@@ -738,6 +1108,14 @@ Credentials
 /cred rotate <name> <value> | /cred remove <name>`
 	}
 
+	if h.store != nil {
+		help += `
+
+MCP Upstreams
+/mcp list | /mcp add <name> --command <cmd> [--transport stdio|http|websocket] [--args "a,b"] [--env "K=V,K=V"] [--header "K=V"] [--timeout 120]
+/mcp remove <name>`
+	}
+
 	help += `
 
 More: /start for welcome, /help for this message`
diff --git a/internal/telegram/commands_test.go b/internal/telegram/commands_test.go
index 0f035eb..dd6a4ab 100644
--- a/internal/telegram/commands_test.go
+++ b/internal/telegram/commands_test.go
@@ -58,6 +58,38 @@ func TestParseCommand(t *testing.T) {
 		{"", true, "", nil},
 		{"/help", false, "help", nil},
 		{"/policy@mybot show", false, "policy", []string{"show"}},
+
+		// Quote-aware tokenization. Documented forms like --args "a,b"
+		// must preserve the comma-separated value as a single token so
+		// downstream flag parsing sees the intended value rather than
+		// two stray positional args.
+		{
+			`/mcp add github --command npx --args "a,b"`,
+			false, "mcp",
+			[]string{"add", "github", "--command", "npx", "--args", "a,b"},
+		},
+		{
+			`/mcp add notion --command https://mcp.notion.com --env "FOO=1,BAR=2"`,
+			false, "mcp",
+			[]string{"add", "notion", "--command", "https://mcp.notion.com", "--env", "FOO=1,BAR=2"},
+		},
+		{
+			`/mcp add api --header "Authorization=Bearer tok with space"`,
+			false, "mcp",
+			[]string{"add", "api", "--header", "Authorization=Bearer tok with space"},
+		},
+		// Single quotes are literal - backslash inside does not escape.
+		{
+			`/policy allow 'host with space'`,
+			false, "policy",
+			[]string{"allow", "host with space"},
+		},
+		// Unterminated quote is treated as not-a-command rather than
+		// silently dropping the quote.
+		{
+			`/mcp add name --args "unterminated`,
+			true, "", nil,
+		},
 	}
 
 	for _, tt := range tests {
@@ -413,6 +445,34 @@ func TestHandleHelp(t *testing.T) {
 	if !strings.Contains(result, "/audit") {
 		t.Error("help should mention /audit")
 	}
+	if !strings.Contains(result, "/mcp") {
+		t.Error("help should mention /mcp when store is configured")
+	}
+	if !strings.Contains(result, "MCP Upstreams") {
+		t.Error("help should include MCP Upstreams section when store is configured")
+	}
+}
+
+// TestHandleHelpNoStore verifies the MCP section is omitted when store is nil,
+// matching how /cred help is gated on vault availability.
+func TestHandleHelpNoStore(t *testing.T) {
+	ptr := new(atomic.Pointer[policy.Engine])
+	eng, err := policy.LoadFromBytes([]byte(`[policy]
+default = "deny"
+`))
+	if err != nil {
+		t.Fatal(err)
+	}
+	ptr.Store(eng)
+	handler := NewCommandHandler(ptr, new(sync.Mutex), "")
+	result := handler.Handle(&Command{Name: "help"})
+
+	if strings.Contains(result, "/mcp") {
+		t.Error("help should not mention /mcp when store is nil")
+	}
+	if strings.Contains(result, "MCP Upstreams") {
+		t.Error("help should not include MCP Upstreams section when store is nil")
+	}
 }
 
 func TestHandleCredNoVault(t *testing.T) {
@@ -792,6 +852,677 @@ func TestCredAddWithoutEnvVar(t *testing.T) {
 	}
 }
 
+func TestHandleMCPNoArgs(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+	result := handler.Handle(&Command{Name: "mcp"})
+
+	if !strings.Contains(result, "Usage: /mcp") {
+		t.Errorf("should show usage when no args, got: %s", result)
+	}
+	if !strings.Contains(result, "list") || !strings.Contains(result, "add") || !strings.Contains(result, "remove") {
+		t.Errorf("usage should mention list/add/remove, got: %s", result)
+	}
+}
+
+func TestHandleMCPNoStore(t *testing.T) {
+	// CommandHandler without a store should report MCP management is unavailable.
+	// Build the engine from a transient store but omit SetStore on the handler.
+	s := newTestStore(t)
+	eng, err := policy.LoadFromStore(s)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ptr := new(atomic.Pointer[policy.Engine])
+	ptr.Store(eng)
+	handler := NewCommandHandler(ptr, new(sync.Mutex), "")
+	// Deliberately do not call SetStore.
+
+	result := handler.Handle(&Command{Name: "mcp", Args: []string{"list"}})
+	if !strings.Contains(result, "not available") {
+		t.Errorf("should report not available when store is not configured, got: %s", result)
+	}
+
+	// Also cover the no-args path. handleMCP's store guard runs before the
+	// usage banner so operators get a clear "not available" diagnostic
+	// rather than a usage string that advertises commands they cannot use.
+	resultNoArgs := handler.Handle(&Command{Name: "mcp", Args: nil})
+	if !strings.Contains(resultNoArgs, "not available") {
+		t.Errorf("should report not available on bare /mcp when store is not configured, got: %s", resultNoArgs)
+	}
+	if strings.Contains(resultNoArgs, "Usage: /mcp") {
+		t.Errorf("store-guard must precede usage banner, got usage instead: %s", resultNoArgs)
+	}
+}
+
+func TestHandleMCPUnknownSubcommand(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+	result := handler.Handle(&Command{Name: "mcp", Args: []string{"bogus"}})
+
+	if !strings.Contains(result, "Unknown mcp subcommand") {
+		t.Errorf("should report unknown subcommand, got: %s", result)
+	}
+}
+
+func TestHandleMCPListEmpty(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	result := handler.Handle(&Command{Name: "mcp", Args: []string{"list"}})
+	if !strings.Contains(result, "No MCP upstreams") {
+		t.Errorf("should report empty list, got: %s", result)
+	}
+}
+
+func TestHandleMCPListWithUpstreams(t *testing.T) {
+	s := newTestStore(t)
+	// Add a stdio upstream with args and env. The env value is a whole-value
+	// vault indirection which is safe to surface verbatim.
+	if _, err := s.AddMCPUpstream("github", "npx", store.MCPUpstreamOpts{
+		Args:       []string{"-y", "@modelcontextprotocol/server-github"},
+		Env:        map[string]string{"GITHUB_PAT": "vault:github_pat"},
+		TimeoutSec: 120,
+		Transport:  "stdio",
+	}); err != nil {
+		t.Fatal(err)
+	}
+	// Add an http upstream with headers and a non-default timeout. The header
+	// value is a templated form ("Bearer vault:notion_token") which is NOT a
+	// whole-value vault pointer, so it must be masked to "****" in the
+	// rendered output.
+	if _, err := s.AddMCPUpstream("notion", "https://mcp.notion.com", store.MCPUpstreamOpts{
+		Headers:    map[string]string{"Authorization": "Bearer vault:notion_token"},
+		TimeoutSec: 60,
+		Transport:  "http",
+	}); err != nil {
+		t.Fatal(err)
+	}
+
+	handler := newTestHandlerWithStore(t, s, nil, "")
+	out := handler.Handle(&Command{Name: "mcp", Args: []string{"list"}})
+
+	// Expect names, transports, commands, and safe-to-show env/header forms.
+	must := []string{
+		"github",
+		"stdio",
+		"<code>npx</code>",
+		"notion",
+		"http",
+		"<code>https://mcp.notion.com</code>",
+		"-y @modelcontextprotocol/server-github",
+		"GITHUB_PAT=vault:github_pat", // whole-value vault pointer is safe
+		"Authorization=****",          // templated value is masked
+		"timeout: 60s",
+	}
+	for _, want := range must {
+		if !strings.Contains(out, want) {
+			t.Errorf("mcp list output missing %q\nfull output:\n%s", want, out)
+		}
+	}
+	// The raw templated header value must NOT leak into chat.
+	forbidden := []string{
+		"Bearer vault:notion_token",
+		"notion_token",
+	}
+	for _, bad := range forbidden {
+		if strings.Contains(out, bad) {
+			t.Errorf("mcp list output must not contain %q\nfull output:\n%s", bad, out)
+		}
+	}
+	// Default (120s) timeout should NOT be rendered.
+	if strings.Contains(out, "timeout: 120s") {
+		t.Errorf("default 120s timeout should be omitted, got: %s", out)
+	}
+}
+
+// TestHandleMCPListRedactsSecrets asserts that raw plaintext env values and
+// raw plaintext header values are masked out of /mcp list output. This locks
+// in the security regression guard called out by the external review: the
+// /mcp add auto-delete-on-send protection is only meaningful if the same
+// values do not later reappear in chat via /mcp list.
+func TestHandleMCPListRedactsSecrets(t *testing.T) {
+	s := newTestStore(t)
+	// Raw plaintext env value and raw plaintext header value. Neither is a
+	// whole-value vault pointer, so both must be masked in the rendered
+	// output.
+	if _, err := s.AddMCPUpstream("leaky", "https://example.com", store.MCPUpstreamOpts{
+		Transport: "http",
+		Env:       map[string]string{"SUPER_SECRET": "ghp_rawtokenvalue"},
+		Headers:   map[string]string{"Authorization": "Bearer sk-liveapikey"},
+	}); err != nil {
+		t.Fatal(err)
+	}
+
+	handler := newTestHandlerWithStore(t, s, nil, "")
+	out := handler.Handle(&Command{Name: "mcp", Args: []string{"list"}})
+
+	// Masked renders must appear.
+	masked := []string{
+		"SUPER_SECRET=****",
+		"Authorization=****",
+	}
+	for _, want := range masked {
+		if !strings.Contains(out, want) {
+			t.Errorf("mcp list should mask value, missing %q\nfull output:\n%s", want, out)
+		}
+	}
+	// Raw secrets must NOT appear anywhere in the output.
+	leaked := []string{
+		"ghp_rawtokenvalue",
+		"sk-liveapikey",
+		"Bearer sk-liveapikey",
+	}
+	for _, bad := range leaked {
+		if strings.Contains(out, bad) {
+			t.Errorf("mcp list leaked plaintext %q\nfull output:\n%s", bad, out)
+		}
+	}
+}
+
+func TestHandleMCPListEscapesHTML(t *testing.T) {
+	s := newTestStore(t)
+	// An http upstream exercises the args/env/header rendering paths
+	// which use htmlCode wrapping internally. Name, command, args, and
+	// the env/header KEYS contain "<" or "&" so we can confirm none of
+	// them leak past the HTML escape. Env and header VALUES are masked
+	// to "****" (see sortedKVLineRedacted and TestHandleMCPListRedactsSecrets),
+	// so we deliberately do not assert on their escaped form. The key
+	// side of the KEY=**** pair is still a meaningful escape target.
+	if _, err := s.AddMCPUpstream("my<srv>", "https://example.com/<svc>", store.MCPUpstreamOpts{
+		Transport: "http",
+		Args:      []string{"--mode=<dev>", "&flag"},
+		Env:       map[string]string{"X<KEY>": "v&a<l>ue"},
+		Headers:   map[string]string{"X-H<dr>": "Bearer <tok>&n"},
+	}); err != nil {
+		t.Fatal(err)
+	}
+
+	handler := newTestHandlerWithStore(t, s, nil, "")
+	out := handler.Handle(&Command{Name: "mcp", Args: []string{"list"}})
+
+	// Raw tags must not appear. The "<" checks catch any unescaped tag
+	// content directly. Value-side raw tags ("<l>", "<tok>") are covered
+	// here too because the redaction mask should have already replaced
+	// them, so their absence is both an escape and a redaction check.
+	badRaw := []string{"<srv>", "<svc>", "<dev>", "<KEY>", "<dr>", "<tok>", "<l>", "&flag"}
+	for _, s := range badRaw {
+		if strings.Contains(out, s) {
+			t.Errorf("unescaped substring %q in output: %s", s, out)
+		}
+	}
+	// Escaped equivalents on the key side (name, command, args, env/header
+	// keys) must appear. Value-side escapes are intentionally not asserted
+	// because the values are masked.
+	goodEsc := []string{
+		"my&lt;srv&gt;",
+		"&lt;svc&gt;",
+		"&lt;dev&gt;",
+		"X&lt;KEY&gt;",
+		"X-H&lt;dr&gt;",
+		"&amp;flag",
+	}
+	for _, s := range goodEsc {
+		if !strings.Contains(out, s) {
+			t.Errorf("expected escaped %q in output: %s", s, out)
+		}
+	}
+}
+
+func TestHandleMCPAddStdio(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{"add", "github", "--command", "npx"},
+	})
+	if !strings.Contains(result, "Added MCP upstream") {
+		t.Fatalf("should confirm add, got: %s", result)
+	}
+	if !strings.Contains(result, "github") || !strings.Contains(result, "stdio") {
+		t.Errorf("expected name and transport in response, got: %s", result)
+	}
+	if !strings.Contains(result, "Restart sluice") {
+		t.Errorf("expected restart notice in response, got: %s", result)
+	}
+
+	upstreams, err := s.ListMCPUpstreams()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(upstreams) != 1 {
+		t.Fatalf("expected 1 upstream, got %d", len(upstreams))
+	}
+	u := upstreams[0]
+	if u.Name != "github" {
+		t.Errorf("name = %q, want %q", u.Name, "github")
+	}
+	if u.Command != "npx" {
+		t.Errorf("command = %q, want %q", u.Command, "npx")
+	}
+	if u.Transport != "stdio" {
+		t.Errorf("transport = %q, want stdio", u.Transport)
+	}
+	if u.TimeoutSec != 120 {
+		t.Errorf("timeout = %d, want 120", u.TimeoutSec)
+	}
+	if len(u.Args) != 0 {
+		t.Errorf("args = %v, want empty", u.Args)
+	}
+	if len(u.Env) != 0 {
+		t.Errorf("env = %v, want empty", u.Env)
+	}
+}
+
+func TestHandleMCPAddWithArgsAndEnv(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{
+			"add", "github", "--command", "npx",
+			"--args", "-y,@modelcontextprotocol/server-github",
+			"--env", "GITHUB_PAT=vault:github_pat,DEBUG=1",
+			"--timeout", "60",
+		},
+	})
+	if !strings.Contains(result, "Added MCP upstream") {
+		t.Fatalf("should confirm add, got: %s", result)
+	}
+
+	upstreams, err := s.ListMCPUpstreams()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(upstreams) != 1 {
+		t.Fatalf("expected 1 upstream, got %d", len(upstreams))
+	}
+	u := upstreams[0]
+	wantArgs := []string{"-y", "@modelcontextprotocol/server-github"}
+	if len(u.Args) != len(wantArgs) {
+		t.Fatalf("args = %v, want %v", u.Args, wantArgs)
+	}
+	for i, a := range wantArgs {
+		if u.Args[i] != a {
+			t.Errorf("args[%d] = %q, want %q", i, u.Args[i], a)
+		}
+	}
+	if u.Env["GITHUB_PAT"] != "vault:github_pat" {
+		t.Errorf("env[GITHUB_PAT] = %q, want vault:github_pat", u.Env["GITHUB_PAT"])
+	}
+	if u.Env["DEBUG"] != "1" {
+		t.Errorf("env[DEBUG] = %q, want 1", u.Env["DEBUG"])
+	}
+	if u.TimeoutSec != 60 {
+		t.Errorf("timeout = %d, want 60", u.TimeoutSec)
+	}
+}
+
+func TestHandleMCPAddHTTPTransport(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{
+			"add", "notion",
+			"--command", "https://mcp.notion.com",
+			"--transport", "http",
+		},
+	})
+	if !strings.Contains(result, "Added MCP upstream") {
+		t.Fatalf("should confirm add, got: %s", result)
+	}
+
+	upstreams, err := s.ListMCPUpstreams()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(upstreams) != 1 {
+		t.Fatalf("expected 1 upstream, got %d", len(upstreams))
+	}
+	u := upstreams[0]
+	if u.Command != "https://mcp.notion.com" {
+		t.Errorf("command = %q, want URL", u.Command)
+	}
+	if u.Transport != "http" {
+		t.Errorf("transport = %q, want http", u.Transport)
+	}
+}
+
+func TestHandleMCPAddWebSocketTransport(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{
+			"add", "realtime",
+			"--command", "wss://mcp.example.com/ws",
+			"--transport", "websocket",
+		},
+	})
+	if !strings.Contains(result, "Added MCP upstream") {
+		t.Fatalf("should confirm add, got: %s", result)
+	}
+
+	upstreams, err := s.ListMCPUpstreams()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(upstreams) != 1 || upstreams[0].Transport != "websocket" {
+		t.Errorf("expected websocket upstream, got %+v", upstreams)
+	}
+}
+
+// TestHandleMCPAddEmptyArgs covers the bare "/mcp add" branch with no flags
+// or positional arguments. The handler should return the usage banner and not
+// mutate the store.
+func TestHandleMCPAddEmptyArgs(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	result := handler.Handle(&Command{Name: "mcp", Args: []string{"add"}})
+	if !strings.Contains(result, "Usage: /mcp add") {
+		t.Errorf("expected usage banner for bare /mcp add, got: %s", result)
+	}
+	if upstreams, _ := s.ListMCPUpstreams(); len(upstreams) != 0 {
+		t.Errorf("no upstream should be created for bare /mcp add, got %d", len(upstreams))
+	}
+}
+
+// TestHandleMCPAddEnvBase64Padding verifies that --env values containing "="
+// characters (e.g. base64-padded tokens) survive parsing intact. Regression
+// to strings.Split on "=" would truncate "abc===padding" to "abc".
+func TestHandleMCPAddEnvBase64Padding(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{
+			"add", "github", "--command", "npx",
+			"--env", "TOKEN=abc===padding",
+		},
+	})
+	if !strings.Contains(result, "Added MCP upstream") {
+		t.Fatalf("should confirm add, got: %s", result)
+	}
+
+	upstreams, err := s.ListMCPUpstreams()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(upstreams) != 1 {
+		t.Fatalf("expected 1 upstream, got %d", len(upstreams))
+	}
+	if got, want := upstreams[0].Env["TOKEN"], "abc===padding"; got != want {
+		t.Errorf("env[TOKEN] = %q, want %q (strings.SplitN on '=' must keep the RHS intact)", got, want)
+	}
+}
+
+// TestHandleMCPAddHTTPHeader verifies the --header flag is parsed for http
+// upstreams so Telegram matches the CLI's --header support. --header is
+// repeatable (matches CLI); pass the flag once per KEY=VAL pair.
+func TestHandleMCPAddHTTPHeader(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{
+			"add", "notion",
+			"--command", "https://mcp.notion.com",
+			"--transport", "http",
+			"--header", "Authorization=Bearer vault:notion",
+			"--header", "X-Custom=xyz",
+		},
+	})
+	if !strings.Contains(result, "Added MCP upstream") {
+		t.Fatalf("should confirm add, got: %s", result)
+	}
+
+	upstreams, err := s.ListMCPUpstreams()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(upstreams) != 1 {
+		t.Fatalf("expected 1 upstream, got %d", len(upstreams))
+	}
+	u := upstreams[0]
+	if u.Headers["Authorization"] != "Bearer vault:notion" {
+		t.Errorf("header[Authorization] = %q, want Bearer vault:notion", u.Headers["Authorization"])
+	}
+	if u.Headers["X-Custom"] != "xyz" {
+		t.Errorf("header[X-Custom] = %q, want xyz", u.Headers["X-Custom"])
+	}
+}
+
+// TestHandleMCPAddHeaderRejectedForStdio verifies --header is rejected for
+// non-http transports to match the CLI's behavior.
+func TestHandleMCPAddHeaderRejectedForStdio(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{
+			"add", "github", "--command", "npx",
+			"--header", "Authorization=Bearer xyz",
+		},
+	})
+	if !strings.Contains(result, "--header is only valid for --transport http") {
+		t.Errorf("expected --header validation error, got: %s", result)
+	}
+	if upstreams, _ := s.ListMCPUpstreams(); len(upstreams) != 0 {
+		t.Errorf("no upstream should be created when --header is misused, got %d", len(upstreams))
+	}
+}
+
+// TestHandleMCPAddDuplicateDoesNotCallContainerManager ensures error-path
+// additions do not invoke WireMCPGateway. This guards against a regression
+// that reintroduces pre-validation container side effects.
+func TestHandleMCPAddDuplicateDoesNotCallContainerManager(t *testing.T) {
+	s := newTestStore(t)
+	if _, err := s.AddMCPUpstream("github", "npx", store.MCPUpstreamOpts{}); err != nil {
+		t.Fatal(err)
+	}
+	handler := newTestHandlerWithStore(t, s, nil, "")
+	mgr := &mockContainerMgr{}
+	handler.SetContainerManager(mgr)
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{"add", "github", "--command", "some-other"},
+	})
+	if !strings.Contains(result, "Failed to add MCP upstream") {
+		t.Errorf("expected duplicate rejection, got: %s", result)
+	}
+	if mgr.wireCalledSafe() {
+		t.Errorf("WireMCPGateway must not be called on the add error path")
+	}
+}
+
+// TestHandleMCPRemoveNotFoundDoesNotCallContainerManager mirrors the add
+// case for the remove error path.
+func TestHandleMCPRemoveNotFoundDoesNotCallContainerManager(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+	mgr := &mockContainerMgr{}
+	handler.SetContainerManager(mgr)
+
+	result := handler.Handle(&Command{Name: "mcp", Args: []string{"remove", "nonexistent"}})
+	if !strings.Contains(result, "No MCP upstream named") {
+		t.Errorf("expected not-found message, got: %s", result)
+	}
+	if mgr.wireCalledSafe() {
+		t.Errorf("WireMCPGateway must not be called on the remove error path")
+	}
+}
+
+func TestHandleMCPAddMissingCommand(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{"add", "github"},
+	})
+	if !strings.Contains(result, "Usage:") {
+		t.Errorf("expected usage on missing --command, got: %s", result)
+	}
+	upstreams, _ := s.ListMCPUpstreams()
+	if len(upstreams) != 0 {
+		t.Errorf("no upstream should be created when --command is missing, got %d", len(upstreams))
+	}
+}
+
+func TestHandleMCPAddMissingName(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{"add", "--command", "npx"},
+	})
+	if !strings.Contains(result, "Usage:") {
+		t.Errorf("expected usage on missing name, got: %s", result)
+	}
+	upstreams, _ := s.ListMCPUpstreams()
+	if len(upstreams) != 0 {
+		t.Errorf("no upstream should be created without a name, got %d", len(upstreams))
+	}
+}
+
+func TestHandleMCPAddInvalidName(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	// "__" is a reserved namespace separator for the MCP gateway.
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{"add", "bad__name", "--command", "npx"},
+	})
+	if !strings.Contains(result, "Invalid upstream name") {
+		t.Errorf("expected invalid name error, got: %s", result)
+	}
+}
+
+func TestHandleMCPAddInvalidTransport(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{"add", "github", "--command", "npx", "--transport", "ftp"},
+	})
+	if !strings.Contains(result, "Invalid transport") {
+		t.Errorf("expected invalid transport error, got: %s", result)
+	}
+}
+
+func TestHandleMCPAddInvalidTimeout(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	// non-numeric surfaces stdlib flag.Parse error via our
+	// "Invalid argument:" wrapper.
+	if r := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{"add", "github", "--command", "npx", "--timeout", "abc"},
+	}); !strings.Contains(r, "Invalid argument") || !strings.Contains(r, "timeout") {
+		t.Errorf("expected invalid timeout error, got: %s", r)
+	}
+	// zero and negative hit our own validation after flag parsing.
+	for _, tv := range []string{"0", "-5"} {
+		if r := handler.Handle(&Command{
+			Name: "mcp",
+			Args: []string{"add", "github", "--command", "npx", "--timeout", tv},
+		}); !strings.Contains(r, "Invalid --timeout") {
+			t.Errorf("expected invalid timeout error for %s, got: %s", tv, r)
+		}
+	}
+}
+
+func TestHandleMCPAddInvalidEnv(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	// env value missing "="
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{"add", "github", "--command", "npx", "--env", "NOT_A_PAIR"},
+	})
+	if !strings.Contains(result, "Invalid --env") {
+		t.Errorf("expected invalid env error, got: %s", result)
+	}
+}
+
+// TestHandleMCPAddInvalidHeader is the --header analog of
+// TestHandleMCPAddInvalidEnv. The fs.Func callback for --header must
+// reject a BADFORMAT token (no "=") with a user-visible error that
+// surfaces through the "Invalid argument:" prefix produced by the
+// stdlib flag.Parse wrapper.
+func TestHandleMCPAddInvalidHeader(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{
+			"add", "github",
+			"--command", "https://api.example.com/mcp",
+			"--transport", "http",
+			"--header", "BADFORMAT",
+		},
+	})
+	if !strings.Contains(result, "Invalid argument") || !strings.Contains(result, "--header") {
+		t.Errorf("expected invalid header error mentioning --header, got: %s", result)
+	}
+	ups, _ := s.ListMCPUpstreams()
+	if len(ups) != 0 {
+		t.Errorf("no upstream should be created on parse failure, got %d", len(ups))
+	}
+}
+
+func TestHandleMCPAddDuplicateName(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	if _, err := s.AddMCPUpstream("github", "npx", store.MCPUpstreamOpts{}); err != nil {
+		t.Fatal(err)
+	}
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{"add", "github", "--command", "some-other"},
+	})
+	if !strings.Contains(result, "Failed to add MCP upstream") {
+		t.Errorf("expected duplicate rejection, got: %s", result)
+	}
+}
+
+func TestHandleMCPAddStrayPositional(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	// Second positional arg should be rejected to avoid silently swallowing
+	// the intended upstream name when someone types /mcp add foo bar --command cmd.
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{"add", "foo", "bar", "--command", "npx"},
+	})
+	if !strings.Contains(result, "Unexpected argument") {
+		t.Errorf("expected rejection of stray arg, got: %s", result)
+	}
+	upstreams, _ := s.ListMCPUpstreams()
+	if len(upstreams) != 0 {
+		t.Errorf("no upstream should be created on parse failure, got %d", len(upstreams))
+	}
+}
+
 func TestCredAddEnvVarConsumedFromValue(t *testing.T) {
 	s := newTestStore(t)
 	handler := newTestHandlerWithStore(t, s, nil, "")
@@ -822,3 +1553,240 @@ func TestCredAddEnvVarConsumedFromValue(t *testing.T) {
 		t.Errorf("expected credential value 'the-secret-value', got %q", string(sb.Bytes()))
 	}
 }
+
+func TestHandleMCPRemove(t *testing.T) {
+	s := newTestStore(t)
+	if _, err := s.AddMCPUpstream("github", "npx", store.MCPUpstreamOpts{
+		Transport: "stdio",
+	}); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := s.AddMCPUpstream("notion", "https://mcp.notion.com", store.MCPUpstreamOpts{
+		Transport: "http",
+	}); err != nil {
+		t.Fatal(err)
+	}
+
+	handler := newTestHandlerWithStore(t, s, nil, "")
+	result := handler.Handle(&Command{Name: "mcp", Args: []string{"remove", "github"}})
+
+	if !strings.Contains(result, "Removed MCP upstream") {
+		t.Errorf("expected removal confirmation, got: %s", result)
+	}
+	if !strings.Contains(result, "github") {
+		t.Errorf("expected removed name in response, got: %s", result)
+	}
+	if !strings.Contains(result, "Restart sluice") {
+		t.Errorf("expected restart notice, got: %s", result)
+	}
+
+	// Verify github was removed but notion remains.
+	upstreams, err := s.ListMCPUpstreams()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(upstreams) != 1 {
+		t.Fatalf("expected 1 remaining upstream, got %d", len(upstreams))
+	}
+	if upstreams[0].Name != "notion" {
+		t.Errorf("wrong upstream remained: %q", upstreams[0].Name)
+	}
+}
+
+func TestHandleMCPRemoveMissingName(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+	result := handler.Handle(&Command{Name: "mcp", Args: []string{"remove"}})
+
+	if !strings.Contains(result, "Usage: /mcp remove") {
+		t.Errorf("expected usage on missing name, got: %s", result)
+	}
+}
+
+func TestHandleMCPRemoveNotFound(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+	result := handler.Handle(&Command{Name: "mcp", Args: []string{"remove", "nonexistent"}})
+
+	if !strings.Contains(result, "No MCP upstream named") {
+		t.Errorf("expected not-found message, got: %s", result)
+	}
+}
+
+func TestHandleMCPRemoveStrayPositional(t *testing.T) {
+	s := newTestStore(t)
+	if _, err := s.AddMCPUpstream("github", "npx", store.MCPUpstreamOpts{Transport: "stdio"}); err != nil {
+		t.Fatal(err)
+	}
+
+	handler := newTestHandlerWithStore(t, s, nil, "")
+	result := handler.Handle(&Command{Name: "mcp", Args: []string{"remove", "github", "extra"}})
+
+	if !strings.Contains(result, "Unexpected argument") {
+		t.Errorf("expected stray arg rejection, got: %s", result)
+	}
+
+	// No-op removal: github must still exist.
+	upstreams, _ := s.ListMCPUpstreams()
+	if len(upstreams) != 1 {
+		t.Errorf("upstream should not be removed on parse failure, got %d", len(upstreams))
+	}
+}
+
+// TestHandleMCPAddDoesNotCallContainerManager verifies that /mcp add does NOT
+// invoke the ContainerManager, because sluice multiplexes all upstreams via a
+// single agent-side entry (mcp.servers.sluice) that is wired once at startup.
+// Re-invoking WireMCPGateway on every mutation would trigger an agent gateway
+// restart without changing anything meaningful. The operator-facing message
+// instructs them to restart sluice so the gateway re-reads the upstream set.
+func TestHandleMCPAddDoesNotCallContainerManager(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	mgr := &mockContainerMgr{}
+	handler.SetContainerManager(mgr)
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{"add", "github", "--command", "npx"},
+	})
+	if !strings.Contains(result, "Added MCP upstream") {
+		t.Fatalf("should confirm add, got: %s", result)
+	}
+	if !strings.Contains(result, "Restart sluice") {
+		t.Errorf("response should instruct operator to restart sluice, got: %s", result)
+	}
+	if mgr.wireCalledSafe() {
+		t.Errorf("WireMCPGateway must not be called on /mcp add (sluice URL is unchanged)")
+	}
+}
+
+// TestHandleMCPRemoveDoesNotCallContainerManager mirrors the add case: the
+// removal only takes effect after a sluice restart, and the agent's openclaw
+// config is not touched.
+func TestHandleMCPRemoveDoesNotCallContainerManager(t *testing.T) {
+	s := newTestStore(t)
+	if _, err := s.AddMCPUpstream("github", "npx", store.MCPUpstreamOpts{Transport: "stdio"}); err != nil {
+		t.Fatal(err)
+	}
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	mgr := &mockContainerMgr{}
+	handler.SetContainerManager(mgr)
+
+	result := handler.Handle(&Command{Name: "mcp", Args: []string{"remove", "github"}})
+	if !strings.Contains(result, "Removed MCP upstream") {
+		t.Fatalf("should confirm remove, got: %s", result)
+	}
+	if !strings.Contains(result, "Restart sluice") {
+		t.Errorf("response should instruct operator to restart sluice, got: %s", result)
+	}
+	if mgr.wireCalledSafe() {
+		t.Errorf("WireMCPGateway must not be called on /mcp remove")
+	}
+}
+
+// TestHandleMCPAddEqualsFormFlag verifies that the single-token "--flag=value"
+// form is accepted alongside the two-token form. Without this, operators
+// typing `/mcp add github --command=npx` would hit the reorderer's two-token
+// assumption.
+func TestHandleMCPAddEqualsFormFlag(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{"add", "github", "--command=npx", "--transport=stdio", "--timeout=45"},
+	})
+	if !strings.Contains(result, "Added MCP upstream") {
+		t.Fatalf("should confirm add, got: %s", result)
+	}
+	ups, _ := s.ListMCPUpstreams()
+	if len(ups) != 1 || ups[0].Command != "npx" || ups[0].Transport != "stdio" || ups[0].TimeoutSec != 45 {
+		t.Errorf("unexpected upstream state: %+v", ups)
+	}
+}
+
+// TestHandleMCPListErrorPath exercises the ListMCPUpstreams error branch by
+// closing the store before the handler is invoked.
+func TestHandleMCPListErrorPath(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+	if err := s.Close(); err != nil {
+		t.Fatal(err)
+	}
+	result := handler.Handle(&Command{Name: "mcp", Args: []string{"list"}})
+	if !strings.Contains(result, "Failed to list MCP upstreams") {
+		t.Errorf("expected list error, got: %s", result)
+	}
+}
+
+// TestHandleMCPAddErrorPath exercises the AddMCPUpstream error branch by
+// closing the store before the handler is invoked. The user-facing message
+// must start with the generic "Failed to add" prefix, not a panic.
+func TestHandleMCPAddErrorPath(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+	if err := s.Close(); err != nil {
+		t.Fatal(err)
+	}
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{"add", "github", "--command", "npx"},
+	})
+	if !strings.Contains(result, "Failed to add MCP upstream") {
+		t.Errorf("expected add error, got: %s", result)
+	}
+}
+
+// TestHandleMCPRemoveErrorPath exercises the RemoveMCPUpstream error branch
+// by closing the store before the handler is invoked.
+func TestHandleMCPRemoveErrorPath(t *testing.T) {
+	s := newTestStore(t)
+	// Seed first so the later close-and-remove hits the DB rather than the
+	// upfront "not found" check.
+	if _, err := s.AddMCPUpstream("github", "npx", store.MCPUpstreamOpts{}); err != nil {
+		t.Fatal(err)
+	}
+	handler := newTestHandlerWithStore(t, s, nil, "")
+	if err := s.Close(); err != nil {
+		t.Fatal(err)
+	}
+	result := handler.Handle(&Command{Name: "mcp", Args: []string{"remove", "github"}})
+	if !strings.Contains(result, "Failed to remove MCP upstream") {
+		t.Errorf("expected remove error, got: %s", result)
+	}
+}
+
+// TestHandleMCPAddRepeatableHeader verifies --header can be passed multiple
+// times (matching the CLI). This is the preferred form: repeatable flags
+// keep header values that contain commas intact, which a CSV form would
+// silently split.
+func TestHandleMCPAddRepeatableHeader(t *testing.T) {
+	s := newTestStore(t)
+	handler := newTestHandlerWithStore(t, s, nil, "")
+
+	result := handler.Handle(&Command{
+		Name: "mcp",
+		Args: []string{
+			"add", "notion",
+			"--command", "https://mcp.notion.com",
+			"--transport", "http",
+			"--header", "Authorization=Bearer xyz",
+			"--header", "X-Custom=a,b,c",
+		},
+	})
+	if !strings.Contains(result, "Added MCP upstream") {
+		t.Fatalf("should confirm add, got: %s", result)
+	}
+	ups, _ := s.ListMCPUpstreams()
+	if len(ups) != 1 {
+		t.Fatalf("expected 1 upstream, got %d", len(ups))
+	}
+	if ups[0].Headers["Authorization"] != "Bearer xyz" {
+		t.Errorf("unexpected Authorization header: %q", ups[0].Headers["Authorization"])
+	}
+	if ups[0].Headers["X-Custom"] != "a,b,c" {
+		t.Errorf("repeatable --header must keep commas intact: %q", ups[0].Headers["X-Custom"])
+	}
+}