feat(web): add web_browse tool with hidden WebView and anti-scraping headers#351
feat(web): add web_browse tool with hidden WebView and anti-scraping headers#351hiqiancheng wants to merge 7 commits into
Conversation
PR #216 moved scripts/ci/pr-template-check.js to apps/desktop/scripts/ci/pr-template-check.js but did not update the workflow reference. This caused the PR Template Check workflow to fail with "Cannot find module" on every PR.
DOMPurify.sanitize returns TrustedHTML when Trusted Types are enabled, which is not assignable to string. Wrap with String() and use `as const` instead of the DOMPurify.Config namespace type.
…headers Add two enhancements to the web fetching capabilities: P3 - Anti-scraping headers for WebFetch: - Add buildBrowserHeaders() with 6 rotating User-Agent strings - Inject full browser-grade headers (Accept, Accept-Language, Referer, etc.) - Rotate UA per minute to avoid detection patterns P4 - New web_browse tool with hidden WebView: - Create WebViewSessionManager with on-demand hidden WebviewWindow - Support 6 commands: open, click, find, scroll, extract, evaluate - 5-minute idle timeout with automatic session cleanup - JavaScript evaluation via Tauri event system with fallback polling - SSRF protection for open commands (blocks private IPs) - Full i18n support (Chinese + English) Inspired by Codex web.run tool design with command-based browsing.
There was a problem hiding this comment.
Sorry @hiqiancheng, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR implements a complete WebBrowse built-in tool that enables AI agents to automate browser interactions through Tauri's WebView. The implementation spans Rust backend (session manager with command handlers), TypeScript frontend (request parsing, validation, execution), native service bridging, tool registry integration, database seeding, and i18n. Additionally, it adds DOMPurify-based HTML sanitization for widgets and browser-like headers for web fetch. ChangesWebBrowse Built-in Tool
Supporting Infrastructure Improvements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs`:
- Around line 396-397: Session current_url is only updated on explicit open
calls and can become stale after navigations triggered by clicks or eval; fix
by, after wait_for_page_stable (and before assembling any navigation-related
response), evaluating window.location.href via the session's JS-eval mechanism
and assigning the returned string to s.current_url (e.g., s.current_url =
Some(href_str.to_string())); apply this pattern in the code paths around the
existing update at s.current_url = Some(url.to_string()) and also at the other
navigation response sites noted (the blocks around the lines you flagged,
currently updating last_activity), so every response assembly reads
window.location.href and writes it back to s.current_url.
- Around line 515-559: The eval result routing is race-prone:
eval_js_with_result registers listeners via setup_eval_result_listener on the
shared EVAL_RESULT_EVENT and also uses the shared global
window.__wb_eval_result, so concurrent calls can receive another call's payload;
fix by correlation and uniqueness per request—modify
setup_eval_result_listener/eval_js_with_result to generate a per-call unique ID
(e.g. uuid), include that ID in the emitted payload and event listener filter
(or use a namespaced event name like `{EVAL_RESULT_EVENT}:{id}`), and change the
fallback global to a per-request key (e.g. window.__wb_eval_result_{id}) or
avoid the fallback under concurrent mode; ensure the listener only resolves when
the payload ID matches and unregister the specific listener before returning.
- Around line 243-247: The current truncation uses text.truncate(max) which
treats max as a byte index and can panic on UTF-8 boundaries; replace that with
a character-safe truncation: compute the byte index of the max-th character
using text.char_indices() (or build a new String from text.chars().take(max))
and then truncate to that byte index before appending the "[截断]" marker; adjust
the logic around max, truncated, and the existing text.push_str call so
non-ASCII content no longer causes a panic.
- Around line 350-362: The helpers get_active_window and get_current_url
currently call self.session.blocking_read()/blocking_write(), which can panic
when invoked from async command handlers; change these helpers to use async
locks (self.session.read().await and self.session.write().await) and propagate
async by making the helpers async, then update every caller (cmd_click,
cmd_find, cmd_scroll, cmd_extract, cmd_evaluate and any other cmd_* call sites)
to await the helpers; ensure you still drop guards before acquiring write locks
(use scoped awaits) and adjust signatures/await points accordingly so no
blocking_* methods are used in async contexts.
In `@apps/desktop/src-tauri/src/lib.rs`:
- Around line 112-113: Remove the redundant state registration by deleting the
explicit call to app.manage(WebViewSessionManager::new(app.handle().clone())) in
lib.rs; rely on setup::setup_app() which already registers
WebViewSessionManager, so ensure only setup::setup_app(app) is called and that
WebViewSessionManager is registered once there (verify setup::setup_app() calls
app.manage(WebViewSessionManager::new(...)) and leave it as the single source of
registration).
In `@apps/desktop/src/database/artifacts/runtime/seed.sql`:
- Around line 131-132: The inserted built-in tool entry for tool_id 'web_browse'
currently uses risk_level 'low' but should be escalated; update the INSERT in
seed.sql to use a higher risk classification (e.g., 'high' or 'medium') for the
risk_level column for the 'web_browse' record and also add/modify an explicit
UPDATE statement that sets built_in_tools.risk_level = 'high' (or chosen level)
WHERE tool_id = 'web_browse' so existing installations are corrected; target the
INSERT/WHERE NOT EXISTS block for tool_id 'web_browse' and the
built_in_tools.risk_level column when making the change.
In `@apps/desktop/src/services/BuiltInToolService/tools/webBrowse/helper.ts`:
- Around line 242-263: The hostname canonicalization in isDisallowedHostname
currently misses trailing-dot variants like "localhost." — update
isDisallowedHostname to remove any trailing dot(s) after calling
stripIpv6Brackets and lowercasing (e.g., trim trailing '.' characters) before
performing the localhost, .local, .localdomain checks and the numeric/IPv6
checks; keep using the existing helpers stripIpv6Brackets, isPrivateIpv4, and
isPrivateIpv6 so the rest of the logic (IPv4 regex, IPv6 detection, final dot
check) operates on the normalized, trailing-dot-stripped hostname.
- Around line 19-39: Remove the duplicated local interfaces
WebBrowseNativeRequest and WebBrowseNativeResponse and import the shared
request/response types used by NativeService instead; specifically, delete the
local declarations in helper.ts, add an import for the shared
WebBrowseNativeRequest and WebBrowseNativeResponse types from the module that
defines NativeService types, and update any local references in functions inside
helper.ts to use those imported types to keep a single source of truth and avoid
contract drift.
In `@apps/desktop/src/services/BuiltInToolService/tools/webBrowse/index.ts`:
- Around line 71-73: parseWebBrowseRequest and toNativeRequest are being called
before entering the guarded try path so their parse/validation errors can escape
normal error normalization; move the calls to parseWebBrowseRequest(args) and
toNativeRequest(parsed) inside the same try/catch (or wrap them in a try) that
normalizes tool errors (the existing guarded path where you handle and return
typed tool errors) so any thrown validation/parsing exceptions are caught and
converted to the expected ToolError shape instead of bubbling up.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b3e39254-60e5-4d60-b0d7-8b60d0d484a3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
apps/desktop/package.jsonapps/desktop/src-tauri/src/commands/built_in_tools.rsapps/desktop/src-tauri/src/commands/mod.rsapps/desktop/src-tauri/src/core/built_in_tools/mod.rsapps/desktop/src-tauri/src/core/built_in_tools/web_browse/commands.rsapps/desktop/src-tauri/src/core/built_in_tools/web_browse/mod.rsapps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rsapps/desktop/src-tauri/src/core/built_in_tools/web_browse/types.rsapps/desktop/src-tauri/src/core/setup.rsapps/desktop/src-tauri/src/lib.rsapps/desktop/src/database/artifacts/runtime/seed.sqlapps/desktop/src/i18n/messages.tsapps/desktop/src/services/BuiltInToolService/registry.tsapps/desktop/src/services/BuiltInToolService/tools/webBrowse/constants.tsapps/desktop/src/services/BuiltInToolService/tools/webBrowse/helper.tsapps/desktop/src/services/BuiltInToolService/tools/webBrowse/index.tsapps/desktop/src/services/BuiltInToolService/tools/webFetch/helper.tsapps/desktop/src/services/BuiltInToolService/tools/webFetch/index.tsapps/desktop/src/services/BuiltInToolService/tools/widgetTool/showWidget/runtime.tsapps/desktop/src/services/BuiltInToolService/types.tsapps/desktop/src/services/NativeService/builtInTools.tsapps/desktop/src/services/NativeService/types.tsapps/desktop/src/views/SettingsView/components/BuiltInTools/types.ts
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.42.3)
apps/desktop/src/services/BuiltInToolService/tools/widgetTool/showWidget/runtime.ts
[warning] 527-527: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: template.innerHTML = String(DOMPurify.sanitize(parsed.body.innerHTML || '
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 529-529: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: template.innerHTML = String(DOMPurify.sanitize(normalizedHtml, purifyConfig))
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 527-527: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: template.innerHTML = String(DOMPurify.sanitize(parsed.body.innerHTML || '
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 529-529: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: template.innerHTML = String(DOMPurify.sanitize(normalizedHtml, purifyConfig))
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🪛 ESLint
apps/desktop/src/services/BuiltInToolService/tools/webBrowse/index.ts
[error] 3-24: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 14-17: Replace ⏎····WEB_BROWSE_TOOL_DESCRIPTION,⏎····WEB_BROWSE_TOOL_INPUT_SCHEMA,⏎ with ·WEB_BROWSE_TOOL_DESCRIPTION,·WEB_BROWSE_TOOL_INPUT_SCHEMA·
(prettier/prettier)
[error] 67-67: '_context' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 75-75: Insert ⏎···········
(prettier/prettier)
apps/desktop/src/services/BuiltInToolService/tools/widgetTool/showWidget/runtime.ts
[error] 528-528: Replace DOMPurify.sanitize(parsed.body.innerHTML·||·'<div></div>',·purifyConfig) with ⏎············DOMPurify.sanitize(parsed.body.innerHTML·||·'<div></div>',·purifyConfig)⏎········
(prettier/prettier)
apps/desktop/src/services/BuiltInToolService/tools/webBrowse/helper.ts
[error] 70-73: Replace ⏎········(parsedArgs.command·===·'click'·||·parsedArgs.command·===·'find')·&&⏎········!parsedArgs.selector⏎···· with (parsedArgs.command·===·'click'·||·parsedArgs.command·===·'find')·&&·!parsedArgs.selector
(prettier/prettier)
🪛 OpenGrep (1.22.0)
apps/desktop/src/services/BuiltInToolService/tools/widgetTool/showWidget/runtime.ts
[WARNING] 528-528: Setting innerHTML with dynamic content can lead to XSS. Use textContent or createElement with proper escaping instead.
(coderabbit.xss.innerhtml-assignment)
[WARNING] 530-530: Setting innerHTML with dynamic content can lead to XSS. Use textContent or createElement with proper escaping instead.
(coderabbit.xss.innerhtml-assignment)
apps/desktop/src/services/BuiltInToolService/tools/webBrowse/helper.ts
[ERROR] 209-209: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 214-214: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🪛 SQLFluff (4.2.1)
apps/desktop/src/database/artifacts/runtime/seed.sql
[error] 132-132: The 'WHERE' keyword should always start a new line.
(LT14)
🔇 Additional comments (21)
apps/desktop/src/services/BuiltInToolService/tools/webFetch/helper.ts (1)
11-11: LGTM!Also applies to: 23-53
apps/desktop/src/services/BuiltInToolService/tools/webFetch/index.ts (1)
19-19: LGTM!Also applies to: 79-79
apps/desktop/src/services/BuiltInToolService/tools/widgetTool/showWidget/runtime.ts (2)
3-3: LGTM!
520-534: Static analysis warnings are false positives — DOMPurify sanitization is correctly applied.The innerHTML assignments are safe because they receive the output of
DOMPurify.sanitize(), which is the canonical safe usage pattern. TheADD_TAGS: ['script']configuration is appropriate here since:
- Scripts are needed for interactive widgets and are re-executed by
runInlineScripts- DOMPurify still strips dangerous event handlers (
onclick,onerror, etc.) andjavascript:URIs- External script sources are validated against the allowlist at line 634
The security model correctly trades script execution (required for widget interactivity) while blocking the actual XSS vectors (event handler injection, malicious URIs).
apps/desktop/src-tauri/src/core/built_in_tools/web_browse/types.rs (1)
8-53: LGTM!apps/desktop/src/services/BuiltInToolService/tools/webBrowse/constants.ts (1)
7-137: LGTM!apps/desktop/src/services/BuiltInToolService/tools/webBrowse/helper.ts (1)
56-240: LGTM!apps/desktop/src/services/NativeService/types.ts (1)
38-61: LGTM!apps/desktop/src-tauri/src/core/built_in_tools/web_browse/commands.rs (1)
14-19: LGTM!apps/desktop/src/services/NativeService/builtInTools.ts (1)
3-8: LGTM!Also applies to: 20-22
apps/desktop/src-tauri/src/commands/built_in_tools.rs (1)
36-45: LGTM!apps/desktop/src-tauri/src/core/built_in_tools/web_browse/mod.rs (1)
5-11: LGTM!apps/desktop/src-tauri/src/core/built_in_tools/mod.rs (1)
10-15: LGTM!apps/desktop/src-tauri/src/commands/mod.rs (1)
61-61: LGTM!apps/desktop/src-tauri/src/core/setup.rs (1)
174-176: LGTM!apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs (1)
149-149: ⚡ Quick winDon’t guard
textContentas null here.
cmd_findusesdocument.querySelectorAll(...), which yieldsElements; forElementnodes,textContentis a string, soel.textContent.trim()won’t throw.> Likely an incorrect or invalid review comment.apps/desktop/src/services/BuiltInToolService/tools/webBrowse/index.ts (1)
26-59: LGTM!Also applies to: 98-120
apps/desktop/src/services/BuiltInToolService/types.ts (1)
26-26: LGTM!apps/desktop/src/services/BuiltInToolService/registry.ts (1)
9-10: LGTM!Also applies to: 63-63
apps/desktop/src/views/SettingsView/components/BuiltInTools/types.ts (1)
81-81: LGTM!Also applies to: 109-111
apps/desktop/src/i18n/messages.ts (1)
351-351: LGTM!Also applies to: 702-708, 1113-1114, 1485-1493
| let max = max_chars.unwrap_or(50_000); | ||
| let truncated = text.len() > max; | ||
| if truncated { | ||
| text.truncate(max); | ||
| text.push_str("\n...[截断]"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs"
echo "== File: $file =="
ls -l "$file" || true
echo
# Show the reported range with some surrounding context
nl -ba "$file" | sed -n '200,280p'
echo
echo "== Search for max_chars definition and origin =="
rg -n "max_chars" "$file"
echo
echo "== Search for any parsing/validation of max_chars in the broader module =="
rg -n "truncate\\(max\\)|chars\\(\\)\\.count\\(|chars\\(\\)\\.take\\(" "$file"
echo
echo "== Search in repo for where this tool call receives max_chars =="
rg -n "max_chars" apps/desktop/src-tauri/src/core/built_in_tools/web_browse -SRepository: TouchAI-org/TouchAI
Length of output: 326
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs"
echo "== File: $file =="
ls -l "$file" || true
echo
echo "== Lines 230-260 (around reported 243-247) =="
cat -n "$file" | sed -n '220,270p'
echo
echo "== Search for max_chars in file =="
rg -n "max_chars" "$file" || true
echo
echo "== Search for truncate/max logic in file =="
rg -n "truncate\\(max\\)|chars\\(\\)\\.count\\(|chars\\(\\)\\.take\\(" "$file" || true
echo
echo "== Search repo for where max_chars comes from in this module =="
rg -n "max_chars" apps/desktop/src-tauri/src/core/built_in_tools/web_browse -S || trueRepository: TouchAI-org/TouchAI
Length of output: 3584
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "max_chars" apps/desktop/src-tauri/src/core/built_in_tools/web_browse -S
echo "== Inspect types.rs (web_browse) =="
types="apps/desktop/src-tauri/src/core/built_in_tools/web_browse/types.rs"
cat -n "$types" | sed -n '1,120p'
echo "== Inspect cmd_extract in session.rs =="
session="apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs"
# show around cmd_extract definition and call
rg -n "fn cmd_extract" "$session"
start=$(rg -n "fn cmd_extract" "$session" | head -n1 | cut -d: -f1)
# print some window around start
from=$((start-20))
to=$((start+80))
cat -n "$session" | sed -n "${from},${to}p"
echo "== Search repo for JSON/request schema or docs mentioning max_chars meaning =="
rg -n "max_chars" -S .
echo "== Search for 'truncate' and for any conversion from char count to byte boundary =="
rg -n "chars\\(\\)\\.count\\(|chars\\(\\)\\.take\\(|char_indices|is_char_boundary|truncate\\(" apps/desktop/src-tauri/src/core/built_in_tools/web_browse -S || trueRepository: TouchAI-org/TouchAI
Length of output: 8853
🏁 Script executed:
#!/bin/bash
set -euo pipefail
base="apps/desktop/src/services/BuiltInToolService/tools/webBrowse"
ls -l "$base" || true
echo "== constants.ts =="
cat -n "$base/constants.ts"
echo
echo "== helper.ts (relevant parts around maxChars) =="
# print around usage of maxChars
rg -n "maxChars|DEFAULT_MAX_CHARS" "$base/helper.ts"
start=$(rg -n "maxChars" "$base/helper.ts" | head -n1 | cut -d: -f1)
from=$((start-40))
to=$((start+140))
cat -n "$base/helper.ts" | sed -n "${from},${to}p"Repository: TouchAI-org/TouchAI
Length of output: 7997
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/desktop/src/services/BuiltInToolService/tools/webBrowse/helper.ts"
echo "== helper.ts (first ~140 lines, includes maxChars wiring) =="
cat -n "$file" | sed -n '1,160p'
echo
echo "== helper.ts lines mentioning maxChars =="
rg -n "maxChars|DEFAULT_MAX_CHARS" "$file"Repository: TouchAI-org/TouchAI
Length of output: 5665
Prevent UTF-8 boundary panic when truncating extracted web content
text.truncate(max) treats max as a byte offset and will panic if it lands in the middle of a UTF-8 codepoint; the request/front-end labels maxChars as “maximum output characters”, so non-ASCII page text can crash here.
File: apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs (around 243-247)
Suggested fix
- let truncated = text.len() > max;
- if truncated {
- text.truncate(max);
+ let truncated = text.chars().count() > max;
+ if truncated {
+ text = text.chars().take(max).collect::<String>();
text.push_str("\n...[截断]");
}📝 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.
| let max = max_chars.unwrap_or(50_000); | |
| let truncated = text.len() > max; | |
| if truncated { | |
| text.truncate(max); | |
| text.push_str("\n...[截断]"); | |
| let max = max_chars.unwrap_or(50_000); | |
| let truncated = text.chars().count() > max; | |
| if truncated { | |
| text = text.chars().take(max).collect::<String>(); | |
| text.push_str("\n...[截断]"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs` around
lines 243 - 247, The current truncation uses text.truncate(max) which treats max
as a byte index and can panic on UTF-8 boundaries; replace that with a
character-safe truncation: compute the byte index of the max-th character using
text.char_indices() (or build a new String from text.chars().take(max)) and then
truncate to that byte index before appending the "[截断]" marker; adjust the logic
around max, truncated, and the existing text.push_str call so non-ASCII content
no longer causes a panic.
| let guard = self.session.blocking_read(); | ||
| let session = guard | ||
| .as_ref() | ||
| .ok_or("WebView 会话不存在,请先执行 open 命令")?; | ||
| let window = self | ||
| .app_handle | ||
| .get_webview_window(&session.window_label) | ||
| .ok_or("WebView 窗口已被销毁,请重新打开")?; | ||
|
|
||
| // 更新活动时间。 | ||
| drop(guard); | ||
| let mut guard = self.session.blocking_write(); | ||
| if let Some(ref mut s) = *guard { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and inspect the surrounding code
FILE="apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs"
echo "== File exists =="
ls -l "$FILE"
echo "== Session.rs: relevant region around 320-430 =="
nl -ba "$FILE" | sed -n '300,430p'
echo "== Find definitions of get_active_window / get_current_url (and surrounding helpers) =="
rg -n "fn get_active_window|fn get_current_url|get_active_window\(|get_current_url\(" "$FILE"
echo "== Show lock type definition around self.session declaration =="
rg -n "session\s*:\s*|RwLock|blocking_read|blocking_write|tokio::sync::RwLock" "$FILE"
echo "== Find where these functions are called from (cmd_* flows) =="
rg -n "get_active_window\(|get_current_url\(" apps/desktop/src-tauri/src/core/built_in_tools/web_browse -S
echo "== Search for tokio::sync::RwLock blocking_read/write docs usage (confirm API usage) =="
rg -n "blocking_(read|write)\(" apps/desktop/src-tauri/src -SRepository: TouchAI-org/TouchAI
Length of output: 311
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs"
echo "== Session.rs: relevant region around 320-430 =="
cat -n "$FILE" | sed -n '300,430p'
echo "== Find definitions of get_active_window / get_current_url =="
rg -n "get_active_window|get_current_url" "$FILE"
echo "== Show session field type / lock type definition context =="
rg -n "session\s*:\s*|RwLock|blocking_read|blocking_write|tokio::sync::RwLock" "$FILE"
echo "== Find where these functions are called from (cmd_* flows) =="
rg -n "get_active_window\(|get_current_url\(" apps/desktop/src-tauri/src/core/built_in_tools/web_browse -S
echo "== Search for tokio::sync::RwLock blocking_read/write usage across codebase (bounded) =="
rg -n "blocking_(read|write)\(" apps/desktop/src-tauri/src -SRepository: TouchAI-org/TouchAI
Length of output: 8086
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs"
echo "== Context around calls to get_active_window/get_current_url (100-220) =="
cat -n "$FILE" | sed -n '80,220p'
echo "== Context around other calls (220-310) =="
cat -n "$FILE" | sed -n '220,310p'
echo "== Inspect get_current_url implementation near 660-730 =="
cat -n "$FILE" | sed -n '640,720p'
echo "== Inspect function signatures for the call sites around 110-210 to see if they are async cmd_* handlers =="
# Print only lines containing 'async fn', 'fn cmd_', and the nearby get_active_window/get_current_url call lines
rg -n "async fn|fn cmd_|get_active_window\(|get_current_url\(" "$FILE" | sed -n '1,220p'Repository: TouchAI-org/TouchAI
Length of output: 15809
🌐 Web query:
tokio::sync::RwLock blocking_read panics
💡 Result:
The tokio::sync::RwLock::blocking_read method in the Tokio library is explicitly designed to panic if it is called from within an asynchronous execution context [1][2]. This behavior is intentional because Tokio's scheduler is cooperative [3]. When a function blocks an executor thread—as blocking_read does by stalling the thread until the lock is acquired—it prevents the runtime from executing other pending asynchronous tasks, which can lead to performance degradation or deadlocks [3]. To prevent such issues, Tokio detects if the caller is running on an asynchronous executor and triggers a panic to warn the developer [3]. If you need to access an RwLock from within an asynchronous context, you should use the non-blocking, asynchronous read method instead: let read_guard = rwlock.read.await; If you must perform a blocking operation from within an asynchronous context, the recommended approach is to offload that work to a dedicated thread pool designed for blocking operations using tokio::task::spawn_blocking [1][2]: let result = tokio::task::spawn_blocking(move || { let read_guard = rwlock.blocking_read; // perform work with read_guard }).await?; Alternatively, in specific scenarios, you can use tokio::task::block_in_place [1][2].
Citations:
- 1: https://docs.rs/tokio/latest/tokio/sync/struct.RwLock.html
- 2: https://github.com/tokio-rs/tokio/blob/master/tokio/src/sync/rwlock.rs
- 3: https://medium.com/@trivajay259/why-tokio-panics-when-you-block-the-thread-and-the-clean-way-around-it-7c25f8555a70
🏁 Script executed:
rg -n "get_active_window\\(|get_current_url\\(" ./Repository: TouchAI-org/TouchAI
Length of output: 1498
Fix Tokio RwLock::blocking_read/blocking_write usage in async flows (prevents runtime panic)
apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs calls self.session.blocking_read() / blocking_write() inside get_active_window (349-367) and get_current_url (683-692). These helpers are invoked from async cmd_* methods (e.g., cmd_click, cmd_find, cmd_scroll, cmd_extract, cmd_evaluate), and Tokio intentionally panics when blocking_* locks are used from an async context. Convert these helpers to async locks (read().await / write().await) and update the call sites to .await.
Suggested fix
- fn get_active_window(&self) -> Result<WebviewWindowHandle, String> {
- let guard = self.session.blocking_read();
+ async fn get_active_window(&self) -> Result<WebviewWindowHandle, String> {
+ let guard = self.session.read().await;
let session = guard
.as_ref()
.ok_or("WebView 会话不存在,请先执行 open 命令")?;
...
- let mut guard = self.session.blocking_write();
+ let mut guard = self.session.write().await;
if let Some(ref mut s) = *guard {
s.last_activity = Instant::now();
}
Ok(WebviewWindowHandle { window })
}- fn get_current_url(&self, _handle: &WebviewWindowHandle) -> String {
- let guard = self.session.blocking_read();
+ async fn get_current_url(&self, _handle: &WebviewWindowHandle) -> String {
+ let guard = self.session.read().await;
...
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs` around
lines 350 - 362, The helpers get_active_window and get_current_url currently
call self.session.blocking_read()/blocking_write(), which can panic when invoked
from async command handlers; change these helpers to use async locks
(self.session.read().await and self.session.write().await) and propagate async
by making the helpers async, then update every caller (cmd_click, cmd_find,
cmd_scroll, cmd_extract, cmd_evaluate and any other cmd_* call sites) to await
the helpers; ensure you still drop guards before acquiring write locks (use
scoped awaits) and adjust signatures/await points accordingly so no blocking_*
methods are used in async contexts.
| s.current_url = Some(url.to_string()); | ||
| s.last_activity = Instant::now(); |
There was a problem hiding this comment.
current_url can become stale after non-open navigation.
Only open updates session current_url. Click/evaluate-triggered navigations will return outdated URL metadata.
Suggested fix direction
Update current_url after wait_for_page_stable / before response assembly by reading window.location.href via eval and writing back to session.
Also applies to: 491-492, 684-692
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs` around
lines 396 - 397, Session current_url is only updated on explicit open calls and
can become stale after navigations triggered by clicks or eval; fix by, after
wait_for_page_stable (and before assembling any navigation-related response),
evaluating window.location.href via the session's JS-eval mechanism and
assigning the returned string to s.current_url (e.g., s.current_url =
Some(href_str.to_string())); apply this pattern in the code paths around the
existing update at s.current_url = Some(url.to_string()) and also at the other
navigation response sites noted (the blocks around the lines you flagged,
currently updating last_activity), so every response assembly reads
window.location.href and writes it back to s.current_url.
| let guard = self.setup_eval_result_listener(); | ||
|
|
||
| // 用 try-catch 包装用户脚本,通过事件把结果发回。 | ||
| let wrapped_js = format!( | ||
| r#" | ||
| (async function() {{ | ||
| try {{ | ||
| const __wb_result = await (async function() {{ {js} }})(); | ||
| // 优先使用 Tauri 事件系统。 | ||
| if (window.__TAURI_INTERNALS__ && window.__TAURI_INTERNALS__.invoke) {{ | ||
| await window.__TAURI_INTERNALS__.invoke('plugin:event|emit', {{ | ||
| event: '{EVAL_RESULT_EVENT}', | ||
| payload: JSON.stringify({{ ok: true, data: __wb_result }}) | ||
| }}); | ||
| }} else {{ | ||
| // 备选:将结果存入全局变量,Rust 侧轮询读取。 | ||
| window.__wb_eval_result = JSON.stringify({{ ok: true, data: __wb_result }}); | ||
| }} | ||
| }} catch(__wb_err) {{ | ||
| if (window.__TAURI_INTERNALS__ && window.__TAURI_INTERNALS__.invoke) {{ | ||
| await window.__TAURI_INTERNALS__.invoke('plugin:event|emit', {{ | ||
| event: '{EVAL_RESULT_EVENT}', | ||
| payload: JSON.stringify({{ ok: false, error: __wb_err.message }}) | ||
| }}); | ||
| }} else {{ | ||
| window.__wb_eval_result = JSON.stringify({{ ok: false, error: __wb_err.message }}); | ||
| }} | ||
| }} | ||
| }})() | ||
| "# | ||
| ); | ||
|
|
||
| handle | ||
| .window | ||
| .eval(&wrapped_js) | ||
| .map_err(|e| format!("JS 执行失败: {e}"))?; | ||
|
|
||
| // 等待事件或超时。 | ||
| let result = tokio::time::timeout(Duration::from_millis(timeout_ms), guard.receiver).await; | ||
|
|
||
| // 注销监听器。 | ||
| handle.window.unlisten(guard._listener_id); | ||
|
|
||
| match result { | ||
| Ok(Ok(payload)) => parse_eval_result(&payload), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and inspect around the referenced lines
FILE="apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs"
echo "== File: $FILE =="
ls -l "$FILE"
echo "== Snippet near 515-610 =="
nl -ba "$FILE" | sed -n '480,640p'
echo "== Find EVAL_RESULT_EVENT definition =="
rg -n "EVAL_RESULT_EVENT" "$FILE"
echo "== Find setup_eval_result_listener definition/calls =="
rg -n "setup_eval_result_listener" "$FILE"
# Show the listener setup implementation (broad range around matches)
# (If multiple matches, show each surrounding block.)
MATCHES=$(rg -n "fn setup_eval_result_listener|setup_eval_result_listener\\(" "$FILE" -n | cut -d: -f1 | head -n 20)
echo "== Candidate line numbers for listener impl: $MATCHES =="
for ln in $MATCHES; do
start=$((ln-60)); end=$((ln+120))
echo "--- Context around line $ln ($start-$end) ---"
nl -ba "$FILE" | sed -n "${start},${end}p"
done
echo "== Search for window.__wb_eval_result usage/global polling =="
rg -n "__wb_eval_result" "$FILE"Repository: TouchAI-org/TouchAI
Length of output: 352
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs"
echo "== File: $FILE =="
ls -l "$FILE"
echo "== Snippet near 515-610 =="
cat -n "$FILE" | sed -n '480,640p' | head -n 220
echo "== Find EVAL_RESULT_EVENT definition/usages (in file) =="
rg -n "EVAL_RESULT_EVENT" "$FILE" || true
echo "== Find setup_eval_result_listener (calls + defs) =="
rg -n "setup_eval_result_listener" "$FILE" || true
echo "== Show contexts for any setup_eval_result_listener mentions =="
rg -n "setup_eval_result_listener" "$FILE" | cut -d: -f1 | head -n 20 | while read -r ln; do
start=$((ln-80)); end=$((ln+160))
echo "--- Context around line $ln ($start-$end) ---"
cat -n "$FILE" | sed -n "${start},${end}p"
done
echo "== Search for __wb_eval_result usage (global polling/fallback) =="
rg -n "__wb_eval_result" "$FILE" || true
echo "== Search for unlisten/receiver usage around the timeout block =="
rg -n "timeout\\(Duration::from_millis\\(timeout_ms\\)|unlisten\\(guard\\._listener_id\\)|guard\\.receiver" "$FILE" || trueRepository: TouchAI-org/TouchAI
Length of output: 37551
Eval result routing is race-prone under concurrent requests (shared event + shared fallback global).
- Every
eval_js_with_result()call registers a one-shot listener on the sameEVAL_RESULT_EVENT("__web_browse_eval_result"). When any JS execution emits the event, all active listeners may accept the first payload, so concurrentevaluate/find/extractcalls can resolve with the wrong result (seeeval_js_with_result+setup_eval_result_listener, lines ~515-590). - The fallback path uses a single shared global
window.__wb_eval_result, which is also uncorrelated across concurrent requests (lines ~592-625).
Suggested fix direction
+ // Generate per-request correlation id
+ let request_id = uuid::Uuid::new_v4().to_string();
+ let event_name = format!("{EVAL_RESULT_EVENT}:{request_id}");
+
- let guard = self.setup_eval_result_listener();
+ let guard = self.setup_eval_result_listener(&event_name);
- event: '{EVAL_RESULT_EVENT}',
+ event: '{event_name}',
- payload: JSON.stringify({ ok: true, data: __wb_result })
+ payload: JSON.stringify({ requestId: '{request_id}', ok: true, data: __wb_result })Also ensure the fallback (window.__wb_eval_result) is correlated per-request (or avoid it under concurrency) and/or serialize tool invocations with a manager-level async mutex if parallel calls are possible.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs` around
lines 515 - 559, The eval result routing is race-prone: eval_js_with_result
registers listeners via setup_eval_result_listener on the shared
EVAL_RESULT_EVENT and also uses the shared global window.__wb_eval_result, so
concurrent calls can receive another call's payload; fix by correlation and
uniqueness per request—modify setup_eval_result_listener/eval_js_with_result to
generate a per-call unique ID (e.g. uuid), include that ID in the emitted
payload and event listener filter (or use a namespaced event name like
`{EVAL_RESULT_EVENT}:{id}`), and change the fallback global to a per-request key
(e.g. window.__wb_eval_result_{id}) or avoid the fallback under concurrent mode;
ensure the listener only resolves when the payload ID matches and unregister the
specific listener before returning.
| app.manage(WebViewSessionManager::new(app.handle().clone())); | ||
| if let Err(err) = setup::setup_app(app) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact lines around the reported snippet
sed -n '80,140p' apps/desktop/src-tauri/src/lib.rs | nl -ba | sed -n '1,120p'
# Find where WebViewSessionManager is registered anywhere in the crate
rg -n "WebViewSessionManager::new|manage\\(WebViewSessionManager" apps/desktop/src-tauri/src -S
# Locate and inspect setup_app
rg -n "fn setup_app|setup_app\\(app\\)" apps/desktop/src-tauri/src -S
setup_file="$(rg -l "fn setup_app" apps/desktop/src-tauri/src -S | head -n 1 || true)"
echo "setup_file=$setup_file"
if [ -n "${setup_file:-}" ]; then
# Print a bounded region around setup_app
rg -n "fn setup_app" "$setup_file" -n
# Grab enough context after the function signature
start_line="$(rg -n "fn setup_app" "$setup_file" | head -n 1 | cut -d: -f1)"
from=$((start_line-30))
to=$((start_line+200))
sed -n "${from},${to}p" "$setup_file" | nl -ba | head -n 260
fi
# Also check for any explicit state insertion behavior or comments about panic
rg -n "state.*insert|manage\\(|State<|panic" apps/desktop/src-tauri/src -SRepository: TouchAI-org/TouchAI
Length of output: 106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact lines around the reported snippet (with line numbers via awk)
awk 'NR>=80 && NR<=140 {printf "%5d:%s\n", NR, $0}' apps/desktop/src-tauri/src/lib.rs
# Find all registrations/usages in this crate
echo "== WebViewSessionManager registrations =="
rg -n "WebViewSessionManager::new|manage\\(WebViewSessionManager" apps/desktop/src-tauri/src -S || true
echo "== setup_app definitions/usages =="
rg -n "fn setup_app|setup_app\\(app\\)" apps/desktop/src-tauri/src -S || true
setup_file="$(rg -l "fn setup_app" apps/desktop/src-tauri/src -S | head -n 1 || true)"
echo "setup_file=$setup_file"
if [ -n "$setup_file" ]; then
# Print a bounded region around setup_app (no full-file output)
start_line="$(rg -n "fn setup_app" "$setup_file" | head -n 1 | cut -d: -f1)"
from=$((start_line-30))
to=$((start_line+220))
if [ "$from" -lt 1 ]; then from=1; fi
echo "== setup_app context (lines $from-$to) in $setup_file =="
awk -v a="$from" -v b="$to" 'NR>=a && NR<=b {printf "%5d:%s\n", NR, $0}' "$setup_file"
fi
echo "== State insertion / panic hints =="
rg -n "state\\.insert|app\\.manage\\(|State<|panic!" apps/desktop/src-tauri/src -S || trueRepository: TouchAI-org/TouchAI
Length of output: 11742
🌐 Web query:
Tauri App::manage panic duplicate state insertion same type
💡 Result:
In Tauri, the app.manage() method is used to register state that can be accessed by commands via the State<'_, T> extractor [1][2]. Regarding your question about duplicate state insertion or type mismatches: 1. Duplicate State Insertion: Tauri's state management uses a type-based registry where each type T can only be managed once [3]. If you call app.manage() multiple times with the exact same type, the subsequent calls will typically overwrite the previous value rather than causing a panic [3]. 2. Panic on Mismatch or Missing State: Panics in Tauri state management occur primarily in two scenarios: * Missing State: If a command requests State<'_, T> but no state of type T has been registered, Tauri will panic at runtime when that command is invoked [4][5][6]. This is a common source of runtime errors, as it is not caught at compile time [1][5][2]. * Type Mismatch: If the type T requested in the command signature does not exactly match the type registered via manage(), the application will panic because the requested state cannot be found for that specific type [1][5][2]. For example, if you manage a Mutex<AppState> but attempt to access State<'_, AppState>, the application will panic because Mutex<AppState> and AppState are distinct types [5][2]. To avoid these issues: * Ensure that the type inside State<'_, T> matches exactly with the type passed to manage() [5][2]. * Use type aliases if you are working with complex types (e.g., Mutex<T>) to ensure consistency across the application, but be careful not to wrap them in an additional Mutex layer [1][2]. * Always register your state during the application setup phase, before any commands that depend on that state are invoked [5][6]. If you are experiencing unexpected panics, verify that your state registration order and types are consistent, and ensure that no frontend commands are invoked before the backend has finished its setup and state initialization [4].
Citations:
- 1: https://github.com/tauri-apps/tauri-docs/blob/v2/src/content/docs/develop/state-management.mdx
- 2: https://v2.tauri.app/develop/state-management/
- 3: https://github.com/tauri-apps/tauri/blob/8718d081/crates/tauri/src/state.rs
- 4: [bug] Unmanaged state panics if invoked tauri-apps/tauri#11949
- 5: https://skillsmp.com/skills/openaec-foundation-tauri-2-claude-skill-package-skills-source-tauri-errors-tauri-errors-runtime-skill-md
- 6: https://stackoverflow.com/questions/75853210/you-must-call-manage-before-using-this-command-when-calling-simple-tauri-fu
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show lib.rs header/imports to resolve which WebViewSessionManager type is in use at line 112
awk 'NR<=120 {printf "%5d:%s\n", NR, $0}' apps/desktop/src-tauri/src/lib.rs
# Show relevant lines in core/setup.rs around the second manage() call (already partially seen, but include imports context nearby)
awk 'NR<=220 {printf "%5d:%s\n", NR, $0}' apps/desktop/src-tauri/src/core/setup.rs
# Confirm the exact type path used at lib.rs line 112 by printing that line with a few surrounding lines
awk 'NR>=100 && NR<=125 {printf "%5d:%s\n", NR, $0}' apps/desktop/src-tauri/src/lib.rs
# Find the definition (struct/impl) of WebViewSessionManager in this crate to compare paths
rg -n "struct WebViewSessionManager|enum WebViewSessionManager|type WebViewSessionManager" apps/desktop/src-tauri/src -S || true
rg -n "built_in_tools::WebViewSessionManager|WebViewSessionManager::new" apps/desktop/src-tauri/src -SRepository: TouchAI-org/TouchAI
Length of output: 15850
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show WebViewSessionManager implementation details (esp. `new`)
awk 'NR>=1 && NR<=120 {print NR ":" $0}' apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs | sed -n '1,140p'
# Narrow around struct + new
rg -n "pub struct WebViewSessionManager|impl WebViewSessionManager|fn new\\(" apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs -n
start=$(rg -n "pub struct WebViewSessionManager" apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs -n | head -n1 | cut -d: -f1)
from=$((start-20))
to=$((start+140))
if [ $from -lt 1 ]; then from=1; fi
sed -n "${from},${to}p" apps/desktop/src-tauri/src/core/built_in_tools/web_browse/session.rs | nl -ba | head -n 220Repository: TouchAI-org/TouchAI
Length of output: 4486
🌐 Web query:
tauri v2 app.manage duplicate state overwrite panic state.rs manage
💡 Result:
In Tauri v2, calling app.manage(state) with a type T that is already managed does not cause a panic, nor does it overwrite the existing state [1]. Instead, the manage method adheres to the following behavior: 1. If the state for type T has already been set, the method returns false and the existing state remains unchanged [1]. 2. If the state for type T has not been set, the method adds the state to the application and returns true [1]. This design ensures that state registration is idempotent; subsequent attempts to manage the same type will simply be ignored by the manager, preserving the initial value [1]. While calling app.manage itself is safe and non-panicking regarding duplicates, you may encounter panics in Tauri state management in other scenarios, such as: - Accessing state that was never managed: If you attempt to access state with app.state:: or via the State guard for a type that was never registered with manage, Tauri will panic at runtime [2][3][4]. - Incorrect Type Usage: If you mismatch the type expected by your command (e.g., using State<', AppState> instead of State<', Mutex>), it will result in a runtime panic because the required type T is not found in the manager [2][3]. To safely handle state access, ensure all types are correctly registered during the app setup phase and avoid registering the same type multiple times if you intend for later registrations to override the initial one (Tauri does not support overwriting). If you need to update state, use interior mutability patterns, such as wrapping your state in a Mutex or RwLock, and update the data inside that container rather than trying to replace the container itself [2][3][5].
Citations:
- 1: https://docs.rs/tauri/2.0.0/tauri/trait.Manager.html
- 2: https://github.com/tauri-apps/tauri-docs/blob/v2/src/content/docs/develop/state-management.mdx
- 3: https://v2.tauri.app/develop/state-management/
- 4: https://github.com/tauri-apps/tauri/blob/8718d081/crates/tauri/src/state.rs
- 5: https://stackoverflow.com/questions/79095760/mutable-app-state-in-tauri-accessible-from-tauricommand
Remove redundant WebViewSessionManager registration (startup isn’t expected to fail)
apps/desktop/src-tauri/src/lib.rs registers WebViewSessionManager in .setup, but core/setup.rs::setup_app() also calls app.manage(WebViewSessionManager::new(...)). In Tauri, duplicate manage() calls for the same state type are ignored (the first registration is kept), so this shouldn’t cause a panic/startup failure—just redundant/unclear initialization.
Suggested fix
- app.manage(WebViewSessionManager::new(app.handle().clone()));
if let Err(err) = setup::setup_app(app) {📝 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.
| app.manage(WebViewSessionManager::new(app.handle().clone())); | |
| if let Err(err) = setup::setup_app(app) { | |
| if let Err(err) = setup::setup_app(app) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src-tauri/src/lib.rs` around lines 112 - 113, Remove the
redundant state registration by deleting the explicit call to
app.manage(WebViewSessionManager::new(app.handle().clone())) in lib.rs; rely on
setup::setup_app() which already registers WebViewSessionManager, so ensure only
setup::setup_app(app) is called and that WebViewSessionManager is registered
once there (verify setup::setup_app() calls
app.manage(WebViewSessionManager::new(...)) and leave it as the single source of
registration).
| SELECT 'web_browse', 'WebBrowse', '浏览交互式网页,支持 JavaScript 渲染和页面交互', 1, 'low', NULL | ||
| WHERE NOT EXISTS (SELECT 1 FROM built_in_tools WHERE tool_id = 'web_browse'); |
There was a problem hiding this comment.
Revisit default risk classification for web_browse.
Line 131 sets risk_level to 'low', but this tool supports interactive commands (click, evaluate) that can perform side-effectful actions on authenticated web sessions. This weakens risk signaling and policy posture.
🛠️ Suggested fix
-SELECT 'web_browse', 'WebBrowse', '浏览交互式网页,支持 JavaScript 渲染和页面交互', 1, 'low', NULL
+SELECT 'web_browse', 'WebBrowse', '浏览交互式网页,支持 JavaScript 渲染和页面交互', 1, 'medium', NULL📝 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.
| SELECT 'web_browse', 'WebBrowse', '浏览交互式网页,支持 JavaScript 渲染和页面交互', 1, 'low', NULL | |
| WHERE NOT EXISTS (SELECT 1 FROM built_in_tools WHERE tool_id = 'web_browse'); | |
| SELECT 'web_browse', 'WebBrowse', '浏览交互式网页,支持 JavaScript 渲染和页面交互', 1, 'medium', NULL | |
| WHERE NOT EXISTS (SELECT 1 FROM built_in_tools WHERE tool_id = 'web_browse'); |
🧰 Tools
🪛 SQLFluff (4.2.1)
[error] 132-132: The 'WHERE' keyword should always start a new line.
(LT14)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/database/artifacts/runtime/seed.sql` around lines 131 - 132,
The inserted built-in tool entry for tool_id 'web_browse' currently uses
risk_level 'low' but should be escalated; update the INSERT in seed.sql to use a
higher risk classification (e.g., 'high' or 'medium') for the risk_level column
for the 'web_browse' record and also add/modify an explicit UPDATE statement
that sets built_in_tools.risk_level = 'high' (or chosen level) WHERE tool_id =
'web_browse' so existing installations are corrected; target the INSERT/WHERE
NOT EXISTS block for tool_id 'web_browse' and the built_in_tools.risk_level
column when making the change.
| export interface WebBrowseNativeRequest { | ||
| command: string; | ||
| url?: string; | ||
| selector?: string; | ||
| direction?: string; | ||
| pixels?: number; | ||
| mode?: string; | ||
| maxChars?: number; | ||
| script?: string; | ||
| timeoutMs?: number; | ||
| } | ||
|
|
||
| /** | ||
| * Rust 侧返回的 WebView 浏览响应。 | ||
| */ | ||
| export interface WebBrowseNativeResponse { | ||
| currentUrl: string; | ||
| content: string; | ||
| title?: string; | ||
| truncated: boolean; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use shared native request/response types to avoid contract drift.
WebBrowseNativeRequest and WebBrowseNativeResponse are duplicated here and in apps/desktop/src/services/NativeService/types.ts. Keep one source of truth and import the shared types.
♻️ Proposed refactor
+import type {
+ WebBrowseNativeRequest,
+ WebBrowseNativeResponse,
+} from '`@/services/NativeService/types`';
import { parseToolArguments } from '../../utils/toolSchema';
@@
-/**
- * 前端侧发送给 Rust 侧 WebViewSessionManager 的请求结构。
- */
-export interface WebBrowseNativeRequest {
- command: string;
- url?: string;
- selector?: string;
- direction?: string;
- pixels?: number;
- mode?: string;
- maxChars?: number;
- script?: string;
- timeoutMs?: number;
-}
-
-/**
- * Rust 侧返回的 WebView 浏览响应。
- */
-export interface WebBrowseNativeResponse {
- currentUrl: string;
- content: string;
- title?: string;
- truncated: boolean;
-}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/services/BuiltInToolService/tools/webBrowse/helper.ts`
around lines 19 - 39, Remove the duplicated local interfaces
WebBrowseNativeRequest and WebBrowseNativeResponse and import the shared
request/response types used by NativeService instead; specifically, delete the
local declarations in helper.ts, add an import for the shared
WebBrowseNativeRequest and WebBrowseNativeResponse types from the module that
defines NativeService types, and update any local references in functions inside
helper.ts to use those imported types to keep a single source of truth and avoid
contract drift.
| function isDisallowedHostname(hostname: string): boolean { | ||
| const normalized = stripIpv6Brackets(hostname).toLowerCase(); | ||
|
|
||
| if ( | ||
| normalized === 'localhost' || | ||
| normalized.endsWith('.localhost') || | ||
| normalized.endsWith('.local') || | ||
| normalized.endsWith('.localdomain') | ||
| ) { | ||
| return true; | ||
| } | ||
|
|
||
| if (/^\d{1,3}(?:\.\d{1,3}){3}$/.test(normalized)) { | ||
| return isPrivateIpv4(normalized); | ||
| } | ||
|
|
||
| if (normalized.includes(':')) { | ||
| return isPrivateIpv6(normalized); | ||
| } | ||
|
|
||
| return !normalized.includes('.'); | ||
| } |
There was a problem hiding this comment.
Harden hostname canonicalization to block trailing-dot localhost bypasses.
isDisallowedHostname can allow localhost. (and similar trailing-dot variants), which weakens SSRF blocking for loopback-style hosts.
🔧 Proposed fix
function isDisallowedHostname(hostname: string): boolean {
- const normalized = stripIpv6Brackets(hostname).toLowerCase();
+ const normalized = stripIpv6Brackets(hostname).toLowerCase();
+ const canonical = normalized.replace(/\.+$/, '');
if (
- normalized === 'localhost' ||
- normalized.endsWith('.localhost') ||
- normalized.endsWith('.local') ||
- normalized.endsWith('.localdomain')
+ canonical === 'localhost' ||
+ canonical.endsWith('.localhost') ||
+ canonical.endsWith('.local') ||
+ canonical.endsWith('.localdomain')
) {
return true;
}
- if (/^\d{1,3}(?:\.\d{1,3}){3}$/.test(normalized)) {
- return isPrivateIpv4(normalized);
+ if (/^\d{1,3}(?:\.\d{1,3}){3}$/.test(canonical)) {
+ return isPrivateIpv4(canonical);
}
- if (normalized.includes(':')) {
- return isPrivateIpv6(normalized);
+ if (canonical.includes(':')) {
+ return isPrivateIpv6(canonical);
}
- return !normalized.includes('.');
+ return !canonical.includes('.');
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/services/BuiltInToolService/tools/webBrowse/helper.ts`
around lines 242 - 263, The hostname canonicalization in isDisallowedHostname
currently misses trailing-dot variants like "localhost." — update
isDisallowedHostname to remove any trailing dot(s) after calling
stripIpv6Brackets and lowercasing (e.g., trim trailing '.' characters) before
performing the localhost, .local, .localdomain checks and the numeric/IPv6
checks; keep using the existing helpers stripIpv6Brackets, isPrivateIpv4, and
isPrivateIpv6 so the rest of the logic (IPv4 regex, IPv6 detection, final dot
check) operates on the normalized, trailing-dot-stripped hostname.
| const parsed = parseWebBrowseRequest(args); | ||
| const nativeRequest = toNativeRequest(parsed); | ||
|
|
There was a problem hiding this comment.
Handle parse/validation errors inside the guarded path.
Line 71 and Line 72 run before try, so invalid tool args can bypass your error normalization and bubble as an exception instead of returning a typed tool error.
🛠️ Suggested fix
export async function executeWebBrowseTool(
args: Record<string, unknown>,
config: Record<string, never>,
_context: BaseBuiltInToolExecutionContext
): Promise<BuiltInToolExecutionResult> {
void config;
-
- const parsed = parseWebBrowseRequest(args);
- const nativeRequest = toNativeRequest(parsed);
+ let parsed: ReturnType<typeof parseWebBrowseRequest> | null = null;
try {
+ parsed = parseWebBrowseRequest(args);
+ const nativeRequest = toNativeRequest(parsed);
const response: WebBrowseNativeResponse = await native.builtInTools.webBrowse(nativeRequest);
const result = formatBrowseResult(parsed.command, parsed, response);
return {
result,
@@
} catch (error) {
const errorMessage = error instanceof Error ? error.message : String(error);
+ if (!parsed) {
+ return {
+ result: errorMessage,
+ isError: true,
+ status: 'error',
+ errorMessage,
+ };
+ }
return {
result: formatBrowseError(parsed.command, parsed, errorMessage),
isError: true,
status: 'error',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/services/BuiltInToolService/tools/webBrowse/index.ts` around
lines 71 - 73, parseWebBrowseRequest and toNativeRequest are being called before
entering the guarded try path so their parse/validation errors can escape normal
error normalization; move the calls to parseWebBrowseRequest(args) and
toNativeRequest(parsed) inside the same try/catch (or wrap them in a try) that
normalizes tool errors (the existing guarded path where you handle and return
typed tool errors) so any thrown validation/parsing exceptions are caught and
converted to the expected ToolError shape instead of bubbling up.
Summary
Add two enhancements to the web fetching capabilities:
P3 - Anti-scraping headers for WebFetch: Enhance the existing
web_fetchtool with browser-grade HTTP headers (rotating User-Agent, Accept, Accept-Language, Referer, etc.) to bypass basic anti-bot detection.P4 - New
web_browsetool: Create a new built-in tool that uses a hidden WebView to render JavaScript-heavy pages and support interactive browsing commands (open, click, find, scroll, extract, evaluate). Inspired by Codex'sweb.runtool design.Related issue or RFC
AI assistance disclosure
Testing evidence
Pre-commit hook (
pnpm test:pr) fails due to pre-existing TypeScript errors inwidgetTool/showWidget/runtime.tsunrelated to this change.Risk notes
web_browsetool entry added to seed.sqlScreenshots or recordings
N/A - Backend tool, no UI changes
Checklist
[WIP]or similar title prefixes.cargo checkandpnpm type:checkfor this code PR.