From 7d6842b316455bcfdcd736944704903525822036 Mon Sep 17 00:00:00 2001 From: Michael Simon Date: Sun, 2 Aug 2026 17:50:20 -0700 Subject: [PATCH 01/11] Give tabs_list a filter, a limit, and a domain summary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 874-tab listing came back as 306 KB in one tool result — past the MCP client's ceiling, so the agent got nothing, and there was no narrower call to fall back to. Roughly a third of it was boilerplate and the rest was just unasked-for. - `query`: case-insensitive AND over whitespace-separated terms, matched against title and URL together. Not a regex — agent-authored regexes backtrack, and substring terms are what triage needs. - `groupBy: "domain"`: counts only, no tabs. The cheap first call that says what a backlog is made of and what to pass as `query` next. - `limit` (default 200) and `sort` (default `recent`), with `matched` and `truncated` so a partial answer can never read as a complete one. - BridgeTab omits every false or unknown field, and `browser` / `connectionId` are hoisted into the top-level `browsers` array unless a listing genuinely merged two browsers. `selectTabs` is shared and runs twice: in the extension so a backlog never crosses the socket whole, and again in Gullet over the merged results, where a per-browser limit would not be the limit the agent asked for. `groupBy` is the exception — Gullet groups the full filtered set, since truncating before grouping would corrupt the counts. Same reconstructed listing: 306 KB → 215 KB from shape alone (still too big, which is the point), 49 KB at the default limit, 0.4 KB grouped. Verified: bun run check. --- docs/BRIDGE.md | 76 +++++++++++-- gullet/src/tools.ts | 172 +++++++++++++++++++--------- gullet/tests/tools.test.ts | 89 ++++++++++++--- src/bridge-methods.ts | 32 +++--- src/bridge-protocol.ts | 209 ++++++++++++++++++++++++++++++++-- tests/bridge-protocol.test.ts | 157 ++++++++++++++++++++++++- 6 files changed, 636 insertions(+), 99 deletions(-) diff --git a/docs/BRIDGE.md b/docs/BRIDGE.md index 6950815..f073215 100644 --- a/docs/BRIDGE.md +++ b/docs/BRIDGE.md @@ -371,14 +371,14 @@ extension and Gullet so the contract is typechecked from one definition. ## Tool surface (v1) -| MCP tool | Backing APIs | Notes | -| ------------ | ------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `tabs_list` | `tabs.query` | id, title, url, `lastAccessed`, `discarded`, `pinned`; on Firefox also `hidden` (≈ other Zen workspaces). Metadata only — cheap over hundreds of tabs. | -| `tabs_load` | `tabs.reload` + `tabs.onUpdated` | Wakes discarded tabs so they can be read. Batched (≤20), three at a time, under a 30s deadline; per-tab `ready`/`pending`/`failed`. Gated on a settings toggle, default off — answers `not-enabled` until then. | -| `tab_read` | `scripting.executeScript` + existing `clip-current.ts` | Returns Defuddle markdown + metadata. Fails cleanly on discarded tabs (see below). | -| `tab_clip` | existing `clip-format.ts` + `obsidian://new` handoff | Files into the vault exactly as manual Devour does, including the Chrome redirect-page dance. | -| `tabs_close` | `tabs.remove` | Batched, ids deduplicated. Entries (title, url, pinned, window, index, private) are recorded in an undo log in `storage.local` _before_ the removal, and the batch id comes back with the result. | -| `undo_close` | reopen from the log | Safety valve for the one destructive act. Omit the batch id to undo the most recent. | +| MCP tool | Backing APIs | Notes | +| ------------ | ------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `tabs_list` | `tabs.query` | id, title, url, `windowId`, `index`, and — only when true — `lastAccessed`, `discarded`, `pinned`, `active`, and on Firefox `hidden` (≈ other Zen workspaces). Filtered with `query`, ordered with `sort`, capped by `limit`, or collapsed to counts with `groupBy: "domain"`. See below. | +| `tabs_load` | `tabs.reload` + `tabs.onUpdated` | Wakes discarded tabs so they can be read. Batched (≤20), three at a time, under a 30s deadline; per-tab `ready`/`pending`/`failed`. Gated on a settings toggle, default off — answers `not-enabled` until then. | +| `tab_read` | `scripting.executeScript` + existing `clip-current.ts` | Returns Defuddle markdown + metadata. Fails cleanly on discarded tabs (see below). | +| `tab_clip` | existing `clip-format.ts` + `obsidian://new` handoff | Files into the vault exactly as manual Devour does, including the Chrome redirect-page dance. | +| `tabs_close` | `tabs.remove` | Batched, ids deduplicated. Entries (title, url, pinned, window, index, private) are recorded in an undo log in `storage.local` _before_ the removal, and the batch id comes back with the result. | +| `undo_close` | reopen from the log | Safety valve for the one destructive act. Omit the batch id to undo the most recent. | Deliberately absent: navigate, click, type, evaluate. @@ -404,10 +404,62 @@ default off, surfaced as **Agent bridge → "Let agents load unloaded tabs"**. A returns the `not-enabled` code — distinct from `unsupported` because this one has a fix the agent can state to the user. -`tabs_list` with no `browser` argument fans out over every connected browser and tags each -tab with its origin, so discovering what is connected costs no extra round trip. The -tab-scoped tools refuse to guess between two browsers, because ids only mean something -within one. +`tabs_list` with no `browser` argument fans out over every connected browser, so +discovering what is connected costs no extra round trip. The tab-scoped tools refuse to +guess between two browsers, because ids only mean something within one — and for the same +reason a listing that actually merged two stamps `connectionId` on each tab. Only then — +when one browser contributed every tab, the top-level `browsers` entry has already said +so, and repeating it per tab is pure boilerplate. + +▸ **A listing is budgeted against a model's context, not against the socket.** The +measurement that drove this, from a real 874-tab Zen: 306 KB of JSON in one tool result, +which is past the client's tool-result ceiling — so the agent got _nothing_, and there was +no narrower call available to fall back to. `browser` and `connectionId` were constants +repeated once per tab (13%). `discarded`, `hidden`, `active`, and `pinned` were 17.6% +while carrying eleven `true` values between them — `hidden` was `false` on all 874. + +**Deleting the boilerplate was not enough, and that is the point.** Reconstructing that +listing from its reported per-field byte totals (304.4 KB, within 0.5% of the original) +and applying the shape changes alone lands at **215 KB** — a 29% cut that is still far +past the ceiling, so the call still fails and the agent still gets nothing. What makes it +usable is narrowing: the default limit brings the same listing to **49 KB**, `query: +"x.com"` to **43 KB**, and `groupBy: "domain"` to **0.4 KB**. So the hoisting and the +omission are worth having, but they are a constant factor on something that scales with +the user's backlog; only the filter changes the shape of the problem. + +Four changes, in the order they matter: + +1. **`query`, `limit`, `sort`.** The one that actually mattered: the session that produced + the measurement wanted "the x.com tabs" and had to ask for all 874 to find them. + `query` is a case-insensitive AND over whitespace-separated terms, matched against title + and URL together, so "github pull" finds a tab whose title and URL each carry one term. + Deliberately not a regex: an agent-authored regex is an unbounded backtracking risk on a + thousand strings, and substring terms are what triage actually needs. +2. **`groupBy: "domain"`** — counts only, no tabs. The real triage primitive: one cheap + call says what the backlog is made of and what to pass as `query` next. The domain is + the hostname minus `www.`, not the registrable domain: eTLD+1 needs the Public Suffix + List, which `bridge-protocol.ts` cannot take as a dependency and which goes stale, and + `mail.google.com` vs `docs.google.com` is the distinction triage wants anyway. +3. **False and unknown fields are omitted**, not sent. Absent means false; absent + `lastAccessed` means the browser reported none. +4. **Constants are hoisted** out of the tabs and into the existing top-level `browsers` + array, per the stamping rule above. + +Two things about `limit` are load-bearing. It **defaults** to `TABS_LIST_DEFAULT_LIMIT` +(200) rather than being opt-in, because the failure it prevents is total — an unbounded +listing returns nothing usable — and truncation is always visible: `matched` counts what +the filter hit, `truncated` says the answer is partial. And the default `sort` is `recent` +rather than the browser's own window order, which is what makes truncation defensible: the +tail that gets cut is the tabs the user touched longest ago, not an arbitrary slice. + +The filter/sort/limit pipeline (`selectTabs`) lives in `bridge-protocol.ts` and runs +**twice**. In the extension, so a backlog never crosses the socket whole; and again in +Gullet over the merged results, because a limit applied per browser is not the limit the +agent asked for. Running it a second time also means an older extension that ignores +`query` still yields a filtered answer instead of a flood. `groupBy` is the exception: +Gullet asks the extension for the full filtered set and groups it there, since a limit +applied before grouping would corrupt the counts — and the full list crossing loopback +costs nothing, which is the whole point of where the budget actually is. **Restoring is exact where it can be and safe where it cannot.** A batch is recreated in ascending index order within each window; inserting a low index after a high one would diff --git a/gullet/src/tools.ts b/gullet/src/tools.ts index 5ac1e41..df870e7 100644 --- a/gullet/src/tools.ts +++ b/gullet/src/tools.ts @@ -5,11 +5,17 @@ import { asRecord, BridgeRequestError, + groupTabsByDomain, isBridgeMethod, + parseTabsListParams, + selectTabs, + TABS_LIST_DEFAULT_LIMIT, + TABS_LIST_MAX_LIMIT, TABS_LOAD_MAX_BATCH, toBridgeError, type BridgeError, type BridgeMethod, + type BridgeTab, } from "../../src/bridge-protocol.js"; import type { McpTool, McpToolResult } from "./mcp.js"; import { selectAll, selectOne, type ConnectionSummary } from "./select.js"; @@ -41,9 +47,15 @@ const BROWSER_PROPERTY = { export const GULLET_INSTRUCTIONS = `Tabglutton's bridge to the user's open browser tabs. -Triage cheaply: tabs_list returns metadata only and is affordable across hundreds of -tabs, so cut on title, URL, and lastAccessed BEFORE reading anything. Only call tab_read -on the survivors. +Narrow before you list. A backlog here is hundreds to thousands of tabs and a full +listing will not fit in your context, so work down: tabs_list with groupBy: "domain" +first to see what the backlog is made of, then tabs_list with a query to pull just the +tabs you want. tabs_list is metadata only — cut on title, URL, and lastAccessed BEFORE +reading anything, and only call tab_read on the survivors. + +It answers with "matched" and "truncated", so you can always tell a complete answer from +a truncated one. If a listing comes back truncated, narrow the query — do not raise the +limit and do not page through the whole backlog. Most tabs in a large backlog are discarded (unloaded), and tab_read and tab_clip cannot reach those. Wake them with tabs_load first — one call for every survivor you mean to read @@ -61,11 +73,36 @@ export const GULLET_TOOLS: readonly McpTool[] = [ name: "tabs_list", title: "List open tabs", description: - "List the user's open tabs with metadata only — id, title, url, lastAccessed, discarded, pinned, active, window, and (Firefox/Zen) hidden. Cheap enough to run across hundreds of tabs; do your triage here before reading any page. `hidden: true` on Zen usually means the tab lives in another workspace. `discarded: true` means the tab is unloaded and cannot be read.", + `List the user's open tabs with metadata only — id, title, url, lastAccessed, windowId, index, and the flags discarded, pinned, active and (Firefox/Zen) hidden. **Flags appear only when true**: no \`discarded\` key means the tab is loaded. \`discarded: true\` means the tab is unloaded and cannot be read until tabs_load wakes it; \`hidden: true\` on Zen usually means the tab lives in another Zen workspace.\n\n` + + `Backlogs are large, so this returns the ${TABS_LIST_DEFAULT_LIMIT} most recently accessed tabs by default and reports \`matched\` (how many the filter actually hit) plus \`truncated: true\` when there were more. Narrow with \`query\` rather than raising \`limit\` — a full listing of a thousand tabs will not fit in your context.\n\n` + + `Start a triage run with \`groupBy: "domain"\`: it returns one row per domain with tab and discarded counts instead of any tabs, which is a few hundred bytes for the whole backlog and tells you what to pass as \`query\` next.`, inputSchema: { type: "object", properties: { ...BROWSER_PROPERTY, + query: { + type: "string", + description: + 'Case-insensitive filter over title and URL. Whitespace splits it into terms that must all match, in either field — "github pull" matches a tab titled "Pull request" at github.com. Do this before raising limit.', + }, + limit: { + type: "integer", + minimum: 1, + maximum: TABS_LIST_MAX_LIMIT, + description: `Max tabs (or domain groups) to return. Defaults to ${TABS_LIST_DEFAULT_LIMIT}.`, + }, + sort: { + type: "string", + enum: ["recent", "oldest", "window"], + description: + 'Order: "recent" (default, most recently accessed first), "oldest" (stale tabs first), or "window" (the order the user sees them in). Combined with limit, "recent" keeps what they were last working on and "oldest" surfaces closing candidates.', + }, + groupBy: { + type: "string", + enum: ["domain"], + description: + "Return per-domain counts instead of tabs: { domain, tabs, discarded, newest }, most tabs first. Honours query and limit. The cheap first call for triaging a backlog you have not seen.", + }, scope: { type: "string", enum: ["all", "current-window"], @@ -221,53 +258,7 @@ async function route( const { browser: _browser, ...params } = args; const summaries = await ctx.connections(); - if (name === "tabs_list") { - // Read-only and id-free, so fanning out over every browser is safe and - // saves the agent a round trip to discover what is connected. - const targets = selectAll(summaries, target); - // Each request carries its own catch, so this Promise.all can never reject: - // one browser timing out must not throw away the listing another already - // returned. A half-answer the agent can see the shape of beats no answer, - // and with two browsers attached the healthy one is usually the one being - // triaged anyway. - const perBrowser = await Promise.all( - targets.map(async (conn) => { - try { - const result = (await ctx.request(conn.connectionId, "tabs_list", params)) as { - tabs?: Array>; - }; - const tabs = (result?.tabs ?? []).map((tab) => ({ - ...tab, - browser: conn.label, - connectionId: conn.connectionId, - })); - return { tabs }; - } catch (err) { - const { code, message } = toBridgeError(err); - return { - tabs: [], - failure: { connectionId: conn.connectionId, browser: conn.label, error: code, message }, - }; - } - }), - ); - const failures = perBrowser.map((r) => r.failure).filter((f) => f !== undefined); - // Every browser failed: there is no partial answer to give, and an empty - // `tabs` array would read as "the user has no tabs" rather than as a fault. - if (failures.length === targets.length) { - const first = failures[0]; - throw new BridgeRequestError( - first?.error ?? "internal", - first?.message ?? "tabs_list failed.", - ); - } - // Tabs carry their origin so ids from two browsers can never be confused. - return { - browsers: targets, - tabs: perBrowser.flatMap((r) => r.tabs), - ...(failures.length > 0 ? { failures } : {}), - }; - } + if (name === "tabs_list") return tabsList(ctx, summaries, target, params); // Everything else is tab-scoped: ids only mean something inside one browser. const conn = selectOne(summaries, target); @@ -280,6 +271,85 @@ async function route( }; } +/** + * Fan a listing out over every connected browser and merge the answers. + * + * Read-only and id-free, so fanning out is safe and saves the agent a round trip + * to discover what is connected. The filter/sort/limit pipeline then runs a + * second time here, over the merged set: a limit applied per browser is not the + * limit the agent asked for, and re-running it is also what lets an older + * extension that ignores `query` still produce a filtered answer. + */ +async function tabsList( + ctx: ToolContext, + summaries: ConnectionSummary[], + target: string | undefined, + params: Record, +): Promise { + const listParams = parseTabsListParams(params); + const targets = selectAll(summaries, target); + // Each request carries its own catch, so this Promise.all can never reject: + // one browser timing out must not throw away the listing another already + // returned. A half-answer the agent can see the shape of beats no answer, and + // with two browsers attached the healthy one is usually the one being triaged. + const perBrowser = await Promise.all( + targets.map(async (conn) => { + try { + const result = (await ctx.request(conn.connectionId, "tabs_list", params)) as { + tabs?: BridgeTab[]; + }; + return { conn, tabs: result?.tabs ?? [] }; + } catch (err) { + const { code, message } = toBridgeError(err); + const failure = { + connectionId: conn.connectionId, + browser: conn.label, + error: code, + message, + }; + return { conn, tabs: [] as BridgeTab[], failure }; + } + }), + ); + const failures = perBrowser.map((r) => r.failure).filter((f) => f !== undefined); + // Every browser failed: there is no partial answer to give, and an empty + // `tabs` array would read as "the user has no tabs" rather than as a fault. + if (failures.length === targets.length) { + const first = failures[0]; + throw new BridgeRequestError(first?.error ?? "internal", first?.message ?? "tabs_list failed."); + } + + // Which browser a tab came from is tracked beside the tabs rather than stamped + // on them: with one browser connected — the normal case — the top-level + // `browsers` entry already says it, and repeating a constant string once per + // tab cost 13% of the listing that started this. + const origin = new WeakMap(); + const merged: BridgeTab[] = []; + for (const { conn, tabs } of perBrowser) { + for (const tab of tabs) { + origin.set(tab, conn); + merged.push(tab); + } + } + const head = { browsers: targets, ...(failures.length > 0 ? { failures } : {}) }; + + if (listParams.groupBy === "domain") { + return { ...head, ...groupTabsByDomain(merged, listParams.limit) }; + } + const selected = selectTabs(merged, listParams); + // Ids only mean something inside one browser, so a listing that actually + // merged two has to say which one each tab belongs to. The test is how many + // browsers *contributed*, not how many were asked: one of two can fail or come + // back empty, and then there is nothing to disambiguate. connectionId rather + // than the label, because labels are self-reported and two can share one. + const contributors = perBrowser.filter((r) => r.tabs.length > 0).length; + const tabs = + contributors > 1 + ? selected.tabs.map((tab) => ({ ...tab, connectionId: origin.get(tab)?.connectionId })) + : selected.tabs; + return { ...head, ...selected, tabs }; +} + // Compact JSON, not pretty-printed: every one of these results goes into a // model's context, and a 300-tab listing does not need indentation. function ok(value: unknown): McpToolResult { diff --git a/gullet/tests/tools.test.ts b/gullet/tests/tools.test.ts index 07affe8..914bd5c 100644 --- a/gullet/tests/tools.test.ts +++ b/gullet/tests/tools.test.ts @@ -66,17 +66,36 @@ describe("tool definitions", () => { }); describe("tabs_list", () => { - test("fans out over every browser and tags each tab with its origin", async () => { - const { call } = caller([zen, chrome], ({ connectionId }) => ({ - tabs: [{ id: connectionId === "conn-1" ? 1 : 2 }], - })); - const result = await call("tabs_list", {}); - expect(payload(result)).toEqual({ + const tab = (id: number, url: string, lastAccessed?: number): Record => ({ + id, + title: `tab ${id}`, + url, + ...(lastAccessed === undefined ? {} : { lastAccessed }), + }); + + test("fans out over every browser and stamps origin only on a merged listing", async () => { + const { call } = caller([zen, chrome], ({ connectionId }) => + connectionId === "conn-1" + ? { tabs: [tab(1, "https://a.test/")] } + : { tabs: [tab(2, "https://b.test/")] }, + ); + expect(payload(await call("tabs_list", {}))).toEqual({ browsers: [zen, chrome], tabs: [ - { id: 1, browser: "Zen", connectionId: "conn-1" }, - { id: 2, browser: "Chrome", connectionId: "conn-2" }, + { ...tab(1, "https://a.test/"), connectionId: "conn-1" }, + { ...tab(2, "https://b.test/"), connectionId: "conn-2" }, ], + matched: 2, + }); + }); + + test("leaves the origin off when only one browser is connected", async () => { + const { call } = caller([zen], () => ({ tabs: [tab(1, "https://a.test/")] })); + // The constant used to be repeated once per tab; `browsers` already says it. + expect(payload(await call("tabs_list", {}))).toEqual({ + browsers: [zen], + tabs: [tab(1, "https://a.test/")], + matched: 1, }); }); @@ -88,14 +107,58 @@ describe("tabs_list", () => { test("forwards its own params but not the routing field", async () => { const { call, sent } = caller([zen], () => ({ tabs: [] })); - await call("tabs_list", { browser: "Zen", scope: "current-window", includeHidden: false }); - expect(sent[0]?.params).toEqual({ scope: "current-window", includeHidden: false }); + await call("tabs_list", { browser: "Zen", scope: "current-window", query: "x" }); + expect(sent[0]?.params).toEqual({ scope: "current-window", query: "x" }); }); test("tolerates a browser that returns no tabs field", async () => { const { call } = caller([zen], () => ({})); expect(payload(await call("tabs_list", {}))).toMatchObject({ tabs: [] }); }); + + test("rejects bad arguments before dialling any browser", async () => { + const { call, sent } = caller([zen], () => ({ tabs: [] })); + const result = await call("tabs_list", { sort: "alphabetical" }); + expect(result.isError).toBe(true); + expect(payload(result)).toMatchObject({ error: "bad-request" }); + expect(sent).toEqual([]); + }); + + // Filtering and truncation run again here, over the merged set: an extension + // that ignored `query` must not flood the agent anyway, and a per-browser + // limit is not the limit the agent asked for. + test("re-applies query and limit across browsers that ignored them", async () => { + const { call } = caller([zen, chrome], ({ connectionId }) => + connectionId === "conn-1" + ? { tabs: [tab(1, "https://x.com/a", 100), tab(2, "https://other.test/", 400)] } + : { tabs: [tab(3, "https://x.com/b", 300), tab(4, "https://x.com/c", 200)] }, + ); + const result = payload(await call("tabs_list", { query: "x.com", limit: 2 })) as { + tabs: Array<{ id: number }>; + matched: number; + truncated: boolean; + }; + expect(result.tabs.map((t) => t.id)).toEqual([3, 4]); + expect(result).toMatchObject({ matched: 3, truncated: true }); + }); + + test("groupBy: domain answers with counts across every browser and no tabs", async () => { + const { call } = caller([zen, chrome], ({ connectionId }) => + connectionId === "conn-1" + ? { tabs: [tab(1, "https://x.com/a"), tab(2, "https://www.x.com/b")] } + : { tabs: [tab(3, "https://x.com/c"), tab(4, "https://other.test/")] }, + ); + const result = payload(await call("tabs_list", { groupBy: "domain" })); + expect(result).toEqual({ + browsers: [zen, chrome], + groups: [ + { domain: "x.com", tabs: 3, discarded: 0 }, + { domain: "other.test", tabs: 1, discarded: 0 }, + ], + domains: 2, + matched: 4, + }); + }); }); describe("tab-scoped tools", () => { @@ -235,9 +298,9 @@ describe("tabs_list with a browser that fails", () => { tabs: Array>; failures: Array>; }; - expect(result.tabs).toEqual([ - { id: 1, title: "kept", browser: zen.label, connectionId: zen.connectionId }, - ]); + // Only Zen answered, so it is the sole entry in `browsers` and the tab needs + // no per-tab origin stamped on it. + expect(result.tabs).toEqual([{ id: 1, title: "kept" }]); expect(result.failures).toEqual([ { connectionId: chrome.connectionId, diff --git a/src/bridge-methods.ts b/src/bridge-methods.ts index 8486285..b9e2962 100644 --- a/src/bridge-methods.ts +++ b/src/bridge-methods.ts @@ -16,6 +16,7 @@ import { parseTabReadParams, parseTabsCloseParams, parseTabsListParams, + selectTabs, parseTabsLoadParams, parseUndoCloseParams, TABS_LOAD_DEADLINE_MS, @@ -151,16 +152,20 @@ function toBridgeTab(tab: browser.tabs.Tab): BridgeTab | null { id: tab.id, title: tab.title ?? "", url, - lastAccessed: tab.lastAccessed ?? 0, - discarded: tab.discarded ?? false, - pinned: tab.pinned, - active: tab.active, windowId: tab.windowId ?? -1, index: tab.index, }; - // Chrome has no `tab.hidden`; omitting the key (rather than sending false) - // keeps "no workspace signal here" distinguishable from "visible". - if (!IS_CHROME && tab.hidden !== undefined) bridgeTab.hidden = tab.hidden; + // Everything below is omitted unless it says something. See BridgeTab: a + // listing is repeated once per tab into a model's context, and false flags + // were most of what it carried. + if (tab.lastAccessed !== undefined && tab.lastAccessed > 0) { + bridgeTab.lastAccessed = tab.lastAccessed; + } + if (tab.discarded === true) bridgeTab.discarded = true; + if (tab.pinned) bridgeTab.pinned = true; + if (tab.active) bridgeTab.active = true; + // Chrome has no `tab.hidden` at all, so there it is never a signal either way. + if (!IS_CHROME && tab.hidden === true) bridgeTab.hidden = true; return bridgeTab; } @@ -434,12 +439,13 @@ export class BridgeMethodRunner { params.scope === "current-window" ? await browser.tabs.query({ currentWindow: true }) : await queryAllTabs(); - const mapped = tabs - .map(toBridgeTab) - .filter((t): t is BridgeTab => t !== null) - .filter((t) => params.includeHidden || t.hidden !== true); - mapped.sort((a, b) => a.windowId - b.windowId || a.index - b.index); - return { tabs: mapped }; + const mapped = tabs.map(toBridgeTab).filter((t): t is BridgeTab => t !== null); + // `groupBy` counts every match, so truncating here would corrupt the counts. + // Gullet does the grouping, over every browser at once — and the full list + // crossing loopback costs nothing, unlike the same list crossing into a + // model's context, which is the only budget any of this is protecting. + const limit = params.groupBy === undefined ? params.limit : Number.POSITIVE_INFINITY; + return selectTabs(mapped, { ...params, limit }); } /** diff --git a/src/bridge-protocol.ts b/src/bridge-protocol.ts index 1057044..2850d9d 100644 --- a/src/bridge-protocol.ts +++ b/src/bridge-protocol.ts @@ -333,31 +333,202 @@ export function isBridgeMethod(value: unknown): value is BridgeMethod { return typeof value === "string" && (BRIDGE_METHODS as readonly string[]).includes(value); } +/** + * One tab as an agent sees it. Every field that is false or unknown is + * **omitted**, not sent — this object is repeated once per tab into a model's + * context, and on a real backlog the boilerplate outweighed the signal: + * measured over 874 tabs, `discarded`/`hidden`/`active`/`pinned` cost 17.6% of + * a 306 KB listing while carrying eleven true values between them. Absent means + * false; absent `lastAccessed` means the browser did not report one. + */ export interface BridgeTab { id: number; title: string; url: string; - /** Epoch ms of last activation; 0 when the browser does not report it. */ - lastAccessed: number; + /** Epoch ms of last activation. Omitted when the browser does not report it. */ + lastAccessed?: number; /** Unloaded tab — `tab_read`/`tab_clip` cannot run a content script in it. */ - discarded: boolean; - pinned: boolean; - active: boolean; + discarded?: boolean; + pinned?: boolean; + active?: boolean; windowId: number; index: number; /** Firefox only. On Zen this approximates "belongs to another workspace". */ hidden?: boolean; } +/** + * - `recent` (default): most recently accessed first. The triage order, and the + * one that makes `limit` mean something — the truncated tail is the tabs the + * user touched longest ago, not an arbitrary slice. + * - `oldest`: the reverse, for finding stale tabs directly. + * - `window`: browser order (window, then position), which is what the user sees. + */ +export type TabsListSort = "recent" | "oldest" | "window"; + +/** + * Default ceiling on tabs returned. A listing is the one bridge result whose + * size scales with the user's backlog, and it lands whole in a model's context: + * 874 tabs came back as 306 KB and blew past the client's tool-result limit, so + * the agent got nothing at all. Truncating is strictly better than that — and + * always reported, via `matched` and `truncated`, so an agent can see it did not + * get everything and narrow with `query` instead of guessing. + */ +export const TABS_LIST_DEFAULT_LIMIT = 200; + +/** Ceiling on an explicit `limit`. Above this a listing is not triage material. */ +export const TABS_LIST_MAX_LIMIT = 2000; + export interface TabsListParams { /** Default "all": every window. "current-window" narrows to the focused one. */ scope?: "all" | "current-window"; /** Firefox: include tabs hidden by another Zen workspace. Default true. */ includeHidden?: boolean; + /** + * Case-insensitive filter over title and URL. Whitespace splits it into terms + * that must **all** match, in either field — "github pull" finds a PR tab + * whose title says "Pull request" and whose URL says github.com. + */ + query?: string; + /** Max tabs (or domain groups) returned. Default {@link TABS_LIST_DEFAULT_LIMIT}. */ + limit?: number; + /** Default "recent". */ + sort?: TabsListSort; + /** Return per-domain counts instead of tabs. `sort` and per-tab fields do not apply. */ + groupBy?: "domain"; +} + +/** `TabsListParams` with every default filled in, as both ends act on it. */ +export interface ResolvedTabsListParams extends TabsListParams { + scope: "all" | "current-window"; + includeHidden: boolean; + sort: TabsListSort; + limit: number; } export interface TabsListResult { tabs: BridgeTab[]; + /** Tabs matching `query` before `limit` was applied. */ + matched: number; + /** True exactly when `matched > tabs.length`. Omitted otherwise. */ + truncated?: boolean; +} + +/** One domain's share of the backlog, from `tabs_list` with `groupBy: "domain"`. */ +export interface TabDomainGroup { + /** Hostname with a leading `www.` dropped; the scheme for schemeless URLs. */ + domain: string; + tabs: number; + /** How many of them are unloaded, so `tabs_load` is needed before reading. */ + discarded: number; + /** Most recent `lastAccessed` in the group. Omitted when none reported one. */ + newest?: number; +} + +export interface TabsListGroupResult { + groups: TabDomainGroup[]; + /** Distinct domains matched, before `limit`. */ + domains: number; + /** Tabs matched across every domain, including groups `limit` cut. */ + matched: number; + truncated?: boolean; +} + +/** + * The domain a tab is filed under for `groupBy`. Deliberately the hostname + * rather than the registrable domain: eTLD+1 needs the Public Suffix List, + * which is a dependency this protocol module cannot take and a table that goes + * stale, and for triage `docs.google.com` vs `mail.google.com` is the + * distinction that matters anyway. + */ +export function tabDomain(url: string): string { + let parsed: URL; + try { + parsed = new URL(url); + } catch { + return ""; + } + const host = parsed.hostname; + if (!host) return parsed.protocol.replace(/:$/, ""); + return host.startsWith("www.") ? host.slice(4) : host; +} + +/** All whitespace-separated terms present in the title or the URL, ignoring case. */ +export function matchesTabQuery(tab: BridgeTab, query: string): boolean { + const terms = query.toLowerCase().split(/\s+/).filter(Boolean); + if (terms.length === 0) return true; + const haystack = `${tab.title}\n${tab.url}`.toLowerCase(); + return terms.every((term) => haystack.includes(term)); +} + +function compareTabs(sort: TabsListSort): (a: BridgeTab, b: BridgeTab) => number { + if (sort === "window") return (a, b) => a.windowId - b.windowId || a.index - b.index; + return (a, b) => { + const x = a.lastAccessed; + const y = b.lastAccessed; + // A tab with no reported lastAccessed sorts last under *both* time orders: + // it is unknown, not ancient, and floating unknowns to the head of "oldest" + // would hand an agent hunting stale tabs a page it knows nothing about. Two + // unknowns tie, and `sort` is stable, so they keep browser order. + if (x === undefined || y === undefined) { + return x === y ? 0 : x === undefined ? 1 : -1; + } + return sort === "recent" ? y - x : x - y; + }; +} + +/** + * Filter, sort, and truncate a listing. Pure and shared, because it runs + * **twice**: in the extension, so a backlog never crosses the socket in full, + * and again in Gullet over the merged results of every connected browser, where + * a per-browser limit would not be the limit the agent asked for. Running it + * again also means an older extension that ignores `query` still yields a + * filtered answer rather than a flood. + */ +export function selectTabs(tabs: BridgeTab[], params: TabsListParams): TabsListResult { + const includeHidden = params.includeHidden ?? true; + const query = params.query?.trim() ?? ""; + const matches = tabs.filter( + (tab) => + (includeHidden || tab.hidden !== true) && (query === "" || matchesTabQuery(tab, query)), + ); + matches.sort(compareTabs(params.sort ?? "recent")); + const limit = params.limit ?? TABS_LIST_DEFAULT_LIMIT; + const result: TabsListResult = { tabs: matches.slice(0, limit), matched: matches.length }; + if (matches.length > result.tabs.length) result.truncated = true; + return result; +} + +/** + * Collapse a listing to per-domain counts — the cheap first call of a triage + * run, which tells an agent what the backlog is made of for a few hundred bytes + * instead of a few hundred kilobytes, and what to then pass as `query`. + */ +export function groupTabsByDomain(tabs: BridgeTab[], limit: number): TabsListGroupResult { + const byDomain = new Map(); + for (const tab of tabs) { + const domain = tabDomain(tab.url); + let group = byDomain.get(domain); + if (!group) { + group = { domain, tabs: 0, discarded: 0 }; + byDomain.set(domain, group); + } + group.tabs += 1; + if (tab.discarded) group.discarded += 1; + if (tab.lastAccessed !== undefined && tab.lastAccessed > (group.newest ?? -1)) { + group.newest = tab.lastAccessed; + } + } + const groups = [...byDomain.values()].sort( + (a, b) => b.tabs - a.tabs || a.domain.localeCompare(b.domain), + ); + const result: TabsListGroupResult = { + groups: groups.slice(0, limit), + domains: groups.length, + matched: tabs.length, + }; + if (groups.length > result.groups.length) result.truncated = true; + return result; } /** @@ -558,7 +729,7 @@ function badRequest(message: string): never { throw new BridgeRequestError("bad-request", message); } -export function parseTabsListParams(raw: unknown): TabsListParams { +export function parseTabsListParams(raw: unknown): ResolvedTabsListParams { const obj = asRecord(raw) ?? {}; const scope = obj.scope; if (scope !== undefined && scope !== "all" && scope !== "current-window") { @@ -568,7 +739,31 @@ export function parseTabsListParams(raw: unknown): TabsListParams { if (includeHidden !== undefined && typeof includeHidden !== "boolean") { badRequest("includeHidden must be a boolean"); } - return { scope: scope ?? "all", includeHidden: includeHidden ?? true }; + const query = obj.query; + if (query !== undefined && typeof query !== "string") badRequest("query must be a string"); + const sort = obj.sort; + if (sort !== undefined && sort !== "recent" && sort !== "oldest" && sort !== "window") { + badRequest(`sort must be "recent", "oldest", or "window"`); + } + const groupBy = obj.groupBy; + if (groupBy !== undefined && groupBy !== "domain") badRequest(`groupBy must be "domain"`); + const limit = obj.limit; + if (limit !== undefined) { + if (typeof limit !== "number" || !Number.isInteger(limit) || limit < 1) { + badRequest("limit must be a positive integer"); + } + if (limit > TABS_LIST_MAX_LIMIT) { + badRequest(`limit must be at most ${TABS_LIST_MAX_LIMIT}; narrow with query instead`); + } + } + return { + scope: scope ?? "all", + includeHidden: includeHidden ?? true, + sort: sort ?? "recent", + limit: limit ?? TABS_LIST_DEFAULT_LIMIT, + ...(query !== undefined && query.trim() !== "" ? { query: query.trim() } : {}), + ...(groupBy !== undefined ? { groupBy } : {}), + }; } function requireTabId(raw: unknown): number { diff --git a/tests/bridge-protocol.test.ts b/tests/bridge-protocol.test.ts index 7d08286..47bff36 100644 --- a/tests/bridge-protocol.test.ts +++ b/tests/bridge-protocol.test.ts @@ -8,7 +8,9 @@ import { classifyBridgeProbe, deriveProof, generateToken, + groupTabsByDomain, isBridgeMethod, + matchesTabQuery, orderedBridgePortCandidates, parseMessage, parseTabClipParams, @@ -17,11 +19,21 @@ import { parseTabsListParams, parseTabsLoadParams, parseUndoCloseParams, + selectTabs, + tabDomain, + TABS_LIST_DEFAULT_LIMIT, + TABS_LIST_MAX_LIMIT, TABS_LOAD_MAX_BATCH, proofsMatch, randomNonce, + type BridgeTab, } from "../src/bridge-protocol.js"; +/** A listing entry with only the fields a case actually exercises. */ +function makeTab(fields: Partial & Pick): BridgeTab { + return { title: "", windowId: 1, index: 0, ...fields }; +} + describe("constants", () => { test("port and proto are the documented values", () => { expect(DEFAULT_BRIDGE_PORT).toBe(4589); @@ -151,14 +163,32 @@ describe("parseMessage()", () => { }); describe("parseTabsListParams()", () => { - test("defaults to every window, hidden included", () => { - expect(parseTabsListParams(undefined)).toEqual({ scope: "all", includeHidden: true }); + test("defaults to every window, hidden included, newest first, capped", () => { + expect(parseTabsListParams(undefined)).toEqual({ + scope: "all", + includeHidden: true, + sort: "recent", + limit: TABS_LIST_DEFAULT_LIMIT, + }); }); test("accepts the documented values", () => { - expect(parseTabsListParams({ scope: "current-window", includeHidden: false })).toEqual({ + expect( + parseTabsListParams({ + scope: "current-window", + includeHidden: false, + query: "github", + limit: 5, + sort: "oldest", + groupBy: "domain", + }), + ).toEqual({ scope: "current-window", includeHidden: false, + query: "github", + limit: 5, + sort: "oldest", + groupBy: "domain", }); }); @@ -169,6 +199,127 @@ describe("parseTabsListParams()", () => { test("rejects a non-boolean includeHidden", () => { expect(() => parseTabsListParams({ includeHidden: "yes" })).toThrow(BridgeRequestError); }); + + test("rejects an unknown sort or groupBy", () => { + expect(() => parseTabsListParams({ sort: "alphabetical" })).toThrow(BridgeRequestError); + expect(() => parseTabsListParams({ groupBy: "window" })).toThrow(BridgeRequestError); + }); + + test("rejects a limit that is not a positive integer within the ceiling", () => { + expect(() => parseTabsListParams({ limit: 0 })).toThrow(BridgeRequestError); + expect(() => parseTabsListParams({ limit: 1.5 })).toThrow(BridgeRequestError); + expect(() => parseTabsListParams({ limit: TABS_LIST_MAX_LIMIT + 1 })).toThrow( + BridgeRequestError, + ); + }); + + // A query of only whitespace would otherwise reach `selectTabs` as a filter + // that matches everything, and be reported back as if it had narrowed. + test("drops a blank query rather than carrying it", () => { + expect(parseTabsListParams({ query: " " })).not.toHaveProperty("query"); + expect(parseTabsListParams({ query: " github " }).query).toBe("github"); + }); +}); + +describe("tabDomain()", () => { + test("strips www but keeps the subdomain that distinguishes a service", () => { + expect(tabDomain("https://www.github.com/a/b")).toBe("github.com"); + expect(tabDomain("https://mail.google.com/")).toBe("mail.google.com"); + expect(tabDomain("https://docs.google.com/")).toBe("docs.google.com"); + }); + + test("falls back to the scheme when there is no host, and to empty when unparseable", () => { + expect(tabDomain("about:blank")).toBe("about"); + expect(tabDomain("file:///Users/x/n.md")).toBe("file"); + expect(tabDomain("not a url")).toBe(""); + }); +}); + +describe("matchesTabQuery()", () => { + const tab = makeTab({ id: 1, title: "Pull request #42", url: "https://github.com/o/r/pull/42" }); + + test("matches case-insensitively across title and url", () => { + expect(matchesTabQuery(tab, "PULL REQUEST")).toBe(true); + expect(matchesTabQuery(tab, "github.com")).toBe(true); + }); + + test("requires every term, but lets them land in different fields", () => { + expect(matchesTabQuery(tab, "github pull")).toBe(true); + expect(matchesTabQuery(tab, "github issue")).toBe(false); + }); +}); + +describe("selectTabs()", () => { + const tabs = [ + makeTab({ id: 1, title: "old", url: "https://x.com/a", lastAccessed: 100 }), + makeTab({ id: 2, title: "new", url: "https://x.com/b", lastAccessed: 300 }), + makeTab({ id: 3, title: "mid", url: "https://news.example/c", lastAccessed: 200 }), + makeTab({ id: 4, title: "unknown", url: "https://x.com/d" }), + ]; + + test("defaults to most recent first", () => { + expect(selectTabs(tabs, {}).tabs.map((t) => t.id)).toEqual([2, 3, 1, 4]); + }); + + test("sorts oldest first without floating unknowns to the top", () => { + // A tab with no lastAccessed is unknown, not ancient — putting it first + // would hand an agent hunting stale tabs a page it knows nothing about. + expect(selectTabs(tabs, { sort: "oldest" }).tabs.map((t) => t.id)).toEqual([1, 3, 2, 4]); + }); + + test("filters on query and reports what the filter hit", () => { + const result = selectTabs(tabs, { query: "x.com" }); + expect(result.tabs.map((t) => t.id)).toEqual([2, 1, 4]); + expect(result.matched).toBe(3); + expect(result.truncated).toBeUndefined(); + }); + + test("flags truncation so a partial answer is never read as a complete one", () => { + const result = selectTabs(tabs, { limit: 2 }); + expect(result.tabs.map((t) => t.id)).toEqual([2, 3]); + expect(result.matched).toBe(4); + expect(result.truncated).toBe(true); + }); + + test("counts matches before the limit, not after", () => { + expect(selectTabs(tabs, { query: "x.com", limit: 1 }).matched).toBe(3); + }); + + test("drops hidden tabs only when asked", () => { + const withHidden = [...tabs, makeTab({ id: 5, url: "https://h/", hidden: true })]; + expect(selectTabs(withHidden, { includeHidden: false }).tabs.map((t) => t.id)).not.toContain(5); + expect(selectTabs(withHidden, {}).tabs.map((t) => t.id)).toContain(5); + }); +}); + +describe("groupTabsByDomain()", () => { + const tabs = [ + makeTab({ id: 1, url: "https://x.com/a", lastAccessed: 100, discarded: true }), + makeTab({ id: 2, url: "https://www.x.com/b", lastAccessed: 300 }), + makeTab({ id: 3, url: "https://x.com/c", discarded: true }), + makeTab({ id: 4, url: "https://news.example/d", lastAccessed: 200 }), + ]; + + test("counts per domain, busiest first", () => { + const result = groupTabsByDomain(tabs, 10); + expect(result.groups).toEqual([ + { domain: "x.com", tabs: 3, discarded: 2, newest: 300 }, + { domain: "news.example", tabs: 1, discarded: 0, newest: 200 }, + ]); + expect(result).toMatchObject({ domains: 2, matched: 4 }); + expect(result.truncated).toBeUndefined(); + }); + + test("omits newest when no tab in the group reported one", () => { + const [group] = groupTabsByDomain([makeTab({ id: 1, url: "https://q/" })], 10).groups; + expect(group).not.toHaveProperty("newest"); + }); + + test("truncates groups but still counts every tab behind them", () => { + const result = groupTabsByDomain(tabs, 1); + expect(result.groups.map((g) => g.domain)).toEqual(["x.com"]); + expect(result).toMatchObject({ domains: 2, matched: 4, truncated: true }); + }); }); describe("parseTabReadParams()", () => { From 10c5ff8f7cc14bff7e286404ffb149fe69f1cd0c Mon Sep 17 00:00:00 2001 From: Michael Simon Date: Sun, 2 Aug 2026 18:35:34 -0700 Subject: [PATCH 02/11] Halve the per-tab cost of a listing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the tabs_list triage work: the shape changes there cut 306 KB to 215 KB, but 23% of what remained was JSON key names repeated 874 times and another 10% was two fields carrying almost nothing. Measured per field rather than guessed. - `index` is gone. It duplicated array order under `sort: "window"`, meant nothing under the others, and nothing read it — the undo log takes position from the live `browser.tabs.Tab`. - `windowId` hoists to the top level when every tab shares one window, which on a single-window Zen is all of them. Not hoisted across a merged listing: two browsers can each call their window 1. - Titles clip at 120 with a trailing `…`. There is no gentler cap — the mean title in that backlog was ~104 chars — and what a clip loses is the site suffix the URL already gives you. - URLs are trimmed structurally, not clipped: `displayUrl` drops tracking params, `www.`, and the trailing slash, keeping the scheme (copyable) and the fragment (an SPA's whole page identity). `TAB_URL_MAX` is a backstop for data: URIs, not the mechanism. `isTrackingParam` is now exported from normalize.ts so there is one list, not two. Rendering is a separate pass from selection and runs once, in Gullet, at the very end — never in the extension. Gullet re-applies `query` over the merged results, and matching against text that clipping had removed would silently drop the tab the agent asked for. Filters see whole strings; only what reaches the model is trimmed. It also tolerates a tab missing title or url rather than throwing away the listing around it. Same 874 tabs: 363 → 185 bytes/tab, and 36 KB at the default limit (was 306 KB, which failed outright). groupBy stays 0.4 KB. Verified: bun run check. --- docs/BRIDGE.md | 65 +++++++++++++++--- gullet/src/tools.ts | 22 ++++-- gullet/tests/tools.test.ts | 46 +++++++++++-- src/normalize.ts | 18 +++-- src/tabs-view.ts | 136 +++++++++++++++++++++++++++++++++++++ tests/tabs-view.test.ts | 104 ++++++++++++++++++++++++++++ 6 files changed, 365 insertions(+), 26 deletions(-) create mode 100644 src/tabs-view.ts create mode 100644 tests/tabs-view.test.ts diff --git a/docs/BRIDGE.md b/docs/BRIDGE.md index f073215..26c2e5f 100644 --- a/docs/BRIDGE.md +++ b/docs/BRIDGE.md @@ -419,15 +419,23 @@ repeated once per tab (13%). `discarded`, `hidden`, `active`, and `pinned` were while carrying eleven `true` values between them — `hidden` was `false` on all 874. **Deleting the boilerplate was not enough, and that is the point.** Reconstructing that -listing from its reported per-field byte totals (304.4 KB, within 0.5% of the original) -and applying the shape changes alone lands at **215 KB** — a 29% cut that is still far +listing from its reported per-field byte totals (within 0.5% of the original) and applying +the shape changes alone lands at **215 KB, 252 bytes a tab** — a 29% cut that is still far past the ceiling, so the call still fails and the agent still gets nothing. What makes it -usable is narrowing: the default limit brings the same listing to **49 KB**, `query: -"x.com"` to **43 KB**, and `groupBy: "domain"` to **0.4 KB**. So the hoisting and the -omission are worth having, but they are a constant factor on something that scales with -the user's backlog; only the filter changes the shape of the problem. +usable is narrowing. Measured over the same 874 tabs: -Four changes, in the order they matter: +| | whole backlog | per tab | at the default limit | +| --------------------------------------------------------------- | ------------- | ------- | -------------------- | +| original | 306 KB | 363 B | — (no limit existed) | +| constants hoisted, false flags dropped | 215 KB | 252 B | 49 KB | +| `index` dropped, `windowId` hoisted, title clipped, URL trimmed | 158 KB | 185 B | **36 KB** | +| `groupBy: "domain"` | **0.4 KB** | — | — | + +So the shape work is worth having — it halved the per-tab cost, and per-tab cost is what +decides how many tabs fit under a given limit — but it is a constant factor on something +that scales with the user's backlog. Only the filter changes the shape of the problem. + +Five changes, in the order they matter: 1. **`query`, `limit`, `sort`.** The one that actually mattered: the session that produced the measurement wanted "the x.com tabs" and had to ask for all 874 to find them. @@ -442,8 +450,27 @@ Four changes, in the order they matter: `mail.google.com` vs `docs.google.com` is the distinction triage wants anyway. 3. **False and unknown fields are omitted**, not sent. Absent means false; absent `lastAccessed` means the browser reported none. -4. **Constants are hoisted** out of the tabs and into the existing top-level `browsers` - array, per the stamping rule above. +4. **Constants are hoisted** out of the tabs — `browser`/`connectionId` into the existing + top-level `browsers` array per the stamping rule above, and `windowId` to the top level + whenever every tab shares one window, which on a single-window Zen is all of them. + `index` is gone outright: it duplicated the array order under `sort: "window"`, meant + nothing under the others, and nothing consumed it — the undo log takes position from + the live `browser.tabs.Tab`, not from a listing. +5. **Titles are clipped and URLs trimmed** (`src/tabs-view.ts`). Titles at + `TAB_TITLE_MAX` (120) with a trailing `…`; there is no gentler cap, because the mean + title in that backlog was ~104 characters, so anything tighter cuts into the body of + the distribution rather than its tail. What a clipped title loses is cheap — titles are + front-loaded and the tail is usually the site suffix (`" | GitHub"`) the URL already + gives you. + + URLs are **not** clipped by default, because a URL cut mid-string stops being a URL: + it cannot be handed back to the user, and two distinct tabs can clip to the same prefix + and read as duplicates. They are trimmed structurally instead — `displayUrl` drops the + click-tracking params (sharing `isTrackingParam` with `normalizeUrl`, so there is one + list), the `www.`, and the trailing slash, which is where long URLs get long. It keeps + the scheme, so the result is still copyable, and keeps the fragment, which for an SPA + is the entire page identity. `TAB_URL_MAX` (200) is a backstop for data: URIs and + pathological paths, not the mechanism. Two things about `limit` are load-bearing. It **defaults** to `TABS_LIST_DEFAULT_LIMIT` (200) rather than being opt-in, because the failure it prevents is total — an unbounded @@ -461,6 +488,26 @@ Gullet asks the extension for the full filtered set and groups it there, since a applied before grouping would corrupt the counts — and the full list crossing loopback costs nothing, which is the whole point of where the budget actually is. +**Selection and rendering are separate passes for a reason.** `renderTabs` +(`src/tabs-view.ts`) runs _once_, in Gullet, at the very end — never in the extension, +even though clipping there would shrink the socket frame. Gullet re-applies `query` over +the merged results, and a query matching text that clipping had already removed would +silently drop the exact tab the agent asked for. So every filter sees whole strings and +only the bytes handed to the model are trimmed. This is the same trade as `groupBy`: the +socket is loopback, and loopback bytes are not the budget anyone is spending. + +`renderTabs` also tolerates a tab missing `title` or `url` rather than throwing. The +extension guarantees both, but a version-skewed one does not, and one malformed entry must +not destroy a listing of eight hundred — the same reason `tabs_list` keeps a failing +browser's partner. + +▸ **This may also settle the first-`tabs_list` timeout** in the open questions below. +_Response size_ is one of the two live hypotheses for it, and a first call that used to +serialise ~300 KB into one WebSocket frame now serialises ~36 KB. That is not a fix, and +it is not evidence — but it does mean the symptom recurring at the new size would rule +response size out and leave startup contention, which is the discriminating test that was +otherwise awkward to run. + **Restoring is exact where it can be and safe where it cannot.** A batch is recreated in ascending index order within each window; inserting a low index after a high one would shift the tab already placed there. A recorded window id is trusted only when a live window diff --git a/gullet/src/tools.ts b/gullet/src/tools.ts index df870e7..d99b313 100644 --- a/gullet/src/tools.ts +++ b/gullet/src/tools.ts @@ -17,6 +17,7 @@ import { type BridgeMethod, type BridgeTab, } from "../../src/bridge-protocol.js"; +import { renderTabs, TAB_TITLE_MAX } from "../../src/tabs-view.js"; import type { McpTool, McpToolResult } from "./mcp.js"; import { selectAll, selectOne, type ConnectionSummary } from "./select.js"; @@ -73,7 +74,8 @@ export const GULLET_TOOLS: readonly McpTool[] = [ name: "tabs_list", title: "List open tabs", description: - `List the user's open tabs with metadata only — id, title, url, lastAccessed, windowId, index, and the flags discarded, pinned, active and (Firefox/Zen) hidden. **Flags appear only when true**: no \`discarded\` key means the tab is loaded. \`discarded: true\` means the tab is unloaded and cannot be read until tabs_load wakes it; \`hidden: true\` on Zen usually means the tab lives in another Zen workspace.\n\n` + + `List the user's open tabs with metadata only — id, title, url, lastAccessed, and the flags discarded, pinned, active and (Firefox/Zen) hidden. **Flags appear only when true**: no \`discarded\` key means the tab is loaded. \`discarded: true\` means the tab is unloaded and cannot be read until tabs_load wakes it; \`hidden: true\` on Zen usually means the tab lives in another Zen workspace. \`windowId\` appears at the top level when every tab shares one window, and per tab otherwise.\n\n` + + `Titles longer than ${TAB_TITLE_MAX} characters are clipped with a trailing "…", and URLs are shortened (tracking parameters and \`www.\` dropped). \`query\` always matches against the **full** title and URL, so a term that was clipped away still finds its tab. Use tab_read for a tab's real content.\n\n` + `Backlogs are large, so this returns the ${TABS_LIST_DEFAULT_LIMIT} most recently accessed tabs by default and reports \`matched\` (how many the filter actually hit) plus \`truncated: true\` when there were more. Narrow with \`query\` rather than raising \`limit\` — a full listing of a thousand tabs will not fit in your context.\n\n` + `Start a triage run with \`groupBy: "domain"\`: it returns one row per domain with tab and discarded counts instead of any tabs, which is a few hundred bytes for the whole backlog and tells you what to pass as \`query\` next.`, inputSchema: { @@ -343,11 +345,23 @@ async function tabsList( // back empty, and then there is nothing to disambiguate. connectionId rather // than the label, because labels are self-reported and two can share one. const contributors = perBrowser.filter((r) => r.tabs.length > 0).length; + // Rendering happens here and only here — after every filter has seen the whole + // strings. renderTabs preserves order one-for-one, which is what lets the + // origin lookup stay keyed on the tabs that went in. + const view = renderTabs(selected.tabs, { hoistWindow: contributors <= 1 }); const tabs = contributors > 1 - ? selected.tabs.map((tab) => ({ ...tab, connectionId: origin.get(tab)?.connectionId })) - : selected.tabs; - return { ...head, ...selected, tabs }; + ? view.tabs.map((tab, i) => ({ + ...tab, + connectionId: origin.get(selected.tabs[i] as BridgeTab)?.connectionId, + })) + : view.tabs; + return { + ...head, + ...(view.windowId === undefined ? {} : { windowId: view.windowId }), + ...selected, + tabs, + }; } // Compact JSON, not pretty-printed: every one of these results goes into a diff --git a/gullet/tests/tools.test.ts b/gullet/tests/tools.test.ts index 914bd5c..216d60d 100644 --- a/gullet/tests/tools.test.ts +++ b/gullet/tests/tools.test.ts @@ -66,12 +66,25 @@ describe("tool definitions", () => { }); describe("tabs_list", () => { - const tab = (id: number, url: string, lastAccessed?: number): Record => ({ + const tab = ( + id: number, + url: string, + lastAccessed?: number, + windowId = 1, + ): Record => ({ id, title: `tab ${id}`, url, + windowId, + index: id, ...(lastAccessed === undefined ? {} : { lastAccessed }), }); + /** A tab as it comes back out: index dropped, windowId hoisted, url trimmed. */ + const shown = (id: number, url: string): Record => ({ + id, + title: `tab ${id}`, + url, + }); test("fans out over every browser and stamps origin only on a merged listing", async () => { const { call } = caller([zen, chrome], ({ connectionId }) => @@ -81,9 +94,11 @@ describe("tabs_list", () => { ); expect(payload(await call("tabs_list", {}))).toEqual({ browsers: [zen, chrome], + // No hoisted windowId: both browsers call their window 1, and claiming a + // single shared window across two browsers would be a lie. tabs: [ - { ...tab(1, "https://a.test/"), connectionId: "conn-1" }, - { ...tab(2, "https://b.test/"), connectionId: "conn-2" }, + { ...shown(1, "https://a.test"), windowId: 1, connectionId: "conn-1" }, + { ...shown(2, "https://b.test"), windowId: 1, connectionId: "conn-2" }, ], matched: 2, }); @@ -91,14 +106,30 @@ describe("tabs_list", () => { test("leaves the origin off when only one browser is connected", async () => { const { call } = caller([zen], () => ({ tabs: [tab(1, "https://a.test/")] })); - // The constant used to be repeated once per tab; `browsers` already says it. + // The constants used to be repeated once per tab; `browsers` and the hoisted + // `windowId` already say both. expect(payload(await call("tabs_list", {}))).toEqual({ browsers: [zen], - tabs: [tab(1, "https://a.test/")], + windowId: 1, + tabs: [shown(1, "https://a.test")], matched: 1, }); }); + test("clips a long title but still matches a query against the full one", async () => { + const buried = `${"x".repeat(200)} needle`; + const { call } = caller([zen], () => ({ + tabs: [{ ...tab(1, "https://a.test/"), title: buried }], + })); + const result = payload(await call("tabs_list", { query: "needle" })) as { + tabs: Array<{ title: string }>; + matched: number; + }; + expect(result.matched).toBe(1); + expect(result.tabs[0]?.title).toEndWith("…"); + expect(result.tabs[0]?.title).not.toContain("needle"); + }); + test("narrows to the named browser", async () => { const { call, sent } = caller([zen, chrome], () => ({ tabs: [] })); await call("tabs_list", { browser: "Chrome" }); @@ -299,8 +330,9 @@ describe("tabs_list with a browser that fails", () => { failures: Array>; }; // Only Zen answered, so it is the sole entry in `browsers` and the tab needs - // no per-tab origin stamped on it. - expect(result.tabs).toEqual([{ id: 1, title: "kept" }]); + // no per-tab origin stamped on it. This tab also has no url — a malformed + // entry renders empty rather than throwing away the listing around it. + expect(result.tabs).toEqual([{ id: 1, title: "kept", url: "" }]); expect(result.failures).toEqual([ { connectionId: chrome.connectionId, diff --git a/src/normalize.ts b/src/normalize.ts index 94f27e1..2e449dd 100644 --- a/src/normalize.ts +++ b/src/normalize.ts @@ -20,13 +20,19 @@ const TRACKING_PARAMS = new Set([ const TRACKING_PREFIXES = ["utm_"]; -function shouldStripParam(key: string, extras: Set): boolean { +/** + * A query parameter that identifies the click, not the page. Exported because + * the bridge's listing view trims the same params for a different purpose — it + * needs a shorter *displayable* URL, where `normalizeUrl` produces a + * scheme-less dedup key — and one list of tracking params is enough. + */ +export function isTrackingParam(key: string): boolean { if (TRACKING_PARAMS.has(key)) return true; - if (extras.has(key)) return true; - for (const prefix of TRACKING_PREFIXES) { - if (key.startsWith(prefix)) return true; - } - return false; + return TRACKING_PREFIXES.some((prefix) => key.startsWith(prefix)); +} + +function shouldStripParam(key: string, extras: Set): boolean { + return isTrackingParam(key) || extras.has(key); } export function normalizeUrl(rawUrl: string | undefined, opts: NormalizeOpts = {}): string | null { diff --git a/src/tabs-view.ts b/src/tabs-view.ts new file mode 100644 index 0000000..a1077c4 --- /dev/null +++ b/src/tabs-view.ts @@ -0,0 +1,136 @@ +// How a selected listing is *rendered* for an agent, as opposed to how it is +// selected (`selectTabs` in bridge-protocol.ts) or transported. Pure, and shared +// with Gullet, which is the only caller: see below for why this runs once at the +// end rather than in the extension's pass. + +import type { BridgeTab } from "./bridge-protocol.js"; +import { isTrackingParam } from "./normalize.js"; + +/** + * Titles are clipped, not summarised. 120 is where the curve turns: measured + * over a real 874-tab backlog the mean title is ~104 characters, so there is no + * "clip the outliers" cap — anything tighter cuts into the body of the + * distribution rather than its tail. At 120 roughly a quarter of tabs lose + * something, and what they lose is cheap, because titles are front-loaded: the + * tail of a long one is usually the site suffix ("… | GitHub") that the URL + * already says. + */ +export const TAB_TITLE_MAX = 120; + +/** + * URLs are trimmed structurally first (see `displayUrl`) and only clipped as a + * backstop, which is why this is generous. A URL cut mid-string stops being a + * URL: it cannot be copied, and two distinct tabs can clip to the same prefix + * and read as duplicates. Losing a data: URI's payload is the case this exists + * for, and there the prefix really is all the information there is. + */ +export const TAB_URL_MAX = 200; + +/** Trailing ellipsis, so a clipped value can never be read as a complete one. */ +const ELLIPSIS = "…"; + +/** + * Tolerates a non-string because the tabs reaching here came off a socket. The + * extension guarantees `title` and `url`, but a version-skewed or malformed one + * does not, and one field missing from one tab must not throw away a listing of + * eight hundred — the same reason `tabs_list` keeps a failing browser's partner. + */ +function clip(value: unknown, max: number): string { + const text = typeof value === "string" ? value : ""; + return text.length <= max ? text : text.slice(0, max - 1) + ELLIPSIS; +} + +/** + * A tab as it appears in a rendered listing. `index` is gone — it duplicated + * the array order under `sort: "window"` and meant nothing under the others, + * and nothing consumes it: the undo log records position from the live + * `browser.tabs.Tab`, not from a listing. `windowId` survives only when the + * listing actually spans more than one window. + */ +export interface RenderedTab { + id: number; + title: string; + url: string; + lastAccessed?: number; + discarded?: boolean; + pinned?: boolean; + active?: boolean; + hidden?: boolean; + windowId?: number; +} + +export interface RenderedTabs { + tabs: RenderedTab[]; + /** The one window every tab is in, hoisted out of them. Omitted otherwise. */ + windowId?: number; +} + +/** + * A shorter URL that is still a URL. Drops the click-tracking params, the `www.` + * and the trailing slash — which is where long URLs get long — while keeping the + * scheme, the parameter order the page actually used, and the fragment. + * + * Keeping the scheme costs ~8 bytes a tab and buys a string an agent can hand + * back to the user verbatim; keeping the fragment is not optional, because for + * an SPA the fragment is the whole page identity. Params keep their original + * order rather than being sorted: `normalizeUrl` sorts because it is building a + * comparison key, and this is not one. + */ +export function displayUrl(raw: string): string { + let url: URL; + try { + url = new URL(raw); + } catch { + // Also the path for a missing or non-string url; see `clip`. + return clip(raw, TAB_URL_MAX); + } + if (url.protocol !== "http:" && url.protocol !== "https:") return clip(raw, TAB_URL_MAX); + + const host = url.hostname.toLowerCase().replace(/^www\./, ""); + const path = url.pathname.length > 1 ? url.pathname.replace(/\/$/, "") : ""; + const kept = [...url.searchParams].filter(([key]) => !isTrackingParam(key)); + const search = kept.length + ? "?" + kept.map(([k, v]) => `${encodeURIComponent(k)}=${encodeURIComponent(v)}`).join("&") + : ""; + return clip(`${url.protocol}//${host}${path}${search}${url.hash}`, TAB_URL_MAX); +} + +/** + * Shape a selected listing for output. + * + * Runs **once, in Gullet**, deliberately — not in the extension's `selectTabs` + * pass, even though doing it there would shrink the socket frame. Gullet + * re-applies `query` over the merged results, and a query matching text that + * clipping had already removed would silently drop the very tab the agent asked + * for. Selection sees whole strings; only what is handed to the model is + * trimmed. The socket is loopback, so the frame size it saves is not a budget + * anyone is spending. + * + * `hoistWindow` is false when more than one browser contributed, since two + * browsers can each call their window `1` and a hoisted id would then claim a + * single window that does not exist. + */ +export function renderTabs( + tabs: readonly BridgeTab[], + opts: { hoistWindow?: boolean } = {}, +): RenderedTabs { + const windows = new Set(tabs.map((tab) => tab.windowId)); + const shared = (opts.hoistWindow ?? true) && windows.size === 1 ? [...windows][0] : undefined; + + const rendered = tabs.map((tab) => { + const out: RenderedTab = { + id: tab.id, + title: clip(tab.title, TAB_TITLE_MAX), + url: displayUrl(tab.url), + }; + if (tab.lastAccessed !== undefined) out.lastAccessed = tab.lastAccessed; + if (tab.discarded) out.discarded = true; + if (tab.pinned) out.pinned = true; + if (tab.active) out.active = true; + if (tab.hidden) out.hidden = true; + if (shared === undefined && tab.windowId !== undefined) out.windowId = tab.windowId; + return out; + }); + + return shared === undefined ? { tabs: rendered } : { tabs: rendered, windowId: shared }; +} diff --git a/tests/tabs-view.test.ts b/tests/tabs-view.test.ts new file mode 100644 index 0000000..52d6db8 --- /dev/null +++ b/tests/tabs-view.test.ts @@ -0,0 +1,104 @@ +import { describe, test, expect } from "bun:test"; +import type { BridgeTab } from "../src/bridge-protocol.js"; +import { displayUrl, renderTabs, TAB_TITLE_MAX, TAB_URL_MAX } from "../src/tabs-view.js"; + +function makeTab(fields: Partial & Pick): BridgeTab { + return { title: "", windowId: 1, index: 0, ...fields }; +} + +describe("displayUrl()", () => { + test("drops the noise that makes URLs long", () => { + expect(displayUrl("https://www.example.com/post/?utm_source=x&utm_medium=y&id=7")).toBe( + "https://example.com/post?id=7", + ); + expect(displayUrl("https://example.com/a/b/")).toBe("https://example.com/a/b"); + }); + + test("keeps the scheme, so the result can still be handed to the user verbatim", () => { + expect(displayUrl("http://example.com/x")).toStartWith("http://"); + expect(displayUrl("https://example.com/x")).toStartWith("https://"); + }); + + // For an SPA the fragment is the whole page identity, so stripping it — which + // normalizeUrl does by default, because it is building a dedup key — would + // collapse every route of an app into one indistinguishable URL. + test("keeps the fragment", () => { + expect(displayUrl("https://example.com/app#/settings/profile")).toBe( + "https://example.com/app#/settings/profile", + ); + }); + + test("keeps parameter order rather than sorting it", () => { + expect(displayUrl("https://example.com/?z=1&a=2")).toBe("https://example.com?z=1&a=2"); + }); + + test("passes through what it cannot parse or does not own", () => { + expect(displayUrl("about:blank")).toBe("about:blank"); + expect(displayUrl("not a url")).toBe("not a url"); + }); + + test("clips only as a backstop, and marks it", () => { + const long = `https://example.com/${"p".repeat(400)}`; + const out = displayUrl(long); + expect(out).toHaveLength(TAB_URL_MAX); + expect(out).toEndWith("…"); + }); +}); + +describe("renderTabs()", () => { + test("clips a long title and marks it, leaving a short one alone", () => { + const long = "t".repeat(TAB_TITLE_MAX + 50); + const [clipped, short] = renderTabs([ + makeTab({ id: 1, url: "https://a.test/", title: long }), + makeTab({ id: 2, url: "https://b.test/", title: "short" }), + ]).tabs; + expect(clipped?.title).toHaveLength(TAB_TITLE_MAX); + expect(clipped?.title).toEndWith("…"); + expect(short?.title).toBe("short"); + }); + + test("drops index and hoists a window every tab shares", () => { + const view = renderTabs([ + makeTab({ id: 1, url: "https://a.test/", windowId: 7, index: 0 }), + makeTab({ id: 2, url: "https://b.test/", windowId: 7, index: 1 }), + ]); + expect(view.windowId).toBe(7); + expect(view.tabs[0]).not.toHaveProperty("windowId"); + expect(view.tabs[0]).not.toHaveProperty("index"); + }); + + test("keeps windowId per tab once a listing spans two windows", () => { + const view = renderTabs([ + makeTab({ id: 1, url: "https://a.test/", windowId: 7 }), + makeTab({ id: 2, url: "https://b.test/", windowId: 8 }), + ]); + expect(view.windowId).toBeUndefined(); + expect(view.tabs.map((t) => t.windowId)).toEqual([7, 8]); + }); + + // Two browsers can each call their window 1; hoisting would then claim a + // single window that does not exist. + test("refuses to hoist across a merged listing", () => { + const view = renderTabs( + [makeTab({ id: 1, url: "https://a.test/" }), makeTab({ id: 2, url: "https://b.test/" })], + { hoistWindow: false }, + ); + expect(view.windowId).toBeUndefined(); + expect(view.tabs.map((t) => t.windowId)).toEqual([1, 1]); + }); + + test("carries only the flags that are true", () => { + const [tab] = renderTabs([ + makeTab({ id: 1, url: "https://a.test/", discarded: true, pinned: false }), + ]).tabs; + expect(tab).toMatchObject({ discarded: true }); + expect(tab).not.toHaveProperty("pinned"); + expect(tab).not.toHaveProperty("active"); + expect(tab).not.toHaveProperty("lastAccessed"); + }); + + test("preserves order one-for-one, which the origin lookup depends on", () => { + const input = [3, 1, 2].map((id) => makeTab({ id, url: `https://${id}.test/` })); + expect(renderTabs(input).tabs.map((t) => t.id)).toEqual([3, 1, 2]); + }); +}); From fdf56a4b39f4a28451ff0cdb826a31bd1180591c Mon Sep 17 00:00:00 2001 From: Michael Simon Date: Sun, 2 Aug 2026 18:55:32 -0700 Subject: [PATCH 03/11] Fix groupBy ignoring query, and give it its own limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caught live against the 874-tab browser that motivated this work: `tabs_list { query: "x.com", groupBy: "domain" }` answered `matched: 874, domains: 298` — the entire backlog, byte-identical to the unfiltered call. The filter lived inside `selectTabs`, and the grouping branch called `groupTabsByDomain(merged, ...)` directly, skipping it. Extracted as `filterTabs`, now called by both paths. What makes this worth a note: against a current extension the bug was invisible, because the extension applies `query` before sending and the grouping saw a pre-filtered set. It only surfaced against a real browser running 0.2.0, which ignores `query` and hands over everything. The version-skew tolerance that Gullet's second pass exists to provide is precisely what kept the bug hidden — a redundant safety pass has to be tested with the primary pass disabled or it is only ever exercised as a no-op. Both regression tests do that: the fake browser ignores `query`. Also: `groupBy` now defaults to `TABS_LIST_DEFAULT_GROUP_LIMIT` (50) rather than the 200 a tab listing gets. That browser has 298 distinct domains and everything past roughly the fiftieth is a single tab — 250 rows of noise around the ~20 that describe the backlog. `domains` still reports the true count. Verified: bun run check, plus live against the real browser. --- docs/BRIDGE.md | 18 ++++++++++++++- gullet/src/tools.ts | 11 ++++++--- gullet/tests/tools.test.ts | 16 +++++++++++++ src/bridge-protocol.ts | 42 +++++++++++++++++++++++++++++------ tests/bridge-protocol.test.ts | 37 ++++++++++++++++++++++++++++++ 5 files changed, 113 insertions(+), 11 deletions(-) diff --git a/docs/BRIDGE.md b/docs/BRIDGE.md index 26c2e5f..2358b5b 100644 --- a/docs/BRIDGE.md +++ b/docs/BRIDGE.md @@ -444,7 +444,11 @@ Five changes, in the order they matter: Deliberately not a regex: an agent-authored regex is an unbounded backtracking risk on a thousand strings, and substring terms are what triage actually needs. 2. **`groupBy: "domain"`** — counts only, no tabs. The real triage primitive: one cheap - call says what the backlog is made of and what to pass as `query` next. The domain is + call says what the backlog is made of and what to pass as `query` next. It honours + `query` too, so it can count one slice rather than the whole backlog, and it gets its + own tighter default limit (`TABS_LIST_DEFAULT_GROUP_LIMIT`, 50): the real 874-tab + browser held **298 distinct domains**, and everything past roughly the fiftieth was a + single tab — 250 rows of noise around the ~20 that describe the backlog. The domain is the hostname minus `www.`, not the registrable domain: eTLD+1 needs the Public Suffix List, which `bridge-protocol.ts` cannot take as a dependency and which goes stale, and `mail.google.com` vs `docs.google.com` is the distinction triage wants anyway. @@ -496,6 +500,18 @@ silently drop the exact tab the agent asked for. So every filter sees whole stri only the bytes handed to the model are trimmed. This is the same trade as `groupBy`: the socket is loopback, and loopback bytes are not the budget anyone is spending. +▸ **The second pass hid a bug from itself, and only a stale extension exposed it.** +`groupBy` grouped the _unfiltered_ merge: `tabs_list { query: "x.com", groupBy: "domain" }` +answered `matched: 874, domains: 298` — the whole backlog, identical to the unfiltered +call. The filter lived inside `selectTabs`, and the grouping branch skipped `selectTabs` +entirely. Against a **current** extension this was invisible, because the extension had +already applied `query` before sending; it only surfaced against a real browser running +0.2.0, which ignores `query` and hands over everything. So the version-skew tolerance that +the second pass exists to provide is exactly what stopped the bug being noticed, and the +skew itself is what revealed it. The filter is now `filterTabs`, called by both paths. +The lesson generalises: a redundant safety pass has to be tested with the primary pass +_disabled_, or it is only ever exercised as a no-op. + `renderTabs` also tolerates a tab missing `title` or `url` rather than throwing. The extension guarantees both, but a version-skewed one does not, and one malformed entry must not destroy a listing of eight hundred — the same reason `tabs_list` keeps a failing diff --git a/gullet/src/tools.ts b/gullet/src/tools.ts index d99b313..8a56c97 100644 --- a/gullet/src/tools.ts +++ b/gullet/src/tools.ts @@ -5,10 +5,12 @@ import { asRecord, BridgeRequestError, + filterTabs, groupTabsByDomain, isBridgeMethod, parseTabsListParams, selectTabs, + TABS_LIST_DEFAULT_GROUP_LIMIT, TABS_LIST_DEFAULT_LIMIT, TABS_LIST_MAX_LIMIT, TABS_LOAD_MAX_BATCH, @@ -91,7 +93,7 @@ export const GULLET_TOOLS: readonly McpTool[] = [ type: "integer", minimum: 1, maximum: TABS_LIST_MAX_LIMIT, - description: `Max tabs (or domain groups) to return. Defaults to ${TABS_LIST_DEFAULT_LIMIT}.`, + description: `Max rows to return. Defaults to ${TABS_LIST_DEFAULT_LIMIT} tabs, or ${TABS_LIST_DEFAULT_GROUP_LIMIT} when groupBy is set — a domain histogram has a long tail of one-tab domains.`, }, sort: { type: "string", @@ -103,7 +105,7 @@ export const GULLET_TOOLS: readonly McpTool[] = [ type: "string", enum: ["domain"], description: - "Return per-domain counts instead of tabs: { domain, tabs, discarded, newest }, most tabs first. Honours query and limit. The cheap first call for triaging a backlog you have not seen.", + "Return per-domain counts instead of tabs: { domain, tabs, discarded, newest }, most tabs first. Honours query, so you can count one slice of the backlog. Answers with `domains` (distinct domains matched) and `matched` (tabs behind them). The cheap first call for triaging a backlog you have not seen.", }, scope: { type: "string", @@ -336,7 +338,10 @@ async function tabsList( const head = { browsers: targets, ...(failures.length > 0 ? { failures } : {}) }; if (listParams.groupBy === "domain") { - return { ...head, ...groupTabsByDomain(merged, listParams.limit) }; + // filterTabs, not `merged`: the extension may be older than this Gullet and + // ignore `query` entirely, and grouping the unfiltered set would answer a + // question nobody asked. + return { ...head, ...groupTabsByDomain(filterTabs(merged, listParams), listParams.limit) }; } const selected = selectTabs(merged, listParams); // Ids only mean something inside one browser, so a listing that actually diff --git a/gullet/tests/tools.test.ts b/gullet/tests/tools.test.ts index 216d60d..2eb94ff 100644 --- a/gullet/tests/tools.test.ts +++ b/gullet/tests/tools.test.ts @@ -173,6 +173,22 @@ describe("tabs_list", () => { expect(result).toMatchObject({ matched: 3, truncated: true }); }); + // Regression, caught live: grouping ran on the unfiltered merge, so a query + // plus groupBy counted the whole backlog. The browser here ignores `query` + // entirely, which is the version skew that exposed it — a newer extension + // pre-filters and would have hidden the bug rather than prevented it. + test("groupBy honours query even when the browser ignored it", async () => { + const { call } = caller([zen], () => ({ + tabs: [tab(1, "https://x.com/a"), tab(2, "https://x.com/b"), tab(3, "https://other.test/c")], + })); + const result = payload(await call("tabs_list", { query: "x.com", groupBy: "domain" })); + expect(result).toMatchObject({ + groups: [{ domain: "x.com", tabs: 2, discarded: 0 }], + domains: 1, + matched: 2, + }); + }); + test("groupBy: domain answers with counts across every browser and no tabs", async () => { const { call } = caller([zen, chrome], ({ connectionId }) => connectionId === "conn-1" diff --git a/src/bridge-protocol.ts b/src/bridge-protocol.ts index 2850d9d..e614a21 100644 --- a/src/bridge-protocol.ts +++ b/src/bridge-protocol.ts @@ -379,6 +379,16 @@ export const TABS_LIST_DEFAULT_LIMIT = 200; /** Ceiling on an explicit `limit`. Above this a listing is not triage material. */ export const TABS_LIST_MAX_LIMIT = 2000; +/** + * Default ceiling on `groupBy: "domain"` rows, which is much tighter than the + * tab default because a domain histogram has a long, uninformative tail. A real + * 874-tab backlog held 298 distinct domains, and everything past roughly the + * fiftieth was a single tab — 250 rows of noise around the ~20 that describe the + * backlog. `domains` still reports the true count, so the tail is visible + * without being spelled out. + */ +export const TABS_LIST_DEFAULT_GROUP_LIMIT = 50; + export interface TabsListParams { /** Default "all": every window. "current-window" narrows to the focused one. */ scope?: "all" | "current-window"; @@ -477,6 +487,27 @@ function compareTabs(sort: TabsListSort): (a: BridgeTab, b: BridgeTab) => number }; } +/** + * The `query` and `includeHidden` cut, on its own. + * + * Split out of `selectTabs` because `groupBy` needs the same filter but none of + * the sorting or truncation, and folding it into `selectTabs` meant the grouping + * path silently skipped it: `tabs_list { query: "x.com", groupBy: "domain" }` + * counted the whole backlog. Caught against a real 874-tab browser, where the + * extension was older than Gullet and so did not pre-filter — which is exactly + * the version skew the second pass exists to cover, so a newer extension would + * have hidden the bug rather than prevented it. Returns a new array; callers + * sort it in place. + */ +export function filterTabs(tabs: readonly BridgeTab[], params: TabsListParams): BridgeTab[] { + const includeHidden = params.includeHidden ?? true; + const query = params.query?.trim() ?? ""; + return tabs.filter( + (tab) => + (includeHidden || tab.hidden !== true) && (query === "" || matchesTabQuery(tab, query)), + ); +} + /** * Filter, sort, and truncate a listing. Pure and shared, because it runs * **twice**: in the extension, so a backlog never crosses the socket in full, @@ -486,12 +517,7 @@ function compareTabs(sort: TabsListSort): (a: BridgeTab, b: BridgeTab) => number * filtered answer rather than a flood. */ export function selectTabs(tabs: BridgeTab[], params: TabsListParams): TabsListResult { - const includeHidden = params.includeHidden ?? true; - const query = params.query?.trim() ?? ""; - const matches = tabs.filter( - (tab) => - (includeHidden || tab.hidden !== true) && (query === "" || matchesTabQuery(tab, query)), - ); + const matches = filterTabs(tabs, params); matches.sort(compareTabs(params.sort ?? "recent")); const limit = params.limit ?? TABS_LIST_DEFAULT_LIMIT; const result: TabsListResult = { tabs: matches.slice(0, limit), matched: matches.length }; @@ -760,7 +786,9 @@ export function parseTabsListParams(raw: unknown): ResolvedTabsListParams { scope: scope ?? "all", includeHidden: includeHidden ?? true, sort: sort ?? "recent", - limit: limit ?? TABS_LIST_DEFAULT_LIMIT, + // The default depends on what is being counted; an explicit limit governs both. + limit: + limit ?? (groupBy === "domain" ? TABS_LIST_DEFAULT_GROUP_LIMIT : TABS_LIST_DEFAULT_LIMIT), ...(query !== undefined && query.trim() !== "" ? { query: query.trim() } : {}), ...(groupBy !== undefined ? { groupBy } : {}), }; diff --git a/tests/bridge-protocol.test.ts b/tests/bridge-protocol.test.ts index 47bff36..5920a16 100644 --- a/tests/bridge-protocol.test.ts +++ b/tests/bridge-protocol.test.ts @@ -8,6 +8,7 @@ import { classifyBridgeProbe, deriveProof, generateToken, + filterTabs, groupTabsByDomain, isBridgeMethod, matchesTabQuery, @@ -21,6 +22,7 @@ import { parseUndoCloseParams, selectTabs, tabDomain, + TABS_LIST_DEFAULT_GROUP_LIMIT, TABS_LIST_DEFAULT_LIMIT, TABS_LIST_MAX_LIMIT, TABS_LOAD_MAX_BATCH, @@ -172,6 +174,13 @@ describe("parseTabsListParams()", () => { }); }); + // A domain histogram has a long tail of one-tab domains, so it gets a tighter + // default than a tab listing. An explicit limit still governs both. + test("defaults groupBy to its own smaller limit", () => { + expect(parseTabsListParams({ groupBy: "domain" }).limit).toBe(TABS_LIST_DEFAULT_GROUP_LIMIT); + expect(parseTabsListParams({ groupBy: "domain", limit: 5 }).limit).toBe(5); + }); + test("accepts the documented values", () => { expect( parseTabsListParams({ @@ -320,6 +329,34 @@ describe("groupTabsByDomain()", () => { expect(result.groups.map((g) => g.domain)).toEqual(["x.com"]); expect(result).toMatchObject({ domains: 2, matched: 4, truncated: true }); }); + + // Regression, caught live against an 874-tab browser: grouping ran on the + // unfiltered set, so `{ query, groupBy }` counted the whole backlog. The + // filter has to be applied by the caller, which is what `filterTabs` is for. + test("counts only what the filter kept, when the caller filters first", () => { + const result = groupTabsByDomain(filterTabs(tabs, { query: "x.com" }), 10); + expect(result).toMatchObject({ domains: 1, matched: 3 }); + expect(result.groups.map((g) => g.domain)).toEqual(["x.com"]); + }); +}); + +describe("filterTabs()", () => { + const tabs = [ + makeTab({ id: 1, url: "https://x.com/a", title: "keep" }), + makeTab({ id: 2, url: "https://y.com/b", title: "drop" }), + makeTab({ id: 3, url: "https://z.com/c", title: "keep", hidden: true }), + ]; + + test("applies query and includeHidden without sorting or truncating", () => { + expect(filterTabs(tabs, { query: "keep" }).map((t) => t.id)).toEqual([1, 3]); + expect(filterTabs(tabs, { query: "keep", includeHidden: false }).map((t) => t.id)).toEqual([1]); + expect(filterTabs(tabs, {}).map((t) => t.id)).toEqual([1, 2, 3]); + }); + + test("returns a new array, so the caller can sort in place", () => { + const out = filterTabs(tabs, {}); + expect(out).not.toBe(tabs as unknown as typeof out); + }); }); describe("parseTabReadParams()", () => { From cf002633511c80c96c01ef2c2df217107450e3a1 Mon Sep 17 00:00:00 2001 From: Michael Simon Date: Sun, 2 Aug 2026 19:10:36 -0700 Subject: [PATCH 04/11] Say that a Zen listing is one workspace, because it is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verified live: tabs_list answered `matched: 874, domains: 298`, then `matched: 160, domains: 66` with a disjoint domain set after a workspace switch. `includeHidden: false` changed neither number, and both readings hoisted the same windowId — so tabs in a non-active workspace are absent from `tabs.query` entirely, not returned flagged `hidden`. That is the condition `probeHeuristic` was written to detect; it is simply true on this Zen. Active-workspace scope is fine and is not changed here. The claim was what was wrong: tabs_list told agents `hidden: true` meant "another workspace", so an agent seeing 160 tabs would report them as the user's whole backlog, with `matched` reading as authoritative either way. The tool description and GULLET_INSTRUCTIONS now state the scoping and tell the agent not to quote a total. Also drops `index` from the tool-surface table, which the rendering change removed a commit ago. Left undone deliberately: nothing in a result says *which* workspace it is, so two listings minutes apart are not comparable. Naming one needs an API Zen does not have; surfacing probeHeuristic's verdict on the listing is a wire change, and is noted in docs/BRIDGE.md rather than made. Verified: bun run check, plus live against the real browser. --- docs/BRIDGE.md | 39 +++++++++++++++++++++++++++++++-------- gullet/src/tools.ts | 7 ++++++- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/docs/BRIDGE.md b/docs/BRIDGE.md index 2358b5b..f748921 100644 --- a/docs/BRIDGE.md +++ b/docs/BRIDGE.md @@ -371,14 +371,14 @@ extension and Gullet so the contract is typechecked from one definition. ## Tool surface (v1) -| MCP tool | Backing APIs | Notes | -| ------------ | ------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `tabs_list` | `tabs.query` | id, title, url, `windowId`, `index`, and — only when true — `lastAccessed`, `discarded`, `pinned`, `active`, and on Firefox `hidden` (≈ other Zen workspaces). Filtered with `query`, ordered with `sort`, capped by `limit`, or collapsed to counts with `groupBy: "domain"`. See below. | -| `tabs_load` | `tabs.reload` + `tabs.onUpdated` | Wakes discarded tabs so they can be read. Batched (≤20), three at a time, under a 30s deadline; per-tab `ready`/`pending`/`failed`. Gated on a settings toggle, default off — answers `not-enabled` until then. | -| `tab_read` | `scripting.executeScript` + existing `clip-current.ts` | Returns Defuddle markdown + metadata. Fails cleanly on discarded tabs (see below). | -| `tab_clip` | existing `clip-format.ts` + `obsidian://new` handoff | Files into the vault exactly as manual Devour does, including the Chrome redirect-page dance. | -| `tabs_close` | `tabs.remove` | Batched, ids deduplicated. Entries (title, url, pinned, window, index, private) are recorded in an undo log in `storage.local` _before_ the removal, and the batch id comes back with the result. | -| `undo_close` | reopen from the log | Safety valve for the one destructive act. Omit the batch id to undo the most recent. | +| MCP tool | Backing APIs | Notes | +| ------------ | ------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `tabs_list` | `tabs.query` | id, title, url, `windowId` (hoisted when shared), and — only when true — `lastAccessed`, `discarded`, `pinned`, `active`, `hidden`. Filtered with `query`, ordered with `sort`, capped by `limit`, or collapsed to counts with `groupBy: "domain"`. **On Zen, covers the active workspace only.** See below. | +| `tabs_load` | `tabs.reload` + `tabs.onUpdated` | Wakes discarded tabs so they can be read. Batched (≤20), three at a time, under a 30s deadline; per-tab `ready`/`pending`/`failed`. Gated on a settings toggle, default off — answers `not-enabled` until then. | +| `tab_read` | `scripting.executeScript` + existing `clip-current.ts` | Returns Defuddle markdown + metadata. Fails cleanly on discarded tabs (see below). | +| `tab_clip` | existing `clip-format.ts` + `obsidian://new` handoff | Files into the vault exactly as manual Devour does, including the Chrome redirect-page dance. | +| `tabs_close` | `tabs.remove` | Batched, ids deduplicated. Entries (title, url, pinned, window, index, private) are recorded in an undo log in `storage.local` _before_ the removal, and the batch id comes back with the result. | +| `undo_close` | reopen from the log | Safety valve for the one destructive act. Omit the batch id to undo the most recent. | Deliberately absent: navigate, click, type, evaluate. @@ -517,6 +517,29 @@ extension guarantees both, but a version-skewed one does not, and one malformed not destroy a listing of eight hundred — the same reason `tabs_list` keeps a failing browser's partner. +▸ **A Zen listing is the active workspace, and `hidden` does not tell you otherwise.** +Measured live on the 874-tab browser: `groupBy: "domain"` answered `matched: 874, +domains: 298`; after a workspace switch the same call answered `matched: 160, domains: 66` +with a disjoint set of domains, and `includeHidden: false` changed neither number. So tabs +in a non-active workspace are **absent from `tabs.query`, not flagged `hidden`** — the +condition `probeHeuristic` in `background.ts` was written to detect (`allInWindow.length +=== visibleInWindow.length`) is simply true here. Both readings hoisted the same +`windowId`, so this is one window enumerating differently, not a second window appearing. + +This is accepted behaviour, not a bug to fix: Zen exposes no workspace API (see AGENTS.md), +and active-workspace scope is the reasonable contract. What was wrong was the _claim_ — +`tabs_list` told agents `hidden: true` meant "another workspace", so an agent seeing 160 +tabs would report them as the user's whole backlog with no hedge, and `matched` reads as +authoritative either way. The tool description and `GULLET_INSTRUCTIONS` now state the +scoping outright. + +The honest remaining gap is that **nothing in the result says which workspace it is**, so +two listings taken minutes apart are not comparable and nothing in the payload reveals it. +Naming the workspace is impossible without an API Zen does not have; the available half- +measure is for the extension to surface `probeHeuristic`'s verdict on the listing itself, +which is a wire change and is not made here. It only became visible at all because the +payload work made two listings small enough to compare at a glance. + ▸ **This may also settle the first-`tabs_list` timeout** in the open questions below. _Response size_ is one of the two live hypotheses for it, and a first call that used to serialise ~300 KB into one WebSocket frame now serialises ~36 KB. That is not a fix, and diff --git a/gullet/src/tools.ts b/gullet/src/tools.ts index 8a56c97..83f6b48 100644 --- a/gullet/src/tools.ts +++ b/gullet/src/tools.ts @@ -60,6 +60,10 @@ It answers with "matched" and "truncated", so you can always tell a complete ans a truncated one. If a listing comes back truncated, narrow the query — do not raise the limit and do not page through the whole backlog. +On Zen, every listing is scoped to the active workspace, and nothing in the result says +which one that is. Treat counts as "this workspace", never "your tabs", and expect the +same call to answer differently after the user switches workspace. + Most tabs in a large backlog are discarded (unloaded), and tab_read and tab_clip cannot reach those. Wake them with tabs_load first — one call for every survivor you mean to read (up to 20), not one call per tab. If tabs_load reports not-enabled, the user has not turned @@ -76,7 +80,8 @@ export const GULLET_TOOLS: readonly McpTool[] = [ name: "tabs_list", title: "List open tabs", description: - `List the user's open tabs with metadata only — id, title, url, lastAccessed, and the flags discarded, pinned, active and (Firefox/Zen) hidden. **Flags appear only when true**: no \`discarded\` key means the tab is loaded. \`discarded: true\` means the tab is unloaded and cannot be read until tabs_load wakes it; \`hidden: true\` on Zen usually means the tab lives in another Zen workspace. \`windowId\` appears at the top level when every tab shares one window, and per tab otherwise.\n\n` + + `List the user's open tabs with metadata only — id, title, url, lastAccessed, and the flags discarded, pinned, active and (Firefox/Zen) hidden. **Flags appear only when true**: no \`discarded\` key means the tab is loaded. \`discarded: true\` means the tab is unloaded and cannot be read until tabs_load wakes it. \`windowId\` appears at the top level when every tab shares one window, and per tab otherwise.\n\n` + + `**On Zen, a listing covers the active workspace only.** Tabs in other workspaces are not returned at all — not flagged, absent — so \`matched\` counts that workspace, not the browser. Never tell the user how many tabs they have "in total" from this; say which workspace you looked at. Switching workspace changes the answer completely.\n\n` + `Titles longer than ${TAB_TITLE_MAX} characters are clipped with a trailing "…", and URLs are shortened (tracking parameters and \`www.\` dropped). \`query\` always matches against the **full** title and URL, so a term that was clipped away still finds its tab. Use tab_read for a tab's real content.\n\n` + `Backlogs are large, so this returns the ${TABS_LIST_DEFAULT_LIMIT} most recently accessed tabs by default and reports \`matched\` (how many the filter actually hit) plus \`truncated: true\` when there were more. Narrow with \`query\` rather than raising \`limit\` — a full listing of a thousand tabs will not fit in your context.\n\n` + `Start a triage run with \`groupBy: "domain"\`: it returns one row per domain with tab and discarded counts instead of any tabs, which is a few hundred bytes for the whole backlog and tells you what to pass as \`query\` next.`, From ab18362f2870dfdc876eaa482f608d8b0c140a75 Mon Sep 17 00:00:00 2001 From: Michael Simon Date: Sun, 2 Aug 2026 21:08:15 -0700 Subject: [PATCH 05/11] Keep the browser's matched instead of recomputing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gullet read only `tabs` from each browser's reply and let its own `selectTabs` pass derive `matched`. But an extension carrying these changes truncates to `limit` before sending, so the tabs that arrive are not the tabs that matched: 200 of 874 would come back reported as `matched: 200` with no `truncated`. That is the agent's one signal it has not seen everything, destroyed exactly when there is more to see — and it would have read as a complete listing of the whole backlog. Now each browser's reported `matched` is kept, falling back to our own count only when a browser sends none, which is how an older extension identifies itself. Resolved per connection and summed: two attached browsers can be different versions. Invisible to every live call made today, because that browser is 0.2.0 and sends everything unfiltered, so page size and match count are the same number. It would have appeared on first contact with the build that fixes the loopback cost. Both regression tests fail against the previous code. The trace that found it was after something else: whether the `Number.POSITIVE_INFINITY` limit used for `groupBy` survives the wire. It does not need to — it is spent on a `slice` inside the extension and never reaches `TabsListResult`, so `JSON.stringify` never gets to turn it into `null`. Noted at the site, since the next person to put a limit on the wire needs to send a real number. Verified: bun run check. --- docs/BRIDGE.md | 19 +++++++++++++++++++ gullet/src/tools.ts | 27 ++++++++++++++++++++++++--- gullet/tests/tools.test.ts | 38 ++++++++++++++++++++++++++++++++++++++ src/bridge-methods.ts | 5 +++++ 4 files changed, 86 insertions(+), 3 deletions(-) diff --git a/docs/BRIDGE.md b/docs/BRIDGE.md index f748921..60668fa 100644 --- a/docs/BRIDGE.md +++ b/docs/BRIDGE.md @@ -500,6 +500,25 @@ silently drop the exact tab the agent asked for. So every filter sees whole stri only the bytes handed to the model are trimmed. This is the same trade as `groupBy`: the socket is loopback, and loopback bytes are not the budget anyone is spending. +▸ **`matched` is the browser's, not Gullet's, and recomputing it was a silent lie.** +Gullet read only `tabs` from each browser's reply and let its second `selectTabs` pass +derive `matched` from what arrived. But a current extension truncates to `limit` _before_ +sending, so the tabs that arrive are not the tabs that matched: 200 of 874 came back and +were reported as `matched: 200` with no `truncated` — the agent's one signal that it had +not seen everything, destroyed exactly when there was more to see. It now keeps each +browser's reported `matched`, falling back to its own count only when a browser sends none +(which is how an older extension identifies itself), resolved per connection and summed — +two attached browsers can be different versions. + +This one was **invisible in live testing**, because the browser it was tested against was +0.2.0 and sends everything unfiltered, so page size and match count were the same number. +It would have appeared on first contact with the very build that fixes the loopback cost. +Found by reading the path rather than running it, which is the argument for tracing a +change end to end before signing a build, not after. (The question that prompted the trace +— whether the `Number.POSITIVE_INFINITY` limit used for `groupBy` survives the wire — was +a non-issue: it is spent on a `slice` inside the extension and never reaches +`TabsListResult`, so `JSON.stringify` never gets the chance to turn it into `null`.) + ▸ **The second pass hid a bug from itself, and only a stale extension exposed it.** `groupBy` grouped the _unfiltered_ merge: `tabs_list { query: "x.com", groupBy: "domain" }` answered `matched: 874, domains: 298` — the whole backlog, identical to the unfiltered diff --git a/gullet/src/tools.ts b/gullet/src/tools.ts index 83f6b48..3efe9d4 100644 --- a/gullet/src/tools.ts +++ b/gullet/src/tools.ts @@ -306,8 +306,20 @@ async function tabsList( try { const result = (await ctx.request(conn.connectionId, "tabs_list", params)) as { tabs?: BridgeTab[]; + matched?: number; + }; + // `matched` is kept, not recomputed. A current extension truncates to + // `limit` before sending, so the tabs that arrive are not the tabs that + // matched, and its `matched` is the only place the real total survives. + // Recomputing it here reported the size of the page as the size of the + // result — the agent's one signal that it had not seen everything, lost + // exactly when there was more to see. An older extension sends no + // `matched`; that is what `undefined` means, and it is counted below. + return { + conn, + tabs: result?.tabs ?? [], + matched: typeof result?.matched === "number" ? result.matched : undefined, }; - return { conn, tabs: result?.tabs ?? [] }; } catch (err) { const { code, message } = toBridgeError(err); const failure = { @@ -316,7 +328,7 @@ async function tabsList( error: code, message, }; - return { conn, tabs: [] as BridgeTab[], failure }; + return { conn, tabs: [] as BridgeTab[], matched: undefined, failure }; } }), ); @@ -366,11 +378,20 @@ async function tabsList( connectionId: origin.get(selected.tabs[i] as BridgeTab)?.connectionId, })) : view.tabs; + // Per browser: its own `matched` when it filtered, otherwise what our filter + // made of everything it sent. Mixing the two is normal — one browser can be + // newer than the other — so this is resolved per connection and then summed, + // never taken from the merged set as a whole. + const matched = perBrowser.reduce( + (sum, r) => sum + (r.matched ?? filterTabs(r.tabs, listParams).length), + 0, + ); return { ...head, ...(view.windowId === undefined ? {} : { windowId: view.windowId }), - ...selected, tabs, + matched, + ...(matched > tabs.length ? { truncated: true } : {}), }; } diff --git a/gullet/tests/tools.test.ts b/gullet/tests/tools.test.ts index 2eb94ff..fa97f60 100644 --- a/gullet/tests/tools.test.ts +++ b/gullet/tests/tools.test.ts @@ -173,6 +173,44 @@ describe("tabs_list", () => { expect(result).toMatchObject({ matched: 3, truncated: true }); }); + // A current extension truncates before sending, so the page that arrives is + // not the match count. Recomputing `matched` here reported 2 of 900 as + // "matched: 2" with no `truncated` — the agent's only signal that more + // existed, lost precisely when it did. Invisible against an extension that + // sends everything, which is why it survived a live run. + test("keeps the browser's matched when the browser truncated for us", async () => { + const { call } = caller([zen], () => ({ + tabs: [tab(1, "https://x.com/a", 400), tab(2, "https://x.com/b", 300)], + matched: 900, + truncated: true, + })); + const result = payload(await call("tabs_list", { query: "x.com", limit: 2 })); + expect(result).toMatchObject({ matched: 900, truncated: true }); + }); + + test("falls back to its own count for a browser that sent no matched", async () => { + const { call } = caller([zen], () => ({ + tabs: [tab(1, "https://x.com/a"), tab(2, "https://x.com/b"), tab(3, "https://other.test/")], + })); + // No `matched` on the wire means the browser did not filter, so the honest + // total is what our own filter kept — not the three tabs it handed over. + expect(payload(await call("tabs_list", { query: "x.com" }))).toMatchObject({ matched: 2 }); + }); + + test("sums matched across browsers of different vintages", async () => { + const { call } = caller([zen, chrome], ({ connectionId }) => + connectionId === "conn-1" + ? { tabs: [tab(1, "https://x.com/a", 400)], matched: 500 } + : { tabs: [tab(2, "https://x.com/b", 300), tab(3, "https://no.test/")] }, + ); + // 500 reported by the new one, plus the single tab our filter keeps from + // the old one's three. + expect(payload(await call("tabs_list", { query: "x.com" }))).toMatchObject({ + matched: 501, + truncated: true, + }); + }); + // Regression, caught live: grouping ran on the unfiltered merge, so a query // plus groupBy counted the whole backlog. The browser here ignores `query` // entirely, which is the version skew that exposed it — a newer extension diff --git a/src/bridge-methods.ts b/src/bridge-methods.ts index b9e2962..ec01999 100644 --- a/src/bridge-methods.ts +++ b/src/bridge-methods.ts @@ -444,6 +444,11 @@ export class BridgeMethodRunner { // Gullet does the grouping, over every browser at once — and the full list // crossing loopback costs nothing, unlike the same list crossing into a // model's context, which is the only budget any of this is protecting. + // + // The Infinity stays local: `selectTabs` spends it on a `slice` and it is + // absent from `TabsListResult`, so it never meets `JSON.stringify`, which + // would silently turn it into `null`. Anything that later puts a limit on + // the wire has to send a real number. const limit = params.groupBy === undefined ? params.limit : Number.POSITIVE_INFINITY; return selectTabs(mapped, { ...params, limit }); } From 92244256a0f338fe43bfa53adcc9363fd4c8851e Mon Sep 17 00:00:00 2001 From: Michael Simon Date: Sun, 2 Aug 2026 21:33:42 -0700 Subject: [PATCH 06/11] Name the other sidecar when no browser is connected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Connected on 20317" in the browser and "No browser is connected" from every tool call are both true when two sidecars hold different tokens, and together they read as a broken bridge. Observed live: a session started before a token change held 4589 with the old token, a session started after it could not peer with that hub — a mismatched token must never be handed a proof — so it bound 20317, and the browser attached to whichever it found first. Every part behaved as designed and the pair of facts was still misleading. `Backend.rivalHubs()` re-probes the candidate ports on the no-connection path and the tool error names them, then points at the token as the thing that keeps two sidecars from merging. Probed live rather than read from the election's observations, because a rival can appear long after we settled — which is the case worth catching. Best-effort by construction: a throw inside the diagnosis must not replace the error it explains, and there is a test for that. Also documents what does *not* cause this: `bridgeToken` is minted only by an explicit click in options and `bridgeLastPort` lives in storage.local, so both survive an extension update. Only an uninstall clears them, which is what happened here. The port moving looks like update fragility and is not. Verified: bun run check. --- docs/BRIDGE.md | 21 ++++++++++++++++++ gullet/src/backend.ts | 34 ++++++++++++++++++++++++++++ gullet/src/main.ts | 1 + gullet/src/tools.ts | 45 +++++++++++++++++++++++++++++++++++++- gullet/tests/tools.test.ts | 40 +++++++++++++++++++++++++++++++++ 5 files changed, 140 insertions(+), 1 deletion(-) diff --git a/docs/BRIDGE.md b/docs/BRIDGE.md index 60668fa..f0c58ca 100644 --- a/docs/BRIDGE.md +++ b/docs/BRIDGE.md @@ -289,6 +289,27 @@ the old agent session is the upgrade path. - The options-page config snippet omits the port in automatic mode and includes the numeric flag only in fixed mode. +▸ **"Connected on 20317" and "no browser is connected" are both true when tokens split.** +Observed live: an agent session started before a token change held 4589 with the old token; +a session started after it could not peer with that hub — a mismatched token must never be +handed a proof — so it bound 20317, and the browser attached to whichever it found first. +Everything behaved as designed, and the user saw a lit badge naming a port while every tool +call insisted nothing was attached. That pair reads as a broken bridge, and cost real time +to unpick. + +The election already knew: `tryExistingHub` records a compatible-marker port it could not +join. `Backend.rivalHubs()` now re-probes candidates on the "no browser" path — live rather +than from those observations, since a rival can appear long after settling — and the tool +error names the port and points at the token. It is best-effort by construction: a throw in +the diagnosis must never replace the error it was explaining. + +**A normal extension update does not cause this.** `bridgeToken` is minted only by an +explicit click in the options page (default `""`, never auto-regenerated) and `bridgeLastPort` +is written to `storage.local` and put first by `orderedBridgePortCandidates` on the next +start — both survive an update. Only an _uninstall_ clears `storage.local`, which is what +happened here: a delete-and-reinstall regenerated the token, and the new token was what split +the realms. Worth stating because the port moving looks like update fragility and is not. + A filesystem rendezvous file is not part of this design. Gullet, Claude, and Codex could all read one, but a WebExtension cannot read an arbitrary config directory. Such a file may be useful later for human diagnostics; it cannot make the two halves discover each other diff --git a/gullet/src/backend.ts b/gullet/src/backend.ts index bf70066..c922769 100644 --- a/gullet/src/backend.ts +++ b/gullet/src/backend.ts @@ -29,6 +29,8 @@ export interface BridgeBackend { request(connectionId: string, method: BridgeMethod, params: unknown): Promise; /** Why nothing can be served right now, or null. Re-read on every call. */ fault(): BridgeError | null; + /** Candidate ports held by another Tabglutton hub. Diagnosis, not routing. */ + rivalHubs(): Promise; stop(): void; } @@ -263,6 +265,18 @@ export class Supervisor implements BridgeBackend { // Both roles wait the same first-call window: a peer inherits it inside the // hub it is attached to, a hub applies it here. No caller gets a knob — the // wait lives at the layer that owns it, so the roles cannot diverge. + /** + * Another Tabglutton hub holding one of the candidate ports, if any. + * + * Exists because "the extension says connected" and "no browser is connected" + * are both true when two hubs run with different tokens, and that pair of + * facts reads as a broken bridge rather than as the split it is. Costs a few + * loopback probes and is only ever called to explain a failure. + */ + async rivalHubs(): Promise { + return rivalHubPorts(this.candidatePorts(), this.activePort); + } + async connections(): Promise { await this.waitForSettling(); if (this.peer) return this.peer.connections(); @@ -287,6 +301,26 @@ export class Supervisor implements BridgeBackend { } } +/** + * Candidate ports answering as a Tabglutton hub that is **not** this process. + * + * Only ever asked on the "no browser is connected" path, so the probes cost + * nothing that matters and are done live rather than read from the election's + * observations — a rival can appear long after we settled, which is exactly the + * case worth catching. + * + * A compatible answer here almost always means a token mismatch: a hub sharing + * our token would have been joined as a peer instead of left running beside us. + * That is the diagnosis the caller turns into advice. + */ +async function rivalHubPorts(candidates: number[], activePort: number | null): Promise { + const others = candidates.filter((port) => port !== activePort); + const probes = await Promise.all( + others.map(async (port) => ((await probeCandidate(port)) === "compatible" ? port : null)), + ); + return probes.filter((port): port is number => port !== null); +} + type CandidateProbe = BridgeProbeIdentity | "silent"; async function probeCandidate(port: number): Promise { diff --git a/gullet/src/main.ts b/gullet/src/main.ts index 54724f1..d22d6b2 100644 --- a/gullet/src/main.ts +++ b/gullet/src/main.ts @@ -82,6 +82,7 @@ export async function main( // A port we never bound is the more proximate problem, and fixing the // token would not make this process serve anything either way. startupError: () => backend.fault() ?? tokenError, + rivalHubs: () => backend.rivalHubs(), }), }); diff --git a/gullet/src/tools.ts b/gullet/src/tools.ts index 3efe9d4..4ba105a 100644 --- a/gullet/src/tools.ts +++ b/gullet/src/tools.ts @@ -38,6 +38,12 @@ export interface ToolContext { * on refusing calls the backend had since become able to serve. */ startupError: () => BridgeError | null; + /** + * Candidate ports held by another Tabglutton hub, asked only when there is no + * browser to serve. Optional so tests and any future embedding can omit it — + * it explains a failure, it never changes one. + */ + rivalHubs?: () => Promise; } const BROWSER_PROPERTY = { @@ -247,7 +253,7 @@ export function createToolCaller( if (fault) throw new BridgeRequestError(fault.code, fault.message); return ok(await route(ctx, name, args)); } catch (err) { - return toolError(err); + return toolError(await explainNoConnection(ctx, err)); } }; } @@ -401,6 +407,43 @@ function ok(value: unknown): McpToolResult { return { content: [{ type: "text", text: JSON.stringify(value) }] }; } +/** + * Name the split when "no browser is connected" is true here and false in the + * browser, which is what two hubs with different tokens produce. + * + * The user sees Tabglutton's badge lit and reports the port it names, while + * every tool call insists nothing is attached — a pair of facts that reads as a + * broken bridge rather than as two sidecars that could not join each other. The + * hub election already handles this correctly (a mismatched token must never be + * handed a proof, so it binds elsewhere); all that was missing was saying so. + * + * Observed for real: an older agent session held 4589 with the token from before + * a reinstall, this one bound 20317 with the new one, and the browser attached + * to whichever it found first. + * + * Best-effort by construction — the probes are loopback and this is already the + * failure path, so a throw here must not replace the real error with its own. + */ +async function explainNoConnection(ctx: ToolContext, err: unknown): Promise { + if (!(err instanceof BridgeRequestError) || err.code !== "no-connection" || !ctx.rivalHubs) { + return err; + } + try { + const ports = await ctx.rivalHubs(); + if (ports.length === 0) return err; + return new BridgeRequestError( + err.code, + `${err.message} Another Tabglutton sidecar is already running on ` + + `127.0.0.1:${ports.join(", ")} and the browser may be attached to that one instead. ` + + `They could not merge, which means their tokens differ: check that this project's ` + + `TABGLUTTON_TOKEN matches the token in Tabglutton's settings, then restart the other ` + + `agent session (or this one) so they share a single connection.`, + ); + } catch { + return err; + } +} + function toolError(err: unknown): McpToolResult { const { code, message } = toBridgeError(err); return { diff --git a/gullet/tests/tools.test.ts b/gullet/tests/tools.test.ts index fa97f60..e19c516 100644 --- a/gullet/tests/tools.test.ts +++ b/gullet/tests/tools.test.ts @@ -309,6 +309,46 @@ describe("tab-scoped tools", () => { }); }); +describe("no-connection diagnosis", () => { + // The pair of facts that produced this: the browser's badge said connected on + // 20317 while every tool call here said nothing was attached. + test("names the rival sidecar and points at the token", async () => { + const { call } = caller([], () => ({}), { rivalHubs: async () => [4589] }); + const result = await call("tabs_list", {}); + expect(result.isError).toBe(true); + const { message } = payload(result) as { message: string }; + expect(message).toContain("127.0.0.1:4589"); + expect(message).toContain("TABGLUTTON_TOKEN"); + }); + + test("stays quiet when this really is the only sidecar", async () => { + const { call } = caller([], () => ({}), { rivalHubs: async () => [] }); + const { message } = payload(await call("tabs_list", {})) as { message: string }; + expect(message).not.toContain("127.0.0.1"); + }); + + // The diagnosis is a courtesy on a path that has already failed; it must never + // replace the real error with a failure of its own. + test("survives a probe that throws", async () => { + const { call } = caller([], () => ({}), { + rivalHubs: () => Promise.reject(new Error("loopback refused")), + }); + const result = await call("tabs_list", {}); + expect(payload(result)).toMatchObject({ error: "no-connection" }); + expect((payload(result) as { message: string }).message).not.toContain("loopback refused"); + }); + + test("leaves every other failure untouched", async () => { + const { call } = caller([zen], () => { + throw new BridgeRequestError("timeout", "tabs_list timed out."); + }); + const probed = caller([zen], () => ({}), { rivalHubs: async () => [4589] }); + expect(payload(await call("tabs_list", {}))).toMatchObject({ error: "timeout" }); + // A healthy browser never consults the diagnosis at all. + expect(payload(await probed.call("tabs_list", {}))).not.toHaveProperty("error"); + }); +}); + describe("error handling", () => { test("no connected browser is reported, not swallowed", async () => { const { call } = caller([], () => ({})); From 127d4e7b8d13d21db3fcc8be6d8bd648c8e6f458 Mon Sep 17 00:00:00 2001 From: Michael Simon Date: Sun, 2 Aug 2026 21:40:05 -0700 Subject: [PATCH 07/11] Let the access token be pasted, not only generated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The field was readonly and Generate was the only way to set it, which is wrong for a shared secret: the value may already exist at the other end — a second browser, a machine already configured, a password manager — and the extension was the one place it could not be entered. Now editable. Three things follow from it: - Persisted on `change`, not `input`. Every write of `bridgeToken` is a revocation, since the handshake pins the token it proved and a live socket drops. Saving per keystroke would tear the bridge down once per character being typed or pasted. - Trimmed. A copied secret routinely arrives with a trailing newline, and that produces a token that looks identical to the sidecar's and fails every handshake. - Empty still never persists. It is ambiguous — mid-paste, or selected-and-deleted on the way to typing — and writing it would revoke access for a keystroke rather than a decision. Turning the bridge off is the toggle; replacing the token is Generate or a paste. Also generalises the rival-sidecar note in docs/BRIDGE.md: it described one incident by port number, when the behaviour is about token realms splitting and has nothing to do with which candidates were involved. Verified: bun run check. --- docs/BRIDGE.md | 40 ++++++++++++++++++++++------------------ options/options.html | 19 ++++++++++++++++--- options/options.ts | 31 +++++++++++++++++++++++++------ 3 files changed, 63 insertions(+), 27 deletions(-) diff --git a/docs/BRIDGE.md b/docs/BRIDGE.md index f0c58ca..dc3e0c7 100644 --- a/docs/BRIDGE.md +++ b/docs/BRIDGE.md @@ -289,26 +289,30 @@ the old agent session is the upgrade path. - The options-page config snippet omits the port in automatic mode and includes the numeric flag only in fixed mode. -▸ **"Connected on 20317" and "no browser is connected" are both true when tokens split.** -Observed live: an agent session started before a token change held 4589 with the old token; -a session started after it could not peer with that hub — a mismatched token must never be -handed a proof — so it bound 20317, and the browser attached to whichever it found first. -Everything behaved as designed, and the user saw a lit badge naming a port while every tool -call insisted nothing was attached. That pair reads as a broken bridge, and cost real time -to unpick. - -The election already knew: `tryExistingHub` records a compatible-marker port it could not -join. `Backend.rivalHubs()` now re-probes candidates on the "no browser" path — live rather -than from those observations, since a rival can appear long after settling — and the tool -error names the port and points at the token. It is best-effort by construction: a throw in -the diagnosis must never replace the error it was explaining. +▸ **"Connected on \" and "no browser is connected" can both be true at once.** When +the token changes while an older agent session is still running, the two sidecars are in +different realms: the newer one cannot peer with the older hub — a mismatched token must +never be handed a proof — so it binds a different candidate, and the browser attaches to +whichever realm it finds first. If that is the older one, the extension's badge reports a +healthy connection on a real port while every tool call in the new session insists nothing +is attached. Every component behaves exactly as designed and the pair of facts still reads +as a broken bridge. Observed live; it cost real time to unpick. + +The election already knew: `tryExistingHub` records a compatible-marker candidate it could +not join. `Backend.rivalHubs()` now re-probes the candidates on the "no browser" path — +live rather than from those recorded observations, since a rival can appear long after we +settled — and the tool error names the endpoint it found and points at the token as the +reason two sidecars did not merge. Best-effort by construction: a throw inside the +diagnosis must never replace the error it was explaining. **A normal extension update does not cause this.** `bridgeToken` is minted only by an -explicit click in the options page (default `""`, never auto-regenerated) and `bridgeLastPort` -is written to `storage.local` and put first by `orderedBridgePortCandidates` on the next -start — both survive an update. Only an _uninstall_ clears `storage.local`, which is what -happened here: a delete-and-reinstall regenerated the token, and the new token was what split -the realms. Worth stating because the port moving looks like update fragility and is not. +explicit action in the options page (default `""`, never auto-regenerated) and +`bridgeLastPort` is written to `storage.local` and put first by +`orderedBridgePortCandidates` on the next start — both survive an update. Only an +_uninstall_ clears `storage.local`. That is the path that produced this: a +delete-and-reinstall regenerated the token, and the new token, not the reinstall, is what +split the realms. Worth stating because the endpoint moving looks like update fragility and +is not. A filesystem rendezvous file is not part of this design. Gullet, Claude, and Codex could all read one, but a WebExtension cannot read an arbitrary config directory. Such a file may be diff --git a/options/options.html b/options/options.html index a3d7dc6..f8844c6 100644 --- a/options/options.html +++ b/options/options.html @@ -211,8 +211,9 @@

Agent bridge

Access token Shared secret proving the sidecar is yours. It is never sent over the socket — both - sides prove they know it. Copy it into Gullet's environment; regenerating it - disconnects any sidecar still using the old one. + sides prove they know it. Copy it into Gullet's environment, or paste in one you + already have to match another browser or machine. Changing it disconnects any sidecar + still using the old one.
@@ -223,7 +224,19 @@

Agent bridge

puts it, so it reads as belonging to the token rather than as a third action alongside Copy and Generate. -->
- + +