Skip to content

Fix: Address potential bugs and improve error handling #863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
PeterDaveHello wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix: Address potential bugs and improve error handling #863

PeterDaveHello wants to merge 3 commits into from
+1,633 −555

Conversation

@PeterDaveHello
Copy link
Member

@PeterDaveHello PeterDaveHello commented Jun 7, 2025 •
edited by coderabbitai bot
Loading

User description

It's a large patch, and I'm unsure if we can get it merged. It's generated by Google's Jules Agent, which serves as a trial run and test lab. We can also review the feedback from other agents.

Jule's commit message:

This commit includes the following changes:

  • In src/background/index.mjs:

    • Ensured that port.proxy.postMessage({ session }) is called in executeApi even if port.proxy already exists, to prevent requests from not being sent to the ChatGPT tab.
    • Added comprehensive try...catch blocks and detailed logging for better error diagnosis and stability.
    • Added 'blocking' to onBeforeSendHeaders listener options as it modifies request headers.

  • In src/content-script/index.jsx:

    • Refactored prepareForForegroundRequests to manageChatGptTabState to ensure the port listener for ChatGPT Web models is dynamically and correctly (re-)registered when you change model configurations.
    • Added comprehensive try...catch blocks and detailed logging throughout the script to improve robustness and debuggability.

These changes aim to improve the overall stability and reliability of the extension. Manual testing is recommended to verify all scenarios, aided by the new logging.

GitHub Copilot's summary:

This pull request enhances logging and error handling in the src/background/index.mjs file to improve debugging and reliability. Key changes include adding detailed debug and error logs, handling potential exceptions in asynchronous operations, and improving the robustness of listener callbacks.

Enhanced Logging:

  • Added extensive console.debug statements to provide detailed insights into function calls, message handling, and API usage, including model-specific logs in executeApi and message type logs in Browser.runtime.onMessage.addListener. [1] [2] [3]
  • Improved logging for onBeforeRequest and onBeforeSendHeaders listeners to capture header modifications and request details.

Error Handling Improvements:

  • Wrapped critical operations in try-catch blocks to prevent runtime errors from impacting the application, including in listener callbacks and asynchronous API calls. Errors are logged with detailed context for easier debugging. [1] [2] [3] [4]
  • Enhanced error responses for specific operations, such as returning structured error objects in the FETCH message handler.

Reliability Enhancements:

  • Added reconnection logic in setPortProxy to handle proxy tab disconnections gracefully, ensuring the application can recover from unexpected disconnects.
  • Improved robustness of Browser.tabs.onUpdated listener by checking for valid URLs before performing side panel updates.

Code Organization:

  • Consolidated error handling in initialization functions (registerPortListener, registerCommands, refreshMenu) to ensure proper logging and fallback behavior during setup.

PR Type

Enhancement, Bug fix

Description

  • Major refactor to improve error handling and logging throughout background and content scripts.

    • Added comprehensive try/catch blocks and detailed debug/error logs.
    • Improved robustness of asynchronous operations and event listeners.

  • Enhanced communication and state management between background and content scripts.

    • Refactored ChatGPT tab state management for reliability.
    • Improved port listener registration and message handling.

  • Improved web request interception and header modification logic.

    • Added 'blocking' to onBeforeSendHeaders for correct header modification.
    • Enhanced error handling in webRequest listeners.

  • Refined UI mounting and selection tool logic in the content script.

    • Added robust checks, logging, and error handling for DOM operations.
    • Improved floating toolbar and static card rendering logic.

Changes walkthrough 📝

Relevant files
Enhancement
index.mjs
Robust error handling and logging in background script     

src/background/index.mjs

  • Added extensive try/catch blocks and detailed logging for all major
    functions and listeners.
  • Improved port proxy setup and ensured messages are always sent to the
    ChatGPT tab.
  • Enhanced webRequest listeners with robust error handling and correct
    use of 'blocking' for header modification.
  • Refactored message handling with better diagnostics and error
    responses.
  • Improved registration and error handling for port listeners, commands,
    and menu refresh.
    		• +400/-202
    index.jsx
    Refactor content script for reliability and diagnostics   

    src/content-script/index.jsx

  • Refactored and renamed foreground request management to
    manageChatGptTabState with robust error handling.
  • Added comprehensive logging and try/catch blocks throughout all async
    and event-driven logic.
  • Improved UI mounting, selection tools, and static card logic with
    better DOM checks and error diagnostics.
  • Enhanced runtime message and storage change handling for dynamic
    configuration updates.
  • Improved port listener registration and ChatGPT Web model handling.
    		• +799/-329
    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.

    • Summary by CodeRabbit

    • New Features

      • Redaction of sensitive fields across logs; broader model/API support; new jump-back notification UI and ChatGPT tab state handling.
      • Expanded message-handling (many new actions), FETCH and cookie retrieval support, enhanced selection/static-card tooling, and side-panel improvements.

    • Bug Fixes

      • Resilient proxy reconnection with exponential backoff, guarded message forwarding, safer tab/window operations, and improved retry/error handling.

    • Refactor

      • Modularized runtime/message flows, centralized reconnection/config, and consolidated logging/error reporting.

    • Chores

      • Enhanced debug logging and ESLint/script environment updates.

    ✏️ Tip: You can customize this high-level summary in your review settings.

    @PeterDaveHello PeterDaveHello requested a review from Copilot June 7, 2025 14:02
    @coderabbitai
    Copy link
    Contributor

    coderabbitai bot commented Jun 7, 2025 •
    edited
    Loading

    📝 Walkthrough

    Walkthrough

    Added guarded proxy reconnection and sensitive-data redaction in the background; expanded runtime messaging, webRequest and tabs listeners; broadened model/API routing and error reporting; and reworked content-script UI mounting, token handling, and new helper functions for session and tab state management.

    Changes

    Cohort / File(s) Change Summary
    Background core
    src/background/index.mjs    		 Added redactSensitiveFields, centralized RECONNECT_CONFIG, robust setPortProxy with exponential backoff/reconnect/cleanup, enhanced executeApi (more models, guarded proxy negotiation), expanded runtime.onMessage handlers (many new message types), improved webRequest and tabs.onUpdated listeners, and wrapped registrations in try/catch. Attention: proxy lifecycle, redaction logic, model routing, and message error paths.
    Content script & UI
    src/content-script/index.jsx    		 Reworked UI mounting/retries, resilient input lookup, added getClaudeSessionKey, prepareForJumpBackNotification, manageChatGptTabState; strengthened token retrieval/overwrite flows, port listener guards, and orchestration for selection tools, static cards, right-click menus, plus extensive logging and error handling. Attention: DOM retry/timeouts, port messaging, and token flows.
    Config & tooling
    .eslintrc.json, package.json    		 Added webextensions env and node-file overrides in ESLint; prefixed lint scripts with npx in package.json. Attention: lint environment differences may affect local/CI lint runs.

    Sequence Diagram(s)

    sequenceDiagram
    participant User
    participant ContentScript
    participant Background
    participant ProxyTab
    participant ExternalAPI

    User->>ContentScript: invoke action / submit request
    ContentScript->>Background: runtime.sendMessage / port.postMessage
    Background->>Background: redact payload, choose model/path
    alt proxy required
        Background->>ProxyTab: setPortProxy / connect (with backoff)
        ProxyTab->>ExternalAPI: forward request
        ExternalAPI-->>ProxyTab: response
        ProxyTab-->>Background: forward response (or error)
    else direct API
        Background->>ExternalAPI: call executeApi (model-specific)
        ExternalAPI-->>Background: response
    end
    Background-->>ContentScript: structured result or error
    ContentScript->>User: render response or show error
    Loading

    Estimated code review effort

    🎯 4 (Complex) | ⏱️ ~45 minutes

    Possibly related PRs

    • Refine built-in prompts #866 — Related changes to content-script selection tools and prompt strings that intersect the reworked selection/toolbar logic.

    Poem

    I nibble logs beneath the moon,
    Threads reconnect with steady tune.
    Tokens cozy, proxies mend,
    Errors caught, then messages send.
    A little rabbit hops—this patch ascends 🐇✨

    🚥 Pre-merge checks | ✅ 2 | ❌ 1
    ❌ Failed checks (1 warning)
    Check name Status Explanation Resolution
    Docstring Coverage ⚠️ Warning Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
    ✅ Passed checks (2 passed)
    Check name Status Explanation
    Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
    Title check ✅ Passed The title accurately summarizes the main changes: the PR is fundamentally about fixing bugs and improving error handling/robustness across background and content scripts.

    ✏️ Tip: You can configure your own custom pre-merge checks in the settings.

    ✨ Finishing touches
    • 📝 Generate docstrings

    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.

    ❤️ Share

    Comment @coderabbitai help to get the list of available commands and usage tips.

    @qodo-code-review
    Copy link
    Contributor

    qodo-code-review bot commented Jun 7, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 Security concerns

    Sensitive information exposure:
    The code previously had a commented out line that would log the access token (line 613). While it's currently commented out with a warning, the presence of this code suggests a risk of accidentally exposing sensitive tokens in logs. Additionally, the code handles various authentication tokens (ChatGPT, Claude, Kimi) and should ensure these are never logged or exposed, even in debug statements.

    ⚡ Recommended focus areas for review

    Reconnection Logic

    The reconnection logic in setPortProxy may cause an infinite loop if the proxy tab consistently fails to connect. There's no retry limit or backoff strategy.

     
    console.warn(`[background] Proxy tab ${proxyTabId} disconnected. Attempting to reconnect.`)
port.proxy = null // Clear the old proxy
// Potentially add a delay or retry limit here
setPortProxy(port, proxyTabId) // Reconnect
    Promise Rejection

    The Promise for waiting for Claude session key has a timeout but the rejection might not be properly handled in all code paths, potentially causing hanging operations.

     
      setTimeout(() => {
      clearInterval(timer);
      if (!claudeSession) {
          console.warn("[content] Timed out waiting for Claude session key.");
          reject(new Error("Timed out waiting for Claude session key."));
      }
  }, 30000); // 30 second timeout
}).catch(err => { // Catch rejection from the Promise itself (e.g. timeout)
  console.error("[content] Failed to get Claude session key for jump back notification:", err);
  // Do not proceed to render if session key is critical and not found
  return;
});
    Duplicate Code

    Similar timeout and polling logic is duplicated between Claude and Kimi authentication flows. This could be refactored into a shared utility function.

     
        await new Promise((resolve, reject) => {
      const timer = setInterval(async () => {
        try {
          claudeSession = await getClaudeSessionKey()
          if (claudeSession) {
            clearInterval(timer)
            console.log('[content] Claude session key found after waiting.')
            resolve()
          }
        } catch (err) {
          // This inner catch is for getClaudeSessionKey failing during setInterval
          console.error('[content] Error polling for Claude session key:', err)
          // Optionally, clearInterval and reject if it's a persistent issue.
        }
      }, 500)
      // Timeout for waiting
      setTimeout(() => {
          clearInterval(timer);
          if (!claudeSession) {
              console.warn("[content] Timed out waiting for Claude session key.");
              reject(new Error("Timed out waiting for Claude session key."));
          }
      }, 30000); // 30 second timeout
    }).catch(err => { // Catch rejection from the Promise itself (e.g. timeout)
      console.error("[content] Failed to get Claude session key for jump back notification:", err);
      // Do not proceed to render if session key is critical and not found
      return;
    });
  } else {
    console.log('[content] Claude session key found immediately.')
  }
}

if (location.hostname === 'kimi.moonshot.cn') {
  console.debug('[content] On kimi.moonshot.cn, checking login status.')
  if (!window.localStorage.refresh_token) {
    console.log('[content] Kimi refresh token not found, attempting to trigger login.')
    setTimeout(() => {
      try {
        document.querySelectorAll('button').forEach((button) => {
          if (button.textContent === '立即登录') {
            console.log('[content] Clicking Kimi login button.')
            button.click()
          }
        })
      } catch (err_click) {
        console.error('[content] Error clicking Kimi login button:', err_click)
      }
    }, 1000)

    await new Promise((resolve, reject) => {
      const timer = setInterval(async () => {
        try {
          const token = window.localStorage.refresh_token
          if (token) {
            clearInterval(timer)
            console.log('[content] Kimi refresh token found after waiting.')
            await setUserConfig({ kimiMoonShotRefreshToken: token })
            console.log('[content] Kimi refresh token saved to config.')
            resolve()
          }
        } catch (err_set) {
           console.error('[content] Error setting Kimi refresh token from polling:', err_set)
        }
      }, 500)
       setTimeout(() => {
          clearInterval(timer);
          if (!window.localStorage.refresh_token) {
              console.warn("[content] Timed out waiting for Kimi refresh token.");
              reject(new Error("Timed out waiting for Kimi refresh token."));
          }
      }, 30000); // 30 second timeout
    }).catch(err => {
      console.error("[content] Failed to get Kimi refresh token for jump back notification:", err);
      return; // Do not proceed
    });

    This comment was marked as outdated.

    Sign in to view

    @qodo-code-review
    Copy link
    Contributor

    qodo-code-review bot commented Jun 7, 2025 •
    edited
    Loading

    PR Code Suggestions ✨

    Latest suggestions up to 3fbed97

    CategorySuggestion                                                                                                                                    Impact
    Incremental [*]
    Handle array form values

    The code assumes that details.requestBody.formData[k] is a single value, but it
    could be an array of values. When appending form data, you should handle the
    case where the value is an array by iterating through each value.

    src/background/index.mjs [556-560]

     if (details.requestBody?.formData) { // Optional chaining
   for (const k in details.requestBody.formData) {
-    formData.append(k, details.requestBody.formData[k])
+    const values = details.requestBody.formData[k];
+    if (Array.isArray(values)) {
+      for (const value of values) {
+        formData.append(k, value);
+      }
+    } else {
+      formData.append(k, values);
+    }
   }
 }
    • Apply / Chat
    Suggestion importance[1-10]: 6

    __

    Why: Valid improvement as formData values can be arrays according to the Chrome extension API. The suggestion properly handles this edge case.

    		Low
    Handle undefined array elements

    The function doesn't handle the case where an array element is undefined. This
    could cause issues when processing arrays with sparse elements or undefined
    values. Add a specific check for undefined values to handle them properly.

    src/background/index.mjs [89-108]

     if (Array.isArray(obj)) {
   const redactedArray = [];
   for (let i = 0; i < obj.length; i++) {
     const item = obj[i];
-    if (item !== null && typeof item === 'object') {
+    if (item === undefined) {
+      redactedArray[i] = undefined;
+    } else if (item !== null && typeof item === 'object') {
       redactedArray[i] = redactSensitiveFields(item, recursionDepth + 1, maxDepth, seen);
     } else if (typeof item === 'string') {
       let isItemSensitive = false;
       const lowerItem = item.toLowerCase();
       for (const keyword of SENSITIVE_KEYWORDS) {
         if (lowerItem.includes(keyword)) {
           isItemSensitive = true;
           break;
         }
       }
       redactedArray[i] = isItemSensitive ? 'REDACTED' : item;
     } else {
       redactedArray[i] = item;
     }
   }
   return redactedArray;
 }
    • Apply / Chat
    Suggestion importance[1-10]: 4

    __

    Why: The current code already handles undefined values correctly in the else branch. Adding explicit undefined handling would improve code clarity but is not necessary.

    		Low
    Fix race condition

    The error handling for Claude session key polling has a potential race
    condition. If the error occurs after promiseSettled has been set to true
    elsewhere but before this check runs, it will incorrectly set promiseSettled
    again and call cleanup() a second time. Add a check to verify promiseSettled is
    still false before proceeding with error handling.

    src/content-script/index.jsx [679-683]

    -if ((errMsg.includes('network') || errMsg.includes('permission')) && !promiseSettled) {
+if (!promiseSettled && (errMsg.includes('network') || errMsg.includes('permission'))) {
   promiseSettled = true;
   cleanup();
   reject(new Error(`Failed to get Claude session key due to: ${err.message}`));
 }
    • Apply / Chat
    Suggestion importance[1-10]: 4

    __

    Why: The suggestion improves code readability by checking !promiseSettled first, but the original code already prevents the race condition with the && !promiseSettled check.

    		Low
    Remove development comment

    There's a comment "Example error check" left in the production code which
    suggests this error handling might be incomplete or was meant to be revised.
    Remove the comment and ensure the error conditions are properly defined for
    production use.

    src/content-script/index.jsx [751]

     const errMsg = err_set.message.toLowerCase();
-if ((errMsg.includes('network') || errMsg.includes('storage')) && !promiseSettled) { // Example error check
+if ((errMsg.includes('network') || errMsg.includes('storage')) && !promiseSettled) {
   promiseSettled = true;
   cleanup();
   reject(new Error(`Failed to process Kimi token: ${err_set.message}`));
 }

    [To ensure code accuracy, apply this suggestion manually]

    Suggestion importance[1-10]: 3

    __

    Why: Valid cleanup suggestion to remove leftover development comment, but has minimal impact on functionality.

    		Low
    Possible issue
    Handle special object types

    The redaction function doesn't handle primitive objects like Date, RegExp, or
    Map/Set, which could lead to errors when logging. These objects are typeof
    'object' but need special handling to avoid errors when iterating through their
    properties.

    src/background/index.mjs [76-87]

     function redactSensitiveFields(obj, recursionDepth = 0, maxDepth = 5, seen = new WeakSet()) {
   if (recursionDepth > maxDepth) {
     return 'REDACTED_TOO_DEEP';
   }
   if (obj === null || typeof obj !== 'object') {
     return obj;
   }
 
+  // Handle special object types
+  if (obj instanceof Date || obj instanceof RegExp || 
+      obj instanceof Error || obj instanceof URL) {
+    return obj.toString();
+  }
+  
+  if (obj instanceof Map || obj instanceof Set) {
+    return `[${obj.constructor.name}]`;
+  }
+
   if (seen.has(obj)) {
     return 'REDACTED_CIRCULAR_REFERENCE';
   }
   seen.add(obj);
    • Apply / Chat
    Suggestion importance[1-10]: 6

    __

    Why: Good suggestion to improve the robustness of the redactSensitiveFields function by handling special object types like Date, RegExp, Map, and Set that could cause errors during property iteration.

    		Low
    Add retry limit for polling

    The interval continues running indefinitely if no session key is found and no
    error occurs. Add a maximum retry count or timeout mechanism to prevent an
    infinite polling loop that could cause memory leaks or excessive resource usage.

    src/content-script/index.jsx [659-685]

    +let retryCount = 0;
+const MAX_RETRIES = 60; // 30 seconds at 500ms intervals
+
 await new Promise((resolve, reject) => {
   timerId = setInterval(async () => {
     if (promiseSettled) {
       console.warn('[content] Promise already settled but Claude interval still running. This indicates a potential cleanup issue.');
       cleanup()
       return
     }
+    
+    retryCount++;
+    if (retryCount >= MAX_RETRIES) {
+      if (!promiseSettled) {
+        promiseSettled = true;
+        cleanup();
+        reject(new Error('Maximum retry count reached while polling for Claude session key.'));
+        return;
+      }
+    }
+    
     try {
       claudeSession = await getClaudeSessionKey()
       if (claudeSession) {
         if (!promiseSettled) {
           promiseSettled = true
           cleanup()
           console.log('[content] Claude session key found after waiting.')
           resolve()
         }
       }
     } catch (err) {
       console.error('[content] Error polling for Claude session key:', err)
       const errMsg = err.message.toLowerCase();
       if ((errMsg.includes('network') || errMsg.includes('permission')) && !promiseSettled) {
         promiseSettled = true;
         cleanup();
         reject(new Error(`Failed to get Claude session key due to: ${err.message}`));
       }
     }
   }, 500)
    • Apply / Chat
    Suggestion importance[1-10]: 4

    __

    Why: The suggestion overlooks that there's already a timeout mechanism (30 seconds) in place to prevent infinite polling. However, adding a retry count could still provide an additional safeguard.

    		Low
    • Update

    Previous suggestions

    ✅ Suggestions up to commit 0b5e7a4
    CategorySuggestion                                                                                                                                    Impact
    Security
    ✅ Fix array handling in redaction
    Suggestion Impact:The commit implemented the core idea of the suggestion by adding special handling for arrays. It now properly checks array items for sensitive content, particularly string values, which was the main security concern raised in the suggestion.

    code diff:

    -  const redactedObj = Array.isArray(obj) ? [] : {};
-  for (const key in obj) {
-    if (Object.prototype.hasOwnProperty.call(obj, key)) {
-      const lowerKey = key.toLowerCase();
-      let isSensitive = false;
-      for (const keyword of SENSITIVE_KEYWORDS) {
-        if (lowerKey.includes(keyword)) {
-          isSensitive = true;
-          break;
-        }
-      }
-      if (isSensitive) {
-        redactedObj[key] = 'REDACTED';
-      } else if (obj[key] !== null && typeof obj[key] === 'object') { // Added obj[key] !== null check
-        redactedObj[key] = redactSensitiveFields(obj[key], recursionDepth + 1, maxDepth, seen);
+  if (Array.isArray(obj)) {
+    const redactedArray = [];
+    for (let i = 0; i < obj.length; i++) {
+      const item = obj[i];
+      if (item !== null && typeof item === 'object') {
+        redactedArray[i] = redactSensitiveFields(item, recursionDepth + 1, maxDepth, seen);
+      } else if (typeof item === 'string') {
+        let isItemSensitive = false;
+        const lowerItem = item.toLowerCase();
+        for (const keyword of SENSITIVE_KEYWORDS) {
+          if (lowerItem.includes(keyword)) {
+            isItemSensitive = true;
+            break;
+          }
+        }
+        redactedArray[i] = isItemSensitive ? 'REDACTED' : item;
       } else {
-        redactedObj[key] = obj[key];
-      }
-    }
+        redactedArray[i] = item;
+      }
+    }
+    return redactedArray;
+  } else {
+    const redactedObj = {};
+    for (const key in obj) {
+      if (Object.prototype.hasOwnProperty.call(obj, key)) {
+        const lowerKey = key.toLowerCase();
+        let isKeySensitive = false;
+        for (const keyword of SENSITIVE_KEYWORDS) {
+          if (lowerKey.includes(keyword)) {
+            isKeySensitive = true;
+            break;
+          }
+        }
+        if (isKeySensitive) {
+          redactedObj[key] = 'REDACTED';
+        } else if (obj[key] !== null && typeof obj[key] === 'object') {
+          redactedObj[key] = redactSensitiveFields(obj[key], recursionDepth + 1, maxDepth, seen);
+        } else {
+          redactedObj[key] = obj[key];
+        }
+      }
+    }
+    return redactedObj;
   }

    The function doesn't properly handle sensitive values in arrays. When iterating
    through object keys, array indices are converted to strings, but the function
    doesn't check array values directly. This could lead to sensitive data leakage
    when redacting arrays containing sensitive strings or objects.

    src/background/index.mjs [76-110]

     function redactSensitiveFields(obj, recursionDepth = 0, maxDepth = 5, seen = new WeakSet()) {
   if (recursionDepth > maxDepth) {
     return 'REDACTED_TOO_DEEP';
   }
   if (obj === null || typeof obj !== 'object') {
     return obj;
   }
 
   if (seen.has(obj)) {
     return 'REDACTED_CIRCULAR_REFERENCE';
   }
   seen.add(obj);
 
   const redactedObj = Array.isArray(obj) ? [] : {};
-  for (const key in obj) {
-    if (Object.prototype.hasOwnProperty.call(obj, key)) {
-      const lowerKey = key.toLowerCase();
-      let isSensitive = false;
-      for (const keyword of SENSITIVE_KEYWORDS) {
-        if (lowerKey.includes(keyword)) {
-          isSensitive = true;
-          break;
+  
+  if (Array.isArray(obj)) {
+    for (let i = 0; i < obj.length; i++) {
+      if (obj[i] !== null && typeof obj[i] === 'object') {
+        redactedObj[i] = redactSensitiveFields(obj[i], recursionDepth + 1, maxDepth, seen);
+      } else if (typeof obj[i] === 'string') {
+        let isSensitive = false;
+        for (const keyword of SENSITIVE_KEYWORDS) {
+          if (obj[i].toLowerCase().includes(keyword)) {
+            isSensitive = true;
+            break;
+          }
         }
+        redactedObj[i] = isSensitive ? 'REDACTED' : obj[i];
+      } else {
+        redactedObj[i] = obj[i];
       }
-      if (isSensitive) {
-        redactedObj[key] = 'REDACTED';
-      } else if (obj[key] !== null && typeof obj[key] === 'object') { // Added obj[key] !== null check
-        redactedObj[key] = redactSensitiveFields(obj[key], recursionDepth + 1, maxDepth, seen);
-      } else {
-        redactedObj[key] = obj[key];
+    }
+  } else {
+    for (const key in obj) {
+      if (Object.prototype.hasOwnProperty.call(obj, key)) {
+        const lowerKey = key.toLowerCase();
+        let isSensitive = false;
+        for (const keyword of SENSITIVE_KEYWORDS) {
+          if (lowerKey.includes(keyword)) {
+            isSensitive = true;
+            break;
+          }
+        }
+        if (isSensitive) {
+          redactedObj[key] = 'REDACTED';
+        } else if (obj[key] !== null && typeof obj[key] === 'object') {
+          redactedObj[key] = redactSensitiveFields(obj[key], recursionDepth + 1, maxDepth, seen);
+        } else {
+          redactedObj[key] = obj[key];
+        }
       }
     }
   }
   return redactedObj;
 }

    [Suggestion processed]

    Suggestion importance[1-10]: 7

    __

    Why: The suggestion enhances security by adding explicit handling for sensitive string values within arrays. While the existing code handles array structure, it doesn't check for sensitive keywords in array string values, which could lead to data leakage. This is a meaningful security improvement.

    		Medium
    Possible issue
    Check port connection before messaging

    The code attempts to send a message to the port after potentially removing event
    listeners, which could lead to a race condition. If the port is already
    disconnected, the postMessage call will fail. Add a check to verify the port is
    still connected before attempting to send the error message.

    src/background/index.mjs [171-185]

     if (port._reconnectAttempts > RECONNECT_CONFIG.MAX_ATTEMPTS) {
   console.error(`[background] Max reconnect attempts (${RECONNECT_CONFIG.MAX_ATTEMPTS}) reached for tab ${proxyTabId}. Giving up.`);
   if (port._portOnMessage) {
       try { port.onMessage.removeListener(port._portOnMessage); }
       catch(e) { console.warn("[background] Error removing _portOnMessage on max retries:", e); }
   }
   if (port._portOnDisconnect) { // Cleanup _portOnDisconnect as well
       try { port.onDisconnect.removeListener(port._portOnDisconnect); }
       catch(e) { console.warn("[background] Error removing _portOnDisconnect on max retries:", e); }
   }
   try { // Notify user about final connection failure
-    port.postMessage({ error: `Connection to ChatGPT tab lost after ${RECONNECT_CONFIG.MAX_ATTEMPTS} attempts. Please refresh the page.` });
+    if (port.onMessage) { // Check if port is still connected
+      port.postMessage({ error: `Connection to ChatGPT tab lost after ${RECONNECT_CONFIG.MAX_ATTEMPTS} attempts. Please refresh the page.` });
+    }
   } catch(e) {
     console.warn("[background] Error sending final error message on max retries:", e);
   }
   return;
 }
    Suggestion importance[1-10]: 6

    __

    Why: The suggestion addresses a potential race condition where the port.postMessage could fail if the port is disconnected. Adding a check for port.onMessage before posting messages is a reasonable defensive programming practice that prevents unnecessary errors.

    		Low
    ✅ Handle consecutive polling failures
    Suggestion Impact:The commit implemented error handling for Claude session key polling, but used a different approach. Instead of counting consecutive errors, it checks for specific error types (network or permission errors) and rejects the promise immediately when those occur.

    code diff:

    @@ -679,12 +675,12 @@
                 }
               } catch (err) {
                 console.error('[content] Error polling for Claude session key:', err)
-                // Example for Qodo: Stop on specific error
-                // if (err.message.includes('NetworkError') && !promiseSettled) {
-                //   promiseSettled = true;
-                //   cleanup();
-                //   reject(new Error(`Failed to get Claude session key: ${err.message}`));
-                // }
+                const errMsg = err.message.toLowerCase();
+                if ((errMsg.includes('network') || errMsg.includes('permission')) && !promiseSettled) {
+                  promiseSettled = true;
+                  cleanup();
+                  reject(new Error(`Failed to get Claude session key due to: ${err.message}`));
+                }
               }

    The interval continues running even if there's an error, but there's no
    mechanism to reject the promise on repeated failures. Add a counter to track
    consecutive errors and reject the promise after a certain threshold to prevent
    infinite polling when the service is unavailable.

    src/content-script/index.jsx [663-689]

     let promiseSettled = false
 let timerId = null
 let timeoutId = null
+let errorCount = 0
+const MAX_CONSECUTIVE_ERRORS = 5
 const cleanup = () => {
   if (timerId) clearInterval(timerId)
   if (timeoutId) clearTimeout(timeoutId)
 }
 
 await new Promise((resolve, reject) => {
   timerId = setInterval(async () => {
     if (promiseSettled) {
       console.warn('[content] Promise already settled but Claude interval still running. This indicates a potential cleanup issue.');
       cleanup()
       return
     }
     try {
       claudeSession = await getClaudeSessionKey()
+      errorCount = 0 // Reset error count on successful call
       if (claudeSession) {
         if (!promiseSettled) {
           promiseSettled = true
           cleanup()
           console.log('[content] Claude session key found after waiting.')
           resolve()
         }
       }
     } catch (err) {
-      console.error('[content] Error polling for Claude session key:', err)
+      errorCount++
+      console.error(`[content] Error polling for Claude session key (${errorCount}/${MAX_CONSECUTIVE_ERRORS}):`, err)
+      if (errorCount >= MAX_CONSECUTIVE_ERRORS && !promiseSettled) {
+        promiseSettled = true
+        cleanup()
+        reject(new Error(`Failed to get Claude session key after ${MAX_CONSECUTIVE_ERRORS} consecutive errors`))
+      }
     }
   }, 500)
    Suggestion importance[1-10]: 6

    __

    Why: The suggestion improves error handling by adding a mechanism to stop infinite polling after consecutive failures. This prevents the code from getting stuck in an endless loop when the service is consistently unavailable, which is a good defensive programming practice.

    		Low
    Incremental [*]
    ✅ Properly handle HTTP errors
    Suggestion Impact:The commit implemented a different approach to handle HTTP errors. Instead of using an early return, it created a responseObject with error information when response is not OK, which achieves the same goal of properly handling HTTP errors.

    code diff:

    @@ -466,22 +510,21 @@
         try {
           const response = await fetch(message.data.input, message.data.init)
           const text = await response.text()
-          if (!response.ok) { // Added check for HTTP error statuses
+          const responseObject = { // Defined for clarity before conditional error property
+            body: text,
+            ok: response.ok,
+            status: response.status,
+            statusText: response.statusText,
+            headers: Object.fromEntries(response.headers),
+          };
+          if (!response.ok) {
+            responseObject.error = `HTTP error ${response.status}: ${response.statusText}`;
             console.warn(`[background] FETCH received error status: ${response.status} for ${message.data.input}`);
           }
           console.debug(
             `[background] FETCH successful for ${message.data.input}, status: ${response.status}`,
           )
-          return [
-            {
-              body: text,
-              ok: response.ok, // Added ok status
-              status: response.status,
-              statusText: response.statusText,
-              headers: Object.fromEntries(response.headers),
-            },
-            null,
-          ]
+          return [responseObject, null];

    The code logs a warning for non-OK HTTP responses but doesn't handle the error
    condition properly. Since the response is still processed as if it were
    successful, this could lead to unexpected behavior when API calls fail.

    src/background/index.mjs [469-471]

     if (!response.ok) { // Added check for HTTP error statuses
   console.warn(`[background] FETCH received error status: ${response.status} for ${message.data.input}`);
+  return [
+    {
+      body: text,
+      ok: response.ok,
+      status: response.status,
+      statusText: response.statusText,
+      headers: Object.fromEntries(response.headers),
+      error: `HTTP error: ${response.status} ${response.statusText}`
+    },
+    null,
+  ]
 }
    Suggestion importance[1-10]: 3

    __

    Why: The current implementation correctly includes ok: response.ok in the response, allowing callers to handle HTTP errors appropriately. The suggested early return would change the API behavior and might break existing code expecting response bodies for error statuses.

    		Low
    ✅ Suggestions up to commit a986d2e
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    ✅ Clean up all event listeners
    Suggestion Impact:The commit implemented the suggestion by adding code to clean up the port._portOnDisconnect listener when max reconnect attempts are reached, along with additional error handling to notify the user about the connection failure

    code diff:

    +        if (port._portOnDisconnect) { // Cleanup _portOnDisconnect as well
+            try { port.onDisconnect.removeListener(port._portOnDisconnect); }
+            catch(e) { console.warn("[background] Error removing _portOnDisconnect on max retries:", e); }
+        }

    The code is missing cleanup for port._portOnDisconnect listener when max
    reconnect attempts are reached. This can lead to memory leaks and potential
    errors if the port is later reused. Add code to remove this listener as well.

    src/background/index.mjs [156-164]

     port._reconnectAttempts = (port._reconnectAttempts || 0) + 1;
 if (port._reconnectAttempts > RECONNECT_CONFIG.MAX_ATTEMPTS) {
   console.error(`[background] Max reconnect attempts (${RECONNECT_CONFIG.MAX_ATTEMPTS}) reached for tab ${proxyTabId}. Giving up.`);
   if (port._portOnMessage) {
       try { port.onMessage.removeListener(port._portOnMessage); }
       catch(e) { console.warn("[background] Error removing _portOnMessage on max retries:", e); }
   }
+  if (port._portOnDisconnect) {
+      try { port.onDisconnect.removeListener(port._portOnDisconnect); }
+      catch(e) { console.warn("[background] Error removing _portOnDisconnect on max retries:", e); }
+  }
   return;
 }

    [Suggestion processed]

    Suggestion importance[1-10]: 7

    __

    Why: This addresses a potential memory leak by ensuring port._portOnDisconnect listener is also removed when max reconnect attempts are reached, preventing orphaned event listeners.

    		Medium
    ✅ Fix null object handling
    Suggestion Impact:The commit implemented the exact suggestion by adding a null check before the typeof check in the redactSensitiveFields function, preventing potential runtime errors when obj[key] is null

    code diff:

    -      } else if (typeof obj[key] === 'object') {
+      } else if (obj[key] !== null && typeof obj[key] === 'object') { // Added obj[key] !== null check

    The redactSensitiveFields function doesn't handle null values correctly. When
    obj[key] is null, the typeof check will return 'object', but null can't be
    processed recursively. Add a specific check for null values.

    src/background/index.mjs [97-98]

     function redactSensitiveFields(obj, recursionDepth = 0, maxDepth = 5, seen = new WeakSet()) {
   if (recursionDepth > maxDepth) {
     return 'REDACTED_TOO_DEEP';
   }
   if (obj === null || typeof obj !== 'object') {
     return obj;
   }
 
   if (seen.has(obj)) {
     return 'REDACTED_CIRCULAR_REFERENCE';
   }
   seen.add(obj);
 
   const redactedObj = Array.isArray(obj) ? [] : {};
   for (const key in obj) {
     if (Object.prototype.hasOwnProperty.call(obj, key)) {
       const lowerKey = key.toLowerCase();
       let isSensitive = false;
       for (const keyword of SENSITIVE_KEYWORDS) {
         if (lowerKey.includes(keyword)) {
           isSensitive = true;
           break;
         }
       }
       if (isSensitive) {
         redactedObj[key] = 'REDACTED';
-      } else if (typeof obj[key] === 'object') {
+      } else if (obj[key] !== null && typeof obj[key] === 'object') {
         redactedObj[key] = redactSensitiveFields(obj[key], recursionDepth + 1, maxDepth, seen);
       } else {
         redactedObj[key] = obj[key];
       }
     }
   }
   return redactedObj;
 }
    Suggestion importance[1-10]: 6

    __

    Why: This prevents potential runtime errors when obj[key] is null by adding an explicit null check before recursive calls, improving the function's defensive programming.

    		Low
    General
    Improve login button detection

    The code uses a fixed selector to find the login button by its text content
    '立即登录', but this approach is brittle and may fail if the UI changes or is
    localized. Use a more reliable selector or add a fallback mechanism.

    src/content-script/index.jsx [702-717]

     if (location.hostname === 'kimi.moonshot.cn') {
   console.debug('[content] On kimi.moonshot.cn, checking login status.')
   if (!window.localStorage.refresh_token) {
     console.log('[content] Kimi refresh token not found, attempting to trigger login.')
     setTimeout(() => {
       try {
+        // Try to find by text content first
+        let loginButton = null;
         document.querySelectorAll('button').forEach((button) => {
-          if (button.textContent === '立即登录') {
-            console.log('[content] Clicking Kimi login button.')
-            button.click()
+          if (button.textContent === '立即登录' || 
+              button.textContent.toLowerCase().includes('login') ||
+              button.getAttribute('data-testid') === 'login-button') {
+            loginButton = button;
+            console.log('[content] Found Kimi login button:', button.textContent);
           }
-        })
+        });
+        
+        if (loginButton) {
+          console.log('[content] Clicking Kimi login button.');
+          loginButton.click();
+        } else {
+          console.warn('[content] Could not find Kimi login button.');
+        }
       } catch (err_click) {
         console.error('[content] Error clicking Kimi login button:', err_click)
       }
     }, 1000)
    Suggestion importance[1-10]: 4

    __

    Why: The suggestion improves robustness by adding fallback mechanisms for button detection, but the current hardcoded approach is functional and this is primarily a maintainability enhancement.

    		Low
    Security
    ✅ Enhance security redaction coverage
    Suggestion Impact:The commit directly implemented the suggestion by adding all the suggested sensitive keywords ('jwt', 'session', 'access', 'private', 'oauth') to the SENSITIVE_KEYWORDS array exactly as recommended

    code diff:

       'credential',
+  'jwt',
+  'session',
+  'access',
+  'private',
+  'oauth',

    The SENSITIVE_KEYWORDS array is missing common sensitive terms like 'jwt',
    'session', and 'access'. Add these terms to ensure comprehensive redaction of
    all sensitive information in logs.

    src/background/index.mjs [60-69]

     const SENSITIVE_KEYWORDS = [
   'apikey',
   'token',
   'secret',
   'password',
   'kimimoonshotrefreshtoken',
   'auth',
   'key',
   'credential',
+  'jwt',
+  'session',
+  'access',
+  'private',
+  'oauth'
 ];

    [Suggestion processed]

    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly identifies missing sensitive keywords like jwt, session, and oauth that should be redacted for better security coverage in logs.

    		Medium
    Possible issue
    ✅ Improve HTTP error handling
    Suggestion Impact:The commit implemented the suggestion by adding a check for response.ok and including the ok property in the returned object. The implementation is slightly different from the suggestion but achieves the same goal.

    code diff:

             try {
           const response = await fetch(message.data.input, message.data.init)
           const text = await response.text()
+          if (!response.ok) { // Added check for HTTP error statuses
+            console.warn(`[background] FETCH received error status: ${response.status} for ${message.data.input}`);
+          }
           console.debug(
             `[background] FETCH successful for ${message.data.input}, status: ${response.status}`,
           )
           return [
             {
               body: text,
+              ok: response.ok, // Added ok status
               status: response.status,
               statusText: response.statusText,
               headers: Object.fromEntries(response.headers),

    The fetch operation doesn't handle non-network errors properly. If the response
    is received but has an error status code (e.g., 404, 500), it will still be
    treated as a success. Add status code checking to properly handle HTTP error
    responses.

    src/background/index.mjs [433-451]

     try {
   const response = await fetch(message.data.input, message.data.init)
   const text = await response.text()
   console.debug(
-    `[background] FETCH successful for ${message.data.input}, status: ${response.status}`,
+    `[background] FETCH for ${message.data.input}, status: ${response.status}`,
   )
+  if (!response.ok) {
+    console.warn(`[background] FETCH received error status: ${response.status} for ${message.data.input}`)
+  }
   return [
     {
       body: text,
       status: response.status,
       statusText: response.statusText,
       headers: Object.fromEntries(response.headers),
+      ok: response.ok
     },
     null,
   ]
 } catch (error) {
   console.error(`[background] FETCH error for ${message.data.input}:`, error)
   return [null, { message: error.message, stack: error.stack }]
 }

    [Suggestion processed]

    Suggestion importance[1-10]: 6

    __

    Why: The suggestion correctly identifies that HTTP error status codes (4xx, 5xx) are currently treated as successful responses. Adding response.ok checking and including the ok property improves error handling for the caller.

    		Low
    ✅ Prevent property access errors
    Suggestion Impact:The commit implemented the suggestion's intent by adding try-catch blocks around port.proxy.postMessage calls in multiple places (lines 99-108, 133-159), which provides more robust error handling when accessing port.proxy properties. This achieves the same goal as the suggested code changes, though with a different implementation approach.

    code diff:

    -        port.proxy.postMessage(msg)
+        try {
+          port.proxy.postMessage(msg)
+        } catch (e) {
+          console.error('[background] Error posting message to proxy tab in _portOnMessage:', e, msg);
+          try {
+            port.postMessage({ error: 'Failed to forward message to target tab. Tab might be closed or an extension error occurred.' });
+          } catch (notifyError) {
+            console.error('[background] Error sending forwarding failure notification back to original sender:', notifyError);
+          }
+        }
       } else {
         console.warn('[background] Port proxy not available to send message:', msg)
       }
@@ -159,6 +196,15 @@
         if (port._portOnMessage) {
             try { port.onMessage.removeListener(port._portOnMessage); }
             catch(e) { console.warn("[background] Error removing _portOnMessage on max retries:", e); }
+        }
+        if (port._portOnDisconnect) {
+            try { port.onDisconnect.removeListener(port._portOnDisconnect); }
+            catch(e) { console.warn("[background] Error removing _portOnDisconnect from main port on max retries:", e); }
+        }
+        try {
+          port.postMessage({ error: `Connection to ChatGPT tab lost after ${RECONNECT_CONFIG.MAX_ATTEMPTS} attempts. Please refresh the page.` });
+        } catch(e) {
+          console.warn("[background] Error sending final error message on max retries:", e);
         }
         return;
       }
@@ -270,11 +316,42 @@
         }
         if (port.proxy) {
           console.debug('[background] Posting message to proxy tab:', { session })
-          port.proxy.postMessage({ session })
+          try {
+            port.proxy.postMessage({ session })
+          } catch (e) {
+            console.warn('[background] Error posting message to existing proxy tab in executeApi (ChatGPT Web Model):', e, '. Attempting to reconnect.', { session });
+            setPortProxy(port, tabId); // Attempt to re-establish the connection
+            if (port.proxy) {
+              console.debug('[background] Proxy re-established. Attempting to post message again.');
+              try {
+                port.proxy.postMessage({ session });
+                console.info('[background] Successfully posted session after proxy reconnection.');
+              } catch (e2) {
+                console.error('[background] Error posting message even after proxy reconnection:', e2, { session });
+                try {
+                  port.postMessage({ error: 'Failed to communicate with ChatGPT tab after reconnection attempt. Try refreshing the page.' });
+                } catch (notifyError) {
+                  console.error('[background] Error sending final communication failure notification back:', notifyError);
+                }
+              }
+            } else {
+              console.error('[background] Failed to re-establish proxy connection. Cannot send session.');
+              try {
+                port.postMessage({ error: 'Could not re-establish connection to ChatGPT tab. Try refreshing the page.' });
+              } catch (notifyError) {
+                console.error('[background] Error sending re-establishment failure notification back:', notifyError);
+              }
+            }
+          }

    The code doesn't check if port.proxy is valid before accessing its properties.
    If port.proxy exists but is in an invalid state (e.g., disconnected or
    destroyed), accessing its properties could throw errors. Add a more robust check
    to prevent potential runtime errors.

    src/background/index.mjs [111-118]

     if (port.proxy) {
     try {
-        if (port._proxyOnMessage) port.proxy.onMessage.removeListener(port._proxyOnMessage);
-        if (port._proxyOnDisconnect) port.proxy.onDisconnect.removeListener(port._proxyOnDisconnect);
+        if (port._proxyOnMessage && port.proxy.onMessage) port.proxy.onMessage.removeListener(port._proxyOnMessage);
+        if (port._proxyOnDisconnect && port.proxy.onDisconnect) port.proxy.onDisconnect.removeListener(port._proxyOnDisconnect);
     } catch(e) {
         console.warn('[background] Error removing old listeners from previous port.proxy instance:', e);
     }
 }
    Suggestion importance[1-10]: 4

    __

    Why: The suggestion adds additional safety checks before accessing port.proxy properties, which could prevent runtime errors. However, the existing code already has try-catch protection, making this a minor defensive improvement.

    		Low
    Possible issue
    ✅ Fix Manifest V3 compatibility
    Suggestion Impact:The commit implemented the exact suggestion by replacing the static ['blocking', 'requestHeaders'] with a conditional expression that only includes 'blocking' for Manifest V2, ensuring compatibility with Manifest V3 browsers

    code diff:

    -    ['blocking', 'requestHeaders'],
+    ['requestHeaders', ...(Browser.runtime.getManifest().manifest_version < 3 ? ['blocking'] : [])],

    The 'blocking' option in the webRequest API is deprecated in Manifest V3 and
    will cause errors in browsers that enforce V3 compatibility. This will prevent
    header modifications from working correctly for Bing API requests.

    src/background/index.mjs [530-566]

     Browser.webRequest.onBeforeSendHeaders.addListener(
   (details) => {
     try {
       console.debug('[background] onBeforeSendHeaders triggered for URL:', details.url)
       const headers = details.requestHeaders
       let modified = false
       for (let i = 0; i < headers.length; i++) {
         if (headers[i]) {
           const headerNameLower = headers[i].name?.toLowerCase();
           if (headerNameLower === 'origin') {
             headers[i].value = 'https://www.bing.com'
             modified = true
           } else if (headerNameLower === 'referer') {
             headers[i].value = 'https://www.bing.com/search?q=Bing+AI&showconv=1&FORM=hpcodx'
             modified = true
           }
         }
       }
       if (modified) {
         console.debug('[background] Modified headers for Bing:', headers)
       }
       return { requestHeaders: headers }
     } catch (error) {
       console.error(
         '[background] Error in onBeforeSendHeaders listener callback:',
         error,
         details,
       )
       return { requestHeaders: details.requestHeaders }
     }
   },
   {
     urls: ['wss://sydney.bing.com/*', 'https://www.bing.com/*'],
     types: ['xmlhttprequest', 'websocket'],
   },
-  ['blocking', 'requestHeaders'],
+  ['requestHeaders', ...(Browser.runtime.getManifest().manifest_version < 3 ? ['blocking'] : [])],
 )

    [Suggestion processed]

    Suggestion importance[1-10]: 7

    __

    Why: Important compatibility fix for Manifest V3 where the blocking option is deprecated. The conditional inclusion based on manifest version ensures the extension works correctly across different browser versions and future-proofs the code.

    		Medium
    ✅ Prevent infinite polling loops
    Suggestion Impact:The commit implemented a commented-out example of the suggested error handling pattern. While not directly implementing the suggestion, it shows the developer acknowledged the issue and provided a template for how to handle specific errors to prevent infinite polling.

    code diff:

    +                // Example for Qodo: Stop on specific error
+                // if (err.message.includes('NetworkError') && !promiseSettled) {
+                //   promiseSettled = true;
+                //   cleanup();
+                //   reject(new Error(`Failed to get Claude session key: ${err.message}`));
+                // }

    The polling interval doesn't handle errors properly. If an exception occurs
    during getClaudeSessionKey(), the interval continues running indefinitely
    without resolving or rejecting the promise. Add error handling to prevent
    infinite polling on persistent errors.

    src/content-script/index.jsx [664-683]

     let timerId = setInterval(async () => {
   if (promiseSettled) {
     console.warn('[content] Promise already settled but Claude interval still running. This indicates a potential cleanup issue.');
     cleanup()
     return
   }
   try {
     claudeSession = await getClaudeSessionKey()
     if (claudeSession) {
       if (!promiseSettled) {
         promiseSettled = true
         cleanup()
         console.log('[content] Claude session key found after waiting.')
         resolve()
       }
     }
   } catch (err) {
     console.error('[content] Error polling for Claude session key:', err)
+    // Prevent infinite polling on persistent errors
+    if (err.message && (err.message.includes('network') || err.message.includes('permission'))) {
+      promiseSettled = true
+      cleanup()
+      reject(new Error(`Failed to get Claude session key: ${err.message}`))
+    }
   }
 }, 500)
    Suggestion importance[1-10]: 6

    __

    Why: Good suggestion to prevent infinite polling when persistent errors occur. The improved error handling checks for specific error types and properly rejects the promise, preventing resource waste and potential memory leaks.

    		Low
    ✅ Handle message serialization errors
    Suggestion Impact:The commit implemented the suggested try-catch block around port.proxy.postMessage(msg) with error handling, though with slightly different error message text and additional nested try-catch for the error notification

    code diff:

         port._portOnMessage = (msg) => {
       console.debug('[background] Message to proxy tab:', msg)
       if (port.proxy) {
-        port.proxy.postMessage(msg)
+        try {
+          port.proxy.postMessage(msg)
+        } catch (e) {
+          console.error('[background] Error posting message to proxy tab in _portOnMessage:', e, msg);
+          try { // Attempt to notify the original sender about the failure
+            port.postMessage({ error: 'Failed to forward message to target tab. Tab might be closed or an extension error occurred.' });
+          } catch (notifyError) {
+            console.error('[background] Error sending forwarding failure notification back to original sender:', notifyError);
+          }
+        }
       } else {

    The code attempts to send a message to port.proxy without checking if the
    message is serializable. Non-serializable objects (like functions, circular
    references) will cause postMessage to throw errors. Add a try-catch block to
    handle potential serialization errors.

    src/background/index.mjs [130-137]

     port._portOnMessage = (msg) => {
   console.debug('[background] Message to proxy tab:', msg)
   if (port.proxy) {
-    port.proxy.postMessage(msg)
+    try {
+      port.proxy.postMessage(msg)
+    } catch (error) {
+      console.error('[background] Failed to send message to proxy tab:', error)
+      port.postMessage({ error: 'Failed to forward message to target tab' })
+    }
   } else {
     console.warn('[background] Port proxy not available to send message:', msg)
   }
 }

    [Suggestion processed]

    Suggestion importance[1-10]: 5

    __

    Why: Valid suggestion to add error handling for postMessage serialization failures. This prevents potential crashes when non-serializable objects are sent through the message port, improving robustness.

    		Low
    Possible issue
    ✅ Handle message serialization errors
    Suggestion Impact:The commit implemented the suggested try-catch block around port.proxy.postMessage(msg) with error handling, though with slightly different error message text and additional nested try-catch for the error notification

    code diff:

         port._portOnMessage = (msg) => {
       console.debug('[background] Message to proxy tab:', msg)
       if (port.proxy) {
-        port.proxy.postMessage(msg)
+        try {
+          port.proxy.postMessage(msg)
+        } catch (e) {
+          console.error('[background] Error posting message to proxy tab in _portOnMessage:', e, msg);
+          try { // Attempt to notify the original sender about the failure
+            port.postMessage({ error: 'Failed to forward message to target tab. Tab might be closed or an extension error occurred.' });
+          } catch (notifyError) {
+            console.error('[background] Error sending forwarding failure notification back to original sender:', notifyError);
+          }
+        }
       } else {

    The code doesn't handle potential errors when posting a message to the proxy
    tab. If the session object contains circular references or non-serializable
    data, postMessage will throw an error that's not caught. Add a try-catch block
    to handle this scenario gracefully.

    src/background/index.mjs [271-278]

     if (port.proxy) {
   console.debug('[background] Posting message to proxy tab:', { session })
-  port.proxy.postMessage({ session })
+  try {
+    port.proxy.postMessage({ session })
+  } catch (error) {
+    console.error('[background] Error posting message to proxy tab:', error)
+    port.postMessage({ error: 'Failed to communicate with ChatGPT tab. Try refreshing the page.' })
+  }
 } else {
   console.error(
     '[background] Failed to send message: port.proxy is still not available after setPortProxy.',
   )
 }
    Suggestion importance[1-10]: 6

    __

    Why: Valid suggestion to add error handling for postMessage which can throw serialization errors, though the impact is moderate since it addresses an edge case.

    		Low
    Possible issue
    ✅ Notify user on connection failure
    Suggestion Impact:The suggestion was implemented in the commit. Lines 60-64 of the diff show the exact implementation of the suggested code that notifies users when connection is permanently lost after max reconnection attempts.

    code diff:

    +        try { // Notify user about final connection failure
+          port.postMessage({ error: `Connection to ChatGPT tab lost after ${RECONNECT_CONFIG.MAX_ATTEMPTS} attempts. Please refresh the page.` });
+        } catch(e) {
+          console.warn("[background] Error sending final error message on max retries:", e);
         }

    When giving up on reconnection attempts, the code should send a notification to
    the client that the connection has been permanently lost. This would provide
    better user experience by informing them of the connection failure rather than
    silently failing.

    src/background/index.mjs [157-164]

     if (port._reconnectAttempts > RECONNECT_CONFIG.MAX_ATTEMPTS) {
   console.error(`[background] Max reconnect attempts (${RECONNECT_CONFIG.MAX_ATTEMPTS}) reached for tab ${proxyTabId}. Giving up.`);
   if (port._portOnMessage) {
       try { port.onMessage.removeListener(port._portOnMessage); }
       catch(e) { console.warn("[background] Error removing _portOnMessage on max retries:", e); }
   }
+  try {
+      port.postMessage({ error: `Connection to ChatGPT tab lost after ${RECONNECT_CONFIG.MAX_ATTEMPTS} attempts. Please refresh the page.` });
+  } catch(e) {
+      console.warn("[background] Error sending final error message on max retries:", e);
+  }
   return;
 }

    [Suggestion processed]

    Suggestion importance[1-10]: 7

    __

    Why: Good user experience improvement by notifying users when connection fails permanently. However, the impact is moderate since this is error handling for an edge case.

    		Medium
    Prevent timer resource leaks

    The cleanup function is defined but not called if an error occurs during the
    promise setup. Add a try-catch block around the entire promise creation to
    ensure cleanup is always called, preventing potential memory leaks from
    lingering intervals and timeouts.

    src/content-script/index.jsx [658-665]

     let promiseSettled = false
 let timerId = null
 let timeoutId = null
 const cleanup = () => {
   if (timerId) clearInterval(timerId)
   if (timeoutId) clearTimeout(timeoutId)
 }
 
+try {
+  await new Promise((resolve, reject) => {
+    // Promise implementation...
+  }).catch((err) => {
+    console.error('[content] Failed to get Claude session key for jump back notification:', err)
+    return
+  });
+} catch (error) {
+  console.error('[content] Unexpected error in Claude session handling:', error);
+  cleanup(); // Ensure cleanup happens even if promise setup fails
+}
+
    Suggestion importance[1-10]: 2

    __

    Why: The existing_code doesn't match the actual implementation in the diff. The code already has proper .catch() error handling, making this suggestion less relevant.

    		Low
    Security
    Detect encoded sensitive data

    The redaction function doesn't handle Base64 encoded secrets or API keys. Add
    detection for common Base64 patterns to prevent leaking sensitive information in
    logs that might be encoded rather than in plain text.

    src/background/index.mjs [60-69]

     const SENSITIVE_KEYWORDS = [
   'apikey',
   'token',
   'secret',
   'password',
   'kimimoonshotrefreshtoken',
   'auth',
   'key',
   'credential',
 ];
 
+const BASE64_PATTERNS = [
+  /^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$/,
+  /^[A-Za-z0-9_-]{21,22}$/,  // URL-safe base64 without padding
+];
+
    Suggestion importance[1-10]: 1

    __

    Why: Just adding BASE64_PATTERNS without integrating them into the redactSensitiveFields function provides no benefit. The improved code doesn't show how these patterns would be used.

    		Low
    ✅ Suggestions up to commit 789d8fe
    CategorySuggestion                                                                                                                                    Impact
    Incremental [*]
    ✅ Fix potential null reference
    Suggestion Impact:The commit implemented the suggestion by adding a check for headers[i] existence before accessing its properties, but with a slightly different implementation approach. Instead of adding the check to each condition, the code was restructured to wrap all header operations in a single if(headers[i]) check.

    code diff:

    -          const headerNameLower = headers[i]?.name?.toLowerCase(); // Apply optional chaining
-          if (headerNameLower === 'origin') {
-            headers[i].value = 'https://www.bing.com'
-            modified = true
-          } else if (headerNameLower === 'referer') {
-            headers[i].value = 'https://www.bing.com/search?q=Bing+AI&showconv=1&FORM=hpcodx'
-            modified = true
+          if (headers[i]) {
+            const headerNameLower = headers[i].name?.toLowerCase();
+            if (headerNameLower === 'origin') {
+              headers[i].value = 'https://www.bing.com'
+              modified = true
+            } else if (headerNameLower === 'referer') {
+              headers[i].value = 'https://www.bing.com/search?q=Bing+AI&showconv=1&FORM=hpcodx'
+              modified = true
+            }
           }

    After using optional chaining for headers[i]?.name, you should verify that
    headers[i] exists before attempting to modify its value property. Otherwise, if
    a header entry is undefined but has a name property that matches, the code will
    throw an error when trying to set the value.

    src/background/index.mjs [543-550]

     const headerNameLower = headers[i]?.name?.toLowerCase(); // Apply optional chaining
-if (headerNameLower === 'origin') {
+if (headers[i] && headerNameLower === 'origin') {
   headers[i].value = 'https://www.bing.com'
   modified = true
-} else if (headerNameLower === 'referer') {
+} else if (headers[i] && headerNameLower === 'referer') {
   headers[i].value = 'https://www.bing.com/search?q=Bing+AI&showconv=1&FORM=hpcodx'
   modified = true
 }

    [Suggestion processed]

    Suggestion importance[1-10]: 8

    __

    Why: This is a valid bug fix - using optional chaining on headers[i]?.name but not checking if headers[i] exists before setting its value property could cause runtime errors.

    		Medium
    ✅ Enhance sensitive data detection
    Suggestion Impact:The commit implemented exactly the suggested enhancement by adding the three recommended keywords ('auth', 'key', 'credential') to the SENSITIVE_KEYWORDS array, improving the detection of sensitive information in the application.

    code diff:

     const SENSITIVE_KEYWORDS = [
-  'apikey', // Covers apiKey, customApiKey, claudeApiKey, etc.
-  'token',  // Covers accessToken, refreshToken, etc.
+  'apikey',
+  'token',
   'secret',
   'password',
   'kimimoonshotrefreshtoken',
+  'auth',
+  'key',
+  'credential',
 ];

    The SENSITIVE_KEYWORDS array should include more comprehensive patterns to catch
    all potential sensitive information. Currently, it might miss some common
    sensitive data patterns like 'auth', 'key', 'credential', etc.

    src/background/index.mjs [60-66]

     const SENSITIVE_KEYWORDS = [
   'apikey', // Covers apiKey, customApiKey, claudeApiKey, etc.
   'token',  // Covers accessToken, refreshToken, etc.
   'secret',
   'password',
   'kimimoonshotrefreshtoken',
+  'auth',   // Covers authorization, oauth, etc.
+  'key',    // Catches generic keys
+  'credential',
 ];

    [Suggestion processed]

    Suggestion importance[1-10]: 6

    __

    Why: Adding more comprehensive keywords like auth, key, and credential would improve the security redaction function, though the current implementation already covers the most common patterns.

    		Low
    ✅ Improve cross-browser compatibility
    Suggestion Impact:The commit implemented a robust fallback mechanism for the Browser.sidePanel API by adding chrome.sidePanel as a fallback option, along with better error handling and checking if functions exist before calling them.

    code diff:

       Browser.tabs.onUpdated.addListener(async (tabId, info, tab) => {
+    const outerTryCatchError = (error) => { // Renamed to avoid conflict with inner error
+      console.error('[background] Error in tabs.onUpdated listener callback (outer):', error, tabId, info);
+    };
     try {
-      // Refined condition: Ensure URL exists and tab loading is complete.
       if (!tab.url || (info.status && info.status !== 'complete')) {
         console.debug(
           `[background] Skipping side panel update for tabId: ${tabId}. Tab URL: ${tab.url}, Info Status: ${info.status}`,
-        )
-        return
+        );
+        return;
       }
       console.debug(
         `[background] tabs.onUpdated event for tabId: ${tabId}, status: ${info.status}, url: ${tab.url}. Proceeding with side panel update.`,
-      )
-      // Use Browser.sidePanel from webextension-polyfill for consistency and cross-browser compatibility
-      if (Browser.sidePanel) {
-        await Browser.sidePanel.setOptions({
-          tabId,
-          pat...

    coderabbitai[bot]
    coderabbitai bot reviewed Jun 7, 2025
    View reviewed changes
    Copy link
    Contributor

    @coderabbitai coderabbitai bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Actionable comments posted: 3

    🧹 Nitpick comments (3)
    src/background/index.mjs (1)

    433-439: Returning {} on failure drops request headers

    If an exception occurs we currently return an empty object, which strips all headers and can break the request:

    -        return {} // Return empty object or original headers on error?
+        return { requestHeaders: details.requestHeaders }
    src/content-script/index.jsx (2)

    622-627: Dead code triggers “defined but never used” lint warning

    prepareForForegroundRequests is kept as a no-op placeholder but still defined.
    Either delete it or export it for tests to silence the linter.

    🧰 Tools
    🪛 GitHub Check: tests

    [failure] 622-622:
    'prepareForForegroundRequests' is defined but never used

    878-916: Risk of multiple registerPortListener registrations

    registerPortListener is invoked every page load; if the script is injected twice (e.g. SPA navigation) duplicate listeners will accumulate.

    Guard with a module-level flag:

    +if (!window.__chatgptboxPortListenerRegistered) {
   registerPortListener(…)
+  window.__chatgptboxPortListenerRegistered = true
+}
    📜 Review details

    Configuration used: CodeRabbit UI
    Review profile: CHILL
    Plan: Pro

    📥 Commits

    Reviewing files that changed from the base of the PR and between 705e961 and cc2b4fe.

    📒 Files selected for processing (2)
    • src/background/index.mjs (2 hunks)
    • src/content-script/index.jsx (2 hunks)
    🧰 Additional context used
    🪛 Biome (1.9.4)
    src/background/index.mjs

    [error] 380-380: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 459-459: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    src/content-script/index.jsx

    [error] 265-265: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 326-326: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 367-367: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 410-410: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 530-530: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 581-581: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 610-610: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    🪛 GitHub Check: tests
    src/background/index.mjs

    [failure] 460-460:
    'chrome' is not defined

    src/content-script/index.jsx

    [failure] 622-622:
    'prepareForForegroundRequests' is defined but never used

    🪛 GitHub Actions: pr-tests
    src/background/index.mjs

    [error] 460-460: ESLint: 'chrome' is not defined. (no-undef)

    coderabbitai[bot]
    coderabbitai bot reviewed Jun 7, 2025
    View reviewed changes
    Copy link
    Contributor

    @coderabbitai coderabbitai bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Actionable comments posted: 1

    ♻️ Duplicate comments (4)
    src/background/index.mjs (4)

    72-88: ⚠️ Potential issue

    Proxy reconnection still risks infinite recursion & listener leaks

    proxyOnDisconnect invokes setPortProxy unconditionally and immediately, without:

    1. Removing the listeners attached to the previous proxy port.
    2. Capping the retry count / adding back-off delay.

    The exact same concerns were raised in the previous review but remain unaddressed.
    Leaving this as-is can snow-ball into dozens of lingering listeners and a tight retry loop on a flapping tab.
    See the earlier suggestion for a bounded retry counter + exponential back-off.

    103-104: ⚠️ Potential issue

    Sensitive configuration is still dumped to the console

    console.debug('[background] Full config details:', config) will happily print API keys, custom URLs and tokens.
    The prior review already flagged this leakage – please remove or redact before shipping.

    -  console.debug('[background] Full config details:', config)
+  // Avoid leaking secrets – only log non-sensitive fields
+  console.debug('[background] Config loaded. modelName:', config.modelName)

    432-439: 🛠️ Refactor suggestion

    Returning {} on error strips all request headers

    If an exception bubbles up we return an empty object, which drops every header and can break the request completely.
    Safer fall-back: return the original headers so the request proceeds unchanged.

    -        return {} // Return empty object or original headers on error?
+        return { requestHeaders: details.requestHeaders }

    458-459: ⚠️ Potential issue

    chrome global is still unguarded – breaks Firefox & eslint

    Accessing chrome.sidePanel without a typeof chrome !== 'undefined' guard triggers no-undef and fails on Firefox where chrome is absent.

    -      if (chrome && chrome.sidePanel) {
+      if (typeof chrome !== 'undefined' && chrome?.sidePanel) {
    🧰 Tools
    🪛 Biome (1.9.4)

    [error] 458-458: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    📜 Review details

    Configuration used: CodeRabbit UI
    Review profile: CHILL
    Plan: Pro

    📥 Commits

    Reviewing files that changed from the base of the PR and between cc2b4fe and 3191f53.

    📒 Files selected for processing (4)
    • .eslintrc.json (1 hunks)
    • package.json (1 hunks)
    • src/background/index.mjs (2 hunks)
    • src/content-script/index.jsx (1 hunks)
    ✅ Files skipped from review due to trivial changes (2)
    • .eslintrc.json
    • package.json
    🧰 Additional context used
    🪛 Biome (1.9.4)
    src/background/index.mjs

    [error] 380-380: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 458-458: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    src/content-script/index.jsx

    [error] 265-265: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 326-326: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 367-367: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 410-410: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 530-530: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 581-581: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 610-610: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    @PeterDaveHello PeterDaveHello requested a review from Copilot June 7, 2025 18:19

    This comment was marked as outdated.

    Sign in to view

    coderabbitai[bot]
    coderabbitai bot reviewed Jun 7, 2025
    View reviewed changes
    Copy link
    Contributor

    @coderabbitai coderabbitai bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Actionable comments posted: 2

    ♻️ Duplicate comments (2)
    src/background/index.mjs (1)

    545-556: ⚠️ Potential issue

    Chrome global usage still present despite past review feedback.

    This code still uses the unguarded chrome global that was flagged in previous reviews as causing ESLint errors and Firefox compatibility issues.

    The pipeline error persists due to unguarded chrome usage. Apply the fix from the past review:

    -// eslint-disable-next-line no-undef
-if (chrome && chrome.sidePanel) {
-  // eslint-disable-next-line no-undef
-  await chrome.sidePanel.setOptions({
+if (typeof chrome !== 'undefined' && chrome?.sidePanel) {
+  await chrome.sidePanel.setOptions({
     tabId,
     path: 'IndependentPanel.html',
     enabled: true,
   })
-  console.debug(`[background] Side panel options set for tab ${tabId} using chrome.sidePanel`)
+  console.debug(`[background] Side panel options set for tab ${tabId} using chrome.sidePanel`)
 } else {
-  console.debug('[background] chrome.sidePanel API also not available.')
+  console.debug('[background] chrome.sidePanel API also not available.')
 }
    🧰 Tools
    🪛 Biome (1.9.4)

    [error] 546-546: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    src/content-script/index.jsx (1)

    719-758: 🛠️ Refactor suggestion

    Kimi token polling has the same race condition issue.

    The same timeout/polling race condition exists in the Kimi refresh token logic.

    Apply the same cleanup pattern as suggested for the Claude session key polling logic above.

    🧹 Nitpick comments (5)
    src/background/index.mjs (3)

    64-69: Previous listener cleanup logic has improved but could be more robust.

    The cleanup logic for old listeners is better than before, but there's still a potential issue: if port._proxyOnMessage or port._proxyOnDisconnect are undefined, the removeListener calls will silently fail rather than throwing the expected error that the try-catch handles.

    Consider checking for listener existence before removal:

     if (port.proxy) {
     try {
-        if (port._proxyOnMessage) port.proxy.onMessage.removeListener(port._proxyOnMessage);
-        if (port._proxyOnDisconnect) port.proxy.onDisconnect.removeListener(port._proxyOnDisconnect);
+        if (port._proxyOnMessage && port.proxy.onMessage.hasListener?.(port._proxyOnMessage)) {
+            port.proxy.onMessage.removeListener(port._proxyOnMessage);
+        }
+        if (port._proxyOnDisconnect && port.proxy.onDisconnect.hasListener?.(port._proxyOnDisconnect)) {
+            port.proxy.onDisconnect.removeListener(port._proxyOnDisconnect);
+        }
     } catch(e) {

    93-124: Exponential backoff implementation is well-designed but could benefit from configuration.

    The reconnection logic with exponential backoff and retry limits addresses the infinite loop concerns from past reviews effectively. The implementation is solid with proper cleanup and error handling.

    Consider making the retry configuration more visible:

    +const RECONNECT_CONFIG = {
+  MAX_ATTEMPTS: 5,
+  BASE_DELAY_MS: 1000,
+  BACKOFF_MULTIPLIER: 2
+};
+
-const MAX_RECONNECT_ATTEMPTS = 5;
+const MAX_RECONNECT_ATTEMPTS = RECONNECT_CONFIG.MAX_ATTEMPTS;
...
-const delay = Math.pow(2, port._reconnectAttempts - 1) * 1000;
+const delay = Math.pow(RECONNECT_CONFIG.BACKOFF_MULTIPLIER, port._reconnectAttempts - 1) * RECONNECT_CONFIG.BASE_DELAY_MS;

    488-503: Header modification logic is correct but could use optional chaining.

    The header modification logic is functionally correct, but Biome suggests using optional chaining for cleaner code.

    Apply the static analysis suggestion:

    -for (let i = 0; i < headers.length; i++) {
-  if (headers[i].name.toLowerCase() === 'origin') {
+for (let i = 0; i < headers?.length; i++) {
+  if (headers[i]?.name?.toLowerCase() === 'origin') {
     headers[i].value = 'https://www.bing.com'
     modified = true
-  } else if (headers[i].name.toLowerCase() === 'referer') {
+  } else if (headers[i]?.name?.toLowerCase() === 'referer') {
     headers[i].value = 'https://www.bing.com/search?q=Bing+AI&showconv=1&FORM=hpcodx'
     modified = true
   }
 }
    src/content-script/index.jsx (2)

    264-276: Selection element containment check could use optional chaining.

    The logic is correct but could benefit from the optional chaining suggested by static analysis tools.

    -if (toolbarContainer && toolbarContainer.contains(e.target)) {
+if (toolbarContainer?.contains(e.target)) {
   console.debug('[content] Mouseup inside toolbar, ignoring.')
   return
 }
 const selectionElement =
   window.getSelection()?.rangeCount > 0 &&
   window.getSelection()?.getRangeAt(0).endContainer.parentElement
-if (toolbarContainer && selectionElement && toolbarContainer.contains(selectionElement)) {
+if (toolbarContainer?.contains(selectionElement)) {
   console.debug('[content] Mouseup selection is inside toolbar, ignoring.')
   return
 }
    🧰 Tools
    🪛 Biome (1.9.4)

    [error] 265-265: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    810-822: Storage change listener could be more specific.

    The storage change listener correctly handles config changes but could be more efficient by checking specific config keys that affect ChatGPT tab state.

    -if (areaName === 'local' && (changes.userConfig || changes.config)) {
+if (areaName === 'local' && 
+    (changes.userConfig?.newValue?.modelName !== changes.userConfig?.oldValue?.modelName ||
+     changes.userConfig?.newValue?.apiMode !== changes.userConfig?.oldValue?.apiMode ||
+     changes.config)) {
   console.log(
     '[content] User config changed in storage, re-evaluating ChatGPT tab state.',
   )
   await manageChatGptTabState()
 }
    📜 Review details

    Configuration used: CodeRabbit UI
    Review profile: CHILL
    Plan: Pro

    📥 Commits

    Reviewing files that changed from the base of the PR and between 3191f53 and d174940.

    📒 Files selected for processing (2)
    • src/background/index.mjs (2 hunks)
    • src/content-script/index.jsx (1 hunks)
    🧰 Additional context used
    🪛 Biome (1.9.4)
    src/background/index.mjs

    [error] 169-169: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 452-452: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 546-546: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    src/content-script/index.jsx

    [error] 265-265: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 326-326: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 367-367: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 410-410: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 530-530: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 581-581: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 610-610: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    🔇 Additional comments (6)
    src/background/index.mjs (2)

    450-461: Request body handling uses optional chaining correctly.

    The requestBody handling logic properly checks for existence before accessing properties, addressing potential null pointer issues.

    🧰 Tools
    🪛 Biome (1.9.4)

    [error] 452-452: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    566-598: Try-catch blocks for initialization provide good error isolation.

    The wrapping of initialization functions (registerPortListener, registerCommands, refreshMenu) in separate try-catch blocks is a good practice for preventing one failure from affecting others.

    src/content-script/index.jsx (4)

    44-78: Retry logic for element detection is well-implemented.

    The retry mechanism with URL change detection prevents infinite loops and provides good user feedback through logging. The logic properly handles the case where elements aren't found after all retries.

    862-872: Textarea value vs textContent issue has been properly fixed.

    The code now correctly uses input.value instead of input.textContent for the textarea element, which addresses the focus hack issue identified in past reviews.

    896-947: Port listener registration logic is well-designed.

    The global flag to prevent duplicate port listener registration is a good pattern, and the error handling around the registration is comprehensive.

    838-892: ChatGPT tab state management function is well-structured.

    The manageChatGptTabState function properly handles the ChatGPT-specific logic with appropriate guards and error handling. The textarea manipulation using value instead of textContent is correctly implemented.

    @PeterDaveHello PeterDaveHello requested a review from Copilot June 7, 2025 19:08

    This comment was marked as outdated.

    Sign in to view

    coderabbitai[bot]
    coderabbitai bot reviewed Jun 7, 2025
    View reviewed changes
    Copy link
    Contributor

    @coderabbitai coderabbitai bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Actionable comments posted: 0

    🧹 Nitpick comments (7)
    src/background/index.mjs (3)

    497-497: Apply optional chaining as suggested by static analysis.

    The code can be simplified using optional chaining to safely access nested properties.

    -          if (details.requestBody && details.requestBody.formData) {
+          if (details.requestBody?.formData) {
    🧰 Tools
    🪛 Biome (1.9.4)

    [error] 497-497: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    216-223: Consider reducing log verbosity for production environments.

    The extensive debug logging, while helpful for development, may impact performance and expose sensitive information in production environments. Consider implementing log level controls or environment-based logging.

    +  const isDebugMode = process.env.NODE_ENV === 'development' || userConfig?.debugMode;
+  
-  console.debug('[background] Full session details (redacted):', redactSensitiveFields(session))
-  console.debug('[background] Full config details (redacted):', redactSensitiveFields(config))
-  if (session.apiMode) {
-    console.debug('[background] Session apiMode details (redacted):', redactSensitiveFields(session.apiMode))
-  }
+  if (isDebugMode) {
+    console.debug('[background] Full session details (redacted):', redactSensitiveFields(session))
+    console.debug('[background] Full config details (redacted):', redactSensitiveFields(config))
+    if (session.apiMode) {
+      console.debug('[background] Session apiMode details (redacted):', redactSensitiveFields(session.apiMode))
+    }
+  }

    54-58: Document reconnection configuration constants.

    The reconnection constants lack documentation about their purpose and impact on user experience.

    +/**
+ * Configuration for exponential backoff reconnection strategy
+ * @constant {Object} RECONNECT_CONFIG
+ * @property {number} MAX_ATTEMPTS - Maximum reconnection attempts before giving up
+ * @property {number} BASE_DELAY_MS - Initial delay between reconnection attempts 
+ * @property {number} BACKOFF_MULTIPLIER - Multiplier for exponential backoff calculation
+ */
 const RECONNECT_CONFIG = {
   MAX_ATTEMPTS: 5,
-  BASE_DELAY_MS: 1000, // Base delay in milliseconds
-  BACKOFF_MULTIPLIER: 2, // Multiplier for exponential backoff
+  BASE_DELAY_MS: 1000,
+  BACKOFF_MULTIPLIER: 2,
 };
    src/content-script/index.jsx (4)

    529-529: Apply optional chaining as suggested by static analysis.

    The nested property access can be simplified using optional chaining.

    -        if (siteAdapterAction && siteAdapterAction.init) {
+        if (siteAdapterAction?.init) {
    🧰 Tools
    🪛 Biome (1.9.4)

    [error] 529-529: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    580-580: Apply optional chaining as suggested by static analysis.

    The property access can be simplified using optional chaining.

    -        if (preElement && preElement.textContent) {
+        if (preElement?.textContent) {
    🧰 Tools
    🪛 Biome (1.9.4)

    [error] 580-580: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    609-609: Apply optional chaining as suggested by static analysis.

    The nested property access can be simplified using optional chaining.

    -    if (data && data.accessToken) {
+    if (data?.accessToken) {
    🧰 Tools
    🪛 Biome (1.9.4)

    [error] 609-609: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    38-41: Consider reducing debug log verbosity in production.

    Similar to the background script, the extensive debug logging may impact performance in production environments. Consider implementing environment-based logging controls.

    +  const isDebugMode = process.env.NODE_ENV === 'development';
+  
-  console.debug('[content] mountComponent called with siteConfig:', siteConfig)
+  if (isDebugMode) {
+    console.debug('[content] mountComponent called with siteConfig:', siteConfig)
+  }
    📜 Review details

    Configuration used: CodeRabbit UI
    Review profile: CHILL
    Plan: Pro

    📥 Commits

    Reviewing files that changed from the base of the PR and between d174940 and a986d2e.

    📒 Files selected for processing (2)
    • src/background/index.mjs (2 hunks)
    • src/content-script/index.jsx (1 hunks)
    🧰 Additional context used
    🪛 Biome (1.9.4)
    src/background/index.mjs

    [error] 497-497: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    src/content-script/index.jsx

    [error] 529-529: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 580-580: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    [error] 609-609: Change to an optional chain.

    Unsafe fix: Change to an optional chain.

    (lint/complexity/useOptionalChain)

    🔇 Additional comments (5)
    src/background/index.mjs (1)

    108-213: Excellent error handling and reconnection logic implementation.

    The setPortProxy function demonstrates comprehensive error handling with proper listener cleanup, exponential backoff, and detailed logging. The implementation correctly addresses the infinite reconnection loop issues identified in previous reviews.

    src/content-script/index.jsx (4)

    44-79: Robust retry logic with proper URL change detection.

    The retry mechanism for element detection is well-implemented with proper checks for URL changes during retries, preventing unnecessary work when navigation occurs.

    655-697: Excellent promise timeout cleanup implementation.

    The Claude session key polling logic properly addresses the memory leak issues identified in previous reviews with proper cleanup of both timers and settled promise protection.

    861-873: Correct textarea value manipulation.

    The code correctly uses the value property instead of textContent for textarea elements, addressing the issue identified in previous reviews.

    895-946: Well-designed global port listener registration.

    The port listener registration includes proper guards to prevent duplicate registration and comprehensive error handling for the ChatGPT Web model processing.

    @PeterDaveHello PeterDaveHello requested a review from Copilot June 7, 2025 19:31

    This comment was marked as outdated.

    Sign in to view

    @PeterDaveHello PeterDaveHello requested a review from Copilot June 8, 2025 06:19

    This comment was marked as outdated.

    Sign in to view

    coderabbitai[bot]
    coderabbitai bot reviewed Jun 8, 2025
    View reviewed changes
    Copy link
    Contributor

    @coderabbitai coderabbitai bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Actionable comments posted: 0

    ♻️ Duplicate comments (1)
    src/background/index.mjs (1)

    658-673: Chrome global usage still needs proper guarding despite ESLint disable.

    While the ESLint disable comment addresses the linting error, the code still has the underlying compatibility issue with Firefox. The chrome global should be properly guarded.

    The existing review comment about this issue is still valid. Replace the ESLint disable approach with proper runtime checking:

    -      if (!sidePanelSet) {
-        // eslint-disable-next-line no-undef
-        if (typeof chrome !== 'undefined' && chrome.sidePanel && typeof chrome.sidePanel.setOptions === 'function') {
+      if (!sidePanelSet && typeof chrome !== 'undefined' && chrome?.sidePanel?.setOptions) {
    🧹 Nitpick comments (3)
    src/background/index.mjs (1)

    76-133: Review the redactSensitiveFields function for potential edge cases and optimization.

    The function has good protection against circular references and deep recursion, but there are a few potential issues:

    1. The seen WeakSet is passed by reference through recursive calls, which could cause issues if the same object appears in different branches of the recursion tree.
    2. String value checking in arrays (lines 96-104) iterates through all sensitive keywords for every string, which could be inefficient for large arrays.

    Consider these optimizations:

    function redactSensitiveFields(obj, recursionDepth = 0, maxDepth = 5, seen = new WeakSet()) {
  if (recursionDepth > maxDepth) {
    return 'REDACTED_TOO_DEEP';
  }
  if (obj === null || typeof obj !== 'object') {
    return obj;
  }

  if (seen.has(obj)) {
    return 'REDACTED_CIRCULAR_REFERENCE';
  }
  seen.add(obj);

+  // Pre-compile sensitive keyword regex for better performance
+  const sensitiveRegex = new RegExp(SENSITIVE_KEYWORDS.join('|'), 'i');
+  
+  const checkSensitive = (str) => sensitiveRegex.test(str.toLowerCase());

  if (Array.isArray(obj)) {
    const redactedArray = [];
    for (let i = 0; i < obj.length; i++) {
      const item = obj[i];
      if (item !== null && typeof item === 'object') {
        redactedArray[i] = redactSensitiveFields(item, recursionDepth + 1, maxDepth, seen);
      } else if (typeof item === 'string') {
-        let isItemSensitive = false;
-        const lowerItem = item.toLowerCase();
-        for (const keyword of SENSITIVE_KEYWORDS) {
-          if (lowerItem.includes(keyword)) {
-            isItemSensitive = true;
-            break;
-          }
-        }
-        redactedArray[i] = isItemSensitive ? 'REDACTED' : item;
+        redactedArray[i] = checkSensitive(item) ? 'REDACTED' : item;
      } else {
        redactedArray[i] = item;
      }
    }
+    seen.delete(obj); // Clean up after processing
    return redactedArray;
  } else {
    const redactedObj = {};
    for (const key in obj) {
      if (Object.prototype.hasOwnProperty.call(obj, key)) {
        const lowerKey = key.toLowerCase();
-        let isKeySensitive = false;
-        for (const keyword of SENSITIVE_KEYWORDS) {
-          if (lowerKey.includes(keyword)) {
-            isKeySensitive = true;
-            break;
-          }
-        }
-        if (isKeySensitive) {
+        if (checkSensitive(key)) {
          redactedObj[key] = 'REDACTED';
        } else if (obj[key] !== null && typeof obj[key] === 'object') {
          redactedObj[key] = redactSensitiveFields(obj[key], recursionDepth + 1, maxDepth, seen);
        } else {
          redactedObj[key] = obj[key];
        }
      }
    }
+    seen.delete(obj); // Clean up after processing
    return redactedObj;
  }
}
    src/content-script/index.jsx (2)

    37-79: Retry mechanism in mountComponent could be optimized.

    The retry loop is well-implemented but could benefit from early termination on certain conditions and better performance:

    async function mountComponent(siteConfig) {
  console.debug('[content] mountComponent called with siteConfig:', siteConfig)
  try {
    const userConfig = await getUserConfig()
    console.debug('[content] User config in mountComponent:', userConfig)

    if (!userConfig.alwaysFloatingSidebar) {
      const retry = 10
      let oldUrl = location.href
      let elementFound = false
+      const selectors = [
+        ...(siteConfig ? [siteConfig.sidebarContainerQuery, siteConfig.appendContainerQuery, siteConfig.resultsContainerQuery].filter(Boolean) : []),
+        userConfig.prependQuery,
+        userConfig.appendQuery
+      ].filter(Boolean);
+      
      for (let i = 1; i <= retry; i++) {
        console.debug(`[content] mountComponent retry ${i}/${retry} for element detection.`)
        if (location.href !== oldUrl) {
          console.log('[content] URL changed during retry, stopping mountComponent.')
          return
        }
-        const e =
-          (siteConfig &&
-            (getPossibleElementByQuerySelector(siteConfig.sidebarContainerQuery) ||
-              getPossibleElementByQuerySelector(siteConfig.appendContainerQuery) ||
-              getPossibleElementByQuerySelector(siteConfig.resultsContainerQuery))) ||
-          getPossibleElementByQuerySelector([userConfig.prependQuery]) ||
-          getPossibleElementByQuerySelector([userConfig.appendQuery])
+        const e = getPossibleElementByQuerySelector(selectors);
        if (e) {
          console.log('[content] Element found for mounting component:', e)
          elementFound = true
          break
        } else {
          console.debug(`[content] Element not found on retry ${i}.`)
          if (i === retry) {
            console.warn('[content] Element not found after all retries for mountComponent.')
            return
          }
-          await new Promise((r) => setTimeout(r, 500))
+          // Progressive delay: 500ms, 750ms, 1000ms, etc.
+          await new Promise((r) => setTimeout(r, 500 + (i - 1) * 250))
        }
      }

    721-770: Duplicate polling logic for Kimi token - consider extracting to utility function.

    The Kimi refresh token polling logic is nearly identical to the Claude session key polling, leading to code duplication.

    Consider extracting the polling logic into a reusable utility function:

    async function pollForCondition(conditionFn, options = {}) {
  const {
    interval = 500,
    timeout = 30000,
    errorHandler = null,
    onSuccess = null
  } = options;
  
  let promiseSettled = false;
  let timerId = null;
  let timeoutId = null;
  
  const cleanup = () => {
    if (timerId) clearInterval(timerId);
    if (timeoutId) clearTimeout(timeoutId);
  };

  return new Promise((resolve, reject) => {
    timerId = setInterval(async () => {
      if (promiseSettled) {
        cleanup();
        return;
      }
      
      try {
        const result = await conditionFn();
        if (result) {
          if (!promiseSettled) {
            promiseSettled = true;
            cleanup();
            if (onSuccess) await onSuccess(result);
            resolve(result);
          }
        }
      } catch (err) {
        if (errorHandler && await errorHandler(err) && !promiseSettled) {
          promiseSettled = true;
          cleanup();
          reject(err);
        }
      }
    }, interval);

    timeoutId = setTimeout(() => {
      if (!promiseSettled) {
        promiseSettled = true;
        cleanup();
        reject(new Error('Polling timeout exceeded'));
      }
    }, timeout);
  }).catch((err) => {
    cleanup();
    throw err;
  });
}

    Then use it for both Claude and Kimi polling.

    📜 Review details

    Configuration used: CodeRabbit UI
    Review profile: CHILL
    Plan: Pro

    📥 Commits

    Reviewing files that changed from the base of the PR and between 0b5e7a4 and 3fbed97.

    📒 Files selected for processing (3)
    • .eslintrc.json (1 hunks)
    • src/background/index.mjs (2 hunks)
    • src/content-script/index.jsx (1 hunks)
    🚧 Files skipped from review as they are similar to previous changes (1)
    • .eslintrc.json
    🔇 Additional comments (5)
    src/background/index.mjs (2)

    135-259: LGTM! The setPortProxy function addresses previous infinite loop concerns.

    The extensive rewrite successfully addresses the past review comments about infinite reconnect loops and listener leaks by:

    1. Implementing proper retry limits with exponential backoff
    2. Cleaning up old listeners before adding new ones
    3. Using named listener functions stored on the port object for easier removal
    4. Comprehensive error handling with try-catch blocks around cleanup operations

    The reconnection logic is now robust and prevents resource leaks.

    271-391: Enhanced executeApi function improves observability and error handling.

    The additions provide excellent debugging capabilities with redacted logging and comprehensive model support. The error handling ensures that failures are properly communicated back to the caller.

    src/content-script/index.jsx (3)

    868-887: Textarea manipulation logic addresses previous textContent issue.

    The fix correctly uses the value property instead of textContent for textarea elements, which resolves the previous issue where the focus hack wasn't working properly.

    903-954: Global port listener registration is well-structured.

    The approach of moving the port listener registration outside the run function and using a global flag to prevent duplicate registration is a good design pattern. The comprehensive error handling ensures robustness.

    651-698: 🛠️ Refactor suggestion

    Potential cleanup issue in Claude session key polling.

    The promise polling logic has improved cleanup but there's still a potential race condition where the cleanup function might not be called if an exception occurs during the interval callback.

              await new Promise((resolve, reject) => {
            timerId = setInterval(async () => {
              if (promiseSettled) {
                console.warn('[content] Promise already settled but Claude interval still running. This indicates a potential cleanup issue.');
                cleanup()
                return
              }
              try {
                claudeSession = await getClaudeSessionKey()
                if (claudeSession) {
                  if (!promiseSettled) {
                    promiseSettled = true
                    cleanup()
                    console.log('[content] Claude session key found after waiting.')
                    resolve()
                  }
                }
              } catch (err) {
                console.error('[content] Error polling for Claude session key:', err)
                const errMsg = err.message.toLowerCase();
                if ((errMsg.includes('network') || errMsg.includes('permission')) && !promiseSettled) {
                  promiseSettled = true;
                  cleanup();
                  reject(new Error(`Failed to get Claude session key due to: ${err.message}`));
+                } else if (!promiseSettled) {
+                  // For non-critical errors, continue polling but log the issue
+                  console.debug('[content] Non-critical error in Claude polling, continuing...');
                }
              }
            }, 500)

            timeoutId = setTimeout(() => {
              if (!promiseSettled) {
                promiseSettled = true
                cleanup()
                console.warn('[content] Timed out waiting for Claude session key.')
                reject(new Error('Timed out waiting for Claude session key.'))
              }
            }, 30000)
-          }).catch((err) => {
+          })
+          .catch((err) => {
+            // Ensure cleanup happens even if promise chain fails
+            cleanup();
            console.error('[content] Failed to get Claude session key for jump back notification:', err)
            return
          })

    Likely an incorrect or invalid review comment.

    @PeterDaveHello PeterDaveHello requested a review from Copilot June 8, 2025 08:24

    This comment was marked as outdated.

    Sign in to view

    @PeterDaveHello PeterDaveHello requested a review from Copilot June 8, 2025 09:29

    This comment was marked as outdated.

    Sign in to view

    @PeterDaveHello
    Copy link
    Member Author

    PeterDaveHello commented Jun 8, 2025

    @coderabbitai review

    @coderabbitai
    Copy link
    Contributor

    coderabbitai bot commented Jun 8, 2025

    ✅ Actions performed

    Review triggered.

    Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

    @PeterDaveHello PeterDaveHello requested a review from Copilot June 8, 2025 10:30
    Copilot AI reviewed Jun 8, 2025
    View reviewed changes
    Copy link

    Copilot AI left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Pull Request Overview

    This PR enhances the extension’s stability and robustness by improving error handling, logging, and development tooling.

    • Updated ESLint commands in package.json to use npx, ensuring the local version of ESLint is used.
    • Modified ESLint configuration by adding webextensions support and Node-specific overrides for build and script files.

    Reviewed Changes

    Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

    File Description
    package.json Updated lint commands to use npx for consistent, local dependency usage.
    .eslintrc.json Added the webextensions environment and Node overrides to better target files.
    Comments suppressed due to low confidence (2)

    package.json:8

    • Using 'npx' ensures that the project's local ESLint version is used. Consider updating the development documentation to note that npx is required to run these lint commands.
    "lint": "npx eslint --ext .js,.mjs,.jsx .",

    .eslintrc.json:8

    • [nitpick] Review the Node-specific override configuration to ensure it aligns with the intended setup for build and script files. It might be helpful to document how these overrides interact with the base config.
    "overrides": [

    @PeterDaveHello
    Copy link
    Member Author

    PeterDaveHello commented Jun 8, 2025 •
    edited
    Loading

    @coderabbitai review

    @coderabbitai
    Copy link
    Contributor

    coderabbitai bot commented Jun 8, 2025

    ✅ Actions performed

    Review triggered.

    Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

    @PeterDaveHello PeterDaveHello requested a review from Copilot January 6, 2026 19:25
    @PeterDaveHello
    Copy link
    Member Author

    PeterDaveHello commented Jan 6, 2026

    /gemini review

    gemini-code-assist[bot]
    gemini-code-assist bot reviewed Jan 6, 2026
    View reviewed changes
    Copy link

    @gemini-code-assist gemini-code-assist bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Code Review

    This pull request significantly improves the robustness and reliability of the extension by introducing comprehensive error handling, detailed logging, and more resilient logic for asynchronous operations and inter-script communication. The refactoring of setPortProxy with a reconnection mechanism and the dynamic state management in manageChatGptTabState are particularly noteworthy enhancements. The code is now much more stable and easier to debug.

    I've found one issue with the new reconnection logic where the exponential backoff is not working as intended. I've left comments with suggestions on how to fix it. Otherwise, the changes are excellent.

    src/background/index.mjs
    }
    }

    function setPortProxy(port, proxyTabId) {
    Copy link

    @gemini-code-assist gemini-code-assist bot Jan 6, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    high

    To fix the broken exponential backoff, setPortProxy should accept a flag to distinguish initial connections from retries.

    Suggested change
    function setPortProxy(port, proxyTabId) {
    function setPortProxy(port, proxyTabId, isRetry = false) {

    src/content-script/index.jsx
    Comment on lines 372 to 415
    return
    }

    deleteToolbar()
    setTimeout(async () => {
    try {
    const selection = window
    .getSelection()
    ?.toString()
    .trim()
    .replace(/^-+|-+$/g, '')
    if (selection) {
    console.debug('[content] Text selected via touch:', selection)
    const touch = e.changedTouches[0]
    toolbarContainer = createElementAtPosition(touch.pageX + 20, touch.pageY + 20)
    await createSelectionTools(toolbarContainer, selection)
    } else {
    console.debug('[content] No text selected on touchend.')
    }
    } catch (err) {
    console.error(
    '[content] Error in touchend setTimeout callback for touch selection tools:',
    err,
    )
    }
    }, 0)
    } catch (error) {
    console.error('[content] Error in touchend listener for touch selection tools:', error)
    }
    })
    document.addEventListener('touchstart', (e) => {
    if (toolbarContainer && toolbarContainer.contains(e.target)) return

    document.querySelectorAll('.chatgptbox-toolbar-container').forEach((e) => e.remove())
    document.addEventListener('touchstart', (e) => {
    try {
    if (toolbarContainer?.contains(e.target)) {
    console.debug('[content] Touchstart inside toolbar, ignoring.')
    return
    }
    console.debug('[content] Touchstart outside toolbar, removing existing toolbars.')
    document.querySelectorAll('.chatgptbox-toolbar-container').forEach((el) => el.remove())
    toolbarContainer = null
    } catch (error) {
    console.error('[content] Error in touchstart listener for touch selection tools:', error)
    }
    })
    }

    let menuX, menuY

    async function prepareForRightClickMenu() {
    console.log('[content] Initializing right-click menu handler.')
    document.addEventListener('contextmenu', (e) => {
    menuX = e.clientX
    menuY = e.clientY
    console.debug(`[content] Context menu opened at X: ${menuX}, Y: ${menuY}`)
    })

    Browser.runtime.onMessage.addListener(async (message) => {
    if (message.type === 'CREATE_CHAT') {
    const data = message.data
    let prompt = ''
    if (data.itemId in toolsConfig) {
    prompt = await toolsConfig[data.itemId].genPrompt(data.selectionText)
    } else if (data.itemId in menuConfig) {
    const menuItem = menuConfig[data.itemId]
    if (!menuItem.genPrompt) return
    else prompt = await menuItem.genPrompt()
    if (prompt) prompt = await cropText(`Reply in ${await getPreferredLanguage()}.\n` + prompt)
    console.log('[content] Received CREATE_CHAT message:', message)
    try {
    const data = message.data
    let prompt = ''
    if (data.itemId in toolsConfig) {
    console.debug('[content] Generating prompt from toolsConfig for item:', data.itemId)
    prompt = await toolsConfig[data.itemId].genPrompt(data.selectionText)
    } else if (data.itemId in menuConfig) {
    console.debug('[content] Generating prompt from menuConfig for item:', data.itemId)
    const menuItem = menuConfig[data.itemId]
    if (!menuItem.genPrompt) {
    console.warn('[content] No genPrompt for menu item:', data.itemId)
    return
    }
    prompt = await menuItem.genPrompt()
    if (prompt) {
    const preferredLanguage = await getPreferredLanguage()
    prompt = await cropText(`Reply in ${preferredLanguage}.\n` + prompt)
    }
    } else {
    console.warn('[content] Unknown itemId for CREATE_CHAT:', data.itemId)
    return
    }
    console.debug('[content] Generated prompt:', prompt)

    const position = data.useMenuPosition
    ? { x: menuX, y: menuY }
    : { x: window.innerWidth / 2 - 300, y: window.innerHeight / 2 - 200 }
    console.debug('[content] Toolbar position for CREATE_CHAT:', position)
    const container = createElementAtPosition(position.x, position.y)
    container.className = 'chatgptbox-toolbar-container-not-queryable'
    const userConfig = await getUserConfig()
    render(
    <FloatingToolbar
    session={initSession({
    modelName: userConfig.modelName,
    apiMode: userConfig.apiMode,
    extraCustomModelName: userConfig.customModelName,
    })}
    selection={data.selectionText}
    container={container}
    triggered={true}
    closeable={true}
    prompt={prompt}
    />,
    container,
    )
    console.log('[content] CREATE_CHAT toolbar rendered.')
    } catch (error) {
    console.error('[content] Error processing CREATE_CHAT message:', error, message)
    }

    const position = data.useMenuPosition
    ? { x: menuX, y: menuY }
    : { x: window.innerWidth / 2 - 300, y: window.innerHeight / 2 - 200 }
    const container = createElementAtPosition(position.x, position.y)
    container.className = 'chatgptbox-toolbar-container-not-queryable'
    const userConfig = await getUserConfig()
    render(
    <FloatingToolbar
    session={initSession({
    modelName: userConfig.modelName,
    apiMode: userConfig.apiMode,
    extraCustomModelName: userConfig.customModelName,
    })}
    selection={data.selectionText}
    container={container}
    triggered={true}
    closeable={true}
    prompt={prompt}
    />,
    container,
    )
    }
    })
    }

    async function prepareForStaticCard() {
    const userConfig = await getUserConfig()
    let siteRegex
    if (userConfig.useSiteRegexOnly) siteRegex = userConfig.siteRegex
    else
    siteRegex = new RegExp(
    (userConfig.siteRegex && userConfig.siteRegex + '|') + Object.keys(siteConfig).join('|'),
    )

    const matches = location.hostname.match(siteRegex)
    if (matches) {
    const siteName = matches[0]
    console.log('[content] Initializing static card.')
    try {
    const userConfig = await getUserConfig()
    let siteRegexPattern
    if (userConfig.useSiteRegexOnly) {
    siteRegexPattern = userConfig.siteRegex
    } else {
    siteRegexPattern =
    (userConfig.siteRegex ? userConfig.siteRegex + '|' : '') +
    Object.keys(siteConfig)
    .filter((k) => k)
    .join('|')
    }

    if (
    userConfig.siteAdapters.includes(siteName) &&
    !userConfig.activeSiteAdapters.includes(siteName)
    )
    if (!siteRegexPattern) {
    console.debug('[content] No site regex pattern defined for static card.')
    return
    }
    const siteRegex = new RegExp(siteRegexPattern)
    console.debug('[content] Static card site regex:', siteRegex)

    const matches = location.hostname.match(siteRegex)
    if (matches) {
    const siteName = matches[0]
    console.log(`[content] Static card matched site: ${siteName}`)

    if (
    userConfig.siteAdapters.includes(siteName) &&
    !userConfig.activeSiteAdapters.includes(siteName)
    ) {
    console.log(
    `[content] Site adapter for ${siteName} is installed but not active. Skipping static card.`,
    )
    return
    }

    let initSuccess = true
    if (siteName in siteConfig) {
    const siteAction = siteConfig[siteName].action
    if (siteAction && siteAction.init) {
    initSuccess = await siteAction.init(location.hostname, userConfig, getInput, mountComponent)
    let initSuccess = true
    if (siteName in siteConfig) {
    const siteAdapterAction = siteConfig[siteName].action
    if (siteAdapterAction?.init) {
    console.debug(`[content] Initializing site adapter action for ${siteName}.`)
    initSuccess = await siteAdapterAction.init(
    location.hostname,
    userConfig,
    getInput,
    mountComponent,
    )
    console.debug(`[content] Site adapter init success for ${siteName}: ${initSuccess}`)
    }
    }
    }

    if (initSuccess) mountComponent(siteConfig[siteName])
    if (initSuccess) {
    console.log(`[content] Mounting static card for site: ${siteName}`)
    await mountComponent(siteConfig[siteName])
    } else {
    console.warn(`[content] Static card init failed for site: ${siteName}`)
    }
    } else {
    console.debug('[content] No static card match for current site:', location.hostname)
    }
    } catch (error) {
    console.error('[content] Error in prepareForStaticCard:', error)
    }
    }

    async function overwriteAccessToken() {
    if (location.hostname !== 'chatgpt.com') {
    console.debug('[content] overwriteAccessToken called for hostname:', location.hostname)
    try {
    if (location.hostname === 'kimi.moonshot.cn') {
    setUserConfig({
    kimiMoonShotRefreshToken: window.localStorage.refresh_token,
    })
    console.log('[content] On kimi.moonshot.cn, attempting to save refresh token.')
    const refreshToken = window.localStorage.refresh_token
    if (refreshToken) {
    await setUserConfig({ kimiMoonShotRefreshToken: refreshToken })
    console.log('[content] Kimi Moonshot refresh token saved.')
    } else {
    console.warn('[content] Kimi Moonshot refresh token not found in localStorage.')
    }
    return
    }
    return
    }

    let data
    if (location.pathname === '/api/auth/session') {
    const response = document.querySelector('pre').textContent
    try {
    data = JSON.parse(response)
    } catch (error) {
    console.error('json error', error)
    if (location.hostname !== 'chatgpt.com') {
    console.debug('[content] Not on chatgpt.com, skipping access token overwrite.')
    return
    }
    } else {
    const resp = await fetch('https://chatgpt.com/api/auth/session')
    data = await resp.json().catch(() => ({}))
    }
    if (data && data.accessToken) {
    await setAccessToken(data.accessToken)
    console.log(data.accessToken)
    }
    }

    async function prepareForForegroundRequests() {
    if (location.hostname !== 'chatgpt.com' || location.pathname === '/auth/login') return

    const userConfig = await getUserConfig()

    if (
    !chatgptWebModelKeys.some((model) =>
    getApiModesStringArrayFromConfig(userConfig, true).includes(model),
    )
    )
    return

    if (location.pathname === '/') {
    const input = document.querySelector('#prompt-textarea')
    if (input) {
    input.textContent = ' '
    input.dispatchEvent(new Event('input', { bubbles: true }))
    setTimeout(() => {
    input.textContent = ''
    input.dispatchEvent(new Event('input', { bubbles: true }))
    }, 300)
    console.log('[content] On chatgpt.com, attempting to overwrite access token.')
    let data
    if (location.pathname === '/api/auth/session') {
    console.debug('[content] On /api/auth/session page.')
    const preElement = document.querySelector('pre')
    if (preElement?.textContent) {
    const response = preElement.textContent
    try {
    data = JSON.parse(response)
    console.debug('[content] Parsed access token data from <pre> tag.')
    } catch (error) {
    console.error('[content] Failed to parse JSON from <pre> tag for access token:', error)
    }
    } else {
    console.warn('[content] <pre> tag not found or empty for access token on /api/auth/session.')
    }
    } else {
    console.debug('[content] Not on /api/auth/session page, fetching token from API endpoint.')
    try {
    const resp = await fetch('https://chatgpt.com/api/auth/session')
    if (resp.ok) {
    data = await resp.json()
    console.debug('[content] Fetched access token data from API endpoint.')
    } else {
    console.warn(
    `[content] Failed to fetch access token, status: ${resp.status}`,
    await resp.text(),
    )
    }
    } catch (error) {
    console.error('[content] Error fetching access token from API:', error)
    }
    }
    }

    await Browser.runtime.sendMessage({
    type: 'SET_CHATGPT_TAB',
    data: {},
    })

    registerPortListener(async (session, port) => {
    if (isUsingChatgptWebModel(session)) {
    const accessToken = await getChatGptAccessToken()
    await generateAnswersWithChatgptWebApi(port, session.question, session, accessToken)
    if (data?.accessToken) {
    await setAccessToken(data.accessToken)
    console.log('[content] ChatGPT Access token has been set successfully from page data.')
    } else {
    console.warn('[content] No access token found in page data or fetch response.')
    }
    })
    } catch (error) {
    console.error('[content] Error in overwriteAccessToken:', error)
    }
    }
    Copy link

    @gemini-code-assist gemini-code-assist bot Jan 6, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    high

    The function prepareForForegroundRequests has been significantly refactored and renamed to manageChatGptTabState. However, the old implementation registered a port listener that could be registered multiple times, leading to potential memory leaks and unexpected behavior. It's recommended to ensure the listener is only registered once.

    chatgpt-codex-connector[bot]
    chatgpt-codex-connector bot reviewed Jan 6, 2026
    View reviewed changes
    Copy link

    @chatgpt-codex-connector chatgpt-codex-connector bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    💡 Codex Review

    Here are some automated review suggestions for this pull request.

    Reviewed commit: 5fd1923f68

    ℹ️ About Codex in GitHub

    Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

    • Open a pull request for review
    • Mark a draft as ready
    • Comment "@codex review".

    If Codex has suggestions, it will comment; otherwise it will react with 👍.

    Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

    src/background/index.mjs
    Comment on lines 642 to +759
    urls: ['wss://sydney.bing.com/*', 'https://www.bing.com/*'],
    types: ['xmlhttprequest', 'websocket'],
    },
    ['requestHeaders'],
    ['requestHeaders', ...(Browser.runtime.getManifest().manifest_version < 3 ? ['blocking'] : [])],
    Copy link

    @chatgpt-codex-connector chatgpt-codex-connector bot Jan 6, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    P1 Badge Add blocking flag for Bing header rewrite on MV3

    The new onBeforeSendHeaders listener still omits the "blocking" option when the manifest version is 3 because of the < 3 guard. The extension’s manifest is MV3 (src/manifest.json), so the listener now runs without the blocking flag and the requestHeaders you return are ignored—Origin/Referer spoofing for Bing never takes effect. WebRequest docs require blocking (plus webRequestBlocking permission) to modify headers; without it Bing web requests will continue to be sent with the original headers.

    Useful? React with 👍 / 👎.

    Copilot AI reviewed Jan 6, 2026
    View reviewed changes
    Copy link

    Copilot AI left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Pull request overview

    Copilot reviewed 4 out of 4 changed files in this pull request and generated 20 comments.

    💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

    src/content-script/index.jsx
    }, 30000)
    }).catch((err) => {
    console.error('[content] Failed to get Claude session key for jump back notification:', err)
    return
    Copy link

    Copilot AI Jan 6, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The .catch() handlers on lines 695 and 767 return early without any value, which means the promise chain will resolve to undefined. This could cause issues if any code is expecting a result from these promises.

    Additionally, the catch blocks log the error and return, but this means the function continues execution and attempts to render the WebJumpBackNotification (line 777-786) even if the session key/token fetch failed. This could lead to unexpected behavior or incomplete initialization.

    Suggested change
    return
    throw err

    Copilot uses AI. Check for mistakes.
    src/content-script/index.jsx
    Comment on lines 46 to 81
    let elementFound = false
    for (let i = 1; i <= retry; i++) {
    console.debug(`[content] mountComponent retry ${i}/${retry} for element detection.`)
    if (location.href !== oldUrl) {
    console.log('[content] URL changed during retry, stopping mountComponent.')
    return
    }
    const e =
    (siteConfig &&
    (getPossibleElementByQuerySelector(siteConfig.sidebarContainerQuery) ||
    getPossibleElementByQuerySelector(siteConfig.appendContainerQuery) ||
    getPossibleElementByQuerySelector(siteConfig.resultsContainerQuery))) ||
    getPossibleElementByQuerySelector([userConfig.prependQuery]) ||
    getPossibleElementByQuerySelector([userConfig.appendQuery])
    if (e) {
    console.log('[content] Element found for mounting component:', e)
    elementFound = true
    break
    } else {
    console.debug(`[content] Element not found on retry ${i}.`)
    if (i === retry) {
    console.warn('[content] Element not found after all retries for mountComponent.')
    return
    }
    await new Promise((r) => setTimeout(r, 500))
    }
    }
    if (!elementFound) {
    console.warn(
    '[content] No suitable element found for non-floating sidebar after retries.',
    )
    return
    }
    Copy link

    Copilot AI Jan 6, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The variable elementFound is set to true when an element is found, but the subsequent check if (!elementFound) after the loop will never trigger the warning because the function returns early when an element is found (line 63, inside the if (e) block). This makes the elementFound variable and the check on lines 73-78 redundant and unreachable code.

    Either remove the unreachable check, or restructure the loop to not return early and instead break when the element is found.

    Copilot uses AI. Check for mistakes.
    src/content-script/index.jsx
    Comment on lines 903 to 1020
    if (!window.__chatGPTBoxPortListenerRegistered) {
    try {
    if (location.hostname === 'chatgpt.com' && location.pathname !== '/auth/login') {
    console.log('[content] Attempting to register port listener for chatgpt.com.')
    registerPortListener(async (session, port) => {
    console.debug(
    `[content] Port listener callback triggered. Session:`,
    session,
    `Port:`,
    port.name,
    )
    try {
    if (isUsingChatgptWebModel(session)) {
    console.log(
    '[content] Session is for ChatGPT Web Model, processing request for question:',
    session.question,
    )
    const accessToken = await getChatGptAccessToken()
    if (!accessToken) {
    console.warn('[content] No ChatGPT access token available for web API call.')
    port.postMessage({ error: 'Missing ChatGPT access token.' })
    return
    }
    await generateAnswersWithChatgptWebApi(port, session.question, session, accessToken)
    console.log('[content] generateAnswersWithChatgptWebApi call completed.')
    } else {
    console.debug(
    '[content] Session is not for ChatGPT Web Model, skipping processing in this listener.',
    )
    }
    } catch (e) {
    console.error('[content] Error in port listener callback:', e, 'Session:', session)
    try {
    port.postMessage({ error: e.message || 'An unexpected error occurred in content script port listener.' })
    } catch (postError) {
    console.error('[content] Error sending error message back via port:', postError)
    }
    }
    })
    console.log('[content] Generic port listener registered successfully for chatgpt.com pages.')
    window.__chatGPTBoxPortListenerRegistered = true
    } else {
    console.debug(
    '[content] Not on chatgpt.com or on login page, skipping port listener registration.',
    )
    }
    } catch (error) {
    console.error('[content] Error registering global port listener:', error)
    }
    } else {
    console.log('[content] Port listener already registered, skipping.')
    }
    Copy link

    Copilot AI Jan 6, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The global flag window.__chatGPTBoxPortListenerRegistered is used to prevent duplicate port listener registration, but there's no mechanism to reset this flag if the listener needs to be re-registered (e.g., after an error or configuration change).

    Additionally, this flag could conflict if the content script is injected multiple times or if there are multiple instances. A more robust approach would be to store this in a module-level variable or use a more sophisticated registration system that can handle re-registration scenarios.

    Copilot uses AI. Check for mistakes.
    src/background/index.mjs
    Comment on lines 317 to 452
    if (port.proxy) {
    console.debug('[background] Posting message to proxy tab:', { session })
    try {
    port.proxy.postMessage({ session })
    } catch (e) {
    console.warn('[background] Error posting message to existing proxy tab in executeApi (ChatGPT Web Model):', e, '. Attempting to reconnect.', { session });
    setPortProxy(port, tabId); // Attempt to re-establish the connection
    if (port.proxy) {
    console.debug('[background] Proxy re-established. Attempting to post message again.');
    try {
    port.proxy.postMessage({ session });
    console.info('[background] Successfully posted session after proxy reconnection.');
    } catch (e2) {
    console.error('[background] Error posting message even after proxy reconnection:', e2, { session });
    try {
    port.postMessage({ error: 'Failed to communicate with ChatGPT tab after reconnection attempt. Try refreshing the page.' });
    } catch (notifyError) {
    console.error('[background] Error sending final communication failure notification back:', notifyError);
    }
    }
    } else {
    console.error('[background] Failed to re-establish proxy connection. Cannot send session.');
    try {
    port.postMessage({ error: 'Could not re-establish connection to ChatGPT tab. Try refreshing the page.' });
    } catch (notifyError) {
    console.error('[background] Error sending re-establishment failure notification back:', notifyError);
    }
    }
    }
    } else {
    console.error(
    '[background] Failed to send message: port.proxy is still not available after initial setPortProxy attempt.',
    );
    try {
    port.postMessage({ error: 'Failed to initialize connection to ChatGPT tab. Try refreshing the page.' });
    } catch (notifyError) {
    console.error('[background] Error sending initial connection failure notification back:', notifyError);
    }
    Copy link

    Copilot AI Jan 6, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The ChatGPT Web Model handling has a complex nested error handling structure that attempts to reconnect the proxy when posting a message fails. However, this logic is problematic:

    1. If the initial port.proxy.postMessage fails (line 320), it calls setPortProxy again (line 323), which will recursively attempt to establish a new connection. This can potentially trigger the existing disconnect handler and create race conditions.

    2. The code checks if (port.proxy) after calling setPortProxy, but setPortProxy may fail silently (wrapped in try-catch at line 136) or partially succeed, leaving port.proxy in an inconsistent state.

    3. Multiple nested try-catch blocks make it difficult to track the actual error state and could mask underlying issues.

    A better approach would be to check if the proxy is valid before attempting to send the message, or to rely on the existing disconnect/reconnect mechanism rather than trying to recover inline.

    Suggested change
    if (port.proxy) {
    console.debug('[background] Posting message to proxy tab:', { session })
    try {
    port.proxy.postMessage({ session })
    } catch (e) {
    console.warn('[background] Error posting message to existing proxy tab in executeApi (ChatGPT Web Model):', e, '. Attempting to reconnect.', { session });
    setPortProxy(port, tabId); // Attempt to re-establish the connection
    if (port.proxy) {
    console.debug('[background] Proxy re-established. Attempting to post message again.');
    try {
    port.proxy.postMessage({ session });
    console.info('[background] Successfully posted session after proxy reconnection.');
    } catch (e2) {
    console.error('[background] Error posting message even after proxy reconnection:', e2, { session });
    try {
    port.postMessage({ error: 'Failed to communicate with ChatGPT tab after reconnection attempt. Try refreshing the page.' });
    } catch (notifyError) {
    console.error('[background] Error sending final communication failure notification back:', notifyError);
    }
    }
    } else {
    console.error('[background] Failed to re-establish proxy connection. Cannot send session.');
    try {
    port.postMessage({ error: 'Could not re-establish connection to ChatGPT tab. Try refreshing the page.' });
    } catch (notifyError) {
    console.error('[background] Error sending re-establishment failure notification back:', notifyError);
    }
    }
    }
    } else {
    console.error(
    '[background] Failed to send message: port.proxy is still not available after initial setPortProxy attempt.',
    );
    try {
    port.postMessage({ error: 'Failed to initialize connection to ChatGPT tab. Try refreshing the page.' });
    } catch (notifyError) {
    console.error('[background] Error sending initial connection failure notification back:', notifyError);
    }
    if (!port.proxy) {
    console.error(
    '[background] Failed to send message: port.proxy is not available after setPortProxy attempt.',
    )
    try {
    port.postMessage({
    error:
    'Failed to initialize connection to ChatGPT tab. Try refreshing the page.',
    })
    } catch (notifyError) {
    console.error(
    '[background] Error sending initial connection failure notification back:',
    notifyError,
    )
    }
    } else {
    console.debug('[background] Posting message to proxy tab:', { session })
    try {
    port.proxy.postMessage({ session })
    } catch (e) {
    console.error(
    '[background] Error posting message to proxy tab in executeApi (ChatGPT Web Model):',
    e,
    { session },
    )
    try {
    port.postMessage({
    error:
    'Failed to communicate with ChatGPT tab. Try refreshing the page.',
    })
    } catch (notifyError) {
    console.error(
    '[background] Error sending communication failure notification back:',
    notifyError,
    )
    }
    }

    Copilot uses AI. Check for mistakes.
    @PeterDaveHello PeterDaveHello requested a review from Copilot January 8, 2026 18:06
    @PeterDaveHello
    Copy link
    Member Author

    PeterDaveHello commented Jan 8, 2026

    @codex review

    chatgpt-codex-connector[bot]
    chatgpt-codex-connector bot reviewed Jan 8, 2026
    View reviewed changes
    Copy link

    @chatgpt-codex-connector chatgpt-codex-connector bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    💡 Codex Review

    Here are some automated review suggestions for this pull request.

    Reviewed commit: 3fef2bbd48

    ℹ️ About Codex in GitHub

    Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

    • Open a pull request for review
    • Mark a draft as ready
    • Comment "@codex review".

    If Codex has suggestions, it will comment; otherwise it will react with 👍.

    Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

    coderabbitai[bot]
    coderabbitai bot reviewed Jan 8, 2026
    View reviewed changes
    Copy link
    Contributor

    @coderabbitai coderabbitai bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Actionable comments posted: 0

    🧹 Nitpick comments (7)
    src/content-script/index.jsx (4)

    43-76: Consider making retry parameters configurable.

    The hardcoded retry count (10) and interval (500ms) work well for most cases, but making these configurable through user settings could improve flexibility for different network conditions or site behaviors.

    286-329: Nested error handling adds complexity.

    The triple-nested try-catch pattern (outer handler → setTimeout → inner logic) is defensive but may obscure the actual source of errors. Consider flattening or using a single error boundary with more specific error context.

    898-911: Textarea manipulation workaround for ChatGPT UI.

    The prompt textarea manipulation (setting to ' ' then clearing after 300ms) appears to be a workaround to trigger focus or enable ChatGPT's UI. While functional, this approach is fragile and may break with ChatGPT UI updates. Consider documenting why this manipulation is necessary.

    736-749: Consider using stable selectors for the Kimi login button.

    Relying on button text matching ('立即登录') is fragile and may break with UI updates, localization changes, or A/B testing. The same file already uses more robust selectors elsewhere (e.g., button[data-testid=login-button] for ChatGPT at line 656). Investigate whether Kimi's login button exposes stable attributes (data-* attributes, IDs, or class names) and update the selector accordingly if available.

    src/background/index.mjs (3)

    102-111: Consider array string value redaction behavior.

    Lines 103-110 redact array string items if they contain sensitive keywords (e.g., ['myApiKey123'] would be redacted). This is conservative (safe for logging) but may redact more than necessary. For most logging use cases, this trade-off is acceptable.

    370-464: Consider extracting ChatGPT Web Model logic into a helper function.

    The ChatGPT Web Model handling (lines 370-464) is significantly more complex than other model branches due to:

    • Tab validation and retrieval
    • Proxy initialization and reconnection
    • Multiple nested error handling layers
    • Fallback to direct API calls

    Extracting this into a dedicated helper function (e.g., executeChatGptWebModel) would improve readability and maintainability while reducing the cognitive load of the main routing function.

    ♻️ Suggested refactoring approach 
    async function executeChatGptWebModel(session, port, config) {
  console.debug('[background] Using ChatGPT Web Model')
  // ... move lines 372-464 here ...
}

// Then in executeApi:
else if (isUsingChatgptWebModel(session)) {
  await executeChatGptWebModel(session, port, config)
}

    768-785: Consider adding error handling to Claude listener for consistency.

    The Claude onBeforeSendHeaders listener modifies headers but lacks the try-catch wrapping and logging present in the other webRequest listeners (lines 687-766). Adding consistent error handling would improve robustness and debugging capability.

    📜 Review details

    Configuration used: defaults

    Review profile: CHILL

    Plan: Pro

    📥 Commits

    Reviewing files that changed from the base of the PR and between 5fd1923 and 3fef2bb.

    📒 Files selected for processing (4)
    • .eslintrc.json
    • package.json
    • src/background/index.mjs
    • src/content-script/index.jsx
    🚧 Files skipped from review as they are similar to previous changes (1)
    • package.json
    🧰 Additional context used
    📓 Path-based instructions (7)
    src/**/*.{js,jsx,mjs,ts,tsx}

    📄 CodeRabbit inference engine (AGENTS.md)

    Use ESLint and enforce React/JSX standards as configured in .eslintrc.json

    Files:

    • src/background/index.mjs
    • src/content-script/index.jsx
    src/**/*.{js,jsx,mjs,css,scss}

    📄 CodeRabbit inference engine (AGENTS.md)

    Use Prettier for consistent code formatting of all JS/JSX/CSS files

    Files:

    • src/background/index.mjs
    • src/content-script/index.jsx
    src/**/index.{jsx,mjs}

    📄 CodeRabbit inference engine (AGENTS.md)

    Entry files should be named index.jsx or index.mjs

    Files:

    • src/background/index.mjs
    • src/content-script/index.jsx
    src/background/index.mjs

    📄 CodeRabbit inference engine (AGENTS.md)

    Background Script handles browser APIs and cross-page communication via src/background/index.mjs

    Files:

    • src/background/index.mjs
    src/**/*.{js,mjs,jsx}

    📄 CodeRabbit inference engine (AGENTS.md)

    Use webextension-polyfill for cross-browser API compatibility

    Files:

    • src/background/index.mjs
    • src/content-script/index.jsx
    src/content-script/index.jsx

    📄 CodeRabbit inference engine (AGENTS.md)

    Content Script must be injected into all web pages and provide main chat functionality via src/content-script/index.jsx

    Files:

    • src/content-script/index.jsx
    src/**/*.jsx

    📄 CodeRabbit inference engine (AGENTS.md)

    Use Preact for React-like components in the extension

    Files:

    • src/content-script/index.jsx
    🧠 Learnings (10)
    📓 Common learnings 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.650Z
Learning: Applies to src/background/index.mjs : Background Script handles browser APIs and cross-page communication via `src/background/index.mjs`

    Learnt from: PeterDaveHello
Repo: ChatGPTBox-dev/chatGPTBox PR: 886
File: build.mjs:167-175
Timestamp: 2025-09-06T17:46:04.304Z
Learning: For the ChatGPTBox browser extension project, license preservation concerns about third-party dependencies may not be applicable according to the maintainer PeterDaveHello, who indicated this is the "wrong scope" for the project.

    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.651Z
Learning: ALWAYS test core extension functionality including popup, content script injection, and keyboard shortcuts after changes
    📚 Learning: 2026-01-06T19:22:56.650Z 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.650Z
Learning: Applies to src/**/*.{js,mjs,jsx} : Use `webextension-polyfill` for cross-browser API compatibility

    Applied to files:

    • .eslintrc.json
    📚 Learning: 2026-01-06T19:22:56.650Z 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.650Z
Learning: Applies to src/**/*.{js,jsx,mjs,ts,tsx} : Use ESLint and enforce React/JSX standards as configured in `.eslintrc.json`

    Applied to files:

    • .eslintrc.json
    📚 Learning: 2026-01-06T19:22:56.650Z 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.650Z
Learning: Applies to src/**/*.{js,jsx,mjs,css,scss} : Use Prettier for consistent code formatting of all JS/JSX/CSS files

    Applied to files:

    • .eslintrc.json
    📚 Learning: 2026-01-06T19:22:56.650Z 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.650Z
Learning: Applies to src/background/index.mjs : Background Script handles browser APIs and cross-page communication via `src/background/index.mjs`

    Applied to files:

    • src/background/index.mjs
    • src/content-script/index.jsx
    📚 Learning: 2026-01-06T19:22:56.650Z 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.650Z
Learning: Applies to src/content-script/index.jsx : Content Script must be injected into all web pages and provide main chat functionality via `src/content-script/index.jsx`

    Applied to files:

    • src/content-script/index.jsx
    📚 Learning: 2026-01-06T19:22:56.650Z 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.650Z
Learning: Applies to src/content-script/selection-tools/**/*.js : Text selection features must be implemented in `src/content-script/selection-tools/`, keeping UI and logic separate and reusing helpers from `src/utils/`

    Applied to files:

    • src/content-script/index.jsx
    📚 Learning: 2026-01-06T19:22:56.650Z 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.650Z
Learning: Applies to src/content-script/menu-tools/**/*.js : Context menu features must be implemented in `src/content-script/menu-tools/`

    Applied to files:

    • src/content-script/index.jsx
    📚 Learning: 2026-01-06T19:22:56.650Z 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.650Z
Learning: Applies to src/content-script/site-adapters/**/*.js : Create custom site-specific integrations in `src/content-script/site-adapters/` and register them in `src/content-script/site-adapters/index.mjs` with minimal selectors and feature detection

    Applied to files:

    • src/content-script/index.jsx
    📚 Learning: 2026-01-06T19:22:56.651Z 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.651Z
Learning: ALWAYS test core extension functionality including popup, content script injection, and keyboard shortcuts after changes

    Applied to files:

    • src/content-script/index.jsx
    🧬 Code graph analysis (2)
    src/background/index.mjs (2)
    src/config/index.mjs (17)
    • key (472-472)
    • config (750-750)
    • defaultConfig (483-658)
    • defaultConfig (483-658)
    • isUsingClaudeWebModel (670-672)
    • isUsingMoonshotWebModel (674-676)
    • isUsingBingWebModel (678-680)
    • isUsingChatGLMApiModel (722-724)
    • isUsingDeepSeekApiModel (710-712)
    • isUsingOllamaApiModel (726-728)
    • isUsingOpenRouterApiModel (714-716)
    • isUsingAimlApiModel (718-720)
    • isUsingAzureOpenAiApiModel (730-732)
    • isUsingGptCompletionApiModel (694-696)
    • isUsingGithubThirdPartyApiModel (734-736)
    • setUserConfig (770-772)
    • getUserConfig (759-764)
    src/utils/model-name-convert.mjs (1)
    • isUsingModelName (130-144)
    src/content-script/index.jsx (4)
    src/config/index.mjs (4)
    • getUserConfig (759-764)
    • config (750-750)
    • setUserConfig (770-772)
    • setAccessToken (774-776)
    src/utils/get-possible-element-by-query-selector.mjs (1)
    • getPossibleElementByQuerySelector (1-15)
    src/utils/create-element-at-position.mjs (1)
    • createElementAtPosition (1-9)
    src/utils/ends-with-question-mark.mjs (1)
    • endsWithQuestionMark (1-8)
    ⏰ 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: CodeQL analysis (javascript-typescript)
    • GitHub Check: Agent
    🔇 Additional comments (33)
    .eslintrc.json (2)

    4-5: LGTM! Appropriate environment configuration for browser extension.

    The addition of the webextensions environment aligns with the project's use of webextension-polyfill and enables ESLint to recognize browser extension-specific globals.

    8-20: LGTM! Correct Node environment overrides for build tooling.

    The overrides properly configure the Node environment for build scripts, workflow scripts, and tooling files, ensuring ESLint correctly validates Node-specific code in these contexts.

    src/content-script/index.jsx (15)

    38-82: Robust retry logic with URL change detection.

    The retry mechanism with URL monitoring prevents stale component mounting. The 10-retry limit with 500ms intervals provides reasonable resilience without excessive delays.

    84-112: Defensive double cleanup pattern.

    The duplicate cleanup logic (lines 84-91 and 105-112) ensures stale containers are removed both before and after getInput calls. While seemingly redundant, this guards against race conditions where new elements might be injected during async operations.

    184-225: LGTM! Proper error handling with safe fallback.

    The function correctly handles both function-type and selector-type queries, returns undefined on errors (safe fallback), and maintains language preference injection logic.

    228-267: LGTM! Robust toolbar management with proper error handling.

    Both deleteToolbar and createSelectionTools have appropriate try-catch blocks and cleanup logic. The separation of concerns (deletion vs. creation) is clear.

    269-333: Comprehensive error handling in selection tools.

    The mouseup handler includes robust error handling at multiple levels: the event handler itself, the setTimeout callback, and inner logic. The improved positioning logic (lines 297-319) intelligently places toolbars relative to input elements when configured.

    374-431: LGTM! Consistent touch event handling.

    The touch handlers mirror the mouse event logic appropriately, providing equivalent functionality for touch devices. The same nested error handling pattern ensures resilience across both input methods.

    435-498: LGTM! Robust right-click menu implementation.

    The handler properly differentiates between toolsConfig and menuConfig, gracefully handles missing genPrompt functions, and provides flexible positioning with the useMenuPosition flag. Error handling is comprehensive.

    500-564: LGTM! Well-structured static card preparation.

    The function properly constructs site regex patterns, validates site adapter activation status, executes site-specific initialization when available, and only mounts components on successful initialization. Error handling and logging are thorough.

    566-631: LGTM! Comprehensive token management for multiple services.

    The function properly handles token retrieval for both Kimi Moonshot (localStorage) and ChatGPT (page data or API), with appropriate early returns, error handling, and detailed logging for debugging token-related issues.

    633-649: LGTM! Clean session key retrieval helper.

    The function provides a focused, single-responsibility utility for retrieving the Claude session key via runtime messaging, with appropriate error handling and a safe null return on failure.

    651-732: Robust polling pattern with proper cleanup.

    The Claude session key polling implements proper cleanup with promiseSettled flag to prevent race conditions, clearInterval/clearTimeout on both success and timeout paths, and 30-second timeout limit. The pattern is well-structured for async credential retrieval.

    734-807: LGTM! Consistent polling pattern for Kimi token.

    The Kimi refresh token polling mirrors the Claude pattern with proper cleanup, timeout handling, and state management. The automated login trigger is a reasonable approach for improving UX, though as noted above, the selector could be more robust.

    828-875: LGTM! Well-orchestrated initialization.

    The run function provides clear, sequential initialization of language settings, message listeners, token management, storage listeners, and UI features. The storage change listener appropriately re-evaluates ChatGPT tab state when configuration changes.

    877-933: LGTM! Dynamic ChatGPT tab state management.

    The function correctly determines whether the current tab should be designated for ChatGPT Web processing based on model configuration, and appropriately registers the tab with the background script. The early returns for non-applicable pages prevent unnecessary processing.

    935-988: LGTM! Guarded port listener registration.

    The registration guard using window.__chatGPTBoxPortListenerRegistered prevents duplicate listeners, and the hostname/pathname checks ensure registration only occurs on relevant pages. Error handling includes fallback messaging when the port is unavailable.

    src/background/index.mjs (16)

    60-64: LGTM! Reasonable reconnection configuration.

    The exponential backoff configuration (max 5 attempts, 1s base delay, 2x multiplier) provides good balance between resilience and avoiding excessive retry attempts. Maximum delay would be 16 seconds (1s → 2s → 4s → 8s → 16s).

    66-80: Comprehensive sensitive keyword list.

    The keyword list covers common sensitive data patterns including API keys, tokens, secrets, passwords, and authentication credentials. The lowercase matching approach (used in the function) ensures case-insensitive detection.

    82-140: Robust redaction with circular reference and depth protection.

    The function properly handles:

    • Circular references via WeakSet tracking
    • Excessive recursion with configurable max depth (5 levels)
    • Both arrays and objects recursively
    • Case-insensitive keyword matching

    The implementation is defensive and production-ready for sensitive data logging.

    142-199: Comprehensive port proxy setup with robust error handling.

    The proxy setup properly:

    • Cleans up previous listeners before establishing new connections
    • Handles bidirectional message forwarding with error recovery
    • Resets reconnection attempts on successful communication
    • Notifies senders when message forwarding fails

    The extensive try-catch blocks ensure resilience even when listener removal fails.

    201-274: Excellent exponential backoff reconnection implementation.

    The disconnect handler implements proper reconnection with:

    • Exponential backoff calculation (line 259-261)
    • Max retry limit enforcement (lines 231-256)
    • Proper cleanup on max retries exceeded
    • Recursive reconnection with controlled attempts
    • Listener cleanup to prevent memory leaks

    The pattern is production-ready and handles edge cases well.

    276-324: LGTM! Thorough main port disconnect cleanup.

    When the main port disconnects (popup/sidebar closes), all listeners and proxy connections are properly cleaned up. Each operation is wrapped in try-catch to ensure cleanup continues even if individual operations fail.

    335-346: LGTM! Redacted logging for debugging.

    The function logs session and config details with sensitive data redacted, providing valuable debugging information while protecting credentials. The logging granularity is appropriate for troubleshooting API routing issues.

    347-536: Comprehensive model routing with extensive error handling.

    The function routes to appropriate API wrappers for all supported models including:

    • Custom models
    • Web models (ChatGPT, Claude, Moonshot, Bing, Gemini)
    • API models (ChatGPT, Claude, Moonshot, ChatGLM, DeepSeek, Ollama, OpenRouter, AIML, Azure, GPT Completion, Github Third Party)

    Top-level error handling ensures failures are reported via port messaging.

    538-673: LGTM! Comprehensive message handling with structured responses.

    The listener handles all message types with:

    • Clear switch statement organization
    • Logging for each message type
    • Structured error responses (especially for FETCH)
    • New GET_COOKIE support
    • Default case for unknown types

    The FETCH handler properly returns structured responses [data, error] for both success and failure cases.

    620-652: Robust FETCH message handling.

    The FETCH handler includes:

    • Bing-specific token retrieval (lines 622-626)
    • Comprehensive response object construction
    • Explicit error property for non-OK responses
    • Proper error return format for exceptions
    • Detailed logging throughout

    The structured [response, null] / [null, error] return pattern is consistent and easy for callers to handle.

    687-727: LGTM! Robust Arkose request capture.

    The listener properly:

    • Captures ChatGPT Arkose public_key requests
    • Handles both formData and raw bytes formats
    • Saves configuration asynchronously with error handling
    • Wraps all logic in try-catch

    The pattern is defensive and production-ready.

    729-766: LGTM! Bing header modification with MV2/MV3 compatibility.

    The listener correctly:

    • Modifies Origin and Referer headers for Bing requests
    • Uses lowercase header name comparison (line 739) for consistency
    • Includes 'blocking' option conditionally based on manifest version (line 765)
    • Wraps logic in try-catch with safe fallback

    The manifest version check ensures compatibility across both Chrome extension architectures.

    787-856: Excellent cross-browser side panel setup with fallback.

    The listener implements robust side panel configuration:

    • Guards against missing URLs and incomplete load status
    • Attempts Browser.sidePanel (WebExtension Polyfill) first
    • Falls back to chrome.sidePanel for native Chrome compatibility
    • Tracks success with sidePanelSet flag to avoid duplicate attempts
    • Comprehensive logging for debugging API availability

    The pattern ensures maximum browser compatibility while providing clear diagnostic output.

    797-801: URL validation guard prevents unnecessary processing.

    The early return for missing URLs or incomplete status prevents side panel configuration from running on partial page loads. This is correct and efficient.

    861-893: LGTM! Robust initialization with individual error isolation.

    Each initialization function (port listener, commands, menu) is wrapped in its own try-catch block, ensuring that a failure in one doesn't prevent the others from initializing. Success logging provides clear confirmation that each component initialized properly.

    862-875: Proper error handling in port listener callback.

    The registered callback wraps executeApi in try-catch and reports errors via port messaging, ensuring clients receive feedback even when API execution fails. The nested error handling is appropriate for this async callback context.

    Copilot AI reviewed Jan 8, 2026
    View reviewed changes
    Copy link

    Copilot AI left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Pull request overview

    Copilot reviewed 4 out of 4 changed files in this pull request and generated 16 comments.

    💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

    src/background/index.mjs
    types: ['xmlhttprequest', 'websocket'],
    },
    ['requestHeaders'],
    ['requestHeaders', ...(Browser.runtime.getManifest().manifest_version < 3 ? ['blocking'] : [])],
    Copy link

    Copilot AI Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The onBeforeSendHeaders listener at line 765 conditionally includes 'blocking' based on manifest version using a ternary operator in a spread array. However, this approach may not work correctly because the extraInfoSpec parameter expects specific string values. In Manifest V3, 'blocking' is not supported for onBeforeSendHeaders - instead you should use 'extraHeaders'. The current implementation may cause the listener to silently fail or not modify headers as expected in Manifest V3 extensions. Verify the manifest version and use the appropriate API for each version.

    Suggested change
    ['requestHeaders', ...(Browser.runtime.getManifest().manifest_version < 3 ? ['blocking'] : [])],
    Browser.runtime.getManifest().manifest_version < 3
    ? ['requestHeaders', 'blocking']
    : ['requestHeaders', 'extraHeaders'],

    Copilot uses AI. Check for mistakes.
    src/content-script/index.jsx
    Comment on lines +104 to +112
    // Ensure cleanup again in case getInput took time and new elements were added
    document.querySelectorAll('.chatgptbox-container,#chatgptbox-container').forEach((e) => {
    try {
    unmountComponentAtNode(e)
    e.remove()
    } catch (err) {
    console.error('[content] Error removing existing chatgptbox container post getInput:', err)
    }
    })
    Copy link

    Copilot AI Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The comment at line 104 states "Ensure cleanup again in case getInput took time and new elements were added", but the cleanup is removing chatgptbox containers that were just created by the same function. If getInput() takes significant time and the user navigates or triggers another mountComponent call, there could be multiple instances of mountComponent running concurrently, and they would interfere with each other by removing containers the other instance created. This is a race condition that could cause components to disappear unexpectedly. Consider adding a cancellation token or generation ID to ensure each mountComponent call only cleans up its own elements.

    Copilot uses AI. Check for mistakes.
    src/content-script/index.jsx
    Comment on lines 227 to 267
    let toolbarContainer
    const deleteToolbar = () => {
    if (toolbarContainer && toolbarContainer.className === 'chatgptbox-toolbar-container')
    toolbarContainer.remove()
    try {
    if (toolbarContainer && toolbarContainer.className === 'chatgptbox-toolbar-container') {
    console.debug('[content] Deleting toolbar:', toolbarContainer)
    toolbarContainer.remove()
    toolbarContainer = null
    }
    } catch (error) {
    console.error('[content] Error in deleteToolbar:', error)
    }
    }

    const createSelectionTools = async (toolbarContainer, selection) => {
    toolbarContainer.className = 'chatgptbox-toolbar-container'
    const userConfig = await getUserConfig()
    render(
    <FloatingToolbar
    session={initSession({
    modelName: userConfig.modelName,
    apiMode: userConfig.apiMode,
    extraCustomModelName: userConfig.customModelName,
    })}
    selection={selection}
    container={toolbarContainer}
    dockable={true}
    />,
    toolbarContainer,
    const createSelectionTools = async (toolbarContainerElement, selection) => {
    console.debug(
    '[content] createSelectionTools called with selection:',
    selection,
    'and container:',
    toolbarContainerElement,
    )
    try {
    toolbarContainerElement.className = 'chatgptbox-toolbar-container'
    const userConfig = await getUserConfig()
    render(
    <FloatingToolbar
    session={initSession({
    modelName: userConfig.modelName,
    apiMode: userConfig.apiMode,
    extraCustomModelName: userConfig.customModelName,
    })}
    selection={selection}
    container={toolbarContainerElement}
    dockable={true}
    />,
    toolbarContainerElement,
    )
    console.log('[content] Selection tools rendered.')
    } catch (error) {
    console.error('[content] Error in createSelectionTools:', error)
    }
    }
    Copy link

    Copilot AI Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Line 233 sets toolbarContainer = null after removing it, but this is inside the deleteToolbar function which operates on the module-level toolbarContainer variable (line 227). However, the createSelectionTools function at line 240 receives toolbarContainerElement as a parameter, and other parts of the code create local toolbar containers. This inconsistent usage of a global variable mixed with function parameters could lead to bugs where the wrong toolbar is deleted or the global state becomes out of sync with actual DOM state. Consider refactoring to use a more consistent pattern, either fully encapsulating state or using a clear singleton pattern.

    Copilot uses AI. Check for mistakes.
    src/background/index.mjs
    Comment on lines +336 to +340
    console.log(
    `[background] executeApi called for model: ${session.modelName}, apiMode: ${session.apiMode}`,
    )
    console.debug('[background] Full session details (redacted):', redactSensitiveFields(session))
    console.debug('[background] Full config details (redacted):', redactSensitiveFields(config))
    if (session.apiMode) {
    console.debug(
    '[background] Session apiMode details (redacted):',
    redactSensitiveFields(session.apiMode),
    )
    }
    Copy link

    Copilot AI Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The excessive logging added throughout the codebase will significantly impact performance and could expose sensitive information in production. While debug logging is valuable during development, this should be implemented with proper log levels and the ability to disable it in production. Consider:

    1. Implementing a proper logging utility with configurable log levels (DEBUG, INFO, WARN, ERROR)
    2. Making debug logs conditional based on a development flag
    3. Removing redundant logs that don't add meaningful diagnostic value

    The sheer volume of console.debug calls will slow down the extension and clutter browser console logs for users.

    Copilot uses AI. Check for mistakes.
    src/content-script/index.jsx
    Comment on lines +125 to +129
    else if (
    userConfig.triggerMode === 'questionMark' &&
    question &&
    endsWithQuestionMark(question.trim())
    )
    Copy link

    Copilot AI Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    This use of variable 'question' always evaluates to true.

    Suggested change
    else if (
    userConfig.triggerMode === 'questionMark' &&
    question &&
    endsWithQuestionMark(question.trim())
    )
    else if (userConfig.triggerMode === 'questionMark' && endsWithQuestionMark(question.trim()))

    Copilot uses AI. Check for mistakes.
    src/content-script/index.jsx
    Comment on lines +772 to +787
    if (!promiseSettled) {
    promiseSettled = true
    cleanup()
    console.log('[content] Kimi refresh token found after waiting.')
    await setUserConfig({ kimiMoonShotRefreshToken: token })
    console.log('[content] Kimi refresh token saved to config.')
    resolve()
    }
    Copy link

    Copilot AI Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    This negation always evaluates to true.

    Suggested change
    if (!promiseSettled) {
    promiseSettled = true
    cleanup()
    console.log('[content] Kimi refresh token found after waiting.')
    await setUserConfig({ kimiMoonShotRefreshToken: token })
    console.log('[content] Kimi refresh token saved to config.')
    resolve()
    }
    promiseSettled = true
    cleanup()
    console.log('[content] Kimi refresh token found after waiting.')
    await setUserConfig({ kimiMoonShotRefreshToken: token })
    console.log('[content] Kimi refresh token saved to config.')
    resolve()

    Copilot uses AI. Check for mistakes.
    @PeterDaveHello
    Copy link
    Member Author

    PeterDaveHello commented Jan 8, 2026

    /gemini review

    @PeterDaveHello
    Copy link
    Member Author

    PeterDaveHello commented Jan 8, 2026

    @codex review

    gemini-code-assist[bot]
    gemini-code-assist bot reviewed Jan 8, 2026
    View reviewed changes
    Copy link

    @gemini-code-assist gemini-code-assist bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Code Review

    This is an excellent and substantial pull request that significantly improves the robustness, error handling, and logging throughout the extension. The introduction of detailed, structured logging with redaction for sensitive data is a great security and debugging enhancement. The refactoring of connection management in setPortProxy with exponential backoff and proper listener cleanup makes the communication between components much more resilient. Similarly, the changes in the content script, like the refactoring of prepareForForegroundRequests to manageChatGptTabState and making it responsive to configuration changes, are great architectural improvements. The extensive use of try...catch blocks makes the entire codebase much more stable. I have one suggestion for a potential future refactoring to improve maintainability.

    src/background/index.mjs
    Comment on lines 335 to 530
    async function executeApi(session, port, config) {
    console.debug('modelName', session.modelName)
    console.debug('apiMode', session.apiMode)
    if (isUsingCustomModel(session)) {
    if (!session.apiMode)
    await generateAnswersWithCustomApi(
    console.log(
    `[background] executeApi called for model: ${session.modelName}, apiMode: ${session.apiMode}`,
    )
    console.debug('[background] Full session details (redacted):', redactSensitiveFields(session))
    console.debug('[background] Full config details (redacted):', redactSensitiveFields(config))
    if (session.apiMode) {
    console.debug(
    '[background] Session apiMode details (redacted):',
    redactSensitiveFields(session.apiMode),
    )
    }
    try {
    if (isUsingCustomModel(session)) {
    console.debug('[background] Using Custom Model API')
    if (!session.apiMode)
    await generateAnswersWithCustomApi(
    port,
    session.question,
    session,
    config.customModelApiUrl.trim() || 'http://localhost:8000/v1/chat/completions',
    config.customApiKey,
    config.customModelName,
    )
    else
    await generateAnswersWithCustomApi(
    port,
    session.question,
    session,
    session.apiMode.customUrl.trim() ||
    config.customModelApiUrl.trim() ||
    'http://localhost:8000/v1/chat/completions',
    session.apiMode.apiKey.trim() || config.customApiKey,
    session.apiMode.customName,
    )
    } else if (isUsingChatgptWebModel(session)) {
    console.debug('[background] Using ChatGPT Web Model')
    let tabId
    if (
    config.chatgptTabId &&
    config.customChatGptWebApiUrl === defaultConfig.customChatGptWebApiUrl
    ) {
    try {
    const tab = await Browser.tabs.get(config.chatgptTabId)
    if (tab) tabId = tab.id
    } catch (e) {
    console.warn(
    `[background] Failed to get ChatGPT tab with ID ${config.chatgptTabId}:`,
    e.message,
    )
    }
    }
    if (tabId) {
    console.debug(`[background] ChatGPT Tab ID ${tabId} found.`)
    if (!port.proxy) {
    console.debug('[background] port.proxy not found, calling setPortProxy.')
    setPortProxy(port, tabId)
    }
    if (port.proxy) {
    console.debug('[background] Posting message to proxy tab:', { session })
    try {
    port.proxy.postMessage({ session })
    } catch (e) {
    console.warn(
    '[background] Error posting message to existing proxy tab in executeApi (ChatGPT Web Model):',
    e,
    '. Attempting to reconnect.',
    { session },
    )
    setPortProxy(port, tabId)
    if (port.proxy) {
    console.debug('[background] Proxy re-established. Attempting to post message again.')
    try {
    port.proxy.postMessage({ session })
    console.info('[background] Successfully posted session after proxy reconnection.')
    } catch (e2) {
    console.error(
    '[background] Error posting message even after proxy reconnection:',
    e2,
    { session },
    )
    try {
    port.postMessage({
    error:
    'Failed to communicate with ChatGPT tab after reconnection attempt. Try refreshing the page.',
    })
    } catch (notifyError) {
    console.error(
    '[background] Error sending final communication failure notification back:',
    notifyError,
    )
    }
    }
    } else {
    console.error(
    '[background] Failed to re-establish proxy connection. Cannot send session.',
    )
    try {
    port.postMessage({
    error:
    'Could not re-establish connection to ChatGPT tab. Try refreshing the page.',
    })
    } catch (notifyError) {
    console.error(
    '[background] Error sending re-establishment failure notification back:',
    notifyError,
    )
    }
    }
    }
    } else {
    console.error(
    '[background] Failed to send message: port.proxy is still not available after initial setPortProxy attempt.',
    )
    try {
    port.postMessage({
    error: 'Failed to initialize connection to ChatGPT tab. Try refreshing the page.',
    })
    } catch (notifyError) {
    console.error(
    '[background] Error sending initial connection failure notification back:',
    notifyError,
    )
    }
    }
    } else {
    console.debug('[background] No valid ChatGPT Tab ID found. Using direct API call.')
    const accessToken = await getChatGptAccessToken()
    await generateAnswersWithChatgptWebApi(port, session.question, session, accessToken)
    }
    } else if (isUsingClaudeWebModel(session)) {
    console.debug('[background] Using Claude Web Model')
    const sessionKey = await getClaudeSessionKey()
    await generateAnswersWithClaudeWebApi(port, session.question, session, sessionKey)
    } else if (isUsingMoonshotWebModel(session)) {
    console.debug('[background] Using Moonshot Web Model')
    await generateAnswersWithMoonshotWebApi(port, session.question, session, config)
    } else if (isUsingBingWebModel(session)) {
    console.debug('[background] Using Bing Web Model')
    const accessToken = await getBingAccessToken()
    if (isUsingModelName('bingFreeSydney', session)) {
    console.debug('[background] Using Bing Free Sydney model')
    await generateAnswersWithBingWebApi(port, session.question, session, accessToken, true)
    } else {
    await generateAnswersWithBingWebApi(port, session.question, session, accessToken)
    }
    } else if (isUsingGeminiWebModel(session)) {
    console.debug('[background] Using Gemini Web Model')
    const cookies = await getBardCookies()
    await generateAnswersWithBardWebApi(port, session.question, session, cookies)
    } else if (isUsingChatgptApiModel(session)) {
    console.debug('[background] Using ChatGPT API Model')
    await generateAnswersWithChatgptApi(port, session.question, session, config.apiKey)
    } else if (isUsingClaudeApiModel(session)) {
    console.debug('[background] Using Claude API Model')
    await generateAnswersWithClaudeApi(port, session.question, session)
    } else if (isUsingMoonshotApiModel(session)) {
    console.debug('[background] Using Moonshot API Model')
    await generateAnswersWithMoonshotCompletionApi(
    port,
    session.question,
    session,
    config.customModelApiUrl.trim() || 'http://localhost:8000/v1/chat/completions',
    config.customApiKey,
    config.customModelName,
    config.moonshotApiKey,
    )
    else
    await generateAnswersWithCustomApi(
    } else if (isUsingChatGLMApiModel(session)) {
    console.debug('[background] Using ChatGLM API Model')
    await generateAnswersWithChatGLMApi(port, session.question, session)
    } else if (isUsingDeepSeekApiModel(session)) {
    console.debug('[background] Using DeepSeek API Model')
    await generateAnswersWithDeepSeekApi(port, session.question, session, config.deepSeekApiKey)
    } else if (isUsingOllamaApiModel(session)) {
    console.debug('[background] Using Ollama API Model')
    await generateAnswersWithOllamaApi(port, session.question, session)
    } else if (isUsingOpenRouterApiModel(session)) {
    console.debug('[background] Using OpenRouter API Model')
    await generateAnswersWithOpenRouterApi(
    port,
    session.question,
    session,
    session.apiMode.customUrl.trim() ||
    config.customModelApiUrl.trim() ||
    'http://localhost:8000/v1/chat/completions',
    session.apiMode.apiKey.trim() || config.customApiKey,
    session.apiMode.customName,
    config.openRouterApiKey,
    )
    } else if (isUsingChatgptWebModel(session)) {
    let tabId
    if (
    config.chatgptTabId &&
    config.customChatGptWebApiUrl === defaultConfig.customChatGptWebApiUrl
    ) {
    const tab = await Browser.tabs.get(config.chatgptTabId).catch(() => {})
    if (tab) tabId = tab.id
    }
    if (tabId) {
    if (!port.proxy) {
    setPortProxy(port, tabId)
    port.proxy.postMessage({ session })
    }
    } else if (isUsingAimlApiModel(session)) {
    console.debug('[background] Using AIML API Model')
    await generateAnswersWithAimlApi(port, session.question, session, config.aimlApiKey)
    } else if (isUsingAzureOpenAiApiModel(session)) {
    console.debug('[background] Using Azure OpenAI API Model')
    await generateAnswersWithAzureOpenaiApi(port, session.question, session)
    } else if (isUsingGptCompletionApiModel(session)) {
    console.debug('[background] Using GPT Completion API Model')
    await generateAnswersWithGptCompletionApi(port, session.question, session, config.apiKey)
    } else if (isUsingGithubThirdPartyApiModel(session)) {
    console.debug('[background] Using Github Third Party API Model')
    await generateAnswersWithWaylaidwandererApi(port, session.question, session)
    } else {
    const accessToken = await getChatGptAccessToken()
    await generateAnswersWithChatgptWebApi(port, session.question, session, accessToken)
    console.warn('[background] Unknown model or session configuration:', session)
    port.postMessage({ error: 'Unknown model configuration' })
    }
    } else if (isUsingClaudeWebModel(session)) {
    const sessionKey = await getClaudeSessionKey()
    await generateAnswersWithClaudeWebApi(port, session.question, session, sessionKey)
    } else if (isUsingMoonshotWebModel(session)) {
    await generateAnswersWithMoonshotWebApi(port, session.question, session, config)
    } else if (isUsingBingWebModel(session)) {
    const accessToken = await getBingAccessToken()
    if (isUsingModelName('bingFreeSydney', session))
    await generateAnswersWithBingWebApi(port, session.question, session, accessToken, true)
    else await generateAnswersWithBingWebApi(port, session.question, session, accessToken)
    } else if (isUsingGeminiWebModel(session)) {
    const cookies = await getBardCookies()
    await generateAnswersWithBardWebApi(port, session.question, session, cookies)
    } else if (isUsingChatgptApiModel(session)) {
    await generateAnswersWithChatgptApi(port, session.question, session, config.apiKey)
    } else if (isUsingClaudeApiModel(session)) {
    await generateAnswersWithClaudeApi(port, session.question, session)
    } else if (isUsingMoonshotApiModel(session)) {
    await generateAnswersWithMoonshotCompletionApi(
    port,
    session.question,
    session,
    config.moonshotApiKey,
    )
    } else if (isUsingChatGLMApiModel(session)) {
    await generateAnswersWithChatGLMApi(port, session.question, session)
    } else if (isUsingDeepSeekApiModel(session)) {
    await generateAnswersWithDeepSeekApi(port, session.question, session, config.deepSeekApiKey)
    } else if (isUsingOllamaApiModel(session)) {
    await generateAnswersWithOllamaApi(port, session.question, session)
    } else if (isUsingOpenRouterApiModel(session)) {
    await generateAnswersWithOpenRouterApi(port, session.question, session, config.openRouterApiKey)
    } else if (isUsingAimlApiModel(session)) {
    await generateAnswersWithAimlApi(port, session.question, session, config.aimlApiKey)
    } else if (isUsingAzureOpenAiApiModel(session)) {
    await generateAnswersWithAzureOpenaiApi(port, session.question, session)
    } else if (isUsingGptCompletionApiModel(session)) {
    await generateAnswersWithGptCompletionApi(port, session.question, session, config.apiKey)
    } else if (isUsingGithubThirdPartyApiModel(session)) {
    await generateAnswersWithWaylaidwandererApi(port, session.question, session)
    } catch (error) {
    console.error(`[background] Error in executeApi for model ${session.modelName}:`, error)
    port.postMessage({ error: error.message || 'An unexpected error occurred in executeApi' })
    }
    }
    Copy link

    @gemini-code-assist gemini-code-assist bot Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    medium

    This function has grown quite large and complex with many if-else if branches for different models. This can make it difficult to maintain and add new models in the future.

    Consider refactoring this using a strategy pattern or a map of handlers. Each model type could have its own handler object/function that encapsulates the logic for checking if it's applicable and executing the API call. This would make the executeApi function much simpler and more extensible.

    For example:

    const modelHandlers = [
  { supports: isUsingCustomModel, execute: handleCustomModel },
  { supports: isUsingChatgptWebModel, execute: handleChatGptWebModel },
  // ... other handlers
];

async function executeApi(session, port, config) {
  // ... logging ...
  try {
    const handler = modelHandlers.find(h => h.supports(session));
    if (handler) {
      await handler.execute(port, session, config);
    } else {
      console.warn('[background] Unknown model or session configuration:', session);
      port.postMessage({ error: 'Unknown model configuration' });
    }
  } catch (error) {
    // ... error handling ...
  }
}

    This is a suggestion for a future refactoring to improve maintainability, as the current PR is already substantial.

    chatgpt-codex-connector[bot]
    chatgpt-codex-connector bot reviewed Jan 8, 2026
    View reviewed changes
    Copy link

    @chatgpt-codex-connector chatgpt-codex-connector bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    💡 Codex Review

    Here are some automated review suggestions for this pull request.

    Reviewed commit: 3fef2bbd48

    ℹ️ About Codex in GitHub

    Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

    • Open a pull request for review
    • Mark a draft as ready
    • Comment "@codex review".

    If Codex has suggestions, it will comment; otherwise it will react with 👍.

    Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

    @PeterDaveHello
    Copy link
    Member Author

    PeterDaveHello commented Jan 8, 2026

    @codex review

    @PeterDaveHello
    Copy link
    Member Author

    PeterDaveHello commented Jan 8, 2026

    /gemini review

    @PeterDaveHello PeterDaveHello requested a review from Copilot January 8, 2026 19:03
    gemini-code-assist[bot]
    gemini-code-assist bot reviewed Jan 8, 2026
    View reviewed changes
    Copy link

    @gemini-code-assist gemini-code-assist bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Code Review

    This pull request introduces a significant and valuable set of changes focused on improving the extension's stability, error handling, and diagnostics. The extensive addition of try...catch blocks, detailed logging with redaction of sensitive data, and robust reconnection logic for port communication in src/background/index.mjs are excellent improvements. Similarly, the refactoring in src/content-script/index.jsx, especially the new manageChatGptTabState logic and improved event handling, greatly enhances the content script's reliability. Overall, this is a high-quality contribution that makes the extension much more resilient. I have one suggestion for consistency.

    src/background/index.mjs
    Comment on lines 723 to 760
    Browser.webRequest.onBeforeSendHeaders.addListener(
    (details) => {
    const headers = details.requestHeaders
    for (let i = 0; i < headers.length; i++) {
    if (headers[i].name === 'Origin') {
    headers[i].value = 'https://www.bing.com'
    } else if (headers[i].name === 'Referer') {
    headers[i].value = 'https://www.bing.com/search?q=Bing+AI&showconv=1&FORM=hpcodx'
    try {
    console.debug('[background] onBeforeSendHeaders triggered for URL:', details.url)
    const headers = details.requestHeaders
    let modified = false
    for (let i = 0; i < headers.length; i++) {
    if (!headers[i]) {
    continue
    }
    const headerNameLower = headers[i].name?.toLowerCase()
    if (headerNameLower === 'origin') {
    headers[i].value = 'https://www.bing.com'
    modified = true
    } else if (headerNameLower === 'referer') {
    headers[i].value = 'https://www.bing.com/search?q=Bing+AI&showconv=1&FORM=hpcodx'
    modified = true
    }
    }
    if (modified) {
    console.debug('[background] Modified headers for Bing:', headers)
    }
    return { requestHeaders: headers }
    } catch (error) {
    console.error(
    '[background] Error in onBeforeSendHeaders listener callback:',
    error,
    details,
    )
    return { requestHeaders: details.requestHeaders }
    }
    return { requestHeaders: headers }
    },
    {
    urls: ['wss://sydney.bing.com/*', 'https://www.bing.com/*'],
    types: ['xmlhttprequest', 'websocket'],
    },
    ['requestHeaders'],
    ['requestHeaders', ...(Browser.runtime.getManifest().manifest_version < 3 ? ['blocking'] : [])],
    )
    Copy link

    @gemini-code-assist gemini-code-assist bot Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    medium

    This is a great improvement to the onBeforeSendHeaders listener, making it more robust with try...catch blocks, safer property access, and correct handling of the blocking flag for different manifest versions.

    For consistency and to improve overall robustness, I'd suggest applying a similar pattern of error handling and safer property access to the other onBeforeSendHeaders listener for claude.ai which follows this one.

    coderabbitai[bot]
    coderabbitai bot reviewed Jan 8, 2026
    View reviewed changes
    Copy link
    Contributor

    @coderabbitai coderabbitai bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Actionable comments posted: 0

    Caution

    Some comments are outside the diff and can’t be posted inline due to platform limitations.

    ⚠️ Outside diff range comments (1)
    src/background/index.mjs (1)

    762-779: Missing 'blocking' option for Claude webRequest listener in Manifest V2.

    The Bing onBeforeSendHeaders listener at line 759 correctly adds 'blocking' for Manifest V2 compatibility, but the Claude listener at line 778 does not. Without 'blocking', header modifications won't take effect in Manifest V2.

    🐛 Proposed fix 
         {
       urls: ['https://claude.ai/*'],
       types: ['xmlhttprequest'],
     },
-    ['requestHeaders'],
+    ['requestHeaders', ...(Browser.runtime.getManifest().manifest_version < 3 ? ['blocking'] : [])],
   )
    🧹 Nitpick comments (8)
    src/background/index.mjs (4)

    253-267: Potential race condition with pending reconnection timeouts.

    If setPortProxy is called again while a previous reconnection timeout is still pending (e.g., due to a manual reconnection attempt or rapid disconnects), multiple timeouts may run concurrently, leading to duplicate connection attempts.

    Consider storing the timeout ID on the port object and clearing it at the start of setPortProxy:

    ♻️ Suggested fix to track and clear pending timeout 
     function setPortProxy(port, proxyTabId) {
   try {
     console.debug(`[background] Attempting to connect to proxy tab: ${proxyTabId}`)
 
+    // Clear any pending reconnection timeout
+    if (port._reconnectTimeoutId) {
+      clearTimeout(port._reconnectTimeoutId)
+      port._reconnectTimeoutId = null
+    }
+
     if (port.proxy) {
       // ... existing cleanup code

    And when setting the timeout:

    -      setTimeout(async () => {
+      port._reconnectTimeoutId = setTimeout(async () => {
         try {
           await Browser.tabs.get(proxyTabId)
         } catch (error) {
           // ...
         }
         // ...
         setPortProxy(port, proxyTabId)
       }, delay)

    381-453: Consider extracting ChatGPT Web Model proxy logic into a helper function.

    The nested error handling for proxy communication (lines 381-453) is thorough but deeply nested. This makes the code harder to follow and maintain. Consider extracting this into a separate helper function like sendToProxyWithRetry(port, tabId, session) to improve readability.

    614-620: Side effect in FETCH handler may be unexpected.

    The FETCH message handler performs a side effect of saving the Bing access token to user config whenever a Bing URL is fetched. This couples the generic FETCH functionality with Bing-specific logic, which may cause confusion or unintended token updates.

    Consider moving this to a separate message type (e.g., FETCH_BING_TOKEN) or documenting this behavior clearly.

    782-789: Simplify error handler pattern.

    The outerTryCatchError function is defined inline but only logs errors. This adds unnecessary indirection. Consider using console.error directly in the catch block for clarity.

    ♻️ Suggested simplification 
       Browser.tabs.onUpdated.addListener(async (tabId, info, tab) => {
-    const outerTryCatchError = (error) => {
-      console.error(
-        '[background] Error in tabs.onUpdated listener callback (outer):',
-        error,
-        tabId,
-        info,
-      )
-    }
     try {
       // ... existing code ...
     } catch (error) {
-      outerTryCatchError(error)
+      console.error(
+        '[background] Error in tabs.onUpdated listener callback:',
+        error,
+        tabId,
+        info,
+      )
     }
   })
    src/content-script/index.jsx (4)

    84-112: Extract duplicate cleanup logic.

    The container cleanup code is duplicated at lines 84-91 and 105-112. Consider extracting it into a helper function to improve maintainability.

    ♻️ Suggested extraction 
    const cleanupExistingContainers = () => {
  document.querySelectorAll('.chatgptbox-container,#chatgptbox-container').forEach((e) => {
    try {
      unmountComponentAtNode(e)
      e.remove()
    } catch (err) {
      console.error('[content] Error removing existing chatgptbox container:', err)
    }
  })
}

    Then replace both occurrences with cleanupExistingContainers().

    515-519: Potential RegExp error from user-provided pattern.

    If userConfig.siteRegex contains invalid regex characters or syntax, creating new RegExp(siteRegexPattern) at line 519 will throw an error. While the outer try-catch will catch it, it would silently disable static cards for all sites.

    Consider validating the regex pattern or handling the specific error case:

    ♻️ Suggested fix 
         if (!siteRegexPattern) {
       console.debug('[content] No site regex pattern defined for static card.')
       return
     }
-    const siteRegex = new RegExp(siteRegexPattern)
+    let siteRegex
+    try {
+      siteRegex = new RegExp(siteRegexPattern)
+    } catch (regexError) {
+      console.error('[content] Invalid site regex pattern:', siteRegexPattern, regexError)
+      return
+    }
     console.debug('[content] Static card site regex:', siteRegex)

    670-815: Consider extracting repeated polling pattern into a helper.

    The polling logic for Claude (lines 670-736) and Kimi (lines 767-810) tokens follows the same pattern: interval polling with timeout, promise settlement tracking, and cleanup. This could be extracted into a reusable helper function to reduce duplication.

    ♻️ Suggested helper function 
    async function pollForCondition({
  checkFn,
  onSuccess,
  intervalMs = 500,
  timeoutMs = 30000,
  description = 'condition',
}) {
  return new Promise((resolve, reject) => {
    let settled = false
    const cleanup = () => {
      settled = true
      clearInterval(timerId)
      clearTimeout(timeoutId)
    }
    const timerId = setInterval(async () => {
      if (settled) return
      try {
        const result = await checkFn()
        if (result) {
          cleanup()
          if (onSuccess) await onSuccess(result)
          resolve(result)
        }
      } catch (err) {
        console.error(`[content] Error polling for ${description}:`, err)
      }
    }, intervalMs)
    const timeoutId = setTimeout(() => {
      if (!settled) {
        cleanup()
        reject(new Error(`Timed out waiting for ${description}.`))
      }
    }, timeoutMs)
  })
}

    839-844: Mixed async patterns.

    Lines 839-844 use .then()/.catch() after await, which is unusual. Consider using pure async/await for consistency:

    ♻️ Suggested fix 
    -    await getPreferredLanguageKey()
-      .then((lang) => {
-        console.log(`[content] Setting language to: ${lang}`)
-        changeLanguage(lang)
-      })
-      .catch((err) => console.error('[content] Error setting preferred language:', err))
+    try {
+      const lang = await getPreferredLanguageKey()
+      console.log(`[content] Setting language to: ${lang}`)
+      changeLanguage(lang)
+    } catch (err) {
+      console.error('[content] Error setting preferred language:', err)
+    }
    📜 Review details

    Configuration used: defaults

    Review profile: CHILL

    Plan: Pro

    📥 Commits

    Reviewing files that changed from the base of the PR and between 3fef2bb and f01b0a7.

    📒 Files selected for processing (2)
    • src/background/index.mjs
    • src/content-script/index.jsx
    🧰 Additional context used
    📓 Path-based instructions (7)
    src/**/*.{js,jsx,mjs,ts,tsx}

    📄 CodeRabbit inference engine (AGENTS.md)

    Use ESLint and enforce React/JSX standards as configured in .eslintrc.json

    Files:

    • src/background/index.mjs
    • src/content-script/index.jsx
    src/**/*.{js,jsx,mjs,css,scss}

    📄 CodeRabbit inference engine (AGENTS.md)

    Use Prettier for consistent code formatting of all JS/JSX/CSS files

    Files:

    • src/background/index.mjs
    • src/content-script/index.jsx
    src/**/index.{jsx,mjs}

    📄 CodeRabbit inference engine (AGENTS.md)

    Entry files should be named index.jsx or index.mjs

    Files:

    • src/background/index.mjs
    • src/content-script/index.jsx
    src/background/index.mjs

    📄 CodeRabbit inference engine (AGENTS.md)

    Background Script handles browser APIs and cross-page communication via src/background/index.mjs

    Files:

    • src/background/index.mjs
    src/**/*.{js,mjs,jsx}

    📄 CodeRabbit inference engine (AGENTS.md)

    Use webextension-polyfill for cross-browser API compatibility

    Files:

    • src/background/index.mjs
    • src/content-script/index.jsx
    src/content-script/index.jsx

    📄 CodeRabbit inference engine (AGENTS.md)

    Content Script must be injected into all web pages and provide main chat functionality via src/content-script/index.jsx

    Files:

    • src/content-script/index.jsx
    src/**/*.jsx

    📄 CodeRabbit inference engine (AGENTS.md)

    Use Preact for React-like components in the extension

    Files:

    • src/content-script/index.jsx
    🧠 Learnings (7)
    📓 Common learnings 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.650Z
Learning: Applies to src/background/index.mjs : Background Script handles browser APIs and cross-page communication via `src/background/index.mjs`
    📚 Learning: 2026-01-06T19:22:56.650Z 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.650Z
Learning: Applies to src/background/index.mjs : Background Script handles browser APIs and cross-page communication via `src/background/index.mjs`

    Applied to files:

    • src/background/index.mjs
    • src/content-script/index.jsx
    📚 Learning: 2026-01-06T19:22:56.650Z 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.650Z
Learning: Applies to src/content-script/selection-tools/**/*.js : Text selection features must be implemented in `src/content-script/selection-tools/`, keeping UI and logic separate and reusing helpers from `src/utils/`

    Applied to files:

    • src/content-script/index.jsx
    📚 Learning: 2026-01-06T19:22:56.650Z 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.650Z
Learning: Applies to src/content-script/index.jsx : Content Script must be injected into all web pages and provide main chat functionality via `src/content-script/index.jsx`

    Applied to files:

    • src/content-script/index.jsx
    📚 Learning: 2026-01-06T19:22:56.650Z 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.650Z
Learning: Applies to src/content-script/menu-tools/**/*.js : Context menu features must be implemented in `src/content-script/menu-tools/`

    Applied to files:

    • src/content-script/index.jsx
    📚 Learning: 2026-01-06T19:22:56.650Z 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.650Z
Learning: Applies to src/content-script/site-adapters/**/*.js : Create custom site-specific integrations in `src/content-script/site-adapters/` and register them in `src/content-script/site-adapters/index.mjs` with minimal selectors and feature detection

    Applied to files:

    • src/content-script/index.jsx
    📚 Learning: 2026-01-06T19:22:56.651Z 
    Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T19:22:56.651Z
Learning: ALWAYS test core extension functionality including popup, content script injection, and keyboard shortcuts after changes

    Applied to files:

    • src/content-script/index.jsx
    🧬 Code graph analysis (1)
    src/background/index.mjs (9)
    src/services/apis/moonshot-web.mjs (1)
    • generateAnswersWithMoonshotWebApi (581-631)
    src/services/apis/claude-web.mjs (1)
    • generateAnswersWithClaudeWebApi (11-61)
    src/utils/model-name-convert.mjs (1)
    • isUsingModelName (130-144)
    src/services/apis/bard-web.mjs (1)
    • generateAnswersWithBardWebApi (10-28)
    src/services/apis/moonshot-api.mjs (1)
    • generateAnswersWithMoonshotCompletionApi (9-12)
    src/services/apis/deepseek-api.mjs (1)
    • generateAnswersWithDeepSeekApi (9-12)
    src/services/apis/openrouter-api.mjs (1)
    • generateAnswersWithOpenRouterApi (9-12)
    src/services/apis/aiml-api.mjs (1)
    • generateAnswersWithAimlApi (9-12)
    src/utils/open-url.mjs (1)
    • openUrl (3-11)
    ⏰ 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: CodeQL analysis (javascript-typescript)
    • GitHub Check: Agent
    🔇 Additional comments (9)
    src/background/index.mjs (3)

    60-75: LGTM!

    The reconnection configuration and sensitive keywords list are well-structured. One observation: the keyword 'session' is quite broad and will redact any field containing "session" (e.g., sessionId, sessionName). If this is intentional for security, it's fine; otherwise, consider a more specific pattern like 'sessionkey' to avoid over-redacting non-sensitive session metadata.

    77-125: LGTM!

    The redactSensitiveFields function is well-implemented with proper safeguards:

    • Circular reference detection using WeakSet
    • Max depth limit to prevent stack overflow
    • Non-mutating approach returning new objects
    • Proper hasOwnProperty check

    855-887: LGTM!

    The registration blocks are properly wrapped in individual try-catch blocks, ensuring that a failure in one registration (port listener, commands, or menu) doesn't prevent the others from being attempted. The success/error logging is helpful for debugging.

    src/content-script/index.jsx (6)

    184-225: LGTM!

    The getInput function handles both function-based and selector-based inputs cleanly, with proper error handling and consistent return values.

    269-431: LGTM!

    The selection tools implementation is well-structured with:

    • Proper toolbar lifecycle management
    • Good handling of edge cases (clicking inside toolbar, selection in input fields)
    • Consistent touch event handling that mirrors mouse events
    • Comprehensive error handling in all event listeners

    433-498: LGTM!

    The right-click menu handler properly captures context menu position and processes CREATE_CHAT messages with appropriate fallback handling for different config sources.

    566-635: LGTM!

    The overwriteAccessToken function properly handles token retrieval for both Kimi and ChatGPT with appropriate early returns, error handling, and logging.

    890-965: LGTM with a note on the promise mutex pattern.

    The promise-based mutex at lines 890-964 effectively prevents concurrent execution of manageChatGptTabState. The textarea manipulation at lines 921-936 is an interesting approach to trigger focus behavior on ChatGPT's page. This is a creative workaround but may be fragile if ChatGPT changes their DOM structure or input handling.

    Consider adding a comment explaining why the textarea manipulation is needed, so future maintainers understand the intent.

    967-1020: LGTM!

    The port listener registration uses a global flag to prevent duplicate registrations, which is appropriate for content scripts that may be re-injected. The error handling is comprehensive, including proper messaging back to the port when access tokens are missing or errors occur.

    chatgpt-codex-connector[bot]
    chatgpt-codex-connector bot reviewed Jan 8, 2026
    View reviewed changes
    Copy link

    @chatgpt-codex-connector chatgpt-codex-connector bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    💡 Codex Review

    Here are some automated review suggestions for this pull request.

    Reviewed commit: f01b0a7da6

    ℹ️ About Codex in GitHub

    Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

    • Open a pull request for review
    • Mark a draft as ready
    • Comment "@codex review".

    If Codex has suggestions, it will comment; otherwise it will react with 👍.

    Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

    src/background/index.mjs
    Comment on lines +383 to +390
    if (!port.proxy) {
    console.debug('[background] port.proxy not found, calling setPortProxy.')
    setPortProxy(port, tabId)
    }
    if (port.proxy) {
    console.debug('[background] Posting message to proxy tab:', { session })
    try {
    port.proxy.postMessage({ session })
    Copy link

    @chatgpt-codex-connector chatgpt-codex-connector bot Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    P2 Badge Prevent duplicate ChatGPT web session dispatch

    Once a proxy is established, setPortProxy already registers _portOnMessage to forward every incoming port message to the proxy tab, so sending port.proxy.postMessage({ session }) again here will duplicate requests after the first successful connection. This means later ChatGPT Web sessions can be processed twice (once by the port forwarder, once by this explicit send), leading to duplicate answers or side effects. Consider only sending here on initial connect (as before) or removing this extra send when a proxy already exists.

    Useful? React with 👍 / 👎.

    Copilot AI reviewed Jan 8, 2026
    View reviewed changes
    Copy link

    Copilot AI left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Pull request overview

    Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.

    💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

    src/content-script/index.jsx
    Comment on lines +924 to +936
    input.value = ' '
    input.dispatchEvent(new Event('input', { bubbles: true }))
    setTimeout(() => {
    if (input && input.value === ' ') {
    input.value = ''
    input.dispatchEvent(new Event('input', { bubbles: true }))
    console.debug('[content] #prompt-textarea manipulation complete.')
    } else if (!input) {
    console.warn(
    '[content] #prompt-textarea no longer available in setTimeout callback.',
    )
    }
    }, 300)
    Copy link

    Copilot AI Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The setTimeout delay for input textarea manipulation is hardcoded to 300ms (line 936), but there's no guarantee this is sufficient time for the DOM to update. On slower devices or under heavy load, this could lead to race conditions where the second part of the manipulation executes before the first is complete.

    Consider using a more reliable approach like MutationObserver or checking the actual state before proceeding with the reset.

    Suggested change
    input.value = ' '
    input.dispatchEvent(new Event('input', { bubbles: true }))
    setTimeout(() => {
    if (input && input.value === ' ') {
    input.value = ''
    input.dispatchEvent(new Event('input', { bubbles: true }))
    console.debug('[content] #prompt-textarea manipulation complete.')
    } else if (!input) {
    console.warn(
    '[content] #prompt-textarea no longer available in setTimeout callback.',
    )
    }
    }, 300)
    const resetPromptTextarea = () => {
    if (!input) {
    console.warn(
    '[content] #prompt-textarea no longer available when attempting reset.',
    )
    return
    }
    if (input.value === ' ') {
    input.value = ''
    input.dispatchEvent(new Event('input', { bubbles: true }))
    console.debug('[content] #prompt-textarea manipulation complete.')
    }
    }
    let observer
    if (typeof MutationObserver !== 'undefined' && input) {
    try {
    observer = new MutationObserver(() => {
    // Once a mutation has occurred and the intermediate state is present,
    // perform the reset and stop observing.
    if (!input || input.value !== ' ') {
    return
    }
    resetPromptTextarea()
    if (observer) {
    observer.disconnect()
    }
    })
    observer.observe(input, {
    attributes: true,
    characterData: true,
    subtree: true,
    })
    } catch (e) {
    console.warn(
    '[content] Failed to attach MutationObserver to #prompt-textarea.',
    e,
    )
    observer = undefined
    }
    }
    input.value = ' '
    input.dispatchEvent(new Event('input', { bubbles: true }))
    // Fallback in case MutationObserver is unavailable or no relevant mutation occurs.
    if (!observer) {
    let attempts = 0
    const maxAttempts = 60 // ~1 second at 60fps
    const tryReset = () => {
    if (!input) {
    console.warn(
    '[content] #prompt-textarea no longer available during fallback reset.',
    )
    return
    }
    if (input.value === ' ') {
    resetPromptTextarea()
    return
    }
    attempts += 1
    if (attempts < maxAttempts) {
    window.requestAnimationFrame(tryReset)
    }
    }
    window.requestAnimationFrame(tryReset)
    }

    Copilot uses AI. Check for mistakes.
    src/background/index.mjs
    Comment on lines +867 to +868
    port.postMessage({ error: e.message || 'An unexpected error occurred in port listener' })
    }
    Copy link

    Copilot AI Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The error handling in the port listener at line 867 is inconsistent with the executeApi function at line 528. Both send error messages via port, but use slightly different error message formats ('An unexpected error occurred in port listener' vs 'An unexpected error occurred in executeApi').

    Consider standardizing error messages and creating a helper function for safely sending error messages through ports to avoid code duplication and ensure consistent error handling.

    Copilot uses AI. Check for mistakes.
    src/background/index.mjs
    Comment on lines +524 to +528
    port.postMessage({ error: 'Unknown model configuration' })
    }
    } else if (isUsingClaudeWebModel(session)) {
    const sessionKey = await getClaudeSessionKey()
    await generateAnswersWithClaudeWebApi(port, session.question, session, sessionKey)
    } else if (isUsingMoonshotWebModel(session)) {
    await generateAnswersWithMoonshotWebApi(port, session.question, session, config)
    } else if (isUsingBingWebModel(session)) {
    const accessToken = await getBingAccessToken()
    if (isUsingModelName('bingFreeSydney', session))
    await generateAnswersWithBingWebApi(port, session.question, session, accessToken, true)
    else await generateAnswersWithBingWebApi(port, session.question, session, accessToken)
    } else if (isUsingGeminiWebModel(session)) {
    const cookies = await getBardCookies()
    await generateAnswersWithBardWebApi(port, session.question, session, cookies)
    } else if (isUsingChatgptApiModel(session)) {
    await generateAnswersWithChatgptApi(port, session.question, session, config.apiKey)
    } else if (isUsingClaudeApiModel(session)) {
    await generateAnswersWithClaudeApi(port, session.question, session)
    } else if (isUsingMoonshotApiModel(session)) {
    await generateAnswersWithMoonshotCompletionApi(
    port,
    session.question,
    session,
    config.moonshotApiKey,
    )
    } else if (isUsingChatGLMApiModel(session)) {
    await generateAnswersWithChatGLMApi(port, session.question, session)
    } else if (isUsingDeepSeekApiModel(session)) {
    await generateAnswersWithDeepSeekApi(port, session.question, session, config.deepSeekApiKey)
    } else if (isUsingOllamaApiModel(session)) {
    await generateAnswersWithOllamaApi(port, session.question, session)
    } else if (isUsingOpenRouterApiModel(session)) {
    await generateAnswersWithOpenRouterApi(port, session.question, session, config.openRouterApiKey)
    } else if (isUsingAimlApiModel(session)) {
    await generateAnswersWithAimlApi(port, session.question, session, config.aimlApiKey)
    } else if (isUsingAzureOpenAiApiModel(session)) {
    await generateAnswersWithAzureOpenaiApi(port, session.question, session)
    } else if (isUsingGptCompletionApiModel(session)) {
    await generateAnswersWithGptCompletionApi(port, session.question, session, config.apiKey)
    } else if (isUsingGithubThirdPartyApiModel(session)) {
    await generateAnswersWithWaylaidwandererApi(port, session.question, session)
    } catch (error) {
    console.error(`[background] Error in executeApi for model ${session.modelName}:`, error)
    port.postMessage({ error: error.message || 'An unexpected error occurred in executeApi' })
    Copy link

    Copilot AI Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Multiple port.postMessage calls throughout the error handling code lack try-catch protection. If the port has been disconnected when these error messages are sent, they will throw exceptions that aren't caught, potentially masking the original error. While some calls have try-catch blocks around them (lines 170-179, 235-240, 411-420, 427-436, 444-452), others don't (lines 524, 528, 867).

    All port.postMessage calls should be wrapped in try-catch blocks for consistency and robustness.

    Copilot uses AI. Check for mistakes.
    src/background/index.mjs
    Comment on lines +759 to 760
    ['requestHeaders', ...(Browser.runtime.getManifest().manifest_version < 3 ? ['blocking'] : [])],
    )
    Copy link

    Copilot AI Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The addition of 'blocking' to the onBeforeSendHeaders listener is conditionally applied based on manifest version, but the syntax uses a spread operator with a ternary that could fail in older JavaScript environments or if Browser.runtime.getManifest() returns unexpected values.

    Additionally, checking manifest version at runtime for every request setup is inefficient. This should be determined once at module initialization and stored in a constant.

    Copilot uses AI. Check for mistakes.
    src/background/index.mjs
    Comment on lines +77 to +125
    function redactSensitiveFields(obj, recursionDepth = 0, maxDepth = 5, seen = new WeakSet()) {
    if (recursionDepth > maxDepth) {
    // Prevent infinite recursion on circular objects or excessively deep structures
    return 'REDACTED_TOO_DEEP'
    }
    if (obj === null || typeof obj !== 'object') {
    return obj
    }
    const portOnMessage = (msg) => {
    port.proxy.postMessage(msg)

    if (seen.has(obj)) {
    return 'REDACTED_CIRCULAR_REFERENCE'
    }
    seen.add(obj)

    if (Array.isArray(obj)) {
    const redactedArray = []
    for (let i = 0; i < obj.length; i++) {
    const item = obj[i]
    if (item !== null && typeof item === 'object') {
    redactedArray[i] = redactSensitiveFields(item, recursionDepth + 1, maxDepth, seen)
    } else {
    redactedArray[i] = item
    }
    }
    return redactedArray
    }
    const proxyOnDisconnect = () => {
    port.proxy = Browser.tabs.connect(proxyTabId)

    const redactedObj = {}
    for (const key in obj) {
    if (Object.prototype.hasOwnProperty.call(obj, key)) {
    const lowerKey = key.toLowerCase()
    let isKeySensitive = false
    for (const keyword of SENSITIVE_KEYWORDS) {
    if (lowerKey.includes(keyword)) {
    isKeySensitive = true
    break
    }
    }
    if (isKeySensitive) {
    redactedObj[key] = 'REDACTED'
    } else if (obj[key] !== null && typeof obj[key] === 'object') {
    redactedObj[key] = redactSensitiveFields(obj[key], recursionDepth + 1, maxDepth, seen)
    } else {
    redactedObj[key] = obj[key]
    }
    }
    }
    const portOnDisconnect = () => {
    port.proxy.onMessage.removeListener(proxyOnMessage)
    port.onMessage.removeListener(portOnMessage)
    port.proxy.onDisconnect.removeListener(proxyOnDisconnect)
    port.onDisconnect.removeListener(portOnDisconnect)
    return redactedObj
    }
    Copy link

    Copilot AI Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The sensitive data redaction function has a potential issue with circular reference detection. The WeakSet is correctly used to track seen objects, but when the function returns early due to circular references, the object remains in the seen set. This could cause issues if the same object appears in multiple branches of the object tree that aren't actually circular.

    Additionally, the redaction logic only checks for keywords in object keys but doesn't account for arrays of objects where sensitive data might be in the values. The function should handle this case more comprehensively.

    Copilot uses AI. Check for mistakes.
    src/content-script/index.jsx
    Comment on lines +683 to +834
    try {
    await new Promise((resolve, reject) => {
    timerId = setInterval(async () => {
    if (promiseSettled) {
    console.warn(
    '[content] Promise already settled but Claude interval still running. This indicates a potential cleanup issue.',
    )
    cleanup()
    return
    }
    try {
    claudeSession = await getClaudeSessionKey()
    if (claudeSession) {
    if (!promiseSettled) {
    promiseSettled = true
    cleanup()
    console.log('[content] Claude session key found after waiting.')
    resolve()
    }
    }
    } catch (err) {
    console.error('[content] Error polling for Claude session key:', err)
    const errMsg = err.message.toLowerCase()
    if (
    (errMsg.includes('network') || errMsg.includes('permission')) &&
    !promiseSettled
    ) {
    promiseSettled = true
    cleanup()
    reject(new Error(`Failed to get Claude session key due to: ${err.message}`))
    }
    }
    }, 500)

    timeoutId = setTimeout(() => {
    if (!promiseSettled) {
    promiseSettled = true
    cleanup()
    console.warn('[content] Timed out waiting for Claude session key.')
    reject(new Error('Timed out waiting for Claude session key.'))
    }
    }, 30000)
    })
    } catch (err) {
    console.error(
    '[content] Failed to get Claude session key for jump back notification:',
    err,
    )
    return
    }
    } else {
    console.log('[content] Claude session key found immediately.')
    }
    }

    const isKimiHost =
    location.hostname === 'kimi.moonshot.cn' ||
    location.hostname === 'kimi.com' ||
    location.hostname === 'www.kimi.com'
    if (isKimiHost) {
    console.debug('[content] On Kimi host, checking login status.')
    if (!window.localStorage.refresh_token) {
    console.log('[content] Kimi refresh token not found, attempting to trigger login.')
    setTimeout(() => {
    try {
    document.querySelectorAll('button').forEach((button) => {
    if (button.textContent === '立即登录') {
    console.log('[content] Clicking Kimi login button.')
    button.click()
    }
    })
    } catch (err_click) {
    console.error('[content] Error clicking Kimi login button:', err_click)
    }
    }, 1000)

    let promiseSettled = false
    let timerId = null
    let timeoutId = null
    const cleanup = () => {
    if (timerId) clearInterval(timerId)
    if (timeoutId) clearTimeout(timeoutId)
    }

    try {
    await new Promise((resolve, reject) => {
    timerId = setInterval(async () => {
    if (promiseSettled) {
    console.warn(
    '[content] Promise already settled but Kimi interval still running. This indicates a potential cleanup issue.',
    )
    cleanup()
    return
    }
    try {
    const token = window.localStorage.refresh_token
    if (token) {
    if (!promiseSettled) {
    promiseSettled = true
    cleanup()
    console.log('[content] Kimi refresh token found after waiting.')
    await setUserConfig({ kimiMoonShotRefreshToken: token })
    console.log('[content] Kimi refresh token saved to config.')
    resolve()
    }
    }
    } catch (err_set) {
    console.error('[content] Error setting Kimi refresh token from polling:', err_set)
    // Do not reject on polling error, let timeout handle failure.
    }
    }, 500)

    timeoutId = setTimeout(() => {
    if (!promiseSettled) {
    promiseSettled = true
    cleanup()
    console.warn('[content] Timed out waiting for Kimi refresh token.')
    reject(new Error('Timed out waiting for Kimi refresh token.'))
    }
    }, 30000)
    })
    clearInterval(timer)
    resolve()
    } catch (err) {
    console.error(
    '[content] Failed to get Kimi refresh token for jump back notification:',
    err,
    )
    return
    }
    }, 500)
    } else {
    console.log('[content] Kimi refresh token found in localStorage.')
    await setUserConfig({ kimiMoonShotRefreshToken: window.localStorage.refresh_token })
    }
    }

    console.log('[content] Rendering WebJumpBackNotification.')
    const div = document.createElement('div')
    document.body.append(div)
    render(
    <WebJumpBackNotification
    container={div}
    chatgptMode={location.hostname === 'chatgpt.com'}
    />,
    div,
    )
    console.log('[content] WebJumpBackNotification rendered.')
    } else {
    console.debug('[content] No chatgptbox_notification param in URL.')
    }
    } catch (error) {
    console.error('[content] Error in prepareForJumpBackNotification:', error)
    }
    }
    Copy link

    Copilot AI Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The polling logic for Claude and Kimi session keys uses promiseSettled flag to prevent issues, but there's still a potential race condition. If the interval fires right as the timeout is triggering, both could try to call cleanup() simultaneously. While this might not cause a critical error due to the guard checks, it could lead to duplicate error messages or unexpected behavior.

    Additionally, the error handling only rejects on specific error message patterns (network/permission), but other errors are silently ignored in the interval callback, which could mask important issues.

    Copilot uses AI. Check for mistakes.
    src/content-script/index.jsx
    Comment on lines +890 to +965
    let manageChatGptTabStatePromise = null

    async function manageChatGptTabState() {
    if (manageChatGptTabStatePromise) {
    console.debug('[content] manageChatGptTabState already running, waiting for in-flight call.')
    return manageChatGptTabStatePromise
    }

    manageChatGptTabStatePromise = (async () => {
    console.debug('[content] manageChatGptTabState called. Current location:', location.href)
    try {
    if (location.hostname !== 'chatgpt.com' || location.pathname === '/auth/login') {
    console.debug(
    '[content] Not on main chatgpt.com page, skipping manageChatGptTabState logic.',
    )
    return
    }

    const userConfig = await getUserConfig()
    console.debug('[content] User config in manageChatGptTabState:', userConfig)
    const isThisTabDesignatedForChatGptWeb = chatgptWebModelKeys.some((model) =>
    getApiModesStringArrayFromConfig(userConfig, true).includes(model),
    )
    console.debug(
    '[content] Is this tab designated for ChatGPT Web:',
    isThisTabDesignatedForChatGptWeb,
    )

    if (isThisTabDesignatedForChatGptWeb) {
    if (location.pathname === '/') {
    console.debug('[content] On chatgpt.com root path.')
    const input = document.querySelector('#prompt-textarea')
    if (input && input.value === '') {
    console.log('[content] Manipulating #prompt-textarea for focus.')
    input.value = ' '
    input.dispatchEvent(new Event('input', { bubbles: true }))
    setTimeout(() => {
    if (input && input.value === ' ') {
    input.value = ''
    input.dispatchEvent(new Event('input', { bubbles: true }))
    console.debug('[content] #prompt-textarea manipulation complete.')
    } else if (!input) {
    console.warn(
    '[content] #prompt-textarea no longer available in setTimeout callback.',
    )
    }
    }, 300)
    } else {
    console.debug(
    '[content] #prompt-textarea not found, not empty (value: "' +
    input?.value +
    '"), or not on root path for manipulation.',
    )
    }
    }

    console.log('[content] Sending SET_CHATGPT_TAB message.')
    await Browser.runtime.sendMessage({
    type: 'SET_CHATGPT_TAB',
    data: {},
    })
    console.log('[content] SET_CHATGPT_TAB message sent successfully.')
    } else {
    console.log('[content] This tab is NOT configured for ChatGPT Web model processing.')
    }
    } catch (error) {
    console.error('[content] Error in manageChatGptTabState:', error)
    }
    })
    })()

    await overwriteAccessToken()
    await prepareForForegroundRequests()
    try {
    await manageChatGptTabStatePromise
    } finally {
    manageChatGptTabStatePromise = null
    }
    }
    Copy link

    Copilot AI Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The manageChatGptTabStatePromise global variable is used to prevent concurrent executions of manageChatGptTabState, but there's a subtle issue. If the function throws an error that's caught internally, the promise still completes successfully, and the variable is set to null in the finally block. However, if the function is called again while an error is being handled, the new call will wait for the failed promise and then proceed, potentially causing duplicate work or race conditions.

    A better pattern would be to reject the promise on error or use a more robust state machine to track the actual state of the operation.

    Copilot uses AI. Check for mistakes.
    src/content-script/index.jsx
    if (userConfig.triggerMode === 'always') triggered = true
    else if (
    userConfig.triggerMode === 'questionMark' &&
    question &&
    Copy link

    Copilot AI Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    This use of variable 'question' always evaluates to true.

    Suggested change
    question &&

    Copilot uses AI. Check for mistakes.
    chatgpt-codex-connector[bot]
    chatgpt-codex-connector bot reviewed Jan 8, 2026
    View reviewed changes
    Copy link

    @chatgpt-codex-connector chatgpt-codex-connector bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    💡 Codex Review

    Here are some automated review suggestions for this pull request.

    Reviewed commit: f01b0a7da6

    ℹ️ About Codex in GitHub

    Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

    • Open a pull request for review
    • Mark a draft as ready
    • Comment "@codex review".

    If Codex has suggestions, it will comment; otherwise it will react with 👍.

    Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

    src/background/index.mjs
    Comment on lines +387 to +390
    if (port.proxy) {
    console.debug('[background] Posting message to proxy tab:', { session })
    try {
    port.proxy.postMessage({ session })
    Copy link

    @chatgpt-codex-connector chatgpt-codex-connector bot Jan 8, 2026

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    P2 Badge Avoid double-forwarding ChatGPT Web sessions

    Once setPortProxy has run, it already installs a port.onMessage handler that forwards every UI message to the proxy tab. This new if (port.proxy) { port.proxy.postMessage({ session }) } path means that after the first request, subsequent ChatGPT Web requests from the same port are forwarded twice (once by the forwarding listener and once here), which can trigger duplicate ChatGPT tab processing/answers. This only happens when the popup/sidebar stays open and sends multiple requests, but it will duplicate responses and waste quota in that common flow.

    Useful? React with 👍 / 👎.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

    Copilot code review Copilot Copilot left review comments

    @chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

    +1 more reviewer

    @gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

    Reviewers whose approvals may not affect merge requirements

    Assignees

    No one assigned

    Labels

    Review effort 4/5

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    2 participants

    @PeterDaveHello