Repository navigation
feat(web): make each web session its own device - #4191
kacper-mikolajczak wants to merge 56 commits into
Conversation
Network dump, audio probe, and recording answer from the request's own provider through resolveRequestScopedWebProvider, next to the interactor's host fallback.
The backend trims the session name, so `a ` and `a` drove one browser as two devices. The device id now reads the same normalizer as the provider, and a name that is empty or holds `:` is refused with web_session_name_invalid instead of parsing as another device's infix.
Only open passed the session name to device selection, so a fresh web batch or replay locked the shared host device for its whole run and serialized against every other web session. Lock-plan resolution now names the session for every command (ADR 0035 rule 3).
…arget A caller with no web session now says so, instead of silently resolving the host device.
…mirrors ADR 0035 The web help topic names the per-session device and the session-name rule. Comments that cited ADR 0035 for web session devices now say the web rule follows the app device rule.
Moves the fresh web batch and replay parallelism test into its own file and folds the web help session line into the closing rule.
|
@kacper-mikolajczak is attempting to deploy a commit to the Callstack Team on Vercel. A member of the Team first needs to authorize it. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 32 files
Reply to a comment to ask cubic a question or push back. It learns from your replies.
View guided diff | Re-trigger cubic
Network dump, audio probe, and recording now resolve through resolveRequestScopedWebProvider, so resolveWebProvider and hasScopedWebProvider have no consumer outside the package. The provider scope test reads the scoped provider through the same function.
|
At b91428c an empty session name can reach the default browser, a concurrent close can lose the record of another open browser, two P2 threads still apply, and the live run predates the latest changes. An HTTP open request with an explicit empty session name falls back to default before web validation, and can navigate the default browser. Please validate the supplied name before fallback or session reuse on every web entry route, and add a router/RPC regression for empty input. Session open Could a close clear the process record while another session is opening? The new browser can record its processes before publication, while the closing session sees no published siblings and clears the record. If the daemon then exits abnormally, orphan recovery cannot find the surviving browser. Please serialize fleet record updates under one platform-web owner, derive survivors from fleet state including pending opens, and add a controlled overlapping close/open regression. This interleaving follows from source ordering and has not been reproduced live. Provider The supplied live check predates the later changes, which the PR says were not live-verified. Please build and run the fixed head and record its SHA, commands and outputs for concurrent named opens and fresh batches and replays, distinct URL and network results, audio probes and recordings from each session, typed invalid-name refusal, cross-daemon same-name refusal, close and idle expiry with the sibling still usable, complete claim and process cleanup, and a tenant-isolated leased open and release. The earlier report has no command or output artifacts. Request binding The tenant-name thread and host-claim thread still apply, as do two lower-priority threads. The shared-browser thread does not establish a shared-browser race because web requests compose the requested browser name and local discovery excludes web. Could selection produce one validated public name, normalized backend key and derived device for preflight, binding and provider composition, and could hostDeviceIdOf and isHostScopedDeviceOf cover claims and first-open synchronization? This first needs a defined public-name versus tenant/backend-key boundary and generic host-scoped-device invariant, amended decisions, replacement of superseded hostApp-only checks, and fleet ownership that includes pending opens. Not blocking: These can be taken or left: record the web host/session ownership and tenant/backend naming boundary in the owning decisions, and use a typed shared WebProvider fixture and typed query and dump inputs in the new tests. The only failing check is Vercel, which requests Callstack team authorization for the external contributor; the changed documentation feeds the site, but no failure log establishes a build or runtime failure. No conflicts are known. Before merge, resolve the applicable tenant-name and host-claim threads and the two defects, align advisory and bound target identity, add production-route regressions, supply the head-specific live evidence, and clear Vercel authorization. |
…efault browser An open with an empty session name fell back to `default` before web validation, so it could drive the default browser, or reopen the default web session. Every web device derivation now reads the supplied name, and a reopen of an existing web session validates it too, so the typed web_session_name_invalid refusal covers both.
…process record A close derived the surviving sessions from the daemon's published sessions only. A browser whose open was in flight, or had finished before its session was published, was missed, so the close cleared the owned-process record while that browser ran and orphan recovery could not find it after an abnormal daemon exit. One platform-web fleet, keyed by the record store, now tracks browsers from the start of their open until their close and serializes every record update (command finalization and orphan cleanup), so a stale scan cannot overwrite a newer one. Survivors come from the published sessions plus that fleet. Teardown closes pass the record store so they leave the fleet too.
… of hanging The parallel-sessions test made each session wait at bind for the other, so one session failing before it bound left the other waiting forever and the test ended on the Vitest timeout. A request that settles now releases that wait, and the failure reaches the response assertions.
The web help said only that a name must not contain ":", but a name that is empty after trimming is refused too (web_session_name_invalid). Help and the commands page now state the same rule.
The new web tests built providers with `as never` casts and passed untyped probe and dump inputs, so a change to WebProvider could not break them at compile time. Each tree now builds its fakes with one typed makeWebProvider, and the inputs take the contract types. test-kit cannot own the fixture (it may not import platform packages) and platform-web may not publish one in its exports, so platform-web keeps it in its test utilities and the root tests in a beside-the-module fixture file.
A web session name was refused only when empty or holding ":", but agent-browser accepts only letters, digits, "_" and "-" in a session name and refuses one whose socket path exceeds 103 bytes, so any other name failed late, inside the backend. requireWebSessionName now states the whole rule, with the length limit computed from the socket path layout, and refuses with web_session_name_invalid. Help and the commands page state the same rule.
…er tenant The web session name was derived separately in device selection, the lock plan, the provider composition, and teardown, and each site read the tenant-prefixed store name, so a tenant open parsed ":" as a device id infix and was refused. Two tenants using one name also could not hold separate browsers. Device selection now derives the session device once from the tenant-relative public name and the tenant id: its id is agent-browser-chrome:session:<key> with key p_<name>, or t_<hash12 of the tenant>_<name> under tenant isolation. The provider and teardown read the key back from the device id (webBackendKeyOf) and the provider's second normalizer is gone. The name limit now leaves room for the key prefix.
The limit is read only inside the kernel, so only the kernel test exports it. The test states the two name bounds (30 and 17) as numbers instead.
…host A web command with no session bound the shared host device while its lock plan named the session device, so a sessionless command and a sessionless record start could run beside the session they overlap, and the provider had no session to key a browser on. The device-selection input is now the request's web session (name and tenant) at every site that resolves a device without a session: resolveCommandDevice and its handlers, the snapshot and selector backends, record start and its recovery, the bounded claim device, and the provider context. Without --session the name is the implicit session name. The host stays an inventory entry and the parent for devicesOverlap. webSessionIdentity moves to core, next to the tenant naming rule, so the lower layers read it.
The claim store and inspection checked ADR 0035 rule 4 only for a device with hostApp on a macOS host, so a web session device was claimed as a standalone device: a live host claim did not refuse a session claim, and a host acquisition ignored live session claims. withAcquisitionLocks, claimHeldDevice, inspectDeviceClaimAcquisition, and the devices claimedBy projection now read the host from the device id (hostDeviceIdOf) and match a host's claims with isHostScopedDeviceOf, for any host that can hold such devices (macOS and web). macOS behavior is unchanged.
The first-open lock keyed by the host for an app device and by the device otherwise, which the inline expression and its comment did not say for web. firstSessionOpenLockKey states it: a web host has no whole-host open, so a web session device keys by itself and the sessions of one host open in parallel. Behavior is unchanged.
… the tenant key ADR 0035 no longer applies to macOS only: it defines host-scoped devices, with macOS app devices and web session devices as the two kinds. Rule 4 now states the claim rule through hostDeviceIdOf and isHostScopedDeviceOf. New rules and reasons: a web session is a device and the web host is never bound; the public name rule matches the backend; the backend key is p_<name> or t_<hash12>_<name>, derived once in the kernel and read back from the device id; web first opens key by the session device. Rejected alternatives and open gaps (implicit sessions share `default`, web admitted under non-web leases, mixed-version daemons) are recorded. ADR 0029 states that device allow lists admit web session devices like app devices. ADR 0007 owns tenancy, so it records the tenant boundary: the key derives from the tenant-relative name and the tenant id, never from the store name.
Boot selection resolves through the session device resolver, which now requires the web session identity; the boot handler did not pass it, so typecheck failed.
There was a problem hiding this comment.
All reported issues were addressed across 61 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 62 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 4 unresolved issues already reported by Cubic.
View guided diff | Re-trigger cubic
scopeRequestSession rewrote a blank session to default before web validation, so a tenant's blank name drove its default browser. It now keeps the supplied name for webSessionIdentity.
… open The supplied session name now travels with every scoped request and into batch steps, so a blank name reaches web validation instead of becoming the default session. Adds batch, replay and HTTP RPC open tests.
A close that returned a failure no longer keeps its session in the fleet, which blocked orphan reaping for the daemon's lifetime.
…decides When every recorded process is dead, the launched sessions of the fleet are dropped under the record lock, so a browser that exited unexpectedly no longer blocks reaping for the daemon's lifetime.
The fleet names a sessionless provider '', so the reap no longer filters it out.
ADR 0035 keeps the generic host-scoped device rules with an explicit scope-change line; ADR 0037 holds the web decisions, rejected alternatives, and open gaps. Web-specific citations now point at 0037.
An open joins the fleet before its provider-startup cleanup, so the cleanup saw its own session and skipped the reap on every open. The starting session has spawned nothing yet and no longer blocks the reap; other fleet sessions still do.
openWebSessionNames reads the backend key from a session device id, and the web host is never bound, so the test now publishes the session's own device.
The old name exceeded the 30-character web session name limit.
The default devices listing excludes web, so the host never appeared.
The mock returns the host unless the request names a web session, and the start result must be ok.
The expected device id no longer comes from the production key derivation.
Spy on the gateway bind and require it is never called.
The request carries no session, so it takes the implicit default session device rather than the host.
A conflict on any other claim no longer passes.
The name is also shortened to fit the web session name limit.
An opening fleet session skips the reap, and an open's own startingSession does not.
There was a problem hiding this comment.
All reported issues were addressed across 28 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
The record-dead check returned true when the probe merely did not answer, so a transient ps failure could evict a launched fleet. A recorded process is now observed alive, observed not alive, or unobserved, and only all-not-alive reads as dead.
…admission Trimming at admission changed other platforms' session keys and threw on a request without a session. Admission no longer trims; the web name rule refuses surrounding whitespace with web_session_name_invalid.
Run-One and run-one are one device since names are lower-cased.
An explicit empty or whitespace-only session name is refused with web_session_name_invalid and never falls back to default. Surrounding whitespace is refused, not trimmed.
The batch and replay cases asserted only ok:false, which passes for any failure. They now assert web_session_name_invalid, with REPLAY_DIVERGENCE as the replay wrapper code.
A launched session whose recorded browser exited is evicted and the cleanup reaps; an unanswered probe evicts nothing and skips.
The record-start fixture bound the harness device instead of the web session device, the RPC open test read a JSON-RPC error as a result, and the provider test missed startingSession.
|
Updated at
Open questions:
Live evidence at this head will follow in a separate comment. |
There was a problem hiding this comment.
All reported issues were addressed across 13 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
View guided diff | Re-trigger cubic
The ASCII name pattern ran after toLowerCase, so characters that fold to ASCII, such as the Kelvin sign U+212A, passed as k. Validate the name as given, then fold it.
Web session names are no longer trimmed; a name with surrounding whitespace is refused with web_session_name_invalid, so the stale equality with the trimmed name failed.
|
At 230c335, the code fixes from the earlier review (#4191 (comment)) look right from reading. Blank, padded and case-variant names are refused, the supplied session name carries into batch and replay steps, and the inline thread fixes are in. I read the code and did not run it. CI is green (13 checks), and there are no conflicts. The one remaining blocker is a live run. The only live run so far is at a0b97c3. Since then every device-facing route changed: per-command session-device binding, the name rule, the tenant backend key, host-scoped claims and the platform-web fleet record (see request-binding.ts). Unit fakes stand in for agent-browser and the process table, so a wrong binding, a lost process record or a leaked browser would only show on a real host. Please build 230c335 or later and post the SHA, commands and outputs for:
Could the web identity be derived once in Not blocking, take or leave: a router On your two questions: keeping implicit sessions shared as No P1 or P2 inline thread still applies at this head; the remaining open threads are fixed or no longer apply and can be resolved. |
Summary
Each web session is now its own device,
agent-browser-chrome:session:<name>, so sessions with different--sessionnames run in parallel on one host. Device selection derives the id before any lock. The existing claim, conflict, anddevice:<id>lock rules therefore apply unchanged, as with the app devices of ADR 0035.batchorreplaylocks its own session device.:is refused withweb_session_name_invalid.resolveRequestScopedWebProvider).devices --platform webno longer reportsclaimedBy.33 files, +640/−94. Closes #4190.
Validation
b91428c7e. CI validates this head. Local gates were not rerun aftera0b97c3e5.a0b97c3e5, using a CLI built from this branch: two parallel sessions each snapshot their own URL and network traffic. Reusing a name from a second daemon getsDEVICE_IN_USE. Closing one session leaves the other working. Claims return to 0.a0b97c3e5found one medium and four low issues.f930dfa0a..c6426aab0address them, andb91428c7eremoves exports they left unused. Those commits are not live-verified.defaultsessions in different workspaces share one browser and conflict. One possible race is unverified: a close that runs while another session's open is still in flight may clear the owned-process record.