feat(auth): store acquired OAuth tokens in the secret store; clobber-safe oauth.json writes

README.md

@@ -15,7 +15,7 @@ npx @modelcontextprotocol/inspector --tui    # TUI
 ```
 
 > [!WARNING]
-> **On a machine with no OS keychain, secrets are saved to a plaintext file by default.** That covers Linux without libsecret or a Secret Service, headless and SSH sessions, Termux, and containers with a mounted volume. OAuth client secrets and stdio `env:` values then go to `~/.mcp-inspector/secrets.json`, unencrypted unless you supply a key. See [Where secrets are stored](./docs/secret-storage.md) for how to get a keychain back, encrypt the file, or keep secrets in memory only.
+> **The Inspector manages secrets — OAuth tokens, OAuth client secrets, and stdio `env:` values — and stores them in the OS keychain, if available, by default.** On a machine with no keychain — Linux without libsecret or a Secret Service, headless and SSH sessions, Termux, and containers with a mounted volume — they are saved to `~/.mcp-inspector/secrets.json` instead, unencrypted unless you supply a key. See [Where secrets are stored](./docs/secret-storage.md) for how to get a keychain back, encrypt the file, or keep secrets in memory only.
 
 > **Upgrading from v1?** Read the [v1 → v2 migration guide](./docs/v1-to-v2-migration.md) — CLI flags, the new `--config` vs. `--catalog` split, the Node engine bump, and what no longer ships.
 

clients/cli/README.md

@@ -273,7 +273,7 @@ Interactive OAuth (connect-time or mid-RPC) requires a TTY on **stdin or stderr*
 
 **Step-up (standard OAuth):** when an RPC needs extra scopes, the CLI prompts on stderr: `Proceed with step-up authorization? [y/N]`. **y** continues (including piped stdin — `echo y | …` or `printf y | …`); **N** or EOF with no answer (`< /dev/null` / Ctrl-D) declines. Piped answers must be **newline-terminated, or stdin must close** — a bare `y` held open without `\n` or EOF is not flushed as a line and times out. A non-TTY stdin that never sends a line within **5 seconds** fails with `auth_required` (`timed out`, not the same as an explicit **N**). Answering **y** only confirms step-up — the following browser/loopback OAuth can still wait up to 15 minutes; for headless CI prefer **`--stored-auth-only`** with tokens already in the store. EMA step-up re-mints silently (no prompt).
 
-**Shared OAuth storage:** the CLI **reuses** tokens from `~/.mcp-inspector/storage/oauth.json` when they already exist (same file as other Inspector clients). That is passive file sharing, not launching another app.
+**Shared OAuth storage:** the CLI **reuses** tokens stored by other Inspector clients — indexed by the shared `~/.mcp-inspector/storage/oauth.json`, with the tokens themselves held in the secret store. That is passive storage sharing, not launching another app.
 
 **Shared with TUI** (config only, not interactive login):
 
@@ -315,7 +315,7 @@ See [EMA / enterprise-managed auth](../../specification/v2_auth_ema.md) and [OAu
 
 #### Stored-auth (web → CLI handoff)
 
-For the common case where OAuth was already completed in the **web inspector on the same machine**, the CLI can reuse the resulting token instead of running its own interactive flow. It reads the shared OAuth state file (the `oauth.json` the web backend writes) directly from disk and injects `Authorization: Bearer <token>` for `--server-url`.
+For the common case where OAuth was already completed in the **web inspector on the same machine**, the CLI can reuse the resulting token instead of running its own interactive flow. It reads the shared OAuth state (the `oauth.json` the web backend writes, joined with the tokens in the secret store) and injects `Authorization: Bearer <token>` for `--server-url`.
 
 | Option                  | Description                                                                                                                                                                                                                                                                                                                                                                                                                         |
 | ----------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
@@ -454,7 +454,7 @@ prose from stderr:
 | `0`  | Success.                                                                      |
 | `1`  | Usage / unexpected error (the catch-all).                                     |
 | `2`  | No MCP App found on the tool (`--app-info` probe).                            |
-| `3`  | Server requires authentication (401/403, `WWW-Authenticate`, OAuth).          |
+| `3`  | Server requires authentication (401/403 or a typed SDK auth error).           |
 | `4`  | Server unreachable (DNS, connection refused, timeout, `fetch failed`).        |
 | `5`  | Tool error (`tools/call` returned `isError:true`, or the tool was not found). |
 | `6`  | `--strict` found an error-severity tool-schema portability problem (`schema_unportable` — the schema is valid JSON Schema, just not portable). |

clients/cli/__tests__/error-handler.test.ts

@@ -6,6 +6,12 @@ import {
   formatErrorOutput,
   handleError,
 } from "../src/error-handler.js";
+import { UnauthorizedError } from "@modelcontextprotocol/client";
+import {
+  SecretFileLockHeldError,
+  SecretStoreUnavailableError,
+} from "@inspector/core/auth/node/secret-store.js";
+import { OAuthStateFileUnrecognizedError } from "@inspector/core/auth/node/oauth-persist-file.js";
 
 /**
  * `handleError` is the binary's last-resort error sink (wired up in
@@ -111,11 +117,70 @@ describe("classifyError", () => {
     expect(envelope.code).toBe("schema_unportable");
   });
 
-  it("classifies a WWW-Authenticate message as AUTH_REQUIRED without a status", () => {
-    const { exitCode } = classifyError(
-      new Error("Dynamic client registration failed: WWW-Authenticate Bearer"),
+  it("classifies a typed UnauthorizedError as AUTH_REQUIRED without a status", () => {
+    // Every genuine auth-required condition in the SDK throws typed
+    // `UnauthorizedError` (or carries a structured 401) — classification is
+    // by type via isUnauthorizedError, not by sniffing message keywords.
+    const { exitCode, envelope } = classifyError(
+      new UnauthorizedError("Failed to authorize"),
     );
     expect(exitCode).toBe(EXIT_CODES.AUTH_REQUIRED);
+    expect(envelope.code).toBe("auth_required");
+  });
+
+  it("classifies a structured 401 buried in the cause chain as AUTH_REQUIRED", () => {
+    // protocolEra negotiation can wrap the real 401 as a nested cause;
+    // isUnauthorizedError walks the chain.
+    const err = new Error("negotiation failed", {
+      cause: Object.assign(new Error("upstream"), { status: 401 }),
+    });
+    const { exitCode } = classifyError(err);
+    expect(exitCode).toBe(EXIT_CODES.AUTH_REQUIRED);
+  });
+
+  it("does not classify prose mentioning OAuth as AUTH_REQUIRED", () => {
+    // The retired keyword heuristic (/…|OAuth/i) reported errors like this
+    // one — a programming/usage failure — as "re-authorize", exit 3. It is
+    // a plain error: exit 1, and the caller reads the message.
+    const { exitCode, envelope } = classifyError(
+      new Error("OAuth storage is required for this operation."),
+    );
+    expect(exitCode).toBe(EXIT_CODES.USAGE);
+    expect(envelope.code).toBe("error");
+  });
+
+  it("classifies SecretStoreUnavailableError as store_unavailable, exit 1", () => {
+    const { exitCode, envelope } = classifyError(
+      new SecretStoreUnavailableError("keychain probe failed"),
+      { url: "https://x.example/mcp" },
+    );
+    expect(exitCode).toBe(EXIT_CODES.USAGE);
+    expect(envelope.code).toBe("store_unavailable");
+    expect(envelope.url).toBe("https://x.example/mcp");
+  });
+
+  it("does not let OAuth wording in a lock-held store error read as auth_required", () => {
+    // SecretFileLockHeldError messages mention the OAuth state file. The
+    // typed operational branch classifies it as store_unavailable — telling
+    // the user the store is busy, not to re-authorize.
+    const { exitCode, envelope } = classifyError(
+      new SecretFileLockHeldError(
+        "Could not lock the OAuth state file: held by another process",
+      ),
+    );
+    expect(exitCode).toBe(EXIT_CODES.USAGE);
+    expect(envelope.code).toBe("store_unavailable");
+  });
+
+  it("classifies an unrecognized OAuth state file as oauth_state_unrecognized", () => {
+    // Repair-the-file advice, not re-authorize (auth_required) and not
+    // retry-later (store_unavailable): the error message tells the user
+    // exactly what to do, and the code lets a script branch on it.
+    const { exitCode, envelope } = classifyError(
+      new OAuthStateFileUnrecognizedError("/tmp/oauth.json", "save"),
+    );
+    expect(exitCode).toBe(EXIT_CODES.USAGE);
+    expect(envelope.code).toBe("oauth_state_unrecognized");
   });
 
   it("classifies ENOTFOUND / fetch failed as UNREACHABLE", () => {
@@ -379,12 +444,13 @@ describe("envelope URL redaction", () => {
   });
 
   it("classifies on the unredacted text", () => {
-    // The only auth signal is inside a parameter value that redaction
-    // replaces; classifying the redacted copy would fall through to USAGE.
+    // The only classification signal (an UNREACHABLE_PATTERN match) is
+    // inside a parameter value that redaction replaces; classifying the
+    // redacted copy would fall through to USAGE.
     const { exitCode, envelope } = classifyError(
-      new Error("Rejected https://srv.example/cb?token=invalid_token"),
+      new Error("Rejected https://srv.example/cb?token=ECONNREFUSED"),
     );
-    expect(exitCode).toBe(EXIT_CODES.AUTH_REQUIRED);
+    expect(exitCode).toBe(EXIT_CODES.UNREACHABLE);
     expect(envelope.message).toBe(
       "Rejected https://srv.example/cb?token=%5BREDACTED%5D",
     );