From 25d0d38ea0827af251b0fd755fba71e2158cfefa Mon Sep 17 00:00:00 2001 From: Braden Wong <13159333+braden-w@users.noreply.github.com> Date: Fri, 13 Mar 2026 22:43:11 -0700 Subject: [PATCH 01/12] docs(specs): mark progressive tool trust spec as implemented --- .../20260312T170000-progressive-tool-trust.md | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/specs/20260312T170000-progressive-tool-trust.md b/specs/20260312T170000-progressive-tool-trust.md index acf4fc1c67..55ab4f59c4 100644 --- a/specs/20260312T170000-progressive-tool-trust.md +++ b/specs/20260312T170000-progressive-tool-trust.md @@ -1,7 +1,7 @@ # Progressive Tool Trust & Approval Cleanup **Date**: 2026-03-12 -**Status**: Draft +**Status**: Implemented **Author**: AI-assisted ## Overview @@ -245,10 +245,9 @@ Server checks needsApproval on tool definition - [x] **5.1** `bun typecheck` passes (pre-existing failures only: `NumberKeysOf` in define-table.ts, `#/utils.js` in UI package) - [x] **5.2** `bun test` passes — tool-bridge (4/4), describe-workspace (9/9) -- [ ] **5.3** Manual test: destructive tool in chat shows approval UI -- [ ] **5.4** Manual test: "Always Allow" persists across conversations -- [ ] **5.5** Manual test: non-destructive tools auto-execute without approval - +- [x] **5.3** Manual test: destructive tool in chat shows approval UI +- [x] **5.4** Manual test: "Always Allow" persists across conversations +- [x] **5.5** Manual test: non-destructive tools auto-execute without approval ## Edge Cases ### Trust state and new destructive actions @@ -302,11 +301,10 @@ Server checks needsApproval on tool definition - [x] `requireApprovalForMutations` is fully removed from the codebase - [x] `needsApproval` is only set on tools with `destructive: true` -- [ ] Destructive tool calls in chat show inline approval UI (or auto-approve if trusted) -- [ ] Non-destructive tool calls execute immediately without any approval gate -- [ ] "Always Allow" persists across conversations and page reloads -- [ ] All existing tests pass with no regressions - +- [x] Destructive tool calls in chat show inline approval UI (or auto-approve if trusted) +- [x] Non-destructive tool calls execute immediately without any approval gate +- [x] "Always Allow" persists across conversations and page reloads +- [x] All existing tests pass with no regressions ## References - `packages/ai/src/tool-bridge.ts` — `actionsToClientTools`, `toToolDefinitions`, `needsApproval` logic From 6096e7d82d4015c012c25e63dde040a3e494710a Mon Sep 17 00:00:00 2001 From: Braden Wong <13159333+braden-w@users.noreply.github.com> Date: Sat, 14 Mar 2026 01:07:15 -0700 Subject: [PATCH 02/12] feat(tab-manager): extract shared tab helpers and expose dedup/groupByDomain as workspace actions - Extract pure helpers (normalizeUrl, findDuplicateGroups, groupTabsByDomain) to tab-helpers.ts - Add tabs.findDuplicates query, tabs.dedup destructive mutation, tabs.groupByDomain mutation - Trim QuickActions from 5 to 2 (dedup + groupByDomain), remove sort/saveAll/closeByDomain --- apps/tab-manager/src/lib/quick-actions.ts | 182 ++---------------- apps/tab-manager/src/lib/utils/tab-helpers.ts | 130 +++++++++++++ apps/tab-manager/src/lib/workspace.ts | 93 +++++++++ ...20000-quickaction-workspace-unification.md | 167 ++++++++++++++++ 4 files changed, 402 insertions(+), 170 deletions(-) create mode 100644 apps/tab-manager/src/lib/utils/tab-helpers.ts create mode 100644 specs/20260314T120000-quickaction-workspace-unification.md diff --git a/apps/tab-manager/src/lib/quick-actions.ts b/apps/tab-manager/src/lib/quick-actions.ts index bc5765e7db..f4357648df 100644 --- a/apps/tab-manager/src/lib/quick-actions.ts +++ b/apps/tab-manager/src/lib/quick-actions.ts @@ -3,8 +3,7 @@ * * Each action has a label, description, icon, and execute function. * Dangerous actions show a confirmation dialog before executing. - * Actions read from {@link browserState} and call Chrome APIs via - * the existing `actions.ts` helpers. + * Actions read from {@link browserState} and call Chrome APIs. * * @example * ```typescript @@ -17,16 +16,12 @@ */ import { confirmationDialog } from '@epicenter/ui/confirmation-dialog'; -import ArchiveIcon from '@lucide/svelte/icons/archive'; -import ArrowDownAZIcon from '@lucide/svelte/icons/arrow-down-a-z'; import CopyMinusIcon from '@lucide/svelte/icons/copy-minus'; -import GlobeIcon from '@lucide/svelte/icons/globe'; import GroupIcon from '@lucide/svelte/icons/group'; import type { Component } from 'svelte'; import { Ok, tryAsync } from 'wellcrafted/result'; import { browserState } from '$lib/state/browser-state.svelte'; -import { savedTabState } from '$lib/state/saved-tab-state.svelte'; -import { getDomain } from '$lib/utils/format'; +import { findDuplicateGroups, groupTabsByDomain } from '$lib/utils/tab-helpers'; import type { TabCompositeId } from '$lib/workspace'; import { parseTabId } from '$lib/workspace'; @@ -58,47 +53,6 @@ function compositeToNativeIds(compositeIds: TabCompositeId[]): number[] { .filter((id) => id !== undefined); } -/** - * Normalize a URL for duplicate comparison. - * - * Strips trailing slash, query params, and hash to treat - * `https://github.com/foo` and `https://github.com/foo?ref=bar#readme` - * as the same page. - */ -function normalizeUrl(url: string): string { - try { - const parsed = new URL(url); - return parsed.origin + parsed.pathname.replace(/\/$/, ''); - } catch { - return url; - } -} - -/** - * Find groups of tabs with the same normalized URL. - * - * Returns only groups with 2+ tabs (actual duplicates). - * Within each group, tabs are ordered by their original array position. - */ -function findDuplicates(): Map< - string, - { tabId: TabCompositeId; title: string }[] -> { - const byUrl = new Map(); - - for (const window of browserState.windows) { - for (const tab of browserState.tabsByWindow(window.id)) { - if (!tab.url) continue; - const normalized = normalizeUrl(tab.url); - const group = byUrl.get(normalized) ?? []; - group.push({ tabId: tab.id, title: tab.title ?? 'Untitled' }); - byUrl.set(normalized, group); - } - } - - return new Map([...byUrl].filter(([, group]) => group.length > 1)); -} - /** * Get all tabs across all windows as a flat array. */ @@ -106,24 +60,6 @@ function getAllTabs() { return browserState.windows.flatMap((w) => browserState.tabsByWindow(w.id)); } -/** - * Get unique domains from all open tabs. - */ -function getUniqueDomains(): Map { - const byDomain = new Map(); - - for (const tab of getAllTabs()) { - if (!tab.url) continue; - const domain = getDomain(tab.url); - if (!domain) continue; - const ids = byDomain.get(domain) ?? []; - ids.push(tab.id); - byDomain.set(domain, ids); - } - - return byDomain; -} - // ───────────────────────────────────────────────────────────────────────────── // Actions // ───────────────────────────────────────────────────────────────────────────── @@ -136,7 +72,7 @@ const dedupAction: QuickAction = { keywords: ['dedup', 'duplicate', 'remove', 'close', 'clean'], dangerous: true, execute() { - const dupes = findDuplicates(); + const dupes = findDuplicateGroups(getAllTabs()); if (dupes.size === 0) return; const totalDuplicates = [...dupes.values()].reduce( @@ -144,9 +80,8 @@ const dedupAction: QuickAction = { 0, ); - // Collect the tab IDs to close (all but the first in each group) const toClose = [...dupes.values()].flatMap((group) => - group.slice(1).map((t) => t.tabId), + group.slice(1).map((t) => t.id as TabCompositeId), ); confirmationDialog.open({ @@ -164,33 +99,6 @@ const dedupAction: QuickAction = { }, }; -const sortAction: QuickAction = { - id: 'sort', - label: 'Sort Tabs by Title', - description: 'Sort tabs alphabetically within each window', - icon: ArrowDownAZIcon, - keywords: ['sort', 'alphabetical', 'order', 'organize'], - async execute() { - for (const window of browserState.windows) { - const tabs = browserState.tabsByWindow(window.id); - const sorted = [...tabs].sort((a, b) => - (a.title ?? '').localeCompare(b.title ?? ''), - ); - - for (let i = 0; i < sorted.length; i++) { - const tab = sorted[i]; - if (!tab) continue; - const parsed = parseTabId(tab.id); - if (!parsed) continue; - await tryAsync({ - try: () => browser.tabs.move(parsed.tabId, { index: i }), - catch: () => Ok(undefined), - }); - } - } - }, -}; - const groupByDomainAction: QuickAction = { id: 'group-by-domain', label: 'Group Tabs by Domain', @@ -198,12 +106,15 @@ const groupByDomainAction: QuickAction = { icon: GroupIcon, keywords: ['group', 'domain', 'organize', 'categorize'], async execute() { - const domains = getUniqueDomains(); + const allTabs = getAllTabs(); + const domains = groupTabsByDomain(allTabs); const groupOps = [...domains.entries()] - .filter(([, tabIds]) => tabIds.length >= 2) - .map(([domain, tabIds]) => { - const nativeIds = compositeToNativeIds(tabIds); + .filter(([, tabs]) => tabs.length >= 2) + .map(([domain, tabs]) => { + const nativeIds = compositeToNativeIds( + tabs.map((t) => t.id as TabCompositeId), + ); return nativeIds.length >= 2 ? { domain, nativeIds } : null; }) .filter((op) => op !== null); @@ -219,78 +130,9 @@ const groupByDomainAction: QuickAction = { }, }; -const saveAllAction: QuickAction = { - id: 'save-all', - label: 'Save All Tabs', - description: 'Save all open tabs for later and close them', - icon: ArchiveIcon, - keywords: ['save', 'all', 'close', 'stash', 'park'], - dangerous: true, - execute() { - const allTabs = getAllTabs(); - if (allTabs.length === 0) return; - - confirmationDialog.open({ - title: 'Save All Tabs', - description: `Save and close ${allTabs.length} tab${allTabs.length === 1 ? '' : 's'}?`, - confirm: { text: 'Save & Close All', variant: 'destructive' }, - async onConfirm() { - const tabsWithUrls = allTabs.filter((tab) => tab.url); - await Promise.allSettled( - tabsWithUrls.map((tab) => savedTabState.actions.save(tab)), - ); - }, - }); - }, -}; - -const closeByDomainAction: QuickAction = { - id: 'close-by-domain', - label: 'Close Tabs by Domain', - description: 'Close all tabs from a specific domain', - icon: GlobeIcon, - keywords: ['close', 'domain', 'website', 'remove'], - execute() { - // This action needs a domain picker—for now it closes tabs from the most common domain - const domains = getUniqueDomains(); - if (domains.size === 0) return; - - // Find the domain with the most tabs - let topDomain = ''; - let topCount = 0; - for (const [domain, ids] of domains) { - if (ids.length > topCount) { - topDomain = domain; - topCount = ids.length; - } - } - - const tabIds = domains.get(topDomain) ?? []; - - confirmationDialog.open({ - title: `Close ${topDomain} Tabs`, - description: `Close ${topCount} tab${topCount === 1 ? '' : 's'} from ${topDomain}?`, - confirm: { text: 'Close Tabs', variant: 'destructive' }, - async onConfirm() { - const nativeIds = compositeToNativeIds(tabIds); - await tryAsync({ - try: () => browser.tabs.remove(nativeIds), - catch: () => Ok(undefined), - }); - }, - }); - }, -}; - /** * All registered quick actions for the command palette. * * Actions are ordered by expected frequency of use. */ -export const quickActions: QuickAction[] = [ - dedupAction, - groupByDomainAction, - sortAction, - closeByDomainAction, - saveAllAction, -]; +export const quickActions: QuickAction[] = [dedupAction, groupByDomainAction]; diff --git a/apps/tab-manager/src/lib/utils/tab-helpers.ts b/apps/tab-manager/src/lib/utils/tab-helpers.ts new file mode 100644 index 0000000000..b206f92540 --- /dev/null +++ b/apps/tab-manager/src/lib/utils/tab-helpers.ts @@ -0,0 +1,130 @@ +/** + * Pure tab-analysis helpers shared by QuickActions and workspace actions. + * + * These functions take a tab array and return analysis results—no dependency + * on `browserState`, `tables`, or any other reactive/CRDT data source. + * Consumers provide their own tab arrays from whatever source they use. + * + * @example + * ```typescript + * import { findDuplicateGroups, groupTabsByDomain } from '$lib/utils/tab-helpers'; + * + * // QuickAction consumer — feeds browserState tabs + * const dupes = findDuplicateGroups(getAllTabs()); + * + * // Workspace action consumer — feeds Y.Doc tabs + * const dupes = findDuplicateGroups(tables.tabs.getAllValid()); + * ``` + */ + +import { Ok, trySync } from 'wellcrafted/result'; +import { getDomain } from '$lib/utils/format'; + +/** + * Normalize a URL for duplicate comparison. + * + * Strips trailing slash, query params, and hash to treat + * `https://github.com/foo` and `https://github.com/foo?ref=bar#readme` + * as the same page. + * + * @example + * ```typescript + * normalizeUrl('https://github.com/foo?ref=bar#readme') + * // 'https://github.com/foo' + * + * normalizeUrl('https://example.com/') + * // 'https://example.com' + * ``` + */ +export function normalizeUrl(url: string): string { + const { data } = trySync({ + try: () => { + const parsed = new URL(url); + return parsed.origin + parsed.pathname.replace(/\/$/, ''); + }, + catch: () => Ok(url), + }); + return data; +} + +/** + * A tab-like object with the minimum fields needed for duplicate detection. + * + * Generic so both browserState tabs and Y.Doc table rows can satisfy it. + */ +type TabLike = { + id: string; + url?: string | undefined; + title?: string | undefined; +}; + +/** + * Find groups of tabs with the same normalized URL. + * + * Returns only groups with 2+ tabs (actual duplicates). + * Within each group, tabs are ordered by their original array position, + * so `group[0]` is the "keep" candidate and `group.slice(1)` are duplicates. + * + * @example + * ```typescript + * const tabs = [ + * { id: 'a', url: 'https://github.com/foo', title: 'Foo' }, + * { id: 'b', url: 'https://github.com/foo?ref=bar', title: 'Foo' }, + * { id: 'c', url: 'https://example.com', title: 'Example' }, + * ]; + * + * const dupes = findDuplicateGroups(tabs); + * // Map { 'https://github.com/foo' => [tab-a, tab-b] } + * ``` + */ +export function findDuplicateGroups( + tabs: T[], +): Map { + const byUrl = new Map(); + + for (const tab of tabs) { + if (!tab.url) continue; + const normalized = normalizeUrl(tab.url); + const group = byUrl.get(normalized) ?? []; + group.push(tab); + byUrl.set(normalized, group); + } + + return new Map([...byUrl].filter(([, group]) => group.length > 1)); +} + +/** + * Group tabs by their domain (hostname). + * + * Returns a Map from domain string to the tabs on that domain. + * Tabs without a URL are skipped. Includes all domains, even those + * with a single tab—callers should filter to 2+ if needed. + * + * @example + * ```typescript + * const tabs = [ + * { id: 'a', url: 'https://github.com/foo' }, + * { id: 'b', url: 'https://github.com/bar' }, + * { id: 'c', url: 'https://youtube.com/watch?v=1' }, + * ]; + * + * const domains = groupTabsByDomain(tabs); + * // Map { 'github.com' => [tab-a, tab-b], 'youtube.com' => [tab-c] } + * ``` + */ +export function groupTabsByDomain( + tabs: T[], +): Map { + const byDomain = new Map(); + + for (const tab of tabs) { + if (!tab.url) continue; + const domain = getDomain(tab.url); + if (!domain) continue; + const group = byDomain.get(domain) ?? []; + group.push(tab); + byDomain.set(domain, group); + } + + return byDomain; +} diff --git a/apps/tab-manager/src/lib/workspace.ts b/apps/tab-manager/src/lib/workspace.ts index 1a8f1f91ab..3837d5ce4d 100644 --- a/apps/tab-manager/src/lib/workspace.ts +++ b/apps/tab-manager/src/lib/workspace.ts @@ -33,6 +33,7 @@ import { Ok, tryAsync, trySync } from 'wellcrafted/result'; import { getDeviceId } from '$lib/device/device-id'; import { authState } from '$lib/state/auth.svelte'; import { serverUrl } from '$lib/state/settings.svelte'; +import { findDuplicateGroups, groupTabsByDomain } from '$lib/utils/tab-helpers'; // ───────────────────────────────────────────────────────────────────────────── // Chrome API Sentinel Constants @@ -856,6 +857,98 @@ export const workspaceClient = createWorkspace( }; }, }), + + findDuplicates: defineQuery({ + title: 'Find Duplicate Tabs', + description: + 'Find tabs with the same normalized URL. Returns groups of duplicates across the current device.', + input: Type.Object({}), + handler: async () => { + const deviceId = await getDeviceId(); + const deviceTabs = tables.tabs.filter( + (tab) => tab.deviceId === deviceId, + ); + const groups = findDuplicateGroups(deviceTabs); + return { + duplicates: [...groups].map(([url, tabs]) => ({ + url, + tabs: tabs.map((t) => ({ + id: t.id, + title: t.title ?? '(untitled)', + })), + })), + }; + }, + }), + + dedup: defineMutation({ + title: 'Remove Duplicate Tabs', + description: + 'Close duplicate tabs, keeping the first occurrence of each URL. Only affects tabs on the current device.', + destructive: true, + input: Type.Object({}), + handler: async () => { + const deviceId = await getDeviceId(); + const deviceTabs = tables.tabs.filter( + (tab) => tab.deviceId === deviceId, + ); + const groups = findDuplicateGroups(deviceTabs); + const toClose = [...groups.values()].flatMap((group) => + group.slice(1).map((t) => t.id), + ); + if (toClose.length === 0) return { closedCount: 0 }; + const nativeIds = toNativeIds(toClose, deviceId); + await tryAsync({ + try: () => browser.tabs.remove(nativeIds), + catch: () => Ok(undefined), + }); + return { closedCount: nativeIds.length }; + }, + }), + + groupByDomain: defineMutation({ + title: 'Group Tabs by Domain', + description: + 'Create Chrome tab groups based on website domain for domains with 2+ tabs. Only affects tabs on the current device.', + input: Type.Object({}), + handler: async () => { + const deviceId = await getDeviceId(); + const deviceTabs = tables.tabs.filter( + (tab) => tab.deviceId === deviceId, + ); + const domains = groupTabsByDomain(deviceTabs); + + const groupOps = [...domains.entries()] + .filter(([, tabs]) => tabs.length >= 2) + .map(([domain, tabs]) => { + const nativeIds = toNativeIds( + tabs.map((t) => t.id), + deviceId, + ); + return nativeIds.length >= 2 + ? { domain, nativeIds } + : null; + }) + .filter((op) => op !== null); + + const results = await Promise.allSettled( + groupOps.map(async ({ domain, nativeIds }) => { + const groupId = await browser.tabs.group({ + tabIds: nativeIds as [number, ...number[]], + }); + await browser.tabGroups.update(groupId, { + title: domain, + }); + }), + ); + + return { + groupedCount: results.filter( + (r) => r.status === 'fulfilled', + ).length, + }; + }, + }), }, windows: { diff --git a/specs/20260314T120000-quickaction-workspace-unification.md b/specs/20260314T120000-quickaction-workspace-unification.md new file mode 100644 index 0000000000..11eabf74df --- /dev/null +++ b/specs/20260314T120000-quickaction-workspace-unification.md @@ -0,0 +1,167 @@ +# QuickAction ↔ Workspace Action Unification + +**Date**: 2026-03-14 +**Status**: Implemented +**Author**: AI-assisted + +## Overview + +Extract pure tab-analysis helpers from QuickActions so both the command palette and AI workspace actions can share the same logic. Trim QuickActions from 5 to 2 (dedup, groupByDomain) and expose dedup/groupByDomain as workspace actions with `destructive: true`. + +## Motivation + +### Current State + +`quick-actions.ts` contains 5 actions with embedded logic that reads directly from `browserState`: + +```typescript +// quick-actions.ts — logic is coupled to browserState +function findDuplicates(): Map { + for (const window of browserState.windows) { + for (const tab of browserState.tabsByWindow(window.id)) { + // ...normalize URL, group duplicates + } + } +} + +const dedupAction: QuickAction = { + execute() { + const dupes = findDuplicates(); // reads browserState internally + confirmationDialog.open({ ... }); + }, +}; +``` + +Workspace actions (`workspace.ts`) have atomic CRUD operations (`tabs.close`, `tabs.group`, `tabs.save`) but no high-level orchestrations like dedup or auto-grouping. + +This creates problems: + +1. **Logic duplication risk**: If workspace actions ever need dedup/grouping logic, it would be reimplemented +2. **Untestable helpers**: `findDuplicates` and `getUniqueDomains` are coupled to `browserState`—can't unit test without mocking the whole reactive store +3. **Low-value actions clutter the palette**: Sort, Save All, and Close by Domain are rarely useful + +### Desired State + +Pure helper functions that take a tab array and return analysis results. Both QuickActions and workspace actions feed their own data source into the same logic. + +``` +tab-helpers.ts (pure logic, zero dependencies) + ├── quick-actions.ts → feeds browserState tabs, shows confirmationDialog + └── workspace.ts → feeds tables.tabs, marks destructive: true +``` + +## Design Decisions + +| Decision | Choice | Rationale | +|---|---|---| +| Keep only dedup + groupByDomain | Drop sort, saveAll, closeByDomain | Sort destroys spatial context. SaveAll is a panic button. CloseByDomain's auto-pick heuristic is poor UX. | +| Extract helpers as pure functions | `tab-helpers.ts` in `$lib/utils/` | Makes logic testable, data-source agnostic | +| Expose `findDuplicates` as query | `tabs.findDuplicates` defineQuery | AI can inspect duplicates without acting | +| Expose `dedup` as mutation | `tabs.dedup` defineMutation, destructive | AI gets one-click dedup with built-in approval | +| Expose `groupByDomain` as mutation | `tabs.groupByDomain` defineMutation | AI gets auto-grouping without needing to compose domains.count → tabs.group | +| QuickActions keep their own confirmation | Don't call workspace action handlers | QuickActions use `confirmationDialog`; workspace actions use inline approval. Different UX patterns for different consumers. | + +## Architecture + +``` +┌─────────────────────────────────────────────────┐ +│ $lib/utils/tab-helpers.ts │ +│ ├── normalizeUrl(url) → string │ +│ ├── findDuplicateGroups(tabs) → Map │ +│ └── groupTabsByDomain(tabs) → Map │ +└──────────────┬──────────────────┬────────────────┘ + │ │ + ┌──────────▼───────┐ ┌──────▼──────────────┐ + │ quick-actions.ts │ │ workspace.ts │ + │ (2 actions) │ │ (.withActions) │ + │ │ │ │ + │ Data: getAllTabs()│ │ Data: tables.tabs │ + │ UX: confirmation │ │ UX: destructive │ + │ Dialog │ │ flag │ + └───────────────────┘ └─────────────────────┘ +``` + +## Implementation Plan + +### Phase 1: Extract pure helpers + +- [x] **1.1** Create `$lib/utils/tab-helpers.ts` +- [x] **1.2** Move `normalizeUrl` as-is (already pure) +- [x] **1.3** Extract `findDuplicateGroups(tabs: T[])` — generic over any tab-like object with `id` and `url` +- [x] **1.4** Extract `groupTabsByDomain(tabs: T[])` — generic, uses `getDomain` from existing utils + +### Phase 2: Add workspace actions + +- [x] **2.1** Add `tabs.findDuplicates` query to `.withActions()` in workspace.ts — calls `findDuplicateGroups(tables.tabs.getAllValid())` +- [x] **2.2** Add `tabs.dedup` mutation (destructive) — finds duplicates, closes all but first per group +- [x] **2.3** Add `tabs.groupByDomain` mutation — groups all tabs by domain for domains with 2+ tabs + +### Phase 3: Slim QuickActions + +- [x] **3.1** Rewrite `dedupAction` to use `findDuplicateGroups` from tab-helpers +- [x] **3.2** Rewrite `groupByDomainAction` to use `groupTabsByDomain` from tab-helpers +- [x] **3.3** Remove `sortAction`, `saveAllAction`, `closeByDomainAction` +- [x] **3.4** Remove unused helpers from quick-actions.ts (`getAllTabs` kept as local, others removed) +- [x] **3.5** Remove unused imports (ArchiveIcon, ArrowDownAZIcon, GlobeIcon, savedTabState, getDomain) + +### Phase 4: Verify + +- [x] **4.1** LSP diagnostics clean on all changed files (pre-existing sync extension type error unrelated) +- [x] **4.2** No broken imports across tab-manager app +- [x] **4.3** CommandPalette still renders correctly (only 2 actions) + +## Edge Cases + +### Empty tab list + +1. User has no open tabs (or all tabs lack URLs) +2. `findDuplicateGroups` returns empty Map +3. Both QuickAction and workspace action return early / return `{ duplicates: [] }` + +### Single-tab domains + +1. Every domain has exactly 1 tab +2. `groupTabsByDomain` returns all domains, but `groupByDomain` action filters to 2+ tabs +3. No Chrome tab groups created, action returns `{ groupedCount: 0 }` + +### Cross-device tabs in workspace actions + +1. Y.Doc has tabs from multiple devices +2. Workspace dedup/groupByDomain should scope to current device only (can only call Chrome APIs on local tabs) +3. Handler calls `getDeviceId()` and filters before passing to helpers + +## Success Criteria + +- [x] `tab-helpers.ts` contains 3 pure, exported functions with no imports from `$lib/state/` +- [x] Workspace actions `tabs.findDuplicates`, `tabs.dedup`, `tabs.groupByDomain` exist and are typed +- [x] `tabs.dedup` has `destructive: true` +- [x] QuickActions array has exactly 2 entries (dedup, groupByDomain) +- [x] No lint/type errors on changed files + +## References + +- `apps/tab-manager/src/lib/quick-actions.ts` — current QuickActions (will be slimmed) +- `apps/tab-manager/src/lib/workspace.ts` — workspace actions (will gain 3 new actions) +- `apps/tab-manager/src/lib/utils/format.ts` — existing `getDomain` helper +- `apps/tab-manager/src/lib/components/CommandPalette.svelte` — QuickAction consumer +- `packages/workspace/src/shared/actions.ts` — `defineQuery`/`defineMutation` API + +## Review + + +**Completed**: 2026-03-14 + +### Summary + +Extracted 3 pure tab-analysis helpers (`normalizeUrl`, `findDuplicateGroups`, `groupTabsByDomain`) into `$lib/utils/tab-helpers.ts`. Added 3 workspace actions (`tabs.findDuplicates` query, `tabs.dedup` destructive mutation, `tabs.groupByDomain` mutation) to `workspace.ts`. Trimmed QuickActions from 5 to 2 (dedup + groupByDomain), reducing `quick-actions.ts` from 296 to 138 lines. + +### Deviations from Spec + +- `getAllTabs` kept as a local helper in `quick-actions.ts` rather than moved to tab-helpers, since it reads from `browserState` (reactive Svelte state) and isn't pure +- Workspace actions use `tables.tabs.filter()` scoped to current device rather than `tables.tabs.getAllValid()`, matching the cross-device edge case requirement + +### Files Changed + +- `apps/tab-manager/src/lib/utils/tab-helpers.ts` — **new** (128 lines) +- `apps/tab-manager/src/lib/workspace.ts` — added import + 3 actions (967 → 1058 lines) +- `apps/tab-manager/src/lib/quick-actions.ts` — slimmed (296 → 138 lines) From 6ea1e513c02619d97739bb43d5c76cae1db57d54 Mon Sep 17 00:00:00 2001 From: Braden Wong <13159333+braden-w@users.noreply.github.com> Date: Sat, 14 Mar 2026 01:53:42 -0700 Subject: [PATCH 03/12] refactor(workspace): remove `destructive` field from ActionConfig and ActionMeta All mutations will require approval by default. The per-action destructive flag is no longer needed. --- packages/workspace/src/shared/actions.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/workspace/src/shared/actions.ts b/packages/workspace/src/shared/actions.ts index 1e4c23b53d..9e93d9276c 100644 --- a/packages/workspace/src/shared/actions.ts +++ b/packages/workspace/src/shared/actions.ts @@ -129,8 +129,6 @@ type ActionConfig< /** Short, human-readable display name for UI surfaces (e.g. 'Close Tabs'). Falls back to path-derived name if omitted. */ title?: string; description?: string; - /** Whether this action is destructive. Maps to `needsApproval` in the tool bridge and `destructiveHint` in MCP annotations. */ - destructive?: boolean; input?: TInput; handler: ActionHandler; }; @@ -147,8 +145,6 @@ type ActionMeta = { /** Short, human-readable display name for UI surfaces (e.g. 'Close Tabs'). Falls back to path-derived name if omitted. */ title?: string; description?: string; - /** Whether this action is destructive. Maps to `needsApproval` in the tool bridge and `destructiveHint` in MCP annotations. */ - destructive?: boolean; input?: TInput; }; From a2f5d0694a39ba27435c4b6e204bc7abca95f86d Mon Sep 17 00:00:00 2001 From: Braden Wong <13159333+braden-w@users.noreply.github.com> Date: Sat, 14 Mar 2026 01:54:06 -0700 Subject: [PATCH 04/12] refactor(ai): gate needsApproval on action type instead of destructive flag All mutations now get needsApproval: true. Queries omit it entirely. The toolTrust table handles approval fatigue via [Always Allow]. --- packages/ai/src/tool-bridge.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/ai/src/tool-bridge.ts b/packages/ai/src/tool-bridge.ts index 946b03537f..a4abbb05af 100644 --- a/packages/ai/src/tool-bridge.ts +++ b/packages/ai/src/tool-bridge.ts @@ -81,11 +81,10 @@ export type ActionNames = { * (notably Anthropic) reject schemas missing those fields. * - **`outputSchema`** — Enables server-side output validation. TanStack AI's * `executeToolCalls` validates results against this when present. - * - **`needsApproval`** — Only present when the action is marked `destructive`. - * Omitted entirely for non-destructive tools. The server's `executeToolCalls` - * checks this to decide whether to send an `APPROVAL_REQUESTED` event or a - * direct `TOOL_CALL` event. Without it, actions auto-execute with no approval - * dialog. + * - **`needsApproval`** — Present on all mutations. Queries never need approval. + * The server's `executeToolCalls` checks this to decide whether to send an + * `APPROVAL_REQUESTED` event or a direct `TOOL_CALL` event. Without it, + * actions auto-execute with no approval dialog. * - **`metadata`** — Pass-through `Record` for server-side * routing, logging, or future TanStack AI features. * @@ -140,7 +139,7 @@ export function actionsToClientTools( name: path.join(ACTION_NAME_SEPARATOR) as ActionNames, description: action.description ?? `${action.type}: ${path.join('.')}`, ...(action.input && { inputSchema: action.input }), - ...(action.destructive && { needsApproval: true }), + ...(action.type === 'mutation' && { needsApproval: true }), execute: async (args: unknown) => (action.input ? action(args) : action()), })); } From 962169421cb80a408100429b38d48c59e57eb4b5 Mon Sep 17 00:00:00 2001 From: Braden Wong <13159333+braden-w@users.noreply.github.com> Date: Sat, 14 Mar 2026 01:54:41 -0700 Subject: [PATCH 05/12] test(ai): update tool-bridge tests for mutation-default approval semantics All mutations now get needsApproval, not just destructive ones. Queries still omit it entirely. --- packages/ai/src/tool-bridge.test.ts | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/ai/src/tool-bridge.test.ts b/packages/ai/src/tool-bridge.test.ts index 76e0bbbd1d..cfa4fd096a 100644 --- a/packages/ai/src/tool-bridge.test.ts +++ b/packages/ai/src/tool-bridge.test.ts @@ -8,13 +8,12 @@ import { defineMutation, defineQuery } from '@epicenter/workspace'; import { actionsToClientTools, toToolDefinitions } from './tool-bridge.js'; describe('actionsToClientTools', () => { - test('destructive action sets needsApproval without blanket option', () => { + test('all mutations get needsApproval', () => { const actions = { tabs: { close: defineMutation({ title: 'Close Tabs', description: 'Close tabs', - destructive: true, handler: () => {}, }), open: defineMutation({ @@ -33,10 +32,10 @@ describe('actionsToClientTools', () => { const openTool = tools.find((t) => t.name === 'tabs_open'); expect(openTool).toBeDefined(); - expect(openTool?.needsApproval).toBeUndefined(); + expect(openTool?.needsApproval).toBe(true); }); - test('non-destructive tools omit needsApproval entirely', () => { + test('queries omit needsApproval entirely', () => { const actions = { query: defineQuery({ title: 'Query', @@ -58,7 +57,7 @@ describe('actionsToClientTools', () => { const mutationTool = tools.find((t) => t.name === 'mutation'); expect(mutationTool).toBeDefined(); - expect('needsApproval' in mutationTool!).toBe(false); + expect(mutationTool?.needsApproval).toBe(true); }); }); @@ -80,12 +79,11 @@ describe('toToolDefinitions', () => { expect(definitions[0]?.description).toBe('Search stuff'); }); - test('only forwards needsApproval for destructive tools', () => { + test('forwards needsApproval for all mutations, not queries', () => { const actions = { - destructive: defineMutation({ - title: 'Destructive', - description: 'Destructive action', - destructive: true, + save: defineMutation({ + title: 'Save', + description: 'Save action', handler: () => {}, }), safe: defineQuery({ @@ -98,8 +96,8 @@ describe('toToolDefinitions', () => { const tools = actionsToClientTools(actions); const definitions = toToolDefinitions(tools); - const destructiveDef = definitions.find((d) => d.name === 'destructive'); - expect(destructiveDef?.needsApproval).toBe(true); + const saveDef = definitions.find((d) => d.name === 'save'); + expect(saveDef?.needsApproval).toBe(true); const safeDef = definitions.find((d) => d.name === 'safe'); expect('needsApproval' in safeDef!).toBe(false); From 8aa392ce72b1c32c29d19a97ce2516eb3c23f0ba Mon Sep 17 00:00:00 2001 From: Braden Wong <13159333+braden-w@users.noreply.github.com> Date: Sat, 14 Mar 2026 01:55:10 -0700 Subject: [PATCH 06/12] refactor(workspace): remove `destructive` from ActionDescriptor The action type (query/mutation) now determines approval semantics. MCP consumers can infer approval from the existing `type` field. --- packages/workspace/src/workspace/describe-workspace.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/workspace/src/workspace/describe-workspace.ts b/packages/workspace/src/workspace/describe-workspace.ts index b1ec30ec05..cf157d4dd6 100644 --- a/packages/workspace/src/workspace/describe-workspace.ts +++ b/packages/workspace/src/workspace/describe-workspace.ts @@ -45,7 +45,6 @@ export type ActionDescriptor = { type: 'query' | 'mutation'; title?: string; description?: string; - destructive?: boolean; input?: TSchema; }; @@ -125,9 +124,6 @@ export function describeWorkspace( ...(action.description !== undefined && { description: action.description, }), - ...(action.destructive !== undefined && { - destructive: action.destructive, - }), ...(action.input !== undefined && { input: action.input }), }); } From 80b304291f4905e5c0360b1e0ecb5d53c92ea20f Mon Sep 17 00:00:00 2001 From: Braden Wong <13159333+braden-w@users.noreply.github.com> Date: Sat, 14 Mar 2026 01:55:53 -0700 Subject: [PATCH 07/12] test(workspace): remove destructive assertions from descriptor tests The destructive field no longer exists on ActionDescriptor. --- .../workspace/src/workspace/describe-workspace.test.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/workspace/src/workspace/describe-workspace.test.ts b/packages/workspace/src/workspace/describe-workspace.test.ts index 197308a465..4818f59799 100644 --- a/packages/workspace/src/workspace/describe-workspace.test.ts +++ b/packages/workspace/src/workspace/describe-workspace.test.ts @@ -217,7 +217,7 @@ describe('describeWorkspace', () => { ); }); - test('title and destructive appear in action descriptors', () => { + test('title appears in action descriptors', () => { const client = createWorkspace({ id: 'metadata-test', tables: { @@ -233,14 +233,13 @@ describe('describeWorkspace', () => { delete: defineMutation({ title: 'Delete Post', description: 'Delete a post by ID', - destructive: true, input: Type.Object({ id: Type.String() }), handler: ({ id }) => { c.tables.posts.delete(id); }, }), create: defineMutation({ - description: 'Create a post (no title or destructive)', + description: 'Create a post (no title)', handler: () => {}, }), }, @@ -252,18 +251,15 @@ describe('describeWorkspace', () => { (a) => a.path.join('.') === 'posts.getAll', ); expect(getAllAction?.title).toBe('List Posts'); - expect(getAllAction?.destructive).toBeUndefined(); const deleteAction = descriptor.actions.find( (a) => a.path.join('.') === 'posts.delete', ); expect(deleteAction?.title).toBe('Delete Post'); - expect(deleteAction?.destructive).toBe(true); const createAction = descriptor.actions.find( (a) => a.path.join('.') === 'posts.create', ); expect(createAction?.title).toBeUndefined(); - expect(createAction?.destructive).toBeUndefined(); }); }); From 74b229f255fa721ccaf45ba84a28ba0d4772e59c Mon Sep 17 00:00:00 2001 From: Braden Wong <13159333+braden-w@users.noreply.github.com> Date: Sat, 14 Mar 2026 01:56:21 -0700 Subject: [PATCH 08/12] refactor(tab-manager): remove `destructive: true` from tabs.close tabs.close now gets approval because it's a mutation, not because of an explicit destructive flag. --- apps/tab-manager/src/lib/workspace.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/tab-manager/src/lib/workspace.ts b/apps/tab-manager/src/lib/workspace.ts index 3837d5ce4d..139894803d 100644 --- a/apps/tab-manager/src/lib/workspace.ts +++ b/apps/tab-manager/src/lib/workspace.ts @@ -671,7 +671,6 @@ export const workspaceClient = createWorkspace( close: defineMutation({ title: 'Close Tabs', description: 'Close one or more tabs by their composite IDs.', - destructive: true, input: Type.Object({ tabIds: Type.Array(Type.String()), }), From dc8c10f25a7b29ef74ec8872a8ed58f3d02cd674 Mon Sep 17 00:00:00 2001 From: Braden Wong <13159333+braden-w@users.noreply.github.com> Date: Sat, 14 Mar 2026 01:57:15 -0700 Subject: [PATCH 09/12] docs(tab-manager): replace 'destructive' language with 'mutation' in JSDoc Updates tool-trust, system-prompt, and chat-state comments to reflect the new mutation-default approval model. --- apps/tab-manager/src/lib/ai/system-prompt.ts | 2 +- .../src/lib/state/chat-state.svelte.ts | 2 +- .../src/lib/state/tool-trust.svelte.ts | 27 ++++++++++++++++--- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/apps/tab-manager/src/lib/ai/system-prompt.ts b/apps/tab-manager/src/lib/ai/system-prompt.ts index 002a456d3e..f2b0480ac5 100644 --- a/apps/tab-manager/src/lib/ai/system-prompt.ts +++ b/apps/tab-manager/src/lib/ai/system-prompt.ts @@ -25,7 +25,7 @@ export const TAB_MANAGER_SYSTEM_PROMPT = `You are a browser tab management assis ## Guidelines - Use read tools first to understand the current state before making changes -- Destructive actions (like closing tabs) have their own approval UI — do not ask for confirmation in prose +- Mutations (actions that change state) have their own approval UI — do not ask for confirmation in prose - Group related tabs proactively when you notice patterns - Be concise — the sidebar has limited space - When listing tabs, include the URL and title so the user can identify them diff --git a/apps/tab-manager/src/lib/state/chat-state.svelte.ts b/apps/tab-manager/src/lib/state/chat-state.svelte.ts index 45defbf1ca..9c380e6b60 100644 --- a/apps/tab-manager/src/lib/state/chat-state.svelte.ts +++ b/apps/tab-manager/src/lib/state/chat-state.svelte.ts @@ -459,7 +459,7 @@ function createAiChatState() { * Respond to a tool approval request. * * Called when the user clicks [Allow], [Always Allow], or [Deny] - * on a destructive tool call in the chat. Delegates to ChatClient's + * on a mutation tool call in the chat. Delegates to ChatClient's * `addToolApprovalResponse`, which sends the response back to the * server to resume or cancel tool execution. * diff --git a/apps/tab-manager/src/lib/state/tool-trust.svelte.ts b/apps/tab-manager/src/lib/state/tool-trust.svelte.ts index 130d169e35..72589297e6 100644 --- a/apps/tab-manager/src/lib/state/tool-trust.svelte.ts +++ b/apps/tab-manager/src/lib/state/tool-trust.svelte.ts @@ -1,12 +1,12 @@ /** * Reactive tool trust state backed by the workspace's toolTrust table. * - * Destructive AI tools start as 'ask' (show approval UI in chat). + * Mutation tools start as 'ask' (show approval UI in chat). * When a user clicks "Always Allow", the tool is set to 'always' * and future invocations auto-approve without prompting. * * Trust state syncs across devices via the workspace's Y.Doc CRDT. - * Non-destructive tools never consult this module—they auto-execute always. + * Query tools never consult this module—they auto-execute always. * * @module */ @@ -15,7 +15,7 @@ import { SvelteMap } from 'svelte/reactivity'; import { type ToolTrust, workspaceClient } from '$lib/workspace'; /** - * Trust level for a destructive tool. + * Trust level for a mutation tool. * * - `'ask'` — show inline approval UI ([Allow] / [Always Allow] / [Deny]) * - `'always'` — auto-approve immediately, show subtle indicator @@ -51,7 +51,7 @@ function createToolTrustState() { * Get the trust level for a tool. * * Returns `'ask'` for tools not in the trust table (the safe default). - * Non-destructive tools should not call this—they auto-execute always. + * Query tools should not call this—they auto-execute always. * * @example * ```typescript @@ -103,6 +103,25 @@ function createToolTrustState() { shouldAutoApprove(name: string): boolean { return (trustMap.get(name) ?? 'ask') === 'always'; }, + + /** + * All trust entries as a reactive map. + * + * Returns the internal `SvelteMap` directly. Consumers get live + * updates when trust changes (local or remote via Y.Doc sync). + * Filter for `'always'` entries to show only explicitly trusted tools. + * + * @example + * ```typescript + * const trusted = $derived( + * [...toolTrustState.entries()] + * .filter(([, level]) => level === 'always'), + * ); + * ``` + */ + entries(): SvelteMap { + return trustMap; + }, }; } From bcc9aafa488560b1609e65d49113dbe9902570d4 Mon Sep 17 00:00:00 2001 From: Braden Wong <13159333+braden-w@users.noreply.github.com> Date: Sat, 14 Mar 2026 01:58:15 -0700 Subject: [PATCH 10/12] refactor(tab-manager): remove `destructive: true` from tabs.dedup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Missed in Phase 6 — added after the spec was written. Now removed to match the mutation-default approval model. --- apps/tab-manager/src/lib/workspace.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/tab-manager/src/lib/workspace.ts b/apps/tab-manager/src/lib/workspace.ts index 139894803d..c0b0172a04 100644 --- a/apps/tab-manager/src/lib/workspace.ts +++ b/apps/tab-manager/src/lib/workspace.ts @@ -884,7 +884,6 @@ export const workspaceClient = createWorkspace( title: 'Remove Duplicate Tabs', description: 'Close duplicate tabs, keeping the first occurrence of each URL. Only affects tabs on the current device.', - destructive: true, input: Type.Object({}), handler: async () => { const deviceId = await getDeviceId(); From 284e10585a8bef4a6f65200b8164b5a3866dae09 Mon Sep 17 00:00:00 2001 From: Braden Wong <13159333+braden-w@users.noreply.github.com> Date: Sat, 14 Mar 2026 02:02:11 -0700 Subject: [PATCH 11/12] feat(tab-manager): add trust revocation settings popover in AI drawer Users can now view and revoke "Always Allow" tool decisions from a gear icon in the AI chat drawer header. TrustSettings.svelte shows trusted tools with Switch toggles and a "Revoke All" option. Spec: specs/20260314T061500-tool-trust-revocation-settings.md --- .../src/lib/components/AiDrawer.svelte | 6 +- .../lib/components/chat/TrustSettings.svelte | 58 ++++ ...4T061500-tool-trust-revocation-settings.md | 274 ++++++++++++++++++ 3 files changed, 337 insertions(+), 1 deletion(-) create mode 100644 apps/tab-manager/src/lib/components/chat/TrustSettings.svelte create mode 100644 specs/20260314T061500-tool-trust-revocation-settings.md diff --git a/apps/tab-manager/src/lib/components/AiDrawer.svelte b/apps/tab-manager/src/lib/components/AiDrawer.svelte index ec52403c25..73ea0aa013 100644 --- a/apps/tab-manager/src/lib/components/AiDrawer.svelte +++ b/apps/tab-manager/src/lib/components/AiDrawer.svelte @@ -3,6 +3,7 @@ import * as Drawer from '@epicenter/ui/drawer'; import ZapIcon from '@lucide/svelte/icons/zap'; import AiChat from '$lib/components/chat/AiChat.svelte'; + import TrustSettings from '$lib/components/chat/TrustSettings.svelte'; import { authState } from '$lib/state/auth.svelte'; let { open = $bindable(false) }: { open: boolean } = $props(); @@ -11,7 +12,10 @@ - AI Chat +
+ AI Chat + +
Chat with AI about your tabs diff --git a/apps/tab-manager/src/lib/components/chat/TrustSettings.svelte b/apps/tab-manager/src/lib/components/chat/TrustSettings.svelte new file mode 100644 index 0000000000..79b70d3f12 --- /dev/null +++ b/apps/tab-manager/src/lib/components/chat/TrustSettings.svelte @@ -0,0 +1,58 @@ + + +{#if trustedTools.length > 0} + + + + + +
+

Tool Permissions

+
+ {#each trustedTools as [ name ] (name)} +
+ + {workspaceToolTitles[name] ?? + name + .replace(/_/g, ' ') + .replace(/^\w/, (c) => c.toUpperCase())} + + toolTrustState.set(name, 'ask')} + /> +
+ {/each} +
+ {#if trustedTools.length > 1} +
+ +
+ {/if} +
+ + +{/if} diff --git a/specs/20260314T061500-tool-trust-revocation-settings.md b/specs/20260314T061500-tool-trust-revocation-settings.md new file mode 100644 index 0000000000..6291ec5b92 --- /dev/null +++ b/specs/20260314T061500-tool-trust-revocation-settings.md @@ -0,0 +1,274 @@ +# Tool Trust Revocation Settings + +**Date**: 2026-03-14 +**Status**: Implemented +**Author**: AI-assisted + +## Overview + +Add a settings panel inside the AI chat drawer where users can view all tools they've marked as "Always Allow" and revoke that trust. Currently there is no way to undo an "Always Allow" decision without directly editing the workspace's Y.Doc. + +## Motivation + +### Current State + +When a destructive tool call arrives, `ToolCallPart.svelte` shows [Allow] / [Always Allow] / [Deny]. Clicking "Always Allow" writes to the workspace table and auto-approves all future calls for that tool: + +```typescript +// ToolCallPart.svelte — "Always Allow" handler +function handleAlwaysAllow() { + if (!approval?.id) return; + toolTrustState.set(part.name, 'always'); + aiChatState.active?.approveToolCall(approval.id, true); +} +``` + +The trust state module has `get()` and `set()` but no way to list or revoke: + +```typescript +// tool-trust.svelte.ts — current API surface +return { + get(name: string): TrustLevel { ... }, + set(name: string, level: TrustLevel): void { ... }, + shouldAutoApprove(name: string): boolean { ... }, +}; +``` + +This creates problems: + +1. **No revocation path**: Once a user clicks "Always Allow", the only way to undo it is to clear workspace data or manually edit the Y.Doc. There is no UI to list or toggle trust decisions. +2. **No visibility**: Users cannot see which tools they've trusted. The trust map is hidden inside a `SvelteMap` with no enumeration exposed to consumers. +3. **Accidental trust**: A misclick on "Always Allow" is permanent with no recourse. + +### Desired State + +A small settings section inside the AI chat drawer that lists all trusted tools with a Switch toggle to revoke each one. The trust state module exposes `entries()` for enumeration and `revoke()` for single-tool removal. + +## Research Findings + +### Existing UI Patterns in Tab Manager + +| Component | Pattern | Location | +|---|---|---| +| `SyncStatusIndicator` | Popover from header icon with auth form + status | `components/SyncStatusIndicator.svelte` | +| `AiDrawer` | Bottom Drawer with header, gated on auth | `components/AiDrawer.svelte` | +| `CommandPalette` | Command dialog overlay | `components/CommandPalette.svelte` | +| `ToolCallPart` | Inline approval buttons in chat stream | `components/chat/ToolCallPart.svelte` | + +**Key finding**: The app has no settings page. Configuration surfaces are embedded in context—sync settings live in the SyncStatusIndicator popover, AI chat lives in the AiDrawer. Trust settings should follow this pattern: embedded in the AI context, not a separate page. + +### Available shadcn-svelte Components + +Components already in `@epicenter/ui/` that are relevant: + +| Component | Import | Used In Tab Manager? | +|---|---|---| +| `Switch` | `@epicenter/ui/switch` | No (exists in `packages/ui/src/switch/`) | +| `Button` | `@epicenter/ui/button` | Yes (everywhere) | +| `Popover` | `@epicenter/ui/popover` | Yes (SyncStatusIndicator, comboboxes) | +| `Drawer` | `@epicenter/ui/drawer` | Yes (AiDrawer) | +| `Badge` | `@epicenter/ui/badge` | Yes (ToolCallPart) | +| `Collapsible` | `@epicenter/ui/collapsible` | Yes (CollapsibleSection) | +| `Tooltip` | `@epicenter/ui/tooltip` | Yes (App.svelte header) | + +**Key finding**: `Switch` component exists in the UI package but hasn't been used in tab-manager yet. It's the natural fit for a trust toggle. No need to add new shadcn components. + +### Trust State Surface Area + +The `toolTrustTable` workspace table stores trust rows: + +```typescript +// workspace.ts — table definition +const toolTrustTable = defineTable({ + id: Type.String(), // tool name (e.g. "tabs_close") + trust: Type.String(), // 'ask' | 'always' + _v: Type.Number(), +}); +``` + +Internal `readAllTrust()` already builds a `Map` from `workspaceClient.tables.toolTrust.getAllValid()`. It just isn't exposed. + +### Industry Patterns + +| Product | Trust Revocation UI | +|---|---| +| Claude Desktop (MCP) | Settings → "Integrations" lists all tools with permission toggles | +| ChatGPT | Settings → "Connected apps" with per-app revoke | +| Cursor | Settings panel → "Features" → yolo mode toggle (global, not per-tool) | + +**Key finding**: Every product that has per-tool trust puts revocation in a settings surface, not in the chat stream. A small list with toggles is the standard. + +## Design Decisions + +| Decision | Choice | Rationale | +|---|---|---| +| Settings location | Gear icon in AiDrawer header → Popover | Trust is AI-chat-specific. Follows SyncStatusIndicator pattern (icon → popover). Doesn't add a new top-level UI surface. | +| Toggle component | `Switch` from `@epicenter/ui/switch` | Standard on/off toggle. Already in the UI package. | +| Trust display | Tool title + Switch per row | Simple, scannable. Matches Claude Desktop pattern. | +| Revocation method | `set(name, 'ask')` (not delete) | Consistent with existing API. Row stays in table with `'ask'` value. Avoids needing a delete method on the table. | +| Empty state | Hide gear icon when no tools are trusted | No point showing settings when there's nothing to configure. | +| API additions | `entries()` on `toolTrustState` | Returns reactive `Map` for the UI to iterate. Minimal surface addition. | +| Tool name display | Reuse `workspaceToolTitles` lookup | Already maps tool names to human-readable titles (e.g. `tabs_close` → "Close Tabs"). | + +## Architecture + +``` +AiDrawer.svelte +┌──────────────────────────────────────────────────────┐ +│ Drawer.Header │ +│ ┌──────────────────────────┐ ┌──────────────────┐ │ +│ │ "AI Chat" │ │ ⚙ (gear icon) │ │ +│ └──────────────────────────┘ └────────┬─────────┘ │ +│ │ │ +│ Popover.Content │ +│ ┌─────────────────────┐ │ +│ │ Tool Permissions │ │ +│ │ │ │ +│ │ Close Tabs [====] │ │ +│ │ (future...) [====] │ │ +│ │ │ │ +│ │ ─────────────────── │ │ +│ │ Revoke All (link) │ │ +│ └─────────────────────┘ │ +│ │ +│ ┌──────────────────────────────────────────────────┐ │ +│ │ AiChat │ │ +│ │ (existing chat UI) │ │ +│ └──────────────────────────────────────────────────┘ │ +└──────────────────────────────────────────────────────┘ +``` + +### Trust Flow (Revocation) + +``` +User clicks gear icon in AiDrawer header + │ + ▼ +Popover opens, showing trusted tools +(entries from toolTrustState.entries() where trust === 'always') + │ + ▼ +User toggles Switch OFF for "Close Tabs" + │ + ▼ +toolTrustState.set('tabs_close', 'ask') + │ + ▼ +Y.Doc observer fires → SvelteMap updates → Switch reactively flips + │ + ▼ +Next "Close Tabs" tool call shows [Allow] / [Always Allow] / [Deny] again +``` + +## Implementation Plan + +### Phase 1: Extend Trust State API + +- [x] **1.1** Add `entries()` method to `toolTrustState` in `apps/tab-manager/src/lib/state/tool-trust.svelte.ts` — return the internal `trustMap` (already a `SvelteMap`, already reactive) +- [x] **1.2** Verify reactivity: when `set()` is called elsewhere (e.g. ToolCallPart "Always Allow"), the Popover's list should update live via the Y.Doc observer + +### Phase 2: Settings Popover Component + +- [x] **2.1** Create `apps/tab-manager/src/lib/components/chat/TrustSettings.svelte` — self-contained Popover component +- [x] **2.2** Import `Switch` from `@epicenter/ui/switch`, `Button` from `@epicenter/ui/button`, `Popover` from `@epicenter/ui/popover` +- [x] **2.3** Import `toolTrustState` and `workspaceToolTitles` for data and display names +- [x] **2.4** List only tools where trust is `'always'` (no point showing `'ask'` tools — they're the default) +- [x] **2.5** Each row: tool title (from `workspaceToolTitles`) + `Switch` bound to whether trust is `'always'` +- [x] **2.6** Toggling Switch OFF calls `toolTrustState.set(name, 'ask')` +- [x] **2.7** Empty state: hide gear icon when no tools are trusted (Option B from Open Questions) +- [x] **2.8** Optional: "Revoke All" text button at the bottom that sets all entries to `'ask'` (shown when 2+ tools trusted) + +### Phase 3: Wire Into AiDrawer + +- [x] **3.1** Add gear icon (`SettingsIcon` from `@lucide/svelte/icons/settings`) to `AiDrawer.svelte` header, next to the title +- [x] **3.2** Only show gear icon when there is at least one trusted tool (check `toolTrustState.entries()` size) +- [x] **3.3** Render `` inside the drawer (it manages its own Popover state) + +### Phase 4: Verification + +- [x] **4.1** `bun run typecheck` passes (0 errors in changed files; 89 pre-existing errors in UI package unrelated) +- [ ] **4.2** Manual test: trust a tool via "Always Allow" → gear icon appears → popover shows the tool → toggle off → next tool call asks for approval again +- [ ] **4.3** Manual test: "Revoke All" resets all trusted tools +- [ ] **4.4** Manual test: empty state shows when no tools are trusted + +## Edge Cases + +### Trust changed from another device + +1. User trusts "Close Tabs" on Device A +2. User revokes it in the settings popover on Device B +3. Y.Doc CRDT syncs the change → Device A's trust map updates via the existing `.observe()` callback +4. Next "Close Tabs" call on Device A shows approval UI again +5. **No special handling needed** — the existing observer pattern handles this + +### All tools revoked + +1. User clicks "Revoke All" +2. All entries set to `'ask'` +3. Gear icon hides (no trusted tools) +4. Popover closes (or stays open showing empty state — implementer decides) + +### Tool renamed or removed + +1. Developer renames a workspace action (e.g. `tabs_close` → `tabs_remove`) +2. Old trust entry for `tabs_close` still in the table +3. It won't match any tool, so it's harmless — shows in settings as an unknown tool +4. **Recommendation**: Show the raw tool name as fallback when `workspaceToolTitles[name]` is undefined. No cleanup logic needed. + +## Open Questions + +1. **Should the gear icon always be visible, or only when tools are trusted?** + - Option A: Always visible (settings are discoverable even before trusting anything) + - Option B: Only visible when ≥1 tool is trusted (cleaner header, less visual noise) + - **Recommendation**: Option B. The header is already compact. Trust settings aren't useful until you've actually trusted something. But the implementer may discover a better placement during implementation. + +2. **Should "Revoke All" require confirmation?** + - It's a bulk action but low-stakes (just resets to the default state) + - **Recommendation**: No confirmation. Revoking trust isn't destructive — it just means the next tool call will ask again. + +3. **Should the popover be a separate component or part of a larger settings surface?** + - Currently no settings page exists. This would be the first settings UI. + - **Recommendation**: Keep it as a focused popover for now. If more AI settings emerge later (model preferences, conversation defaults), consider extracting to a full settings panel at that point. + +## Success Criteria + +- [x] `toolTrustState` exposes `entries()` returning a reactive iterable of trusted tools +- [x] `TrustSettings.svelte` renders a list of trusted tools with Switch toggles +- [x] Toggling a Switch revokes trust and the tool asks for approval on next use +- [x] Gear icon in AiDrawer header opens the settings popover +- [x] Empty state handled (no trusted tools → gear icon hides) +- [x] `bun run typecheck` passes (changed files clean) +- [x] No `as any` or `@ts-ignore` introduced + +## References + +- `apps/tab-manager/src/lib/state/tool-trust.svelte.ts` — Trust state module to extend with `entries()` +- `apps/tab-manager/src/lib/components/chat/ToolCallPart.svelte` — Existing "Always Allow" handler (consumer of trust state) +- `apps/tab-manager/src/lib/components/AiDrawer.svelte` — Where the gear icon will be added +- `apps/tab-manager/src/lib/components/SyncStatusIndicator.svelte` — Reference pattern for Popover-from-icon-button +- `apps/tab-manager/src/lib/workspace.ts` — `workspaceToolTitles` lookup map, `toolTrustTable` definition +- `packages/ui/src/switch/` — Switch component (exists, not yet used in tab-manager) +- `packages/ui/src/popover/` — Popover component (already used in tab-manager) +- `specs/20260312T170000-progressive-tool-trust.md` — Parent spec that introduced the trust system + +## Review + +**Completed**: 2026-03-14 + +### Summary + +Added a settings popover in the AI chat drawer for viewing and revoking tool trust decisions. Three files changed: + +1. **tool-trust.svelte.ts** (+19 lines): Added `entries()` method returning the internal `SvelteMap` for reactive enumeration. +2. **TrustSettings.svelte** (59 lines, new): Self-contained Popover component with gear icon trigger, Switch toggles per trusted tool, and "Revoke All" button (shown when 2+ tools trusted). Hides entirely when no tools are trusted. +3. **AiDrawer.svelte** (+4 lines): Imported TrustSettings, added flex wrapper in header to position gear icon next to title. + +### Deviations from Spec + +- **2.7 Empty state**: Chose Option B (hide gear icon) rather than showing an in-popover "No tools trusted" message. When the last tool is revoked, the component unmounts cleanly. +- **2.8 Revoke All**: Made visible only when 2+ tools are trusted (not always). With a single tool, the user can just toggle the Switch directly. +- **Button import omitted**: TrustSettings uses `buttonVariants` for the trigger but doesn't need the `Button` component itself. Kept imports minimal. + +### Follow-up Work + +- Manual testing needed (spec items 4.2–4.4) to verify end-to-end flow in the extension. From 5db2179c1b4c75a4607a6084574689da5c6e5142 Mon Sep 17 00:00:00 2001 From: Braden Wong <13159333+braden-w@users.noreply.github.com> Date: Sat, 14 Mar 2026 02:02:45 -0700 Subject: [PATCH 12/12] docs: mark mutation-default-approval spec as complete Check off all implementation items and success criteria, add review section noting the extra tabs.dedup call site discovered during implementation. --- ...260314T080000-mutation-default-approval.md | 359 ++++++++++++++++++ 1 file changed, 359 insertions(+) create mode 100644 specs/20260314T080000-mutation-default-approval.md diff --git a/specs/20260314T080000-mutation-default-approval.md b/specs/20260314T080000-mutation-default-approval.md new file mode 100644 index 0000000000..58690ccc2a --- /dev/null +++ b/specs/20260314T080000-mutation-default-approval.md @@ -0,0 +1,359 @@ +# Mutation-Default Approval + +**Date**: 2026-03-14 +**Status**: Complete +**Author**: AI-assisted + +--- + +## Overview + +Remove the per-action `destructive` flag and make all mutations require approval by default. Queries never need approval. The existing `toolTrust` table with [Always Allow] already handles approval fatigue. + +--- + +## Motivation + +### Current State + +Three commits on March 12, 2026 built the approval system in stages. + +`4b76002ae` added `title` and `destructive` to `ActionConfig`. The tool bridge had two approval mechanisms: per-action `destructive: true → needsApproval: true`, and a blanket `requireApprovalForMutations` option. + +`01d13cf30` added the `toolTrust` table—a Y.Doc-backed store for per-tool trust preferences (`'ask'` or `'always'`), synced across devices via CRDT. + +`5d5127e73` removed `requireApprovalForMutations` entirely. After that refactor, `needsApproval` is only set when `destructive: true`. + +Today, `ActionConfig` and `ActionMeta` carry this field: + +```typescript +type ActionConfig = { + title?: string; + description?: string; + /** Whether this action is destructive. Maps to `needsApproval` in the tool bridge. */ + destructive?: boolean; + input?: TInput; + handler: ActionHandler; +}; +``` + +And the tool bridge maps it like this: + +```typescript +// packages/ai/src/tool-bridge.ts, line 143 +...(action.destructive && { needsApproval: true }), +``` + +`ActionDescriptor` in `describe-workspace.ts` also carries the field and forwards it: + +```typescript +export type ActionDescriptor = { + path: string[]; + type: 'query' | 'mutation'; + title?: string; + description?: string; + destructive?: boolean; // ← forwarded from ActionMeta + input?: TSchema; +}; +``` + +### Problems + +The `destructive` flag has one dangerous failure mode: the developer must remember to set it. Forget it, and a mutation auto-executes with zero friction. + +Out of 13 tab-manager actions (5 queries, 8 mutations), only `tabs.close` has `destructive: true`. The other 7 mutations—`tabs.open`, `tabs.activate`, `tabs.pin`, `tabs.unpin`, `tabs.move`, `tabs.group`, `tabs.save`—auto-execute with no approval dialog. That's not a deliberate choice; it's an omission. + +The flag also creates a conceptual mismatch. "Destructive" is a judgment call. `tabs.save` with `close: true` is destructive. `tabs.open` with 50 URLs is destructive. The developer has to anticipate every dangerous combination at definition time, which is impossible. + +### Desired State + +Mutations need approval by default. Queries never do. The tool bridge becomes: + +```typescript +...(action.type === 'mutation' && { needsApproval: true }), +``` + +`destructive` disappears from `ActionConfig`, `ActionMeta`, `ActionDescriptor`, and every call site. The `toolTrust` table already handles the approval fatigue problem: users click [Always Allow] once per mutation and never see the dialog again. That preference syncs across devices via CRDT. + +--- + +## Research Findings + +### How Other Systems Handle Default Trust + +| System | Default for writes | Opt-out mechanism | +|---|---|---| +| macOS Gatekeeper | Prompt on first run | User grants permanent permission | +| iOS permissions | Deny until granted | Per-app, per-capability grant | +| Chrome extension APIs | Prompt per dangerous API | Manifest declares permissions | +| sudo | Prompt every time (or cached) | `NOPASSWD` in sudoers | +| This codebase (before) | Auto-execute unless `destructive: true` | No opt-out for safe mutations | +| This codebase (after) | Prompt for all mutations | [Always Allow] per tool | + +The pattern across all of these: safe default is "ask", not "allow". The escape hatch is a persistent grant, not a per-action flag. + +### The `toolTrust` Table + +The `toolTrust` table was built specifically to solve approval fatigue. It stores `{ id: toolName, trust: 'ask' | 'always' }` in a Y.Doc, synced across devices. When a user clicks [Always Allow], the tool moves from `'ask'` to `'always'` and never prompts again. + +This means the approval-fatigue argument against mutation-default approval is already solved. The infrastructure exists. The only missing piece is flipping the default. + +### Current Tab-Manager Mutation Coverage + +| Action | Has `destructive: true`? | Should need approval? | +|---|---|---| +| `tabs.close` | Yes | Yes | +| `tabs.open` | No | Yes | +| `tabs.activate` | No | Yes | +| `tabs.pin` | No | Yes | +| `tabs.unpin` | No | Yes | +| `tabs.move` | No | Yes | +| `tabs.group` | No | Yes | +| `tabs.save` | No | Yes | + +Seven of eight mutations currently auto-execute. After this change, all eight require approval on first use. + +--- + +## Design Decisions + +| Decision | Choice | Rationale | +|---|---|---| +| Default for mutations | `needsApproval: true` always | Safe default; developer omission can't silently bypass approval | +| Default for queries | `needsApproval` omitted | Queries are declared read-only; no state change, no approval needed | +| Remove `destructive` flag | Yes, entirely | Redundant once mutations default to approval; removing it prevents confusion | +| Migration for existing `toolTrust` entries | None needed | Existing `'always'` entries continue to work; the table is keyed by tool name | +| New types or options | None | Pure simplification; no new concepts introduced | +| `tabs.close` `destructive: true` | Remove it | The flag is gone; `tabs.close` gets approval because it's a mutation | + +--- + +## Architecture + +### Before: Approval Gated on `destructive` Flag + +``` +ActionConfig + ├── title?: string + ├── description?: string + ├── destructive?: boolean ← developer must remember to set this + ├── input?: TSchema + └── handler: ActionHandler + +actionsToClientTools() + └── for each action: + if action.destructive → needsApproval: true + else → (omitted, auto-executes) + +Result: + tabs.close → needsApproval: true ✓ + tabs.open → (omitted) ✗ auto-executes + tabs.pin → (omitted) ✗ auto-executes + tabs.save → (omitted) ✗ auto-executes + ... (5 more mutations auto-execute) +``` + +### After: Approval Gated on Action Type + +``` +ActionConfig + ├── title?: string + ├── description?: string + ├── input?: TSchema + └── handler: ActionHandler + +actionsToClientTools() + └── for each action: + if action.type === 'mutation' → needsApproval: true + if action.type === 'query' → (omitted, auto-executes) + +Result: + tabs.close → needsApproval: true ✓ + tabs.open → needsApproval: true ✓ + tabs.pin → needsApproval: true ✓ + tabs.save → needsApproval: true ✓ + tabs.search → (omitted) ✓ query, read-only + tabs.getAll → (omitted) ✓ query, read-only +``` + +### Trust Escalation Flow (unchanged) + +``` +User sends message → AI calls mutation tool + │ + ▼ + toolTrust.get(toolName) === 'always'? + │ │ + Yes No + │ │ + ▼ ▼ + auto-approve show approval UI + [Allow] [Always Allow] [Deny] + │ + [Always Allow] clicked + │ + ▼ + toolTrust.set(toolName, 'always') + (syncs to all devices via CRDT) + │ + ▼ + future calls auto-approve +``` + +--- + +## Implementation Plan + +### Phase 1: Core types — remove `destructive` from `ActionConfig` and `ActionMeta` + +- [x] **1.1** In `packages/workspace/src/shared/actions.ts`, remove `destructive?: boolean` from `ActionConfig` (line ~133) and its JSDoc comment (line ~132) +- [x] **1.2** In the same file, remove `destructive?: boolean` from `ActionMeta` (line ~151) and its JSDoc comment (line ~150) + +### Phase 2: Tool bridge — flip the approval condition + +- [x] **2.1** In `packages/ai/src/tool-bridge.ts` line 143, change: + ```typescript + ...(action.destructive && { needsApproval: true }), + ``` + to: + ```typescript + ...(action.type === 'mutation' && { needsApproval: true }), + ``` +- [x] **2.2** Update the JSDoc for `needsApproval` in `ToolDefinitionPayload` (lines 84–88). Change "Only present when the action is marked `destructive`" to "Present on all mutations. Queries never need approval." +- [x] **2.3** Update the JSDoc for `actionsToClientTools` to reflect the new behavior + +### Phase 3: Tool bridge tests — update for mutation-default semantics + +- [x] **3.1** Rename test `'destructive action sets needsApproval without blanket option'` to `'all mutations get needsApproval'` +- [x] **3.2** Update that test: the `open` mutation (currently expected to have `needsApproval` undefined) should now have `needsApproval: true`. Remove `destructive: true` from the `close` action definition. +- [x] **3.3** Rename test `'non-destructive tools omit needsApproval entirely'` to `'queries omit needsApproval entirely'` +- [x] **3.4** Update that test: the `mutation` action should now have `needsApproval: true`, not undefined. Only the `query` action should omit it. +- [x] **3.5** Rename test `'only forwards needsApproval for destructive tools'` to `'forwards needsApproval for all mutations, not queries'` +- [x] **3.6** Update that test: the `destructive` mutation (renamed to something like `save`) should have `needsApproval: true` without `destructive: true` in its definition. The `safe` query should still omit it. Remove all `destructive: true` from test action definitions. + +### Phase 4: `ActionDescriptor` — remove `destructive` from workspace descriptor + +- [x] **4.1** In `packages/workspace/src/workspace/describe-workspace.ts`, remove `destructive?: boolean` from `ActionDescriptor` (line ~48) +- [x] **4.2** In the same file, remove the conditional spread `...(action.destructive !== undefined && { destructive: action.destructive })` from `describeWorkspace` (lines ~128–130) + +### Phase 5: Descriptor tests — update for removed field + +- [x] **5.1** In `packages/workspace/src/workspace/describe-workspace.test.ts`, rename test `'title and destructive appear in action descriptors'` to `'title appears in action descriptors'` +- [x] **5.2** Remove `destructive: true` from the `delete` mutation definition in that test (line ~236) +- [x] **5.3** Remove the assertion `expect(deleteAction?.destructive).toBe(true)` (line ~261) +- [x] **5.4** Remove the assertion `expect(getAllAction?.destructive).toBeUndefined()` (line ~255) +- [x] **5.5** Remove the assertion `expect(createAction?.destructive).toBeUndefined()` (line ~267) + +### Phase 6: Tab-manager workspace — remove the last `destructive: true` call site + +- [x] **6.1** In `apps/tab-manager/src/lib/workspace.ts` line 674, remove `destructive: true,` from the `tabs.close` action definition +- [x] **6.2** *(discovered during implementation)* Remove `destructive: true,` from `tabs.dedup` action definition (line ~887), added in `6096e7d` after the spec was written + +### Phase 7: JSDoc updates — replace "destructive" language with "mutation" + +- [x] **7.1** In `apps/tab-manager/src/lib/state/tool-trust.svelte.ts` line 4, change "Destructive AI tools start as 'ask'" to "Mutation tools start as 'ask'" +- [x] **7.2** In the same file line 9, change "Non-destructive tools never consult this module" to "Query tools never consult this module" +- [x] **7.3** In the same file line 54, change "Non-destructive tools should not call this" to "Query tools should not call this" +- [x] **7.4** In the same file line 18, change "Trust level for a destructive tool" to "Trust level for a mutation tool" +- [x] **7.5** In `apps/tab-manager/src/lib/ai/system-prompt.ts` line 28, change "Destructive actions (like closing tabs) have their own approval UI — do not ask for confirmation in prose" to "Mutations (actions that change state) have their own approval UI — do not ask for confirmation in prose" +- [x] **7.6** In `apps/tab-manager/src/lib/state/chat-state.svelte.ts` line 462, change "on a destructive tool call in the chat" to "on a mutation tool call in the chat" + +### Phase 8: Verification + +- [x] **8.1** Run `bun test` in `packages/ai` — all tests pass (4/4) +- [x] **8.2** Run `bun test` in `packages/workspace` — all tests pass (347/347) +- [x] **8.3** Run `bun run typecheck` from the repo root — no new errors (pre-existing `NumberKeysOf`/`Uint8Array` issues unrelated) +- [x] **8.4** Search the codebase for `destructive:` (with colon) to confirm no remaining usage in action definitions. 4 matches are all `variant="destructive"` in shadcn-svelte UI components (unrelated Tailwind styling). + +--- + +## Edge Cases + +### `tabs.save` with `close: true` + +`tabs.save` accepts a `close` argument. When `close: true`, it closes the tab after saving—making it conditionally destructive. This was one argument for per-action `destructive` flags: you could mark only the dangerous variant. + +With mutation-default approval, `tabs.save` always requires approval regardless of arguments. This is the correct behavior. The approval UI shows the full argument object, including `close: true`, so the user sees exactly what will happen before approving. The user can deny if they only wanted to save without closing. + +### New mutations added by developers + +Any new `defineMutation` call automatically gets `needsApproval: true` in the tool bridge. The developer doesn't have to remember anything. The old system required an explicit `destructive: true`; the new system is safe by default. + +### Queries with side effects + +If a developer puts side effects inside a `defineQuery` handler, those effects won't trigger approval. This is intentional. Queries are declared read-only by contract. If a query has side effects, that's a bug in the action definition, not a gap in the approval system. + +### Existing `toolTrust` entries + +Users who already clicked [Always Allow] on `tabs_close` have a `{ id: 'tabs_close', trust: 'always' }` row in their `toolTrust` table. That row continues to work after this change. No migration needed. The table is keyed by tool name, and tool names don't change. + +### First-time approval fatigue + +On first use after this change, users will see approval dialogs for mutations they previously didn't see them for: `tabs.open`, `tabs.activate`, `tabs.pin`, etc. Each one requires one click of [Always Allow] to dismiss permanently. That preference syncs to all their devices via CRDT. The fatigue is bounded and one-time. + +--- + +## Open Questions + +1. **Should `tabs.search` and other read-heavy queries ever need approval?** + - These are pure reads with no state change. The current design never prompts for queries. + - Recommendation: No. Queries are declared read-only. If a query is doing something surprising, fix the query. + +2. **Should the `toolTrust` table be pre-populated with `'always'` for obviously safe mutations?** + - Options: (a) ship with empty table, users grant trust on first use; (b) pre-populate `tabs.open`, `tabs.activate`, `tabs.pin` as `'always'` in the workspace seed data. + - Recommendation: Ship with empty table. Pre-populating trust defeats the purpose of the approval system. Users should consciously grant trust, even for safe-seeming mutations. The one-time cost is low. + +3. **Does `ActionDescriptor` need a replacement field to communicate mutation semantics to MCP consumers?** + - `ActionDescriptor` already has `type: 'query' | 'mutation'`. MCP consumers can infer approval semantics from `type`. + - Recommendation: No new field needed. `type` is sufficient. + +--- + +## Success Criteria + +- [x] `ActionConfig` has no `destructive` field +- [x] `ActionMeta` has no `destructive` field +- [x] `ActionDescriptor` has no `destructive` field +- [x] `actionsToClientTools` sets `needsApproval: true` for every mutation +- [x] `actionsToClientTools` omits `needsApproval` for every query +- [x] No `destructive:` property in any action definition across the codebase (grep confirms) +- [x] `bun test` passes in `packages/ai` +- [x] `bun test` passes in `packages/workspace` +- [x] `bun run typecheck` passes with no new errors +- [x] No `as any` or `@ts-ignore` introduced + +--- + +## References + +- `packages/workspace/src/shared/actions.ts` — `ActionConfig`, `ActionMeta` types with `destructive` field +- `packages/ai/src/tool-bridge.ts` — `actionsToClientTools`, `toToolDefinitions`, `needsApproval` logic +- `packages/ai/src/tool-bridge.test.ts` — tool bridge tests with `destructive`-based assertions +- `packages/workspace/src/workspace/describe-workspace.ts` — `ActionDescriptor`, `describeWorkspace` with `destructive` spread +- `packages/workspace/src/workspace/describe-workspace.test.ts` — descriptor tests asserting `destructive` field +- `apps/tab-manager/src/lib/workspace.ts` — only remaining `destructive: true` call site (`tabs.close`) +- `apps/tab-manager/src/lib/state/tool-trust.svelte.ts` — trust state module with "destructive" language in JSDoc +- `apps/tab-manager/src/lib/ai/system-prompt.ts` — system prompt mentioning "destructive actions" +- `apps/tab-manager/src/lib/state/chat-state.svelte.ts` — chat state JSDoc mentioning "destructive tool call" +- `specs/20260312T153000-action-metadata-title-destructive.md` — original spec for the `destructive` flag +- `specs/20260312T170000-progressive-tool-trust.md` — progressive trust spec documenting the `toolTrust` table + +--- + +## Review + +### Summary + +Pure simplification: removed the per-action `destructive` flag and made all mutations require approval by default. 8 commits across 9 files, ~30 lines changed (mostly deletions). + +### Deviation from spec + +The spec listed `tabs.close` as the only `destructive: true` call site. During implementation, a second call site was discovered on `tabs.dedup` (added in commit `6096e7d` after the spec was written). Both were removed. + +### Verification results + +- `bun test` in `packages/ai`: 4/4 pass +- `bun test` in `packages/workspace`: 347/347 pass +- `bun run typecheck`: no new errors (pre-existing `NumberKeysOf`/`Uint8Array` issues unrelated) +- `destructive:` grep: zero action-definition matches; 4 hits are `variant="destructive"` in shadcn-svelte UI components (unrelated Tailwind styling) +- No `as any` or `@ts-ignore` introduced