fix(server): fall back to the connected tab when a lease cannot launch - #861
fix(server): fall back to the connected tab when a lease cannot launch#861kushals256 wants to merge 6 commits into
Conversation
Chromium missing or acquire throwing used to dead-end reticle_lease even when a human tab was already connected and driveable. Isolation still mints when launch works; the fallback is not a pool lease. Addresses the fallback ask of reticlehq#691. Signed-off-by: Kushal S <skushal.mys@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Every other branch of this check names its evidence. The missing-browser line prints the path it probed — its own comment says why: "naming the path is what turns 'missing' from a verdict into evidence". The mismatch line prints the wanted revision and what the root actually holds. The playwright-absent branch said a bare "the playwright package is not installed" with nothing to check it against. A reporter on Windows installed that exact Playwright version in BOTH the frontend and the repo root, confirmed Chromium sat in the npx cache, and kept reading the same line. They were not wrong and the line was not lying. It means "not installed WHERE THIS PROCESS CAN RESOLVE IT", and a daemon run from npx or a global install resolves from its own location and never from the user's project. With no roots printed there is no way to see that, and the only reading left is that the check is broken — the unbreakable loop this file's header was written to end. The existence filter is what makes it evidence rather than noise. Node's ancestor chain starts with directories nobody ever created: measured from the built daemon the two nearest are `dist/cli/node_modules` and `dist/node_modules`, and the informative one — the daemon's own `node_modules` — is third. Printing the first two unfiltered would name two paths the reader cannot check and hide the one they can, which is the same failure in a new coat. Gathered in the probe rather than the hint, so the hint stays pure: only the probe knows which module did the resolving. Part of #691. Items 1 and 2 of that issue were already done (install-deps for a missing shared library, and the wrong-revision line); item 3 is #861, in review. Signed-off-by: Divyanshu Shekhar <imdshekhar@gmail.com>
|
Reviewed. The fallback is right and I want it in — one change requested first, from a duplicate PR that arrived 11 hours after yours. #871 (@Bhumika-1432006, now closed in your favour) implemented the same ask with a different envelope, and theirs is better on one specific point: // yours
{ "fellBackToExisting": true, "expiresInMs": 0 }
// theirs
{ "noManagedBrowser": { "sessionId": "...", "reason": "Chromium is not installed …" } }Please carry the Two smaller notes, both optional:
Everything else — covering the preflight and the launch, keeping isolation when launch works, and the throttled-tab case — is exactly what #691 asked for. Add the reason and I will merge it. |
Summary
reticle_leasedead-ended when Chromium could not launch (missing binary, acquire throw), even when a human tab was already connected and driveable — including one flagged throttled. Reporters then drove that tab by hand and it worked.reticle_sessionsalready lists a live non-leased tab, acquire now returns that session withfellBackToExisting: trueandexpiresInMs: 0. It is not a pool lease; the HUD stays on that tab. Isolation still mints when launch works.Addresses the fallback ask of #691.
Test plan
acquirethrow with a live tab returns that session instead of "is the app running?"pnpm --filter @reticlehq/server exec tsc -band lease-tools unit tests greenMade with Cursor