Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/background/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,19 @@ async function handleMessage(msg: Message): Promise<unknown> {
await dropTab(msg.tabId)
return
}
case 'activateTab':
return chrome.tabs.update(msg.tabId, { active: true })
case 'activateTab': {
try {
await chrome.tabs.update(msg.tabId, { active: true })
return
} catch (e) {
// Stale ref: the tab is gone but folder.items still points at
// its id. Drop the ref so the next refresh removes the ghost
// row, then surface a generic "missing tab" error to the panel.
console.warn('[Spaces] activateTab failed; dropping ref', msg.tabId, e)
await dropTab(msg.tabId)
throw new Error('Tab is no longer available')
}
}
case 'reconcile':
return reconcileIfStale()
case 'pinUrl':
Expand Down
25 changes: 25 additions & 0 deletions src/background/reconcile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,31 @@ describe('reconcile', () => {
expect(store.folders[space.rootFolderId].items.filter(it => it.kind === 'tab' && it.tabId === t1.id!)).toEqual([])
})

it('dedupes duplicate tab refs within a folder even when no tabs are dead', async () => {
const space = await createSpace({ name: 'S', color: 'blue', windowId: 1 })
const t1 = await chrome.tabs.create({ windowId: 1 })
await registerTab(t1)

// Simulate the race that this fix targets: the same live tabId
// appears twice in root.items. (In the field this comes from
// switchTo's starter push running concurrently with
// chrome.tabs.onCreated → registerTab.)
const dirty = await loadStore()
dirty.folders[space.rootFolderId].items.push({ kind: 'tab', tabId: t1.id! })
dirty.folders[space.rootFolderId].items.push({ kind: 'tab', tabId: t1.id! })
await chrome.storage.local.set({ spaceStore: dirty })

const result = await reconcile()
// No dead tabs to report, but we still wrote the dedup-ed store.
expect(result.dropped).toBe(0)

const store = await loadStore()
const tabRefs = store.folders[space.rootFolderId].items.filter(
(it) => it.kind === 'tab' && it.tabId === t1.id!,
)
expect(tabRefs).toHaveLength(1)
})

describe('reconcileIfStale', () => {
it('throttles reconcile calls within 30 seconds', async () => {
vi.useFakeTimers()
Expand Down
11 changes: 9 additions & 2 deletions src/background/reconcile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,29 @@ import { pruneDeadTabs } from './space-manager'
// Drop tab refs and TabRecord entries for tabs that no longer exist.
// Called at SW startup; the store can drift if tabs were closed while the
// SW was suspended (MV3 events fired during suspension are not replayed).
// Also kicks in when a folder's items hold duplicate entries for the
// same tabId — pruneDeadTabs dedupes those alongside the dead-tab cull.
export async function reconcile(): Promise<{ dropped: number }> {
const tabs = await chrome.tabs.query({})
const liveIds = new Set(
tabs.map((t) => t.id).filter((id): id is number => typeof id === 'number'),
)
const before = await loadStore()
const droppedIds = new Set<number>()
let hasDuplicate = false
for (const id of Object.keys(before.tabs).map(Number)) {
if (!liveIds.has(id)) droppedIds.add(id)
}
for (const f of Object.values(before.folders)) {
const seen = new Set<number>()
for (const it of f.items) {
if (it.kind === 'tab' && !liveIds.has(it.tabId)) droppedIds.add(it.tabId)
if (it.kind !== 'tab') continue
if (!liveIds.has(it.tabId)) droppedIds.add(it.tabId)
else if (seen.has(it.tabId)) hasDuplicate = true
else seen.add(it.tabId)
}
}
if (droppedIds.size === 0) return { dropped: 0 }
if (droppedIds.size === 0 && !hasDuplicate) return { dropped: 0 }
await updateStore((s) => {
pruneDeadTabs(s, liveIds)
})
Expand Down
28 changes: 25 additions & 3 deletions src/background/space-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -598,12 +598,24 @@ export async function switchTo(spaceId: SpaceId, windowId?: number): Promise<voi
if (typeof created.id === 'number') {
activatedId = created.id
targetTabIds.add(created.id)
// chrome.tabs.onCreated → registerTab runs concurrently and (now
// that activeSpaceByWindow points at this Space) appends the new
// tab into root.items too. Without this guard the two writers
// each push, producing a duplicate item entry every time switchTo
// runs through the starter path.
await updateStore((s) => {
s.tabs[created.id!] = { tabId: created.id!, windowId: winId }
const sp = s.spaces[spaceId]
if (!sp) return
const root = s.folders[sp.rootFolderId]
if (root) root.items.push({ kind: 'tab', tabId: created.id! })
if (!root) return
if (
!root.items.some(
(it) => it.kind === 'tab' && it.tabId === created.id!,
)
) {
root.items.push({ kind: 'tab', tabId: created.id! })
}
})
}
} catch {
Expand Down Expand Up @@ -1091,15 +1103,25 @@ export async function reattachOrphanSpaces(): Promise<void> {
})
}

// Used by reconcile to drop stale tab refs.
// Used by reconcile to drop stale tab refs. Also deduplicates tab refs
// within each folder — race-condition fallouts (e.g. a previous
// registerTab vs switchTo starter push) can leave the same tabId in
// folder.items more than once, which surfaces as ghost rows in the side
// panel that keep replicating across switches. Keeping the dedup pass
// here means every reconcile sweep (panel mount + visibility change)
// self-heals existing damage even if the original racer is long gone.
export function pruneDeadTabs(s: SpaceStore, liveTabIds: Set<number>): boolean {
let changed = false
for (const f of Object.values(s.folders)) {
const before = f.items.length
const seenTabs = new Set<number>()
f.items = f.items.filter((it) => {
if (it.kind === 'folder') return true
if (it.kind === 'live') return true
return liveTabIds.has(it.tabId)
if (!liveTabIds.has(it.tabId)) return false
if (seenTabs.has(it.tabId)) return false
seenTabs.add(it.tabId)
return true
})
if (f.items.length !== before) changed = true
if (f.live) {
Expand Down