Skip to content

Conversation

@PeterDaveHello
Copy link
Member

@PeterDaveHello PeterDaveHello commented Jun 7, 2025

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

      • Jump‑back notifications, static inline cards, enhanced selection tools and right‑click menu, plus broader in‑site model/session support.
    • Bug Fixes & Reliability

      • More resilient reconnection, redaction and token/session handling; safer messaging/proxy flows; guarded UI mounting with cross‑browser fallbacks; robust request/tab listeners and improved diagnostics/logging.
    • Chores

      • Manifest updated to enable stronger request handling for improved reliability.

    ✏️ 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

    📝 Walkthrough

    Walkthrough

    Centralized redaction and reconnection added to background; runtime messaging, webRequest/tab listeners, and side-panel guards expanded. Content script adds idempotent mounting, multi-token (Claude/Kimi/ChatGPT) management, selection/UI tooling, and new run/manage utilities.

    Changes

    Cohort / File(s) Summary
    Background Service
    src/background/index.mjs
    Introduced RECONNECT_CONFIG, SENSITIVE_KEYWORDS, and redactSensitiveFields. Replaced port proxy with setPortProxy (reconnect timers, listener cleanup, redacted logging, resilient forwarding). Expanded executeApi with redaction/instrumentation and broad dispatch. Hardened runtime message handling, webRequest/tab listeners, side-panel fallbacks, and registration/error logging.
    Content Script — UI & Token Orchestration
    src/content-script/index.jsx
    Reworked mounting with retry/URL guards and container cleanup. Added robust getInput, idempotent selection/touch/right-click tooling, static card/site-adapter logic, guarded overwriteAccessToken, new getClaudeSessionKey, prepareForJumpBackNotification, manageChatGptTabState, and run. Added guarded port listener registration, pervasive logging, and defensive error handling.
    Manifest / Permissions
    src/manifest.v2.json
    Added webRequestBlocking permission.

    Sequence Diagram(s)

    sequenceDiagram
        participant Port as Background Port
        participant Manager as Reconnect Manager
        participant Handler as Message Handler
        participant Redactor as Redaction Layer
        participant API as API Executor
    
        Port->>Manager: connect
        Manager->>Handler: deliver incoming message
        Handler->>Redactor: redact sensitive fields
        Redactor-->>Handler: redacted payload
        Handler->>API: executeApi(request)
        API-->>Handler: response / error
        Handler->>Port: send response
    
        alt Connection lost
            Port--xManager: disconnect
            Manager->>Manager: exponential backoff retry
            Manager->>Port: reconnect attempt
        end
    
    Loading
    sequenceDiagram
        participant Runtime as Browser Runtime
        participant Script as Content Script
        participant TokenMgr as Token Manager
        participant Claude as Claude Service
        participant UI as UI Components
    
        Runtime->>Script: inject / start
        Script->>Script: run()
        Script->>TokenMgr: detect / overwrite tokens
        TokenMgr->>Claude: request sessionKey
        Claude-->>TokenMgr: sessionKey or null
        Script->>Script: mountComponent (with retry)
        Script->>UI: init selection tools / static card
    
        alt Jump-back required
            Script->>TokenMgr: poll token readiness
            TokenMgr-->>Script: token ready
            Script->>UI: render WebJumpBackNotification
        end
    
    Loading

    Estimated code review effort

    🎯 4 (Complex) | ⏱️ ~45 minutes

    Poem

    🐇 I hop through ports with careful paws,

    I hush the secrets, mend the laws,
    I chase each token, mount each view,
    I stitch reconnects and scrub what's new,
    Small rabbit fixes, big-safe hues.

    🚥 Pre-merge checks | ✅ 1 | ❌ 2
    ❌ Failed checks (1 warning, 1 inconclusive)
    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.
    Title check ❓ Inconclusive The title 'Fix: Address potential bugs and improve error handling' is vague and generic, using non-descriptive terms that don't convey the specific nature of the substantial refactoring work across multiple files. Consider a more specific title that highlights the primary change, such as 'Refactor: Add comprehensive error handling and logging to background and content script' or similar to better reflect the scope of the changes.
    ✅ Passed checks (1 passed)
    Check name Status Explanation
    Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

    ✏️ 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

    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.

    @qodo-code-review
    Copy link
    Contributor

    qodo-code-review bot commented Jun 7, 2025

    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...

    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)

    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.

    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.

    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.

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

    This comment was marked as outdated.

    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.

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

    This comment was marked as outdated.

    @PeterDaveHello
    Copy link
    Member Author

    @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
    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

    @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
    Copy link
    Member Author

    @codex review

    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 2 out of 2 changed files in this pull request and generated 7 comments.


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

    )
    }
    } catch (e) {
    console.error('[content] Error in port listener callback:', e, 'Session:', session)
    Copy link

    Copilot AI Jan 25, 2026

    Choose a reason for hiding this comment

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

    The error logging at line 1022 logs the entire session object to the console, which could contain sensitive information like API keys, tokens, or user data. While this is helpful for debugging, it poses a security risk in production. The background script has implemented a redaction mechanism (redactSensitiveFields function), but the content script doesn't use it. Consider applying similar redaction here or reducing what gets logged.

    Copilot uses AI. Check for mistakes.
    .catch((e) => console.error('[background] Error saving Arkose req url and form:', e))
    }
    } catch (error) {
    console.error('[background] Error in onBeforeRequest listener callback:', error, details)
    Copy link

    Copilot AI Jan 25, 2026

    Choose a reason for hiding this comment

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

    The error handler logs the entire details object which could contain sensitive request information. While this is useful for debugging, in production it could expose sensitive data. Consider implementing selective logging or using the redaction mechanism for error logs that include request details.

    Copilot uses AI. Check for mistakes.
    Comment on lines 259 to 1009
    let selectionToolsInitialized = false

    async function prepareForSelectionTools() {
    if (selectionToolsInitialized) {
    console.debug('[content] Selection tools already initialized, skipping.')
    return
    }
    selectionToolsInitialized = true
    console.log('[content] Initializing selection tools.')
    document.addEventListener('mouseup', (e) => {
    if (toolbarContainer && toolbarContainer.contains(e.target)) return
    const selectionElement =
    window.getSelection()?.rangeCount > 0 &&
    window.getSelection()?.getRangeAt(0).endContainer.parentElement
    if (toolbarContainer && selectionElement && toolbarContainer.contains(selectionElement)) return

    deleteToolbar()
    setTimeout(async () => {
    const selection = window
    .getSelection()
    ?.toString()
    .trim()
    .replace(/^-+|-+$/g, '')
    if (selection) {
    let position

    const config = await getUserConfig()
    if (!config.selectionToolsNextToInputBox) position = { x: e.pageX + 20, y: e.pageY + 20 }
    else {
    const inputElement = selectionElement.querySelector('input, textarea')
    if (inputElement) {
    position = getClientPosition(inputElement)
    position = {
    x: position.x + window.scrollX + inputElement.offsetWidth + 50,
    y: e.pageY + 30,
    try {
    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 (selectionElement && toolbarContainer?.contains(selectionElement)) {
    console.debug('[content] Mouseup selection is inside toolbar, ignoring.')
    return
    }

    deleteToolbar()
    setTimeout(async () => {
    try {
    const selection = window
    .getSelection()
    ?.toString()
    .trim()
    .replace(/^-+|-+$/g, '')
    if (selection) {
    console.debug('[content] Text selected:', selection)
    let position

    const config = await getUserConfig()
    if (!config.selectionToolsNextToInputBox) {
    position = { x: e.pageX + 20, y: e.pageY + 20 }
    } else {
    const activeElement = document.activeElement
    const inputElement =
    selectionElement?.querySelector('input, textarea') ||
    (activeElement?.matches('input, textarea') ? activeElement : null)

    if (inputElement) {
    console.debug(
    '[content] Input element found for positioning toolbar:',
    inputElement,
    )
    const clientRect = getClientPosition(inputElement)
    position = {
    x: clientRect.x + window.scrollX + inputElement.offsetWidth + 50,
    y: e.pageY + 30,
    }
    } else {
    position = { x: e.pageX + 20, y: e.pageY + 20 }
    }
    }
    console.debug('[content] Toolbar position:', position)
    toolbarContainer = createElementAtPosition(position.x, position.y)
    await createSelectionTools(toolbarContainer, selection)
    } else {
    position = { x: e.pageX + 20, y: e.pageY + 20 }
    console.debug('[content] No text selected on mouseup.')
    }
    } catch (err) {
    console.error('[content] Error in mouseup setTimeout callback for selection tools:', err)
    }
    toolbarContainer = createElementAtPosition(position.x, position.y)
    await createSelectionTools(toolbarContainer, selection)
    }
    })
    }, 0)
    } catch (error) {
    console.error('[content] Error in mouseup listener for selection tools:', error)
    }
    })
    document.addEventListener('mousedown', (e) => {
    if (toolbarContainer && toolbarContainer.contains(e.target)) return

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

    document.addEventListener('keydown', (e) => {
    if (
    toolbarContainer &&
    !toolbarContainer.contains(e.target) &&
    (e.target.nodeName === 'INPUT' || e.target.nodeName === 'TEXTAREA')
    ) {
    setTimeout(() => {
    if (!window.getSelection()?.toString().trim()) deleteToolbar()
    })
    try {
    if (
    toolbarContainer &&
    !toolbarContainer.contains(e.target) &&
    (e.target.nodeName === 'INPUT' || e.target.nodeName === 'TEXTAREA')
    ) {
    console.debug('[content] Keydown in input/textarea outside toolbar.')
    setTimeout(() => {
    try {
    if (!window.getSelection()?.toString().trim()) {
    console.debug('[content] No selection after keydown, deleting toolbar.')
    deleteToolbar()
    }
    } catch (err_inner) {
    console.error('[content] Error in keydown setTimeout callback:', err_inner)
    }
    }, 0)
    }
    } catch (error) {
    console.error('[content] Error in keydown listener for selection tools:', error)
    }
    })
    }

    let selectionToolsTouchInitialized = false

    async function prepareForSelectionToolsTouch() {
    if (selectionToolsTouchInitialized) {
    console.debug('[content] Touch selection tools already initialized, skipping.')
    return
    }
    selectionToolsTouchInitialized = true
    console.log('[content] Initializing touch selection tools.')
    document.addEventListener('touchend', (e) => {
    if (toolbarContainer && toolbarContainer.contains(e.target)) return
    if (
    toolbarContainer &&
    window.getSelection()?.rangeCount > 0 &&
    toolbarContainer.contains(window.getSelection()?.getRangeAt(0).endContainer.parentElement)
    )
    return

    deleteToolbar()
    setTimeout(() => {
    const selection = window
    .getSelection()
    ?.toString()
    .trim()
    .replace(/^-+|-+$/g, '')
    if (selection) {
    toolbarContainer = createElementAtPosition(
    e.changedTouches[0].pageX + 20,
    e.changedTouches[0].pageY + 20,
    )
    createSelectionTools(toolbarContainer, selection)
    try {
    if (toolbarContainer?.contains(e.target)) {
    console.debug('[content] Touchend inside toolbar, ignoring.')
    return
    }
    })
    if (
    window.getSelection()?.rangeCount > 0 &&
    toolbarContainer?.contains(window.getSelection()?.getRangeAt(0).endContainer.parentElement)
    ) {
    console.debug('[content] Touchend selection is inside toolbar, ignoring.')
    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
    let rightClickMenuInitialized = false

    async function prepareForRightClickMenu() {
    if (rightClickMenuInitialized) {
    console.debug('[content] Right-click menu already initialized, skipping.')
    return
    }
    rightClickMenuInitialized = true
    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 }
    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,
    )
    const useMenuPosition =
    data.useMenuPosition && Number.isFinite(menuX) && Number.isFinite(menuY)
    const position = 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)
    }
    }
    })
    }

    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).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}`)

    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)
    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 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(siteName, siteConfig[siteName])
    if (initSuccess) {
    console.log(`[content] Mounting static card for site: ${siteName}`)
    await mountComponent(siteName, 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') {
    if (location.hostname === 'kimi.moonshot.cn' || location.hostname.includes('kimi.com')) {
    setUserConfig({
    kimiMoonShotRefreshToken: window.localStorage.refresh_token,
    })
    console.debug('[content] overwriteAccessToken called for hostname:', location.hostname)
    try {
    const isKimiHost =
    location.hostname === 'kimi.moonshot.cn' ||
    location.hostname === 'kimi.com' ||
    location.hostname === 'www.kimi.com'
    if (isKimiHost) {
    console.log(`[content] On ${location.hostname}, 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
    }

    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} ${resp.statusText}`,
    )
    }
    } catch (error) {
    console.error('[content] Error fetching access token from API:', error)
    }
    }
    } else {
    const resp = await fetch('https://chatgpt.com/api/auth/session')
    data = await resp.json().catch(() => ({}))

    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)
    }
    if (data && data.accessToken) {
    await setAccessToken(data.accessToken)
    console.log(data.accessToken)
    }

    async function getClaudeSessionKey() {
    console.debug('[content] getClaudeSessionKey called.')
    try {
    const sessionKey = await Browser.runtime.sendMessage({
    type: 'GET_COOKIE',
    data: { url: 'https://claude.ai/', name: 'sessionKey' },
    })
    console.debug(
    '[content] Claude session key from background:',
    sessionKey ? 'found' : 'not found',
    )
    return sessionKey
    } catch (error) {
    console.error('[content] Error in getClaudeSessionKey sending message:', error)
    return null
    }
    }

    async function prepareForForegroundRequests() {
    if (location.hostname !== 'chatgpt.com' || location.pathname === '/auth/login') return
    async function prepareForJumpBackNotification() {
    console.log('[content] Initializing jump back notification.')
    try {
    if (
    location.hostname === 'chatgpt.com' &&
    document.querySelector('button[data-testid=login-button]')
    ) {
    console.log('[content] ChatGPT login button found, user not logged in. Skipping jump back.')
    return
    }

    const userConfig = await getUserConfig()
    const url = new URL(window.location.href)
    if (url.searchParams.has('chatgptbox_notification')) {
    console.log('[content] chatgptbox_notification param found in URL.')

    if (
    !chatgptWebModelKeys.some((model) =>
    getApiModesStringArrayFromConfig(userConfig, true).includes(model),
    )
    )
    return
    if (location.hostname === 'claude.ai') {
    console.debug('[content] On claude.ai, checking login status.')
    let claudeSession = await getClaudeSessionKey()
    if (!claudeSession) {
    console.log('[content] Claude session key not found, waiting for it...')
    let promiseSettled = false
    let timerId = null
    let timeoutId = null
    const cleanup = () => {
    if (timerId) clearTimeout(timerId)
    if (timeoutId) clearTimeout(timeoutId)
    }

    // 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)
    // }
    // }

    await Browser.runtime.sendMessage({
    type: 'SET_CHATGPT_TAB',
    data: {},
    })
    try {
    await new Promise((resolve, reject) => {
    const poll = async () => {
    if (promiseSettled) return
    try {
    claudeSession = await getClaudeSessionKey()
    if (claudeSession && !promiseSettled) {
    promiseSettled = true
    cleanup()
    console.log('[content] Claude session key found after waiting.')
    resolve()
    return
    }
    } catch (err) {
    console.error('[content] Error polling for Claude session key:', err)
    const rawMessage =
    typeof err?.message === 'string' ? err.message : String(err ?? '')
    const errMsg = rawMessage.toLowerCase()
    const isNetworkError = /\bnetwork\b/.test(errMsg)
    const isPermissionError = /\bpermission\b/.test(errMsg)
    if ((isNetworkError || isPermissionError) && !promiseSettled) {
    promiseSettled = true
    cleanup()
    reject(new Error(`Failed to get Claude session key due to: ${rawMessage}`))
    return
    }
    }
    if (!promiseSettled) {
    timerId = setTimeout(poll, 500)
    }
    }

    registerPortListener(async (session, port) => {
    if (isUsingChatgptWebModel(session)) {
    const accessToken = await getChatGptAccessToken()
    await generateAnswersWithChatgptWebApi(port, session.question, session, accessToken)
    }
    })
    }
    poll()

    async function getClaudeSessionKey() {
    return Browser.runtime.sendMessage({
    type: 'GET_COOKIE',
    data: { url: 'https://claude.ai/', name: 'sessionKey' },
    })
    }
    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.')
    }
    }

    async function prepareForJumpBackNotification() {
    if (
    location.hostname === 'chatgpt.com' &&
    document.querySelector('button[data-testid=login-button]')
    ) {
    console.log('chatgpt not logged in')
    return
    }
    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 {
    const loginContainer = document.querySelector('.user-info-container')
    if (!loginContainer) {
    console.warn('[content] Kimi login container not found, skipping auto-click.')
    return
    }
    console.log('[content] Clicking Kimi login container.')
    loginContainer.click()
    } catch (err_click) {
    console.error('[content] Error clicking Kimi login container:', err_click)
    }
    }, 1000)

    const url = new URL(window.location.href)
    if (url.searchParams.has('chatgptbox_notification')) {
    if (location.hostname === 'claude.ai' && !(await getClaudeSessionKey())) {
    console.log('claude not logged in')

    await new Promise((resolve) => {
    const timer = setInterval(async () => {
    const token = await getClaudeSessionKey()
    if (token) {
    clearInterval(timer)
    resolve()
    let promiseSettled = false
    let timerId = null
    let timeoutId = null
    const cleanup = () => {
    if (timerId) clearTimeout(timerId)
    if (timeoutId) clearTimeout(timeoutId)
    }
    }, 500)
    })
    }

    if (
    (location.hostname === 'kimi.moonshot.cn' || location.hostname.includes('kimi.com')) &&
    !window.localStorage.refresh_token
    ) {
    console.log('kimi not logged in')
    setTimeout(() => {
    document.querySelector('.user-info-container').click()
    }, 1000)

    await new Promise((resolve) => {
    const timer = setInterval(() => {
    const token = window.localStorage.refresh_token
    if (token) {
    setUserConfig({
    kimiMoonShotRefreshToken: token,
    try {
    await new Promise((resolve, reject) => {
    const poll = async () => {
    if (promiseSettled) return
    try {
    const token = window.localStorage.refresh_token
    if (token) {
    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()
    return
    }
    } 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.
    }
    if (!promiseSettled) {
    timerId = setTimeout(poll, 500)
    }
    }

    poll()

    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)
    }
    }

    async function run() {
    console.log('[content] Script run started.')
    try {
    await getPreferredLanguageKey()
    .then((lang) => {
    console.log(`[content] Setting language to: ${lang}`)
    changeLanguage(lang)
    })
    .catch((err) => console.error('[content] Error setting preferred language:', err))

    Browser.runtime.onMessage.addListener(async (message) => {
    console.debug('[content] Received runtime message:', message)
    try {
    if (message.type === 'CHANGE_LANG') {
    console.log('[content] Processing CHANGE_LANG message:', message.data)
    changeLanguage(message.data.lang)
    }
    } catch (error) {
    console.error('[content] Error in global runtime.onMessage listener:', error, message)
    }
    })

    await overwriteAccessToken()
    const isChatGptHost = location.hostname === 'chatgpt.com'
    if (isChatGptHost) {
    await manageChatGptTabState()

    Browser.storage.onChanged.addListener(async (changes, areaName) => {
    console.debug('[content] Storage changed:', changes, 'in area:', areaName)
    try {
    if (areaName === 'local' && (changes.userConfig || changes.config)) {
    console.log(
    '[content] User config changed in storage, re-evaluating ChatGPT tab state.',
    )
    await manageChatGptTabState()
    }
    } catch (error) {
    console.error('[content] Error in storage.onChanged listener:', error)
    }
    })
    }

    const div = document.createElement('div')
    document.body.append(div)
    render(
    <WebJumpBackNotification container={div} chatgptMode={location.hostname === 'chatgpt.com'} />,
    div,
    )
    await prepareForSelectionTools()
    await prepareForSelectionToolsTouch()
    prepareForStaticCard().catch((error) => {
    console.error('[content] Error in prepareForStaticCard (unhandled):', error)
    })
    await prepareForRightClickMenu()
    prepareForJumpBackNotification().catch((error) => {
    console.error('[content] Error in prepareForJumpBackNotification (unhandled):', error)
    })

    console.log('[content] Script run completed successfully.')
    } catch (error) {
    console.error('[content] Error in run function:', error)
    }
    }

    async function run() {
    await getPreferredLanguageKey().then((lang) => {
    changeLanguage(lang)
    })
    Browser.runtime.onMessage.addListener(async (message) => {
    if (message.type === 'CHANGE_LANG') {
    const data = message.data
    changeLanguage(data.lang)
    let manageChatGptTabStatePromise = null
    let chatGPTBoxPortListenerRegistered = false

    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()
    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.')
    if (document.activeElement === input) {
    console.debug('[content] #prompt-textarea already focused; skipping injection.')
    } else {
    const injectedValue = ' '
    input.value = injectedValue
    input.dispatchEvent(new Event('input', { bubbles: true }))
    setTimeout(() => {
    const currentInput = document.querySelector('#prompt-textarea')
    if (!currentInput?.isConnected) {
    console.warn(
    '[content] #prompt-textarea no longer available in setTimeout callback.',
    )
    return
    }
    if (document.activeElement === currentInput) {
    console.debug('[content] #prompt-textarea focused; skipping injection cleanup.')
    return
    }
    if (currentInput.value === injectedValue) {
    currentInput.value = ''
    currentInput.dispatchEvent(new Event('input', { bubbles: true }))
    console.debug('[content] #prompt-textarea manipulation complete.')
    } else if (currentInput.value.startsWith(injectedValue)) {
    currentInput.value = currentInput.value.slice(injectedValue.length)
    currentInput.dispatchEvent(new Event('input', { bubbles: true }))
    console.debug('[content] Removed injected leading space from #prompt-textarea.')
    }
    }, 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
    }
    }

    prepareForSelectionTools()
    prepareForSelectionToolsTouch()
    prepareForStaticCard()
    prepareForRightClickMenu()
    prepareForJumpBackNotification()
    if (!chatGPTBoxPortListenerRegistered) {
    Copy link

    Copilot AI Jan 25, 2026

    Choose a reason for hiding this comment

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

    The initialization flags (selectionToolsInitialized, selectionToolsTouchInitialized, rightClickMenuInitialized, chatGPTBoxPortListenerRegistered) prevent re-initialization, but they're never reset. This means if the script is reinjected or reloaded in the same context, these features won't be re-initialized, potentially leading to broken functionality. Consider whether these should be reset under certain conditions or if the script lifecycle guarantees they're only needed once.

    Copilot uses AI. Check for mistakes.
    Comment on lines +269 to +271
    const delay =
    Math.pow(RECONNECT_CONFIG.BACKOFF_MULTIPLIER, port._reconnectAttempts - 1) *
    RECONNECT_CONFIG.BASE_DELAY_MS
    Copy link

    Copilot AI Jan 25, 2026

    Choose a reason for hiding this comment

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

    The exponential backoff delay calculation uses Math.pow with attempt numbers, but there's no maximum delay cap. With MAX_ATTEMPTS of 5 and a backoff multiplier of 2, the delays will be 1s, 2s, 4s, 8s, which is reasonable. However, if these constants are ever changed to larger values, the delay could become excessively long. Consider adding a maximum delay cap for robustness.

    Copilot uses AI. Check for mistakes.
    if (userConfig.triggerMode === 'always') triggered = true
    else if (
    userConfig.triggerMode === 'questionMark' &&
    question &&
    Copy link

    Copilot AI Jan 25, 2026

    Choose a reason for hiding this comment

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

    The condition checks if endsWithQuestionMark(question.trim()) but on line 120 the question variable is already checked for truthiness in the condition. However, looking at line 119-122, if question is falsy, the second part of the OR won't execute due to short-circuit evaluation, so this is safe. That said, for code clarity and to prevent potential issues if the code is refactored, consider adding an explicit null/undefined check before calling trim().

    Suggested change
    question &&
    typeof question === 'string' &&

    Copilot uses AI. Check for mistakes.
    Comment on lines +425 to +505
    const hasMatchingProxy = Boolean(port.proxy && port._proxyTabId === tabId)
    if (!hasMatchingProxy) {
    if (port.proxy) {
    console.debug(
    `[background] Existing proxy tab ${port._proxyTabId} does not match ${tabId}; reconnecting.`,
    )
    } else {
    console.debug('[background] port.proxy not found, calling setPortProxy.')
    }
    setPortProxy(port, tabId)
    }
    if (port.proxy && port._proxyTabId === tabId) {
    if (hasMatchingProxy) {
    console.debug('[background] Proxy already established; forwarding session.')
    }
    console.debug('[background] Posting message to proxy tab:', { session: redactedSession })
    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: redactedSession },
    )
    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: redactedSession },
    )
    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 25, 2026

    Choose a reason for hiding this comment

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

    There's a potential race condition in the proxy setup logic. Between checking hasMatchingProxy on line 425 and calling setPortProxy on line 434, the proxy state could change (e.g., if a disconnect happens). Then on line 436, the code checks if port.proxy exists and matches the tabId, but this check happens after setPortProxy which is synchronous at the top level but sets up async reconnection logic. If setPortProxy fails to establish the connection immediately, the condition on line 436 might be false, leading to the error path on line 492. Consider making the proxy establishment more atomic or adding explicit waiting for the connection to be established.

    Copilot uses AI. Check for mistakes.
    Comment on lines +1004 to +1022
    console.log(
    '[content] Session is for ChatGPT Web Model, processing request for question:',
    session.question,
    )
    Copy link

    Copilot AI Jan 25, 2026

    Choose a reason for hiding this comment

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

    The code logs the full session.question at line 1006, which could contain sensitive user queries or personal information. While the background script has a redaction mechanism for sensitive fields, the content script doesn't use it. User questions might contain personal data, passwords, or other sensitive information that shouldn't be logged. Consider either redacting sensitive parts of the question or reducing the logging level for production builds.

    Copilot uses AI. Check for mistakes.
    @chatgpt-codex-connector
    Copy link

    Codex Review: Didn't find any major issues. Swish!

    ℹ️ 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

    @codex review

    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 2 out of 2 changed files in this pull request and generated 7 comments.


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

    Comment on lines +441 to +490
    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: redactedSession },
    )
    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: redactedSession },
    )
    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,
    )
    }
    }
    }
    Copy link

    Copilot AI Jan 25, 2026

    Choose a reason for hiding this comment

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

    In the ChatGPT Web Model handling, when the first attempt to post a message fails, the code calls setPortProxy to reconnect and then attempts to post the message again. However, if setPortProxy itself throws an error or fails to establish the proxy (e.g., the tab was closed), the second attempt will also fail. The nested error handling here is complex, and the fallback logic might not handle all error scenarios correctly. Consider simplifying this logic or adding a maximum retry count to prevent infinite loops in edge cases.

    Copilot uses AI. Check for mistakes.
    Comment on lines +805 to +816
    if (details.requestBody?.formData) {
    for (const k in details.requestBody.formData) {
    const values = details.requestBody.formData[k]
    if (Array.isArray(values)) {
    for (const value of values) {
    formData.append(k, value)
    }
    } else if (values != null) {
    formData.append(k, values)
    }
    }
    }
    Copy link

    Copilot AI Jan 25, 2026

    Choose a reason for hiding this comment

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

    The Arkose form data handling has been improved to handle array values, but there's a potential issue. When details.requestBody.formData[k] contains multiple values (array), the code appends each value separately. However, the URLSearchParams API will serialize this as multiple parameters with the same key. When reconstructing the form data, this might not match the original request format if the server expects a different encoding for array values. Verify that this serialization approach is compatible with the Arkose API's expectations.

    Copilot uses AI. Check for mistakes.
    Comment on lines +935 to +986
    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.')
    if (document.activeElement === input) {
    console.debug('[content] #prompt-textarea already focused; skipping injection.')
    } else {
    const injectedValue = ' '
    input.value = injectedValue
    input.dispatchEvent(new Event('input', { bubbles: true }))
    setTimeout(() => {
    const currentInput = document.querySelector('#prompt-textarea')
    if (!currentInput?.isConnected) {
    console.warn(
    '[content] #prompt-textarea no longer available in setTimeout callback.',
    )
    return
    }
    if (document.activeElement === currentInput) {
    console.debug('[content] #prompt-textarea focused; skipping injection cleanup.')
    return
    }
    if (currentInput.value === injectedValue) {
    currentInput.value = ''
    currentInput.dispatchEvent(new Event('input', { bubbles: true }))
    console.debug('[content] #prompt-textarea manipulation complete.')
    } else if (currentInput.value.startsWith(injectedValue)) {
    currentInput.value = currentInput.value.slice(injectedValue.length)
    currentInput.dispatchEvent(new Event('input', { bubbles: true }))
    console.debug('[content] Removed injected leading space from #prompt-textarea.')
    }
    }, 300)
    }
    } else {
    console.debug(
    '[content] #prompt-textarea not found, not empty (value: "' +
    input?.value +
    '"), or not on root path for manipulation.',
    )
    }
    }
    Copy link

    Copilot AI Jan 25, 2026

    Choose a reason for hiding this comment

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

    The prompt textarea manipulation logic in manageChatGptTabState has a race condition. Between checking if the textarea is focused in the outer check and the setTimeout callback, the user could have interacted with the page. The code checks document.activeElement === input twice but doesn't prevent concurrent executions. If manageChatGptTabState is called multiple times rapidly (e.g., from storage change events), multiple setTimeout callbacks could be queued, leading to unexpected behavior. Consider using the manageChatGptTabStatePromise guard more comprehensively or adding a flag to prevent concurrent textarea manipulations.

    Copilot uses AI. Check for mistakes.
    Comment on lines +240 to +302
    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)
    }
    }
    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
    }

    const delay =
    Math.pow(RECONNECT_CONFIG.BACKOFF_MULTIPLIER, port._reconnectAttempts - 1) *
    RECONNECT_CONFIG.BASE_DELAY_MS
    console.log(
    `[background] Attempting reconnect #${port._reconnectAttempts} in ${
    delay / 1000
    }s for tab ${proxyTabId}.`,
    )

    port._reconnectTimerId = setTimeout(async () => {
    if (port._isClosed) {
    console.debug('[background] Main port closed; skipping proxy reconnect.')
    return
    }
    port._reconnectTimerId = null
    try {
    await Browser.tabs.get(proxyTabId)
    } catch (error) {
    console.warn(
    `[background] Proxy tab ${proxyTabId} no longer exists. Aborting reconnect.`,
    error,
    )
    return
    }
    console.debug(
    `[background] Retrying connection to tab ${proxyTabId}, attempt ${port._reconnectAttempts}.`,
    )
    try {
    setPortProxy(port, proxyTabId)
    } catch (error) {
    console.warn(`[background] Error reconnecting to tab ${proxyTabId}:`, error)
    }
    }, delay)
    }
    Copy link

    Copilot AI Jan 25, 2026

    Choose a reason for hiding this comment

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

    The reconnection logic in setPortProxy schedules reconnection attempts even when the main port is still open. The check if (port._isClosed) at line 279 prevents reconnection after the main port closes, but there's no mechanism to cancel pending reconnection timers if the proxy successfully reconnects through other means (e.g., if another caller invokes setPortProxy before the timer fires). This could lead to redundant reconnection attempts. Consider storing and clearing the reconnection timer reference more carefully, or checking if the proxy is already connected when the timer fires.

    Copilot uses AI. Check for mistakes.
    Comment on lines +423 to +442
    if (tabId) {
    console.debug(`[background] ChatGPT Tab ID ${tabId} found.`)
    const hasMatchingProxy = Boolean(port.proxy && port._proxyTabId === tabId)
    if (!hasMatchingProxy) {
    if (port.proxy) {
    console.debug(
    `[background] Existing proxy tab ${port._proxyTabId} does not match ${tabId}; reconnecting.`,
    )
    } else {
    console.debug('[background] port.proxy not found, calling setPortProxy.')
    }
    setPortProxy(port, tabId)
    }
    if (port.proxy && port._proxyTabId === tabId) {
    if (hasMatchingProxy) {
    console.debug('[background] Proxy already established; forwarding session.')
    }
    console.debug('[background] Posting message to proxy tab:', { session: redactedSession })
    try {
    port.proxy.postMessage({ session })
    Copy link

    Copilot AI Jan 25, 2026

    Choose a reason for hiding this comment

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

    According to the PR description, this change ensures messages are sent to the ChatGPT tab even when port.proxy already exists. However, the logic at lines 436-442 will always attempt to send the message when port.proxy exists and matches the tabId, regardless of whether hasMatchingProxy was true initially. This is the intended fix, but the surrounding conditional logic with hasMatchingProxy is now somewhat redundant since the message is posted regardless of whether the proxy was just created or already existed. Consider simplifying this logic for clarity.

    Copilot uses AI. Check for mistakes.
    Comment on lines +589 to +595
    const config = await getUserConfig()
    if (config.kimiMoonShotRefreshToken) {
    await setUserConfig({ kimiMoonShotRefreshToken: '' })
    console.log('[content] Kimi Moonshot refresh token cleared.')
    }
    console.warn('[content] Kimi Moonshot refresh token not found in localStorage.')
    }
    Copy link

    Copilot AI Jan 25, 2026

    Choose a reason for hiding this comment

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

    In the overwriteAccessToken function for Kimi hosts, when the refresh token is not found, the code clears the stored token only if it was previously set. However, the check if (config.kimiMoonShotRefreshToken) at line 590 could be bypassed if the config is an empty string (which is falsy). Consider using if (config.kimiMoonShotRefreshToken !== undefined && config.kimiMoonShotRefreshToken !== null) to be more explicit, or simply always call setUserConfig to ensure consistency.

    Copilot uses AI. Check for mistakes.
    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: ac9d6e7ddd

    ℹ️ 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".

    Comment on lines 875 to 892
    Browser.storage.onChanged.addListener(async (changes, areaName) => {
    console.debug('[content] Storage changed:', changes, 'in area:', areaName)
    try {
    if (areaName === 'local' && (changes.userConfig || changes.config)) {
    console.log(
    '[content] User config changed in storage, re-evaluating ChatGPT tab state.',
    )
    await manageChatGptTabState()

    Choose a reason for hiding this comment

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

    P2 Badge Re-evaluate ChatGPT tab state on actual config keys

    The new storage.onChanged handler only runs when changes.userConfig or changes.config exist, but setUserConfig writes individual keys (e.g., modelName, apiMode, chatgptTabId) into storage.local, so those properties never appear. In practice, changing model settings in the popup while staying on chatgpt.com won’t trigger manageChatGptTabState, so the ChatGPT tab won’t be re-registered until a reload. Consider checking for any relevant key changes (or simply any areaName === 'local') instead of changes.userConfig/config.

    Useful? React with 👍 / 👎.

    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.

    Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

    @PeterDaveHello
    Copy link
    Member Author

    @codex review

    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 2 out of 2 changed files in this pull request and generated 7 comments.


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

    Comment on lines +262 to +266
    if (selectionToolsInitialized) {
    console.debug('[content] Selection tools already initialized, skipping.')
    return
    }
    selectionToolsInitialized = true
    Copy link

    Copilot AI Jan 26, 2026

    Choose a reason for hiding this comment

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

    The logic for checking if selectionToolsInitialized prevents re-initialization, but the flag is set at the module level and never reset. If the content script is reloaded or re-injected (which can happen in single-page applications or when the extension is updated), the event listeners won't be registered again because the flag remains true. Consider resetting this flag when appropriate or using a more robust initialization check.

    Copilot uses AI. Check for mistakes.
    Comment on lines +354 to +363
    setTimeout(() => {
    try {
    if (!window.getSelection()?.toString().trim()) {
    console.debug('[content] No selection after keydown, deleting toolbar.')
    deleteToolbar()
    }
    } catch (err_inner) {
    console.error('[content] Error in keydown setTimeout callback:', err_inner)
    }
    }, 0)
    Copy link

    Copilot AI Jan 26, 2026

    Choose a reason for hiding this comment

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

    Multiple setTimeout callbacks use arrow functions with additional try-catch blocks nested inside them (e.g., lines 354-363, 956-977). This creates deeply nested error handling that makes the code harder to read. Consider extracting these callback functions into named functions with their own error handling, which would improve readability and make the error handling structure clearer.

    Copilot uses AI. Check for mistakes.
    Comment on lines +374 to +378
    if (selectionToolsTouchInitialized) {
    console.debug('[content] Touch selection tools already initialized, skipping.')
    return
    }
    selectionToolsTouchInitialized = true
    Copy link

    Copilot AI Jan 26, 2026

    Choose a reason for hiding this comment

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

    Similar issue as with selectionToolsInitialized - the selectionToolsTouchInitialized flag is set at module level and never reset. This prevents re-initialization of touch event listeners if the content script is reloaded or re-injected, which can happen in SPAs or during extension updates.

    Copilot uses AI. Check for mistakes.
    Comment on lines +781 to 815
    try {
    await new Promise((resolve, reject) => {
    const poll = async () => {
    if (promiseSettled) return
    try {
    const token = window.localStorage.refresh_token
    if (token) {
    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()
    return
    }
    } 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.
    }
    if (!promiseSettled) {
    timerId = setTimeout(poll, 500)
    }
    }

    poll()

    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)
    })
    Copy link

    Copilot AI Jan 26, 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 Kimi refresh token has nearly identical structure to the Claude polling logic (lines 696-738), creating significant code duplication. Both implement the same polling pattern with promiseSettled flag, cleanup function, and timeout handling. Consider extracting this into a reusable utility function like 'pollForCondition(checkFn, timeoutMs, pollIntervalMs)' to improve maintainability and reduce the risk of bugs.

    Copilot uses AI. Check for mistakes.
    Comment on lines +696 to +726
    try {
    await new Promise((resolve, reject) => {
    const poll = async () => {
    if (promiseSettled) return
    try {
    claudeSession = await getClaudeSessionKey()
    if (claudeSession && !promiseSettled) {
    promiseSettled = true
    cleanup()
    console.log('[content] Claude session key found after waiting.')
    resolve()
    return
    }
    } catch (err) {
    console.error('[content] Error polling for Claude session key:', err)
    const rawMessage =
    typeof err?.message === 'string' ? err.message : String(err ?? '')
    const errMsg = rawMessage.toLowerCase()
    const isNetworkError = /\bnetwork\b/.test(errMsg)
    const isPermissionError = /\bpermission\b/.test(errMsg)
    if ((isNetworkError || isPermissionError) && !promiseSettled) {
    promiseSettled = true
    cleanup()
    reject(new Error(`Failed to get Claude session key due to: ${rawMessage}`))
    return
    }
    }
    if (!promiseSettled) {
    timerId = setTimeout(poll, 500)
    }
    }
    Copy link

    Copilot AI Jan 26, 2026

    Choose a reason for hiding this comment

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

    The polling implementation for Claude session key uses a recursive setTimeout pattern with error handling that rejects on network/permission errors. However, this pattern has a subtle issue: if an error occurs during poll(), it logs the error and checks if it's a network/permission error, but continues polling otherwise. This means transient errors could accumulate in the console. Consider adding a maximum error count threshold before giving up, or improving the error categorization logic.

    Copilot uses AI. Check for mistakes.
    Comment on lines +949 to +983
    console.log('[content] Manipulating #prompt-textarea for focus.')
    if (document.activeElement === input) {
    console.debug('[content] #prompt-textarea already focused; skipping injection.')
    } else {
    const injectedValue = ' '
    input.value = injectedValue
    input.dispatchEvent(new Event('input', { bubbles: true }))
    setTimeout(() => {
    const currentInput = document.querySelector('#prompt-textarea')
    if (!currentInput?.isConnected) {
    console.warn(
    '[content] #prompt-textarea no longer available in setTimeout callback.',
    )
    return
    }
    if (document.activeElement === currentInput) {
    console.debug('[content] #prompt-textarea focused; skipping injection cleanup.')
    return
    }
    if (currentInput.value === injectedValue) {
    currentInput.value = ''
    currentInput.dispatchEvent(new Event('input', { bubbles: true }))
    console.debug('[content] #prompt-textarea manipulation complete.')
    } else if (currentInput.value.startsWith(injectedValue)) {
    currentInput.value = currentInput.value.slice(injectedValue.length)
    currentInput.dispatchEvent(new Event('input', { bubbles: true }))
    console.debug('[content] Removed injected leading space from #prompt-textarea.')
    }
    }, 300)
    }
    } else {
    console.debug(
    '[content] #prompt-textarea not found, not empty (value: "' +
    input?.value +
    '"), or not on root path for manipulation.',
    Copy link

    Copilot AI Jan 26, 2026

    Choose a reason for hiding this comment

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

    The manageChatGptTabState function manipulates the #prompt-textarea element by injecting a space and then removing it after 300ms to work around focus issues. This approach is fragile and could interfere with user input or cause race conditions if the user interacts with the textarea during this window. The logic also has multiple nested checks for element availability and focus state. Consider using a more robust approach like directly calling focus() on the element or using the Textarea API if available, rather than manipulating its value.

    Suggested change
    console.log('[content] Manipulating #prompt-textarea for focus.')
    if (document.activeElement === input) {
    console.debug('[content] #prompt-textarea already focused; skipping injection.')
    } else {
    const injectedValue = ' '
    input.value = injectedValue
    input.dispatchEvent(new Event('input', { bubbles: true }))
    setTimeout(() => {
    const currentInput = document.querySelector('#prompt-textarea')
    if (!currentInput?.isConnected) {
    console.warn(
    '[content] #prompt-textarea no longer available in setTimeout callback.',
    )
    return
    }
    if (document.activeElement === currentInput) {
    console.debug('[content] #prompt-textarea focused; skipping injection cleanup.')
    return
    }
    if (currentInput.value === injectedValue) {
    currentInput.value = ''
    currentInput.dispatchEvent(new Event('input', { bubbles: true }))
    console.debug('[content] #prompt-textarea manipulation complete.')
    } else if (currentInput.value.startsWith(injectedValue)) {
    currentInput.value = currentInput.value.slice(injectedValue.length)
    currentInput.dispatchEvent(new Event('input', { bubbles: true }))
    console.debug('[content] Removed injected leading space from #prompt-textarea.')
    }
    }, 300)
    }
    } else {
    console.debug(
    '[content] #prompt-textarea not found, not empty (value: "' +
    input?.value +
    '"), or not on root path for manipulation.',
    console.log('[content] Ensuring #prompt-textarea is focused.')
    if (document.activeElement === input) {
    console.debug('[content] #prompt-textarea already focused; skipping focus().')
    } else if (typeof input.focus === 'function') {
    input.focus()
    console.debug('[content] Called focus() on #prompt-textarea.')
    } else {
    console.warn('[content] #prompt-textarea does not support focus().')
    }
    } else {
    console.debug(
    '[content] #prompt-textarea not found, not empty (value: "' +
    input?.value +
    '"), or not on root path for focus.',

    Copilot uses AI. Check for mistakes.
    })
    } catch (e) {
    console.warn('[background] Error sending final error message on max retries:', e)
    }
    Copy link

    Copilot AI Jan 26, 2026

    Choose a reason for hiding this comment

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

    The reconnection logic in setPortProxy implements exponential backoff with a maximum of 5 attempts, but the delay calculation uses Math.pow(BACKOFF_MULTIPLIER, port._reconnectAttempts - 1). When _reconnectAttempts is 1 (first retry), the delay is Math.pow(2, 0) * 1000 = 1000ms. However, after max retries are reached, the listeners are removed and an error is sent, but the port itself is not disconnected. This could leave the port in an inconsistent state. Consider explicitly calling port.disconnect() or ensuring the port is properly cleaned up after max retries.

    Suggested change
    }
    }
    // Mark the port as closed and explicitly disconnect to avoid leaving it in an inconsistent state.
    port._isClosed = true
    try {
    port.disconnect()
    } catch (e) {
    console.warn('[background] Error disconnecting port on max retries:', e)
    }

    Copilot uses AI. Check for mistakes.
    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: 2294ea4380

    ℹ️ 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".

    types: ['xmlhttprequest', 'websocket'],
    },
    ['requestHeaders'],
    ['requestHeaders', ...(Browser.runtime.getManifest().manifest_version < 3 ? ['blocking'] : [])],

    Choose a reason for hiding this comment

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

    P2 Badge Add MV2 blocking permission or drop blocking option

    For the Firefox/MV2 build, this now requests the blocking extraInfoSpec when manifest_version < 3, but src/manifest.v2.json does not include the webRequestBlocking permission. In MV2 this causes onBeforeSendHeaders.addListener to fail registration (permission error), so the Bing/Claude header overrides won’t run, and because the listeners are in the same try block it can also skip later listener setup. Either add the permission to the MV2 manifest or avoid using blocking there.

    Useful? React with 👍 / 👎.

    @PeterDaveHello
    Copy link
    Member Author

    @codex review

    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

    🤖 Fix all issues with AI agents
    In `@src/background/index.mjs`:
    - Around line 850-873: The listener added to
    Browser.webRequest.onBeforeSendHeaders logs the raw details.requestHeaders which
    can contain sensitive tokens/cookies; change the debug calls in the
    onBeforeSendHeaders handler (the callback that reads details.requestHeaders and
    sets modified) to avoid printing header values directly—instead log only header
    names or a redacted representation (e.g., map headers to names or to {name,
    value: '<REDACTED>'}) when calling console.debug('[background] Modified headers
    for Bing:', ...), and ensure any other debug of details.requestHeaders or
    headers uses that sanitized view rather than the raw headers array.
    
    🧹 Nitpick comments (1)
    src/background/index.mjs (1)

    988-1012: Manual regression testing reminder.

    Please re‑test popup flow, content‑script injection, and keyboard shortcuts after these changes. Based on learnings, core extension functionality should be validated after updates.

    Comment on lines 850 to +873
    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++) {
    const header = headers[i]
    if (!header || !header.name) {
    continue
    }
    const headerNameLower = header.name.toLowerCase()
    if (headerNameLower === 'origin') {
    header.value = 'https://www.bing.com'
    modified = true
    } else if (headerNameLower === 'referer') {
    header.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 }
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    ⚠️ Potential issue | 🟠 Major

    Avoid logging raw request headers (can leak cookies/tokens).

    headers may contain sensitive values (e.g., cookies/authorization). Please log only names or a redacted view.

    🔧 Safer logging
    -        if (modified) {
    -          console.debug('[background] Modified headers for Bing:', headers)
    -        }
    +        if (modified) {
    +          console.debug(
    +            '[background] Modified headers for Bing (names only):',
    +            headers.map((h) => h?.name).filter(Boolean),
    +          )
    +        }
    🤖 Prompt for AI Agents
    In `@src/background/index.mjs` around lines 850 - 873, The listener added to
    Browser.webRequest.onBeforeSendHeaders logs the raw details.requestHeaders which
    can contain sensitive tokens/cookies; change the debug calls in the
    onBeforeSendHeaders handler (the callback that reads details.requestHeaders and
    sets modified) to avoid printing header values directly—instead log only header
    names or a redacted representation (e.g., map headers to names or to {name,
    value: '<REDACTED>'}) when calling console.debug('[background] Modified headers
    for Bing:', ...), and ensure any other debug of details.requestHeaders or
    headers uses that sanitized view rather than the raw headers array.
    

    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 3 out of 3 changed files in this pull request and generated 9 comments.


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

    Comment on lines +679 to 682
    console.warn('[background] Rejecting FETCH message from untrusted sender:', sender)
    return [null, { message: 'Unauthorized sender' }]
    }

    Copy link

    Copilot AI Jan 26, 2026

    Choose a reason for hiding this comment

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

    The sender.id check compares against Browser.runtime.id, which is the extension's own ID. However, this check is insufficient because content scripts from the same extension will pass this check. Since FETCH can make arbitrary HTTP requests, this could be exploited if a malicious website can trigger the content script to send a FETCH message. Consider adding additional validation such as checking sender.tab to ensure the request comes from an expected tab, or implementing a whitelist of allowed URLs.

    Suggested change
    console.warn('[background] Rejecting FETCH message from untrusted sender:', sender)
    return [null, { message: 'Unauthorized sender' }]
    }
    console.warn('[background] Rejecting FETCH message from untrusted sender (id mismatch):', sender)
    return [null, { message: 'Unauthorized sender' }]
    }
    const senderUrl = sender?.url
    const senderTabId = sender?.tab?.id
    const extensionBaseUrl = Browser.runtime.getURL('')
    const isFromExtensionPage =
    !senderTabId &&
    typeof senderUrl === 'string' &&
    senderUrl.startsWith(extensionBaseUrl)
    if (!isFromExtensionPage) {
    console.warn('[background] Rejecting FETCH message from non-extension context:', sender)
    return [null, { message: 'Unauthorized context' }]
    }

    Copilot uses AI. Check for mistakes.
    Comment on lines +759 to +771
    setTimeout(() => {
    try {
    const loginContainer = document.querySelector('.user-info-container')
    if (!loginContainer) {
    console.warn('[content] Kimi login container not found, skipping auto-click.')
    return
    }
    console.log('[content] Clicking Kimi login container.')
    loginContainer.click()
    } catch (err_click) {
    console.error('[content] Error clicking Kimi login container:', err_click)
    }
    }, 1000)
    Copy link

    Copilot AI Jan 26, 2026

    Choose a reason for hiding this comment

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

    The Kimi login flow automatically clicks the login container after 1 second without user consent. This could be unexpected behavior for users and may violate user expectations about automated interactions. Additionally, the querySelector('.user-info-container') may not find the correct element or could match unintended elements. Consider either removing the auto-click behavior or adding a user confirmation step before automatically clicking login elements.

    Copilot uses AI. Check for mistakes.
    dockable={true}
    />,
    toolbarContainer,
    const createSelectionTools = async (toolbarContainerElement, selection) => {
    Copy link

    Copilot AI Jan 26, 2026

    Choose a reason for hiding this comment

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

    The toolbarContainer variable is module-scoped and nullable, but the deleteToolbar function sets it to null after removing it. However, in createSelectionTools, the parameter is named toolbarContainerElement but the module-level toolbarContainer is still used elsewhere. This could lead to confusion about which container variable is being used. Consider renaming the parameter to avoid shadowing concerns or making the naming more explicit.

    Copilot uses AI. Check for mistakes.
    Comment on lines +689 to +702
    if (!fetchInput.startsWith('https://') && !fetchInput.startsWith('http://')) {
    console.warn('[background] Rejecting FETCH for non-http(s) URL:', fetchInput)
    return [null, { message: 'Unsupported fetch protocol' }]
    }

    console.log('[background] Processing FETCH message for URL:', fetchInput)
    if (fetchInput.includes('bing.com')) {
    console.debug('[background] Fetching Bing access token for FETCH message.')
    const accessToken = await getBingAccessToken()
    await setUserConfig({ bingAccessToken: accessToken })
    }

    try {
    const response = await fetch(fetchInput, message.data?.init)
    Copy link

    Copilot AI Jan 26, 2026

    Choose a reason for hiding this comment

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

    The URL validation only checks that the URL starts with 'https://' or 'http://', but doesn't validate the actual URL format. A malformed URL string could still pass this check. Use URL constructor validation (which is done later) first, or wrap the URL string validation in a try-catch to ensure it's a valid URL before the protocol check.

    Suggested change
    if (!fetchInput.startsWith('https://') && !fetchInput.startsWith('http://')) {
    console.warn('[background] Rejecting FETCH for non-http(s) URL:', fetchInput)
    return [null, { message: 'Unsupported fetch protocol' }]
    }
    console.log('[background] Processing FETCH message for URL:', fetchInput)
    if (fetchInput.includes('bing.com')) {
    console.debug('[background] Fetching Bing access token for FETCH message.')
    const accessToken = await getBingAccessToken()
    await setUserConfig({ bingAccessToken: accessToken })
    }
    try {
    const response = await fetch(fetchInput, message.data?.init)
    let url
    try {
    url = new URL(fetchInput)
    } catch (e) {
    console.warn('[background] Rejecting FETCH for invalid URL:', fetchInput, e)
    return [null, { message: 'Invalid fetch URL' }]
    }
    if (url.protocol !== 'https:' && url.protocol !== 'http:') {
    console.warn('[background] Rejecting FETCH for non-http(s) URL:', fetchInput)
    return [null, { message: 'Unsupported fetch protocol' }]
    }
    const validatedUrl = url.toString()
    console.log('[background] Processing FETCH message for URL:', validatedUrl)
    if (validatedUrl.includes('bing.com')) {
    console.debug('[background] Fetching Bing access token for FETCH message.')
    const accessToken = await getBingAccessToken()
    await setUserConfig({ bingAccessToken: accessToken })
    }
    try {
    const response = await fetch(validatedUrl, message.data?.init)

    Copilot uses AI. Check for mistakes.
    }
    try {
    port.postMessage({
    error: `Connection to ChatGPT tab lost after ${RECONNECT_CONFIG.MAX_ATTEMPTS} attempts. Please refresh the page.`,
    Copy link

    Copilot AI Jan 26, 2026

    Choose a reason for hiding this comment

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

    The error message "Connection to ChatGPT tab lost after X attempts. Please refresh the page." is somewhat ambiguous about which page to refresh. Users might not know whether to refresh the ChatGPT tab, the current tab, or the extension popup. Consider making the error message more specific, e.g., "Connection to ChatGPT tab lost. Please refresh both the ChatGPT tab and this page."

    Suggested change
    error: `Connection to ChatGPT tab lost after ${RECONNECT_CONFIG.MAX_ATTEMPTS} attempts. Please refresh the page.`,
    error: `Connection to ChatGPT tab lost after ${RECONNECT_CONFIG.MAX_ATTEMPTS} attempts. Please refresh both the ChatGPT tab and this page.`,

    Copilot uses AI. Check for mistakes.
    Comment on lines 268 to +329
    document.addEventListener('mouseup', (e) => {
    if (toolbarContainer && toolbarContainer.contains(e.target)) return
    const selectionElement =
    window.getSelection()?.rangeCount > 0 &&
    window.getSelection()?.getRangeAt(0).endContainer.parentElement
    if (toolbarContainer && selectionElement && toolbarContainer.contains(selectionElement)) return

    deleteToolbar()
    setTimeout(async () => {
    const selection = window
    .getSelection()
    ?.toString()
    .trim()
    .replace(/^-+|-+$/g, '')
    if (selection) {
    let position
    try {
    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 (selectionElement && toolbarContainer?.contains(selectionElement)) {
    console.debug('[content] Mouseup selection is inside toolbar, ignoring.')
    return
    }

    const config = await getUserConfig()
    if (!config.selectionToolsNextToInputBox) position = { x: e.pageX + 20, y: e.pageY + 20 }
    else {
    const inputElement = selectionElement.querySelector('input, textarea')
    if (inputElement) {
    position = getClientPosition(inputElement)
    position = {
    x: position.x + window.scrollX + inputElement.offsetWidth + 50,
    y: e.pageY + 30,
    deleteToolbar()
    setTimeout(async () => {
    try {
    const selection = window
    .getSelection()
    ?.toString()
    .trim()
    .replace(/^-+|-+$/g, '')
    if (selection) {
    console.debug('[content] Text selected:', selection)
    let position

    const config = await getUserConfig()
    if (!config.selectionToolsNextToInputBox) {
    position = { x: e.pageX + 20, y: e.pageY + 20 }
    } else {
    const activeElement = document.activeElement
    const inputElement =
    selectionElement?.querySelector('input, textarea') ||
    (activeElement?.matches('input, textarea') ? activeElement : null)

    if (inputElement) {
    console.debug(
    '[content] Input element found for positioning toolbar:',
    inputElement,
    )
    const clientRect = getClientPosition(inputElement)
    position = {
    x: clientRect.x + window.scrollX + inputElement.offsetWidth + 50,
    y: e.pageY + 30,
    }
    } else {
    position = { x: e.pageX + 20, y: e.pageY + 20 }
    }
    }
    console.debug('[content] Toolbar position:', position)
    toolbarContainer = createElementAtPosition(position.x, position.y)
    await createSelectionTools(toolbarContainer, selection)
    } else {
    position = { x: e.pageX + 20, y: e.pageY + 20 }
    console.debug('[content] No text selected on mouseup.')
    }
    } catch (err) {
    console.error('[content] Error in mouseup setTimeout callback for selection tools:', err)
    }
    toolbarContainer = createElementAtPosition(position.x, position.y)
    await createSelectionTools(toolbarContainer, selection)
    }
    })
    }, 0)
    } catch (error) {
    console.error('[content] Error in mouseup listener for selection tools:', error)
    }
    Copy link

    Copilot AI Jan 26, 2026

    Choose a reason for hiding this comment

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

    This PR adds try-catch blocks around nearly every operation, but many of these simply log errors without proper recovery or user feedback. Over-aggressive error catching can mask real issues and make debugging harder. Consider being more selective about where to catch errors—catch them at boundaries where you can provide meaningful recovery or user feedback, rather than wrapping every operation. For example, the try-catch in mouseup/touchend listeners (lines 269-329) catches errors that should probably propagate or be handled more specifically.

    Copilot uses AI. Check for mistakes.
    Comment on lines +728 to +732
    const senderId = sender?.id
    if (!senderId || senderId !== Browser.runtime.id) {
    console.warn('[background] Rejecting GET_COOKIE message from untrusted sender:', sender)
    return null
    }
    Copy link

    Copilot AI Jan 26, 2026

    Choose a reason for hiding this comment

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

    Similar to the FETCH handler, the GET_COOKIE sender validation only checks sender.id against Browser.runtime.id. This means any content script from this extension can request cookies for any URL. Combined with the FETCH handler, a compromised content script on a malicious website could potentially access sensitive cookies. Consider restricting cookie access to specific trusted URLs or implementing additional sender validation.

    Copilot uses AI. Check for mistakes.
    Comment on lines +578 to +754
    const isKimiHost =
    location.hostname === 'kimi.moonshot.cn' ||
    location.hostname === 'kimi.com' ||
    location.hostname === 'www.kimi.com'
    if (isKimiHost) {
    console.log(`[content] On ${location.hostname}, 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 {
    const config = await getUserConfig()
    if (config.kimiMoonShotRefreshToken) {
    await setUserConfig({ kimiMoonShotRefreshToken: '' })
    console.log('[content] Kimi Moonshot refresh token cleared.')
    }
    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
    }

    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} ${resp.statusText}`,
    )
    }
    } catch (error) {
    console.error('[content] Error fetching access token from API:', error)
    }
    }
    } else {
    const resp = await fetch('https://chatgpt.com/api/auth/session')
    data = await resp.json().catch(() => ({}))

    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)
    }
    if (data && data.accessToken) {
    await setAccessToken(data.accessToken)
    console.log(data.accessToken)
    }

    async function getClaudeSessionKey() {
    console.debug('[content] getClaudeSessionKey called.')
    try {
    const sessionKey = await Browser.runtime.sendMessage({
    type: 'GET_COOKIE',
    data: { url: 'https://claude.ai/', name: 'sessionKey' },
    })
    console.debug(
    '[content] Claude session key from background:',
    sessionKey ? 'found' : 'not found',
    )
    return sessionKey
    } catch (error) {
    console.error('[content] Error in getClaudeSessionKey sending message:', error)
    return null
    }
    }

    async function prepareForForegroundRequests() {
    if (location.hostname !== 'chatgpt.com' || location.pathname === '/auth/login') return
    async function prepareForJumpBackNotification() {
    console.log('[content] Initializing jump back notification.')
    try {
    if (
    location.hostname === 'chatgpt.com' &&
    document.querySelector('button[data-testid=login-button]')
    ) {
    console.log('[content] ChatGPT login button found, user not logged in. Skipping jump back.')
    return
    }

    const userConfig = await getUserConfig()
    const url = new URL(window.location.href)
    if (url.searchParams.has('chatgptbox_notification')) {
    console.log('[content] chatgptbox_notification param found in URL.')

    if (
    !chatgptWebModelKeys.some((model) =>
    getApiModesStringArrayFromConfig(userConfig, true).includes(model),
    )
    )
    return
    if (location.hostname === 'claude.ai') {
    console.debug('[content] On claude.ai, checking login status.')
    let claudeSession = await getClaudeSessionKey()
    if (!claudeSession) {
    console.log('[content] Claude session key not found, waiting for it...')
    let promiseSettled = false
    let timerId = null
    let timeoutId = null
    const cleanup = () => {
    if (timerId) clearTimeout(timerId)
    if (timeoutId) clearTimeout(timeoutId)
    }

    // 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)
    // }
    // }

    await Browser.runtime.sendMessage({
    type: 'SET_CHATGPT_TAB',
    data: {},
    })
    try {
    await new Promise((resolve, reject) => {
    const poll = async () => {
    if (promiseSettled) return
    try {
    claudeSession = await getClaudeSessionKey()
    if (claudeSession && !promiseSettled) {
    promiseSettled = true
    cleanup()
    console.log('[content] Claude session key found after waiting.')
    resolve()
    return
    }
    } catch (err) {
    console.error('[content] Error polling for Claude session key:', err)
    const rawMessage =
    typeof err?.message === 'string' ? err.message : String(err ?? '')
    const errMsg = rawMessage.toLowerCase()
    const isNetworkError = /\bnetwork\b/.test(errMsg)
    const isPermissionError = /\bpermission\b/.test(errMsg)
    if ((isNetworkError || isPermissionError) && !promiseSettled) {
    promiseSettled = true
    cleanup()
    reject(new Error(`Failed to get Claude session key due to: ${rawMessage}`))
    return
    }
    }
    if (!promiseSettled) {
    timerId = setTimeout(poll, 500)
    }
    }

    registerPortListener(async (session, port) => {
    if (isUsingChatgptWebModel(session)) {
    const accessToken = await getChatGptAccessToken()
    await generateAnswersWithChatgptWebApi(port, session.question, session, accessToken)
    }
    })
    }
    poll()

    async function getClaudeSessionKey() {
    return Browser.runtime.sendMessage({
    type: 'GET_COOKIE',
    data: { url: 'https://claude.ai/', name: 'sessionKey' },
    })
    }
    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.')
    }
    }

    async function prepareForJumpBackNotification() {
    if (
    location.hostname === 'chatgpt.com' &&
    document.querySelector('button[data-testid=login-button]')
    ) {
    console.log('chatgpt not logged in')
    return
    }
    const isKimiHost =
    location.hostname === 'kimi.moonshot.cn' ||
    location.hostname === 'kimi.com' ||
    location.hostname === 'www.kimi.com'
    Copy link

    Copilot AI Jan 26, 2026

    Choose a reason for hiding this comment

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

    The isKimiHost check is duplicated in two places (lines 578-581 and lines 751-754) with identical logic. This violates the DRY (Don't Repeat Yourself) principle and makes maintenance harder. Consider extracting this check into a helper function or constant defined once at the module level.

    Copilot uses AI. Check for mistakes.
    'kimimoonshotrefreshtoken',
    'credential',
    'jwt',
    'session',
    Copy link

    Copilot AI Jan 26, 2026

    Choose a reason for hiding this comment

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

    The keyword 'session' is too broad and will redact non-sensitive session data (like conversation sessions, model sessions, etc.) that may be useful for debugging. This could make the extensive logging less useful. Consider removing 'session' from the list or making it more specific (e.g., 'sessionkey', 'sessionid') to avoid over-redaction.

    Suggested change
    'session',
    'sessionkey',
    'sessionid',

    Copilot uses AI. Check for mistakes.
    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: 79a3d29448

    ℹ️ 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".

    Comment on lines +988 to +992
    console.log('[content] Sending SET_CHATGPT_TAB message.')
    await Browser.runtime.sendMessage({
    type: 'SET_CHATGPT_TAB',
    data: {},
    })

    Choose a reason for hiding this comment

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

    P2 Badge Prevent chatgptTabId ping-pong across tabs

    In run() you now register a storage change listener that includes chatgptTabId and calls manageChatGptTabState() whenever it changes (around lines 875–892), and manageChatGptTabState() always sends SET_CHATGPT_TAB when this tab is configured for ChatGPT Web. With two or more chatgpt.com tabs open, any change to chatgptTabId will cause every tab to re‑assert itself, so the stored tab id can bounce back and forth indefinitely and the proxy may target the wrong tab. Consider removing chatgptTabId from the watched keys or short‑circuiting when the stored id already matches this tab.

    Useful? React with 👍 / 👎.

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

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants