feat(mcp): approve multiple tools in one invocation

CHANGELOG.md

@@ -57,6 +57,14 @@ to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Changed
 
+- **`c1i mcp tools approve` now takes one or more tool ids.** Approving a
+  toolset meant one invocation — a fresh process, token and TLS handshake —
+  per tool; `approve id1 id2 id3` now does them in a single process. The API
+  has no batch approve, so each id is still its own request (one confirmation
+  line each, failures don't stop the rest, non-zero exit if any fail), but the
+  round-trip and startup cost drops sharply. Backward compatible: a single id
+  behaves exactly as before.
+
 - **Fixtures and command documentation now use placeholder identifiers**, and
   a test keeps them that way: it rejects tenant-copied object ids, tenant
   hostnames, and prose naming the tenant an observation came from.

CLAUDE.md

@@ -95,7 +95,11 @@ tracked root file outside its allowlist. Stage explicit paths rather than `-A`.
     `deny`, `comment`, `set-owner`, `resync-tools`, `source`, `usage`, …):
     the resource's own id is positional (`Use: "get <thing-id>"`,
     `cobra.ExactArgs(1)`, read via `args[0]`); parent ids stay flags
-    (`--app-id`, `--connector-id` when it is a *parent*). Don't validate the
+    (`--app-id`, `--connector-id` when it is a *parent*). An action that
+    applies the *same* mutation to several instances pluralizes that
+    positional rather than moving to a flag (`mcp tools approve <tool-id>...`,
+    `cobra.MinimumNArgs(1)`, loop over `args`) — the id stays positional,
+    scope ids stay flags. Don't validate the
     positional id with `requireNonEmpty` — `cobra.ExactArgs` enforces presence;
     match the flat commands (`users get <user-id>`). **Presence is not
     non-emptiness:** `ExactArgs(1)` accepts `""`, `"/"`, or `"."`, and each

README.md

@@ -366,7 +366,7 @@ c1i mcp servers connections list   [--page-size N] [--limit N]
 c1i mcp tools list    --app-id <id> --connector-id <id> [--page-size N] [--page-token TOKEN] [--limit N]
 c1i mcp tools get     <tool-id> --app-id <id> --connector-id <id>
 c1i mcp tools search  --app-id <id> --connector-id <id> [--query <text>] [--state ...] [--classification ...] [--page-size N] [--limit N]
-c1i mcp tools approve <tool-id> --app-id <id> --connector-id <id> [--state approved|disabled|pending]
+c1i mcp tools approve <tool-id>... --app-id <id> --connector-id <id> [--state approved|disabled|pending]
 c1i mcp tools delete  <tool-id> --app-id <id> --connector-id <id>
 c1i mcp tools history <tool-id> --app-id <id> --connector-id <id> [--page-size N] [--limit N]
 
@@ -393,7 +393,7 @@ c1i mcp gateway call <tool-name> [--args '{"k":"v"}'] [--gateway-url <url>]
 
 **Auth for `register` / `update-credentials`:** convenience flags cover the simple methods — `--auth none`, `--auth bearer-token --bearer-token TOKEN`, `--auth custom-header --header-name NAME --header-value VALUE`, `--auth basic-auth --basic-auth-username USER --basic-auth-password PASS`. For OAuth2 / AWS SigV4 / Google service-account auth, pass the full config object via `--hosted-config-file` / `--external-config-file` (JSON file, or `-` for stdin) — generate a ready-to-edit skeleton with `--print-config-template --auth <method> [--type hosted]` instead of hand-writing it. Secrets are sealed server-side; reads only ever return `*_configured` booleans, never the values. `--token-sharing shared|per-user` sets the server's token-sharing mode (case-insensitive; `per_user`/`peruser` are also accepted). Per the register help, `per-user` is only valid with `oauth2` in authorization-code or passthrough mode, `bearerToken`, `customHeader`, or `basicAuth`. Note that a read-back can legitimately differ from what you sent: the backend may store a *resolved* OAuth2 grant such as `..._MODE_AUTHORIZATION_CODE` in place of the input mode, so that is a normal round-trip, not a bug. `--source-app-id` names the source app for a connector-backed HOSTED server.
 
