Route computer and playwright calls directly to the VM - #157
Conversation
Add computer and playwright to the default browser routing allowlist so screenshot and execute traffic can skip the control plane once metro-api records activity on the kernel proxy.
The default now includes computer and playwright, not just curl.
The asserted list can grow; the test name should not.
rgarcia
left a comment
rgarcia
left a comment
There was a problem hiding this comment.
QA’d commit 1b28d62 across sync and async clients.
- Browser-routing suite: 26 passed; PR CI is green.
- Focused experiments confirmed
computer/batch, clipboard, screenshot, and Playwright calls route directly with the session JWT and withoutAuthorization. process/*,fs/*, andtelemetry/eventsretain API-origin routing and API authorization.- Missing
base_url, custom allowlists, and the empty-value kill switch behave correctly. - Retry rewriting is idempotent and preserves request parameters.
Two non-blocking notes:
- The
computerprefix routes allcomputer/*endpoints, while the description and tests focus on screenshot/execute. Please consider one nested computer endpoint test. - A stale session JWT produces a direct-VM 401 with no automatic cache refresh or control-plane fallback. A follow-up could evict the route on 401/403 or document
client.browsers.get(id)as the refresh path.
No blocking findings.
A direct-to-VM 401/403 now evicts the cached route and retries the original request against the API.
|
addressed the stale JWT note: a direct-VM 401/403 now evicts the cached route and retries the original request on the control plane. no get() refresh — if the jwt is stale the API path still works, and a later create/retrieve will recache. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 f525a24. Configure here.
Lint failed on the stale-JWT retry change.
A metro 401/403 with a jwt query param is enough to fall back to the control plane, so concurrent requests still retry after the first eviction.
|
addressed the concurrent stale-JWT race: retry now keys off the request jwt query param, not a still-present cache entry. first eviction no longer blocks other in-flight 401/403s from falling back to the control plane. same fallback is now in node #170 and go #164. |
rgarcia
left a comment
rgarcia
left a comment
There was a problem hiding this comment.
The stale-JWT fallback still has the cache invalidation race now covered in the Node and Go reviews.
_session_id_from_direct_vm_response() identifies the current cached route by base URL, then cache.delete(session_id) removes it without checking whether its JWT is the one that failed. If request A leaves with jwt-old, another request refreshes the same base URL to jwt-new, and A later returns 401, the fresh jwt-new route is deleted. I reproduced this against head 728330d: after replacing the cached route before processing the old 401, cache.get("sess-1") returns None.
The latest change correctly fixes the separate race where concurrent stale 401s stopped retrying after the first eviction, but it does not protect a newer cache entry from an older response.
Please make eviction conditional on the current route’s base URL and JWT matching the failed request, and add a regression test that refreshes the route while the stale request is in flight and asserts the fresh route survives. The existing routing suite passes, but does not cover this interleaving.
3 participants
Summary
Default direct-to-VM routing now includes
computerandplaywrightin addition tocurlandtelemetry/stream.computer/screenshotandplaywright/executerewrite to{base_url}/...with the session JWT and dropAuthorization.process/*,fs/*, andtelemetry/eventsstay on the API origin.KERNEL_BROWSER_ROUTING_SUBRESOURCESstill overrides the default list. An empty value still disables routing.Metro-api activity recording for
/browser/kernel/*has been merged in kernel/kernel#3417. That was the hard gate for this allowlist flip. The kill switch is leavingbase_urlunset or settingKERNEL_BROWSER_ROUTING_SUBRESOURCESto empty.Latency (eu-west)
Same hop skip as the Node/Go SDKs. Comparison script: https://gist.github.com/tnsardesai/6704dfc86e2c0785e03beaf274aa672d
Same
eu-westsession. Published SDK still hitsapi.onkernel.com; this allowlist rewrite hits metro-api.Test plan
curl,telemetry/stream,computer,playwrightNote
Medium Risk
Changes default request routing and retry/auth fallback for browser subresources, including JWT eviction on 401/403. Misrouting or retry bugs could send traffic to the wrong origin or leak/drop auth headers.
Overview
Default direct-to-VM routing now includes
computerandplaywrightalongsidecurlandtelemetry/stream. Screenshot and Playwright execute rewrite to the sessionbase_urlwith JWT and dropAuthorization.process,fs, andtelemetry/eventsstay on the API origin.KERNEL_BROWSER_ROUTING_SUBRESOURCESstill overrides; empty still disables.On 401/403 from a JWT-query VM request, the client evicts the cached route and retries so the follow-up hits the control plane with the API key. Successful delete/release still evicts as before.
Reviewed by Cursor Bugbot for commit 728330d. Bugbot is set up for automated code reviews on this repo. Configure here.