Conversation
…ponse handling; add unit tests for McpServerController
… practices and execution criteria
- Replace webpack/rspack with WXT for modern browser extension development - Add WXT config with Chrome and Firefox target support - Introduce entrypoints directory structure (popup, options, background, content scripts) - Add sidepanel feature with React components and routing - Create ai-elements UI component library for consistent styling - Update build scripts: dev, build, zip for WXT workflow - Add qwen compatibility test for AI providers - Update all source files to adapt to WXT module system and APIs - Remove deprecated webpack configs and public manifest files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
This pull request is abnormally large and would use a significant amount of tokens to review. If you still wish to review it, comment |
|
augment review |
🤖 Augment PR SummarySummary: This PR migrates the browser extension build system from webpack/rspack to the WXT framework, restructures entrypoints, and adds significant new features. Changes:
Technical Notes: The sidepanel uses AI SDK's 🤖 Was this summary useful? React with 👍 or 👎 |
| action: { | ||
| default_icon: icons, | ||
| }, | ||
| permissions: ["storage", "tabs", "contextMenus"], |
There was a problem hiding this comment.
The sidePanel permission is missing from the manifest permissions array. The sidepanel entrypoint exists at entrypoints/sidepanel/, and the background script calls chrome.sidePanel.open(), but without the "sidePanel" permission the API will be unavailable and the sidepanel won't register. Similarly, the "scripting" permission is needed for chrome.scripting.registerContentScripts() in entrypoints/background.ts.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| {} | ||
| ); | ||
|
|
||
| Promise.resolve(run(stores)) |
There was a problem hiding this comment.
Race condition: the transaction.oncomplete/onerror handlers are set after the run(stores) promise resolves, but IndexedDB transactions auto-commit when all their requests finish and the microtask queue is empty. If run() only fires synchronous IDB requests (which requestToPromise wraps), the transaction may already have completed or errored by the time these handlers are attached, causing the outer promise to never resolve.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| // Setup RSS feed detection listener | ||
| setupRssDetectionListener(); | ||
| window.addEventListener("message", function (event: MessageEvent<Message>) { | ||
| if (event.data && typeof event.data.type === 'string' && !event.data.type.includes('wxt:')) { |
There was a problem hiding this comment.
The filter !event.data.type.includes('wxt:') uses includes which is a substring match — any message type containing wxt: anywhere in the string will be silently dropped. Consider using startsWith('wxt:') for a more precise check, since WXT internal messages use the wxt: prefix.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| status: "complete", | ||
| }; | ||
|
|
||
| const allMessages = [...messages, userMsg]; |
There was a problem hiding this comment.
The sendMessage callback captures messages from state in its closure (dependency array line 581). If runAgent triggers state updates that change messages before the next render, a rapid second sendMessage call could use a stale messages snapshot, potentially losing the first message. Consider using a ref or functional state updater to always read the latest messages.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| info: chrome.contextMenus.OnClickData, | ||
| tab?: chrome.tabs.Tab | ||
| ): Promise<void> { | ||
| const targetTab = await resolveContextMenuTab(tab); |
There was a problem hiding this comment.
chrome.tabs.sendMessage to targetTab.id will fail here because targetTab.id is typed as number | undefined — the targetTab?.id guard on line 953 only checked that targetTab had an id, but the inner callback references targetTab.id without the ! assertion while TypeScript would flag it. More importantly, if the content script hasn't loaded, this sendMessage will silently fail with no user feedback.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
…oving 'wxt:' check
… with updated parsing and handling
Summary
yarn dev,yarn build,yarn zipfor WXT workflowKey Changes
wxt.config.tssupporting Chrome and Firefoxentrypoints/directory following WXT conventionsai-elements/components (conversation, message, reasoning, sources, suggestion, tool)qwenCompatibility.test.tsfor AI provider testingTest plan
yarn devto verify WXT dev server starts correctlyyarn buildto verify production build succeedsyarn testto verify all tests passyarn typecheckto verify TypeScript compilation🤖 Generated with Claude Code