diff --git a/README.md b/README.md index 010696944..ebd74bb9c 100644 --- a/README.md +++ b/README.md @@ -341,6 +341,7 @@ If wigolo earns a place in your setup, three things keep it going: a ⭐ **star* - **Browser won't launch on Linux** — `wigolo warmup --browser` installs the OS libraries (or prints the exact command). - **Native build error / unusual Node** — use an LTS: **Node 20, 22, or 24**. - **Behind a proxy** — `USE_PROXY=true` + `PROXY_URL`; add `NODE_EXTRA_CA_CERTS` for TLS-inspecting proxies. +- **Your agent asks permission on every call** — allow the tools in your client, then restart it; rules are read at session start. [Details](docs/troubleshooting.md#your-agent-keeps-asking-permission). The full guide covers per-symptom fixes, a "what still works when X fails" map, platform notes (incl. linux-arm64), and offline installs: **[docs/troubleshooting.md](docs/troubleshooting.md)**. diff --git a/docs/installation.md b/docs/installation.md index 05e325420..63b40bb07 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -79,6 +79,8 @@ wigolo carries registry manifests at the repo root — `smithery.yaml`, `glama.j npx wigolo init --agents=claude-code,cursor ``` +If your agent prompts for permission on every wigolo tool call after wiring, see [troubleshooting](./troubleshooting.md#your-agent-keeps-asking-permission). + For OpenCode, wigolo writes the global `~/.config/opencode/opencode.json` entry in OpenCode's local MCP format: ```json diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index d4cc47a09..c8688549d 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -16,6 +16,7 @@ wigolo doctor --fix # repairs the known failure classes automatically | `wigolo serve` exits: port in use | The daemon deliberately does not auto-rebind. The error names a free port to retry with, e.g. `wigolo serve --port 3334`. | | `wigolo serve` refuses to start on a non-loopback host | Working as designed (fail-closed). Set `WIGOLO_API_TOKEN` / `WIGOLO_API_TOKEN_FILE`, or explicitly pass `--allow-unauthenticated`. See [self-hosting](./self-hosting.md#binding-beyond-loopback). | | Fetch result says `blocked_by_challenge` | See [below](#blocked_by_challenge). | +| Your agent asks permission before every wigolo tool call | See [below](#your-agent-keeps-asking-permission). | | Search results feel thin / an engine seems dead | Degraded engines are *reported*, not hidden — check `engine_warnings`, `engine_telemetry`, and `engine_pool` in the response, and `wigolo doctor`'s per-engine table (it names the env var when an engine just wants a key, e.g. `WIGOLO_GITHUB_TOKEN`, `BRAVE_API_KEY`). | | Results are stale | Pass `force_refresh: true` (news, prices, changelogs), or clear scoped entries: `wigolo cache clear --url-pattern="*example.com*"`. Lifetimes are tunable: `CACHE_TTL_SEARCH`, `CACHE_TTL_CONTENT`. | | Everything fails behind a corporate proxy | Set `USE_PROXY=true` and `PROXY_URL` (credentials go to the OS keychain, not disk). See [configuration](./configuration.md#fetch-and-browser-engine). | @@ -50,6 +51,53 @@ Two honest facts to calibrate expectations: - **IP reputation is scored.** From datacenter IPs (VPS, CI, cloud), some challenge-protected sites will not clear even though the identical request works from a residential connection. That's a property of where you're running, not a knob wigolo forgot. - **The opt-in lever is a proxy** whose IP reputation matches your legitimate-research use — see [self-hosting](./self-hosting.md#the-datacenter-ip-reality). Credentials are keychain-stored, and politeness (robots.txt, per-domain rate limits) still applies. +## Your agent keeps asking permission + +Every wigolo tool reports MCP capability hints (`readOnlyHint`, `destructiveHint`, +`idempotentHint`, `openWorldHint`) in its `tools/list` entry, and most clients use those to +auto-approve the read-only ones. Seven of the ten are read-only. Three are not, and clients are +told so deliberately — prompting on these is correct, not a bug: + +| Tool | Why it is not read-only | +| --- | --- | +| `fetch` | `actions` runs live `click` / `type` on the page, so it can submit forms and trigger navigation | +| `cache` | `clear` deletes cached rows | +| `watch` | `create` / `delete` mutate the persistent job store | + +`fetch` is the surprising one, and it is the tool you call most. Its default path only reads, but +a capability hint describes what a tool *can* do, not what a given call does, and the hints are +static per tool — so it has to declare the widest behaviour. If you never pass `actions` and want +`fetch` auto-approved anyway, allow it explicitly with the rule below. + +Clients that ignore the hints need an explicit allow rule. + +**Claude Code in plan mode** (observed on 2.1.220). Plan mode refuses any MCP tool that is not +annotated read-only, and it decides that *before* it looks at your allow rules — so an allow +rule cannot lift it. +Before wigolo shipped these hints, every tool was treated as non-read-only and prompted on every +call in plan mode no matter what was in `settings.json`. If you are on an older wigolo, upgrade. +The three non-read-only tools above still prompt in plan mode, correctly: they change state. + +**Claude Code, normal modes.** Add to `~/.claude/settings.json`: + +```json +{ + "permissions": { + "allow": ["mcp__wigolo__*"] + } +} +``` + +Then **restart Claude Code**. This is the step people miss: permission rules are read once at +session start, so a session that was already open when you edited the file keeps prompting until +you restart it, and it looks like the rule did not work. + +The `mcp__wigolo__` prefix must be literal — the server segment cannot contain a glob, so +`mcp__*` is skipped with a warning and approves nothing. + +If the server name is not `wigolo` in your config, use whatever name you registered it under — +the rule matches the configured server name, not the package name. `claude mcp list` shows it. + ## Platform notes **Node version.** wigolo runs on **Node 20, 22, or 24** (LTS). Very new or unusual Node builds may not have prebuilt native binaries yet and will try to compile from source (which needs a C/C++ toolchain) — stick to an LTS to avoid that. diff --git a/src/server.ts b/src/server.ts index 92a61bc9c..1e91fdcf1 100644 --- a/src/server.ts +++ b/src/server.ts @@ -317,57 +317,148 @@ export function createMcpServer(subsystems: Subsystems): Server { }; }); + // Capability hints per MCP `tools/list`. Hosts use these to decide whether a + // call needs a permission prompt, so each one describes the WIDEST behaviour + // its tool can reach, not the common case. + // + // Read-only tools still populate the local content cache. That store is an + // implementation detail rather than caller-visible state, which is why they + // stay `readOnlyHint: true` while `cache` — the tool that exposes the store + // directly — does not. + // + // `idempotentHint` is inert wherever `readOnlyHint` is true (spec: "meaningful + // only when readOnlyHint == false"), so those tools carry `true` for + // consistency rather than as a claim about output stability. server.setRequestHandler(ListToolsRequestSchema, async () => ({ tools: [ { name: 'fetch', description: TOOL_DESCRIPTIONS.fetch, inputSchema: FETCH_TOOL_SCHEMA, + // Not read-only: `actions` accepts `click` and `type`, which run as live + // Playwright interactions on the target page (tool-schemas.ts), so a + // caller can submit a form or trigger navigation. The hints cover the + // widest reachable behaviour, and a click on an arbitrary page can + // destroy remote state, so `destructiveHint` is true even though the + // no-actions path — the common one — only reads. + annotations: { + title: 'Fetch a page', + readOnlyHint: false, + destructiveHint: true, + idempotentHint: false, + openWorldHint: true, + }, }, { name: 'search', description: TOOL_DESCRIPTIONS.search, inputSchema: SEARCH_TOOL_SCHEMA, + annotations: { + title: 'Web search', + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, + }, }, { name: 'crawl', description: TOOL_DESCRIPTIONS.crawl, inputSchema: CRAWL_TOOL_SCHEMA, + annotations: { + title: 'Crawl a site', + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, + }, }, { name: 'cache', description: TOOL_DESCRIPTIONS.cache, inputSchema: CACHE_TOOL_SCHEMA, + // `clear` deletes rows; `check_changes` re-fetches every matching URL + // over the network, so this is not a closed-world tool either. + annotations: { + title: 'Search or clear the local cache', + readOnlyHint: false, + destructiveHint: true, + idempotentHint: false, + openWorldHint: true, + }, }, { name: 'extract', description: TOOL_DESCRIPTIONS.extract, inputSchema: EXTRACT_TOOL_SCHEMA, + annotations: { + title: 'Extract structured data', + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, + }, }, { name: 'find_similar', description: TOOL_DESCRIPTIONS.find_similar, inputSchema: FIND_SIMILAR_TOOL_SCHEMA, + annotations: { + title: 'Find similar pages', + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, + }, }, { name: 'research', description: TOOL_DESCRIPTIONS.research, inputSchema: RESEARCH_TOOL_SCHEMA, + annotations: { + title: 'Deep research', + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, + }, }, { name: 'agent', description: TOOL_DESCRIPTIONS.agent, inputSchema: AGENT_TOOL_SCHEMA, + annotations: { + title: 'Autonomous data gathering', + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, + }, }, { name: 'diff', description: TOOL_DESCRIPTIONS.diff, inputSchema: DIFF_TOOL_SCHEMA, + annotations: { + title: 'Diff two versions', + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: false, + }, }, { name: 'watch', description: TOOL_DESCRIPTIONS.watch, inputSchema: WATCH_TOOL_SCHEMA, + // `create`/`delete`/`pause`/`resume` mutate the persistent job store. + annotations: { + title: 'Watch a URL for changes', + readOnlyHint: false, + destructiveHint: true, + idempotentHint: false, + openWorldHint: true, + }, }, ], })); diff --git a/tests/unit/server/tool-annotations.test.ts b/tests/unit/server/tool-annotations.test.ts new file mode 100644 index 000000000..d33cea374 --- /dev/null +++ b/tests/unit/server/tool-annotations.test.ts @@ -0,0 +1,226 @@ +/** + * MCP capability-hint coverage for `tools/list`. + * + * Hosts decide whether a tool call needs a permission prompt from these + * annotations, so a tool that ships without them prompts on every call. The + * exact matrix is pinned here: adding an eleventh tool without annotating it + * fails this suite rather than silently regressing the permission story in + * every host wigolo installs into. + * + * The two non-read-only rows are the point of the test. `cache` accepts + * `clear` and `watch` accepts `create`/`delete`, so claiming either is + * read-only would be a lie a host would act on. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; +import { resetConfig } from '../../../src/config.js'; +import { _resetMigrationGuard } from '../../../src/cache/migrations/runner.js'; + +vi.mock('../../../src/cache/db.js', async () => { + const actual = await vi.importActual( + '../../../src/cache/db.js', + ); + return { + ...actual, + initDatabase: (_path?: string) => actual.initDatabase(':memory:'), + }; +}); + +vi.mock('../../../src/fetch/browser-pool.js', () => { + class MockMultiBrowserPool { + shutdown = vi.fn().mockResolvedValue(undefined); + fetchWithBrowser = vi.fn(); + getConfiguredTypes = vi.fn().mockReturnValue(['chromium']); + getStats = vi.fn().mockReturnValue([]); + } + return { + MultiBrowserPool: MockMultiBrowserPool, + BrowserPool: class MockBrowserPool extends MockMultiBrowserPool { + acquire = vi.fn(); + release = vi.fn(); + }, + }; +}); + +vi.mock('../../../src/fetch/http-client.js', () => ({ + httpFetch: vi.fn(), +})); + +vi.mock('../../../src/fetch/router.js', () => ({ + SmartRouter: class MockSmartRouter { + constructor(_httpClient: unknown, _browserPool: unknown) {} + fetch = vi.fn(); + getDomainStats = vi.fn(); + }, +})); + +vi.mock('../../../src/searxng/bootstrap.js', () => ({ + resolveSearchBackend: vi.fn().mockResolvedValue({ type: 'scraping' }), + bootstrapNativeSearxng: vi.fn(), + getBootstrapState: vi.fn().mockReturnValue(null), +})); + +vi.mock('../../../src/searxng/process.js', () => ({ + SearxngProcess: vi.fn().mockImplementation(() => ({ + start: vi.fn().mockResolvedValue(null), + stop: vi.fn().mockResolvedValue(undefined), + getUrl: vi.fn().mockReturnValue(null), + })), +})); + +vi.mock('../../../src/searxng/docker.js', () => ({ + DockerSearxng: vi.fn().mockImplementation(() => ({ + start: vi.fn().mockResolvedValue(null), + stop: vi.fn().mockResolvedValue(undefined), + })), +})); + +vi.mock('../../../src/embedding/embed.js', () => ({ + getEmbeddingService: () => ({ + init: vi.fn().mockResolvedValue(undefined), + isAvailable: () => false, + shutdown: vi.fn(), + }), + resetEmbeddingService: vi.fn(), +})); + +const HINT_KEYS = ['readOnlyHint', 'destructiveHint', 'idempotentHint', 'openWorldHint'] as const; + +type HintMatrix = Record<(typeof HINT_KEYS)[number], boolean>; + +// Three tools are not read-only, and none of them look it from the name alone: +// `fetch` — `actions` runs live click/type via Playwright +// `cache` — `clear` deletes rows +// `watch` — create/delete mutate the job store +// `diff` is the only closed-world tool: it resolves its `url` sides from the +// local cache and returns `cache_miss` rather than fetching (src/tools/diff.ts). +// `cache` looks local but `check_changes` re-fetches over the network +// (src/tools/cache.ts). +const EXPECTED: Record = { + fetch: { readOnlyHint: false, destructiveHint: true, idempotentHint: false, openWorldHint: true }, + search: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: true }, + crawl: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: true }, + cache: { readOnlyHint: false, destructiveHint: true, idempotentHint: false, openWorldHint: true }, + extract: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: true }, + find_similar: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: true }, + research: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: true }, + agent: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: true }, + diff: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false }, + watch: { readOnlyHint: false, destructiveHint: true, idempotentHint: false, openWorldHint: true }, +}; + +async function connectClient() { + const { initSubsystems, createMcpServer } = await import('../../../src/server.js'); + const subs = await initSubsystems(); + const server = createMcpServer(subs); + + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: 'test-client', version: '1.0' }); + await Promise.all([ + server.connect(serverTransport), + client.connect(clientTransport), + ]); + + const teardown = async () => { + await client.close(); + await server.close(); + await subs.shutdown(); + }; + + return { client, teardown }; +} + +describe('tools/list capability annotations', () => { + let tmpDataDir: string; + + beforeEach(() => { + tmpDataDir = mkdtempSync(join(tmpdir(), 'wigolo-tool-annotations-')); + process.env.WIGOLO_DATA_DIR = tmpDataDir; + // `pluginsDir` defaults to `/plugins`, so the line above already + // isolates it — but pin it anyway so an exported WIGOLO_PLUGINS_DIR in the + // developer's shell can't make `initSubsystems()` import real plugin code. + process.env.WIGOLO_PLUGINS_DIR = join(tmpDataDir, 'plugins'); + resetConfig(); + _resetMigrationGuard(); + vi.clearAllMocks(); + }); + afterEach(() => { + delete process.env.WIGOLO_DATA_DIR; + delete process.env.WIGOLO_PLUGINS_DIR; + resetConfig(); + try { rmSync(tmpDataDir, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('every tool carries annotations with a non-empty title', async () => { + const { client, teardown } = await connectClient(); + try { + const res = await client.listTools(); + expect(res.tools).toHaveLength(Object.keys(EXPECTED).length); + for (const tool of res.tools) { + expect(tool.annotations, `${tool.name} has no annotations`).toBeDefined(); + expect(typeof tool.annotations?.title).toBe('string'); + expect((tool.annotations?.title as string).length).toBeGreaterThan(0); + } + } finally { + await teardown(); + } + }); + + it('every hint is an explicit boolean, never left undefined', async () => { + // An absent hint is not the same as `false`: the spec lets a host fall + // back to its own default, which is what makes tools prompt. + const { client, teardown } = await connectClient(); + try { + const res = await client.listTools(); + for (const tool of res.tools) { + for (const hint of HINT_KEYS) { + expect( + typeof tool.annotations?.[hint], + `${tool.name}.${hint} is not a boolean`, + ).toBe('boolean'); + } + } + } finally { + await teardown(); + } + }); + + it('matches the pinned hint matrix', async () => { + const { client, teardown } = await connectClient(); + try { + const res = await client.listTools(); + const actual = Object.fromEntries( + res.tools.map((t) => [ + t.name, + { + readOnlyHint: t.annotations?.readOnlyHint, + destructiveHint: t.annotations?.destructiveHint, + idempotentHint: t.annotations?.idempotentHint, + openWorldHint: t.annotations?.openWorldHint, + }, + ]), + ); + expect(actual).toEqual(EXPECTED); + } finally { + await teardown(); + } + }); + + it('the three state-changing tools are not advertised as read-only', async () => { + const { client, teardown } = await connectClient(); + try { + const res = await client.listTools(); + const notReadOnly = res.tools + .filter((t) => t.annotations?.readOnlyHint === false) + .map((t) => t.name) + .sort(); + expect(notReadOnly).toEqual(['cache', 'fetch', 'watch']); + } finally { + await teardown(); + } + }); +});