feat: added browser to sidebar

[ErikBjare]

Wanted to make something Lovable-lite, this browser sidebar preview is a first step (still needs actual integration with agent, like letting it set a preview URL and read the logs).

---

Important

Add BrowserPreview component to sidebar for URL preview and console log capture, with updates to RightSidebar and a new console proxy script.

• Components:
  • Add BrowserPreview component in BrowserPreview.tsx for URL preview and console log display.
  • Update RightSidebar.tsx to include a new "Browser" tab with BrowserPreview.
• Functionality:
  • BrowserPreview allows URL input, refresh, mobile/desktop toggle, and console visibility toggle.
  • Captures console logs from iframe using consoleProxyScript in consoleProxy.ts.
  • Logs are cleared on URL change or refresh.
• Scripts:
  • consoleProxyScript captures and forwards console logs from iframe to parent window.

^{This description was created by for 3095a2d. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to 414d432 in 2 minutes and 37 seconds. Click for details.

• Reviewed 289 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 5 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/components/BrowserPreview.tsx:63

• Draft comment:
  Ensure that using new Function to inject the console proxy script is safe for your use-case; double-check origin and script trust.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to double-check the safety of using new Function, which violates the rule against asking the author to confirm or double-check things. It doesn't provide a specific suggestion or point out a specific issue with the code.

2. src/components/BrowserPreview.tsx:62

• Draft comment:
  Injection via new Function(consoleProxyScript) bypasses CSP and could pose security risks. Consider directly injecting the script text instead.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a legitimate security concern - using new Function() is similar to eval() and can bypass Content Security Policy protections. However, this is in a development/debugging tool (BrowserPreview component) that's injecting a console proxy script, likely only used in development. The script content comes from an imported constant, not user input. The suggested alternative of direct script injection wouldn't meaningfully improve security in this context. I could be underestimating the security risk. Even in development tools, security best practices should be followed. The code could potentially be used in other contexts where CSP matters more. While security best practices are important, this is clearly a development-only component (evidenced by defaultUrl being localhost:8080) where CSP bypass isn't a meaningful risk. The console proxy script is from a trusted source. The comment raises a technically correct but practically insignificant concern given the development-only nature of this component. The suggested change wouldn't meaningfully improve security.

3. src/components/BrowserPreview.tsx:40

• Draft comment:
  The message event listener does not validate event.origin. Validate the origin to ensure only trusted messages update the logs.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a legitimate security concern. When using postMessage, it's a best practice to validate the origin to prevent malicious sites from injecting fake console messages. However, in this specific case, the component is designed to display console logs from an arbitrary URL that the user can input, so origin validation may actually be intentionally omitted to allow this functionality. The risk is also relatively low since it only affects console message display. I might be underestimating the security risk. Even if the component is meant to work with arbitrary URLs, allowing any origin to inject messages could potentially be used for XSS if the log rendering isn't properly sanitized. While the security concern is valid, the component's purpose is specifically to display console logs from user-specified URLs, so origin validation would break the intended functionality. The risk is mitigated by only accepting messages of type 'console' and proper escaping in the render logic. The comment should be deleted because adding origin validation would break the component's core functionality of displaying console logs from arbitrary URLs.

4. src/utils/consoleProxy.ts:38

• Draft comment:
  Using '*' as the targetOrigin in window.parent.postMessage may expose messages to unintended recipients. Specify a strict target origin if possible.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a legitimate security concern - using '' allows any window to receive these messages. However, this is a console proxy for debugging purposes, and the messages only contain console.log data that would be visible in the browser's console anyway. The risk is minimal since no sensitive data is being transmitted. Additionally, this is likely intentional to allow flexibility in development environments where origins may change. I might be underestimating the security risk - what if error messages contain sensitive information? Also, best practices generally discourage using '' in postMessage. While using '' isn't ideal, the practical risk is very low here since this is debug tooling and the data would be visible in the console anyway. The flexibility benefit outweighs the minimal security risk. The comment should be removed. While technically correct, the security risk is minimal for this debug tooling use case, and the flexibility of '' is likely intentional.

5. src/components/RightSidebar.tsx:27

• Draft comment:
  The dynamic sidebar width logic is somewhat complex. Consider extracting this logic for better readability and maintainability.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

Workflow ID: wflow_HIufEF8nhLUAEx0k

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Consider adding a sandbox attribute to the iframe to mitigate risks when loading arbitrary URLs.

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed 3095a2d in 1 minute and 49 seconds. Click for details.

• Reviewed 93 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 6 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/components/BrowserPreview.tsx:65

• Draft comment:
  Consider adding error handling for the iframe content manipulation here. If consoleProxyScript or the DOM manipulation fails, it may crash the iframe load.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The code does have potential failure points and error handling could be valuable. However, the main change in this diff is just switching from document.createElement to iframe.contentWindow.document.createElement. The error handling suggestion is more about the overall structure that existed before. We should focus on comments about what changed. The error handling suggestion is valid from a code quality perspective. Missing error handling could cause silent failures or crashes. While error handling would be good, this comment is about pre-existing code structure rather than the specific changes made in this diff. The change itself (using iframe.contentWindow.document.createElement) is an improvement for correctness. Delete this comment since it's not specifically about the changes made in this diff, but rather about pre-existing code structure.

2. src/components/BrowserPreview.tsx:69

• Draft comment:
  Using new Function for script injection may face CSP issues; consider if a safer alternative or validation is needed.
• Reason this comment was not posted:
  Comment was on unchanged code.

3. src/components/BrowserPreview.tsx:91

• Draft comment:
  The onKeyDown handler triggers URL refresh on Enter; ensure the URL is validated to prevent potential issues with malformed or unsafe URLs.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While URL validation is generally good practice, several factors make this comment less critical: 1) This appears to be an internal dev tool (localhost default) 2) Browsers have built-in URL validation and sanitization for iframes 3) The sandbox attribute is already commented out, suggesting security was considered 4) No clear evidence of actual vulnerability. The comment is more speculative than pointing out a definite issue. I could be underestimating the security implications. Malformed URLs could potentially cause crashes or unexpected behavior even with browser protections. While URL validation wouldn't hurt, there's no strong evidence that lack of validation would cause actual problems here. The browser's built-in protections are likely sufficient for this dev tool use case. Delete the comment as it's speculative ("potential issues") rather than pointing out a definite problem, and browser security features already provide basic URL protection.

4. src/components/BrowserPreview.tsx:28

• Draft comment:
  Logs are being cleared both in the useEffect on currentUrl changes and again in handleRefresh. Consider consolidating this behavior for clarity.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

5. src/components/BrowserPreview.tsx:69

• Draft comment:
  Using the Function constructor to inject consoleProxyScript can be risky. Ensure that consoleProxyScript is a trusted, static value to avoid injection vulnerabilities.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

6. src/components/BrowserPreview.tsx:129

• Draft comment:
  The iframe's sandbox attribute is commented out. Consider enabling it (or revisiting its configuration) to improve security when loading external content.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_OoGs9IbTazTRJPtR

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}