diff --git a/README.md b/README.md index 0c44c75..7c45999 100644 --- a/README.md +++ b/README.md @@ -81,8 +81,16 @@ browser-hand open --url https://example.com --page-name demo --group-title "DNS browser-hand snapshot --page-name demo browser-hand click --page-name demo --text "More information..." browser-hand screenshot --page-name demo +browser-hand close --page-name demo # done: close the tab Browser Hand opened ``` +`close` needs an explicit selector (`--page-name`, `--query`, `--target-*`, or `--group-title`) and never closes the active tab by default. It returns `{ closed, targetId, url, title }`, and `closed` is only `true` once Chrome no longer lists the tab. + +- A selector that matches several tabs is refused unless you pass `--all`. +- `--group-title "DNS records"` closes every tab in that Browser Hand tab group. +- It only closes tabs Browser Hand opened. A tab you already had open is refused; `--all` and `--group-title` skip such tabs and list them under `skipped`. Pass `--include-user-tabs` to close one anyway. +- Ownership is tracked by the relay, so restart it (`npm run build:relay && npm run relay`) and reload the extension after upgrading. A relay that predates this fails closed instead of guessing. + The first `open` of a task can name the Chrome tab group. Later opens with the same title join that group. A different title starts a new group and a new color. Leave the flag off and the group stays named Browser Hand. ![Chrome tab group titled DNS records, holding the Example Domain and Example Domains tabs](docs/img/tab-group-title.png) diff --git a/cli-js/src/cli.js b/cli-js/src/cli.js index 66d05e9..ae0da6b 100644 --- a/cli-js/src/cli.js +++ b/cli-js/src/cli.js @@ -16,6 +16,7 @@ * browser-hand evaluate --code 'document.title' * browser-hand goto --url https://example.com * browser-hand open --url https://example.com --page-name smoke + * browser-hand close --page-name smoke * browser-hand doctor * * Output: JSON on stdout. Exits 0 on success, 1 on tool/handler error, @@ -64,8 +65,13 @@ Operations: focus [--focus window|tab] Bring a tab forward for human input (one-shot; default window). --reason Required for good audit (e.g. 2fa, confirm-publish). tabs [--query ] List every http(s) tab. Unique-active first; last-focused when stamped. - doctor [--verbose] [--browser ] Health check: status, browser:{name,version}, tabCount, active tab. - --browser asserts which connected browser you expect. + close (--page-name | --query | --target-id | --group-title ) + Close a tab Browser Hand opened. Returns { closed, targetId, url, title }. + Never closes the active tab by default and refuses a selector + that matches several tabs unless --all is given. --group-title + closes every tab in that Browser Hand tab group. Refuses a tab + you had open unless --include-user-tabs is passed. + doctor [--verbose] Health check: status, tabCount, and the active tab. Not a tab list. relay Start a persistent extension relay (foreground) so the Chrome extension has a stable endpoint to connect to. Run once and leave it up; Ctrl-C to stop. Idempotent — @@ -87,6 +93,9 @@ Global flags: --role Click by ARIA role. Requires --name. --name Accessible name for --role. Exact match. --group-title Optional tab-group label for open/goto. A new title starts a new group. + With close: the Browser Hand tab group to close. + --all close: allow a selector that matches several tabs to close all of them. + --include-user-tabs close: also close tabs Browser Hand did not open (tabs you had open). -h, --help Show this help. Examples: @@ -97,6 +106,9 @@ Examples: browser-hand tabs browser-hand tabs --query stripe browser-hand snapshot --query stripe + browser-hand close --page-name smoke + browser-hand close --query example.com --all + browser-hand close --group-title "DNS records" browser-hand doctor browser-hand click --page-name work --role combobox --name "Type" browser-hand click --page-name work --ref e4 @@ -116,6 +128,10 @@ function parseArgs(argv) { args.quiet = true; } else if (tok === "--full-page") { args.fullPage = true; + } else if (tok === "--all") { + args.all = true; + } else if (tok === "--include-user-tabs") { + args.includeUserTabs = true; } else if (tok.startsWith("--")) { const key = tok.slice(2); const next = argv[i + 1]; @@ -276,6 +292,24 @@ function buildHandlerInput(operation, args) { input.query = args.query.trim(); } break; + case "close": + if (args["group-title"] === true) { + fail("--group-title requires a value"); + } + if (typeof args["group-title"] === "string" && args["group-title"].trim()) { + input.groupTitle = args["group-title"].trim(); + } + if (args.all === true) input.all = true; + if (args.includeUserTabs === true) input.includeUserTabs = true; + if (!input.pageName && !input.target && !input.groupTitle) { + fail( + "close requires --page-name, --query, --target-id/--target-url/--target-title, or --group-title" + ); + } + if (input.pageName && (input.target || input.groupTitle)) { + fail("close --page-name cannot be combined with --query, --target-*, or --group-title"); + } + break; case "snapshot": if (typeof args.scope === "string" && args.scope.trim()) { const scope = args.scope.trim(); diff --git a/cli-js/src/tool.js b/cli-js/src/tool.js index 51610d7..25f5dd5 100644 --- a/cli-js/src/tool.js +++ b/cli-js/src/tool.js @@ -122,6 +122,19 @@ export function isHttpPageTarget(item) { return Boolean(item && item.type === "page" && /^https?:\/\//i.test(String(item.url || ""))); } +/** + * Every page the extension lists: http(s), file://, and about:blank. Mirrors + * `TabManager.isControllableUrl`, so a tab Browser Hand opened and has not + * navigated yet is still a tab. + */ +export function isControllablePageTarget(item) { + return Boolean( + item && + item.type === "page" && + /^(?:https?:\/\/|file:\/\/|about:blank(?:\?|$))/i.test(String(item.url || "")) + ); +} + function clipText(value, cap) { const text = String(value || ""); if (text.length <= cap) return text; @@ -147,8 +160,8 @@ export function rankTabTargets(targets = []) { }); } -export function filterTabTargets(targets = [], query = {}) { - let pages = (targets || []).filter(isHttpPageTarget); +export function filterTabTargets(targets = [], query = {}, { includeNonHttp = false } = {}) { + let pages = (targets || []).filter(includeNonHttp ? isControllablePageTarget : isHttpPageTarget); const idNeedle = query.id ? String(query.id) : ""; const urlNeedle = query.url ? String(query.url).toLowerCase() : ""; const titleNeedle = query.title ? String(query.title).toLowerCase() : ""; @@ -1286,6 +1299,20 @@ function getCachedNamedPage(pageName) { return cache[pageName] || null; } +/** Drop cached name → tab entries that point at tabs which no longer exist. */ +function forgetNamedPageTargets(targetIds = []) { + if (targetIds.length === 0) return; + const cache = loadNamedPageTargetCache(); + let changed = false; + for (const [name, entry] of Object.entries(cache)) { + if (targetIds.includes(entry?.targetId)) { + delete cache[name]; + changed = true; + } + } + if (changed) saveNamedPageTargetCache(cache); +} + function setCachedNamedPage(pageName, entry) { const cache = loadNamedPageTargetCache(); cache[pageName] = { @@ -1561,6 +1588,236 @@ async function runCurrentTabs(input = {}, timeoutMs) { } } +const CLOSE_SELECTOR_HELP = + "close needs --page-name, --query, --target-id/--target-url/--target-title, or --group-title. It never closes the active tab by default."; + +/** Same normalization the extension applies to tab-group titles. */ +function groupTitleKey(raw) { + return String(raw ?? "") + .replace(/\s+/g, " ") + .trim() + .slice(0, 40) + .trimEnd(); +} + +/** + * Decide which tabs `close` acts on. Pure: no relay or Chrome access. + * + * - Needs an explicit selector. It never falls back to the active tab. + * - A selector matching several tabs is refused unless `all` is set. A lone + * `groupTitle` already means "the whole group". + * - Without `includeUserTabs`, only tabs Browser Hand opened (`browserHandOwned` + * from the relay) are closable. A single user tab is refused; in a multi-tab + * selection user tabs are skipped and reported. + */ +export function planCloseTargets(targets = [], opts = {}) { + const { pageName, namedTargetId, target = {}, all = false, includeUserTabs = false } = opts; + const group = typeof opts.groupTitle === "string" ? groupTitleKey(opts.groupTitle) : ""; + const hasTargetSelector = Boolean( + target.id || target.url || target.title || target.name || target.query + ); + + if (!namedTargetId && !hasTargetSelector && !group) { + throw new Error(CLOSE_SELECTOR_HELP); + } + if (namedTargetId && (hasTargetSelector || group)) { + throw new Error( + "close --page-name cannot be combined with --query, --target-*, or --group-title" + ); + } + + const selectorLabel = JSON.stringify({ + ...(pageName ? { pageName } : {}), + ...(hasTargetSelector ? target : {}), + ...(group ? { groupTitle: group } : {}), + }); + + let candidates; + if (namedTargetId) { + // A named page is an exact id, so a tab still on about:blank can be closed too. + candidates = (targets || []).filter( + (item) => item?.type === "page" && item.targetId === namedTargetId + ); + } else { + // Unlike snapshot/click, close must see about:blank and file:// tabs: a + // group close that skipped them would leave tabs open and still say `closed`. + candidates = filterTabTargets(targets, target, { includeNonHttp: true }); + if (group) { + candidates = candidates.filter((item) => groupTitleKey(item.groupTitle) === group); + } + } + + if (candidates.length === 0) { + const noGroupInfo = + group && !(targets || []).some((item) => typeof item?.groupTitle === "string"); + throw new Error( + `No Chrome tab matched ${selectorLabel}` + + (noGroupInfo + ? ". No tab reports a group title; reload the Browser Hand extension if the group exists." + : "") + ); + } + + const wholeGroup = Boolean(group) && !hasTargetSelector; + if (candidates.length > 1 && !all && !wholeGroup) { + throw new Error( + `Ambiguous Chrome tab target ${selectorLabel} matched ${candidates.length} tabs: ${formatTargetCandidates(candidates)}. Pass --all to close every match.` + ); + } + + if (includeUserTabs) { + return { matched: candidates.length, closable: candidates, skipped: [] }; + } + + if (!(targets || []).some((item) => typeof item?.browserHandOwned === "boolean")) { + throw new Error( + "The relay does not report which tabs Browser Hand opened (it predates ownership tracking). Rebuild and restart it (`npm run build:relay && npm run relay`), or pass --include-user-tabs to close anyway." + ); + } + + const closable = candidates.filter((item) => item.browserHandOwned === true); + const skipped = candidates + .filter((item) => item.browserHandOwned !== true) + .map((item) => ({ + ...compactTarget(item), + reason: "not opened by Browser Hand; pass --include-user-tabs to close it", + })); + if (closable.length === 0) { + throw new Error( + `Refusing to close ${candidates.length === 1 ? "a tab" : "tabs"} Browser Hand did not open: ${formatTargetCandidates(candidates)}. Pass --include-user-tabs to close ${candidates.length === 1 ? "it" : "them"}.` + ); + } + return { matched: candidates.length, closable, skipped }; +} + +/** + * Poll until none of `targetIds` is listed any more. Returns the ids that are + * still open. The relay can answer a close with success without closing + * anything (adopted tabs), so the live list is the only proof. + */ +export async function waitForTargetsClosed( + listTargets, + targetIds, + { timeoutMs = 3000, intervalMs = 150, sleep = delay } = {} +) { + const deadline = Date.now() + timeoutMs; + let open = new Set(targetIds); + for (;;) { + const live = new Set((await listTargets()).map((item) => item?.targetId || item?.id)); + open = new Set([...open].filter((id) => live.has(id))); + if (open.size === 0 || Date.now() >= deadline) return open; + await sleep(intervalMs); + } +} + +/** `{ closed, targetId, url, title }` for one tab; a `tabs` list for --all / --group-title. */ +export function buildCloseResult({ tabs, skipped = [], multi = false, pageName, groupTitle }) { + const allClosed = tabs.length > 0 && tabs.every((tab) => tab.closed); + const base = { success: allClosed, mode: "current", operation: "close" }; + if (!multi) { + const [tab] = tabs; + return { + ...base, + ...tab, + ...(pageName ? { pageName } : {}), + ...(skipped.length > 0 ? { skipped } : {}), + }; + } + return { + ...base, + closed: allClosed, + count: tabs.filter((tab) => tab.closed).length, + ...(groupTitle ? { groupTitle } : {}), + tabs, + skipped, + }; +} + +async function runCurrentClose(input, timeoutMs) { + const info = await ensureRelayConnected(Math.min(timeoutMs, 15000)); + const cdp = new CdpClient(info.wsEndpoint); + try { + let namedTargetId; + if (input.pageName) { + const page = await lookupNamedRelayPage(input.pageName); + if (!page?.targetId) { + const known = await listNamedRelayPages(); + const count = Array.isArray(known?.pages) ? known.pages.length : 0; + throw new Error(namedPageNotFoundMessage(input.pageName, count)); + } + namedTargetId = page.targetId; + } + const listOpen = async () => (await cdp.send("Target.getTargets"))?.targetInfos || []; + const includeUserTabs = input.includeUserTabs === true; + const groupTitle = + typeof input.groupTitle === "string" && input.groupTitle.trim() ? input.groupTitle : ""; + const plan = planCloseTargets(await listOpen(), { + pageName: input.pageName, + namedTargetId, + target: input.target, + groupTitle, + all: input.all === true, + includeUserTabs, + }); + + const picked = plan.closable.map((item) => compactTarget(item)); + const sendErrors = new Map(); + for (const tab of picked) { + try { + await cdp.send("Target.closeTarget", { + targetId: tab.id, + ...(includeUserTabs ? { force: true } : {}), + }); + } catch (err) { + sendErrors.set(tab.id, err.message); + } + } + // The live list is the proof; the relay can report success without closing. + let stillOpen; + try { + stillOpen = await waitForTargetsClosed( + listOpen, + picked.map((tab) => tab.id) + ); + } catch (err) { + // Closing the last tab of the last window quits Chrome and drops the relay socket. + throw new Error( + `Lost the relay while confirming the close (${err.message}). If that was the last open tab, Chrome may have quit. Run \`browser-hand tabs\` to check.` + ); + } + const results = picked.map((tab) => { + const closed = !stillOpen.has(tab.id); + return { + closed, + targetId: tab.id, + url: tab.url, + title: tab.title, + ...(closed + ? {} + : { error: sendErrors.get(tab.id) || "Tab is still open after Target.closeTarget" }), + }; + }); + const closedIds = results.filter((tab) => tab.closed).map((tab) => tab.targetId); + + // A closed tab leaves stale name → target entries that would otherwise be + // "restored" by the next open/goto on that name. + forgetNamedPageTargets(closedIds); + if (input.pageName && closedIds.length > 0) { + await deleteNamedRelayPage(input.pageName); + } + + return buildCloseResult({ + tabs: results, + skipped: plan.skipped, + multi: input.all === true || Boolean(groupTitle), + pageName: input.pageName, + groupTitle, + }); + } finally { + cdp.close(); + } +} + async function runCurrentOperation(input, timeoutMs) { const deadlineAt = Date.now() + (Number.isFinite(timeoutMs) ? timeoutMs : 0); const operation = input.operation; @@ -1578,6 +1835,10 @@ async function runCurrentOperation(input, timeoutMs) { }; } + if (operation === "close") { + return await runCurrentClose(input, timeoutMs); + } + const info = await ensureRelayConnected(Math.min(timeoutMs, 15000)); // Registering needs a running relay, so this waits for ensureRelayConnected. if ((operation === "open" || operation === "goto") && typeof input.url === "string") { @@ -3842,6 +4103,7 @@ const operations = [ "focus", "doctor", "tabs", + "close", "steps", "run_script", ]; @@ -3869,6 +4131,7 @@ Current-mode operations: - focus: one-shot human-in-the-loop focus (tab|window) with reason; does not change the popup default (background). - doctor: health check. Returns status, tabCount, and the active tab. verbose lists every tab; prefer tabs for inventory. - tabs: list every http(s) tab (active/focused first). Fast inventory — do not use doctor to find a tab. +- close: close a tab. Needs pageName, query/target, or groupTitle; never the active tab by default. Refuses when the selector matches several tabs unless all:true (a lone groupTitle closes the whole group). Only tabs Browser Hand opened are closed; a tab the user had open is refused unless includeUserTabs:true. Returns { closed, targetId, url, title }; all/groupTitle return a tabs list plus skipped. Targeting: - target defaults to { strategy: "active" }. @@ -4032,6 +4295,20 @@ Resolved placeholder values are redacted from tool output.`, type: "boolean", description: "Current screenshot: capture beyond viewport when true.", }, + groupTitle: { + type: "string", + description: + "open/goto: name the Chrome tab group. close: close every tab in the Browser Hand tab group with this title.", + }, + all: { + type: "boolean", + description: "close only: allow a selector that matches several tabs to close all of them.", + }, + includeUserTabs: { + type: "boolean", + description: + "close only: also close tabs Browser Hand did not open (tabs the user had open). Off by default.", + }, }, required: [], }, diff --git a/cli-js/test/close-tab.test.js b/cli-js/test/close-tab.test.js new file mode 100644 index 0000000..638f20b --- /dev/null +++ b/cli-js/test/close-tab.test.js @@ -0,0 +1,360 @@ +/** + * `browser-hand close`: explicit selector, no accidental multi-close, and only + * tabs Browser Hand opened unless the caller overrides. + * + * The relay stamps `browserHandOwned` on every Target.getTargets entry and the + * extension stamps `groupTitle`; these tests feed planCloseTargets the same + * shapes without Chrome. + */ + +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import path from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +import { buildHandlerInput, parseArgs } from "../src/cli.js"; +import { + buildCloseResult, + filterTabTargets, + isControllablePageTarget, + planCloseTargets, + waitForTargetsClosed, +} from "../src/tool.js"; + +const cli = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../src/cli.js"); + +function page(id, partial = {}) { + return { + type: "page", + targetId: id, + title: partial.title || `Tab ${id}`, + url: partial.url || `https://example.com/${id}`, + browserHandOwned: partial.owned === true, + ...(partial.groupTitle ? { groupTitle: partial.groupTitle } : {}), + }; +} + +const ids = (tabs) => tabs.map((tab) => tab.targetId); + +test("close needs an explicit selector and never falls back to the active tab", () => { + const targets = [{ ...page("tab-1", { owned: true }), active: true, focused: true }]; + assert.throws(() => planCloseTargets(targets, {}), /never closes the active tab/); + assert.throws(() => planCloseTargets(targets, { target: {} }), /never closes the active tab/); +}); + +test("a page name cannot be mixed with other selectors", () => { + assert.throws( + () => + planCloseTargets([page("tab-1", { owned: true })], { + pageName: "work", + namedTargetId: "tab-1", + target: { query: "example" }, + }), + /cannot be combined/ + ); +}); + +test("a named page closes by exact id, including a tab still on about:blank", () => { + const blank = { ...page("tab-7", { owned: true }), url: "about:blank" }; + const plan = planCloseTargets([blank, page("tab-8", { owned: true })], { + pageName: "work", + namedTargetId: "tab-7", + }); + assert.deepEqual(ids(plan.closable), ["tab-7"]); +}); + +test("a selector that matches nothing is an error that names the selector", () => { + assert.throws( + () => planCloseTargets([page("tab-1", { owned: true })], { target: { query: "stripe" } }), + /No Chrome tab matched .*stripe/ + ); +}); + +test("a selector matching several tabs is refused unless --all is passed", () => { + const targets = [ + page("tab-1", { owned: true, title: "Stripe A" }), + page("tab-2", { owned: true, title: "Stripe B" }), + ]; + assert.throws( + () => planCloseTargets(targets, { target: { query: "stripe" } }), + /Ambiguous Chrome tab target .* matched 2 tabs.*--all/ + ); + const plan = planCloseTargets(targets, { target: { query: "stripe" }, all: true }); + assert.deepEqual(ids(plan.closable), ["tab-1", "tab-2"]); +}); + +test("ambiguity counts the user's tabs too, not just the ones Browser Hand opened", () => { + const targets = [ + page("tab-1", { owned: true, title: "Docs" }), + page("tab-2", { owned: false, title: "Docs" }), + ]; + assert.throws(() => planCloseTargets(targets, { target: { query: "docs" } }), /Ambiguous/); +}); + +test("a tab the user had open is refused by default", () => { + const targets = [page("tab-1", { owned: false, title: "Gmail" })]; + assert.throws( + () => planCloseTargets(targets, { target: { query: "gmail" } }), + /Refusing to close a tab Browser Hand did not open.*--include-user-tabs/ + ); +}); + +test("--include-user-tabs closes a tab Browser Hand did not open", () => { + const targets = [page("tab-1", { owned: false, title: "Gmail" })]; + const plan = planCloseTargets(targets, { target: { query: "gmail" }, includeUserTabs: true }); + assert.deepEqual(ids(plan.closable), ["tab-1"]); + assert.deepEqual(plan.skipped, []); +}); + +test("--all closes the tabs Browser Hand opened and reports the user's as skipped", () => { + const targets = [ + page("tab-1", { owned: true, title: "Docs one" }), + page("tab-2", { owned: false, title: "Docs two" }), + page("tab-3", { owned: true, title: "Docs three" }), + ]; + const plan = planCloseTargets(targets, { target: { query: "docs" }, all: true }); + assert.deepEqual(ids(plan.closable), ["tab-1", "tab-3"]); + assert.equal(plan.matched, 3); + assert.equal(plan.skipped.length, 1); + assert.equal(plan.skipped[0].id, "tab-2"); + assert.match(plan.skipped[0].reason, /not opened by Browser Hand/); +}); + +test("--all with only user tabs is refused rather than closing nothing quietly", () => { + const targets = [page("tab-1", { owned: false }), page("tab-2", { owned: false })]; + assert.throws( + () => planCloseTargets(targets, { target: { query: "example" }, all: true }), + /Refusing to close tabs Browser Hand did not open/ + ); +}); + +test("a lone group title closes the whole group without --all", () => { + const targets = [ + page("tab-1", { owned: true, groupTitle: "DNS records" }), + page("tab-2", { owned: true, groupTitle: "DNS records" }), + page("tab-3", { owned: true, groupTitle: "Invoice review" }), + page("tab-4", { owned: true }), + ]; + const plan = planCloseTargets(targets, { groupTitle: " DNS records " }); + assert.deepEqual(ids(plan.closable), ["tab-1", "tab-2"]); +}); + +test("a group close leaves the user's tabs in that group open", () => { + const targets = [ + page("tab-1", { owned: true, groupTitle: "DNS records" }), + page("tab-2", { owned: false, groupTitle: "DNS records" }), + ]; + const plan = planCloseTargets(targets, { groupTitle: "DNS records" }); + assert.deepEqual(ids(plan.closable), ["tab-1"]); + assert.deepEqual( + plan.skipped.map((tab) => tab.id), + ["tab-2"] + ); +}); + +test("close selects Browser Hand tabs that are still on about:blank or file://", () => { + const blank = { ...page("tab-1", { owned: true }), url: "about:blank" }; + const local = { ...page("tab-2", { owned: true }), url: "file:///tmp/gym/index.html" }; + const targets = [blank, local, page("tab-3", { owned: true })]; + assert.deepEqual(ids(planCloseTargets(targets, { target: { id: "tab-1" } }).closable), ["tab-1"]); + assert.deepEqual(ids(planCloseTargets(targets, { target: { id: "tab-2" } }).closable), ["tab-2"]); +}); + +test("a group close takes every tab in the group, including about:blank and file:// ones", () => { + const group = { owned: true, groupTitle: "DNS records" }; + const targets = [ + { ...page("tab-1", group), url: "about:blank" }, + { ...page("tab-2", group), url: "file:///tmp/gym/index.html" }, + page("tab-3", group), + ]; + const plan = planCloseTargets(targets, { groupTitle: "DNS records" }); + assert.deepEqual(ids(plan.closable), ["tab-1", "tab-2", "tab-3"]); + assert.equal(plan.matched, 3); +}); + +test("a user's about:blank or file:// tab in a group is reported as skipped, not dropped", () => { + const user = { owned: false, groupTitle: "DNS records" }; + const targets = [ + page("tab-1", { owned: true, groupTitle: "DNS records" }), + { ...page("tab-2", user), url: "file:///Users/me/notes.html" }, + { ...page("tab-3", user), url: "about:blank" }, + ]; + const plan = planCloseTargets(targets, { groupTitle: "DNS records" }); + assert.deepEqual(ids(plan.closable), ["tab-1"]); + assert.deepEqual( + plan.skipped.map((tab) => tab.id), + ["tab-2", "tab-3"] + ); +}); + +test("close sees the pages the extension lists, while tabs/snapshot/click stay http(s)-only", () => { + const targets = [ + { type: "page", targetId: "web", url: "https://example.com/" }, + { type: "page", targetId: "blank", url: "about:blank" }, + { type: "page", targetId: "blank-q", url: "about:blank?x=1" }, + { type: "page", targetId: "local", url: "file:///tmp/a.html" }, + { type: "page", targetId: "chrome", url: "chrome://extensions/" }, + { type: "page", targetId: "ext", url: "chrome-extension://abc/page.html" }, + { type: "page", targetId: "lookalike", url: "about:blankly" }, + { type: "service_worker", targetId: "sw", url: "https://example.com/sw.js" }, + ]; + assert.deepEqual(ids(targets.filter(isControllablePageTarget)), [ + "web", + "blank", + "blank-q", + "local", + ]); + assert.deepEqual(ids(filterTabTargets(targets, {})), ["web"]); + assert.deepEqual(ids(filterTabTargets(targets, {}, { includeNonHttp: true })), [ + "web", + "blank", + "blank-q", + "local", + ]); +}); + +test("a group title narrowed by --query still needs --all to close several tabs", () => { + const targets = [ + page("tab-1", { owned: true, title: "A", groupTitle: "DNS records" }), + page("tab-2", { owned: true, title: "B", groupTitle: "DNS records" }), + ]; + const opts = { groupTitle: "DNS records", target: { query: "example.com" } }; + assert.throws(() => planCloseTargets(targets, opts), /Ambiguous/); + assert.equal(planCloseTargets(targets, { ...opts, all: true }).closable.length, 2); +}); + +test("an unknown group says when no tab reports a group title at all", () => { + assert.throws( + () => planCloseTargets([page("tab-1", { owned: true })], { groupTitle: "DNS records" }), + /No tab reports a group title; reload the Browser Hand extension/ + ); + assert.throws( + () => + planCloseTargets([page("tab-1", { owned: true, groupTitle: "Other" })], { + groupTitle: "DNS records", + }), + (err) => /No Chrome tab matched/.test(err.message) && !/reload/.test(err.message) + ); +}); + +test("a relay that never reports ownership fails closed with a restart hint", () => { + const legacy = [{ type: "page", targetId: "tab-1", title: "Docs", url: "https://example.com/" }]; + assert.throws( + () => planCloseTargets(legacy, { target: { id: "tab-1" } }), + /predates ownership tracking.*npm run relay/ + ); + // The explicit override does not depend on ownership data. + const plan = planCloseTargets(legacy, { target: { id: "tab-1" }, includeUserTabs: true }); + assert.deepEqual(ids(plan.closable), ["tab-1"]); +}); + +test("waitForTargetsClosed returns once the tabs are gone", async () => { + const lists = [[{ targetId: "tab-1" }, { targetId: "tab-2" }], [{ targetId: "tab-2" }], []]; + let calls = 0; + const stillOpen = await waitForTargetsClosed( + async () => lists[Math.min(calls++, lists.length - 1)], + ["tab-1", "tab-2"], + { sleep: async () => {} } + ); + assert.equal(stillOpen.size, 0); + assert.equal(calls, 3); +}); + +test("waitForTargetsClosed reports a tab that never leaves the list", async () => { + const stillOpen = await waitForTargetsClosed( + async () => [{ targetId: "tab-1" }, { targetId: "tab-9" }], + ["tab-1", "tab-2"], + { timeoutMs: 20, intervalMs: 5 } + ); + assert.deepEqual([...stillOpen], ["tab-1"]); +}); + +test("a single close returns { closed, targetId, url, title }", () => { + const result = buildCloseResult({ + tabs: [{ closed: true, targetId: "tab-1", url: "https://example.com/", title: "Example" }], + pageName: "demo", + }); + assert.equal(result.success, true); + assert.equal(result.operation, "close"); + assert.equal(result.closed, true); + assert.equal(result.targetId, "tab-1"); + assert.equal(result.url, "https://example.com/"); + assert.equal(result.title, "Example"); + assert.equal(result.pageName, "demo"); +}); + +test("a tab that is still open after close is not reported as closed", () => { + const result = buildCloseResult({ + tabs: [{ closed: false, targetId: "tab-1", url: "u", title: "t", error: "still open" }], + }); + assert.equal(result.success, false); + assert.equal(result.closed, false); + assert.equal(result.error, "still open"); +}); + +test("a multi close lists every tab and only succeeds when all of them closed", () => { + const tabs = [ + { closed: true, targetId: "tab-1", url: "u1", title: "t1" }, + { closed: false, targetId: "tab-2", url: "u2", title: "t2", error: "still open" }, + ]; + const partial = buildCloseResult({ tabs, multi: true, groupTitle: "DNS records" }); + assert.equal(partial.success, false); + assert.equal(partial.closed, false); + assert.equal(partial.count, 1); + assert.equal(partial.groupTitle, "DNS records"); + assert.equal(partial.tabs.length, 2); + + const whole = buildCloseResult({ tabs: [tabs[0]], multi: true, skipped: [{ id: "tab-9" }] }); + assert.equal(whole.success, true); + assert.equal(whole.closed, true); + assert.deepEqual(whole.skipped, [{ id: "tab-9" }]); +}); + +test("the CLI parses --all and --include-user-tabs as flags that take no value", () => { + const args = parseArgs(["close", "--all", "--include-user-tabs", "--query", "docs"]); + assert.equal(args.all, true); + assert.equal(args.includeUserTabs, true); + assert.equal(args.query, "docs"); + + const input = buildHandlerInput("close", args); + assert.equal(input.operation, "close"); + assert.equal(input.all, true); + assert.equal(input.includeUserTabs, true); + assert.deepEqual(input.target, { query: "docs" }); +}); + +test("the CLI passes --group-title through to close", () => { + const input = buildHandlerInput("close", parseArgs(["close", "--group-title", "DNS records"])); + assert.equal(input.groupTitle, "DNS records"); + assert.equal(input.all, undefined); + assert.equal(input.includeUserTabs, undefined); +}); + +function runCli(args) { + return spawnSync(process.execPath, [cli, ...args], { encoding: "utf8" }); +} + +test("close with no selector is an argument error", () => { + const result = runCli(["close"]); + assert.equal(result.status, 2); + assert.match(result.stderr, /close requires --page-name, --query/); +}); + +test("close --all alone is still an argument error", () => { + const result = runCli(["close", "--all"]); + assert.equal(result.status, 2); + assert.match(result.stderr, /close requires --page-name, --query/); +}); + +test("close --page-name cannot be combined with --query", () => { + const result = runCli(["close", "--page-name", "work", "--query", "docs"]); + assert.equal(result.status, 2); + assert.match(result.stderr, /cannot be combined/); +}); + +test("close --group-title without a value is an argument error", () => { + const result = runCli(["close", "--group-title"]); + assert.equal(result.status, 2); + assert.match(result.stderr, /--group-title requires a value/); +}); diff --git a/extension/__tests__/TabManager.test.ts b/extension/__tests__/TabManager.test.ts index 515a00e..94f00ed 100644 --- a/extension/__tests__/TabManager.test.ts +++ b/extension/__tests__/TabManager.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, vi } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { fakeBrowser } from "wxt/testing"; import { TabManager } from "../services/TabManager"; import type { Logger } from "../utils/logger"; @@ -419,6 +419,48 @@ describe("TabManager", () => { active: true, }); }); + + describe("tab group titles", () => { + // The stubbed chrome global has no tabGroups; tests install a partial double. + const globalChrome = chrome as unknown as { tabGroups?: unknown }; + let previousTabGroups: unknown; + + beforeEach(() => { + previousTabGroups = globalChrome.tabGroups; + }); + + afterEach(() => { + globalChrome.tabGroups = previousTabGroups; + }); + + it("stamps the tab-group title on tabs that sit in a titled group", async () => { + globalChrome.tabGroups = { + query: vi.fn().mockResolvedValue([ + { id: 11, title: "DNS records" }, + { id: 12, title: "" }, + ]), + }; + mockTabsQuery.mockResolvedValue([ + chromeTab({ id: 1, groupId: 11 }), + chromeTab({ id: 2, groupId: 12 }), + chromeTab({ id: 3, groupId: -1 }), + ]); + + const infos = await tabManager.listTargets(); + expect(infos.find((info) => info.targetId === "tab-1")?.groupTitle).toBe("DNS records"); + expect(infos.find((info) => info.targetId === "tab-2")).not.toHaveProperty("groupTitle"); + expect(infos.find((info) => info.targetId === "tab-3")).not.toHaveProperty("groupTitle"); + }); + + it("still lists tabs when tab groups cannot be read", async () => { + globalChrome.tabGroups = { query: vi.fn().mockRejectedValue(new Error("no tabGroups")) }; + mockTabsQuery.mockResolvedValue([chromeTab({ id: 1, groupId: 11 })]); + + const infos = await tabManager.listTargets(); + expect(infos).toHaveLength(1); + expect(infos[0]).not.toHaveProperty("groupTitle"); + }); + }); }); describe("evaluateViaScripting", () => { diff --git a/extension/services/TabManager.ts b/extension/services/TabManager.ts index dae795e..a811b83 100644 --- a/extension/services/TabManager.ts +++ b/extension/services/TabManager.ts @@ -177,14 +177,31 @@ export class TabManager { async listTargets(): Promise { await this.refreshFocusFlags(); const tabs = await chrome.tabs.query({}); + const groupTitles = await this.groupTitlesById(); const infos: TargetInfo[] = []; for (const tab of tabs) { const targetInfo = await this.register(tab); - if (targetInfo) infos.push(targetInfo); + if (!targetInfo) continue; + const groupTitle = tab.groupId != null ? groupTitles.get(tab.groupId) : undefined; + infos.push(groupTitle ? { ...targetInfo, groupTitle } : targetInfo); } return infos; } + /** Chrome tab-group id -> title, so `close --group-title` can find a session's tabs. */ + private async groupTitlesById(): Promise> { + const titles = new Map(); + try { + for (const group of await chrome.tabGroups.query({})) { + const title = typeof group.title === "string" ? group.title.trim() : ""; + if (title) titles.set(group.id, title); + } + } catch { + // tabGroups is unavailable in some test doubles and old Chrome builds. + } + return titles; + } + /** * Register all current browser tabs so the relay can list/target them. */ diff --git a/extension/utils/types.ts b/extension/utils/types.ts index 74716ab..124007f 100644 --- a/extension/utils/types.ts +++ b/extension/utils/types.ts @@ -93,6 +93,8 @@ export interface TargetInfo { focused?: boolean; windowId?: number; attached?: boolean; + /** Title of the Chrome tab group the tab sits in. Only stamped by listTargets. */ + groupTitle?: string; } /** How automation may take foreground focus. Default is background (never steal). */ diff --git a/relay/src/relay.ts b/relay/src/relay.ts index efb480b..d273ed1 100644 --- a/relay/src/relay.ts +++ b/relay/src/relay.ts @@ -210,6 +210,19 @@ export async function serveRelay(options: RelayOptions = {}): Promise { const info = target.targetInfo as TargetInfo & { focused?: boolean; active?: boolean }; @@ -446,7 +459,13 @@ export async function serveRelay(options: RelayOptions = {}): Promise ({ ...targetInfo, attached: true, + browserHandOwned: isBrowserHandOwned(targetInfo.targetId), })), }; } @@ -550,6 +570,7 @@ export async function serveRelay(options: RelayOptions = {}): Promise ({ ...t.targetInfo, attached: true, + browserHandOwned: isBrowserHandOwned(t.targetId), })), }; } diff --git a/skills/browser-hand/SKILL.md b/skills/browser-hand/SKILL.md index 3d82707..1c78fe3 100644 --- a/skills/browser-hand/SKILL.md +++ b/skills/browser-hand/SKILL.md @@ -92,6 +92,7 @@ browser-hand steps --page-name work --quiet --steps '[{"op":"click","role":"comb browser-hand type --page-name work --label "Bio" --text "Hello" browser-hand evaluate --page-name work --code 'document.title' --quiet browser-hand screenshot --page-name work +browser-hand close --page-name work ``` Full reference: `references/extension-cli.md`. @@ -100,6 +101,7 @@ Full reference: `references/extension-cli.md`. - Prefer **named pages** (`--page-name`) for multi-step work. - On the first `open` of a task, pass `--group-title` with a few words ("DNS records", "Invoice review"). That names the Chrome tab group and gives it a color. Later commands in the same task omit it and join that group. A different title starts a new group. Leaving it off keeps the group named Browser Hand. +- Close the tabs you opened when the task is done: `close --page-name work`, or `close --group-title "DNS records"` for the whole group. `close` only closes tabs Browser Hand opened and refuses a tab the user had open. Never add `--include-user-tabs` on your own; only when the user names the tab to close. A selector that matches several tabs is refused without `--all`, and `close` never defaults to the active tab. Read `closed` in the result: it is `true` only after the tab left Chrome. - Pass **`--quiet`** on snapshot, click, fill, evaluate, and steps. It drops the repeated `mode` / `target` / `url` / `title` wrapper. - `doctor` returns `status`, `browser: {name, version}`, `tabCount`, and the active tab. It is a health check. Use `tabs` for the inventory. `doctor --verbose` is only for diagnosing target selection. `doctor --browser ` asserts which connected browser you expect and warns on a mismatch. - **Never open tabs with OS commands.** Do not fall back to `open -a "Google Chrome" `, `start`, `xdg-open`, or any shell launcher — that opens the wrong browser (not the one with the extension) and loses the session. Always use `browser-hand open`/`goto`. If `open` fails, report the error; do not route around it. diff --git a/skills/browser-hand/references/extension-cli.md b/skills/browser-hand/references/extension-cli.md index f91a03d..2ad7e72 100644 --- a/skills/browser-hand/references/extension-cli.md +++ b/skills/browser-hand/references/extension-cli.md @@ -19,6 +19,7 @@ browser-hand tabs browser-hand tabs --query stripe browser-hand snapshot --quiet browser-hand open --url https://example.com --page-name smoke +browser-hand close --page-name smoke ``` Run `doctor` before authenticated multi-step work. Only `status: "tab_bootstrap_works"` means the bridge is healthy. @@ -141,6 +142,25 @@ Browser Hand keeps that tab in the session and does not reattach. Later commands Do not use `doctor` to find a tab. Doctor is a health check; `tabs` is the inventory. +## Closing tabs + +```bash +browser-hand close --page-name smoke # the tab you opened under that name +browser-hand close --query example.com --all # every Browser Hand tab that matches +browser-hand close --group-title "DNS records" # the whole Browser Hand tab group +browser-hand close --target-id tab-12 --include-user-tabs # a tab the user had open (explicit override) +``` + +Selection uses the same flags as snapshot and click (`--page-name`, `--query`, `--target-url`, `--target-title`, `--target-id`), with three differences, all for safety: + +- **An explicit selector is required.** There is no "current tab" default. +- **Several matches are refused** unless `--all` is passed. A lone `--group-title` already means the whole group. `--page-name` cannot be mixed with another selector. +- **Only tabs Browser Hand opened are closed.** The relay stamps each tab it created (`browserHandOwned`). A tab the user already had open, including one `open` adopted because it was on the same URL, is refused. With `--all` or `--group-title` those tabs are skipped and listed under `skipped`. `--include-user-tabs` lifts the rule. + +Single close: `{ "success": true, "closed": true, "targetId": "tab-12", "url": "...", "title": "..." }`. With `--all` or `--group-title` you get `closed` (true when every selected tab closed), `count`, a `tabs` list of the same four fields, and `skipped`. After the close the CLI re-reads Chrome's tab list. `closed: false` with an `error` means the tab is still open, so do not assume it is gone. Exit code is 1 in that case. + +If the error says the relay "predates ownership tracking", rebuild and restart it. If `--group-title` finds no tab, reload the extension: older builds do not report group titles. + ## Agent-oriented recovery - **Username / password manager:** debugger may detach while the tab stays open. The extension soft-detaches and can fall back to `chrome.scripting` for evaluate/fill/type/click. Re-open the page if a pure CDP feature (some screenshots) still fails.