Repository navigation
refactor(playground): remove session token - #3965
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe playground no longer uses per-session URL tokens or token-based API authorization. Rust startup and server paths, frontend URL construction, token exports, and WebSocket connection-status handling were simplified accordingly. ChangesPlayground access and client integration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
rossirpaulo
marked this pull request as ready for review
July 10, 2026 19:05
rossirpaulo
enabled auto-merge
July 10, 2026 19:07
Binary size checks passed✅ 7 passed
Generated by |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
typescript2/app-vscode-webview/src/App.tsx (1)
109-110: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWS fallback hardcodes
ws://; derive scheme fromwindow.location.protocol.
remoteUrlsFromLocationinremote-backend.tspickswssforhttps:origins, but this fallback always usesws://. If the standalone/iframe page is ever served over HTTPS, the browser will block the insecurews://connection as mixed content. Align the two paths.♻️ Proposed fix to match protocol-aware scheme selection
- const wsUrl = - window.__PLAYGROUND_WS_URL ?? `ws://${window.location.host}/api/ws`; + const scheme = window.location.protocol === 'https:' ? 'wss' : 'ws'; + const wsUrl = + window.__PLAYGROUND_WS_URL ?? `${scheme}://${window.location.host}/api/ws`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@typescript2/app-vscode-webview/src/App.tsx` around lines 109 - 110, Update the `wsUrl` fallback in `App` to derive the WebSocket scheme from `window.location.protocol`, using `wss` for HTTPS and `ws` otherwise, while preserving the existing host and `/api/ws` path. Keep the explicit `window.__PLAYGROUND_WS_URL` override unchanged and align the logic with `remoteUrlsFromLocation`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@typescript2/app-vscode-webview/src/App.tsx`:
- Around line 109-110: Update the `wsUrl` fallback in `App` to derive the
WebSocket scheme from `window.location.protocol`, using `wss` for HTTPS and `ws`
otherwise, while preserving the existing host and `/api/ws` path. Keep the
explicit `window.__PLAYGROUND_WS_URL` override unchanged and align the logic
with `remoteUrlsFromLocation`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 819738a0-286e-40ca-bcb3-d13625b3129e
⛔ Files ignored due to path filters (2)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_keyword_playground.snapis excluded by!**/*.snap
📒 Files selected for processing (14)
baml_language/crates/baml_builtins2/keyword_docs/baml_keywords.yamlbaml_language/crates/baml_lsp_server/Cargo.tomlbaml_language/crates/baml_lsp_server/src/lib.rsbaml_language/crates/baml_lsp_server/src/playground_sender.rsbaml_language/crates/baml_lsp_server/src/playground_server.rstypescript2/app-vscode-webview/src/App.tsxtypescript2/app-vscode-webview/src/RemoteEditorView.tsxtypescript2/app-vscode-webview/vite.config.tstypescript2/pkg-editor/package.jsontypescript2/pkg-editor/src/index.tstypescript2/pkg-editor/src/playground-token.tstypescript2/pkg-editor/src/remote-backend.tstypescript2/pkg-playground/src/index.tstypescript2/pkg-playground/src/ports/WebSocketRuntimePort.ts
💤 Files with no reviewable changes (8)
- typescript2/pkg-editor/src/playground-token.ts
- baml_language/crates/baml_lsp_server/Cargo.toml
- typescript2/pkg-playground/src/index.ts
- typescript2/pkg-editor/src/index.ts
- typescript2/app-vscode-webview/vite.config.ts
- baml_language/crates/baml_builtins2/keyword_docs/baml_keywords.yaml
- typescript2/pkg-editor/package.json
- typescript2/pkg-playground/src/ports/WebSocketRuntimePort.ts
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Jul 10, 2026
# Conflicts: # baml_language/crates/baml_lsp_server/src/lib.rs # baml_language/crates/baml_lsp_server/src/playground_sender.rs # baml_language/crates/baml_lsp_server/src/playground_server.rs
tsenart
pushed a commit
to perfloop-oss/baml
that referenced
this pull request
Jul 13, 2026
…ment + Phase 1 bounded ingress (BoundaryML#4000) Combines BoundaryML#3969 (Phase 0) and BoundaryML#3976 (Phase 1) into one PR against canary, with latest canary merged in (including the BoundaryML#3965 session-token removal and BoundaryML#3973 test-metadata work, both semantically reconciled). Supersedes both stacked PRs. Implements `docs/design/lsp-latency-and-mutex-fix.md` (committed in this PR) through Phase 1; Phase 2 (salsa snapshot reads) remains behind the doc's spike gate. ## Root cause The 0.14.0 debounced engine rebuild holds the project DB mutex across compiles while `try_lock_db` maps any contention to `-32001`; stale build candidates could install over newer edits, busy diagnostic reads looked empty, and playground/browser routing could leak stale session output. ## Phase 0 — correctness containment (from BoundaryML#3969) - Monotonic `SourceRevision`; owned compilation/diagnostic/engine candidates; **revision-conditional engine commit** through the same gate as source mutations — stale installs impossible by construction (rendezvous-tested at the exact commit boundary). - `DiagnosticsFence` judging staleness against the **authoritative revision**: a superseded rebuild's late diagnostics or a candidate racing `mark_dirty` can never regress newer markers. - Bounded request-lane waits (1s) mapping to typed errors — `ContentModified`/`RequestFailed`; **`-32001` is never emitted**. Busy is never conflated with empty; poison is terminal. - Salsa-memoized `file_annotations`/`semantic_tokens`; 150/300 ms debounced tails; single-flight rebuilds; UTF-8/UTF-16 `PositionCodec` (C1). ## Phase 1 — bounded ingress, cancellation, client gating (from BoundaryML#3976) - **IngressScheduler + LspRuntime** in `baml_lsp_server`: stdio and browser LSP sessions through one bounded scheduler (per-class byte/item budgets, lifecycle barriers, response ownership); `$/cancelRequest` handled on the transport thread — queued requests answer **`-32800`**, running cancellable requests observe a dispatch-scoped token (entry + post-source-guard safe points; never salsa unwinding); browser takeover tombstones the old session's sink and replays overlays. - Hardened framing/output: 16 KiB header cap; oversized messages/responses degrade **per-message** (`-32803`) instead of killing the session/process; serialize-once shared outbound frames with accurate budget accounting; connection-scoped sessions (per-connection encoding negotiation). - **Playground run-gating (D6)**: during rebuild windows (`is_bex_current == false` or a `projectNotReady` rejection) Run/test/preview disable with "Preparing current build…" and re-enable on the next current update — no protocol version bump. Closes the fail-closed D7 UX window Phase 0 introduces. - **Reconnect correctness**: fail-closed hello; bounded first-connect queue; session-scoped commands never replay into a new session (synthesized rejections); the desired-runtime lease survives reconnects. - **Track A**: canonical multi-root ownership (`projectRoots.ts`) + routing; closing the last `.baml` document no longer stops the owner server; aggregate status preserves startup failures. ## Measured (same harness/machine, 400 generated functions) | Metric | 0.14.1 (regressed) | this PR | |---|---:|---:| | Typing inlayHint p50 / p95 | 313.6 / 379.4 ms | **~2 / ~52 ms** | | Post-pause `-32001` failures | 46–57 per feature/window | **0** | | Unanswered requests | 144–160 | **0** | | Rebuild-window inlay/hover/completion | mass failures | 218/218 each | | Explicit `$/cancelRequest` | rejected/ignored | **`-32800`** | | Refresh storms per 10-edit burst | — | 0 | ## Deliberately not included - **Phase 2 salsa snapshots** (removes the residual post-pause hitch on very large projects): gated by the design doc's own spike-and-signoff decision — `panic=abort` release profile vs salsa's unwinding cancellation must be resolved first. - **D5 demand-gating server side**: client seam ships dormant (typed + tested, nothing sends it). - The alternate implementation's five-lock core rewrite (redundant with Phase 0's invariants), playground protocol-v3 lockstep (compat hazard; equivalent wire-compatible fixes included), and stdlib `testing.*` API additions (unauthorized surface). ## Validation - `bex_project` 45/45 (+1 pre-existing ignored) incl. commit-boundary rendezvous, fence, bounded-wait classification; `baml_lsp_server` 64/64 (22 scheduler, 9 runtime, framing/budget); workspace `cargo check --lib` clean; clippy `-D warnings` clean; wasm check clean. - TS: playground 125/125, extension 13/13, webview 19/19 (incl. gating), all typechecks. - Latency harness re-run on this exact merged head (numbers above). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01VEMxti5WQ8bCgJUuX9AQvo <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved LSP responsiveness and stability with bounded messaging, cancellation handling, session recovery, and safer browser takeover behavior. * Added negotiated UTF-8/UTF-16 position handling for more accurate diagnostics, ranges, formatting, and semantic highlighting. * Added project-aware VS Code routing for nested projects, symlinks, and changing project markers. * Added runtime leasing support for playground connections. * **Bug Fixes** * Prevented runs and tests from starting while a project build is stale or unavailable, with clearer preparation feedback. * Improved cancellation, error reporting, diagnostics updates, and run generation consistency. * **Documentation** * Expanded contributor and Rust workspace setup, testing, formatting, and debugging guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Fable 5 <[email protected]>
This branch was previously deployed
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.
Summary
?token=propagation.Why
#3949 introduced the session token alongside the SSH playground changes. We have decided not to use token-based authorization for the local playground, so this removes the feature and all supporting structures rather than leaving compatibility plumbing or dead code behind.
Impact
baml playgroundnow prints and opens a bare local URL. Playground API and WebSocket requests no longer require a query parameter. The origin guard remains in place: remapped loopback origins used by SSH tunnels are accepted, while hostile remote origins are rejected.Validation
cargo fmt --all -- --checkcargo test -p baml_lsp_server --lib— 24 passedcargo test -p baml_cli describe_command_tests— 89 passedpnpm --filter @b/pkg-playground typecheckpnpm --filter @b/pkg-proto generate && pnpm --filter app-vscode-webview buildSummary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation