diff options
| author | Igor Soarez <igor@soarez.org> | 2026-11-08 17:11:15 +0000 |
|---|---|---|
| committer | Igor Soarez <igor@soarez.org> | 2026-11-08 17:11:15 +0000 |
| commit | b8b63ea4d1bad9c4bbac6b05b03cce5f2cfa3f94 (patch) | |
| tree | 255a0979c1d590ea759e27980ca3d9d655b690f5 | |
| parent | b5f7a9eb2cf46867085b46514d6c9faf0d552fad (diff) | |
A window shows only one tab, so sessions sharing one hid each other's tabs: no
real input and blank screenshots for every session but the one whose tab was
active (castle-pirc-web, 2026-11-08). Time-sharing the active tab would flip tabs
under the user while they sign in for an agent, so revert 66fbd30: a session
gets its own unfocused window. Kept: chrome_tab new for a session with no window
opens one for it instead of landing in the user's focused window. The primer
and chrome_tab description now say why to reuse a tab.
| -rw-r--r-- | extensions/chrome-profile-bridge/browser-extension/service_worker.js | 45 | ||||
| -rw-r--r-- | extensions/chrome-profile-bridge/index.ts | 4 | ||||
| -rw-r--r-- | test-suite/unit/background-policy.test.mjs | 12 | ||||
| -rw-r--r-- | test-suite/unit/shared-window.test.mjs | 45 |
4 files changed, 40 insertions, 66 deletions
diff --git a/extensions/chrome-profile-bridge/browser-extension/service_worker.js b/extensions/chrome-profile-bridge/browser-extension/service_worker.js index 0ceabda..d47b385 100644 --- a/extensions/chrome-profile-bridge/browser-extension/service_worker.js +++ b/extensions/chrome-profile-bridge/browser-extension/service_worker.js @@ -226,26 +226,12 @@ function isPiChromeOwnedTarget(tabId, sessionKey) { return false; } -// The window agents share. Every session has its own tab group and a group cannot span windows, -// but sessions do not need their own windows: new tabs go to the window that already holds any -// "Pi Session:" group, so agents stay together instead of opening one window each. Returns -// undefined when no such window exists (the first session then creates one). -async function sharedPiWindowId() { - if (!chrome.tabGroups || typeof chrome.windows?.get !== "function") return undefined; - const groups = await chrome.tabGroups.query({}).catch(() => []); - for (const group of groups) { - if (!/^pi session:/i.test((group.title || "").trim()) || typeof group.windowId !== "number") continue; - const win = await chrome.windows.get(group.windowId).catch(() => null); - if (win && (win.type === undefined || win.type === "normal")) return win.id ?? group.windowId; - } - return undefined; -} - // Create a fresh automation target for `sessionKey`. If this session already has a tab group, // create the tab inside that group's window so one Pi session keeps one Chrome tab group (Chrome -// groups cannot span windows). If it has none, use the shared Pi window; only when there is no -// Pi window yet, create one; fall back to a tab. When the tab is created in a pre-existing window, -// leave windowId unset so cleanup only closes our tab, never that whole window. +// groups cannot span windows). If it has none, give the session its own window: a window shows +// only one tab, so sessions sharing a window would hide each other's tabs, and a hidden tab gets no +// real input and blank screenshots (measured 2026-11-08). Fall back to a tab. When the tab is +// created in a pre-existing window, leave windowId unset so cleanup only closes our tab. async function createAutomationTarget(sessionKey, groupTitle) { const existingGroup = groupTitle ? await findGroupRecordByTitle(groupTitle) : null; if (existingGroup && typeof existingGroup.windowId === "number") { @@ -254,14 +240,6 @@ async function createAutomationTarget(sessionKey, groupTitle) { await persistAutomationTargets(); return tab; } - const sharedWindowId = await sharedPiWindowId(); - if (sharedWindowId !== undefined) { - // Someone else's window: leave windowId unset so cleanup only ever closes our tab. - const tab = await chrome.tabs.create({ url: "about:blank", active: false, windowId: sharedWindowId }); - automationTargets.set(sessionKey, { windowId: undefined, tabId: typeof tab.id === "number" ? tab.id : undefined }); - await persistAutomationTargets(); - return tab; - } if (chrome.windows && typeof chrome.windows.create === "function") { try { const win = await chrome.windows.create({ url: "about:blank", focused: false }); @@ -1911,15 +1889,20 @@ async function dispatch(action, params) { const existingGroup = await findGroupRecordByTitle(groupTitle); const createParams = { url: params.url || "about:blank", active: foregroundRequested(params) }; if (existingGroup && typeof existingGroup.windowId === "number") createParams.windowId = existingGroup.windowId; - else { - // No group of this session yet: use the shared Pi window, never the user's focused one. - const sharedWindowId = await sharedPiWindowId(); - if (sharedWindowId !== undefined) createParams.windowId = sharedWindowId; + // No group of this session yet: use the window of its automation tab if it has one, else open + // the tab in a window of its own. Never the user's focused window, where Chrome would put it. + let ownWindow = false; + if (createParams.windowId === undefined) { + const home = await resolveOwnedAutomationTarget(sessionKeyOf(params)).catch(() => null); + if (home && typeof home.windowId === "number") createParams.windowId = home.windowId; + else ownWindow = typeof chrome.windows?.create === "function"; } // Register the creation as in flight until the tab is tracked, so early capture (which only // runs in tracked tabs) waits for it instead of skipping the new tab's first page load. const creation = (async () => { - const created = await chrome.tabs.create(createParams); + const created = ownWindow + ? (await chrome.windows.create({ url: createParams.url, focused: false })).tabs[0] + : await chrome.tabs.create(createParams); await trackSessionTab(sessionKeyOf(params), created.id, true); return created; })(); diff --git a/extensions/chrome-profile-bridge/index.ts b/extensions/chrome-profile-bridge/index.ts index cf8671d..525de6e 100644 --- a/extensions/chrome-profile-bridge/index.ts +++ b/extensions/chrome-profile-bridge/index.ts @@ -1278,7 +1278,7 @@ Usage rules: 7. If you hit a native file-picker or privileged browser prompt gate, tell the user; generic clicks/typing/CSP gates are handled by Chrome input. 8. Run /chrome doctor when in doubt about connectivity or capabilities. 9. A page can open dialogs you cannot see: alert, confirm, prompt, and the \"Leave site? Changes you made may not be saved\" prompt of a page with unsaved changes. They are dismissed for you (stay, cancel). \`chrome_navigate\` reports one by failing, and \`leaveIfAsked: true\` leaves anyway; use that only when you know nothing unsaved matters, and ask the user when unsure. If a click or key press that should have navigated did nothing, a Leave-site prompt may have stopped it: check the page, then ask the user before discarding anything. -10. All agent sessions share one browser window; your tabs sit in your own colored tab group inside it. Reuse a tab you already have (navigate it) rather than opening new ones, and open a new tab only when you need two pages at once. Do not open new windows unless the user asks. +10. Your session has its own browser window, with your tabs in your own colored tab group. Only the active tab of a window is visible, and a hidden tab gets no real input and blank screenshots, so reuse your tab (navigate it) rather than opening new ones; open a second tab only when you need two pages at once, and expect only the active one to take input. Do not open new windows unless the user asks. 11. If a tool fails saying another extension (a password manager such as 1Password) has a prompt open on the tab, decide whether it is in your way or part of the task. If it is just in your way, call \`chrome_clear_overlay\` for the tab. If you need the user to sign in, authorize or approve something with it, leave it alone and tell the user what you are waiting for. Navigating the tab away also frees it.`; // Shared handlers, dispatched by the unified /chrome command below. @@ -1654,7 +1654,7 @@ Usage rules: pi.registerTool({ name: "chrome_tab", label: "Chrome Tab", - description: "List, create, activate, close, group, ungroup, or inspect tabs in the user's existing Chrome profile via the companion extension. New/grouped tabs always use this session's Pi tab group, inside the one window all agent sessions share; prefer navigating a tab you already have to opening another. Background mode keeps new tabs inactive and blocks activate; ask the user to run /chrome background off for foreground/watch mode. activate/close/group/ungroup require a target (targetId/urlIncludes/titleIncludes); with no target they act on this session's pi-chrome automation tab if one exists, and otherwise error rather than touching the user's active tab.", + description: "List, create, activate, close, group, ungroup, or inspect tabs in the user's existing Chrome profile via the companion extension. New/grouped tabs always use this session's Pi tab group, in the session's own window; prefer navigating a tab you already have to opening another, since only a window's active tab is visible. Background mode keeps new tabs inactive and blocks activate; ask the user to run /chrome background off for foreground/watch mode. activate/close/group/ungroup require a target (targetId/urlIncludes/titleIncludes); with no target they act on this session's pi-chrome automation tab if one exists, and otherwise error rather than touching the user's active tab.", promptSnippet: "List/open/activate/close/group existing Chrome tabs through the companion extension.", parameters: Type.Object({ action: StringEnum(tabActionValues), diff --git a/test-suite/unit/background-policy.test.mjs b/test-suite/unit/background-policy.test.mjs index efbca78..ea08be0 100644 --- a/test-suite/unit/background-policy.test.mjs +++ b/test-suite/unit/background-policy.test.mjs @@ -297,6 +297,14 @@ function workerHarness({ withWindows = true } = {}) { }; return h; } +// A new Pi tab must not take over the user's window: it is either inactive, or the only tab of a +// new, unfocused window of the session's own (so it is visible to the agent without stealing focus). +function assertNewTabStaysOutOfTheWay(h, tab) { + if (tab.active === false) return; + assert.notEqual(tab.windowId, 1, "an active new tab must not be in the user's window"); + assert.ok(h.calls.some((c) => c.action === "windows.create" && c.params.focused === false), "its window was created unfocused"); +} + function assertNoFocus(h) { assert.equal(h.focusedWindow(), 1); assert.equal(h.tabs.get(1).active, true); @@ -325,7 +333,7 @@ test("worker advertises support, keeps new tabs inactive, and rejects activation assert.equal((await h.w.dispatch("tab.version", {})).capabilities.hardBackground, true); for (const flags of [{}, { foreground: false }, { background: true, foreground: true }]) { const result = await h.w.dispatch("tab.new", { ...flags, sessionKey: "alpha", groupTitle: "Pi Session: alpha" }); - assert.equal(result.tab.active, false); + assertNewTabStaysOutOfTheWay(h, result.tab); await assert.rejects(h.w.dispatch("tab.activate", { ...flags, targetId: "missing" }), /background mode/); } assertNoFocus(h); @@ -339,7 +347,7 @@ test("worker advertises support, keeps new tabs inactive, and rejects activation test("background-only worker aliases force safe behavior even with foreground flags", async () => { const h = workerHarness(); const opened = await h.w.dispatch("tab.new.background", { foreground: true, background: false }); - assert.equal(opened.tab.active, false); + assertNewTabStaysOutOfTheWay(h, opened.tab); const shot = await h.w.dispatch("page.screenshot.background", { targetId: "2", foreground: true, background: false }); assert.equal(shot.method, "cdp"); assertNoFocus(h); diff --git a/test-suite/unit/shared-window.test.mjs b/test-suite/unit/shared-window.test.mjs index 7afeb77..66041f4 100644 --- a/test-suite/unit/shared-window.test.mjs +++ b/test-suite/unit/shared-window.test.mjs @@ -1,5 +1,5 @@ -// Agent sessions share one window: a new session's tab goes into the window that already holds a -// "Pi Session:" group, and a window is created only when there is none. +// Each session has its own window: a window shows only one tab, so sessions sharing one hid each +// other's tabs (no real input, blank screenshots). A session's extra tabs go into its own window. import assert from "node:assert/strict"; import fs from "node:fs"; import vm from "node:vm"; @@ -35,44 +35,27 @@ function worker({ groups = [], windows = { 5: { id: 5, type: "normal" } } } = {} } const piGroup = (windowId, name = "Pi Session: claude-code 1 other") => ({ id: 1, windowId, title: name }); +const ops = (calls) => calls.filter((c) => c.method !== "storage.set").map((c) => c.method); -test("a new session's tab goes into the window that already holds another Pi session", async () => { +test("a new session gets its own window even when another session's window exists", async () => { const { w, calls } = worker({ groups: [piGroup(5)] }); await w.createAutomationTarget("claude-code 2 mine", "Pi Session: claude-code 2 mine"); - assert.deepEqual(calls.filter((c) => c.method !== "storage.set").map((c) => c.method), ["tabs.create"]); - assert.equal(calls[0].props.windowId, 5); + assert.deepEqual(ops(calls), ["windows.create"]); }); -test("with no Pi window yet, the first session creates one", async () => { - const { w, calls } = worker({ groups: [] }); +test("a session that already has a group adds its tab to that group's window", async () => { + const { w, calls } = worker({ groups: [piGroup(5, "Pi Session: claude-code 2 mine")] }); await w.createAutomationTarget("claude-code 2 mine", "Pi Session: claude-code 2 mine"); - assert.deepEqual(calls.filter((c) => c.method !== "storage.set").map((c) => c.method), ["windows.create"]); + assert.deepEqual(ops(calls), ["tabs.create"]); + assert.equal(calls.find((c) => c.method === "tabs.create").props.windowId, 5); }); -test("groups that are not Pi sessions do not attract agent tabs", async () => { - const { w, calls } = worker({ groups: [{ id: 2, windowId: 5, title: "Work" }] }); - await w.createAutomationTarget("claude-code 2 mine", "Pi Session: claude-code 2 mine"); - assert.deepEqual(calls.filter((c) => c.method !== "storage.set").map((c) => c.method), ["windows.create"]); -}); - -test("a Pi group whose window is gone, or is not a normal window, is skipped", async () => { - const { w, calls } = worker({ groups: [piGroup(7), piGroup(8)], windows: { 8: { id: 8, type: "popup" } } }); - await w.createAutomationTarget("claude-code 2 mine", "Pi Session: claude-code 2 mine"); - assert.deepEqual(calls.filter((c) => c.method !== "storage.set").map((c) => c.method), ["windows.create"]); -}); - -test("the shared window is never recorded as ours, so cleanup cannot close it", async () => { - const { w, calls } = worker({ groups: [piGroup(5)] }); - await w.createAutomationTarget("claude-code 2 mine", "Pi Session: claude-code 2 mine"); - const stored = calls.filter((c) => c.method === "storage.set").map((c) => c.json).join(""); - assert.doesNotMatch(stored, /"windowId":5/); -}); - -test("chrome_tab new for a session with no group uses the shared window, not the focused one", async () => { +test("chrome_tab new for a session with no group goes to the session's own window, not the focused one", async () => { const { w, calls } = worker({ groups: [piGroup(5)] }); w.groupTab = async (tab) => tab; w.bringToFront = async () => {}; - await w.dispatch("tab.new", { url: "about:blank", groupTitle: "Pi Session: claude-code 2 mine", sessionKey: "claude-code 2 mine" }); - const created = calls.find((c) => c.method === "tabs.create"); - assert.equal(created.props.windowId, 5); + await w.dispatch("tab.new", { url: "https://x.test/", groupTitle: "Pi Session: claude-code 2 mine", sessionKey: "claude-code 2 mine" }); + assert.deepEqual(ops(calls), ["windows.create"]); + assert.equal(calls[0].props.url, "https://x.test/"); + assert.equal(calls[0].props.focused, false); }); |
