A session holds up to eight browsers, and every tool can name one (0.15.0) - #1266
Merged
Conversation
The first slice of a bigger change: one session is going to hold up to eight browsers, and nothing could address a second one because every tool called `registry.ensure()` with no argument and landed on the same key. ⛔ The machinery was already there and unreachable. The registry has kept browsers by id all along, with a lock per id so two callers racing start one browser rather than two, the configuration remembered so a rebuild is the SAME PERSON with the same seed and the same exit, and tab numbering that does not restart across a rebuild. None of it could be used, because no caller could name an id. So the registry is untouched here: the key is composed instead, from a session and a browser, and everything it already gets right starts applying per browser, which is where it was always needed. Every tool takes `session_id` and `browser_id`, both optional. A client that sends neither behaves exactly as before, which is the promise this slice is built around and the first thing its tests pin. The retry is addressed too. Dropping and rebuilding the default key while the action was aimed at another browser would kill a browser nobody asked about and hand back the wrong one - the same class of mistake as rebuilding from the environment: it succeeds, at the wrong thing. Three existing tests set the registry up by hand under the bare default key and had to move to the composed one. Two failed honestly. The third kept PASSING while it had stopped testing anything: its answer had become "no browser is running yet", and "the password is not in that string" is trivially true of a string that mentions no session at all. It now checks the answer is about the proxied session before asserting the password is absent. The ceiling of eight is declared here as a constant with the measurement that chose it - 61 processes and 6,515 MB for eight live browsers, the eighth taking 13.6 s to start against the first one's 6.8 - but it is not enforced yet. That is the next slice.
The second slice. The first made every tool able to say which browser it meant; this gives a session browsers to mean, and a ceiling that says what it costs. `browser_open` starts another browser in the session and makes it the one unaddressed commands go to, because opening one only to address every following command by hand would make the common case the tedious one. Each browser has its own tabs, its own cookies and its own identity, and shares none of them. `browser_close` frees one, and it FORGETS. That is the distinction the registry already draws between `drop` and `close_all`, now available for one browser: `drop` is recovery, where the identity is kept so the replacement is the same person, while a deliberate close must not let a browser come back wearing an identity its owner shut down. `registry.forget` is the verb that was missing. `browser_list` reports what a session holds without starting anything, because the interface will ask it to draw its panes and a question that starts a browser cannot be asked casually. `browser_focus` moves where the unaddressed commands land; naming a browser still reaches it whatever the focus is. The count is read from the registry's own keys rather than from a list kept beside them. A second list is a second truth: a browser dropped by a failed retry would still be in it, and the ceiling would refuse a slot that is free. ⛔ The ceiling refuses the ninth and says what eight cost - 61 processes, about 6.5 GB, the eighth taking twice as long to start as the first. A refusal that only says "no" invites the next reader to raise the number, and one test reads that sentence for exactly that reason. Three gates caught the new surface and were right to: the expected tool list, the tool table in the docs, and the verb table the interface narrates with - a tool with no verb reads as "Calling browser_open" to whoever is watching.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The first slice of a bigger change: one session is going to hold up to eight browsers, and today nothing can address a second one, because every tool calls
registry.ensure()with no argument and lands on the same key.The design this belongs to is in the workbench, at
docs_research/chat-ui-performance/30-PROGETTO-sessioni-e-otto-browser.md.The machinery was already there, and unreachable
The registry has kept browsers by id all along, and not sketchily:
None of it could be used, because no caller could name an id.
So the registry is untouched by this change. The key is composed instead, from a session and a browser, and everything the registry already gets right starts applying per browser, which is where it was always needed.
What changes
Every tool takes
session_idandbrowser_id, both optional. A client that sends neither behaves exactly as before - that is the promise this slice is built around, and the first thing its tests pin.The retry path is addressed too. Dropping and rebuilding the default key while the action was aimed at another browser would kill a browser nobody asked about and hand back the wrong one: the same class of mistake as rebuilding from the environment, in that it succeeds, at the wrong thing.
Three existing tests, and one that had stopped testing anything
Three tests set the registry up by hand under the bare default key. Two failed honestly when the tools moved to composed keys.
The third kept passing while it no longer tested anything: its answer had become "no browser is running yet", and "the proxy password is not in that string" is trivially true of a string that mentions no session at all. It now checks the answer is about the proxied session before asserting the password is absent.
The ceiling
MAX_BROWSERS_PER_SESSION = 8is declared here with the measurement that chose it - 61 processes and 6,515 MB for eight live browsers, with the eighth taking 13.6 s to start against the first one's 6.8 - but it is not enforced yet. That is the next slice, together with the tools that open and close browsers.Test plan
pytest -q: 369 passed (baseline 361, +8), 3 xfailed unchangedensure();addressed()losing its defaults; one tool losing its address; one tool losing both parametersbrowser_click, so a lost address there is invisible to everything except the test that reads the sourcecheck_english_only.pyandcheck_content.pyclean, both also run with--selftestbrowser_read_htmltrimmed back under the 1024-character truncation thatmcp_tools_to_openaiapplies, so its new note is not cut before the model sees it