Preserve fresh per-request identity in vMCP backend transports

[tgrunnagle]

Summary

• vMCP backend HTTP clients captured the request *auth.Identity once at session/client-creation time and re-injected that snapshot on every outgoing backend request, overriding the fresh identity placed on req.Context() by auth.TokenValidator.Middleware. Because that fresh identity carries upstream tokens transparently refreshed by upstreamtoken.InProcessService.GetAllValidTokens, the override silently re-injected stale upstream access tokens for the entire session lifetime — users saw a forced re-auth roughly every 24h (the upstream provider's access-token lifespan).
• The captured identity is now treated as a fallback: it is injected only when req.Context() carries no identity. This preserves the streamable-HTTP Close()/DELETE teardown path — mcp-go constructs that request from context.Background(), which loses the per-request identity — while letting normal backend calls use the freshly refreshed identity from the request context.
• The fix is applied to both transport chains that exhibited the bug: identityPropagatingRoundTripper in pkg/vmcp/client/client.go (per-call client) and the new identityRoundTripper in pkg/vmcp/session/internal/backend/mcp_session.go (persistent session-backed client). Existing health-check marker propagation in the per-call client is preserved.

Closes #5323

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• Linting (task lint-fix)
• Manual testing (describe below)

Unit-test coverage added/updated:

• pkg/vmcp/client/client_test.go — round-tripper tests reorganized into Per-request / Fallback / Health-check sections; new cases assert that a fresh identity on req.Context() is preserved unchanged, that the fallback is injected only when the request context has no identity, and that the health-check marker behavior is unchanged.
• pkg/vmcp/session/internal/backend/roundtripper_test.go — mirrors the above for the session-backed identityRoundTripper.
• pkg/vmcp/session/internal/backend/mcp_session_identity_refresh_test.go (new) — higher-level regression test that exercises the full transport chain (headerForwardRoundTripper → identityRoundTripper → authRoundTripper with UpstreamInjectStrategy → http.DefaultTransport) against a fake backend. Asserts that the upstream Authorization header on each tools/call reflects the identity placed on the per-request context, NOT the identity captured at session-init time. Verified to fail under the pre-fix always-override behavior, so it is a true [vMCP] identityPropagatingRoundTripper captures stale identity, bypassing per-request upstream token refresh #5323 regression guard.

Manual verification: traced through auth.TokenValidator.Middleware → InProcessService.GetAllValidTokens → vMCP outgoing-auth strategies (upstream_inject, tokenexchange, aws_sts) to confirm that all three read identity from req.Context() and therefore benefit from the fix.

API Compatibility

• This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Changes

File	Change
pkg/vmcp/client/client.go	identityPropagatingRoundTripper no longer overrides identity on req.Context(); the captured identity is now a fallback used only when the request context carries none. Doc comments rewritten to document the invariant.
pkg/vmcp/session/internal/backend/mcp_session.go	New identityRoundTripper with the same fallback-only semantics, inserted into the session-backed transport chain. Documents asymmetric health-check handling (not needed in this chain).
pkg/vmcp/client/client_test.go	Reorganized round-tripper tests; new cases for per-request identity preservation, fallback-only injection, and health-check marker behavior.
pkg/vmcp/session/internal/backend/roundtripper_test.go	Same coverage for the session-backed round-tripper.
pkg/vmcp/session/internal/backend/mcp_session_identity_refresh_test.go	New higher-level regression test exercising the full transport chain end-to-end.

Does this introduce a user-facing change?

Yes. Users of vMCP backends configured with upstreamInject, tokenExchange, or awsSTS outgoing auth no longer see forced re-auth when the upstream provider's access token expires mid-session. Upstream tokens are transparently refreshed on every tool call as long as the vMCP-issued refresh token remains valid.

Special notes for reviewers

• The fix is intentionally scoped to the round-tripper layer. The broader Phase 2 work tracked in [vMCP] Proactive auth credential refresh via per-request identity resolution (Phase 2) #3877 (proactive refresh, session-scoped client lifecycle per THV-0038) and the reactive 401/403 retry safety net in [vMCP] Handle auth credential expiration with retry and proactive refresh #3869 remain open and unchanged by this PR. This change is essentially Part 1 of [vMCP] Proactive auth credential refresh via per-request identity resolution (Phase 2) #3877, filed separately because it has a concrete user-visible impact worth landing on its own.
• Two duplicate round-tripper implementations now exist (one in pkg/vmcp/client, one in pkg/vmcp/session/internal/backend). They share the same invariant and were left duplicated to keep this PR focused on the bug fix. Consolidation is tracked in follow-up issue [vMCP] Consolidate duplicate identity-propagation round-trippers into shared internal transport package #5333.
• The higher-level regression test stops short of wiring auth.TokenValidator.Middleware end-to-end (which would pull in pkg/auth/upstreamtoken and a JWT signer) and instead injects the fresh identity directly onto the per-request context — which is exactly what the middleware does on every authenticated request. Extending the test to literally wire the middleware + a fake UpstreamTokenRefresher (AC5 as written on the issue) is tracked in follow-up issue [vMCP] Extend identity-refresh integration test to wire TokenValidator.Middleware + fake UpstreamTokenRefresher #5334.
• Invariant to preserve in future changes: a non-nil identity in req.Context() must never be silently overridden by a captured one. The doc comments on both round-trippers state this explicitly.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.35%. Comparing base (2ce3ef7) to head (76ad527).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5335      +/-   ##
==========================================
- Coverage   68.36%   68.35%   -0.02%     
==========================================
  Files         620      620              
  Lines       63316    63258      -58     
==========================================
- Hits        43285    43239      -46     
+ Misses      16800    16791       -9     
+ Partials     3231     3228       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tgrunnagle]

