feat(storage): account isolation via organization UUID - #29
Merged
DevanshuNEU merged 8 commits intoApr 6, 2026
Merged
Conversation
All storage keys are now namespaced with the Claude organization UUID,
isolating conversation history, daily summaries, and weekly summaries
between accounts sharing the same browser.
Data pipeline:
- inject.ts extracts the org UUID from the API URL
(/organizations/{orgId}/chat_conversations/.../completion) and
attaches it as organizationId on TOKEN_BATCH and STREAM_COMPLETE
bridge payloads.
- Content script captures organizationId from bridge messages and
forwards it on RECORD_TURN, FINALIZE_CONVERSATION, SET_ACTIVE_CONV,
and GET_CONVERSATION messages to the background.
- Background writes activeOrg_{tabId} alongside activeConv_{tabId}
in session storage so the side panel knows which account to query.
Storage layer (lib/conversation-store.ts):
- All key constants replaced with functions that accept accountId:
convKey(accountId, convId), convIndexKey(accountId), etc.
- Every public function gains accountId as first parameter:
recordTurn, getConversation, listConversations, finalizeConversation,
computeDailySummary, getDailySummary, computeWeeklySummary, etc.
- Legacy read-through migration in getConversation: checks old global
key as fallback, copies to new scoped key on first read.
- extractOrganizationId() added alongside extractConversationId().
Background alarms:
- getActiveOrgIds() collects all known org IDs from session storage
(activeOrg_*) plus a persistent knownOrgIds set in local storage.
- computeDailySummary, computeWeeklySummary, pruneConversations alarms
now iterate over all known accounts.
Side panel:
- useDashboardData reads activeOrg_{tabId} from session storage.
- All queries pass the org ID through to the storage layer.
Message types:
- organizationId added to TokenBatchPayload, StreamCompletePayload,
RecordTurnMessage, FinalizeConversationMessage, GetConversationMessage,
SetActiveConvMessage.
- Bridge validation accepts optional organizationId string.
Tests: all 516 tests updated with TEST_ORG constant, all passing.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughOrganization-scoped conversation handling was added: org IDs are detected from Claude-related requests and propagated from inject → content → background, used to namespace storage keys and APIs, added per-org alarm handling, and implemented read-through migration from legacy unscoped records into per-org keys. Changes
Sequence DiagramsequenceDiagram
participant Inject as Inject (page)
participant Content as ContentScript
participant Background as BackgroundScript
participant Store as StorageLayer
participant Sidepanel as Sidepanel/Dashboard
Note right of Inject: extractOrgId(url) -> organizationId
Inject->>Content: ORGANIZATION_DETECTED {organizationId}
Content->>Background: SET_ACTIVE_CONV {conversationId, organizationId}
Content-->>Background: RECORD_TURN {organizationId, conversationId, tokens...}
Background->>Store: recordTurn(accountId, convId, ...)
Store-->>Background: ack
Background->>Background: set session keys activeConv_{tab}, activeOrg_{tab}
Sidepanel->>Background: getActiveOrgIds()
Background-->>Sidepanel: [orgA, orgB]
Sidepanel->>Store: listConversations(orgA,...)
Store-->>Sidepanel: org-scoped conversation list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
19 new tests covering the account isolation feature: extractOrganizationId (6 tests): - Extracts UUID from completion API URL - Lowercases the UUID - Handles query strings - Returns null for page URLs, empty strings, non-matching URLs Account isolation (5 tests): - Same conversation ID under two accounts produces separate records - listConversations returns only the queried account's data - Daily summaries are isolated per account (same date, different tokens) - pruneConversations only deletes from the target account - finalizeConversation only affects the target account Legacy data migration (3 tests): - Reads from old global key when scoped key does not exist - Copies legacy data to new scoped key on first read - New scoped key takes precedence over old global key Bridge validation: organizationId (5 tests): - TOKEN_BATCH with valid organizationId passes - TOKEN_BATCH without organizationId passes (backward compat) - TOKEN_BATCH with non-string organizationId fails - STREAM_COMPLETE with valid organizationId passes - STREAM_COMPLETE with non-string organizationId fails Total: 535 passing (was 516).
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
entrypoints/background.ts (2)
280-284:⚠️ Potential issue | 🟠 MajorMissing
activeOrg_storage onRECORD_TURN.The handler stores
activeConv_{tabId}but notactiveOrg_{tabId}. IfRECORD_TURNfires before or without aSET_ACTIVE_CONVmessage, the tab will have a conversation ID but no org ID. When the tab closes,cleanTabStorage(line 174) skips finalization becauseorgIdis undefined.Fix: Store both keys together
if (tabId !== undefined) { + const setData: Record<string, string> = { + [`activeConv_${tabId}`]: message.conversationId, + }; + if (message.organizationId) { + setData[`activeOrg_${tabId}`] = message.organizationId; + } browser.storage.session.set({ - [`activeConv_${tabId}`]: message.conversationId, + ...setData, }).catch(() => { /* non-critical */ }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@entrypoints/background.ts` around lines 280 - 284, In the RECORD_TURN message handler, you're only persisting [`activeConv_${tabId}`] but not the corresponding [`activeOrg_${tabId}`], which leads cleanTabStorage to skip finalization when orgId is missing; update the RECORD_TURN branch (the code that checks tabId !== undefined and calls browser.storage.session.set) to store both keys atomically by including [`activeOrg_${tabId}`]: message.orgId along with [`activeConv_${tabId}`]: message.conversationId so both conversation and org are set together.
195-198:⚠️ Potential issue | 🟡 MinorBug: Orphan cleanup does not include
activeOrg_keys.The regex excludes
activeOrg_from orphan detection. If the service worker misses a tab-close event (e.g., browser crash), staleactiveOrg_keys will accumulate in session storage.Fix: Add `activeOrg` to the regex alternation
- const match = key.match(/^(?:tabState|sessionCost|activeConv)_(\d+)$/); + const match = key.match(/^(?:tabState|sessionCost|activeConv|activeOrg)_(\d+)$/);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@entrypoints/background.ts` around lines 195 - 198, The orphan detection regex used to build orphanKeys (filtering keys from allData against activeIds) omits keys starting with "activeOrg_", so stale activeOrg_* entries aren't cleaned; update the regex in the orphanKeys filter (the key.match(...) call used to compute orphanKeys) to include "activeOrg" in the alternation (e.g., add activeOrg alongside tabState, sessionCost, activeConv) so match[1] still captures the id and orphan entries for activeOrg_* are detected and removed.
🧹 Nitpick comments (2)
lib/conversation-store.ts (1)
279-301: Legacy migration preserves old data for rollback safety.The read-through migration copies data from the legacy key to the new scoped key but does not delete the old key. This is a reasonable approach for safe rollout since it allows rollback without data loss. However, consider adding a future cleanup mechanism to remove legacy keys after sufficient migration time.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/conversation-store.ts` around lines 279 - 301, getConversation currently migrates legacy records by copying from legacyConvKey(id) to the new convKey(accountId, id) and updating the index but leaves the old legacy key in place; add a cleanup mechanism so legacy keys are removed after successful migration (or scheduled for later deletion) to avoid stale duplicates: after successful store().set({ [key]: oldRecord }) and addToIndex(convIndexKey(accountId), id) either delete the legacy entry via store().delete(legacyConvKey(id)) or enqueue a background task for deferred deletion, and expose a helper like scheduleLegacyCleanup(id) to centralize that logic and make the cleanup policy configurable.entrypoints/sidepanel/hooks/useDashboardData.ts (1)
130-144: Storage change listener should check for org-scoped key patterns.The listener checks
k.startsWith('conv:')which will match both legacy keys (conv:{convId}) and new org-scoped keys (conv:{orgId}:{convId}). This is acceptable but consider that after full migration, only org-scoped keys will exist. TheconvIndexcheck on line 132 only matches the legacy global key, not org-scopedconvIndex:{orgId}.Consider updating the index key check
- const hasConvChange = keys.some(k => k.startsWith('conv:') || k === 'convIndex'); + const hasConvChange = keys.some(k => k.startsWith('conv:') || k.startsWith('convIndex'));This would also trigger on org-scoped index changes like
convIndex:org-123.🤖 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 130 - 144, The storage-change listener currently only treats the legacy global index key 'convIndex' specially; update the index check so it also matches org-scoped index keys (e.g., 'convIndex:{orgId}') by changing the test used in the hasConvChange branch to consider keys that equal 'convIndex' OR start with 'convIndex:'. Locate the block using keys.some(...) alongside loadConversations, loadActiveConversation, loadToday and tabIdRef, and replace the existing convIndex equality check with a startsWith-aware check so org-scoped index updates trigger the same refresh behavior.
🤖 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/background.ts`:
- Around line 374-404: The alarm handlers currently call getActiveOrgIds()
without handling rejections, causing unhandled promise rejections if that call
fails; update each branch (alarm.name === 'computeDailySummary',
'computeWeeklySummary', and 'pruneOldData') to add a .catch on the
getActiveOrgIds() promise and log the error (include context such as which alarm
failed) instead of leaving it unhandled, while keeping the existing per-org
.catch for computeDailySummary, computeWeeklySummary, and pruneConversations to
handle per-org errors.
- Around line 174-176: The current guard (if (convId && orgId)) skips
finalizeConversation when convId exists but orgId is missing, leaving
conversations unfinalized; update the block so that when convId is present but
orgId is falsy you attempt a fallback org ID (e.g., derive from activeOrg_
state, a stored default org, or a getDefaultOrgId helper) and call
finalizeConversation(orgFallback, convId), and if no fallback is available emit
a processLogger.warn/console.warn indicating missing activeOrg_ and the convId
so the issue is visible; ensure the finalizeConversation call remains wrapped
with .catch(() => { /* non-critical */ }) to preserve non-blocking behavior.
In `@entrypoints/claude-ai.content.ts`:
- Around line 114-120: Currently the initial restore calls
fetchStoredRecord(currentOrgId, currentConversationId) even when currentOrgId is
null, causing legacy unscoped records to be missed; change the logic around
browser.runtime.sendMessage/SET_ACTIVE_CONV so you only call fetchStoredRecord
when currentOrgId is non-null (i.e., defer restore on initial load), and add the
same restore call into the bridge message handler that sets currentOrgId so
legacy records are fetched once the bridge supplies the org id (ensure you
update the handler that sets currentOrgId/currentConversationId to invoke
fetchStoredRecord when appropriate).
---
Outside diff comments:
In `@entrypoints/background.ts`:
- Around line 280-284: In the RECORD_TURN message handler, you're only
persisting [`activeConv_${tabId}`] but not the corresponding
[`activeOrg_${tabId}`], which leads cleanTabStorage to skip finalization when
orgId is missing; update the RECORD_TURN branch (the code that checks tabId !==
undefined and calls browser.storage.session.set) to store both keys atomically
by including [`activeOrg_${tabId}`]: message.orgId along with
[`activeConv_${tabId}`]: message.conversationId so both conversation and org are
set together.
- Around line 195-198: The orphan detection regex used to build orphanKeys
(filtering keys from allData against activeIds) omits keys starting with
"activeOrg_", so stale activeOrg_* entries aren't cleaned; update the regex in
the orphanKeys filter (the key.match(...) call used to compute orphanKeys) to
include "activeOrg" in the alternation (e.g., add activeOrg alongside tabState,
sessionCost, activeConv) so match[1] still captures the id and orphan entries
for activeOrg_* are detected and removed.
---
Nitpick comments:
In `@entrypoints/sidepanel/hooks/useDashboardData.ts`:
- Around line 130-144: The storage-change listener currently only treats the
legacy global index key 'convIndex' specially; update the index check so it also
matches org-scoped index keys (e.g., 'convIndex:{orgId}') by changing the test
used in the hasConvChange branch to consider keys that equal 'convIndex' OR
start with 'convIndex:'. Locate the block using keys.some(...) alongside
loadConversations, loadActiveConversation, loadToday and tabIdRef, and replace
the existing convIndex equality check with a startsWith-aware check so
org-scoped index updates trigger the same refresh behavior.
In `@lib/conversation-store.ts`:
- Around line 279-301: getConversation currently migrates legacy records by
copying from legacyConvKey(id) to the new convKey(accountId, id) and updating
the index but leaves the old legacy key in place; add a cleanup mechanism so
legacy keys are removed after successful migration (or scheduled for later
deletion) to avoid stale duplicates: after successful store().set({ [key]:
oldRecord }) and addToIndex(convIndexKey(accountId), id) either delete the
legacy entry via store().delete(legacyConvKey(id)) or enqueue a background task
for deferred deletion, and expose a helper like scheduleLegacyCleanup(id) to
centralize that logic and make the cleanup policy configurable.
🪄 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: ed531336-7cd7-4cd1-ac89-19c9505db9c2
📒 Files selected for processing (9)
entrypoints/background.tsentrypoints/claude-ai.content.tsentrypoints/inject.tsentrypoints/sidepanel/hooks/useDashboardData.tslib/bridge-validation.tslib/conversation-store.tslib/message-types.tstests/unit/conversation-dna.test.tstests/unit/conversation-store.test.ts
The initial SET_ACTIVE_CONV fires at page load before any API call, so
organizationId is null. The background never writes activeOrg_{tabId},
and the side panel cannot scope its queries. Result: dashboard shows
all zeros and "No conversations yet" despite the overlay working fine.
Fix: when the content script receives the first bridge message carrying
an organizationId (from TOKEN_BATCH or STREAM_COMPLETE), re-send
SET_ACTIVE_CONV with the now-known org ID. The background writes
activeOrg_{tabId}, the side panel reads it, and all queries are scoped.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/conversation-store.test.ts (1)
612-638: Also assert that migration makes the conversation discoverable.This only verifies the copied record.
getConversation()also updates the scoped index inlib/conversation-store.ts:279-301; if that regresses,listConversations(TEST_ORG, ...)breaks while this test still passes.Suggested assertion
// New scoped key should now exist. const scopedKey = `conv:${TEST_ORG}:migrate-me`; expect(mockStore._raw[scopedKey]).toBeDefined(); expect((mockStore._raw[scopedKey] as ConversationRecord).totalInputTokens).toBe(100); + expect(await listConversations(TEST_ORG, 10)).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: 'migrate-me' }), + ]), + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/conversation-store.test.ts` around lines 612 - 638, The test currently only asserts the legacy record was copied to the new scoped key but doesn't verify the migration updated the scoped index, so add an assertion that the migrated conversation is discoverable via the listing API: after calling getConversation(TEST_ORG, 'migrate-me') call listConversations(TEST_ORG, ...) (or the existing helper used in tests) and assert the returned list includes the conversation id 'migrate-me' (or the scoped key), ensuring the code paths in getConversation and the scoped index update (the logic referenced around getConversation/listConversations) are exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/conversation-store.test.ts`:
- Around line 496-499: The test for extractOrganizationId is using a non-UUID
fragment ("aabb0042-ccdd-eeff") which allows partial hex/hyphen matches; update
the test to use a full valid UUID string (e.g., 8-4-4-4-12 format) in the URL
passed to extractOrganizationId and assert that exact UUID is returned, and add
a negative case asserting that a partial fragment does NOT return a value (or
returns null/undefined) to prevent accepting malformed org IDs; reference the
extractOrganizationId function in your changes.
---
Nitpick comments:
In `@tests/unit/conversation-store.test.ts`:
- Around line 612-638: The test currently only asserts the legacy record was
copied to the new scoped key but doesn't verify the migration updated the scoped
index, so add an assertion that the migrated conversation is discoverable via
the listing API: after calling getConversation(TEST_ORG, 'migrate-me') call
listConversations(TEST_ORG, ...) (or the existing helper used in tests) and
assert the returned list includes the conversation id 'migrate-me' (or the
scoped key), ensuring the code paths in getConversation and the scoped index
update (the logic referenced around getConversation/listConversations) are
exercised.
🪄 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: 1d972806-31f6-40a4-9d1d-e416541f05b8
📒 Files selected for processing (3)
entrypoints/claude-ai.content.tstests/unit/bridge-validation.test.tstests/unit/conversation-store.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- entrypoints/claude-ai.content.ts
listConversations now checks the legacy global convIndex when the account-scoped index is empty. If legacy entries exist, all records are copied to account-scoped keys and added to the new index in one pass. This ensures HISTORY shows pre-migration conversations. Side panel init order changed: loadConversations runs before loadToday so the bulk migration completes before the daily summary tries to aggregate from the (now-populated) account-scoped conversation records.
Three changes to handle logout and account switching:
1. background.ts: tabs.onUpdated listener detects when a tab navigates
away from claude.ai (logout redirect to accounts.google.com). Clears
activeConv_{tabId} and activeOrg_{tabId} from session storage so the
side panel knows the account context is gone.
2. useDashboardData: loadActiveConversation now detects when the org ID
changes (account switch) or is cleared (logout). On logout: resets
orgIdRef and clears today/conversations/activeConv state to empty.
On account switch: re-runs loadConversations and loadToday with the
new org scope.
3. Live subscription: now listens for activeOrg_ changes in session
storage (not just activeConv_), so the dashboard refreshes when the
background clears the org key on logout.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
entrypoints/background.ts (1)
268-277:⚠️ Potential issue | 🟠 MajorReject scoped writes until
organizationIdis resolved.The content and bridge path can still produce
organizationId: nullorundefinedbefore the first org-bearing message arrives. Passing that straight intorecordTurn()orfinalizeConversation()creates pseudo-scoped keys likeconv:null:*instead of isolating data per account. Fail fast here when the org is missing.Suggested guard at the message boundary
if (message.type === 'RECORD_TURN') { const tabId = sender.tab?.id; + if (!message.organizationId) { + sendResponse({ ok: false }); + return false; + } recordTurn(message.organizationId, message.conversationId, { inputTokens: message.inputTokens, outputTokens: message.outputTokens, model: message.model, @@ if (message.type === 'FINALIZE_CONVERSATION') { + if (!message.organizationId) { + sendResponse({ ok: false }); + return false; + } finalizeConversation(message.organizationId, message.conversationId) .then(() => sendResponse({ ok: true }))Also applies to: 294-301
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@entrypoints/background.ts` around lines 268 - 277, The message handler must reject scoped writes when organizationId is missing: in the RECORD_TURN branch (where recordTurn(...) is called) add a guard that checks message.organizationId is non-null/undefined and if missing, log or ignore and return early to avoid creating keys like conv:null:*, and apply the same early-return guard in the finalizeConversation branch (the place that calls finalizeConversation(...)) so no scoped writes occur until an org-bearing message has been resolved.
🤖 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/background.ts`:
- Around line 354-361: The onUpdated listener that checks changeInfo.url
currently only removes activeConv_{tabId} and activeOrg_{tabId}; change it to
call the centralized per-tab cleanup (e.g., cleanTabStorage(tabId)) or a new
helper that first finalizes the conversation and then clears all per-tab keys
(activeConv_, activeOrg_, tabState_{tabId}, sessionCost_{tabId}, etc.) so
sessionCost_ and tabState_ do not persist across navigations; update the
browser.tabs.onUpdated callback to invoke cleanTabStorage(tabId) (or the
extracted helper) instead of removing only the two keys.
- Around line 321-329: When handling SetActiveConvMessage in
entrypoints/background.ts, avoid leaving a stale activeOrg_${tabId} when
message.conversationId is set but message.organizationId is null; update the
branch that uses convKey and orgKey so that if message.conversationId exists you
always set activeConv_${tabId} and explicitly remove activeOrg_${tabId} when
message.organizationId is null (otherwise set it when present), using
browser.storage.session.set/remove (preserving the existing .catch(() => {})
behavior) so the panel won't continue querying the new conversation under the
old org.
In `@lib/conversation-store.ts`:
- Around line 407-423: The current bulk migration loop that reads
LEGACY_CONV_INDEX_KEY and copies every legacy id into the first accountId breaks
org isolation; instead, remove the bulk copy and implement deferred, per-record
migration: stop iterating legacyIndex in the listConversations path and leave
legacy entries in place, then change the code paths that read individual
conversations (e.g., the function that calls store().get(legacyConvKey(id)) or
getConversation) to migrate a single legacy record only when its org provenance
is known or matches the current accountId — use legacyConvKey and readIndex to
locate the record, inspect the record metadata for org/account ownership, and
only call store().set({ [convKey(accountId, id)]: record }) and
addToIndex(convIndexKey(accountId), id) when the record truly belongs to that
account; alternatively move legacy records to a neutral legacy bucket instead of
importing them into an account index to prevent double-imports.
---
Outside diff comments:
In `@entrypoints/background.ts`:
- Around line 268-277: The message handler must reject scoped writes when
organizationId is missing: in the RECORD_TURN branch (where recordTurn(...) is
called) add a guard that checks message.organizationId is non-null/undefined and
if missing, log or ignore and return early to avoid creating keys like
conv:null:*, and apply the same early-return guard in the finalizeConversation
branch (the place that calls finalizeConversation(...)) so no scoped writes
occur until an org-bearing message has been resolved.
🪄 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: fc6a5b22-e4f3-4fbf-9717-f85d27d0cda7
📒 Files selected for processing (3)
entrypoints/background.tsentrypoints/sidepanel/hooks/useDashboardData.tslib/conversation-store.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- entrypoints/sidepanel/hooks/useDashboardData.ts
The org ID was only extracted from completion endpoints, which meant
Saar was blind to the account until the user sent their first message.
Between login and first message, the side panel showed stale data from
the previous account.
Fix: inject.ts now watches ALL fetches to /api/organizations/ (not just
completions). Claude.ai makes dozens of API calls on page load:
conversations list, settings, user info, etc. The first one that hits
/api/organizations/{orgId}/ triggers ORGANIZATION_DETECTED, a new
lightweight bridge message that carries just the org ID.
The content script handles ORGANIZATION_DETECTED by setting currentOrgId
and immediately re-sending SET_ACTIVE_CONV with the org ID. The side
panel picks up the org change via its session storage listener and
scopes all queries to the correct account.
Timeline:
- Before: org ID available after first user message (~seconds to minutes)
- After: org ID available within milliseconds of page load (first API call)
The TOKEN_BATCH/STREAM_COMPLETE fallback path is kept for robustness in
case ORGANIZATION_DETECTED is missed (inject.ts loaded late, etc.).
- background.ts: add .catch() on getActiveOrgIds() in all three alarm handlers
- background.ts: log warning in cleanTabStorage when convId exists but orgId missing
- background.ts: RECORD_TURN now stores activeOrg_{tabId} alongside activeConv_{tabId}
- background.ts: RECORD_TURN guards against missing organizationId
- background.ts: orphan cleanup regex now includes activeOrg_ keys
- background.ts: onUpdated calls cleanTabStorage (finalize + full cleanup) on navigation away
- background.ts: SET_ACTIVE_CONV removes stale activeOrg_ when organizationId is null
- content: fetchStoredRecord deferred until org ID is known; ORGANIZATION_DETECTED retries restore
- conversation-store: delete legacy key after per-record migration in getConversation
- conversation-store: listConversations reads legacy data without bulk-migrating to first accountId
- useDashboardData: convIndex check uses startsWith to catch scoped convIndex:{orgId} keys
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 (2)
entrypoints/claude-ai.content.ts (1)
143-155:⚠️ Potential issue | 🟠 MajorAdd the missing
event.sourcebridge check.This listener currently validates origin, namespace, token, and schema, but it still skips the
sourcelayer required for the Claude bridge.Suggested fix
window.addEventListener('message', (event) => { + if (event.source !== window) return; if (event.origin !== window.location.origin) return; if (!event.data || event.data.namespace !== LCO_NAMESPACE) return; if (event.data.token !== sessionToken) return; if (!isValidBridgeSchema(event.data)) return;As per coding guidelines, "Validate all incoming postMessages with 5 layers: origin, source, namespace LCO_V1, session token, and schema validation; drop messages that fail any check".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@entrypoints/claude-ai.content.ts` around lines 143 - 155, The message event listener (window.addEventListener callback) is missing the required event.source check: add a guard that ensures event.source === window to validate the Claude bridge sender before proceeding (in addition to existing checks against LCO_NAMESPACE, sessionToken, isValidBridgeSchema, and origin). Update the listener where msg is derived (and before casting to LcoBridgeMessage) to early-return when event.source is not the expected window reference so all five validation layers (origin, source, namespace LCO_NAMESPACE, sessionToken, and isValidBridgeSchema) are enforced.entrypoints/sidepanel/hooks/useDashboardData.ts (1)
69-117:⚠️ Potential issue | 🟠 MajorUse the freshly read
activeOrg_value as the source of truth.When the tab gets its first org (
'' -> uuid),orgChangedstays false, sotodayandconversationsnever reload. The inverse edge also bites: ifactiveConv_exists whileactiveOrg_has been cleared, this code keepsorgIdRef.currentfrom the previous account and can query the wrong scope. Base the guard and refresh logic onorgIdfrom session storage, clear the ref immediately when it is absent, and treat any org transition as a refresh.Suggested fix
const convId = result[cKey] as string | undefined; const orgId = result[oKey] as string | undefined; - // Detect account switch or logout: org ID changed or was cleared. const prevOrg = orgIdRef.current; - if (orgId) { - orgIdRef.current = orgId; - } - - // Account changed (switched accounts or logged out and back in). - // Re-fetch history and today for the new account scope. - const orgChanged = prevOrg !== '' && orgId !== undefined && orgId !== prevOrg; + const nextOrg = orgId ?? ''; + const orgChanged = nextOrg !== prevOrg; + orgIdRef.current = nextOrg; - if (!convId || !orgIdRef.current) { + if (!convId || !nextOrg) { setActiveConv(null); setActiveHealth(null); - // Org cleared (logout): reset dashboard to empty state. - if (!orgId && prevOrg) { - orgIdRef.current = ''; + if (orgChanged) { setToday(null); setConversations([]); } return; } - const conv = await getConversation(orgIdRef.current, convId); + const conv = await getConversation(nextOrg, convId); setActiveConv(conv); @@ // Account switched: reload history and today for the new org. if (orgChanged) { - loadConversations(); - loadToday(); + await Promise.all([loadConversations(), loadToday()]); }🤖 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 69 - 117, The code is using orgIdRef.current instead of the freshly read orgId from session storage which prevents reloads on first-login and can query the wrong org after logout; update the logic in the block that reads cKey/oKey so that you: treat the session-read orgId as the source of truth (assign orgIdRef.current = orgId immediately when orgId is present and clear it to '' immediately when orgId is absent), compute orgChanged by comparing prevOrg to the freshly read orgId (not orgIdRef.current), use orgId (or early-return if absent) when calling getConversation, and trigger loadConversations/loadToday whenever prevOrg !== orgId to ensure any org transition reloads history and today; keep existing calls to setActiveConv, setActiveHealth, setToday, and setConversations but base their control flow on the session-read orgId and convId.
♻️ Duplicate comments (1)
entrypoints/claude-ai.content.ts (1)
151-200:⚠️ Potential issue | 🟠 MajorExtract a shared hydration path and use it in the fallback branch too.
ORGANIZATION_DETECTEDrestores stored totals, but theTOKEN_BATCH/STREAM_COMPLETEfallback only setscurrentOrgId. If early org detection is missed, the first resumed turn is still computed and recorded from a zero baseline. Pull the restore sequence into an orchestrator helper and run it before processing the first fallback message.Based on learnings, "Applies to entrypoints/claude-ai.content.ts : Message bridge (postMessage listener, validation, forwarding) in content script must remain a thin relay with no business logic; keep it separate from orchestrator section".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@entrypoints/claude-ai.content.ts` around lines 151 - 200, The restore-on-org-detection logic in the ORGANIZATION_DETECTED branch (the sequence that calls fetchStoredRecord, updates cumulativeInput/cumulativeOutput/cumulativeCost, calls applyRestoredConversation and buildConvStateFromRecord, computes health via computeHealthScore, sets state/convState and calls overlay.render) must be extracted into a single helper (e.g., restoreConversationForOrg or similar) and invoked from both the ORGANIZATION_DETECTED path and the fallback that checks 'organizationId' in msg (TOKEN_BATCH/STREAM_COMPLETE fallback) before proceeding; ensure the helper accepts currentOrgId and currentConversationId and preserves the navGeneration check, returns/updates state/convState/cumulative* values, and keep the message-bridge thin by only calling that orchestrator helper from these branches and not duplicating the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/conversation-store.ts`:
- Around line 107-122: The scoped key builders (convKey, convIndexKey, dailyKey,
dailyIndexKey, weeklyKey, weeklyIndexKey) currently accept empty accountId and
produce keys like "conv::id"; update each builder to validate the accountId and
reject empty/missing values at the boundary (e.g., if accountId is falsy or
empty string throw an Error or assert) so callers cannot create scoped keys for
the empty-org bucket; keep the legacy key builders (legacyConvKey,
LEGACY_CONV_INDEX_KEY, legacyDailyKey, LEGACY_DAILY_INDEX_KEY) unchanged for
read-through migration.
---
Outside diff comments:
In `@entrypoints/claude-ai.content.ts`:
- Around line 143-155: The message event listener (window.addEventListener
callback) is missing the required event.source check: add a guard that ensures
event.source === window to validate the Claude bridge sender before proceeding
(in addition to existing checks against LCO_NAMESPACE, sessionToken,
isValidBridgeSchema, and origin). Update the listener where msg is derived (and
before casting to LcoBridgeMessage) to early-return when event.source is not the
expected window reference so all five validation layers (origin, source,
namespace LCO_NAMESPACE, sessionToken, and isValidBridgeSchema) are enforced.
In `@entrypoints/sidepanel/hooks/useDashboardData.ts`:
- Around line 69-117: The code is using orgIdRef.current instead of the freshly
read orgId from session storage which prevents reloads on first-login and can
query the wrong org after logout; update the logic in the block that reads
cKey/oKey so that you: treat the session-read orgId as the source of truth
(assign orgIdRef.current = orgId immediately when orgId is present and clear it
to '' immediately when orgId is absent), compute orgChanged by comparing prevOrg
to the freshly read orgId (not orgIdRef.current), use orgId (or early-return if
absent) when calling getConversation, and trigger loadConversations/loadToday
whenever prevOrg !== orgId to ensure any org transition reloads history and
today; keep existing calls to setActiveConv, setActiveHealth, setToday, and
setConversations but base their control flow on the session-read orgId and
convId.
---
Duplicate comments:
In `@entrypoints/claude-ai.content.ts`:
- Around line 151-200: The restore-on-org-detection logic in the
ORGANIZATION_DETECTED branch (the sequence that calls fetchStoredRecord, updates
cumulativeInput/cumulativeOutput/cumulativeCost, calls applyRestoredConversation
and buildConvStateFromRecord, computes health via computeHealthScore, sets
state/convState and calls overlay.render) must be extracted into a single helper
(e.g., restoreConversationForOrg or similar) and invoked from both the
ORGANIZATION_DETECTED path and the fallback that checks 'organizationId' in msg
(TOKEN_BATCH/STREAM_COMPLETE fallback) before proceeding; ensure the helper
accepts currentOrgId and currentConversationId and preserves the navGeneration
check, returns/updates state/convState/cumulative* values, and keep the
message-bridge thin by only calling that orchestrator helper from these branches
and not duplicating the logic.
🪄 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: 652534b8-1717-4b4c-a3c5-2bd6e9b3a977
📒 Files selected for processing (7)
entrypoints/background.tsentrypoints/claude-ai.content.tsentrypoints/inject.tsentrypoints/sidepanel/hooks/useDashboardData.tslib/bridge-validation.tslib/conversation-store.tslib/message-types.ts
✅ Files skipped from review due to trivial changes (1)
- entrypoints/background.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/bridge-validation.ts
- entrypoints/inject.ts
- lib/message-types.ts
- conversation-store: key builders validate accountId and throw on empty string preventing silent writes to the empty-org bucket (conv::id, convIndex:, etc.) - content: add event.source === window as layer 2 of the bridge message listener; the check was present on the LCO_TOKEN_REQ listener but missing from the main bridge, allowing same-origin iframes to pass the origin check - content: extract restore logic into scheduleConversationRestore helper; now called from both ORGANIZATION_DETECTED and the TOKEN_BATCH/STREAM_COMPLETE fallback so state is restored regardless of which path sets the org ID first - useDashboardData: treat session-read orgId as the source of truth; assign orgIdRef.current = orgId ?? '' immediately (not conditionally) and compute orgChanged as prevOrg !== (orgId ?? '') so first-login transitions (prevOrg '' to real org) correctly trigger loadConversations and loadToday; use orgId directly in getConversation call for type safety
There was a problem hiding this comment.
🧹 Nitpick comments (1)
entrypoints/sidepanel/hooks/useDashboardData.ts (1)
134-138: Comment is misleading:listConversationsdoes not trigger bulk migration.The comment states that
loadConversationstriggers bulk legacy migration, butlistConversationsexplicitly avoids bulk migration (per lines 411-415 inconversation-store.ts). It reads legacy records directly without migrating them. Per-record migration only happens ingetConversationwhen individual conversations are accessed.The sequential await is still correct for initialization ordering, but the comment should reflect the actual behavior.
Suggested comment fix
- // loadConversations first: it triggers bulk legacy migration if - // the account-scoped index is empty. loadToday depends on migrated - // conversation records to compute the daily summary correctly. + // loadConversations before loadToday: ensures the conversation list + // is populated before computing daily summary. Note: legacy records + // are read directly without migration; per-record migration happens + // in getConversation when individual conversations are accessed.🤖 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 134 - 138, Update the misleading comment above the sequential awaits: state that loadConversations (which calls listConversations) does NOT perform a bulk legacy migration but reads legacy records in place, and that per-record migration occurs in getConversation when individual conversations are accessed; keep the sequential awaits (await loadConversations(); await loadToday();) because loadToday depends on conversation data being available, but remove the claim that loadConversations triggers a bulk migration and reference the functions loadConversations, listConversations, getConversation, and loadToday in the updated comment.
🤖 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 134-138: Update the misleading comment above the sequential
awaits: state that loadConversations (which calls listConversations) does NOT
perform a bulk legacy migration but reads legacy records in place, and that
per-record migration occurs in getConversation when individual conversations are
accessed; keep the sequential awaits (await loadConversations(); await
loadToday();) because loadToday depends on conversation data being available,
but remove the claim that loadConversations triggers a bulk migration and
reference the functions loadConversations, listConversations, getConversation,
and loadToday in the updated comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 43af39f1-6895-4497-9dac-d571465f389c
📒 Files selected for processing (3)
entrypoints/claude-ai.content.tsentrypoints/sidepanel/hooks/useDashboardData.tslib/conversation-store.ts
DevanshuNEU
added a commit
to DevanshuNEU/lco-fork
that referenced
this pull request
Apr 6, 2026
bridge-validation.test.ts:
- add ORGANIZATION_DETECTED section (4 tests): valid message, missing
organizationId, empty string organizationId, non-string organizationId
conversation-store.test.ts:
- add legacy fallback via listConversations (3 tests): returns legacy records
when scoped index empty, does not migrate to scoped index, both accounts
can read the same legacy records independently
- add extractOrgId parity (4 tests): assert lib/conversation-store and
inject.ts inline regex produce identical output for 4 URL patterns
- add empty accountId throws (2 tests): recordTurn and getConversation both
reject empty string accountId with the assertAccountId error
set-active-conv.test.ts:
- update mirrored SetActiveConvMessage to include organizationId: string | null
- update mirrored handleSetActiveConv to mirror full background.ts logic:
write activeOrg_{tabId} when organizationId present, remove stale key when
null, remove both keys on clear
- update 8 existing tests to include organizationId: null and fix assertions
for the two-key remove (was ['activeConv_42'], now ['activeConv_42', 'activeOrg_42'])
- add 3 new tests: writes activeOrg_ alongside activeConv_, removes stale
activeOrg_ when organizationId null, removes both keys on null conversationId
tests/integration/account-isolation.test.ts (new file, 3 tests):
- listConversations and daily summaries are scoped per account end-to-end
- legacy records visible to both accounts without migration (no scoped index
created, legacy convIndex untouched)
- same conversation ID across two accounts produces independent records
with independent finalization state
3 tasks
DevanshuNEU
added a commit
that referenced
this pull request
Apr 6, 2026
…st-hardening test(account-isolation): harden coverage for PR #29
This was referenced Apr 7, 2026
This was referenced Apr 26, 2026
15 tasks
DevanshuNEU
added a commit
that referenced
this pull request
May 4, 2026
…-36] Rule 0 returns Healthy when turnCount <= 2 AND contextPct < 30, before the per-model classifier runs. Blocks stale growthRate, isDetailHeavy, and any future projection wrapper from escalating a session that has no real history yet. Acceptance criteria: - Healthy on any conversation with turnCount <= 2 AND contextPct < 30 regardless of prior tab/conversation state - Wrappers like escalateForProjection still run on the returned HealthScore so a real draft can escalate after the guard - AC #3 (overlay resets on new chat) already satisfied by existing SPA-nav reset path in claude-ai.content.ts (PR #29) 12 new tests: positive path (5), boundary (3), does-not-mask (4).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/organizations/{orgId}/chat_conversations/.../completion) by inject.tsWhy this matters
Before this PR, switching Claude accounts (or using multiple accounts in the same browser) mixed conversation history, token costs, and daily summaries. The TODAY card, HISTORY list, and ACTIVE CONVERSATION all showed combined data from every account. This was a data integrity bug.
Architecture
The organization UUID is already present in every Claude API completion URL. inject.ts extracts it inline (
/organizations\/([0-9a-f-]+)\//i) and attaches it to TOKEN_BATCH and STREAM_COMPLETE bridge payloads. The content script captures it and forwards it on all background messages. The background uses it to scope every storage call.Storage keys changed from
conv:{convId}toconv:{orgId}:{convId},convIndextoconvIndex:{orgId}, etc. All index and summary functions accept accountId as their first parameter.Test plan
bun run compile-- no type errorsbun run test-- 516 passing (all tests updated with TEST_ORG constant)bun run build-- clean build, 996 kBSummary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests