Handle close-all-chats errors

[PeterDaveHello]

Summary

• Wait for active-tab lookup and close-message delivery in closeAllChats.
• Treat missing tabs or tab IDs as a no-op.
• Log browser API failures internally so fire-and-forget callers do not leak rejected promises.
• Add focused command dispatch, menu-tool, and side-panel regression coverage.

Why

The action previously returned before browser operations completed and did not handle rejected tab APIs. Failures could become unhandled rejections, while the close-all command path had no focused regression coverage.

Validation

• npm test (47 test files)
• npm run test:coverage on Node.js 22 (36.91% lines, 79.70% branches, 71.34% functions)
• npm run lint
• npm run build
• Required Chromium build artifacts verified
• git diff --check
• Manual browser smoke not run; it requires an interactive browser-extension session.

Summary by CodeRabbit

• Bug Fixes

  • Improved “close all chats” reliability when no active tab or tab ID is available.
  • Added error handling for failures while querying tabs or sending chat-close requests.
  • Prevented side panel actions when required tab or window information is missing.
  • Added clearer handling when side panel functionality is unavailable.

• Tests

  • Added coverage for command actions, chat closing, side panel opening, and failure scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7a69afd6-1505-4672-b9ca-bf5618c2e4ac

📥 Commits

Reviewing files that changed from the base of the PR and between 2419486 and 6a3499f.

📒 Files selected for processing (3)

• src/content-script/menu-tools/index.mjs
• tests/unit/background/commands.test.mjs
• tests/unit/content-script/menu-tools.test.mjs

---

📝 Walkthrough

Walkthrough

The menu-tools module updates internal import specifiers and makes closeAllChats handle missing tabs, missing IDs, and browser API failures. Unit tests cover menu actions and background command dispatch across success, failure, and no-op cases.

Changes

Menu and command behavior

Layer / File(s)	Summary
Menu action safety and tests src/content-script/menu-tools/index.mjs, tests/unit/content-script/menu-tools.test.mjs	Internal imports use .mjs specifiers, while closeAllChats now guards tab IDs, awaits message delivery, and logs failures. Tests cover close-chat sequencing, missing tabs, errors, and side-panel validation.
Background command dispatch coverage tests/unit/background/commands.test.mjs	Tests cover command action dispatch, prompt-command query and send failures, missing active tabs, and ignored unknown commands.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and matches the main change: improving close-all-chats error handling.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-code-review]

PR Summary by Qodo

Handle closeAllChats tab/query failures and await completion

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Await active-tab lookup and CLOSE_CHATS delivery in closeAllChats.
• Treat missing active tab IDs as a no-op; log browser API failures.
• Add unit tests for command dispatch and menu-tools error handling.

Diagram

graph TD
  A["src/background/commands.mjs"] --> B["menu-tools config"] --> C("closeAllChats action") --> D{{"Browser.tabs.query"}} --> E{{"Browser.tabs.sendMessage"}} --> F("Content script")
  B --> G("openSidePanel action") --> H{{"chrome.sidePanel.open"}}

  subgraph Legend
    direction LR
    _mod["Module"] ~~~ _act("Action") ~~~ _api{{"Browser API"}}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Extract a shared safeActiveTabMessage() helper

• ➕ Centralizes active-tab lookup, null checks, and error logging for all tab-message actions
• ➕ Reduces duplication if more actions adopt the same resilience pattern
• ➖ Slightly more indirection for a currently small code surface
• ➖ May be premature if closeAllChats is the only hardened action

2. Move error handling into background command dispatcher

• ➕ Guarantees fire-and-forget safety even if action implementations forget try/catch
• ➕ Keeps content-script actions simpler
• ➖ Harder to tailor error messages/context per action
• ➖ Doesn't help non-command call sites that invoke actions directly

Recommendation: The PR’s approach (make closeAllChats awaitable, no-op on missing tabs, catch/log browser API failures) is the most direct fix with minimal behavioral change. Consider extracting a shared active-tab messaging helper only if more actions need the same pattern.

Files changed (3) +281 / -5

Bug fix (1) +12 / -5

index.mjsMake closeAllChats await tab lookup/message and swallow tab API errors +12/-5

Make closeAllChats await tab lookup/message and swallow tab API errors

• Converts closeAllChats from a fire-and-forget promise chain to async/await, awaiting both the active-tab query and CLOSE_CHATS sendMessage. Treats missing tabs or tab IDs as a no-op and logs query/message failures to avoid leaking unhandled rejections. Also updates imports to explicit .mjs extensions for utility modules.

src/content-script/menu-tools/index.mjs

Tests (2) +269 / -0

commands.test.mjsAdd command-dispatch regression tests for closeAllChats and prompt commands +113/-0

Add command-dispatch regression tests for closeAllChats and prompt commands

• Introduces unit tests asserting command handlers call the correct menu-tool action and do so synchronously for openSidePanel. Adds coverage for prompt-command error paths: active-tab query failures, missing active tabs, sendMessage failures, and unknown command no-ops.

tests/unit/background/commands.test.mjs

menu-tools.test.mjsAdd menu-tool action tests for closeAllChats and openSidePanel behavior +156/-0

Add menu-tool action tests for closeAllChats and openSidePanel behavior

• Adds focused tests verifying closeAllChats waits for sendMessage completion, no-ops when tab/id is missing, and logs failures for query/sendMessage errors. Adds openSidePanel tests ensuring synchronous invocation, and rejection when the API or required identifiers are missing.

tests/unit/content-script/menu-tools.test.mjs

[gemini-code-assist]

Code Review

This pull request updates imports to use the .mjs extension and refactors the closeAllChats action to use async/await with error handling. It also adds comprehensive unit tests for background commands and menu tools. Feedback was provided regarding the closeAllChats action, which attempts to use background-only Browser.tabs APIs without checking the fromBackground flag, potentially causing runtime errors when executed from a content script context.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (3 files)

• src/content-script/menu-tools/index.mjs - 0 issues (changed lines 92-103: await query/sendMessage, no-op on missing tab id, catch/log failures)
• tests/unit/background/commands.test.mjs - 0 issues (new)
• tests/unit/content-script/menu-tools.test.mjs - 0 issues (new)

Notes

• The changed source (closeAllChats) is correct and an improvement over the previous fire-and-forget .then() chain. All real invocation paths (src/background/menus.mjs, src/background/commands.mjs) call menu actions with fromBackground=true, so Browser.tabs is available; the content script only handles CLOSE_CHATS messages and never invokes this action with fromBackground=false. An existing bot comment raised the fromBackground=false path, but that condition is pre-existing and not introduced by this PR.
• New tests follow the existing test harness conventions (__TEST_BROWSER_SHIM__, chrome.commands.onCommand._trigger) and correctly assert the new await/no-op/error-logging behavior.

---

_{Reviewed by hy3:free · Input: 66.6K · Output: 3.2K · Cached: 274.1K}

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

[PeterDaveHello]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

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.