Conversation
Proof-of-work snapshot: WorkOS browser sessions, DeviceRoom RPC, shared UI parity and friendly device names. Pins published GPUI runtime fixes. Excludes local credentials, generated artifacts and experimental apps. Locked WASM build and scoped tests passed; exact native build remains unverified after validation timeouts. Not PR-ready.
|
Pushed 08fcd03: production deployment preparation and multiuser authentication. Keeping this PR in draft. Known unresolved issue from manual phone testing: emojis do not render correctly in chat messages or when pasted into the input. This is a rendering failure, not a passed emoji check; root cause has not yet been investigated. Previously verified taps, scrolling, keyboard behavior and popovers remain recorded separately. |
Resolve UI integration conflicts, pin published browser runtime fixes, refresh web lockfiles, harden workerd test execution, and correct rollout documentation.
|
Updated to The emoji issue mentioned in my earlier comment is fixed in staging. I’ve also manually verified terminal input/focus, mobile drawers, reconnect behavior and the mobile tap-highlight fix. This update resolves the upstream conflicts and pins the published runtime fixes, so the web build no longer depends on local runtime overrides. Local validation passed: locked release WASM build, 21 lifecycle tests, 44 unit tests, 24 workerd tests and 39 production-routing checks. The workerd runner assertions were investigated and addressed; repeated runs are now clean. Desktop/mobile browser startup checks also passed. Please land zui #8 first; I’ll then repin to its upstream revision. Hosted CI still needs confirmation. Production has not been deployed, and the WorkOS/storage rollout checklist remains a pre-merge requirement. |
wingleeio
left a comment
There was a problem hiding this comment.
Thanks for the thorough work here — the design is sound and the security posture (PKCE + state/nonce, __Host- cookies, CSRF header + Origin checks, sealed provider tokens, owner-scoped sessions, browser forced to role=client, catalog-only preview relay) is careful. I audited the vendored crate, the forked git dependency, CI/deploy changes, the postinstall hook, and the edge auth code, and found nothing unsafe. Findings below, ordered by importance.
Blocking / should fix before merge
-
Pin mismatch between description, README, and manifest. The PR body says zui is pinned at
c1700b8d…,apps/web/README.mdanddocs/web-deployment-session-summary.mdsayaca6b042…, andapps/web/Cargo.toml/Cargo.lockactually pin8d78b9c2…. Per your own checklist, zui #8 should land first and this should be repinned to azeronsh/zuirev, with the[patch."https://github.com/zeronsh/zui"]block removed. As written,deploy.ymlbuilds productionweb.zeron.shassets from a personal fork. -
Per-frame authorization round-trip to a single global DO (
edge/src/device-room.tswebSocketMessage→browserSessionActive→validateBrowserSession). Every relayed frame in both directions for a browser client does a sub-request to the onebrowser-sessions-v1BrowserSessionStoreinstance, withrefresh: trueby default (so a frame path can trigger a WorkOS refresh). Terminal output is many frames/sec per client; this makes one DO a global throughput ceiling and adds latency to every frame. There's also a correctness concern:webSocketMessageis nowasyncand awaits a non-storage sub-request, so workerd can deliver the next message before the previous one finishes validating — frames on a stream could be relayed out of order. The alarm-based recheck (BROWSER_SESSION_RECHECK_MS) plus the/browser-revokepush already cover revocation; I'd drop the per-frame check or make it a local, time-bounded cache on the socket attachment. -
Contributor-specific infra committed to the repo.
edge/wrangler.staging.jsonchardcodes a personal Cloudflareaccount_id, thezeron-test.embedez.comcustom domain, and a separate WorkOS client id;docs/web-deployment-session-summary.mdis a session log of that staging deploy. Either template the staging config or drop it and the session summary. -
/auth/browser/*broker API is dead code for this PR.edge/src/browser-auth.tsexposes exchange/refresh/revoke returning raw provider access+refresh tokens to any holder ofBROWSER_BROKER_TOKEN. Nothing in the browser flow uses it,BROWSER_BROKER_TOKENisn't inEnvor wrangler config, and its only consumer is the "not implemented"SELF_HOSTING_ROADMAP.md. It's fail-closed (501) when unset, but it's a new credential-issuing surface plus a===timing-unsafe token compare. Suggest removing it (and the roadmap doc) from this PR and landing it with the self-hosting work.
Should fix
-
forward()inedge/src/index.tsdoesn't strip the new internal headers. It clearsROOM_KIND_HEADERfor exactly this reason but passes throughx-comet-browser-session-hash,x-comet-browser-room, andx-comet-browser-session-storefrom native bearer clients. DeviceRoom re-validates the hash and matches owner so I couldn't find an escalation, but the/browser-revokeroute's only gate is that store header. Add the three to the delete list for defense in depth. -
Host device name is now sent to and stored on the edge (
crates/engine/src/lib.rs.with_device_name(local_device_name(...))→&name=→browser_devices.name). That's the machine hostname by default. Worth a note in the description and probably an opt-out, since it's a new piece of PII in DO storage. -
postinstallpatchingnode_modules(edge/scripts/patch-vitest-pool.mjs) will breaknpm cion any@cloudflare/vitest-pool-workersbump. Preferpatch-package/ npmoverrides, or upstream the fix and drop the hook.
Scope
- This PR bundles at least six independently reviewable changes: the
RpcClientwriter/shutdown refactor (affects native too —ServerFramenow rejects envelopes without exactly one ofok/item/err/done; I checkedserver.rsand it's compatible), large UI module splits (composer/input.rs,*/presentation.rs— confirmed these are essentially moves), the alacritty vendoring (verified byte-identical to crates.io 0.26.0 apart from wasmcfggating), thezeron-rpc/zeron-syntaxfeature-gating, the web app, and the edge BFF + deploy changes. Splitting the native-affecting refactors out first would make the remaining browser work much easier to review and bisect.
Nits
dtolnay/rust-toolchain@masterin both workflows; the repo otherwise uses@stable.browser-sessions.tsswallowsALTER TABLEerrors with barecatch {}on every DO construction — a migration table would be clearer than try/catch schema evolution.web-validation.ymlanddeploy.ymlduplicate the ~40-line WASM build/check block; a composite action would keep them in sync.
|
Good morning — here is my attempt to un-slop this PR and address the review properly.
I also changed the Rust setup actions to use the stable action tag and replaced exception-driven SQL column migrations with explicit schema inspection. The final upstream zui repin remains required after zui #8 merges. The PR also currently conflicts with upstream |
Summary
Adds an authenticated browser build of the shared GPUI interface. The browser renders the shared Shell/application state and connects to a native Zeron device through the existing RPC protocol and DeviceRoom relay. Execution stays on the native host; the browser does not run an engine or use loopback IPC.
Included
Merge-readiness sweep
7655c662c2e0e1dea91a9c4d9929725e219abd82, resolved the root lock conflict and reviewed the preview/sidebar auto-merges.Gratenes/zui@c1700b8de27a07b5bc1a190440b64e7323c008f9, including the input/runtime fixes from Fix browser runtime wakes, fonts, touch and text input zui#8.Validation
trunk build --release --locked: passed, usingnightly-2026-09-08, Trunk 0.21.14 and the published runtime pin; no local Cargo source override.npm ciin the clean checkout (44 unit + 23 workerd).cargo check --locked -p zeron: passed after reconciliation, using locally supplied Linux development libraries.Commands/configuration:
apps/web/README.md.Hosted checks: Web validation workflow.
Manual coverage and limits
The author has verified phone taps, scrolling, keyboard behavior and popovers; phone copy/paste and emoji testing remains pending. Device/browser versions and the tested revision were not recorded.
Earlier development checks reported real WorkOS login, device selection, chat, terminal commands, file editing, Git diffs, queue operations, previews and theme persistence. Those reports are not a reproducible end-to-end suite for this final commit; this sweep does not claim to repeat those authenticated flows.
Remote project preview is HTTP-only: WebSocket upgrades and Vite HMR are not supported. Full desktop/browser parity, one-command deployment and direct/self-hosted transport are outside this change;
SELF_HOSTING_ROADMAP.mdremains a proposal.Before merge
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.