fix(sandbox): auto-bypass CSB free-tier trust interstitial and guard preload script race#3088
Open
Fizza-Mukhtar wants to merge 2 commits intoonlook-dev:mainfrom
Open
fix(sandbox): auto-bypass CSB free-tier trust interstitial and guard preload script race#3088Fizza-Mukhtar wants to merge 2 commits intoonlook-dev:mainfrom
Fizza-Mukhtar wants to merge 2 commits intoonlook-dev:mainfrom
Conversation
|
@Fizza-Mukhtar is attempting to deploy a commit to the Onlook Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a CSB interstitial auto-click during scraping, prevents starting the Penpal connection while the sandbox preload script is still LOADING, and adds a 3s retry for failed preload script injection attempts. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Server
participant SandboxIframe
participant SandboxManager
participant ParentApp
Browser->>Server: Request project page / scrape
Server-->>SandboxIframe: Respond with iframe URL (+csbFreeActions)
SandboxIframe->>SandboxManager: load → inject preload script
SandboxManager-->>SandboxManager: on inject failure → set NOT_INJECTED, start 3s retry
SandboxManager->>SandboxIframe: (retry) ensurePreloadScriptExists → inject preload
SandboxIframe->>ParentApp: preload script loaded → signal READY
ParentApp->>ParentApp: check PreloadScriptState
ParentApp-->>ParentApp: if LOADING → log & skip Penpal setup
ParentApp->>SandboxIframe: when READY → establish Penpal connection
ParentApp->>SandboxIframe: Penpal handshake completes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/client/src/components/store/editor/sandbox/index.ts`:
- Around line 113-119: The setTimeout started in the constructor/initializer can
fire after the manager is disposed — store its ID (e.g., a new field like
preloadRetryTimeoutId) when calling setTimeout around the block that checks
this.preloadScriptState and calls void this.ensurePreloadScriptExists(), and
then clear that timeout in the class clear() method (use
clearTimeout(this.preloadRetryTimeoutId) and null the field) so the retry
callback cannot run against a cleared SandboxManager.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e83d5184-e326-45f8-a64e-263b4db1768d
📒 Files selected for processing (3)
apps/web/client/src/app/project/[id]/_components/canvas/frame/view.tsxapps/web/client/src/components/store/editor/sandbox/index.tsapps/web/client/src/server/api/routers/project/project.ts
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
1 participant
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.
Description
Two separate issues were causing the Penpal connection to time out when using
Onlook with a free-tier CodeSandbox account:
The CSB free-tier trust interstitial ("You are opening a CodeSandbox preview,
do you want to continue?") was never auto-dismissed. The code to click
#btn-answer-yesexisted inproject.tsbut was commented out, leaving theiframe stuck on the interstitial page so the Penpal handshake never started.
A race condition in preload script injection — if
ensurePreloadScriptExists()failed on first attempt (sandbox not ready yet), there was no retry and
preloadScriptStatestayedNOT_INJECTEDpermanently. The iframe would loadbefore the preload script was ready, so the child-side Penpal connect never fired.
Changes made:
project.ts— Uncommented and activatedcsbFreeActions, passing{ type: 'click', selector: '#btn-answer-yes' }asactionsto FirecrawlscrapeUrlso the trust interstitial is auto-dismissed on screenshot capture.sandbox/index.ts— On preload script injection failure, schedules a singleretry after 3s to handle the race where the sandbox is not yet fully ready on
first attempt.
view.tsx— Added a guard insetupPenpalConnectionthat skips the connectionattempt if
activeSandbox.preloadScriptState === LOADING, waiting for the nextonLoadevent instead of connecting too early.Related Issues
Fixes #3087
Type of Change
Testing
Penpal connection setappears in browser console with no timeout errorScreenshots (if applicable)
N/A — bug manifested as a console timeout error, not a visual UI change.
Additional Notes
The
csbFreeActionsfix only affects thecaptureScreenshotmutation which usesFirecrawl to scrape the preview URL. The preload script retry and LOADING guard
together fix the deeper race condition that caused timeouts even after manually
dismissing the interstitial.
Summary by CodeRabbit