Multi-agent review summary

Reviewed against the repo's .claude/rules/ (go-style, vmcp-anti-patterns, security, testing, pr-creation) using 5 specialist agents (auth/security, concurrency, Go code quality, vMCP architecture, general). Codex cross-review skipped (CLI not installed).

Bottom line: the fix is correct and well-tested at the layer it targets. No HIGH findings. Six pre-merge polish items improve clarity, robustness, and drift-resistance.

Consensus findings (score ≥ 7)

#	Severity	Where	Title
1	MEDIUM	mcp_session_identity_refresh_test.go	"Maintenance note" + Initialize assertion is a lint-driven test smell
2	MEDIUM	both round-trippers	Duplicate implementation risks drift; add cross-reference comments now
3	MEDIUM	client.go factory comment	Doc claim understates fallback scope (injects on any context lacking identity)
4	LOW	pkg/auth/context.go (not in diff)	IdentityFromContext returns ok=true for typed-nil *Identity
5	LOW	4 files	Identity-invariant prose duplicated across 4 files (drift risk)
6	LOW	both round-tripper structs	Document immutability invariant on the captured *Identity

Details on #1, #2, #3, #5, #6 are inline. #4 is independent of this PR's diff so noted here:

pkg/auth/context.go:51 — IdentityFromContext does ctx.Value(...).(*Identity) and returns (nil, true) if a caller stored a typed-nil *Identity via raw context.WithValue. The new round-tripper guards if !hasIdentity && fallbackIdentity != nil so a typed-nil would suppress the fallback and pass nil to UpstreamInjectStrategy, which would nil-deref on identity.UpstreamTokens[providerName]. WithIdentity is nil-safe so this is latent rather than active. Recommendation: tighten to return identity, ok && identity != nil. One-line, defensive, no caller impact. Out of scope for this PR — file a follow-up if you agree.

What's solid

• Identity invariant correctly inverted and consistently applied in both transport chains
• Regression test (TestHTTPSession_PerRequestIdentity_DrivesUpstreamAuthHeader) verified to fail under pre-fix behavior — true #5323 regression guard
• No new log statements, no credential leakage risk introduced
• No public API change; all renames on unexported types
• Doc comments accurately describe behavior; field rename identity → fallbackIdentity makes the invariant self-documenting
• Inner round-trippers (authRoundTripper, headerForwardRoundTripper) still clone before mutating, so the new "skip-clone-when-no-mutation" fast path is safe
• Test files correctly colocated with existing fakeBackend helpers
• The doc comment correctly notes WHY health-check propagation is absent in the session-backed connector (forward-pointer for future maintainers)

Dropped findings (consensus < 7, listed for auditability)

• Mutable mutated flag violates immutable-assignment style (LOW, 6) — stylistic taste
• sess.Close() in t.Cleanup lacks a bounded path (LOW, 6) — in-process httptest, low practical risk
• Upstream mcp-go fix not tracked anywhere (LOW, 6) — fixing Close() to reuse session ctx upstream would obviate the fallback entirely
• Over-long "Scope note" comment on defaultClientFactory (LOW, 5) — specialists disagreed; appropriate for the bug's subtlety
• E2E test bypasses TokenValidator.Middleware (LOW, 6) — acknowledged in PR description and tracked as #5334
• Behavior change is a soft backwards-compat break (LOW, 5) — OOT callers relying on the buggy override
• Several INFO-level items (test naming, magic strings, 5s timeouts, line count vs CLAUDE.md cap)

Process notes

• PR is in draft state. Per .claude/rules/pr-creation.md ("Reviewer trust signals"), mark ready-for-review before requesting human review, or note in the description that draft is intentional pending follow-up.
• Net production code is small (~10 lines of real logic + comments + renames). The PR's line count is dominated by tests and doc comments, well under the 400-line cap for non-test/non-doc changes.

[tgrunnagle]

Re: body finding #4 (IdentityFromContext returns (nil, true) for typed-nil *Identity) — filed as #5337 for follow-up. Keeping it out of this PR to stay focused on the #5323 transport-layer regression; the hazard is latent (no in-tree caller currently stores a typed-nil) and the fix is a 1-line defensive tighten at the read site, well-scoped to its own PR.