Skip to content

Commit c3a72a5

Browse files
JohnMcLearclaude
andcommitted
fix(tabs): coalesce concurrent open() calls so rapid clicks don't duplicate
User report: "I was able to get a pad to open twice if I click it quickly multiple times. It shows up multiple times in the Tab view." Root cause: TabManager.open() looks for an existing tab via this.tabs.find(...) and bails early if found, but factory.create() is async (it awaits webContents.loadURL). Two clicks <100ms apart both pass the existence check while neither has pushed its tab yet, then both push, then both register events — duplicate tabs. Add an inflight Map<key, Promise<OpenTab>> keyed by "\${workspaceId}|\${padName}". Concurrent calls that find an inflight promise return that same promise, so all callers resolve to the same OpenTab and only one factory.create + viewHost.add fires. Also re- check for an existing tab AFTER the await, in case some other path created one while we were waiting. Three regression tests pin the contract: - two concurrent opens for the same key resolve to one tab - three concurrent opens still resolve to one tab - after coalescing, a later open with the same key still dedups Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6b31531 commit c3a72a5

2 files changed

Lines changed: 107 additions & 21 deletions

File tree

src/main/tabs/tab-manager.ts

Lines changed: 50 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ export class TabManager {
4545
private readonly tabs: Internal[] = [];
4646
private activeWorkspaceId: string | null = null;
4747
private activeTabId: string | null = null;
48+
/**
49+
* Tracks open() calls that are still awaiting factory.create() so a
50+
* second rapid click on the same pad doesn't race past the existence
51+
* check and end up creating a duplicate tab. Keyed by
52+
* `${workspaceId}|${padName}`.
53+
*/
54+
private readonly inflight = new Map<string, Promise<OpenTab>>();
4855

4956
constructor(private readonly opts: TabManagerOptions) {}
5057

@@ -76,27 +83,50 @@ export class TabManager {
7683
this.emitTabs();
7784
return existing.tab;
7885
}
79-
const view = await this.opts.factory.create({
80-
workspaceId: input.workspaceId,
81-
src: input.src,
82-
preloadPath: this.opts.preloadPath,
83-
});
84-
const tab: OpenTab = {
85-
tabId: randomUUID(),
86-
workspaceId: input.workspaceId,
87-
padName: input.padName,
88-
title: input.padName,
89-
state: 'loading',
90-
};
91-
this.tabs.push({ tab, view });
92-
this.opts.viewHost.add(view);
93-
if (input.workspaceId === this.activeWorkspaceId) {
94-
this.activeTabId = tab.tabId;
86+
// Coalesce rapid concurrent opens for the same (workspace, pad) pair.
87+
// Without this, two clicks <100ms apart both see no existing tab,
88+
// both await factory.create(), both push their own view, and the
89+
// user ends up with duplicate tabs.
90+
const key = `${input.workspaceId}|${input.padName}`;
91+
const inflight = this.inflight.get(key);
92+
if (inflight) return inflight;
93+
94+
const promise = (async () => {
95+
const view = await this.opts.factory.create({
96+
workspaceId: input.workspaceId,
97+
src: input.src,
98+
preloadPath: this.opts.preloadPath,
99+
});
100+
// Re-check after the await: another path (e.g. a concurrent
101+
// restoration) may have created the tab while we were waiting.
102+
const raced = this.tabs.find(
103+
(t) => t.tab.workspaceId === input.workspaceId && t.tab.padName === input.padName,
104+
);
105+
if (raced) return raced.tab;
106+
const tab: OpenTab = {
107+
tabId: randomUUID(),
108+
workspaceId: input.workspaceId,
109+
padName: input.padName,
110+
title: input.padName,
111+
state: 'loading',
112+
};
113+
this.tabs.push({ tab, view });
114+
this.opts.viewHost.add(view);
115+
if (input.workspaceId === this.activeWorkspaceId) {
116+
this.activeTabId = tab.tabId;
117+
}
118+
this.applyVisibility();
119+
this.wireViewEvents(tab.tabId, view);
120+
return tab;
121+
})();
122+
this.inflight.set(key, promise);
123+
try {
124+
const tab = await promise;
125+
this.emitTabs();
126+
return tab;
127+
} finally {
128+
this.inflight.delete(key);
95129
}
96-
this.applyVisibility();
97-
this.wireViewEvents(tab.tabId, view);
98-
this.emitTabs();
99-
return tab;
100130
}
101131

102132
close(tabId: string): void {

tests/main/tabs/tab-manager.spec.ts

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,14 @@ import type { PadView } from '../../../src/main/pads/pad-view-factory';
44

55
function fakeView(): PadView {
66
return {
7-
webContents: { loadURL: vi.fn().mockResolvedValue(undefined), on: vi.fn(), id: 1 },
7+
webContents: {
8+
loadURL: vi.fn().mockResolvedValue(undefined),
9+
on: vi.fn(),
10+
getUserAgent: vi.fn(() => 'Mozilla/5.0 Test'),
11+
setUserAgent: vi.fn(),
12+
focus: vi.fn(),
13+
id: 1,
14+
},
815
setBounds: vi.fn(),
916
setVisible: vi.fn(),
1017
};
@@ -55,6 +62,55 @@ describe('TabManager', () => {
5562
expect(factory.create).toHaveBeenCalledTimes(1);
5663
});
5764

65+
// REGRESSION: 2026-05-05 — user reported a pad opening multiple times
66+
// when clicked quickly in succession. The dedup check looked for an
67+
// existing tab BEFORE awaiting factory.create(); two concurrent opens
68+
// both passed the check and both pushed their own view, producing
69+
// duplicate tabs. Coalesce concurrent opens for the same key.
70+
it('coalesces rapid concurrent opens for the same (workspaceId, padName)', async () => {
71+
// Slow factory: returns a promise that resolves on the next tick so
72+
// both calls overlap inside open().
73+
let resolveCreate!: (v: PadView) => void;
74+
factory.create = vi.fn().mockImplementation(
75+
() => new Promise<PadView>((r) => { resolveCreate = r; }),
76+
);
77+
const p1 = mgr.open({ workspaceId: WS_A, padName: 'samepad', src: 'https://x/p/samepad' });
78+
const p2 = mgr.open({ workspaceId: WS_A, padName: 'samepad', src: 'https://x/p/samepad' });
79+
// Resolve the (single) factory call.
80+
resolveCreate(fakeView());
81+
const [a, b] = await Promise.all([p1, p2]);
82+
expect(b.tabId).toBe(a.tabId);
83+
expect(factory.create).toHaveBeenCalledTimes(1);
84+
expect(host.add).toHaveBeenCalledTimes(1);
85+
});
86+
87+
it('coalesces three concurrent opens too — all share one tab', async () => {
88+
let resolveCreate!: (v: PadView) => void;
89+
factory.create = vi.fn().mockImplementation(
90+
() => new Promise<PadView>((r) => { resolveCreate = r; }),
91+
);
92+
const opens = [
93+
mgr.open({ workspaceId: WS_A, padName: 'pad', src: 'https://x/p/pad' }),
94+
mgr.open({ workspaceId: WS_A, padName: 'pad', src: 'https://x/p/pad' }),
95+
mgr.open({ workspaceId: WS_A, padName: 'pad', src: 'https://x/p/pad' }),
96+
];
97+
resolveCreate(fakeView());
98+
const results = await Promise.all(opens);
99+
const ids = new Set(results.map((t) => t.tabId));
100+
expect(ids.size).toBe(1);
101+
expect(factory.create).toHaveBeenCalledTimes(1);
102+
});
103+
104+
it('after a coalesced open finishes, a later open with the same key returns the same tab', async () => {
105+
const a = await mgr.open({ workspaceId: WS_A, padName: 'p', src: 'https://x/p/p' });
106+
const b = await mgr.open({ workspaceId: WS_A, padName: 'p', src: 'https://x/p/p' });
107+
expect(b.tabId).toBe(a.tabId);
108+
// The inflight map should be empty after each open() resolves —
109+
// verified indirectly by factory.create being called exactly once
110+
// across two completed (non-overlapping) opens.
111+
expect(factory.create).toHaveBeenCalledTimes(1);
112+
});
113+
58114
it('positions the active view to the main area on resize', async () => {
59115
mgr.setActiveWorkspace(WS_A);
60116
const tab = await mgr.open({ workspaceId: WS_A, padName: 'p', src: 'https://x/p/p' });

0 commit comments

Comments
 (0)