Make agent-browser launches eager and load-independent - #537
Conversation
Deploying mouseterm with
|
| Latest commit: |
a82c709
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://22dce75a.mouseterm.pages.dev |
| Branch Preview URL: | https://agent-browser.mouseterm.pages.dev |
dormouse-bot
left a comment
There was a problem hiding this comment.
Three findings, all inline. The eager-swap rollback paths in Wall.tsx (visible / minimized / killed) and the controller's new single-flight relaunch guard all check out against the tests; the first finding is that the host has no equivalent guard now that relaunch returns before open does.
dormouse-bot
left a comment
There was a problem hiding this comment.
The three fixes check out against their tests — the generation guard covers the relaunch-supersedes-relaunch case the last review raised, the zero-exit message distinguishes the two wsPort === undefined outcomes, and the unconditional session arm reconciles at an already-live port. One finding inline: the generation is bumped only by popOut/popIn, so the other way a session's daemon gap opens — an explicit close from the webview — leaves the previous relaunch's sweep armed.
Minor, same guard: closeStrayBlankTabs's current parameter defaults to () => true, so a second call site opts out of the invalidation check by omitting an argument. Making it required would need binaryPath reordered (a required parameter can't follow an optional one), so it's your call whether it's worth the churn for a one-caller function.
dormouse-bot
left a comment
There was a problem hiding this comment.
The command('close') fix reads correctly — the delete is synchronous, ahead of the close spawn, so a sweep left by a fast-returning relaunch is invalidated before the close can queue behind open; and making current required closes the omit-the-argument hole. One finding inline: closePoppedOut is the third path that closes a session's browser without touching relaunchGenerations.
dormouse-bot
left a comment
There was a problem hiding this comment.
relaunchGenerations.clear() lands synchronously ahead of the close spawns, so every pending sweep's current() is already false when the shutdown close releases its open — and both hosts (deactivate() in vscode-ext/src/extension.ts, shutdown() in standalone/sidecar/main.js) call closePoppedOut only at teardown, so the blanket clear() can't cancel a sweep that still matters. The regression test drives exactly that release-on-second-close path. One finding inline, on the spec sentence rather than the code.
Summary
Verification