feat(tab-awareness): detect active tab URL and gate live data in side panel [LCO-38] - #32
Conversation
… panel [LCO-38] - Export isTabOnClaude() from useDashboardData -- the single URL gate for all live-data loaders; takes a tabId, calls chrome.tabs.get, returns false on any error or non-Claude URL - Add isClaudeTab: boolean to DashboardData; defaults false until init resolves - Clear budget on non-Claude tab; keep today and conversations (historical) - Guard onStorageChanged budget reload with isClaudeTabRef so background alarms cannot re-populate the budget card while the user is on Gmail - Add onTabUpdated listener for URL changes within the same tab (navigate away from claude.ai without switching tabs -- onTabActivated never fires for this) - Stale-check in onTabActivated: discard async resolution if user switched again - Clear isClaudeTab and budget on tab close - App.tsx: render not-Claude banner between Today and Usage Budget when !isClaudeTab - dashboard.css: add .lco-dash-not-claude-banner (muted, italic, no border) - 15 new tests in tests/unit/tab-awareness.test.ts covering all URL and error paths
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds tab-awareness to the sidepanel dashboard: it detects whether the active Chrome tab is on Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ChromeAPI as Chrome Tabs API
participant Hook as useDashboardData
participant App as App Component
User->>ChromeAPI: Activate or navigate tab
ChromeAPI->>Hook: onActivated / onUpdated event (tabId)
Hook->>ChromeAPI: chrome.tabs.get(tabId)
ChromeAPI-->>Hook: { url: "...", id: tabId }
Hook->>Hook: isTabOnClaude(tabId) -> true/false
alt isClaudeTab == true
Hook->>Hook: loadBudget() (subscribe/storage reads)
Hook->>App: emit { isClaudeTab: true, budget: {...} }
App->>App: render Usage Budget card
else isClaudeTab == false
Hook->>Hook: setBudget(null)
Hook->>App: emit { isClaudeTab: false, budget: null }
App->>App: render not-Claude banner
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
entrypoints/sidepanel/hooks/useDashboardData.ts (1)
57-65: URL matching uses substring check.The
includes(CLAUDE_DOMAIN)approach works for the current use case but would also match subdomains likeapi.claude.aiand theoretically crafted URLs likehttps://not-claude.ai.example.com. For a side panel gate this is low risk, but if you want stricter matching, consider parsing the URL and checking the hostname.Optional: stricter hostname check
export async function isTabOnClaude(tabId: number): Promise<boolean> { try { const tab = await chrome.tabs.get(tabId); - return tab.url?.includes(CLAUDE_DOMAIN) ?? false; + if (!tab.url) return false; + try { + const { hostname } = new URL(tab.url); + return hostname === CLAUDE_DOMAIN || hostname.endsWith('.' + CLAUDE_DOMAIN); + } catch { + return false; + } } catch { - // Tab closed, extension lacks permission for that URL, or API unavailable. return false; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@entrypoints/sidepanel/hooks/useDashboardData.ts` around lines 57 - 65, isTabOnClaude currently uses a substring check (tab.url?.includes(CLAUDE_DOMAIN)) which can false‑positive on crafted subdomains; update isTabOnClaude to parse tab.url with the URL constructor and compare the parsed hostname to CLAUDE_DOMAIN (or optionally allow subdomain suffixes via hostname === CLAUDE_DOMAIN || hostname.endsWith(`.${CLAUDE_DOMAIN}`)), handle missing/invalid tab.url before parsing, and keep the existing try/catch so failures still return false; locate this logic in the isTabOnClaude function and replace the includes-based check with the hostname comparison.tests/unit/tab-awareness.test.ts (1)
116-145: Consider adding subdomain test coverage.The edge case tests are thorough. One optional enhancement: a test for subdomains like
api.claude.aiorconsole.claude.aiwould document the expected behavior sinceisTabOnClaudeusesincludes(CLAUDE_DOMAIN)which would match these. If subdomains should be treated differently, this test would catch a regression.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/tab-awareness.test.ts` around lines 116 - 145, Add a unit test in tests/unit/tab-awareness.test.ts that covers subdomains so behavior is explicit: call mockTabsGet.mockResolvedValueOnce(mockTab('https://api.claude.ai/path')) (or 'https://console.claude.ai') and assert await isTabOnClaude(tabId) returns true (since implementation uses includes(CLAUDE_DOMAIN)); if subdomains should instead be excluded, update isTabOnClaude to perform exact host matching against CLAUDE_DOMAIN and write the test to expect false accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@entrypoints/sidepanel/hooks/useDashboardData.ts`:
- Around line 57-65: isTabOnClaude currently uses a substring check
(tab.url?.includes(CLAUDE_DOMAIN)) which can false‑positive on crafted
subdomains; update isTabOnClaude to parse tab.url with the URL constructor and
compare the parsed hostname to CLAUDE_DOMAIN (or optionally allow subdomain
suffixes via hostname === CLAUDE_DOMAIN ||
hostname.endsWith(`.${CLAUDE_DOMAIN}`)), handle missing/invalid tab.url before
parsing, and keep the existing try/catch so failures still return false; locate
this logic in the isTabOnClaude function and replace the includes-based check
with the hostname comparison.
In `@tests/unit/tab-awareness.test.ts`:
- Around line 116-145: Add a unit test in tests/unit/tab-awareness.test.ts that
covers subdomains so behavior is explicit: call
mockTabsGet.mockResolvedValueOnce(mockTab('https://api.claude.ai/path')) (or
'https://console.claude.ai') and assert await isTabOnClaude(tabId) returns true
(since implementation uses includes(CLAUDE_DOMAIN)); if subdomains should
instead be excluded, update isTabOnClaude to perform exact host matching against
CLAUDE_DOMAIN and write the test to expect false accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5164e9d1-0399-4186-abe6-b3c45134d7b3
📒 Files selected for processing (4)
entrypoints/sidepanel/App.tsxentrypoints/sidepanel/dashboard.cssentrypoints/sidepanel/hooks/useDashboardData.tstests/unit/tab-awareness.test.ts
…L gate [LCO-38]
includes('claude.ai') false-positives on notclaude.ai and on URLs that contain
claude.ai as a query parameter. Extract isClaudeUrl() helper that uses the URL
constructor and compares the parsed hostname directly to CLAUDE_DOMAIN.
Apply the same fix to the onTabUpdated inline check which had the identical bug.
Add two tests that document the corrected behavior:
- notclaude.ai now returns false (was true with includes)
- api.claude.ai returns false (exact hostname only; no subdomain allowance)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
entrypoints/sidepanel/hooks/useDashboardData.ts (1)
215-228:⚠️ Potential issue | 🟠 MajorGuard
loadBudget()against stale completions.This function always commits its result after
await getUsageLimits(...). If the user leaves Claude while that await is in flight, Line 329 or Line 355 clears the budget, but the older request can still resolve later and repopulate live data on a non-Claude tab.Proposed fix
- const loadBudget = useCallback(async () => { + const loadBudget = useCallback(async (expectedTabId = tabIdRef.current) => { try { const orgId = orgIdRef.current; - if (!orgId) return; + if (!orgId || expectedTabId === null || !isClaudeTabRef.current) return; const limits = await getUsageLimits(orgId); + if ( + !isClaudeTabRef.current || + tabIdRef.current !== expectedTabId || + orgIdRef.current !== orgId + ) { + return; + } if (!limits) { setBudget(null); return; } setBudget(computeUsageBudget(limits, Date.now()));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@entrypoints/sidepanel/hooks/useDashboardData.ts` around lines 215 - 228, loadBudget currently always applies results after await and can overwrite newer state from other tabs; capture orgId into a local const (already done) and after each await (both when limits is falsy and before calling setBudget(computeUsageBudget(...))) verify that orgIdRef.current === orgId (and return early if not) so stale responses don't repopulate budget; apply this check around both the limits falsy branch and the setBudget call (functions to edit: loadBudget, using orgIdRef, getUsageLimits, setBudget, computeUsageBudget).
🧹 Nitpick comments (1)
entrypoints/sidepanel/hooks/useDashboardData.ts (1)
61-63: Trim the future-feature references from these docs.These JSDoc blocks describe patterns for features that do not exist in this file yet. The current contract is clear without the forward-looking guidance.
As per coding guidelines, "No speculative code, stubs, placeholders, or 'future-proofing' for features that do not exist yet".
Also applies to: 94-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@entrypoints/sidepanel/hooks/useDashboardData.ts` around lines 61 - 63, The JSDoc for the useDashboardData hook contains speculative "future-feature" wording (e.g., mentions of per-tab features like pre-submit estimates or delta tracking); remove those forward-looking sentences and trim the block to only describe the current contract: that useDashboardData is the single gate for loading live data such as Usage Budget. Update both JSDoc occurrences (the block around the top comment and the other block referenced nearby) to be concise and concrete, leaving no references to features that don't exist.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@entrypoints/sidepanel/hooks/useDashboardData.ts`:
- Around line 233-247: The init() function currently uses a truthy check on
tab.id which skips valid tabId 0; update the conditional to explicitly test for
null/undefined (e.g., if (tab?.id !== undefined && tab?.id !== null) or if
(typeof tab.id === 'number')) so tabIdRef.current is set for id 0,
isTabOnClaude(tab.id) and subsequent calls to applyIsClaudeTab,
loadActiveConversation and setBudget run correctly; adjust references in init()
where tab?.id is used (tabIdRef.current, isTabOnClaude, loadActiveConversation)
to rely on the explicit null/undefined check instead of a truthy check.
---
Outside diff comments:
In `@entrypoints/sidepanel/hooks/useDashboardData.ts`:
- Around line 215-228: loadBudget currently always applies results after await
and can overwrite newer state from other tabs; capture orgId into a local const
(already done) and after each await (both when limits is falsy and before
calling setBudget(computeUsageBudget(...))) verify that orgIdRef.current ===
orgId (and return early if not) so stale responses don't repopulate budget;
apply this check around both the limits falsy branch and the setBudget call
(functions to edit: loadBudget, using orgIdRef, getUsageLimits, setBudget,
computeUsageBudget).
---
Nitpick comments:
In `@entrypoints/sidepanel/hooks/useDashboardData.ts`:
- Around line 61-63: The JSDoc for the useDashboardData hook contains
speculative "future-feature" wording (e.g., mentions of per-tab features like
pre-submit estimates or delta tracking); remove those forward-looking sentences
and trim the block to only describe the current contract: that useDashboardData
is the single gate for loading live data such as Usage Budget. Update both JSDoc
occurrences (the block around the top comment and the other block referenced
nearby) to be concise and concrete, leaving no references to features that don't
exist.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 10a1b88a-5c1d-4d92-b31a-c1d9e7ba36fb
📒 Files selected for processing (2)
entrypoints/sidepanel/hooks/useDashboardData.tstests/unit/tab-awareness.test.ts
…m JSDoc [LCO-38] loadBudget: add orgIdRef.current === orgId check after getUsageLimits resolves; without this, a slow storage read from the previous org would overwrite the newly-cleared or newly-loaded budget state after an account switch or tab change. init(): replace truthy if (tab?.id) with typeof tab?.id === 'number' so tabId 0 is not silently skipped; Chrome assigns IDs from 1 in practice but the guard was logically wrong for a number field. JSDoc: remove speculative references to pre-submit estimates, delta tracking, and efficiency score -- features that do not exist yet; per CLAUDE.md no forward-looking wording in production code.
Problem
The side panel is window-scoped. When the user switches from a claude.ai tab to Gmail, GitHub, or any other page, the Usage Budget card continues showing the last-seen session percentages with no visual indicator that the data is stale. Two gaps:
budgetwas never cleared on tab switch.Solution
isTabOnClaude(tabId)-- the single gateExported from
useDashboardData.ts. Takes a tab ID, callschrome.tabs.get, checks the URL againstCLAUDE_DOMAIN. Returnsfalseon any error (tab closed, no permission, undefined URL). Every future live-data feature calls this before loading.isClaudeTab: booleaninDashboardDataAdded to the hook's return type with a JSDoc comment explaining the intended usage pattern for future features (pre-submit estimates, delta tracking, etc.).
Budget cleared on non-Claude tab
init(): gatesloadBudget()call ononClaudeonTabActivated: clears budget immediately when switching to a non-Claude tab; reloads when switching backonStorageChanged: guards budget reload withisClaudeTabRef-- prevents background alarms from silently re-populating the card while the user is on a non-Claude tabonTabUpdated: new listener for URL changes within the same tab (navigating away without switching tabs --onTabActivateddoes not fire for this)onTabRemoved: clearsisClaudeTaband budget when the tracked tab is closedHistorical data untouched
todayandconversationsare org-scoped and always visible, on Claude and non-Claude tabs alike.Stale async resolution guard
onTabActivatedis async (needs toawait isTabOnClaude()). If the user switches tabs faster than the API resolves, the stale result is discarded:if (tabIdRef.current !== info.tabId) return.App.tsx
Subtle banner between Today and Usage Budget sections when
!isClaudeTab:Usage Budget card receives
budget={null}(already handled by the hook) and renders its existing empty state.CSS
.lco-dash-not-claude-banner: muted, italic, no border, no icon. Informs without alarming. Respectsprefers-reduced-motion.Files changed
entrypoints/sidepanel/hooks/useDashboardData.tsisTabOnClaude(),isClaudeTabstate + ref, gate in all loaders,onTabUpdatedlistenerentrypoints/sidepanel/App.tsxisClaudeTab, render not-Claude bannerentrypoints/sidepanel/dashboard.css.lco-dash-not-claude-bannerstylestests/unit/tab-awareness.test.tsTest results
Acceptance criteria
isTabOnClaude()is the single gate -- no inline URL checks scattered across loadersbun run compile+bun run buildcleanSummary by CodeRabbit
New Features
Style
Tests