fix(extension): stop iframe hangs and add debug mode - #229
Conversation
chrome.scripting.executeScript({ allFrames: true }) could block forever on
unresponsive cross-origin frames; add timeouts with main-frame fallbacks,
raise snapshot depth for Draft.js widgets, and ship a debug-log UI for runs.
Co-authored-by: Cursor <cursoragent@cursor.com>
WalkthroughThe extension adds a persistent debug logger with popup controls. Runtime flows now emit diagnostics and use timed frame operations with main-frame fallbacks. DOM actions and response extraction return more detailed failure information. ChangesExtension observability and execution resilience
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@runners/extension/agentContext.js`:
- Around line 16-35: Update summarizeAgentFromTab and
_summarizeAgentFromTabInner so the timeout-owned AbortController signal is
passed through to callLlm, aborting the request when CONTEXT_TIMEOUT_MS expires.
Replace Promise.race with explicit timer cleanup on success, timeout, or error,
while preserving any existing user-stop cancellation behavior.
In `@runners/extension/debugLog.js`:
- Around line 76-103: Serialize flush, clearDebugLogs, getDebugLogs, and
exportDebugLogs through a shared promise chain so storage read-modify-write
operations cannot overlap. Ensure each queued operation drains _buffer before
retrieving or exporting logs, and that clearDebugLogs runs in order after any
in-flight flush so cleared logs are not restored.
- Around line 20-23: Update the initialization around FLAG_KEY to expose a
readiness promise that resolves after chrome.storage.local.get completes. Await
that promise in the OPFOR_DEBUG_STATUS and toggle operation handlers, and defer
initial dbg entries until readiness resolves so stored state is applied before
any status, mutation, or diagnostic output.
In `@runners/extension/domActions.js`:
- Around line 19-24: Bound both main-frame fallback injections with the existing
timeout mechanism: in runners/extension/domActions.js (lines 19-24), update
preparePageForChat’s main-frame shadow-patch executeScript call; in
runners/extension/responseExtractor.js (lines 63-67), update scanBestFrame’s
main-frame snapshot injection. Race each call against a bounded timeout while
preserving the existing fallback behavior.
In `@runners/extension/popup.html`:
- Around line 2805-2818: Add an accessible name to the button identified by
debugToggle, using aria-label="Debug mode" or associating it with the visible
Debug mode label via aria-labelledby; preserve its existing switch semantics and
behavior.
In `@runners/extension/service_worker.js`:
- Around line 308-343: Update runners/extension/service_worker.js lines 308-343
in the OPFOR_DEBUG_TOGGLE, OPFOR_DEBUG_EXPORT, OPFOR_DEBUG_CLEAR, and related
async handlers to catch rejected storage operations and call sendResponse with {
ok: false, error }, while preserving successful responses. Update
runners/extension/popup.js lines 3016-3039 so the debug switch changes only
after a successful { ok: true } response, reverts on failure, and handles
chrome.runtime.lastError; apply equivalent failure handling to export and clear
commands.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e76117d9-cb96-4daa-8925-2d6acc93f287
📒 Files selected for processing (15)
runners/extension/agentContext.jsrunners/extension/chatLocator.jsrunners/extension/debugLog.jsrunners/extension/domActions.jsrunners/extension/domTarget.jsrunners/extension/frameDiscovery.jsrunners/extension/frame_actuate.jsrunners/extension/frame_snapshot.jsrunners/extension/llm.jsrunners/extension/llmUiActions.jsrunners/extension/orchestrator.jsrunners/extension/popup.htmlrunners/extension/popup.jsrunners/extension/responseExtractor.jsrunners/extension/service_worker.js
| const CONTEXT_TIMEOUT_MS = 45_000; | ||
|
|
||
| /** | ||
| * Use the reader LLM to infer what the on-page chat agent is from DOM snapshots. | ||
| */ | ||
| export async function summarizeAgentFromTab(tabId, readerCfg, siteUrl = "") { | ||
| dbg("context", "summarizeAgentFromTab called", { tabId, siteUrl: String(siteUrl).slice(0, 120) }); | ||
|
|
||
| return Promise.race([ | ||
| _summarizeAgentFromTabInner(tabId, readerCfg, siteUrl), | ||
| new Promise((_, reject) => | ||
| setTimeout( | ||
| () => | ||
| reject( | ||
| new Error(`Agent context detection timed out after ${CONTEXT_TIMEOUT_MS / 1000}s`) | ||
| ), | ||
| CONTEXT_TIMEOUT_MS | ||
| ) | ||
| ), | ||
| ]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Abort the LLM request when the context timeout expires.
Promise.race returns the timeout error, but _summarizeAgentFromTabInner continues its callLlm request. The request can consume provider capacity and cost after the run has moved to manual input or failed.
Create a timeout-owned AbortController, pass its signal to callLlm, abort it when the timeout expires, and clear the timer. Preserve user-stop cancellation.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 26-32: Avoid using the initial state variable in setState
Context: setTimeout(
() =>
reject(
new Error(Agent context detection timed out after ${CONTEXT_TIMEOUT_MS / 1000}s)
),
CONTEXT_TIMEOUT_MS
)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🤖 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 `@runners/extension/agentContext.js` around lines 16 - 35, Update
summarizeAgentFromTab and _summarizeAgentFromTabInner so the timeout-owned
AbortController signal is passed through to callLlm, aborting the request when
CONTEXT_TIMEOUT_MS expires. Replace Promise.race with explicit timer cleanup on
success, timeout, or error, while preserving any existing user-stop cancellation
behavior.
| // Boot: read the stored flag once so hot-path checks are synchronous. | ||
| chrome.storage.local.get([FLAG_KEY], (data) => { | ||
| _enabled = !!data?.[FLAG_KEY]; | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Wait for the stored debug state before using it.
Lines 20-23 leave _enabled as false until the storage callback completes. A restarted worker can drop initial diagnostics and return false for OPFOR_DEBUG_STATUS when opforDebug is already enabled. A toggle in this interval can also be overwritten by the stale read.
Expose a readiness promise. Await it before status and toggle operations. Defer initial dbg entries until it resolves.
🤖 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 `@runners/extension/debugLog.js` around lines 20 - 23, Update the
initialization around FLAG_KEY to expose a readiness promise that resolves after
chrome.storage.local.get completes. Await that promise in the OPFOR_DEBUG_STATUS
and toggle operation handlers, and defer initial dbg entries until readiness
resolves so stored state is applied before any status, mutation, or diagnostic
output.
| function scheduleFlush() { | ||
| if (_flushTimer) return; | ||
| _flushTimer = setTimeout(flush, 300); | ||
| } | ||
|
|
||
| async function flush() { | ||
| _flushTimer = null; | ||
| if (!_buffer.length) return; | ||
|
|
||
| const batch = _buffer.splice(0); | ||
| try { | ||
| const data = await chrome.storage.local.get(STORAGE_KEY); | ||
| const existing = Array.isArray(data?.[STORAGE_KEY]) ? data[STORAGE_KEY] : []; | ||
| const merged = [...existing, ...batch].slice(-MAX_LOG_ENTRIES); | ||
| await chrome.storage.local.set({ [STORAGE_KEY]: merged }); | ||
| } catch { | ||
| // Storage full or unavailable — logs are still in the console. | ||
| } | ||
| } | ||
|
|
||
| export async function getDebugLogs() { | ||
| const data = await chrome.storage.local.get(STORAGE_KEY); | ||
| return Array.isArray(data?.[STORAGE_KEY]) ? data[STORAGE_KEY] : []; | ||
| } | ||
|
|
||
| export async function clearDebugLogs() { | ||
| _buffer = []; | ||
| await chrome.storage.local.remove(STORAGE_KEY); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Serialize log storage operations.
Lines 85-90 perform an asynchronous read-modify-write. A second flush can read the same prior value and overwrite the first batch. clearDebugLogs() can also remove the key between the read and write, then an in-flight flush restores cleared logs. exportDebugLogs() can omit entries still in _buffer.
Queue flush, clear, retrieval, and export operations through one promise chain. Drain the buffer before retrieval or export.
Also applies to: 109-110
🧰 Tools
🪛 ast-grep (0.45.0)
[error] 77-77: React's useState should not be directly called
Context: setTimeout(flush, 300)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[warning] 77-77: Avoid using the initial state variable in setState
Context: setTimeout(flush, 300)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🤖 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 `@runners/extension/debugLog.js` around lines 76 - 103, Serialize flush,
clearDebugLogs, getDebugLogs, and exportDebugLogs through a shared promise chain
so storage read-modify-write operations cannot overlap. Ensure each queued
operation drains _buffer before retrieving or exporting logs, and that
clearDebugLogs runs in order after any in-flight flush so cleared logs are not
restored.
| try { | ||
| await chrome.scripting.executeScript({ | ||
| target: { tabId, frameIds: [0] }, | ||
| files: ["frame_shadow_patch.js"], | ||
| world: "MAIN", | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound both main-frame fallback calls.
The all-frame timeout does not protect these fallback calls. If the main frame is unresponsive, preparePageForChat or scanBestFrame can still wait indefinitely.
runners/extension/domActions.js#L19-L24: race the main-frame shadow-patch injection against a bounded timeout.runners/extension/responseExtractor.js#L63-L67: race the main-frame snapshot injection against a bounded timeout.
📍 Affects 2 files
runners/extension/domActions.js#L19-L24(this comment)runners/extension/responseExtractor.js#L63-L67
🤖 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 `@runners/extension/domActions.js` around lines 19 - 24, Bound both main-frame
fallback injections with the existing timeout mechanism: in
runners/extension/domActions.js (lines 19-24), update preparePageForChat’s
main-frame shadow-patch executeScript call; in
runners/extension/responseExtractor.js (lines 63-67), update scanBestFrame’s
main-frame snapshot injection. Race each call against a bounded timeout while
preserving the existing fallback behavior.
| <div class="toggle-row"> | ||
| <div class="body"> | ||
| <div class="label">Debug mode</div> | ||
| <div class="help">Log every step to the service-worker console and storage.</div> | ||
| </div> | ||
| <button | ||
| id="debugToggle" | ||
| class="toggle" | ||
| role="switch" | ||
| aria-checked="false" | ||
| type="button" | ||
| > | ||
| <span class="knob"></span> | ||
| </button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add an accessible name to debugToggle.
The adjacent “Debug mode” div does not label the switch. Screen readers can announce an unnamed switch.
Add aria-label="Debug mode" or associate the visible label with aria-labelledby.
🤖 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 `@runners/extension/popup.html` around lines 2805 - 2818, Add an accessible
name to the button identified by debugToggle, using aria-label="Debug mode" or
associating it with the visible Debug mode label via aria-labelledby; preserve
its existing switch semantics and behavior.
| if (message?.type === "OPFOR_DEBUG_TOGGLE") { | ||
| (async () => { | ||
| const on = !!message.enabled; | ||
| await setDebugEnabled(on); | ||
| sendResponse({ ok: true, enabled: on }); | ||
| })(); | ||
| return true; | ||
| } | ||
|
|
||
| if (message?.type === "OPFOR_DEBUG_STATUS") { | ||
| sendResponse({ ok: true, enabled: isDebugEnabled() }); | ||
| return true; | ||
| } | ||
|
|
||
| if (message?.type === "OPFOR_DEBUG_EXPORT") { | ||
| (async () => { | ||
| const text = await exportDebugLogs(); | ||
| sendResponse({ ok: true, text }); | ||
| })(); | ||
| return true; | ||
| } | ||
|
|
||
| if (message?.type === "OPFOR_DEBUG_CLEAR") { | ||
| (async () => { | ||
| await clearDebugLogs(); | ||
| sendResponse({ ok: true }); | ||
| })(); | ||
| return true; | ||
| } | ||
|
|
||
| if (message?.type === "OPFOR_DEBUG_GET_LOGS") { | ||
| (async () => { | ||
| const logs = await getDebugLogs(); | ||
| sendResponse({ ok: true, logs }); | ||
| })(); | ||
| return true; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Return debug-command failures and confirm state before updating the UI.
When a storage operation rejects, the service-worker async handler exits without calling sendResponse. The popup updates debugToggle before it confirms persistence and ignores runtime errors. The switch can show enabled while logging remains disabled.
runners/extension/service_worker.js#L308-L343: catch each asynchronous command failure and return{ ok: false, error }.runners/extension/popup.js#L3016-L3039: update the switch only after{ ok: true }; revert it and handlechrome.runtime.lastErroron failure. Handle export and clear failures similarly.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 310-310: Avoid using the initial state variable in setState
Context: setDebugEnabled(on)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
📍 Affects 2 files
runners/extension/service_worker.js#L308-L343(this comment)runners/extension/popup.js#L3016-L3039
🤖 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 `@runners/extension/service_worker.js` around lines 308 - 343, Update
runners/extension/service_worker.js lines 308-343 in the OPFOR_DEBUG_TOGGLE,
OPFOR_DEBUG_EXPORT, OPFOR_DEBUG_CLEAR, and related async handlers to catch
rejected storage operations and call sendResponse with { ok: false, error },
while preserving successful responses. Update runners/extension/popup.js lines
3016-3039 so the debug switch changes only after a successful { ok: true }
response, reverts on failure, and handles chrome.runtime.lastError; apply
equivalent failure handling to export and clear commands.
2 participants
Problem
On pages with unresponsive cross-origin iframes (e.g. Google Maps embeds),
chrome.scripting.executeScript({ allFrames: true })could block forever. The extension hung at "Detecting chat widget…" / agent-context detection with no timeout or visibility into what failed.Separately, Chat360/Draft.js widgets nest message text deeper than the snapshot walker’s depth-15 limit, so bot replies were never extracted even when sends succeeded.
Solution
Promise.racetimeouts with main-frame-only fallbacks (collectFrames, AX snapshots, shadow patch, response scan, agent-context summarize).frame_snapshot.jscollect depth from 15 → 25 so deeply nested message bodies are captured.debugLog.js+ popup toggle / export / clear) that logs locate, LLM UI actions, send, and extract steps to the service worker + storage.frame_actuate.jsfailure details when submit is not accepted.Changes
runners/extension/debugLog.js(new)runners/extension/{agentContext,chatLocator,domActions,domTarget,frameDiscovery,frame_actuate,frame_snapshot,llm,llmUiActions,orchestrator,popup,responseExtractor,service_worker}.*Issue
N/A
How to test
runners/extensionin Chrome.Screenshots
N/A — debug toggle lives under Advanced settings in the side panel.
Made with Cursor
Summary by CodeRabbit
New Features
Improvements