fix(mcp): bind stored OAuth tokens to their server URL and clean on remove

[liuhao1024]

What does this PR do?

Fixes two of the three MCP OAuth token-store defects from #90703:

1. Token/endpoint binding — HermesTokenStorage keyed tokens by server name alone, so after a server's url was edited in config (or another server's name sanitized to the same _safe_filename output), the token minted for the old endpoint was silently attached to the new provider — the old access/refresh token was sent to a different authorization server. The storage now accepts the server's configured URL: set_tokens records it in the token file, and get_tokens refuses the file when the recorded URL does not match (normalized; trailing-slash insensitive). Legacy files without the field pass through once and are lazily stamped with the current URL — existing users are not forced to re-login; protection applies on the next read or any later url change. Both provider-construction paths (tools/mcp_oauth.build_oauth_auth and MCPOAuthManager._build_provider) pass the server URL through; CLI login flows construct the storage without a URL and are unaffected. This binding also neutralizes the harmful half of the filename-collision issue: a colliding server's token fails the URL check instead of being served.

2. Removal cleanup — mcp.servers.remove (TUI RPC) and the REST DELETE /api/mcp/servers/{name} edited config.yaml only, leaving the removed server's mcp-tokens/<name>.json (including the refresh token) on disk; a server re-added under the same name silently resumed the old OAuth session. Both paths now call remove_oauth_tokens(name) best-effort after a successful config removal (cleanup failure logs a warning, never blocks the removal result).

Deliberately out of scope: changing the _safe_filename scheme itself (e.g. a URL-derived hash suffix to disambiguate colliding names) — that renames on-disk files for every existing install and deserves its own migration.

Related Issue

Fixes #90703

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• tools/mcp_oauth.py — HermesTokenStorage.__init__ accepts server_url; get_tokens enforces the recorded-URL match (fail-closed with a warning naming both endpoints) and lazily stamps legacy files; set_tokens records the URL; build_oauth_auth passes server_url into the storage
• tools/mcp_oauth_manager.py — _build_provider passes entry.server_url into the storage
• tui_gateway/methods_tools.py — mcp.servers.remove calls remove_oauth_tokens best-effort after removal
• hermes_cli/web_routers/mcp.py — REST delete path does the same
• tests/tools/test_mcp_oauth_cold_load_expiry.py — five binding tests: mismatched URL refuses, matching URL loads, legacy file passes through AND is stamped, set_tokens records the URL, unbound storage ignores the field
• tests/tui_gateway/test_mcp_profile_rpcs.py — remove deletes the profile's stored token file

How to Test

1. python -m pytest tests/tools/test_mcp_oauth_cold_load_expiry.py tests/tools/test_mcp_oauth.py tests/tui_gateway/test_mcp_profile_rpcs.py -q — should pass (80 passed), including the six new tests
2. Observed result: with the source changes stashed, five of the six new tests fail (tokens load regardless of recorded URL; the token file survives removal); with the fix, all pass
3. Manual equivalence for the binding: write mcp-tokens/srv.json with hermes_server_url: https://old/mcp, construct HermesTokenStorage("srv", server_url="https://new/mcp") — get_tokens() returns None with a warning naming both endpoints

Checklist

• Code follows the project's style guidelines
• Self-review completed
• Comments added for complex logic (why lazy stamping instead of forced re-login; why cleanup is best-effort; why the binding only filters when the reader knows its endpoint)
• Tests added that prove the fix is effective or prevent regression (fail-closed mismatch, migration, persistence, both removal paths)
• All new and existing tests pass locally (80 passed across the three suites)
• Platform: macOS (pure Python logic, platform-independent)

[liuhao1024]

Quick note on the first CI run's red All required checks pass, since it's worth explaining rather than leaving to dig out.

The only failure was:

tests/e2e/test_platform_commands.py::TestSlashCommands::test_plaintext_restart_gateway_in_group_stays_plain_text[telegram]
E   AssertionError: Expected 'mock' to have been called once. Called 0 times.

This branch cannot reach that test: the diff is MCP OAuth token storage (tools/mcp_oauth.py, tools/mcp_oauth_manager.py), the two server-removal call sites (tui_gateway/methods_tools.py, hermes_cli/web_routers/mcp.py), and two test files under tests/tools/ + tests/tui_gateway/ — nothing on the Telegram slash-command path the e2e exercises. The identical failure also hit #90564's first run (a PowerShell-script-only diff that cannot reach it either), which points at a flaky mock-timing issue in that e2e rather than any branch change.