-`mcp tools approve` is the standard post-registration step: newly discovered tools (from `register` or `resync-tools`) start in `PENDING_REVIEW`, and an admin approves each one for the gateway to proxy calls. History endpoints return records newest-first.
+`mcp tools approve` is the standard post-registration step: newly discovered tools (from `register` or `resync-tools`) start in `PENDING_REVIEW`, and an admin approves them for the gateway to proxy calls. It takes one or more tool ids — the API has no batch approve, so each id is a separate request, but one invocation covers a whole toolset (pipe `mcp tools search --app-id <id> --connector-id <id> --state pending --fields id | jq -r .id`). History endpoints return records newest-first.
 
 **`mcp gateway`** closes the configure-then-verify loop: after registering a server and approving its tools, `list-tools` runs the MCP handshake against the live gateway and shows what's actually callable, and `call` invokes a tool and prints its result. The gateway URL defaults to the `-mcp` host derived from `--url` (e.g. `acme.conductor.one` → `acme-mcp.conductor.one/v1`); override with `--gateway-url`. Your standard C1 token is accepted by the gateway, so no extra auth is needed. `call` always prints the full result, but exits `7` (not `0`) when the result itself reports `isError: true` — the call succeeded, the tool didn't.
 

cmd/args_positional_test.go

@@ -270,7 +270,9 @@ func TestMigratedSingleResourceCommandsUsePositionalID(t *testing.T) {
 		{path: []string{"mcp", "servers", "catalog", "get"}, retiredFlag: "catalog-id"},
 
 		{path: []string{"mcp", "tools", "get"}, retiredFlag: "id", mustKeepFlag: []string{"app-id", "connector-id"}},
-		{path: []string{"mcp", "tools", "approve"}, retiredFlag: "id", mustKeepFlag: []string{"app-id", "connector-id"}},
+		// "approve" is deliberately NOT here: it takes one OR more ids (batch
+		// approval), so it accepts a 2nd positional. Its contract is pinned by
+		// TestMcpToolsApproveIsMultiIDPositional instead.
 		{path: []string{"mcp", "tools", "delete"}, retiredFlag: "id", mustKeepFlag: []string{"app-id", "connector-id"}},
 		{path: []string{"mcp", "tools", "history"}, retiredFlag: "id", mustKeepFlag: []string{"app-id", "connector-id"}},
 
@@ -336,6 +338,37 @@ func TestMigratedSingleResourceCommandsUsePositionalID(t *testing.T) {
 	}
 }
 
+// TestMcpToolsApproveIsMultiIDPositional pins C146: approve is the one
+// migrated action that takes MORE than one id (batch approval), so unlike its
+// single-resource siblings it accepts a 2nd positional. The rest of the
+// migration contract still holds: the id stays positional (no --id flag) and
+// the scope ids stay flags.
+func TestMcpToolsApproveIsMultiIDPositional(t *testing.T) {
+	cmd := findCommand(t, rootCmd, "mcp", "tools", "approve")
+
+	spec, err := parseUsePositionals(cmd.Use)
+	if err != nil {
+		t.Fatalf("%v", err)
+	}
+	if spec.required != 1 || spec.optional != 0 {
+		t.Errorf("Use %q should document a required positional, got required=%d optional=%d", cmd.Use, spec.required, spec.optional)
+	}
+	if argsAccepts(cmd, 0) {
+		t.Error("approve accepted 0 args; at least one tool id is required")
+	}
+	if !argsAccepts(cmd, 1) || !argsAccepts(cmd, 2) {
+		t.Error("approve must accept one OR more tool ids (batch approval)")
+	}
+	if hasFlag(cmd, "id") {
+		t.Error("the tool id must be positional-only, not an --id flag")
+	}
+	for _, f := range []string{"app-id", "connector-id"} {
+		if !hasFlag(cmd, f) {
+			t.Errorf("scope flag --%s must remain registered", f)
+		}
+	}
+}
+
 // TestCollectionCommandsKeepFlagIDsAndNoPositional guards against
 // over-correction: collection/create/relationship commands were NOT part of
 // the id->positional migration (they don't address one existing resource by

