fix(auth): carry stripped-namespace secrets over before purging (#2630)

[cliffhall]

Closes #2630

Problem

When a 2.9.x Inspector saves a state file, it strips the secretsNamespace stamp and keeps each entry's residue. For a confidential DCR client, that residue includes client_id and the auth method. But 2.9.x can only see the legacy shared ids, which 2.10 adoption had already emptied. The next 2.10 save re-adopted the file under a fresh namespace and purged the superseded namespace first (#2560). That namespace held the only copy of the client_secret, so the next token exchange failed until the user re-authorized.

Fix

This takes the issue's first option: carry the secrets over, and purge only what nothing references.

• Adoption (adoptSecretsNamespace): for every server and IdP issuer the file still indexes, the copy step reads the legacy ids first. For any field they lack, it falls back to the namespaces the ledger records, newest first. The superseded namespaces are purged only after the new stamp has committed with those values copied under it, so a failed stamp still rolls back with nothing deleted. A file with no entries has nothing to carry and purges immediately, as before.
• Reads (readOAuthStore): an unstamped file uses the same fallback. The in-memory state a save starts from therefore already holds the secret. Without this, a save that touches that same server would see no client_secret in its snapshot and delete the value adoption just carried over. A stamped file never falls back.
• Ledger (recordedNamespaces): returns the recorded namespaces newest first, using insertion order, which JSON preserves. It is best-effort like the rest of the ledger.

Precedence is legacy ids, then the newest namespace, then older ones. Anything the older Inspector wrote since the strip (for example, its own re-registration's secret) wins. A stripped namespace only fills what it alone still holds.

This also stops the same loss for other fields: tokens and IdP sessions of servers the stripping save never touched used to be purged along with the client secret.

Tests

• oauth-secrets-namespace.test.ts gets a new #2630 block with five tests:

  • the issue's repro: the secret is carried into the new namespace and the old one is purged
  • read-then-save of the same server keeps the secret
  • a legacy value written since the strip wins
  • the newest namespace wins, and older ones fill only the gaps
  • a stamped file never falls back

  Against the unfixed oauth-persist-file.ts, the first three fail; the last two are regression guards.

• oauth-namespace-ledger.test.ts: recordedNamespaces ordering, the no-ledger case, and the unreadable-ledger case.

• docs/secret-storage.md describes the carry-over.

Gate

npm run local:gate is green through every stage up to smoke:web:firefox, which can't launch on macOS 27 (#2625). local:storybook was then run on its own and passed. The Firefox smoke ran in the Linux Playwright container (mcr.microsoft.com/playwright:v1.62.1-noble) against a clean git archive export and passed.

One earlier gate run hit a one-off failure in storage.test.ts > clearAllOAuthClientState clears every server in the default store. That file passes 3/3 in isolation, both with and without this change. The cause is that the suite uses the developer's real ~/.mcp-inspector/storage/oauth.json, which is shared across workers and processes. That is filed as #2670 and is not touched here.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Cross-backend re-adoption can still purge the only keyring copy after an alternate backend commits a namespace without carrying the secret.

1 open finding

• Cross-backend re-adoption can delete the sole secret copy · New

What changed in this PR

Adds recovery of OAuth secrets after older Inspector versions strip namespace stamps.

Changes:

• Carries legacy/fallback secrets into newly adopted namespaces before purging.
• Adds newest-first namespace-ledger lookup.
• Documents and tests migration precedence and stamped-file behavior.

File	Description
core/​auth/​node/​oauth-persist-file.ts	Implements fallback reads and carry-over during adoption.
core/​auth/​node/​oauth-namespace-ledger.ts	Exposes recorded namespaces newest-first.
oauth-secrets-namespace.test.ts	Tests cross-version secret recovery and precedence.
oauth-namespace-ledger.test.ts	Tests namespace ordering and failure handling.
docs/​secret-storage.md	Documents carry-over behavior.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[cliffhall]

Copilot review, round 1: one finding (cross-backend re-adoption can still purge the only keychain copy). I declined it as out of scope and filed it as #2672, with the reasoning in its thread: the suggested reconciliation into a stamped namespace would resurrect deliberately cleared values. No code changed.

Review loop closed. The round held only an out-of-scope finding, which is the stop condition in pr-flow 7c.