fix(chat): handle HTML artifacts and browser overlays - #279
Merged
Conversation
alwaysmavs
marked this pull request as ready for review
July 28, 2026 08:28
WalkthroughThe change adds browser preview capture and modal-aware native view visibility, including preview rendering, ordered show/hide operations, layout observation, and drag-region styling. It adds local filesystem path detection with native clipboard and file-opening actions, localized text, error handling, and tests. HTML files are excluded from intermediate-code processing and standalone HTML reports are verified as web-page artifacts. Sequence Diagram(s)sequenceDiagram
participant RendererModal
participant BrowserPanel
participant BrowserService
participant BrowserManager
RendererModal->>BrowserPanel: aria-modal changes
BrowserPanel->>BrowserService: hide or show browser view
BrowserPanel->>BrowserService: capturePreview(sessionId)
BrowserService->>BrowserManager: capturePreview(sessionId)
BrowserManager-->>BrowserPanel: PNG preview data URL or null
BrowserPanel-->>RendererModal: render preview beneath backdrop
sequenceDiagram
participant MessageLinkSafetyModal
participant NativeClipboard
participant ChatService
MessageLinkSafetyModal->>MessageLinkSafetyModal: derive local path
MessageLinkSafetyModal->>NativeClipboard: writeClipboardText(localPath)
MessageLinkSafetyModal->>ChatService: invoke openLocalPath
ChatService-->>MessageLinkSafetyModal: success or error
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches✨ Simplify code
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: 3
🤖 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.
Inline comments:
In `@src/components/ai-elements/message-streamdown.tsx`:
- Around line 235-243: Guard the openLink flow with a pending state so repeated
activations cannot invoke chatService.invoke("openLocalPath", ...) while the
native request is awaiting. Update the related button state at the controls
around the open action to disable it during the request, and reliably clear the
pending state when the operation completes or fails while preserving the
existing onClose behavior.
In `@src/routes/Chat/BrowserPanel.tsx`:
- Around line 39-41: Update the preview capture flow around capturePreview so
every completed request sets preview state to its returned value, including
null, and track request ordering to ignore responses from superseded captures.
Ensure only the latest capture can update setPreviewDataUrl, preventing stale
screenshots from reappearing.
- Around line 68-114: Serialize native-view visibility transitions in the
BrowserPanel effect by tracking the latest desired visibility and invalidating
stale show/hide operations, ensuring a show that resolves after an overlay opens
or unmount begins cannot override the subsequent hide. Update BrowserPanel.tsx
lines 68-114 accordingly. Add a delayed-show regression test in
BrowserPanel.test.ts lines 72-105 that opens the overlay before show resolves
and verifies the final native-view action is hide.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d4a34b9-0ae0-4e4f-93cd-f0b3253fdfad
📒 Files selected for processing (13)
docs/ai/integrated-browser-implementation.mdelectron/browser/common.tselectron/browser/node.tselectron/chat/node.test.tselectron/chat/turn-output-files.tssrc/components/ai-elements/message-streamdown.test.tssrc/components/ai-elements/message-streamdown.tsxsrc/i18n/app-messages.en.tssrc/i18n/app-messages.zh.tssrc/lib/message-link-target.test.tssrc/lib/message-link-target.tssrc/routes/Chat/BrowserPanel.test.tssrc/routes/Chat/BrowserPanel.tsx
💤 Files with no reviewable changes (1)
- electron/chat/turn-output-files.ts
alwaysmavs
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/routes/Chat/BrowserPanel.test.ts (1)
108-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
getBoundingClientRectmock literal.This exact bounds object is now duplicated three times in the file (pre-existing test at Lines 73-83, here, and again at Lines 159-169). Extracting a shared helper/constant would prevent drift if the mocked bounds ever need to change.
♻️ Suggested refactor
+const MOCK_BOUNDS: DOMRect = { + bottom: 500, + height: 400, + left: 600, + right: 1100, + top: 100, + width: 500, + x: 600, + y: 100, + toJSON: () => undefined, +} + it("hides after an in-flight show when a modal opens", async () => { - vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockReturnValue({ - bottom: 500, - height: 400, - left: 600, - right: 1100, - top: 100, - width: 500, - x: 600, - y: 100, - toJSON: () => undefined, - }) + vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockReturnValue(MOCK_BOUNDS)🤖 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 `@src/routes/Chat/BrowserPanel.test.ts` around lines 108 - 118, Extract the repeated getBoundingClientRect bounds object into a shared helper or constant in BrowserPanel.test.ts, then reuse it in all three mockReturnValue calls, including the test around the current mock. Preserve the existing mocked dimensions and behavior while eliminating the duplicated literal.
🤖 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 `@src/routes/Chat/BrowserPanel.test.ts`:
- Around line 108-118: Extract the repeated getBoundingClientRect bounds object
into a shared helper or constant in BrowserPanel.test.ts, then reuse it in all
three mockReturnValue calls, including the test around the current mock.
Preserve the existing mocked dimensions and behavior while eliminating the
duplicated literal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 72ac05a8-2356-472d-8eed-644826a7ac4f
📒 Files selected for processing (4)
src/components/ai-elements/message-streamdown.test.tssrc/components/ai-elements/message-streamdown.tsxsrc/routes/Chat/BrowserPanel.test.tssrc/routes/Chat/BrowserPanel.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/ai-elements/message-streamdown.tsx
- src/routes/Chat/BrowserPanel.tsx
- src/components/ai-elements/message-streamdown.test.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.
Issue
Generated HTML reports were treated as intermediate execution files instead of final artifacts, so users only saw a raw message link. Those local links were also routed through external-link behavior, producing encoded filesystem paths that could neither be opened reliably nor copied through Electron's trusted clipboard path.
The integrated browser introduced two related desktop interaction problems. Empty toolbar space was not draggable, and Electron's native
WebContentsViewalways composited above renderer-owned modal backdrops. Hiding the native view fixed the stacking issue but made the preview appear to disappear while a modal was open.Root cause
.htmland.htmwere included in the intermediate-code extension classifier.navigator.clipboardAPI directly and silently swallowed failures.WebContentsViewcannot be covered by renderer DOM, so ordinaryz-indexchanges cannot make modal overlays appear above it.Fix
file:link targets and open local files through the existing trustedopenLocalPathservice.Validation
pnpm run formatpnpm run lintpnpm run ts-check