chore: rollup of #2568 CLI sub-issue PRs (not for merge)

clients/cli/__tests__/README.md

@@ -24,33 +24,35 @@ npm run validate          # format:check && lint && typecheck && test
 
 ## Test files
 
-| File                                    | Focus                                       |
-| --------------------------------------- | ------------------------------------------- |
-| `cli.test.ts`                           | Core connect / method / config matrix       |
-| `tools.test.ts`                         | `tools/call` argument coercion              |
-| `headers.test.ts`                       | `--header` merging                          |
-| `metadata.test.ts`                      | `--metadata` / `--tool-metadata`            |
-| `methods.test.ts`                       | Method allow-list / rejection               |
-| `app-info.test.ts`                      | `--app-info` probe paths                    |
-| `format-json.test.ts`                   | `--format json` envelopes                   |
-| `format-output.test.ts`                 | Text/json writers                           |
-| `emit-result.test.ts`                   | Result emission helpers                     |
-| `method-types.test.ts`                  | `ONE_SHOT_METHODS` / guards                 |
-| `run-method.test.ts`                    | Handler dispatch against a real test server |
-| `run-method-mocks.test.ts`              | Handler edge cases with mocks               |
-| `servers-list.test.ts`                  | `servers/list` / `servers/show` + redaction |
-| `cliOAuth.test.ts`                      | Connect / mid-RPC OAuth recovery            |
-| `cli-oauth-navigation.test.ts`          | OSC 8, arm/disarm, `MCP_AUTO_OPEN_ENABLED`  |
-| `open-url.test.ts`                      | `open` package wrapper                      |
-| `oauth-runner.test.ts`                  | Runner client-config / CIMD flags           |
-| `oauth-interactive.test.ts`             | Loopback callback OAuth (in-process)        |
-| `stored-auth.test.ts`                   | `--use-stored-auth` / handoff / wait        |
-| `clear-stored-auth-for-relogin.test.ts` | Dual-key `--relogin` clear                  |
-| `programmatic-ergonomics.test.ts`       | Flag conflicts, timeouts, ergonomics        |
-| `error-handler.test.ts`                 | Exit codes / error shaping                  |
-| `style.test.ts`                         | ANSI / OSC 8 helpers                        |
-| `e2e.test.ts`                           | Out-of-process binary smoke                 |
-| `helpers/assertions.test.ts`            | Assertion helpers                           |
+| File                                    | Focus                                                                                   |
+| --------------------------------------- | --------------------------------------------------------------------------------------- |
+| `cli.test.ts`                           | Core connect / method / config matrix                                                   |
+| `tools.test.ts`                         | `tools/call` argument coercion                                                          |
+| `headers.test.ts`                       | `--header` merging                                                                      |
+| `metadata.test.ts`                      | `--metadata` / `--tool-metadata`                                                        |
+| `methods.test.ts`                       | Method allow-list / rejection                                                           |
+| `app-info.test.ts`                      | `--app-info` probe paths                                                                |
+| `format-json.test.ts`                   | `--format json` envelopes                                                               |
+| `format-output.test.ts`                 | Text/json writers                                                                       |
+| `emit-result.test.ts`                   | Result emission helpers                                                                 |
+| `output-file.test.ts`                   | `--output` / `--output-format` (#2431)                                                  |
+| `method-types.test.ts`                  | `ONE_SHOT_METHODS` / guards                                                             |
+| `run-method.test.ts`                    | Handler dispatch against a real test server                                             |
+| `run-method-mocks.test.ts`              | Handler edge cases with mocks                                                           |
+| `servers-list.test.ts`                  | `servers/list` / `servers/show` + redaction                                             |
+| `servers-write.test.ts`                 | `servers/add` / `servers/edit` / `servers/remove` against a temp catalog + secret store |
+| `cliOAuth.test.ts`                      | Connect / mid-RPC OAuth recovery                                                        |
+| `cli-oauth-navigation.test.ts`          | OSC 8, arm/disarm, `MCP_AUTO_OPEN_ENABLED`                                              |
+| `open-url.test.ts`                      | `open` package wrapper                                                                  |
+| `oauth-runner.test.ts`                  | Runner client-config / CIMD flags                                                       |
+| `oauth-interactive.test.ts`             | Loopback callback OAuth (in-process)                                                    |
+| `stored-auth.test.ts`                   | `--use-stored-auth` / handoff / wait                                                    |
+| `clear-stored-auth-for-relogin.test.ts` | Dual-key `--relogin` clear                                                              |
+| `programmatic-ergonomics.test.ts`       | Flag conflicts, timeouts, ergonomics                                                    |
+| `error-handler.test.ts`                 | Exit codes / error shaping                                                              |
+| `style.test.ts`                         | ANSI / OSC 8 helpers                                                                    |
+| `e2e.test.ts`                           | Out-of-process binary smoke                                                             |
+| `helpers/assertions.test.ts`            | Assertion helpers                                                                       |
 
 ## Helpers
 

clients/cli/__tests__/cli.test.ts

@@ -1,5 +1,5 @@
 import { afterAll, beforeAll, describe, it, expect, vi } from "vitest";
-import { mkdtempSync } from "node:fs";
+import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { runCli } from "./helpers/cli-runner.js";
@@ -85,6 +85,109 @@ describe("CLI Tests", () => {
       expect(toolNames).toContain("get_annotated_message");
     });
 
+    // #2435. In-process, a stdio server's own stderr goes straight to the
+    // worker's fd 2 rather than through the captured `process.stderr.write`,
+    // so the child-stderr half is asserted out of process in e2e.test.ts.
+    it.each(["-q", "--quiet"])(
+      "%s prints only the result payload",
+      async (flag) => {
+        const { command, args } = getTestMcpServerCommand();
+        const result = await runCli([
+          command,
+          ...args,
+          flag,
+          "--method",
+          "tools/list",
+        ]);
+
+        expectCliSuccess(result);
+        expect(expectValidJson(result)).toHaveProperty("tools");
+        expect(result.stderr).toBe("");
+      },
+    );
+
+    // A usage error makes Commander print its own `error: …` line during
+    // `parse()`, before the envelope. Under `--quiet` that line is dropped and
+    // stderr is the envelope alone (Copilot on #2576).
+    it("drops Commander's usage diagnostic under --quiet, keeping only the envelope", async () => {
+      const loud = await runCli([NO_SERVER_SENTINEL, "--method"]);
+      expectCliFailure(loud);
+      expect(loud.stderr).toMatch(/^error: option '--method <method>'/);
+
+      const quiet = await runCli([NO_SERVER_SENTINEL, "-q", "--method"]);
+      expectCliFailure(quiet);
+      const lines = quiet.stderr.trimEnd().split("\n");
+      expect(lines).toHaveLength(1);
+      expect(JSON.parse(lines[0]!)).toMatchObject({
+        error: { message: expect.stringMatching(/--method <method>/) },
+      });
+    });
+
+    // Core reports advisories with `console.warn` from many places — here
+    // `cleanRoots()` on a hand-edited entry. Vitest replaces `console`, so the
+    // captured stderr cannot show it; the spy is what observes the channel.
+    // Under `--quiet` the run swaps `console.warn` out and restores it after
+    // (Copilot on #2576).
+    it("mutes core's console.warn advisories for a --quiet run, and restores console.warn after", async () => {
+      const { command, args } = getTestMcpServerCommand();
+      const dir = mkdtempSync(join(tmpdir(), "cli-quiet-roots-"));
+      const configPath = join(dir, "mcp.json");
+      // A root with no `uri` is dropped with a warning (core/mcp/serverList.ts).
+      writeFileSync(
+        configPath,
+        JSON.stringify({
+          mcpServers: {
+            s: { type: "stdio", command, args, roots: [{ name: "no-uri" }] },
+          },
+        }),
+      );
+      const run = (extra: string[]) =>
+        runCli(["--config", configPath, ...extra, "--method", "tools/list"]);
+      try {
+        const loud = vi.spyOn(console, "warn").mockImplementation(() => {});
+        expectCliSuccess(await run([]));
+        expect(loud).toHaveBeenCalledWith(
+          "Dropping root without a string `uri`:",
+          expect.anything(),
+        );
+        loud.mockRestore();
+
+        const quiet = vi.spyOn(console, "warn").mockImplementation(() => {});
+        expectCliSuccess(await run(["-q"]));
+        expect(quiet).not.toHaveBeenCalled();
+        expect(console.warn).toBe(quiet);
+        quiet.mockRestore();
+      } finally {
+        rmSync(dir, { recursive: true, force: true });
+      }
+    });
+
+    // Quiet is read from Commander's parsed state, so a combined `-qe` counts
+    // and a `-q` that is another option's value does not (Copilot on #2576).
+    it("applies the pre-parse --quiet handling when -q is combined with -e", async () => {
+      const result = await runCli([
+        NO_SERVER_SENTINEL,
+        "-qe",
+        "A=1",
+        "--method",
+      ]);
+      expectCliFailure(result);
+      const lines = result.stderr.trimEnd().split("\n");
+      expect(lines).toHaveLength(1);
+      expect(JSON.parse(lines[0]!)).toHaveProperty("error");
+    });
+
+    it("does not treat -q as the flag when it is another option's value", async () => {
+      const result = await runCli([
+        NO_SERVER_SENTINEL,
+        "--client-secret",
+        "-q",
+        "--method",
+      ]);
+      expectCliFailure(result);
+      expect(result.stderr).toMatch(/^error: option '--method <method>'/);
+    });
+
     it("should fail with nonexistent method", async () => {
       const result = await runCli([
         NO_SERVER_SENTINEL,

clients/cli/__tests__/cliOAuth.test.ts

@@ -1184,4 +1184,127 @@ describe("cliOAuth", () => {
       }
     }
   });
+
+  // #2435: `--quiet` drops the status lines, never the prompts a human has to
+  // act on. The step-up [y/N] still prints, because the flow cannot finish
+  // without an answer to it.
+  describe("--quiet", () => {
+    const QUIET_CALLBACK = CALLBACK_URL_CONFIG;
+    function fakeClient(connect = vi.fn().mockResolvedValue(undefined)) {
+      return {
+        connect,
+        disconnect: vi.fn().mockResolvedValue(undefined),
+        authenticate: vi.fn(),
+        beginInteractiveAuthorization: vi.fn(),
+        completeOAuthFlow: vi.fn(),
+        checkAuthChallengeSatisfied: vi.fn().mockResolvedValue(false),
+      };
+    }
+
+    it("runCliInteractiveOAuth writes no success line", async () => {
+      vi.spyOn(
+        runnerInteractive,
+        "runRunnerInteractiveOAuth",
+      ).mockResolvedValue({ kind: "success" });
+      const stderrSpy = vi
+        .spyOn(process.stderr, "write")
+        .mockImplementation(() => true);
+
+      await runCliInteractiveOAuth(
+        fakeClient(),
+        new MutableRedirectUrlProvider(),
+        QUIET_CALLBACK,
+        { quiet: true },
+      );
+
+      expect(stderrSpy).not.toHaveBeenCalled();
+    });
+
+    it("withCliAuthRecoveryRetry writes no retry line after AuthRecoveryRequired, but keeps the step-up prompt", async () => {
+      vi.spyOn(
+        runnerInteractive,
+        "runRunnerInteractiveOAuth",
+      ).mockResolvedValue({ kind: "success" });
+      const stderrSpy = vi
+        .spyOn(process.stderr, "write")
+        .mockImplementation(() => true);
+      const fn = vi
+        .fn()
+        .mockRejectedValueOnce(
+          new AuthRecoveryRequiredError(
+            new URL("https://as.example/authorize"),
+            { reason: "insufficient_scope", requiredScopes: ["weather:read"] },
+          ),
+        )
+        .mockResolvedValueOnce("ok");
+
+      const result = await withCliAuthRecoveryRetry(
+        fakeClient(),
+        OAUTH_HTTP_CONFIG,
+        new MutableRedirectUrlProvider(),
+        QUIET_CALLBACK,
+        makeFakeServerSettings(),
+        fn,
+        { ...INTERACTIVE, confirmStepUp: async () => true, quiet: true },
+      );
+
+      expect(result).toBe("ok");
+      const written = stderrSpy.mock.calls.map((c) => String(c[0])).join("");
+      expect(written).toContain("Proceed with step-up authorization? [y/N]");
+      expect(written).not.toContain("Authorization complete");
+    });
+
+    it("withCliAuthRecoveryRetry writes no retry line after a plain 401", async () => {
+      vi.spyOn(
+        runnerInteractive,
+        "runRunnerInteractiveOAuth",
+      ).mockResolvedValue({ kind: "success" });
+      const stderrSpy = vi
+        .spyOn(process.stderr, "write")
+        .mockImplementation(() => true);
+      const fn = vi
+        .fn()
+        .mockRejectedValueOnce(new Error("RPC failed (401)"))
+        .mockResolvedValueOnce("ok");
+
+      const result = await withCliAuthRecoveryRetry(
+        fakeClient(),
+        OAUTH_HTTP_CONFIG,
+        new MutableRedirectUrlProvider(),
+        QUIET_CALLBACK,
+        undefined,
+        fn,
+        { ...INTERACTIVE, quiet: true },
+      );
+
+      expect(result).toBe("ok");
+      expect(stderrSpy).not.toHaveBeenCalled();
+    });
+
+    it("connectInspectorWithOAuth writes no success line after a 401 on connect", async () => {
+      vi.spyOn(
+        runnerInteractive,
+        "runRunnerInteractiveOAuth",
+      ).mockResolvedValue({ kind: "success" });
+      const stderrSpy = vi
+        .spyOn(process.stderr, "write")
+        .mockImplementation(() => true);
+      const connect = vi
+        .fn()
+        .mockRejectedValueOnce(new Error("RPC failed (401)"))
+        .mockResolvedValue(undefined);
+
+      await connectInspectorWithOAuth(
+        fakeClient(connect),
+        OAUTH_HTTP_CONFIG,
+        new MutableRedirectUrlProvider(),
+        QUIET_CALLBACK,
+        undefined,
+        { ...INTERACTIVE, quiet: true },
+      );
+
+      expect(connect).toHaveBeenCalledTimes(2);
+      expect(stderrSpy).not.toHaveBeenCalled();
+    });
+  });
 });