cmd/mcp_tools_approve.go

@@ -9,19 +9,28 @@ import (
 )
 
 var mcpToolsApproveCmd = &cobra.Command{
-	Use:   "approve <tool-id>",
-	Short: "Approve an MCP tool (sets state=APPROVED)",
-	Long: `Approve an MCP tool by setting its state to MCP_TOOL_STATE_APPROVED.
+	Use:   "approve <tool-id>...",
+	Short: "Approve one or more MCP tools (sets state=APPROVED)",
+	Long: `Approve MCP tools by setting each tool's state to MCP_TOOL_STATE_APPROVED.
 
 This is the standard tool-approval workflow: newly discovered tools land in
 MCP_TOOL_STATE_PENDING_REVIEW and admin approval moves them to APPROVED so the
 MCP gateway will proxy calls to them.
 
+Pass one or more tool ids. The API has no batch approve, so each id is sent as
+its own request and prints its own confirmation line; if one fails the rest are
+still attempted and the command exits non-zero. All ids share --app-id,
+--connector-id and --state, so a selector pipes straight in:
+
+  c1i mcp tools approve $(c1i mcp tools search --app-id A --connector-id C \
+    --classification read --state pending --fields id | jq -r .id) \
+    --app-id A --connector-id C
+
 Use --state to target a different lifecycle state (disabled, pending). The
 approve command is a thin wrapper around the underlying Update RPC; use
 "c1i api --path=..." directly if you need to update other fields (display name,
 classification, visibility).`,
-	Args: cobra.ExactArgs(1),
+	Args: cobra.MinimumNArgs(1),
 	RunE: func(cmd *cobra.Command, args []string) error {
 		baseURL, err := GetBaseURL()
 		if err != nil {
@@ -30,7 +39,6 @@ classification, visibility).`,
 
 		appID, _ := cmd.Flags().GetString("app-id")
 		connectorID, _ := cmd.Flags().GetString("connector-id")
-		id := args[0]
 		state, _ := cmd.Flags().GetString("state")
 		if state == "" {
 			state = "MCP_TOOL_STATE_APPROVED"
@@ -44,41 +52,77 @@ classification, visibility).`,
 			return &usageError{fmt.Errorf("--state=removed is system-managed by sync; use \"mcp tools delete\" to soft-delete a tool, or --state=disabled to block it")}
 		}
 
-		body := map[string]any{
-			"tool": map[string]any{
-				"id":          id,
-				"appId":       appID,
-				"connectorId": connectorID,
-				"state":       state,
-			},
-			"updateMask": "state",
+		pathFor := func(id string) string {
+			return client.Path("/api/v1/apps/%s/connectors/%s/mcp_tools/%s", appID, connectorID, id)
+		}
+		bodyFor := func(id string) map[string]any {
+			return map[string]any{
+				"tool": map[string]any{
+					"id":          id,
+					"appId":       appID,
+					"connectorId": connectorID,
+					"state":       state,
+				},
+				"updateMask": "state",
+			}
 		}
 
-		path := client.Path("/api/v1/apps/%s/connectors/%s/mcp_tools/%s", appID, connectorID, id)
 		if dryRunActive() {
-			return printDryRun(cmd, "POST", path, body)
+			for _, id := range args {
+				if err := printDryRun(cmd, "POST", pathFor(id), bodyFor(id)); err != nil {
+					return err
+				}
+			}
+			return nil
 		}
 
 		c, err := newClient(cmd, baseURL)
 		if err != nil {
 			return fmt.Errorf("authentication failed: %w", err)
 		}
-		data, err := c.Post(cmd.Context(), path, body)
-		if err != nil {
-			return fmt.Errorf("API error: %w", err)
-		}
 
-		var resp struct {
-			Tool struct {
-				ID    string `json:"id"`
-				State string `json:"state"`
-			} `json:"tool"`
+		out := cmd.OutOrStdout()
+		approveOne := func(id string) error {
+			data, err := c.Post(cmd.Context(), pathFor(id), bodyFor(id))
+			if err != nil {
+				return fmt.Errorf("API error: %w", err)
+			}
+			var resp struct {
+				Tool struct {
+					ID    string `json:"id"`
+					State string `json:"state"`
+				} `json:"tool"`
+			}
+			if err := json.Unmarshal(data, &resp); err != nil {
+				return fmt.Errorf("failed to parse response: %w", err)
+			}
+			_, _ = fmt.Fprintf(out, "Updated tool: id=%s state=%s\n", resp.Tool.ID, resp.Tool.State)
+			return nil
 		}
-		if err := json.Unmarshal(data, &resp); err != nil {
-			return fmt.Errorf("failed to parse response: %w", err)
+
+		var firstErr error
+		failed := 0
+		for _, id := range args {
+			if err := approveOne(id); err != nil {
+				failed++
+				if firstErr == nil {
+					firstErr = err
+				}
+				// One line per id only when batching; a single id's error is
+				// the returned one, so printing it here too would double it.
+				if len(args) > 1 {
+					_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "tool %s: %v\n", id, err)
+				}
+			}
 		}
 
-		_, _ = fmt.Fprintf(cmd.OutOrStdout(), "Updated tool: id=%s state=%s\n", resp.Tool.ID, resp.Tool.State)
+		if firstErr != nil {
+			if len(args) == 1 {
+				return firstErr
+			}
+			// Wrap the first failure so exit-code classification still fires.
+			return fmt.Errorf("approved %d of %d tools; %d failed: %w", len(args)-failed, len(args), failed, firstErr)
+		}
 		return nil
 	},
 }

cmd/mcp_tools_approve_batch_test.go

@@ -0,0 +1,46 @@
+package cmd
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+)
+
+// TestMcpToolsApproveBatchesOnePostPerId pins C146: `approve` takes one or
+// more tool ids and sends one request per id (the API has no batch approve).
+// Before the fix, ExactArgs(1) rejected a second id outright.
+func TestMcpToolsApproveBatchesOnePostPerId(t *testing.T) {
+	resetRootURLFlag(t)
+	resetRootDryRunFlag(t)
+	// approve's own --app-id/--connector-id live on the shared command object;
+	// clear them before and after so this test neither inherits nor leaks a
+	// Changed flag (the C182 order-dependence class).
+	resetCmds(t, mcpToolsApproveCmd)
+	t.Cleanup(func() { resetCmds(t, mcpToolsApproveCmd) })
+	t.Setenv("C1I_URL", "")
+	withDryRun(t)
+
+	var out bytes.Buffer
+	rootCmd.SetOut(&out)
+	rootCmd.SetErr(&out)
+	t.Cleanup(func() { rootCmd.SetOut(nil); rootCmd.SetErr(nil) })
+	rootCmd.SetArgs([]string{
+		"mcp", "tools", "approve", "tool-a", "tool-b", "tool-c",
+		"--app-id", "app-x", "--connector-id", "conn-y",
+		"--dry-run", "--url", "acme.conductor.one",
+	})
+	if err := rootCmd.ExecuteContext(t.Context()); err != nil {
+		t.Fatalf("approve of three ids errored: %v", err)
+	}
+
+	got := out.String()
+	for _, id := range []string{"tool-a", "tool-b", "tool-c"} {
+		want := "[dry-run] POST /api/v1/apps/app-x/connectors/conn-y/mcp_tools/" + id
+		if !strings.Contains(got, want) {
+			t.Errorf("missing preview for %s;\nwant substring %q\ngot:\n%s", id, want, got)
+		}
+	}
+	if n := strings.Count(got, "[dry-run] POST"); n != 3 {
+		t.Errorf("got %d POST previews, want 3 (one per id)", n)
+	}
+}