Fall back to the control plane on a stale session JWT

[tnsardesai]

Summary

A direct-to-VM 401/403 with a session jwt query param now evicts the cached route and retries the original request against the API.

This is the same fallback as kernel-python-sdk#157. It did not land in #164 because that PR merged before this commit.

Retry keys off the request JWT, not a still-present cache entry, so concurrent 401s still fall back after the first eviction.

Test plan

• go test ./lib/browserrouting
• screenshot 401 on metro retries on the API origin with Authorization restored
• cached route is evicted after the stale JWT

---

Note

Medium Risk
Touches request rewriting, auth header restoration, and retry of browser session traffic. Behavior is client-side routing with conservative no-retry when the body cannot be replayed.

Overview
Direct-to-VM requests that get 401/403 with a session jwt query param now retry against the original API origin instead of failing on a stale cached route.

DirectVMRoutingMiddleware restores the original URL, host, and Authorization header, strips jwt, evicts the cached session route, and reissues the request. Fallback is keyed off the request JWT (not a still-present cache entry), so concurrent 401s still retry after the first eviction. If the body cannot be rewound (GetBody missing or failing), the original auth response is returned unchanged.

^{Reviewed by Cursor Bugbot for commit c342a58. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 82ad2ea. Configure here.}

[rgarcia]

There is a cache invalidation race in the stale-JWT fallback.

cache.Delete(sessionID) unconditionally deletes whatever route is current when the stale request returns. If request A uses stale route A, another request stores refreshed route B, and then request A receives its 401, A deletes the fresh route B. I reproduced this deterministically by storing route B inside the VM handler before returning 401; cache.Load is empty after fallback. Calls remain functional through the control plane, but direct routing stays disabled until another cache-populating request.

Please add a compare-and-delete operation under the cache mutex that removes the entry only when its JWT/base URL still match the route used for the failed request. The control-plane fallback should still happen when the comparison loses, so concurrent stale 401s recover without deleting a newer route. Please add the corresponding race regression test.

There is also an unresolved BugBot finding on the latest commit: if GetBody() returns an error, the middleware returns the 401 response plus the rewind error without closing the response. requestconfig treats the error as decisive, masking the auth response and potentially leaking the connection. Returning the original readable 401 with a nil error would match the existing no-GetBody branch.

The normal rewind path and the full/race test suites otherwise look good.

[Sayan-]

res.Body.Close() on the fallback path is unguarded, while sniffAndPopulateCache at line 250 of the same file checks res.Body == nil. Only reachable through a custom CustomHTTPDoer, so it's cheap insurance rather than a live bug.

Also worth mirroring the guard the outer retry loop has: shouldRetry in internal/requestconfig/requestconfig.go bails when req.Body != nil && req.GetBody == nil, because such a body can't be replayed. The middleware fallback has no equivalent check. It doesn't bite today, since every generated method sends a *bytes.Buffer and gets GetBody set at requestconfig.go:395-412, which is what lets net/http rewind across the second next(req). It would only matter for a caller-supplied raw io.Reader body, which lands in the default branch with GetBody nil.

Same note as the Node PR: the description says this matches kernel-python-sdk#157, but that branch has no 401/403 fallback, so Python is currently the only SDK routing computer and playwright direct without stale-JWT recovery.

Confirmed end to end that the fallback preserves the request body, restores Authorization, strips jwt, and evicts the route, and that falling back on 403 upgrades metro-api's plaintext "Pool session not leased" to the control plane's proper 400 envelope.

[Sayan-]

Withdrawing my earlier review: I was on 128110f, and both points had already landed. The close-after-rewind ordering is 82ad2ea and the nil guard plus the GetBody-failure path is c342a58. prepareControlPlaneFallback covers exactly what I was asking for. Sorry for the noise.

My parity note was also wrong. #157 has since picked up the fallback in f525a24, 3580671 and 728330d, so Python is no longer missing it. Disregard that paragraph.

One data point on the original body-rewind finding, since it was filed as High Severity. I tested at 128110f, before a956805 added the explicit rewind, and the body survived both hops: net/http rewinds from req.GetBody, which requestconfig.go:395-412 sets for the *bytes.Buffer and *bytes.Reader cases that every generated method produces. Confirmed the same with a bare double Client.Do. So a956805 reads as defensive rather than a bug fix. Still worth keeping, since it makes the contract explicit instead of relying on transport internals, and it covers the default branch where GetBody is nil.

On the open race, I reproduced it independently and it confirms: storing a refreshed route for the same session while the stale request is in flight leaves cache.Load empty after fallback.

after fallback: present=false jwt=""     // expected jwt-FRESH

A compare-and-delete under the existing mutex is the right shape. Comparing JWT rather than base URL is the part that matters, since the VM address is unchanged when only the JWT rotates.