I've rebased onto current main (044acf2) and force-pushed to retrigger — same single commit, no content changes.

[andrexibiza]

Blocking exact-head review of 485978cf19f355d984293bcadb15b4597198c3ec against current main and the OAuth-lifecycle architecture already split out in #84963.

This head improves two call sites, but it does not yet establish credential identity or convergent teardown. Several tests currently codify fail-open behavior.

1. Legacy "lazy stamping" sends an unbound token to the new endpoint once

get_tokens() treats a file with no hermes_server_url as valid for whatever configured URL is current, returns the token, and stamps that URL onto disk. That means the exact first request after an endpoint edit or filename collision can still send endpoint A's access/refresh token to endpoint B. The new test_legacy_file_without_url_lazily_stamps_it makes this bypass expected behavior.

An unbound legacy credential has no authority to select its own new binding. When the caller knows the endpoint, legacy state must fail closed and require reauthorization, or be migrated only after an authenticated flow proves the resource/issuer identity. Never bind by first use.

2. Only <name>.json is bound; client registration and OAuth metadata remain cross-endpoint

HermesTokenStorage also loads <name>.client.json and <name>.meta.json through the same lossy name namespace. This patch does not bind or validate either file. A server URL change or sanitized-name collision can therefore reuse:

• a dynamically registered client_id / client_secret and redirect registration from another authorization server;
• cached authorization-server metadata, including the token endpoint/issuer provenance.

Refusing the access-token file is not enough if the subsequent reauth can send another server's client secret or trust another server's discovery result.

The persisted unit needs one collision-resistant identity envelope covering the raw server name, normalized MCP resource endpoint, discovered authorization-server issuer/metadata origin, client-registration identity, and provider generation. Every read must validate that envelope before any token, client info, or metadata enters the provider.

3. The filename collision remains live and Fixes #90703 is false topology

Leaving _safe_filename() unchanged still allows distinct raw names to overwrite the same three files. URL mismatch may stop one token read when endpoints differ, but it does not stop overwrite, deletion, denial of service, same-endpoint credential sharing, or one colliding server's removal deleting the other's credentials.

Use a readable slug plus a collision-resistant hash of the raw UTF-8 server identity, with an explicit migration path. Until that lands, this PR cannot close #90703; at minimum the issue relation must be Refs, not Fixes.

4. Removal surfaces do not converge and the REST path cleans the wrong profile

The REST DELETE removes config inside _profile_scope(profile), exits that scope, and only then calls remove_oauth_tokens(name). For a named profile, cleanup therefore resolves the default HERMES_HOME: the target profile's refresh token survives and the default profile's same-named token may be deleted instead.

The whole-map PUT /api/mcp/servers path can also remove server keys through _replace_mcp_servers() without any credential cleanup. And both new paths call the disk helper directly rather than the manager lifecycle operation, so a cached provider can remain alive after config/token deletion.

Centralize one idempotent removal primitive that, under the target profile identity, removes config, generation-fences/evicts the provider, and deletes the complete credential bundle. CLI, TUI RPC, REST DELETE, and whole-map replacement must all use it and report partial cleanup truthfully; duplicating best-effort try/except blocks is not convergent semantics.

5. Endpoint changes/removal need provider-generation fencing

The manager discards its cache entry when server_url changes, but an older in-flight provider can still finish refresh/auth and write back to the same name-derived files after the new generation exists or after removal. A URL string in the payload detects some later reads; it does not stop stale-generation writeback.

Preserve #84963's OAuth-lifecycle split and generation invariant: each provider/storage handle is minted for a specific identity generation; writes and cleanup succeed only while that generation is still current. This work belongs in the lifecycle child with token cleanup and provider ownership, not as a parallel cache identity.

Required witnesses before merge:

• legacy unbound files never release credentials to a known endpoint;
• two colliding raw names remain isolated across token, client, metadata, and removal;
• URL/issuer change invalidates the whole bundle;
• stale generation cannot write after reconfigure/remove;
• named-profile REST DELETE deletes only that profile's bundle;
• whole-map replacement and every other removal surface share the same cleanup result;
• removal evicts live provider state as well as deleting files.

Exact-head CI is green, but it currently does not exercise these identity and lifecycle boundaries.