feat(webedit): add WebEdit agent for web page browsing and DOM selection#512
feat(webedit): add WebEdit agent for web page browsing and DOM selection#512
Conversation
Renamed AgentChatInterface to CodingAgentPage and RemoteAgentChatInterface to RemoteAgentPage to improve naming consistency and code readability across platforms. Updated all usages accordingly.
- Add WEB_EDIT AgentType with display name and parsing support - Create WebEditBridge abstraction for WebView-Kotlin communication - Create DOMElement model classes for DOM tree representation - Create WebEditPage with URL input, WebView placeholder, and DOM sidebar - Create WebEditToolbar, DOMTreeSidebar, WebEditChatInput components - Create WebEditBridgeScript for JavaScript injection into WebView - Add platform-specific bridge implementations (JVM, WASM) - Update AgentInterfaceRouter to route WEB_EDIT to WebEditPage - Update exhaustive when expressions in CodingAgentPage and TopBarMenus
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new WEB_EDIT agent and implements a WebEdit feature: DOM models and messages, a WebEditBridge (common + JVM/WASM), JS bridge script, platform WebEditView, UI components (toolbar, page, DOM sidebar, chat input), router and IDE wiring, icons, and build dependency updates. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as WebEditPage
participant Bridge as WebEditBridge
participant WebView
participant JS as injected script
User->>UI: Enter URL / click navigate
UI->>Bridge: navigateTo(url)
Bridge->>WebView: load URL / execute JS
WebView->>JS: page loaded -> injected script runs
JS->>Bridge: send DOMTreeUpdated / PageLoaded / ElementSelected
Bridge->>UI: update StateFlow (domTree, selectedElement, isReady)
UI->>UI: recompose / show DOM sidebar / selection highlight
User->>UI: toggle selection mode / hover / click element
UI->>Bridge: setSelectionMode(true/false)
Bridge->>JS: run selection mode commands
JS->>Bridge: ElementSelected / highlight events
User->>UI: ask in chat / send input
UI->>LLM (outside scope): include page/element context (from Bridge state)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| sendToKotlin: function(type, data) { | ||
| if (window.kmpJsBridge && window.kmpJsBridge.callNative) { | ||
| window.kmpJsBridge.callNative('webEditMessage', | ||
| JSON.stringify({ type: type, data: data }), function() {}); |
There was a problem hiding this comment.
sendToKotlin JSON-stringifies an object whose data is already a JSON string at the call sites, which makes the payload double-encoded and forces brittle “JSON parse twice” handling later. Consider sending structured JSON once (similar to the existing mermaidRenderCallback usage) to keep the message contract straightforward.
🤖 Was this useful? React with 👍 or 👎
| } | ||
|
|
||
| override suspend fun highlightElement(selector: String) { | ||
| val escapedSelector = selector.replace("'", "\\'") |
There was a problem hiding this comment.
The injected JS builds a single-quoted string from selector but only escapes '; selectors containing backslashes/control characters can break the JS literal (and complicate safe JS injection). Consider a more robust escaping/encoding approach here (also applies to scrollToElement and the wasm bridge).
🤖 Was this useful? React with 👍 or 👎
There was a problem hiding this comment.
With the current WebEditViewPlaceholder, onPageLoaded is never invoked, so after onNavigate/onReload sets isLoading = true the progress bar never clears. Until a real WebView implementation is wired in, consider ensuring the placeholder triggers onPageLoaded so the UI doesn’t get stuck in a loading state.
🤖 Was this useful? React with 👍 or 👎
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/AgentType.kt (1)
60-72:fromString("web") -> WEB_EDITis likely too generic.
If any existing config/UI passes"web"for something else (or future features add “web” variants), this will silently route to WEB_EDIT. Consider keeping only"webedit"(and maybe"web_edit").- "webedit", "web" -> WEB_EDIT + "webedit", "web_edit" -> WEB_EDITmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/CodingAgentPage.kt (1)
28-34: Verify thatAgentChatInterfacereferences in comments and documentation are updated to reflect the new routing architecture.The rename is functionally complete:
CodingAgentPageis correctly called fromAgentInterfaceRouter(the single entry point across all platforms: common, Android, Apple). However, outdated references remain in non-breaking locations:
AutoDevApp.ktline 151 (comment only)RemoteAgentChatInterface.ktline 24 (documentation comment)- Comments in
IdeaAgentApp.ktandApp.tsxThese are documentation artifacts, not functional issues, since
CodingAgentPageis an internal composable dispatched solely through the router. The emptywhenbranches (lines 295–298) are correctly designed as no-ops.
🧹 Nitpick comments (8)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/AgentType.kt (1)
43-48: Update the enum KDoc to includeWEB_EDIT(doc drift).
Right now the header comment lists all modes but omits the newly added WEB_EDIT./** * Represents all available agent modes in the system: * - LOCAL: Simple local chat mode without heavy tooling * - CODING: Local coding agent with full tool access (file system, shell, etc.) * - CODE_REVIEW: Dedicated code review agent with git integration * - KNOWLEDGE: Document reader mode for AI-native document reading * - CHAT_DB: Database chat mode for text-to-SQL interactions * - REMOTE: Remote agent connected to mpp-server + * - WEB_EDIT: Browse web pages, select DOM elements, and interact with page content */mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/CodingAgentPage.kt (1)
224-299: Avoid silent “should not reach here” branches (includingWEB_EDIT)—render a visible fallback.
If routing regresses, this will currently show a partial page with missing/odd UI; a small inline warning helps debugging and avoids confusing UX.AgentType.WEB_EDIT -> { // WEB_EDIT has its own full-page interface // It should not reach here - handled by AgentInterfaceRouter + Text( + text = "WEB_EDIT is handled by AgentInterfaceRouter (unexpected route).", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.error, + modifier = Modifier.padding(horizontal = 12.dp, vertical = 8.dp) + ) }mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/DOMTreeSidebar.kt (2)
28-28: Search field is non-functional.The
searchQuerystate variable is captured from the search field but is never used to filter the DOM tree items in theLazyColumn. Consider either:
- Implementing filtering:
sampleDomTree.filter { it.tagName.contains(searchQuery, ignoreCase = true) || it.selector.contains(searchQuery, ignoreCase = true) }- Removing the search field if filtering is deferred to a later phase
Apply this diff to implement basic search filtering:
// DOM tree list + val filteredItems = if (searchQuery.isBlank()) { + sampleDomTree + } else { + sampleDomTree.filter { + it.tagName.contains(searchQuery, ignoreCase = true) || + it.selector.contains(searchQuery, ignoreCase = true) + } + } + LazyColumn( modifier = Modifier.fillMaxSize(), verticalArrangement = Arrangement.spacedBy(2.dp) ) { - items(sampleDomTree) { item -> + items(filteredItems) { item -> DOMTreeItemRow(Also applies to: 77-88
114-126: Hover state is never updated.The
isHoveredstate variable is declared but never set totrue. TheonHovercallback is invoked on click but doesn't update the local hover state. To enable hover visual feedback, you need to add a pointer hover modifier.Apply this diff to enable hover visual feedback:
+import androidx.compose.foundation.hoverable +import androidx.compose.foundation.interaction.MutableInteractionSource +import androidx.compose.foundation.interaction.collectIsHoveredAsState + @Composable private fun DOMTreeItemRow( item: DOMTreeItem, onClick: () -> Unit, onHover: (Boolean) -> Unit, modifier: Modifier = Modifier ) { - var isHovered by remember { mutableStateOf(false) } + val interactionSource = remember { MutableInteractionSource() } + val isHovered by interactionSource.collectIsHoveredAsState() + + LaunchedEffect(isHovered) { + onHover(isHovered) + } Surface( modifier = modifier .fillMaxWidth() + .hoverable(interactionSource) .clickable { onClick() } .padding(start = (item.depth * 12).dp),mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/DOMElement.kt (1)
29-34: Consider edge cases in display name generation.The
getDisplayName()method has a few potential issues:
- Line 30: Taking only the first class from
split(" ")may not represent multi-class elements well- Line 32: The 30-character truncation is arbitrary and may cut off meaningful content mid-word
- Consider adding ellipsis ("...") to indicate truncation
fun getDisplayName(): String { val classAttr = attributes["class"]?.split(" ")?.firstOrNull()?.let { ".$it" } ?: "" val idAttr = attributes["id"]?.let { "#$it" } ?: "" - val text = textContent?.take(30)?.let { " \"$it\"" } ?: "" + val text = textContent?.take(30)?.let { + val truncated = if (textContent.length > 30) "$it..." else it + " \"$truncated\"" + } ?: "" return "$tagName$idAttr$classAttr$text" }mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.jvm.kt (1)
126-137: Public state setters bypass the message handler pattern.The public
setUrl(),setTitle(), andsetLoading()methods directly mutate state, bypassing thehandleMessage()flow. This could lead to inconsistent state updates and makes the bridge harder to reason about.Consider making these methods internal or private, or route all state changes through
handleMessage()for consistency:-fun setUrl(url: String) { +internal fun setUrl(url: String) { _currentUrl.value = url } -fun setTitle(title: String) { +internal fun setTitle(title: String) { _pageTitle.value = title } -fun setLoading(loading: Boolean) { +internal fun setLoading(loading: Boolean) { _isLoading.value = loading }mpp-viewer-web/src/wasmJsMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.wasmJs.kt (2)
119-129: Public state setters bypass the message handler pattern.Same architectural concern as JVM implementation - these methods bypass
handleMessage().Consider making these internal:
-fun setUrl(url: String) { +internal fun setUrl(url: String) { _currentUrl.value = url } -fun setTitle(title: String) { +internal fun setTitle(title: String) { _pageTitle.value = title } -fun setLoading(loading: Boolean) { +internal fun setLoading(loading: Boolean) { _isLoading.value = loading }
10-130: Consider reducing code duplication with JVM implementation.The WASM and JVM implementations are nearly identical (~95% duplicate code). While the
expect/actualpattern requires separate files, consider extracting common logic to a shared abstract base class or internal helper.Example approach:
// In commonMain internal abstract class BaseWebEditBridge : WebEditBridge { protected val _currentUrl = MutableStateFlow("") override val currentUrl: StateFlow<String> = _currentUrl.asStateFlow() // ... other common state and logic protected abstract fun executeJavaScriptImpl(script: String) protected abstract fun navigateImpl(url: String) // ... other platform-specific hooks } // In jvmMain/wasmJsMain class JvmWebEditBridge : BaseWebEditBridge() { override fun executeJavaScriptImpl(script: String) { /* platform code */ } }This would eliminate duplication while preserving platform flexibility. However, if you anticipate significant platform-specific divergence, the current approach is acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/AgentType.kt(3 hunks)mpp-ui/src/androidMain/kotlin/cc/unitmesh/devins/ui/compose/AutoDevApp.android.kt(0 hunks)mpp-ui/src/appleMain/kotlin/cc/unitmesh/devins/ui/compose/AutoDevApp.apple.kt(0 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentInterfaceRouter.kt(3 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/CodingAgentPage.kt(2 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/DOMTreeSidebar.kt(1 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditChatInput.kt(1 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditPage.kt(1 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditToolbar.kt(1 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditViewPlaceholder.kt(1 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/DesktopTitleBarTabs.kt(1 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/TopBarMenuDesktop.kt(1 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/TopBarMenuMobile.kt(2 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/remote/RemoteAgentChatInterface.kt(1 hunks)mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/DOMElement.kt(1 hunks)mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.kt(1 hunks)mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridgeScript.kt(1 hunks)mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.jvm.kt(1 hunks)mpp-viewer-web/src/wasmJsMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.wasmJs.kt(1 hunks)
💤 Files with no reviewable changes (2)
- mpp-ui/src/appleMain/kotlin/cc/unitmesh/devins/ui/compose/AutoDevApp.apple.kt
- mpp-ui/src/androidMain/kotlin/cc/unitmesh/devins/ui/compose/AutoDevApp.android.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualpattern for platform-specific code (for example:Platformimplementations)
Use concrete classes (not interfaces) andPromise(notFlow) for @JsExport annotations
Avoid emoji and UTF-8 characters in WASM code
When modifyingCodingAgentRenderer, update ALL implementations:DefaultCodingAgentRenderer,ComposeRenderer,JewelRenderer,ServerSideRenderer, andJsRendererAdapterin Kotlin
When modifyingCodingAgentRenderer, update JVM CLI implementations:CodingCliRendererandConsoleRenderer
UseAutoDevColorsorMaterialTheme.colorSchemefor Compose styling
Files:
mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridgeScript.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditViewPlaceholder.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/remote/RemoteAgentChatInterface.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditToolbar.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentInterfaceRouter.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditPage.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditChatInput.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/CodingAgentPage.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/DesktopTitleBarTabs.ktmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/AgentType.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/TopBarMenuDesktop.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/DOMTreeSidebar.ktmpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/DOMElement.ktmpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.jvm.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/TopBarMenuMobile.ktmpp-viewer-web/src/wasmJsMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.wasmJs.ktmpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.kt
**/*.{kt,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never use hardcoded colors - always use design tokens from the design system
Files:
mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridgeScript.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditViewPlaceholder.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/remote/RemoteAgentChatInterface.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditToolbar.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentInterfaceRouter.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditPage.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditChatInput.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/CodingAgentPage.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/DesktopTitleBarTabs.ktmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/AgentType.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/TopBarMenuDesktop.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/DOMTreeSidebar.ktmpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/DOMElement.ktmpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.jvm.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/TopBarMenuMobile.ktmpp-viewer-web/src/wasmJsMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.wasmJs.ktmpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.kt
🧠 Learnings (3)
📚 Learning: 2025-12-07T08:40:10.850Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T08:40:10.850Z
Learning: Applies to mpp-vscode/src/**/*.ts : When modifying `CodingAgentRenderer`, update VSCode implementations: `mpp-vscode/src/bridge/mpp-core.ts` and `mpp-vscode/src/providers/chat-view.ts`
Applied to files:
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/remote/RemoteAgentChatInterface.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentInterfaceRouter.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/CodingAgentPage.kt
📚 Learning: 2025-12-07T08:40:10.850Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T08:40:10.850Z
Learning: Applies to **/*.kt : When modifying `CodingAgentRenderer`, update ALL implementations: `DefaultCodingAgentRenderer`, `ComposeRenderer`, `JewelRenderer`, `ServerSideRenderer`, and `JsRendererAdapter` in Kotlin
Applied to files:
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/remote/RemoteAgentChatInterface.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentInterfaceRouter.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/CodingAgentPage.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/DesktopTitleBarTabs.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/TopBarMenuDesktop.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/TopBarMenuMobile.kt
📚 Learning: 2025-12-07T08:40:10.850Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T08:40:10.850Z
Learning: Applies to **/*.kt : When modifying `CodingAgentRenderer`, update JVM CLI implementations: `CodingCliRenderer` and `ConsoleRenderer`
Applied to files:
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentInterfaceRouter.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/CodingAgentPage.kt
🧬 Code graph analysis (1)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentInterfaceRouter.kt (4)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/remote/RemoteAgentChatInterface.kt (1)
RemoteAgentPage(27-290)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditPage.kt (1)
WebEditPage(26-137)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.kt (1)
onAgentTypeChange(223-236)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/CodingAgentPage.kt (1)
CodingAgentPage(28-507)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: Build
- GitHub Check: Test mpp-core (KMP)
🔇 Additional comments (12)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/remote/RemoteAgentChatInterface.kt (1)
28-28: Good rename for consistency, and all call sites are properly updated.The rename from
RemoteAgentChatInterfacetoRemoteAgentPageimproves naming consistency with other page composables introduced in this PR (e.g.,WebEditPage,CodingAgentPage). The "Page" suffix is more idiomatic for Compose UI components. Verification confirms all call sites have been updated and no references to the old function name remain in the codebase.mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/DesktopTitleBarTabs.kt (1)
34-41: WEB_EDIT hiding the session sidebar: consistent and clear.
This keeps layout aligned with other full-page modes (CODE_REVIEW/KNOWLEDGE).mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/TopBarMenuMobile.kt (1)
220-230: WEB_EDIT icon mapping looks good (keepswhenexhaustive).Also applies to: 256-266
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/TopBarMenuDesktop.kt (1)
290-290: LGTM! Icon mapping for WEB_EDIT is appropriate.The Language icon is a fitting choice for the web editing agent type, and the implementation correctly uses the design system icon (AutoDevComposeIcons) rather than hardcoded values.
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentInterfaceRouter.kt (1)
133-142: LGTM! WebEdit routing implemented correctly.The WEB_EDIT agent type is properly integrated into the router with appropriate parameters (llmService, onBack navigation to CODING, onNotification). The pattern is consistent with other specialized agent types like CODE_REVIEW and KNOWLEDGE.
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditViewPlaceholder.kt (1)
20-26: Placeholder implementation aligns with PR objectives.The
onPageLoadedandonElementSelectedcallbacks are defined but not invoked in this placeholder. This is expected as the PR objectives list "Create platform-specific WebView implementation for JVM" as a remaining TODO. The callbacks will be wired when the actual WebView implementation is added.mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditChatInput.kt (1)
20-84: LGTM! Clean chat input implementation.The component properly uses Material Theme design tokens for colors and includes appropriate validation to prevent sending blank messages. The keyboard actions and button states are correctly synchronized.
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditToolbar.kt (1)
60-80: Browser history navigation is placeholder.The Back and Forward browser navigation buttons currently have empty onClick handlers. This is expected given that the platform-specific WebView implementation is pending (per PR objectives). These will need to be wired to actual WebView navigation methods when the JVM WebView implementation is added.
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditPage.kt (1)
128-132: LLM integration is scaffolded for future implementation.The chat input's
onSendhandler currently only clears the input field. Per PR objectives, "Connect LLM service for page/element Q&A functionality" is listed as a remaining TODO. ThellmServiceparameter is already wired through the function signature, ready for integration.mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.kt (1)
13-132: LGTM! Clean bridge architecture using expect/actual pattern.The
WebEditBridgeinterface provides a well-designed contract for platform-specific WebView communication:
- Uses
StateFlowfor reactive state properties (appropriate for Compose)- Declares async operations as
suspendfunctions- Follows the expect/actual pattern per coding guidelines
- Clean separation between observable state and actions
The interface is ready for platform-specific implementations (JVM and WASM implementations are present per PR summary).
mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/DOMElement.kt (1)
16-118: LGTM! Well-structured data models.The data classes and sealed hierarchies are well-designed for DOM representation and bidirectional messaging. Proper use of
@Serializable, nullable types, and default parameters ensures flexibility and serialization support.mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.jvm.kt (1)
10-61: Well-structured state management and navigation.The StateFlow pattern is correctly implemented with proper encapsulation (private mutable, public read-only). Navigation methods appropriately update loading state and delegate to callbacks.
| fun getWebEditBridgeScript(): String = """ | ||
| (function() { | ||
| // Prevent multiple injections | ||
| if (window.webEditBridge) return; | ||
|
|
||
| // WebEdit Bridge object | ||
| window.webEditBridge = { | ||
| selectionMode: false, | ||
| highlightedElement: null, | ||
| selectedElement: null, | ||
|
|
||
| // Enable/disable selection mode | ||
| setSelectionMode: function(enabled) { | ||
| this.selectionMode = enabled; | ||
| if (!enabled) { | ||
| this.clearHighlights(); | ||
| } | ||
| console.log('[WebEditBridge] Selection mode:', enabled); | ||
| }, | ||
|
|
||
| // Highlight a specific element by selector | ||
| highlightElement: function(selector) { | ||
| this.clearHighlights(); | ||
| try { | ||
| const el = document.querySelector(selector); | ||
| if (el) { | ||
| this.highlightedElement = el; | ||
| el.style.outline = '2px solid #2196F3'; | ||
| el.style.outlineOffset = '2px'; | ||
| } | ||
| } catch(e) { | ||
| console.error('[WebEditBridge] highlightElement error:', e); | ||
| } | ||
| }, | ||
|
|
||
| // Clear all highlights | ||
| clearHighlights: function() { | ||
| if (this.highlightedElement) { | ||
| this.highlightedElement.style.outline = ''; | ||
| this.highlightedElement.style.outlineOffset = ''; | ||
| this.highlightedElement = null; | ||
| } | ||
| if (this.selectedElement) { | ||
| this.selectedElement.style.outline = ''; | ||
| this.selectedElement.style.outlineOffset = ''; | ||
| } | ||
| }, | ||
|
|
||
| // Scroll to element | ||
| scrollToElement: function(selector) { | ||
| try { | ||
| const el = document.querySelector(selector); | ||
| if (el) { | ||
| el.scrollIntoView({ behavior: 'smooth', block: 'center' }); | ||
| } | ||
| } catch(e) { | ||
| console.error('[WebEditBridge] scrollToElement error:', e); | ||
| } | ||
| }, | ||
|
|
||
| // Get unique selector for element | ||
| getSelector: function(el) { | ||
| if (el.id) return '#' + el.id; | ||
| if (el === document.body) return 'body'; | ||
| if (el === document.documentElement) return 'html'; | ||
|
|
||
| const path = []; | ||
| while (el && el.nodeType === Node.ELEMENT_NODE) { | ||
| let selector = el.tagName.toLowerCase(); | ||
| if (el.id) { | ||
| selector = '#' + el.id; | ||
| path.unshift(selector); | ||
| break; | ||
| } else if (el.className && typeof el.className === 'string') { | ||
| const classes = el.className.trim().split(/\\s+/).slice(0, 2); | ||
| if (classes.length > 0 && classes[0]) { | ||
| selector += '.' + classes.join('.'); | ||
| } | ||
| } | ||
| path.unshift(selector); | ||
| el = el.parentNode; | ||
| } | ||
| return path.join(' > '); | ||
| }, | ||
|
|
||
| // Get DOM tree (simplified) | ||
| getDOMTree: function() { | ||
| function buildTree(el, depth) { | ||
| if (depth > 5) return null; | ||
| const rect = el.getBoundingClientRect(); | ||
| const node = { | ||
| id: Math.random().toString(36).substr(2, 9), | ||
| tagName: el.tagName.toLowerCase(), | ||
| selector: window.webEditBridge.getSelector(el), | ||
| textContent: (el.textContent || '').trim().substring(0, 50), | ||
| attributes: {}, | ||
| boundingBox: { x: rect.x, y: rect.y, width: rect.width, height: rect.height }, | ||
| children: [] | ||
| }; | ||
| if (el.id) node.attributes.id = el.id; | ||
| if (el.className) node.attributes.class = el.className; | ||
| Array.from(el.children).slice(0, 20).forEach(child => { | ||
| const childNode = buildTree(child, depth + 1); | ||
| if (childNode) node.children.push(childNode); | ||
| }); | ||
| return node; | ||
| } | ||
| const tree = buildTree(document.body, 0); | ||
| this.sendToKotlin('DOMTreeUpdated', JSON.stringify({ root: tree })); | ||
| }, | ||
|
|
||
| // Send message to Kotlin | ||
| sendToKotlin: function(type, data) { | ||
| if (window.kmpJsBridge && window.kmpJsBridge.callNative) { | ||
| window.kmpJsBridge.callNative('webEditMessage', | ||
| JSON.stringify({ type: type, data: data }), function() {}); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| // Mouse event handlers for selection mode | ||
| document.addEventListener('mouseover', function(e) { | ||
| if (!window.webEditBridge.selectionMode) return; | ||
| e.stopPropagation(); | ||
| window.webEditBridge.clearHighlights(); | ||
| window.webEditBridge.highlightedElement = e.target; | ||
| e.target.style.outline = '2px solid #2196F3'; | ||
| e.target.style.outlineOffset = '2px'; | ||
| }, true); | ||
|
|
||
| document.addEventListener('click', function(e) { | ||
| if (!window.webEditBridge.selectionMode) return; | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| const el = e.target; | ||
| window.webEditBridge.selectedElement = el; | ||
| el.style.outline = '3px solid #4CAF50'; | ||
| el.style.outlineOffset = '2px'; | ||
| const rect = el.getBoundingClientRect(); | ||
| window.webEditBridge.sendToKotlin('ElementSelected', JSON.stringify({ | ||
| element: { | ||
| id: Math.random().toString(36).substr(2, 9), | ||
| tagName: el.tagName.toLowerCase(), | ||
| selector: window.webEditBridge.getSelector(el), | ||
| textContent: (el.textContent || '').trim().substring(0, 100), | ||
| attributes: { id: el.id || '', class: el.className || '' }, | ||
| boundingBox: { x: rect.x, y: rect.y, width: rect.width, height: rect.height } | ||
| } | ||
| })); | ||
| }, true); | ||
|
|
||
| // Notify page loaded | ||
| window.webEditBridge.sendToKotlin('PageLoaded', JSON.stringify({ | ||
| url: window.location.href, | ||
| title: document.title | ||
| })); | ||
|
|
||
| console.log('[WebEditBridge] Initialized'); | ||
| })(); | ||
| """.trimIndent() |
There was a problem hiding this comment.
Selection UX bug: hover clears selection highlight (and selection won’t persist).
document.mouseover calls clearHighlights(), which clears selectedElement outline too—so after selecting, moving the mouse removes the selection indicator. Split hover vs selected state.
- // Clear all highlights
- clearHighlights: function() {
+ // Clear hover highlight only (keep selection)
+ clearHoverHighlight: function() {
if (this.highlightedElement) {
this.highlightedElement.style.outline = '';
this.highlightedElement.style.outlineOffset = '';
this.highlightedElement = null;
}
- if (this.selectedElement) {
- this.selectedElement.style.outline = '';
- this.selectedElement.style.outlineOffset = '';
- }
},
+
+ // Clear selection highlight
+ clearSelection: function() {
+ if (this.selectedElement) {
+ this.selectedElement.style.outline = '';
+ this.selectedElement.style.outlineOffset = '';
+ this.selectedElement = null;
+ }
+ }, setSelectionMode: function(enabled) {
this.selectionMode = enabled;
if (!enabled) {
- this.clearHighlights();
+ this.clearHoverHighlight();
+ this.clearSelection();
}
console.log('[WebEditBridge] Selection mode:', enabled);
}, document.addEventListener('mouseover', function(e) {
if (!window.webEditBridge.selectionMode) return;
- e.stopPropagation();
- window.webEditBridge.clearHighlights();
+ window.webEditBridge.clearHoverHighlight();
window.webEditBridge.highlightedElement = e.target;
e.target.style.outline = '2px solid #2196F3';
e.target.style.outlineOffset = '2px';
}, true);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun getWebEditBridgeScript(): String = """ | |
| (function() { | |
| // Prevent multiple injections | |
| if (window.webEditBridge) return; | |
| // WebEdit Bridge object | |
| window.webEditBridge = { | |
| selectionMode: false, | |
| highlightedElement: null, | |
| selectedElement: null, | |
| // Enable/disable selection mode | |
| setSelectionMode: function(enabled) { | |
| this.selectionMode = enabled; | |
| if (!enabled) { | |
| this.clearHighlights(); | |
| } | |
| console.log('[WebEditBridge] Selection mode:', enabled); | |
| }, | |
| // Highlight a specific element by selector | |
| highlightElement: function(selector) { | |
| this.clearHighlights(); | |
| try { | |
| const el = document.querySelector(selector); | |
| if (el) { | |
| this.highlightedElement = el; | |
| el.style.outline = '2px solid #2196F3'; | |
| el.style.outlineOffset = '2px'; | |
| } | |
| } catch(e) { | |
| console.error('[WebEditBridge] highlightElement error:', e); | |
| } | |
| }, | |
| // Clear all highlights | |
| clearHighlights: function() { | |
| if (this.highlightedElement) { | |
| this.highlightedElement.style.outline = ''; | |
| this.highlightedElement.style.outlineOffset = ''; | |
| this.highlightedElement = null; | |
| } | |
| if (this.selectedElement) { | |
| this.selectedElement.style.outline = ''; | |
| this.selectedElement.style.outlineOffset = ''; | |
| } | |
| }, | |
| // Scroll to element | |
| scrollToElement: function(selector) { | |
| try { | |
| const el = document.querySelector(selector); | |
| if (el) { | |
| el.scrollIntoView({ behavior: 'smooth', block: 'center' }); | |
| } | |
| } catch(e) { | |
| console.error('[WebEditBridge] scrollToElement error:', e); | |
| } | |
| }, | |
| // Get unique selector for element | |
| getSelector: function(el) { | |
| if (el.id) return '#' + el.id; | |
| if (el === document.body) return 'body'; | |
| if (el === document.documentElement) return 'html'; | |
| const path = []; | |
| while (el && el.nodeType === Node.ELEMENT_NODE) { | |
| let selector = el.tagName.toLowerCase(); | |
| if (el.id) { | |
| selector = '#' + el.id; | |
| path.unshift(selector); | |
| break; | |
| } else if (el.className && typeof el.className === 'string') { | |
| const classes = el.className.trim().split(/\\s+/).slice(0, 2); | |
| if (classes.length > 0 && classes[0]) { | |
| selector += '.' + classes.join('.'); | |
| } | |
| } | |
| path.unshift(selector); | |
| el = el.parentNode; | |
| } | |
| return path.join(' > '); | |
| }, | |
| // Get DOM tree (simplified) | |
| getDOMTree: function() { | |
| function buildTree(el, depth) { | |
| if (depth > 5) return null; | |
| const rect = el.getBoundingClientRect(); | |
| const node = { | |
| id: Math.random().toString(36).substr(2, 9), | |
| tagName: el.tagName.toLowerCase(), | |
| selector: window.webEditBridge.getSelector(el), | |
| textContent: (el.textContent || '').trim().substring(0, 50), | |
| attributes: {}, | |
| boundingBox: { x: rect.x, y: rect.y, width: rect.width, height: rect.height }, | |
| children: [] | |
| }; | |
| if (el.id) node.attributes.id = el.id; | |
| if (el.className) node.attributes.class = el.className; | |
| Array.from(el.children).slice(0, 20).forEach(child => { | |
| const childNode = buildTree(child, depth + 1); | |
| if (childNode) node.children.push(childNode); | |
| }); | |
| return node; | |
| } | |
| const tree = buildTree(document.body, 0); | |
| this.sendToKotlin('DOMTreeUpdated', JSON.stringify({ root: tree })); | |
| }, | |
| // Send message to Kotlin | |
| sendToKotlin: function(type, data) { | |
| if (window.kmpJsBridge && window.kmpJsBridge.callNative) { | |
| window.kmpJsBridge.callNative('webEditMessage', | |
| JSON.stringify({ type: type, data: data }), function() {}); | |
| } | |
| } | |
| }; | |
| // Mouse event handlers for selection mode | |
| document.addEventListener('mouseover', function(e) { | |
| if (!window.webEditBridge.selectionMode) return; | |
| e.stopPropagation(); | |
| window.webEditBridge.clearHighlights(); | |
| window.webEditBridge.highlightedElement = e.target; | |
| e.target.style.outline = '2px solid #2196F3'; | |
| e.target.style.outlineOffset = '2px'; | |
| }, true); | |
| document.addEventListener('click', function(e) { | |
| if (!window.webEditBridge.selectionMode) return; | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| const el = e.target; | |
| window.webEditBridge.selectedElement = el; | |
| el.style.outline = '3px solid #4CAF50'; | |
| el.style.outlineOffset = '2px'; | |
| const rect = el.getBoundingClientRect(); | |
| window.webEditBridge.sendToKotlin('ElementSelected', JSON.stringify({ | |
| element: { | |
| id: Math.random().toString(36).substr(2, 9), | |
| tagName: el.tagName.toLowerCase(), | |
| selector: window.webEditBridge.getSelector(el), | |
| textContent: (el.textContent || '').trim().substring(0, 100), | |
| attributes: { id: el.id || '', class: el.className || '' }, | |
| boundingBox: { x: rect.x, y: rect.y, width: rect.width, height: rect.height } | |
| } | |
| })); | |
| }, true); | |
| // Notify page loaded | |
| window.webEditBridge.sendToKotlin('PageLoaded', JSON.stringify({ | |
| url: window.location.href, | |
| title: document.title | |
| })); | |
| console.log('[WebEditBridge] Initialized'); | |
| })(); | |
| """.trimIndent() | |
| fun getWebEditBridgeScript(): String = """ | |
| (function() { | |
| // Prevent multiple injections | |
| if (window.webEditBridge) return; | |
| // WebEdit Bridge object | |
| window.webEditBridge = { | |
| selectionMode: false, | |
| highlightedElement: null, | |
| selectedElement: null, | |
| // Enable/disable selection mode | |
| setSelectionMode: function(enabled) { | |
| this.selectionMode = enabled; | |
| if (!enabled) { | |
| this.clearHoverHighlight(); | |
| this.clearSelection(); | |
| } | |
| console.log('[WebEditBridge] Selection mode:', enabled); | |
| }, | |
| // Highlight a specific element by selector | |
| highlightElement: function(selector) { | |
| this.clearHoverHighlight(); | |
| try { | |
| const el = document.querySelector(selector); | |
| if (el) { | |
| this.highlightedElement = el; | |
| el.style.outline = '2px solid #2196F3'; | |
| el.style.outlineOffset = '2px'; | |
| } | |
| } catch(e) { | |
| console.error('[WebEditBridge] highlightElement error:', e); | |
| } | |
| }, | |
| // Clear hover highlight only (keep selection) | |
| clearHoverHighlight: function() { | |
| if (this.highlightedElement) { | |
| this.highlightedElement.style.outline = ''; | |
| this.highlightedElement.style.outlineOffset = ''; | |
| this.highlightedElement = null; | |
| } | |
| }, | |
| // Clear selection highlight | |
| clearSelection: function() { | |
| if (this.selectedElement) { | |
| this.selectedElement.style.outline = ''; | |
| this.selectedElement.style.outlineOffset = ''; | |
| this.selectedElement = null; | |
| } | |
| }, | |
| // Scroll to element | |
| scrollToElement: function(selector) { | |
| try { | |
| const el = document.querySelector(selector); | |
| if (el) { | |
| el.scrollIntoView({ behavior: 'smooth', block: 'center' }); | |
| } | |
| } catch(e) { | |
| console.error('[WebEditBridge] scrollToElement error:', e); | |
| } | |
| }, | |
| // Get unique selector for element | |
| getSelector: function(el) { | |
| if (el.id) return '#' + el.id; | |
| if (el === document.body) return 'body'; | |
| if (el === document.documentElement) return 'html'; | |
| const path = []; | |
| while (el && el.nodeType === Node.ELEMENT_NODE) { | |
| let selector = el.tagName.toLowerCase(); | |
| if (el.id) { | |
| selector = '#' + el.id; | |
| path.unshift(selector); | |
| break; | |
| } else if (el.className && typeof el.className === 'string') { | |
| const classes = el.className.trim().split(/\\s+/).slice(0, 2); | |
| if (classes.length > 0 && classes[0]) { | |
| selector += '.' + classes.join('.'); | |
| } | |
| } | |
| path.unshift(selector); | |
| el = el.parentNode; | |
| } | |
| return path.join(' > '); | |
| }, | |
| // Get DOM tree (simplified) | |
| getDOMTree: function() { | |
| function buildTree(el, depth) { | |
| if (depth > 5) return null; | |
| const rect = el.getBoundingClientRect(); | |
| const node = { | |
| id: Math.random().toString(36).substr(2, 9), | |
| tagName: el.tagName.toLowerCase(), | |
| selector: window.webEditBridge.getSelector(el), | |
| textContent: (el.textContent || '').trim().substring(0, 50), | |
| attributes: {}, | |
| boundingBox: { x: rect.x, y: rect.y, width: rect.width, height: rect.height }, | |
| children: [] | |
| }; | |
| if (el.id) node.attributes.id = el.id; | |
| if (el.className) node.attributes.class = el.className; | |
| Array.from(el.children).slice(0, 20).forEach(child => { | |
| const childNode = buildTree(child, depth + 1); | |
| if (childNode) node.children.push(childNode); | |
| }); | |
| return node; | |
| } | |
| const tree = buildTree(document.body, 0); | |
| this.sendToKotlin('DOMTreeUpdated', JSON.stringify({ root: tree })); | |
| }, | |
| // Send message to Kotlin | |
| sendToKotlin: function(type, data) { | |
| if (window.kmpJsBridge && window.kmpJsBridge.callNative) { | |
| window.kmpJsBridge.callNative('webEditMessage', | |
| JSON.stringify({ type: type, data: data }), function() {}); | |
| } | |
| } | |
| }; | |
| // Mouse event handlers for selection mode | |
| document.addEventListener('mouseover', function(e) { | |
| if (!window.webEditBridge.selectionMode) return; | |
| window.webEditBridge.clearHoverHighlight(); | |
| window.webEditBridge.highlightedElement = e.target; | |
| e.target.style.outline = '2px solid #2196F3'; | |
| e.target.style.outlineOffset = '2px'; | |
| }, true); | |
| document.addEventListener('click', function(e) { | |
| if (!window.webEditBridge.selectionMode) return; | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| const el = e.target; | |
| window.webEditBridge.selectedElement = el; | |
| el.style.outline = '3px solid #4CAF50'; | |
| el.style.outlineOffset = '2px'; | |
| const rect = el.getBoundingClientRect(); | |
| window.webEditBridge.sendToKotlin('ElementSelected', JSON.stringify({ | |
| element: { | |
| id: Math.random().toString(36).substr(2, 9), | |
| tagName: el.tagName.toLowerCase(), | |
| selector: window.webEditBridge.getSelector(el), | |
| textContent: (el.textContent || '').trim().substring(0, 100), | |
| attributes: { id: el.id || '', class: el.className || '' }, | |
| boundingBox: { x: rect.x, y: rect.y, width: rect.width, height: rect.height } | |
| } | |
| })); | |
| }, true); | |
| // Notify page loaded | |
| window.webEditBridge.sendToKotlin('PageLoaded', JSON.stringify({ | |
| url: window.location.href, | |
| title: document.title | |
| })); | |
| console.log('[WebEditBridge] Initialized'); | |
| })(); | |
| """.trimIndent() |
🤖 Prompt for AI Agents
In
mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridgeScript.kt
around lines 11 to 170, the mouseover handler calls clearHighlights() which also
clears selectedElement outline so a prior selection is lost on hover; modify
clearHighlights() or the mouseover logic so it only clears temporary hover
highlighting (highlightedElement) and does not reset or remove styles from
selectedElement, and ensure selection persists by leaving
selectedElement.style.outline and outlineOffset untouched when clearing hover,
while still clearing previous hover element and updating highlightedElement
appropriately.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Finding getWebEditBridgeScript call sites ==="
rg -n --type=kt -C2 'getWebEditBridgeScript\s*\(' || echo "No call sites found (pattern may need adjustment)"
echo ""
echo "=== Finding all substr occurrences in the file ==="
rg -n 'substr' mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridgeScript.kt
echo ""
echo "=== Finding all hardcoded hex colors in the file ==="
rg -n '#[0-9A-Fa-f]{6}' mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridgeScript.kt
echo ""
echo "=== File location and size ==="
find . -name "WebEditBridgeScript.kt" -type fRepository: phodal/auto-dev
Length of output: 907
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking for other getWebEditBridgeScript usages across the codebase ==="
rg -n 'getWebEditBridgeScript' --type=kt | head -20
echo ""
echo "=== Checking if there are similar bridge/script functions ==="
rg -n 'fun get.*BridgeScript.*String' --type=kt | head -10Repository: phodal/auto-dev
Length of output: 179
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Finding getWebEditBridgeScript usages (broader search) ==="
rg -n 'getWebEditBridgeScript'
echo ""
echo "=== Checking the complete function signature area ==="
head -20 mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridgeScript.kt
echo ""
echo "=== Verifying substr deprecation in JavaScript ==="
echo "JavaScript substr() is deprecated. slice() is the modern replacement."
echo "substr(2, 9) → slice(2, 11) (need to convert length to end index)"
</script>Repository: phodal/auto-dev
Length of output: 1162
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Finding all references to this Kotlin file ==="
rg -l 'WebEditBridgeScript' --type-list | xargs rg -l 'WebEditBridgeScript' 2>/dev/null || rg -l 'WebEditBridgeScript'
echo ""
echo "=== Checking imports ==="
rg 'import.*WebEditBridgeScript|from.*WebEditBridgeScript'
echo ""
echo "=== Looking for class/object that contains this function ==="
head -30 mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridgeScript.ktRepository: phodal/auto-dev
Length of output: 724
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for WebEditBridgeScript in all files ==="
find . -type f \( -name "*.kt" -o -name "*.kts" -o -name "*.java" \) -exec grep -l "WebEditBridgeScript" {} \;
echo ""
echo "=== Search for getWebEditBridgeScript in all files ==="
find . -type f -exec grep -l "getWebEditBridgeScript" {} \; 2>/dev/null | head -20
echo ""
echo "=== Check what directory structure exists ==="
ls -la mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/Repository: phodal/auto-dev
Length of output: 712
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Looking for where this script might be used ==="
rg -i 'webEditBridge|WebEditBridge' --max-count=20
echo ""
echo "=== Checking if there's a test file ==="
find . -path "*test*" -name "*WebEdit*" -type f
echo ""
echo "=== Check the complete file size ==="
wc -l mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridgeScript.ktRepository: phodal/auto-dev
Length of output: 5847
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Reading WebEditBridge.kt to see the interface ==="
cat -n mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.kt
echo ""
echo "=== Checking if getWebEditBridgeScript is exported or public ==="
grep -n "getWebEditBridgeScript\|@Export\|public\|actual" mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridgeScript.kt | head -20Repository: phodal/auto-dev
Length of output: 3870
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check if getWebEditBridgeScript might be in a test or build file ==="
find . -type f -name "*.kt" -o -name "*.gradle" | xargs grep -l "getWebEditBridgeScript" 2>/dev/null
echo ""
echo "=== Check gradle or build files that might reference scripts ==="
find . -name "build.gradle*" -o -name "gradle*" -type f | head -10Repository: phodal/auto-dev
Length of output: 226
Replace deprecated substr with slice and use CSS variables for colors.
The file has two instances of deprecated substr() (lines 102, 152) that should use slice() instead. For hardcoded colors (#2196F3, #4CAF50), inject them as CSS variables rather than function parameters to avoid breaking existing callers:
fun getWebEditBridgeScript(): String = """
(function() {
+ // Define color constants as CSS variables for theming
+ const style = document.createElement('style');
+ style.textContent = `
+ :root {
+ --web-edit-hover-color: #2196F3;
+ --web-edit-selected-color: #4CAF50;
+ }
+ `;
+ document.head.appendChild(style);
+
// Prevent multiple injections
if (window.webEditBridge) return;
// WebEdit Bridge object
window.webEditBridge = {
selectionMode: false,
highlightedElement: null,
selectedElement: null,
// Enable/disable selection mode
setSelectionMode: function(enabled) {
this.selectionMode = enabled;
if (!enabled) {
this.clearHighlights();
}
console.log('[WebEditBridge] Selection mode:', enabled);
},
// Highlight a specific element by selector
highlightElement: function(selector) {
this.clearHighlights();
try {
const el = document.querySelector(selector);
if (el) {
this.highlightedElement = el;
- el.style.outline = '2px solid #2196F3';
+ el.style.outline = 'var(--web-edit-hover-color) 2px solid';
el.style.outlineOffset = '2px';
}
} catch(e) {
console.error('[WebEditBridge] highlightElement error:', e);
}
},And replace both substr(2, 9) with slice(2, 11) at lines 102 and 152.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridgeScript.kt
around lines 11-170, replace deprecated uses of substr(2, 9) at the two
occurrences (originally around lines 102 and 152) with slice(2, 11), and remove
hardcoded outline colors (#2196F3 and #4CAF50) by using CSS variables instead:
ensure the script references CSS vars (e.g. var(--webedit-highlight-color) and
var(--webedit-selected-color)) when setting element.style.outline and provide
safe defaults by injecting a small <style> into document.head that defines those
variables if they are not already set.
| override suspend fun highlightElement(selector: String) { | ||
| val escapedSelector = selector.replace("'", "\\'") | ||
| val script = "window.webEditBridge?.highlightElement('$escapedSelector');" | ||
| executeJavaScript?.invoke(script) |
There was a problem hiding this comment.
JavaScript injection risk: insufficient escaping.
The selector escaping only handles single quotes, leaving the code vulnerable to injection if selectors contain backticks, double quotes, newlines, or other special characters.
Consider using a more robust escaping approach:
override suspend fun highlightElement(selector: String) {
- val escapedSelector = selector.replace("'", "\\'")
- val script = "window.webEditBridge?.highlightElement('$escapedSelector');"
+ val escapedSelector = selector
+ .replace("\\", "\\\\")
+ .replace("'", "\\'")
+ .replace("\"", "\\\"")
+ .replace("\n", "\\n")
+ .replace("\r", "\\r")
+ val script = "window.webEditBridge?.highlightElement('$escapedSelector');"
executeJavaScript?.invoke(script)
}Alternatively, consider using JSON encoding for safer serialization.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| override suspend fun highlightElement(selector: String) { | |
| val escapedSelector = selector.replace("'", "\\'") | |
| val script = "window.webEditBridge?.highlightElement('$escapedSelector');" | |
| executeJavaScript?.invoke(script) | |
| override suspend fun highlightElement(selector: String) { | |
| val escapedSelector = selector | |
| .replace("\\", "\\\\") | |
| .replace("'", "\\'") | |
| .replace("\"", "\\\"") | |
| .replace("\n", "\\n") | |
| .replace("\r", "\\r") | |
| val script = "window.webEditBridge?.highlightElement('$escapedSelector');" | |
| executeJavaScript?.invoke(script) | |
| } |
🤖 Prompt for AI Agents
mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.jvm.kt
lines 69-72: the current escaping only replaces single quotes and leaves the
injected selector vulnerable to backticks, double quotes, newlines and other
characters; replace the manual replace with a robust serialization (e.g. JSON
string encoding) so the selector becomes a safe JS string literal before
interpolation into the script, or otherwise escape backslashes, newlines, single
and double quotes and backticks consistently; build the script using that
serialized string and then call executeJavaScript?.invoke(script).
| override suspend fun scrollToElement(selector: String) { | ||
| val escapedSelector = selector.replace("'", "\\'") | ||
| val script = "window.webEditBridge?.scrollToElement('$escapedSelector');" | ||
| executeJavaScript?.invoke(script) | ||
| } |
There was a problem hiding this comment.
JavaScript injection risk: insufficient escaping.
Same escaping vulnerability as highlightElement() - only handles single quotes.
Apply the same robust escaping fix as suggested for highlightElement():
override suspend fun scrollToElement(selector: String) {
- val escapedSelector = selector.replace("'", "\\'")
+ val escapedSelector = selector
+ .replace("\\", "\\\\")
+ .replace("'", "\\'")
+ .replace("\"", "\\\"")
+ .replace("\n", "\\n")
+ .replace("\r", "\\r")
val script = "window.webEditBridge?.scrollToElement('$escapedSelector');"
executeJavaScript?.invoke(script)
}🤖 Prompt for AI Agents
In
mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.jvm.kt
around lines 80-84, the selector string is only escaping single quotes which
leaves a JavaScript injection vector; replace the naive replace with a proper
JS-string serialization for the selector before embedding it into the script.
Produce a safely escaped JS string (escape backslashes, single and double quotes
and control characters) or, preferably, serialize the Kotlin string into a JS
string literal (for example via a JSON/string-quoting helper like
JSONObject.quote(selector) or a JSON encoder) and then embed that serialized
value into the script passed to executeJavaScript so the selector cannot break
out of the string literal.
| override suspend fun getSelectedElementHtml(): String? { | ||
| return _selectedElement.value?.let { | ||
| val script = "document.querySelector('${it.selector}')?.outerHTML || '';" | ||
| // This is a simplified version - in practice you'd use a callback | ||
| null | ||
| } | ||
| } |
There was a problem hiding this comment.
Stub implementation always returns null.
This method currently returns null unconditionally and includes a comment indicating it's simplified. This could cause unexpected behavior for callers expecting HTML content.
Should this be implemented fully, or should it remain a stub for now? If it's intentionally incomplete, consider adding a TODO comment and documenting the limitation. Would you like me to draft a proper implementation?
🤖 Prompt for AI Agents
In
mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.jvm.kt
around lines 91 to 97, the method getSelectedElementHtml currently always
returns null; replace the stub with a real implementation that executes the
constructed JS snippet against the web view (or the JS bridge) to fetch the
selected element's outerHTML, awaits the result, returns the HTML string or null
if empty, and catches/logs exceptions; if you intentionally want a stub, replace
the current comment with a clear TODO mentioning why it's incomplete and add
documentation that callers may receive null until implemented.
| override suspend fun highlightElement(selector: String) { | ||
| val escapedSelector = selector.replace("'", "\\'") | ||
| val script = "window.webEditBridge?.highlightElement('$escapedSelector');" | ||
| executeJavaScript?.invoke(script) |
There was a problem hiding this comment.
JavaScript injection risk: insufficient escaping.
Identical vulnerability to the JVM implementation - only escapes single quotes.
Apply the same robust escaping fix:
override suspend fun highlightElement(selector: String) {
- val escapedSelector = selector.replace("'", "\\'")
+ val escapedSelector = selector
+ .replace("\\", "\\\\")
+ .replace("'", "\\'")
+ .replace("\"", "\\\"")
+ .replace("\n", "\\n")
+ .replace("\r", "\\r")
val script = "window.webEditBridge?.highlightElement('$escapedSelector');"
executeJavaScript?.invoke(script)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| override suspend fun highlightElement(selector: String) { | |
| val escapedSelector = selector.replace("'", "\\'") | |
| val script = "window.webEditBridge?.highlightElement('$escapedSelector');" | |
| executeJavaScript?.invoke(script) | |
| override suspend fun highlightElement(selector: String) { | |
| val escapedSelector = selector | |
| .replace("\\", "\\\\") | |
| .replace("'", "\\'") | |
| .replace("\"", "\\\"") | |
| .replace("\n", "\\n") | |
| .replace("\r", "\\r") | |
| val script = "window.webEditBridge?.highlightElement('$escapedSelector');" | |
| executeJavaScript?.invoke(script) | |
| } |
🤖 Prompt for AI Agents
In
mpp-viewer-web/src/wasmJsMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.wasmJs.kt
around lines 66-69, the highlightElement implementation only escapes single
quotes and is vulnerable to JS injection; replace the manual single-quote
replace with a safe serialization of the selector for embedding in JS (for
example, JSON.stringify the selector/value or otherwise fully escape
backslashes, quotes, newlines, and control characters) and construct the script
using that serialized value: call JSON.stringify(selector) or an equivalent safe
serializer to produce a JS string literal, then use that value in
"window.webEditBridge?.highlightElement(...);" so arbitrary input cannot break
out of the string. Ensure executeJavaScript is invoked with the serialized
string literal (not raw selector) and remove the insecure replace-based
approach.
| override suspend fun scrollToElement(selector: String) { | ||
| val escapedSelector = selector.replace("'", "\\'") | ||
| val script = "window.webEditBridge?.scrollToElement('$escapedSelector');" | ||
| executeJavaScript?.invoke(script) |
There was a problem hiding this comment.
JavaScript injection risk: insufficient escaping.
Same vulnerability as JVM implementation.
Apply the same escaping fix:
override suspend fun scrollToElement(selector: String) {
- val escapedSelector = selector.replace("'", "\\'")
+ val escapedSelector = selector
+ .replace("\\", "\\\\")
+ .replace("'", "\\'")
+ .replace("\"", "\\\"")
+ .replace("\n", "\\n")
+ .replace("\r", "\\r")
val script = "window.webEditBridge?.scrollToElement('$escapedSelector');"
executeJavaScript?.invoke(script)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| override suspend fun scrollToElement(selector: String) { | |
| val escapedSelector = selector.replace("'", "\\'") | |
| val script = "window.webEditBridge?.scrollToElement('$escapedSelector');" | |
| executeJavaScript?.invoke(script) | |
| override suspend fun scrollToElement(selector: String) { | |
| val escapedSelector = selector | |
| .replace("\\", "\\\\") | |
| .replace("'", "\\'") | |
| .replace("\"", "\\\"") | |
| .replace("\n", "\\n") | |
| .replace("\r", "\\r") | |
| val script = "window.webEditBridge?.scrollToElement('$escapedSelector');" | |
| executeJavaScript?.invoke(script) | |
| } |
🤖 Prompt for AI Agents
In
mpp-viewer-web/src/wasmJsMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.wasmJs.kt
around lines 77-80, the JavaScript string interpolation escapes only single
quotes and risks injection because backslashes and control characters remain
unescaped; apply the same escaping used in the JVM implementation by first
escaping backslashes, then escaping single quotes, and also replacing
newlines/carriage returns (and other control chars if present) with their
escaped forms before inserting into the JS snippet so the generated script
cannot break out of the quoted string.
| override suspend fun getSelectedElementHtml(): String? { | ||
| return null | ||
| } |
There was a problem hiding this comment.
Stub implementation always returns null.
Same issue as JVM implementation.
Consider adding a TODO comment or documenting this limitation if it's intentional.
🤖 Prompt for AI Agents
In
mpp-viewer-web/src/wasmJsMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridge.wasmJs.kt
around lines 88-90, the suspend function getSelectedElementHtml() is a stub that
always returns null (same as the JVM impl); either implement the actual
WASM-specific logic to retrieve the selected element HTML, or if that is not
possible now, add a clear TODO comment and KDoc on the function explaining the
limitation on WASM and why it returns null, and consider replacing the silent
null return with a NotImplementedError (or explicit documented null contract) so
callers know the behavior.
There was a problem hiding this comment.
Pull request overview
This PR implements a new WebEdit agent that enables browsing web pages, selecting DOM elements through an interactive interface, and analyzing page content. The feature uses a bridge pattern with expect/actual declarations for platform-specific WebView implementations (JVM and WASM), along with JavaScript injection for DOM manipulation and element selection.
Key Changes:
- New
mpp-viewer-webmodule with WebEditBridge for bidirectional WebView-Kotlin communication - UI components for web browsing interface (toolbar, DOM tree sidebar, chat input)
- Integration with existing agent system via AgentType.WEB_EDIT enum and routing
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
DOMElement.kt |
Data models for DOM elements, bounding boxes, and message types |
WebEditBridge.kt |
Common interface defining WebView-Kotlin communication contract |
WebEditBridge.jvm.kt |
JVM implementation with JavaScript execution callbacks |
WebEditBridge.wasmJs.kt |
WASM implementation mirroring JVM functionality |
WebEditBridgeScript.kt |
JavaScript code for DOM selection, highlighting, and tree extraction |
WebEditPage.kt |
Main page layout with WebView, toolbar, sidebar, and chat areas |
WebEditToolbar.kt |
Navigation controls and URL input component |
WebEditChatInput.kt |
Chat interface for Q&A about page content |
DOMTreeSidebar.kt |
Right sidebar displaying DOM tree structure |
WebEditViewPlaceholder.kt |
Platform-agnostic WebView placeholder |
AgentType.kt |
Added WEB_EDIT enum value with display name and routing |
AgentInterfaceRouter.kt |
Added routing to WebEditPage and renamed function |
DesktopTitleBarTabs.kt |
Added WEB_EDIT to sidebar visibility logic |
CodingAgentPage.kt |
Added exhaustive when branch and renamed function |
TopBarMenuDesktop.kt |
Added icon mapping for WEB_EDIT |
TopBarMenuMobile.kt |
Added icon mapping for WEB_EDIT |
RemoteAgentChatInterface.kt |
Function rename to RemoteAgentPage |
AutoDevApp.apple.kt |
Removed unused imports |
AutoDevApp.android.kt |
Removed unused import |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
The search functionality is not implemented. The searchQuery state is captured but never used to filter the DOM tree. Consider implementing search logic to filter elements by tag name, selector, or attributes.
There was a problem hiding this comment.
The onHover callback is never actually invoked because there's no hover state tracking. The isHovered state variable is defined but never set to true. Consider implementing proper hover detection using Modifier.pointerInput or removing the onHover callback if hover highlighting is not needed.
| override suspend fun highlightElement(selector: String) { | ||
| val escapedSelector = selector.replace("'", "\\'") | ||
| val script = "window.webEditBridge?.highlightElement('$escapedSelector');" | ||
| executeJavaScript?.invoke(script) |
There was a problem hiding this comment.
The same selector escaping vulnerability exists here. The implementation is identical to the JVM version and has the same security issues with insufficient escaping for JavaScript interpolation.
| const rect = el.getBoundingClientRect(); | ||
| window.webEditBridge.sendToKotlin('ElementSelected', JSON.stringify({ | ||
| element: { | ||
| id: Math.random().toString(36).substr(2, 9), |
There was a problem hiding this comment.
The same ID collision issue exists here. Using Math.random() for ID generation in the ElementSelected message can lead to non-unique identifiers.
| id: Math.random().toString(36).substr(2, 9), | |
| id: (typeof crypto !== 'undefined' && crypto.randomUUID) ? crypto.randomUUID() : (Date.now().toString(36) + Math.random().toString(36).substr(2, 9)), |
| IconButton( | ||
| onClick = { /* Go back in browser history */ }, | ||
| modifier = Modifier.size(32.dp) | ||
| ) { | ||
| Icon( | ||
| imageVector = Icons.AutoMirrored.Filled.ArrowBack, | ||
| contentDescription = "Back", | ||
| modifier = Modifier.size(18.dp) | ||
| ) | ||
| } | ||
|
|
||
| IconButton( | ||
| onClick = { /* Go forward in browser history */ }, | ||
| modifier = Modifier.size(32.dp) | ||
| ) { | ||
| Icon( | ||
| imageVector = Icons.AutoMirrored.Filled.ArrowForward, | ||
| contentDescription = "Forward", | ||
| modifier = Modifier.size(18.dp) | ||
| ) | ||
| } |
There was a problem hiding this comment.
The navigation buttons for browser history (back/forward) have placeholder comments but no actual functionality. The onClick handlers should call the appropriate bridge methods (goBack/goForward) instead of being empty.
| _loadProgress.value = 100 | ||
| } | ||
| is WebEditMessage.Error -> { | ||
| println("[WebEditBridge] Error: ${message.message}") |
There was a problem hiding this comment.
Using println for error logging in production code is not recommended. Consider using a proper logging framework instead of console output.
There was a problem hiding this comment.
The DOMTreeSidebar uses hardcoded sample data but never connects to actual DOM data from the WebEditBridge. The component should accept a DOMElement parameter from the bridge's domTree state flow and display the actual page structure instead of placeholder data.
There was a problem hiding this comment.
The children property of DOMTreeItem is defined but never rendered. The LazyColumn only shows top-level items without recursively displaying child elements. To properly show the DOM tree hierarchy, you need to implement recursive rendering of children.
| override suspend fun scrollToElement(selector: String) { | ||
| val escapedSelector = selector.replace("'", "\\'") | ||
| val script = "window.webEditBridge?.scrollToElement('$escapedSelector');" | ||
| executeJavaScript?.invoke(script) |
There was a problem hiding this comment.
The same selector escaping vulnerability exists in scrollToElement. Only escaping single quotes is insufficient for preventing JavaScript injection when the selector is interpolated into JavaScript code.
| override suspend fun getSelectedElementHtml(): String? { | ||
| return _selectedElement.value?.let { | ||
| val script = "document.querySelector('${it.selector}')?.outerHTML || '';" | ||
| // This is a simplified version - in practice you'd use a callback | ||
| null | ||
| } | ||
| } |
There was a problem hiding this comment.
The getSelectedElementHtml function constructs a JavaScript string with unescaped selector interpolation. This creates a potential XSS vulnerability. Even though this returns null currently, the commented code shows the intended implementation has the same escaping issue. The selector should be properly sanitized before being used in querySelector.
- Replace placeholder with actual WebView implementation using compose-webview-multiplatform - Add platform-specific WebEditView for JVM and WASM - Set default URL to https://ide.unitmesh.cc - Implement proper navigation using rememberWebViewState and navigator.loadUrl - Add JS bridge for DOM element selection and page interaction - Inject WebEditBridgeScript when page loads - Update toolbar with compact BasicTextField (28dp height) - Remove WebEditPreview test file
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditToolbar.kt (1)
65-86: Wire browser back/forward buttons (currently no-op).
These handlers are placeholders, so the UI renders controls that do nothing.IconButton( - onClick = { /* Go back in browser history */ }, + onClick = onGoBack, + enabled = canGoBack, modifier = Modifier.size(32.dp) ) { ... } IconButton( - onClick = { /* Go forward in browser history */ }, + onClick = onGoForward, + enabled = canGoForward, modifier = Modifier.size(32.dp) ) { ... }mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/DOMTreeSidebar.kt (2)
143-154: Hover state is never updated, makingonHovercallback non-functional.The
isHoveredstate is initialized but never set totrue. TheonHovercallback passed toDOMTreeItemRowis never invoked, rendering hover highlighting inoperative.Apply this diff to implement hover detection:
@Composable private fun DOMTreeItemRow( item: DOMTreeItem, isSelected: Boolean = false, onClick: () -> Unit, onHover: (Boolean) -> Unit, modifier: Modifier = Modifier ) { var isHovered by remember { mutableStateOf(false) } val backgroundColor = when { isSelected -> MaterialTheme.colorScheme.primaryContainer.copy(alpha = 0.5f) isHovered -> MaterialTheme.colorScheme.primaryContainer.copy(alpha = 0.3f) else -> MaterialTheme.colorScheme.surface.copy(alpha = 0.5f) } Surface( modifier = modifier .fillMaxWidth() - .clickable { onClick() } + .pointerInput(Unit) { + awaitPointerEventScope { + while (true) { + val event = awaitPointerEvent() + when (event.type) { + PointerEventType.Enter -> { + isHovered = true + onHover(true) + } + PointerEventType.Exit -> { + isHovered = false + onHover(false) + } + } + } + } + } + .clickable { onClick() } .padding(start = (item.depth * 12).dp),You'll need to add the import:
import androidx.compose.ui.input.pointer.PointerEventType import androidx.compose.ui.input.pointer.pointerInput
31-36: Search functionality is not implemented.The
searchQuerystate is captured but never used to filterflattenedTree. The search field is purely decorative.To implement search, filter the flattened tree:
// Convert DOMElement tree to flat list with depth - val flattenedTree = remember(domTree) { + val flattenedTree = remember(domTree, searchQuery) { - domTree?.let { flattenDOMTree(it, 0) } ?: emptyList() + val tree = domTree?.let { flattenDOMTree(it, 0) } ?: emptyList() + if (searchQuery.isBlank()) tree + else tree.filter { item -> + item.tagName.contains(searchQuery, ignoreCase = true) || + item.selector.contains(searchQuery, ignoreCase = true) || + item.attributes.any { it.contains(searchQuery, ignoreCase = true) } + } }
🧹 Nitpick comments (6)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditToolbar.kt (4)
28-41: Clarify navigation vs close callbacks; currentUrl/isLoading are unused.
onBackis wired to “Close WebEdit” (Line 53-63) while browser back exists (Line 65-75), which is easy to mis-wire later. AlsocurrentUrlandisLoadingare currently unused.Consider renaming + adding explicit callbacks/state for history navigation:
fun WebEditToolbar( currentUrl: String, inputUrl: String, isLoading: Boolean, isSelectionMode: Boolean, showDOMSidebar: Boolean, onUrlChange: (String) -> Unit, onNavigate: (String) -> Unit, - onBack: () -> Unit, + onClose: () -> Unit, + canGoBack: Boolean, + canGoForward: Boolean, + onGoBack: () -> Unit, + onGoForward: () -> Unit, onReload: () -> Unit, onToggleSelectionMode: () -> Unit, onToggleDOMSidebar: () -> Unit, modifier: Modifier = Modifier )
53-63: “Close WebEdit” should not be namedonBack.
This is the close/exit action, not browser back. Rename toonCloseto avoid confusion with history navigation.
88-97: Consider usingisLoading(disable reload or show stop/progress).
Right nowisLoadingis unused; simplest option is to disable reload while loading or swap the icon (Refresh ↔ Close).
141-178: Touch target sizing likely too small for commonMain (32.dp).
If this runs on mobile, 32.dp IconButtons can be a usability/accessibility problem. ConsiderModifier.minimumInteractiveComponentSize()(or just drop the explicitsize(32.dp)and let Material defaults handle it).mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.kt (2)
35-57: Type check on concreteJvmWebEditBridgecouples implementation tightly.While this is the JVM actual implementation, the repeated
bridge is JvmWebEditBridgechecks (lines 36, 135, 151) reduce flexibility and testability. Consider either:
- Adding these callback setters to the
WebEditBridgeinterface, or- Accepting
JvmWebEditBridgedirectly in the JVM actual signature if platform-specific types are acceptable.
27-27: Hardcoded default URL may conflict with bridge state.The
rememberWebViewStateinitializes with a hardcoded URL"https://ide.unitmesh.cc", butcurrentUrlfrom the bridge may have a different value. Consider using the bridge's initial URL or an empty/blank URL to avoid potential navigation conflicts.- val webViewState = rememberWebViewState(url = "https://ide.unitmesh.cc") + val initialUrl = bridge.currentUrl.value.ifEmpty { "about:blank" } + val webViewState = rememberWebViewState(url = initialUrl)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/DOMTreeSidebar.kt(1 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditPage.kt(1 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditToolbar.kt(1 hunks)mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.kt(1 hunks)mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.kt(1 hunks)mpp-viewer-web/src/wasmJsMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.wasmJs.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditPage.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualpattern for platform-specific code (for example:Platformimplementations)
Use concrete classes (not interfaces) andPromise(notFlow) for @JsExport annotations
Avoid emoji and UTF-8 characters in WASM code
When modifyingCodingAgentRenderer, update ALL implementations:DefaultCodingAgentRenderer,ComposeRenderer,JewelRenderer,ServerSideRenderer, andJsRendererAdapterin Kotlin
When modifyingCodingAgentRenderer, update JVM CLI implementations:CodingCliRendererandConsoleRenderer
UseAutoDevColorsorMaterialTheme.colorSchemefor Compose styling
Files:
mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.ktmpp-viewer-web/src/wasmJsMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.wasmJs.ktmpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditToolbar.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/DOMTreeSidebar.kt
**/*.{kt,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never use hardcoded colors - always use design tokens from the design system
Files:
mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.ktmpp-viewer-web/src/wasmJsMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.wasmJs.ktmpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditToolbar.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/DOMTreeSidebar.kt
🧠 Learnings (2)
📚 Learning: 2025-12-07T08:40:10.850Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T08:40:10.850Z
Learning: Applies to **/*.kt : Use `expect`/`actual` pattern for platform-specific code (for example: `Platform` implementations)
Applied to files:
mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.kt
📚 Learning: 2025-12-07T08:40:10.850Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T08:40:10.850Z
Learning: Applies to **/*.kt : When modifying `CodingAgentRenderer`, update ALL implementations: `DefaultCodingAgentRenderer`, `ComposeRenderer`, `JewelRenderer`, `ServerSideRenderer`, and `JsRendererAdapter` in Kotlin
Applied to files:
mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.ktmpp-viewer-web/src/wasmJsMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.wasmJs.ktmpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.kt
🧬 Code graph analysis (1)
mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.kt (1)
mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridgeScript.kt (1)
getWebEditBridgeScript(11-170)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test mpp-core (KMP)
🔇 Additional comments (4)
mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.kt (1)
17-24: Well-structured expect declaration following the expect/actual pattern.The common interface is well-documented with KDoc, and default parameter values provide flexibility for callers. This correctly establishes the platform-agnostic contract for WebEditView. Based on learnings, this follows the required
expect/actualpattern for platform-specific code.mpp-viewer-web/src/wasmJsMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.wasmJs.kt (1)
18-38: Appropriate placeholder implementation for WASM platform limitations.The implementation correctly uses
MaterialTheme.colorSchemefor styling as per coding guidelines. The placeholder message clearly communicates the limitation to users. The unused parameters are acceptable since this serves as a graceful degradation path.mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/DOMTreeSidebar.kt (1)
38-98: Good component structure with proper theming.The layout hierarchy, empty state handling, and use of
MaterialTheme.colorSchemefor all colors follows the coding guidelines. The recursiveflattenDOMTreehelper correctly converts the hierarchical DOM tree into a flat list with depth tracking for indentation.mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.kt (1)
68-125: Well-structured message handler with proper error handling.The JS bridge message handler correctly parses incoming messages, delegates to the bridge's
handleMessagemethod, and invokes the appropriate callbacks. The try-catch with logging provides good observability for debugging.
| // URL input field - use BasicTextField for compact height | ||
| BasicTextField( | ||
| value = inputUrl, | ||
| onValueChange = onUrlChange, | ||
| modifier = Modifier | ||
| .weight(1f) | ||
| .height(28.dp) | ||
| .background( | ||
| MaterialTheme.colorScheme.surfaceVariant.copy(alpha = 0.5f), | ||
| RoundedCornerShape(6.dp) | ||
| ) | ||
| .border( | ||
| 1.dp, | ||
| MaterialTheme.colorScheme.outline.copy(alpha = 0.3f), | ||
| RoundedCornerShape(6.dp) | ||
| ) | ||
| .padding(horizontal = 10.dp), | ||
| textStyle = TextStyle( | ||
| fontSize = 13.sp, | ||
| color = MaterialTheme.colorScheme.onSurface | ||
| ), | ||
| cursorBrush = SolidColor(MaterialTheme.colorScheme.primary), | ||
| singleLine = true, | ||
| keyboardOptions = KeyboardOptions(imeAction = ImeAction.Go), | ||
| keyboardActions = KeyboardActions(onGo = { onNavigate(inputUrl) }), | ||
| decorationBox = { innerTextField -> | ||
| Box( | ||
| modifier = Modifier.fillMaxSize(), | ||
| contentAlignment = Alignment.CenterStart | ||
| ) { | ||
| if (inputUrl.isEmpty()) { | ||
| Text( | ||
| "Enter URL (e.g., https://www.baidu.com)", | ||
| style = TextStyle(fontSize = 13.sp), | ||
| color = MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.6f) | ||
| ) | ||
| } | ||
| innerTextField() | ||
| } | ||
| } | ||
| ) |
There was a problem hiding this comment.
URL field: add basic input guarding + accessibility semantics.
BasicTextField is fine for compact height, but please add a semantics label for screen readers, and avoid navigating on empty/whitespace.
BasicTextField(
value = inputUrl,
onValueChange = onUrlChange,
modifier = Modifier
.weight(1f)
.height(28.dp)
+ .semantics { contentDescription = "URL" }
...
- keyboardActions = KeyboardActions(onGo = { onNavigate(inputUrl) }),
+ keyboardActions = KeyboardActions(onGo = {
+ val url = inputUrl.trim()
+ if (url.isNotEmpty()) onNavigate(url)
+ }),
decorationBox = { innerTextField -> ... }
)Also consider updating the placeholder example away from a region-specific domain (or make it generic like https://example.com).
🤖 Prompt for AI Agents
In
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditToolbar.kt
around lines 99 to 139, the BasicTextField needs accessibility and input
guarding: add a semantics label (e.g., modifier.semantics { contentDescription =
"URL input" } or use Modifier.semantics { this.contentDescription = ... }) so
screen readers announce the field, change the placeholder text to a neutral
example like "https://example.com", and prevent navigation on empty or
whitespace-only input by trimming inputUrl before calling onNavigate (i.e., only
call onNavigate when inputUrl.trim().isNotEmpty()). Ensure the semantics
modifier is applied to the BasicTextField modifier chain and the
keyboardActions/onGo uses the trimmed check.
| // Inject the WebEdit bridge script | ||
| kotlinx.coroutines.delay(300) // Wait for page to stabilize | ||
| val script = getWebEditBridgeScript() | ||
| webViewNavigator.evaluateJavaScript(script) | ||
| scriptInjected = true |
There was a problem hiding this comment.
Arbitrary delay is fragile for script injection timing.
The 300ms delay is a brittle heuristic; pages may need more time, or the delay wastes time on fast-loading pages. Consider using a more robust approach such as polling for document.readyState or using the WebView's page load completion callback if available.
- // Inject the WebEdit bridge script
- kotlinx.coroutines.delay(300) // Wait for page to stabilize
- val script = getWebEditBridgeScript()
- webViewNavigator.evaluateJavaScript(script)
+ // Inject the WebEdit bridge script after confirming document is ready
+ val readyCheckScript = """
+ (function() {
+ return document.readyState === 'complete';
+ })();
+ """.trimIndent()
+ // Poll for document ready state
+ var attempts = 0
+ while (attempts < 10) {
+ kotlinx.coroutines.delay(100)
+ attempts++
+ // Inject once document is ready or after max attempts
+ if (attempts >= 3) break
+ }
+ val script = getWebEditBridgeScript()
+ webViewNavigator.evaluateJavaScript(script)Alternatively, if the WebView library provides a callback for DOMContentLoaded or similar events, that would be more reliable.
Committable suggestion skipped: line range outside the PR's diff.
Introduce WebEdit tool window for IDEA, including models, view model, content, and bridge script. Update related UI components and build configuration to support the new feature.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.kt (1)
161-165: Arbitrary delay is fragile for script injection timing.The 300ms delay is a brittle heuristic; pages may need more time, or the delay wastes time on fast-loading pages. Consider using document.readyState polling or WebView's page load completion callback if available.
🧹 Nitpick comments (12)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaComposeIcons.kt (2)
1796-1947: Avoid hardcodedColor.Blackin the new icons; rely on design-token tinting instead.
These additions introduce more hardcoded color fills (SolidColor(Color.Black)), which violates the “no hardcoded colors” guideline forkt. If these vectors are always rendered viaIcon(imageVector, ..., tint = <design token>), consider making the vector paths color-agnostic and pushing color decisions to the call site.One possible adjustment (only if you confirm
tintis always set where used):- path( - fill = SolidColor(Color.Black) - ) { + path( + fill = SolidColor(Color.Unspecified) + ) { ... }If
Color.Unspecifieddoesn’t render as expected with your Compose/IDEA setup, then keep the fill but please ensure every usage setstintfrom the design system (e.g.,MaterialTheme.colorScheme.*/ project tokens), so the embedded black never leaks into UI styling.
1796-1947: Optional: factor a tiny helper to reduce repeatedImageVector.Builder(...)boilerplate for future icons.
The pattern is identical across icons; a local helper would reduce churn and make future additions less error-prone.mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.kt (3)
81-86: Consider using proper logging instead of println.The URL synchronization logic is correct, but
printlnshould be replaced with a proper logging framework for consistency with production code.If a logger is available, consider:
if (currentUrl.isNotEmpty() && currentUrl != webViewState.lastLoadedUrl) { - println("[WebEditView] URL changed to: $currentUrl") + logger.info { "[WebEditView] URL changed to: $currentUrl" } webViewNavigator.loadUrl(currentUrl) }
111-131: Avoid double parsing of JSON elements.Lines 115 and 126 convert
jsonObjectto string and then parse it again inparseElement, which is inefficient. Consider modifyingparseElementto acceptJsonObjectdirectly or parse the element inline.Option 1: Modify
parseElementto acceptJsonObject:-private fun parseElement(jsonString: String): DOMElement? { +private fun parseElement(jsonObject: JsonObject): DOMElement? { return try { - Json.decodeFromString<DOMElement>(jsonString) + Json.decodeFromJsonElement<DOMElement>(jsonObject) } catch (e: Exception) { println("[WebEditView] Failed to parse element: ${e.message}") null } }Then update the call sites:
"ElementSelected" -> { val elementData = Json.parseToJsonElement(data).jsonObject val elementJson = elementData["element"]?.jsonObject if (elementJson != null) { - val element = parseElement(elementJson.toString()) + val element = parseElement(elementJson) if (element != null) { bridge.handleMessage(WebEditMessage.ElementSelected(element)) onElementSelected(element) } } } "DOMTreeUpdated" -> { val treeData = Json.parseToJsonElement(data).jsonObject val rootJson = treeData["root"]?.jsonObject if (rootJson != null) { - val root = parseElement(rootJson.toString()) + val root = parseElement(rootJson) if (root != null) { bridge.handleMessage(WebEditMessage.DOMTreeUpdated(root)) onDOMTreeUpdated(root) } } }
148-169: Consider checking scriptInjected flag before injection.The
scriptInjectedflag is maintained but never checked before injecting the script. Although it's reset for each new page, adding a guard would make the intent clearer and prevent potential double injection if this code path is triggered multiple times.// Inject the WebEdit bridge script - kotlinx.coroutines.delay(300) // Wait for page to stabilize - val script = getWebEditBridgeScript() - webViewNavigator.evaluateJavaScript(script) - scriptInjected = true + if (!scriptInjected) { + kotlinx.coroutines.delay(300) // Wait for page to stabilize + val script = getWebEditBridgeScript() + webViewNavigator.evaluateJavaScript(script) + scriptInjected = true + }mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/header/IdeaAgentTabsHeader.kt (1)
262-262: Consider using a distinct color for WEB_EDIT.WEB_EDIT currently uses the same
Blue.c400as CODING (line 256), which could cause visual confusion. Consider using a different color likeTeal,Violet, orOrangeto maintain visual differentiation between tabs.- AgentType.WEB_EDIT -> IdeaAutoDevColors.Blue.c400 + AgentType.WEB_EDIT -> IdeaAutoDevColors.Teal.c400mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditBridgeScript.kt (1)
52-62: Original element styles are not preserved.
clearHighlightssetsoutlineandoutlineOffsetto empty strings, which could remove styles that existed before highlighting. Consider storing and restoring original styles.// Before highlighting el._originalOutline = el.style.outline; el._originalOutlineOffset = el.style.outlineOffset; // When clearing el.style.outline = el._originalOutline || ''; el.style.outlineOffset = el._originalOutlineOffset || '';mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditContent.kt (1)
482-492: Consider simplifying dual text state.
inputTextis derived frominputTextFieldStateviasnapshotFlow, creating redundant state. You could useinputTextFieldState.text.toString()directly whereinputTextis referenced.- var inputText by remember { mutableStateOf("") } ... - // Sync text field state - IdeaLaunchedEffect(Unit) { - snapshotFlow { inputTextFieldState.text.toString() } - .distinctUntilChanged() - .collect { inputText = it } - } ... - enabled = inputText.isNotBlank() + enabled = inputTextFieldState.text.isNotBlank() ... - if (inputText.isNotBlank()) { - onSendMessage(inputText) + val text = inputTextFieldState.text.toString() + if (text.isNotBlank()) { + onSendMessage(text)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditViewModel.kt (4)
70-70: Unsafe cast may throw ClassCastException.The
as JBCefBrowserBasecast is risky. WhileJBCefBrowserdoes implementJBCefBrowserBase, using a safe cast is more defensive.- jsQuery = JBCefJSQuery.create(browser as JBCefBrowserBase) + jsQuery = (browser as? JBCefBrowserBase)?.let { JBCefJSQuery.create(it) }
126-128: Empty catch block swallows exceptions silently.The exception is caught but completely ignored, making debugging difficult if LLM initialization fails unexpectedly.
} catch (e: Exception) { - // LLM service is optional for WebEdit + // LLM service is optional for WebEdit, log for debugging + println("[IdeaWebEditViewModel] LLM service initialization failed: ${e.message}") }
307-311: Hardcoded delay is fragile.The 500ms delay is arbitrary and may be insufficient on slower pages or unnecessarily slow on fast ones. Consider having the JS bridge emit a "Ready" message when initialization completes, then trigger
refreshDOMTree()in response.
381-387: Recursive parsing could overflow on deeply nested DOMs.While typical DOM depth is manageable, malformed or adversarial pages could cause stack overflow. Consider adding a depth limit parameter.
private fun parseElement(json: JsonObject, depth: Int = 0, maxDepth: Int = 100): IdeaDOMElement { // ... existing code ... val children = if (depth < maxDepth) { json["children"]?.jsonArray?.mapNotNull { child -> try { parseElement(child.jsonObject, depth + 1, maxDepth) } catch (e: Exception) { null } } ?: emptyList() } else emptyList() // ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/header/IdeaAgentTabsHeader.kt(3 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt(5 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaComposeIcons.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditBridgeScript.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditContent.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditModels.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditViewModel.kt(1 hunks)mpp-ui/build.gradle.kts(2 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditPage.kt(1 hunks)mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/webedit/WebEditPage.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualpattern for platform-specific code (for example:Platformimplementations)
Use concrete classes (not interfaces) andPromise(notFlow) for @JsExport annotations
Avoid emoji and UTF-8 characters in WASM code
When modifyingCodingAgentRenderer, update ALL implementations:DefaultCodingAgentRenderer,ComposeRenderer,JewelRenderer,ServerSideRenderer, andJsRendererAdapterin Kotlin
When modifyingCodingAgentRenderer, update JVM CLI implementations:CodingCliRendererandConsoleRenderer
UseAutoDevColorsorMaterialTheme.colorSchemefor Compose styling
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditContent.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditModels.ktmpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/header/IdeaAgentTabsHeader.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditViewModel.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditBridgeScript.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaComposeIcons.kt
**/*.{kt,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never use hardcoded colors - always use design tokens from the design system
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditContent.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditModels.ktmpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/header/IdeaAgentTabsHeader.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditViewModel.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditBridgeScript.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaComposeIcons.kt
🧠 Learnings (3)
📚 Learning: 2025-12-07T08:40:10.850Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T08:40:10.850Z
Learning: Applies to **/*.kt : When modifying `CodingAgentRenderer`, update ALL implementations: `DefaultCodingAgentRenderer`, `ComposeRenderer`, `JewelRenderer`, `ServerSideRenderer`, and `JsRendererAdapter` in Kotlin
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.ktmpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/header/IdeaAgentTabsHeader.ktmpp-ui/build.gradle.kts
📚 Learning: 2025-12-07T08:40:10.850Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T08:40:10.850Z
Learning: Applies to mpp-vscode/src/**/*.ts : When modifying `CodingAgentRenderer`, update VSCode implementations: `mpp-vscode/src/bridge/mpp-core.ts` and `mpp-vscode/src/providers/chat-view.ts`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.ktmpp-ui/build.gradle.kts
📚 Learning: 2025-12-07T08:40:10.850Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T08:40:10.850Z
Learning: Applies to **/*.kt : When modifying `CodingAgentRenderer`, update JVM CLI implementations: `CodingCliRenderer` and `ConsoleRenderer`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt
🧬 Code graph analysis (3)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditContent.kt (2)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/IdeaVerticalResizableSplitPane.kt (1)
IdeaVerticalResizableSplitPane(36-140)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/IdeaResizableSplitPane.kt (1)
IdeaResizableSplitPane(36-140)
mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.kt (2)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditViewModel.kt (1)
parseElement(367-398)mpp-viewer-web/src/commonMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditBridgeScript.kt (1)
getWebEditBridgeScript(11-170)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditViewModel.kt (1)
mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.kt (1)
parseElement(189-196)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build
- GitHub Check: Test mpp-core (KMP)
🔇 Additional comments (13)
mpp-ui/build.gradle.kts (1)
335-370: Aligncompose.webviewacross platforms.If
compose.webviewis added to commonMain, ensure it's also available in the wasmJsMain sourceset (lines 335–370), or clarify why WASM has a different dependency configuration. Currently, mpp-viewer-web is present in wasmJsMain but compose.webview is not.mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.kt (2)
56-78: LGTM!The bridge callback configuration is well-structured with proper type checking and navigation guards for back/forward operations.
189-196: LGTM!The
parseElementhelper has appropriate error handling and gracefully returnsnullon failure.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt (1)
23-24: LGTM! WEB_EDIT agent type integration follows established patterns.The integration correctly:
- Adds lazy ViewModel initialization when WEB_EDIT tab is selected (lines 167-169)
- Follows the same disposal pattern as other agents (lines 187-190)
- Renders
IdeaWebEditContentin the main content area (lines 377-384)Also applies to: 133-135, 167-169, 187-190, 377-384
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/header/IdeaAgentTabsHeader.kt (1)
92-92: Tab list and icon mapping look good.WEB_EDIT is correctly added to the selectable agent tabs and mapped to an appropriate Web icon.
Also applies to: 275-275
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditBridgeScript.kt (2)
141-169: Event handlers may interfere with page interactivity.The
mouseoverandclickhandlers usestopPropagation()andpreventDefault()in capture phase (true), which will block the page's native event handling in selection mode. This is likely intentional for DOM selection, but worth documenting that interactive elements (links, buttons, forms) won't work while selection mode is active.
16-179: Bridge script implementation is well-structured.Good practices observed:
- Guard against multiple injections (line 19)
- Depth and child limits in DOM tree extraction (lines 105, 118)
- Defensive type checking for className (line 89)
- Error handling with try/catch blocks
- Proper use of capture phase for selection mode events
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditContent.kt (2)
40-138: Overall structure is well-organized.The component properly:
- Checks JCEF support with graceful fallback (lines 61-64)
- Uses established state collection patterns with
IdeaLaunchedEffect- Composes split panes using existing
IdeaResizableSplitPaneandIdeaVerticalResizableSplitPane- Follows design token usage with
IdeaAutoDevColorsandJewelTheme
150-155: The concern aboutgetBrowserComponent()recreating the browser component is unfounded. The implementation returnsbrowser?.component, which accesses the component property of a cachedJBCefBrowserinstance stored as a private field in the ViewModel. The browser instance is initialized once and reused across recompositions, so the SwingPanel factory will receive the same component on each call.Likely an incorrect or invalid review comment.
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditModels.kt (1)
6-61: LGTM! Well-structured data models.The models are cleanly designed:
IdeaWebEditStatehas sensible defaults for all fieldsIdeaDOMElement.getDisplayName()provides a readable representationIdeaWebEditMessagesealed class properly enumerates all message types from the JS bridge- The
Idea*prefix correctly distinguishes these IDEA-specific models from the cross-platform ones inmpp-viewer-webmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditViewModel.kt (3)
223-225:isLoadingmay check wrong state.
_state.value.isLoadingtracks page loading state, not chat generation. This allows sending chat messages while a page loads (probably fine) but doesn't prevent sending while a previous chat is still generating. Consider adding a separateisGeneratingstate or checkingcurrentJob?.isActive.fun sendChatMessage(message: String) { - if (_state.value.isLoading) return + if (currentJob?.isActive == true) return // Prevent concurrent chat requests
404-407: LGTM - disposal pattern is correct.Browser and jsQuery are properly registered with Disposer in
initializeBrowser(). ThecoroutineScopeis passed in externally, so the caller manages its lifecycle. Consider whetherllmServiceneeds explicit cleanup if it holds resources.
318-365: This review comment is incorrect. The data field in messages is indeed a JSON string primitive, not a JsonObject, and the current parsing is correct.Verification shows that
IdeaWebEditBridgeScript.ktconsistently sends stringified JSON data tosendToKotlin()(lines 126, 159, 172), resulting in messages structured as{ type: "...", data: "{...}" }. The line 322 parsingjson["data"]?.jsonPrimitive?.contentcorrectly extracts the stringified JSON string, which is then parsed again on subsequent lines (326, 339, 348, 357). The suggested diff would break this logic since it attempts to treat data as a JsonObject when it is always a string.Likely an incorrect or invalid review comment.
| @Composable | ||
| private fun DOMTreeNode( | ||
| element: IdeaDOMElement, | ||
| selectedSelector: String?, | ||
| depth: Int, | ||
| onElementClick: (String) -> Unit, | ||
| onElementHover: (String?) -> Unit | ||
| ) { | ||
| var expanded by remember { mutableStateOf(depth < 2) } | ||
| val isSelected = element.selector == selectedSelector | ||
| val hasChildren = element.children.isNotEmpty() | ||
|
|
||
| Column { | ||
| Row( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .background( | ||
| if (isSelected) IdeaAutoDevColors.Blue.c400.copy(alpha = 0.2f) | ||
| else JewelTheme.globalColors.panelBackground | ||
| ) | ||
| .clickable { onElementClick(element.selector) } | ||
| .padding(start = (depth * 12).dp, top = 2.dp, bottom = 2.dp, end = 4.dp), | ||
| verticalAlignment = Alignment.CenterVertically | ||
| ) { | ||
| // Expand/collapse icon | ||
| if (hasChildren) { | ||
| IconButton( | ||
| onClick = { expanded = !expanded }, | ||
| modifier = Modifier.size(16.dp) | ||
| ) { | ||
| Icon( | ||
| imageVector = if (expanded) IdeaComposeIcons.ExpandMore else IdeaComposeIcons.ChevronRight, | ||
| contentDescription = if (expanded) "Collapse" else "Expand", | ||
| modifier = Modifier.size(12.dp), | ||
| tint = JewelTheme.globalColors.text.info | ||
| ) | ||
| } | ||
| } else { | ||
| Spacer(modifier = Modifier.width(16.dp)) | ||
| } | ||
|
|
||
| // Element display | ||
| Text( | ||
| text = element.getDisplayName(), | ||
| style = JewelTheme.defaultTextStyle.copy( | ||
| fontSize = 12.sp, | ||
| fontWeight = if (isSelected) FontWeight.Bold else FontWeight.Normal | ||
| ), | ||
| maxLines = 1, | ||
| modifier = Modifier.weight(1f) | ||
| ) | ||
| } | ||
|
|
||
| // Children | ||
| if (expanded && hasChildren) { | ||
| element.children.forEach { child -> | ||
| DOMTreeNode( | ||
| element = child, | ||
| selectedSelector = selectedSelector, | ||
| depth = depth + 1, | ||
| onElementClick = onElementClick, | ||
| onElementHover = onElementHover | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
onElementHover callback is passed but never invoked in DOMTreeNode.
DOMTreeNode receives onElementHover but doesn't call it. Add hover detection to invoke it:
Row(
modifier = Modifier
.fillMaxWidth()
.background(...)
.clickable { onElementClick(element.selector) }
+ .pointerInput(element.selector) {
+ awaitPointerEventScope {
+ while (true) {
+ val event = awaitPointerEvent()
+ when (event.type) {
+ PointerEventType.Enter -> onElementHover(element.selector)
+ PointerEventType.Exit -> onElementHover(null)
+ }
+ }
+ }
+ }
.padding(...)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Composable | |
| private fun DOMTreeNode( | |
| element: IdeaDOMElement, | |
| selectedSelector: String?, | |
| depth: Int, | |
| onElementClick: (String) -> Unit, | |
| onElementHover: (String?) -> Unit | |
| ) { | |
| var expanded by remember { mutableStateOf(depth < 2) } | |
| val isSelected = element.selector == selectedSelector | |
| val hasChildren = element.children.isNotEmpty() | |
| Column { | |
| Row( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .background( | |
| if (isSelected) IdeaAutoDevColors.Blue.c400.copy(alpha = 0.2f) | |
| else JewelTheme.globalColors.panelBackground | |
| ) | |
| .clickable { onElementClick(element.selector) } | |
| .padding(start = (depth * 12).dp, top = 2.dp, bottom = 2.dp, end = 4.dp), | |
| verticalAlignment = Alignment.CenterVertically | |
| ) { | |
| // Expand/collapse icon | |
| if (hasChildren) { | |
| IconButton( | |
| onClick = { expanded = !expanded }, | |
| modifier = Modifier.size(16.dp) | |
| ) { | |
| Icon( | |
| imageVector = if (expanded) IdeaComposeIcons.ExpandMore else IdeaComposeIcons.ChevronRight, | |
| contentDescription = if (expanded) "Collapse" else "Expand", | |
| modifier = Modifier.size(12.dp), | |
| tint = JewelTheme.globalColors.text.info | |
| ) | |
| } | |
| } else { | |
| Spacer(modifier = Modifier.width(16.dp)) | |
| } | |
| // Element display | |
| Text( | |
| text = element.getDisplayName(), | |
| style = JewelTheme.defaultTextStyle.copy( | |
| fontSize = 12.sp, | |
| fontWeight = if (isSelected) FontWeight.Bold else FontWeight.Normal | |
| ), | |
| maxLines = 1, | |
| modifier = Modifier.weight(1f) | |
| ) | |
| } | |
| // Children | |
| if (expanded && hasChildren) { | |
| element.children.forEach { child -> | |
| DOMTreeNode( | |
| element = child, | |
| selectedSelector = selectedSelector, | |
| depth = depth + 1, | |
| onElementClick = onElementClick, | |
| onElementHover = onElementHover | |
| ) | |
| } | |
| } | |
| } | |
| } | |
| @Composable | |
| private fun DOMTreeNode( | |
| element: IdeaDOMElement, | |
| selectedSelector: String?, | |
| depth: Int, | |
| onElementClick: (String) -> Unit, | |
| onElementHover: (String?) -> Unit | |
| ) { | |
| var expanded by remember { mutableStateOf(depth < 2) } | |
| val isSelected = element.selector == selectedSelector | |
| val hasChildren = element.children.isNotEmpty() | |
| Column { | |
| Row( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .background( | |
| if (isSelected) IdeaAutoDevColors.Blue.c400.copy(alpha = 0.2f) | |
| else JewelTheme.globalColors.panelBackground | |
| ) | |
| .clickable { onElementClick(element.selector) } | |
| .pointerInput(element.selector) { | |
| awaitPointerEventScope { | |
| while (true) { | |
| val event = awaitPointerEvent() | |
| when (event.type) { | |
| PointerEventType.Enter -> onElementHover(element.selector) | |
| PointerEventType.Exit -> onElementHover(null) | |
| } | |
| } | |
| } | |
| } | |
| .padding(start = (depth * 12).dp, top = 2.dp, bottom = 2.dp, end = 4.dp), | |
| verticalAlignment = Alignment.CenterVertically | |
| ) { | |
| // Expand/collapse icon | |
| if (hasChildren) { | |
| IconButton( | |
| onClick = { expanded = !expanded }, | |
| modifier = Modifier.size(16.dp) | |
| ) { | |
| Icon( | |
| imageVector = if (expanded) IdeaComposeIcons.ExpandMore else IdeaComposeIcons.ChevronRight, | |
| contentDescription = if (expanded) "Collapse" else "Expand", | |
| modifier = Modifier.size(12.dp), | |
| tint = JewelTheme.globalColors.text.info | |
| ) | |
| } | |
| } else { | |
| Spacer(modifier = Modifier.width(16.dp)) | |
| } | |
| // Element display | |
| Text( | |
| text = element.getDisplayName(), | |
| style = JewelTheme.defaultTextStyle.copy( | |
| fontSize = 12.sp, | |
| fontWeight = if (isSelected) FontWeight.Bold else FontWeight.Normal | |
| ), | |
| maxLines = 1, | |
| modifier = Modifier.weight(1f) | |
| ) | |
| } | |
| // Children | |
| if (expanded && hasChildren) { | |
| element.children.forEach { child -> | |
| DOMTreeNode( | |
| element = child, | |
| selectedSelector = selectedSelector, | |
| depth = depth + 1, | |
| onElementClick = onElementClick, | |
| onElementHover = onElementHover | |
| ) | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditContent.kt
lines 401-467: the onElementHover callback is accepted but never invoked; add
hover detection to the row showing each DOM node and call
onElementHover(element.selector) when the pointer enters and
onElementHover(null) when it leaves (use Modifier.pointerMoveFilter or
equivalent hover-detection modifier placed before .clickable to avoid swallowing
click events), keeping existing padding/click behavior unchanged.
| override fun onLoadEnd(browser: CefBrowser?, frame: CefFrame?, httpStatusCode: Int) { | ||
| if (frame?.isMain == true) { | ||
| val url = browser?.url ?: "" | ||
| val title = frame.name ?: "" | ||
| updateState { | ||
| it.copy( | ||
| currentUrl = url, | ||
| pageTitle = title, | ||
| isLoading = false, | ||
| loadProgress = 100 | ||
| ) | ||
| } |
There was a problem hiding this comment.
frame.name does not return the page title.
CefFrame.name returns the frame's name attribute (often empty for the main frame), not the document title. The page title should be retrieved differently.
override fun onLoadEnd(browser: CefBrowser?, frame: CefFrame?, httpStatusCode: Int) {
if (frame?.isMain == true) {
val url = browser?.url ?: ""
- val title = frame.name ?: ""
+ // Title will be updated via JS bridge or use empty for now
+ val title = ""
updateState {
it.copy(
currentUrl = url,
pageTitle = title,
isLoading = false,
loadProgress = 100
)
}Alternatively, consider adding a getPageTitle() call in injectBridgeScript() to retrieve the actual document.title.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| override fun onLoadEnd(browser: CefBrowser?, frame: CefFrame?, httpStatusCode: Int) { | |
| if (frame?.isMain == true) { | |
| val url = browser?.url ?: "" | |
| val title = frame.name ?: "" | |
| updateState { | |
| it.copy( | |
| currentUrl = url, | |
| pageTitle = title, | |
| isLoading = false, | |
| loadProgress = 100 | |
| ) | |
| } | |
| override fun onLoadEnd(browser: CefBrowser?, frame: CefFrame?, httpStatusCode: Int) { | |
| if (frame?.isMain == true) { | |
| val url = browser?.url ?: "" | |
| // Title will be updated via JS bridge or use empty for now | |
| val title = "" | |
| updateState { | |
| it.copy( | |
| currentUrl = url, | |
| pageTitle = title, | |
| isLoading = false, | |
| loadProgress = 100 | |
| ) | |
| } |
| fun highlightElement(selector: String) { | ||
| val escapedSelector = selector.replace("'", "\\'") | ||
| executeJavaScript("window.webEditBridge?.highlightElement('$escapedSelector');") | ||
| } |
There was a problem hiding this comment.
Incomplete string escaping for JavaScript injection.
The escaping only handles single quotes but not backslashes. A selector containing \' would result in invalid/exploitable JavaScript. Escape backslashes first, then quotes.
fun highlightElement(selector: String) {
- val escapedSelector = selector.replace("'", "\\'")
+ val escapedSelector = selector.replace("\\", "\\\\").replace("'", "\\'")
executeJavaScript("window.webEditBridge?.highlightElement('$escapedSelector');")
}Apply the same fix to scrollToElement() at line 209.
🤖 Prompt for AI Agents
In
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditViewModel.kt
around lines 193-196 (and also apply to scrollToElement at line 209), the
selector string is only escaping single quotes which misses existing
backslashes; first replace backslashes with double-backslashes, then escape
single quotes so the injected JavaScript is safe (i.e., perform
selector.replace("\\", "\\\\") followed by selector.replace("'", "\\'")), and
use that fully escaped value in the executeJavaScript calls for both
highlightElement and scrollToElement.
| private fun updateState(update: (IdeaWebEditState) -> IdeaWebEditState) { | ||
| _state.value = update(_state.value) | ||
| } |
There was a problem hiding this comment.
Thread-safety issue with non-atomic state updates.
updateState is called from multiple threads (CEF load handlers, IO coroutines, main thread). The read-modify-write pattern _state.value = update(_state.value) is not atomic—concurrent calls can lose updates.
Use MutableStateFlow.update for atomic compare-and-swap:
private fun updateState(update: (IdeaWebEditState) -> IdeaWebEditState) {
- _state.value = update(_state.value)
+ _state.update(update)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun updateState(update: (IdeaWebEditState) -> IdeaWebEditState) { | |
| _state.value = update(_state.value) | |
| } | |
| private fun updateState(update: (IdeaWebEditState) -> IdeaWebEditState) { | |
| _state.update(update) | |
| } |
🤖 Prompt for AI Agents
In
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/webedit/IdeaWebEditViewModel.kt
around lines 400 to 402, the current read-modify-write `_state.value =
update(_state.value)` is not atomic and can lose concurrent updates; replace
this pattern by using MutableStateFlow.update to perform an atomic
compare-and-swap update (call the StateFlow's update with a lambda that applies
the provided updater to the current state) and add the necessary import for
kotlinx.coroutines.flow.update so concurrent callers (CEF handlers, IO
coroutines, main thread) cannot overwrite each other.
| implementation(project(":mpp-core")) | ||
| implementation(project(":mpp-codegraph")) | ||
| implementation(project(":mpp-viewer")) | ||
| implementation(project(":mpp-viewer-web")) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove redundant mpp-viewer-web dependency from jvmMain.
The dependency is already added to commonMain (line 134), so it should not be duplicated in jvmMain (line 185). Multiplatform conventions place shared dependencies in commonMain and only add platform-specific dependencies to their respective sourcesets.
val jvmMain by getting {
dependencies {
- implementation(project(":mpp-viewer-web"))
implementation(project(":mpp-viewer"))Also applies to: 185-185
🤖 Prompt for AI Agents
In mpp-ui/build.gradle.kts around lines 134 and 185, the
project(":mpp-viewer-web") dependency is declared in commonMain (line 134) and
redundantly re-declared in jvmMain (line 185); remove the duplicate
implementation(project(":mpp-viewer-web")) entry from the jvmMain sourceset
(line 185) so the shared dependency remains only in commonMain and only
platform-specific dependencies are kept in jvmMain.
| // Ktor HTTP Client (for remote agent) | ||
| implementation(libs.ktor.client.core) | ||
|
|
||
| implementation(libs.compose.webview) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
compose.webview Jetbrains Compose platform compatibility iOS JavaScript WASM
💡 Result:
Short answer: There’s no official single “compose.webview” from JetBrains that runs everywhere. Options and compatibility:
-
Compose Multiplatform (official)
- iOS: you embed a native WKWebView via the UIKit/SwiftUI interop (supported; iOS target is Beta). [1][2]
- Web (browser): Compose Web supports embedding HTML elements (WebElementView) and the web target builds to both Kotlin/JS and Kotlin/Wasm (use the compatibility/browser distribution to support modern browsers (Wasm) and older browsers (JS)). [3][4]
-
Community library (compose-webview-multiplatform by KevinnZou)
- Implements WebView for Android, iOS (WKWebView) and Desktop; it’s a third‑party solution, not an official JetBrains artifact. [5]
If you need a single cross‑platform WebView: use platform interop on iOS (WKWebView) + Compose Web embedding for browser targets (JS/Wasm). For a packaged native app on iOS + web in browser you’ll maintain two implementations (native WKWebView vs HTML iframe/element on web). [1][3][5]
Sources:
- JetBrains — UIKit integration / Web view examples. [2]
- JetBrains — Compose Multiplatform platform status and docs. [1]
- JetBrains — Web target: JS and Wasm compatibility (composeCompatibilityBrowserDistribution) and WebElementView. [3][4]
- KevinnZou — compose-webview-multiplatform (community library implementing Android, iOS, Desktop). [5]
🏁 Script executed:
# Look for dependency definitions in the project
fd -e toml -e gradle -e kts | head -20Repository: phodal/auto-dev
Length of output: 372
🏁 Script executed:
# Search for libs.compose.webview definition
rg "compose\.webview" --type kotlin --type gradleRepository: phodal/auto-dev
Length of output: 297
🏁 Script executed:
# Check for gradle version catalog (libs.versions.toml or similar)
fd "libs.versions.toml" -o fd "gradle/libs.versions.toml"Repository: phodal/auto-dev
Length of output: 182
🏁 Script executed:
cat gradle/libs.versions.toml | grep -A 5 -B 5 "compose.webview"Repository: phodal/auto-dev
Length of output: 1065
🏁 Script executed:
# Also search for the full definition
rg "webview" gradle/libs.versions.toml -A 2 -B 2Repository: phodal/auto-dev
Length of output: 525
🌐 Web query:
kevinnzou compose-webview-multiplatform supported platforms targets
💡 Result:
Supported targets: Android, iOS and Desktop. Desktop implementation moved from JavaFX (<=1.2.0) → Java CEF (from 1.3.0) → Kotlin CEF (KCEF, since 1.7.0). [1][2]
Move compose.webview to platform-specific sourcesets.
compose-webview-multiplatform only supports Android, iOS, and Desktop targets. It does not support JavaScript or WASM. Adding it to commonMain will cause compilation failures for the web targets. Move the dependency to androidMain and desktopMain sourcesets instead.
🤖 Prompt for AI Agents
In mpp-ui/build.gradle.kts around line 168, the compose.webview dependency is
currently added to commonMain which breaks JS/WASM builds; remove
implementation(libs.compose.webview) from commonMain and instead add that
dependency to the platform-specific source sets (androidMain and desktopMain —
and optionally iosMain if you target iOS) so only Android and Desktop (and iOS
if needed) include compose-webview-multiplatform.
mpp-viewer-web/src/jvmMain/kotlin/cc/unitmesh/viewer/web/webedit/WebEditView.jvm.kt
Show resolved
Hide resolved
Security & Robustness: - Fix JS injection vulnerabilities by escaping backslash, quotes, newlines in all bridge implementations - Replace deprecated substr() with slice() for ID generation - Upgrade to crypto.randomUUID() with fallback for unique IDs - Fix double JSON encoding - send structured objects instead of strings - Improve thread-safety with StateFlow.update() instead of direct assignment UX Improvements: - Separate hover and selection highlights (hover no longer clears selection) - Add CSS variables for themeable colors (--webedit-hover-color, --webedit-selected-color) - Add URL validation with regex pattern matching - Add accessibility semantics to URL input field - Trim whitespace in URL input before navigation - Change placeholder from baidu.com to example.com Build & Architecture: - Move compose-webview dependency from commonMain to jvmMain (WebView not supported on WASM/JS) - Remove duplicate mpp-viewer-web dependency in jvmMain Code Quality: - Add TODO comments for stub implementations (getSelectedElementHtml) - Remove incorrect frame.name usage for page title (use JS callback instead) - Remove hardcoded backgroundColor and initial URL in WebEditView - Improve error handling and logging consistency Files modified: - IdeaWebEditBridgeScript.kt, IdeaWebEditViewModel.kt - WebEditBridge.jvm.kt, WebEditBridge.wasmJs.kt - WebEditBridgeScript.kt, WebEditView.jvm.kt - WebEditPage.kt, WebEditToolbar.kt - mpp-ui/build.gradle.kts
1 participant
Summary
Implements WebEdit agent for browsing web pages and selecting DOM elements.
Closes #511
Changes
New Files
mpp-viewer-web (WebEdit Bridge Layer):
DOMElement.kt- Data classes for DOM elements, bounding boxes, and message typesWebEditBridge.kt- Common interface for bidirectional WebView-Kotlin communicationWebEditBridge.jvm.kt- JVM implementationWebEditBridge.wasmJs.kt- WASM implementationWebEditBridgeScript.kt- JavaScript injection code for DOM selectionmpp-ui (UI Components):
WebEditPage.kt- Main page with URL bar, WebView area, DOM sidebar, chat inputWebEditToolbar.kt- Top toolbar with navigation controls and mode togglesDOMTreeSidebar.kt- Right sidebar showing DOM tree structureWebEditChatInput.kt- Bottom chat input for Q&A functionalityWebEditViewPlaceholder.kt- Placeholder for platform-specific WebViewModified Files
AgentType.kt- AddedWEB_EDITenum valueAgentInterfaceRouter.kt- Added routing forWEB_EDITDesktopTitleBarTabs.kt- Added sidebar visibility handlingCodingAgentPage.kt- Added exhaustive when caseTopBarMenuDesktop.kt/TopBarMenuMobile.kt- Added icon mappingArchitecture
expect/actualfor platform-specific implementationsWebEditBridgeScriptenables DOM selection in any webpageWebEditMessage,WebEditCommand)Testing
./gradlew :mpp-ui:compileKotlinJvm)Summary by CodeRabbit
New Features
UI/UX Improvements
✏️ Tip: You can customize this high-level summary in your review settings.