diff --git a/DECISIONS.md b/DECISIONS.md index 17500843..416b28d7 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1292,5 +1292,7 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-27] generateOpenApiSpec now emits the registry's `example` as an OpenAPI media-type `example` on JSON request bodies and JSON 200 responses, and the portable renderOperationMd (skills/openapi-to-skill) renders it as a fenced json block. EndpointDefinition.example has been REQUIRED since Phase 1 and all 125 endpoints populate example.response, but the generator never touched it, so the worked examples reached only the docs markdown builder (lib/docs/content/reference.ts): /api/v1/openapi.json carried zero examples and the generated skills/accounted-api had 0 json blocks across all 12 reference files. Chosen over adding a new registry field (the data already existed, only the delivery was missing) and over hand-writing examples into the overlays (they would drift from the schemas). Examples attach to `application/json` only: a multipart body's example is a file part JSON cannot express, and a binary (application/pdf) response's registry example describes the JSON envelope that endpoint does not send, so attaching it there would be a lie (pinned by openapi-examples.test.ts). SKILL.md is byte-identical: the 3304 added lines land in the on-demand references/*.md, not the always-loaded entry file. Rationale is Anthropic's tool-use-examples finding (72% to 90% on complex parameter handling): a condensed schema states a field's shape, an example states the conventions the shape cannot express. POST fiscal-periods/:id/currency-revaluation gained the one missing example.request so the exhaustive coverage assertions hold. [2026-08-27] /api/v1 now emits `Retry-After` on a 429 (RATE_LIMITED), sourced from a new exported `RATE_LIMIT_RETRY_AFTER_SECONDS` in lib/auth/api-keys.ts that the MCP server's previously hardcoded '60' also consumes. skills/accounted-api and its overlays (quickstart.md, conventions.md) have told agents to "honor Retry-After" on a 429 since the skill shipped, but no v1 route ever sent one: the wrapper's auth failure path early-returns through v1ErrorResponseFromCode, whose finalize() set only X-Request-Id and Gnubok-Version. 60 is an exact upper bound, not a guess: validate_and_increment_api_key uses a FIXED one-minute tumbling window and the limited branch deliberately does not slide it, so the current window can never have more than 60 s left. Chosen over emitting the IETF `RateLimit` / `RateLimit-Policy` fields (draft-ietf-httpapi-ratelimit-headers-11, 2026-05-23), which is the correct destination but needs a migration first: the RPC computes remaining quota and the exact reset instant internally yet its RETURNS TABLE carries no count/limit/window column, so TypeScript cannot see them. Also corrected the withApiV1 doc comment, which claimed step 8 stamps `X-RateLimit-Limit`; stampHeaders never did, and error paths bypass stampHeaders entirely. NOT fixed here and worth its own change: those same error paths also skip WRAPPED_RESPONSE_HEADERS, so v1 401/403/404/429/500 bodies ship without nosniff/CSP/HSTS. [2026-08-27] The tools/list payload guard now measures BOTH tool namespaces and asserts on the larger, and the ceiling moves 63.4K to 63.6K to cover it. `?tool_namespace=accounted` rewrites every gnubok_* reference to accounted_* (+3 chars each), costing ~209 tokens across the default catalog, so the accounted projection measured 63 491 tokens against a 63 400 ceiling: already ~90 tokens over, and untested, because the guard only ever serialized the gnubok projection (63 282, which passed). CLAUDE.md points new MCP installs at accounted-mcp and the accounted_* aliases, so the untested projection is the one a new user's client actually receives. The bump buys no new catalog surface: it re-points an existing ceiling at the real worst case. Verified the guard bites by temporarily asserting 63 450 and watching it fail on 63 491. A second test pins the DIRECTION of the delta (accounted > gnubok) so Math.max cannot silently stop describing reality if a future rename flips it. Not chosen: raising the ceiling without measuring both (leaves the same blind spot) or shrinking the catalog to fit 63.4K (a real option, but it is the Code Mode conversation, not a test fix). +[2026-08-27] gnubok_call_tool is implemented as a REWRITE in the tools/call dispatcher, not as a tool whose execute() forwards to another tool. Server-side every tool was always callable (the dispatcher resolves the name against the whole `tools` array; isDefaultCatalogTool gates only what tools/list SHOWS), so the failure was purely client-side and is what forced gnubok_reconcile_match back into the default catalog on 2026-08-26 ("a search-only tool is uncallable on Claude.ai"). A forwarding wrapper would have run the inner tool's execute() directly and thereby skipped the scope check, the unknown-argument guard, company routing + membership check, the test-key write block, the staging _meta and telemetry, all of which live between resolution and execute. Rewriting {tool, arguments} into a direct call BEFORE resolution makes every one of them apply to the real target for free; the wrapper's own execute() throws, and a test asserts that, so the rewrite cannot be silently removed. Restricted to annotations.readOnlyHint === true: a write must be named directly so the client sees its own annotations and its approval contract rather than a generic wrapper's (this refuses 13 of the 18 search-only tools and unlocks the 5 read ones). Doubly closed to anonymous callers: the pre-auth gate keys on the OUTER name and gnubok_call_tool is deliberately absent from PUBLIC_TOOLS, and the in-dispatcher re-check then keys on the INNER name; nothing is lost because all three public tools are in the default catalog. Telemetry gained errorKind 'bridge_refused' in BOTH lib/events/types.ts and the server's local payload type: widening only the local one type-checks under vitest (which does not typecheck) and fails `npm run build`, which is how it was caught. +[2026-08-27] gnubok_get_agent_briefing's outputSchema condensed 7 743 -> 4 565 chars by replacing four sub-schema interiors (ledger_context, dimensions, skatteverket_connection, recommended_tools) with a permissive `{type:'object'}` plus a fuller description, keeping the property declared so the top-level `additionalProperties: false` still holds. Deleting the properties was not an option for that reason, and adding `additionalProperties:false` to the condensed forms would have rejected the real payload. Safe because agent-briefing.test.ts pins the RUNTIME shape of all four blocks, so the contract stays guarded while the schema stops carrying 3 178 chars of documentation into every tools/list. Kept intact: `company` (its accounting_method prose drives the settlement posting), `atoms` and `memory` (they tell the agent to fetch bodies via gnubok_load_skill). Net with the new tool: guarded (accounted) projection 63 491 -> 62 942 tokens, and the payload ceiling TIGHTENS 63.6K -> 63.1K, the first downward move in that ledger. [2026-08-27] Cockpit auto-landing gated to byrå owner/admin (isCockpitLandingRole; landing route + '/' bounce), superseding the 2026-08-05 all-members widening: plain members land like regular users and open the cockpit from the nav; the middleware zero-company steer stays ungated because a member with no client companies has nowhere else to land. Allowlist over role!=='member' so future roles default to the regular landing. [2026-08-27] Klarmarkera (markPeriodClosedExternally) gets an undo, reopenExternallyClosedPeriod, allowed only while the closed state still comes from klarmarkera (closed_externally set, no closing entry): that close was a person's control decision without a bokslutsverifikat, so reversing it strands nothing, whereas a closePeriod close keeps its closing entry and stays irreversible here. The reopen clears the lock too, because the reason to reopen is to change the period's contents (Forsslund Systems 2026-08-27: five imported years klarmarkerade, then the prior-year SIE turned out wrong; replace refused the closed year, unlock refused the closed state, no way back). Audit_log row plus period.unlocked event; the MCP staged-op surface (lock/unlock) does not get a reopen op yet, follow-up. diff --git a/extensions/general/mcp-server/__tests__/call-tool-bridge.test.ts b/extensions/general/mcp-server/__tests__/call-tool-bridge.test.ts new file mode 100644 index 00000000..b896ddc0 --- /dev/null +++ b/extensions/general/mcp-server/__tests__/call-tool-bridge.test.ts @@ -0,0 +1,251 @@ +/** + * Tests for the gnubok_call_tool bridge in the MCP dispatcher. + * + * Server-side, every tool has always been callable: `tools/call` resolves the + * name against the whole `tools` array, and `isDefaultCatalogTool` gates only + * what tools/list SHOWS. The failure was purely client-side, and DECISIONS.md + * records the consequence on 2026-08-26: `gnubok_reconcile_match` had to be + * promoted back into the default catalog because "a search-only tool is + * uncallable on Claude.ai". + * + * The bridge gives such a client one visible name to forward through. It is + * implemented as a REWRITE ahead of tool resolution rather than as a wrapper + * that calls the inner tool's execute(), because everything between resolution + * and execute (scope check, unknown-argument guard, company routing, the + * test-key write block, staging _meta, telemetry) must apply to the real + * target. These tests exist to prove it does. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { eventBus } from '@/lib/events/bus' + +vi.mock('@/lib/supabase/server', () => ({ + createClient: vi.fn(), + createServiceClient: vi.fn(), +})) + +vi.mock('@/lib/auth/api-keys', async (importOriginal) => { + const actual = await importOriginal() + const chain: unknown = new Proxy( + {}, + { + get(_t, prop) { + if (prop === 'then') { + return (resolve: (v: unknown) => void) => resolve({ data: null, error: null }) + } + return () => chain + }, + }, + ) + const membershipChain: unknown = new Proxy( + {}, + { + get(_t, prop) { + if (prop === 'then') { + return (resolve: (v: unknown) => void) => + resolve({ + data: { company_id: '11111111-1111-4111-8111-111111111111', role: 'owner' }, + error: null, + }) + } + return () => membershipChain + }, + }, + ) + return { + ...actual, + extractBearerToken: vi.fn().mockReturnValue('test-token'), + validateApiKey: vi.fn().mockResolvedValue({ + userId: 'user-1', + companyId: '11111111-1111-4111-8111-111111111111', + scopes: ['transactions:read', 'reports:read', 'pending_operations:approve'], + apiKeyId: 'key-1', + apiKeyName: 'Live Key', + mode: 'live', + }), + createServiceClientNoCookies: vi.fn(() => ({ + from: (table: string) => (table === 'company_members' ? membershipChain : chain), + rpc: () => chain, + })), + } +}) + +vi.mock('@/lib/entitlements/has-capability', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, hasCapability: vi.fn().mockResolvedValue(true) } +}) + +import { handleMcpRequest, tools, isDefaultCatalogTool } from '../server' +import { validateApiKey, extractBearerToken } from '@/lib/auth/api-keys' + +function mcpToolCall(name: string, args: Record = {}): Request { + return new Request('http://localhost:3000/api/extensions/ext/mcp-server/mcp', { + method: 'POST', + headers: { 'Content-Type': 'application/json', Authorization: 'Bearer test-token' }, + body: JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'tools/call', + params: { name, arguments: args }, + }), + }) +} + +interface ToolCalledEvent { + tool: string + success: boolean + isError: boolean + errorKind: string | null + latencyMs: number +} + +function captureNextToolCalled(): Promise { + return new Promise((resolve) => { + const off = eventBus.on('mcp.tool_called', (payload) => { + off() + resolve(payload as unknown as ToolCalledEvent) + }) + }) +} + +async function parsedToolResult( + response: Response, +): Promise<{ isError: boolean; payload: Record }> { + const json = await response.json() + const result = json.result as { isError?: boolean; content: { text: string }[] } + return { isError: result.isError === true, payload: JSON.parse(result.content[0].text) } +} + +const bridgeTool = tools.find((t) => t.name === 'gnubok_call_tool')! + +describe('gnubok_call_tool registration', () => { + it('is in the default catalog and read-only', () => { + expect(bridgeTool).toBeDefined() + expect(isDefaultCatalogTool(bridgeTool)).toBe(true) + expect(bridgeTool.annotations.readOnlyHint).toBe(true) + }) + + it('has no direct implementation: the dispatcher rewrite is load-bearing', async () => { + // If this ever resolves instead of throwing, the rewrite was removed and + // every bridged call would have skipped the read-only check above it. + await expect( + bridgeTool.execute({}, 'company-id', 'user-id', {} as never, { type: 'api_key' }), + ).rejects.toThrow(/no direct implementation/i) + }) +}) + +describe('gnubok_call_tool bridge', () => { + beforeEach(() => { + vi.clearAllMocks() + eventBus.clear() + }) + + it('forwards to the inner tool and attributes telemetry to it, not to the wrapper', async () => { + const eventPromise = captureNextToolCalled() + + await handleMcpRequest(mcpToolCall('gnubok_call_tool', { tool: 'gnubok_list_skills' })) + + const event = await eventPromise + expect(event.tool).toBe('gnubok_list_skills') + expect(event.errorKind).not.toBe('bridge_refused') + }) + + it('reaches a search-only read tool, which is the whole point', async () => { + const searchOnlyRead = tools.find( + (t) => !isDefaultCatalogTool(t) && t.annotations.readOnlyHint === true, + )! + expect(searchOnlyRead).toBeDefined() + const eventPromise = captureNextToolCalled() + + await handleMcpRequest(mcpToolCall('gnubok_call_tool', { tool: searchOnlyRead.name })) + + const event = await eventPromise + expect(event.tool).toBe(searchOnlyRead.name) + expect(event.errorKind).not.toBe('bridge_refused') + }) + + it('refuses a write target so the staging and approval contract stays visible', async () => { + const eventPromise = captureNextToolCalled() + + const response = await handleMcpRequest( + mcpToolCall('gnubok_call_tool', { + tool: 'gnubok_approve_pending_operation', + arguments: { operation_id: 'op-1' }, + }), + ) + const { isError, payload } = await parsedToolResult(response) + + expect(isError).toBe(true) + expect(JSON.stringify(payload)).toContain('gnubok_approve_pending_operation') + const event = await eventPromise + expect(event.errorKind).toBe('bridge_refused') + // Refused before execute(): nothing is staged, nothing is approved. + expect(event.latencyMs).toBe(0) + }) + + it('refuses a call with no tool name', async () => { + const eventPromise = captureNextToolCalled() + + const response = await handleMcpRequest(mcpToolCall('gnubok_call_tool', {})) + const { isError } = await parsedToolResult(response) + + expect(isError).toBe(true) + const event = await eventPromise + expect(event.errorKind).toBe('bridge_refused') + }) + + it('enforces the INNER tool scope, not the wrapper (which has none)', async () => { + vi.mocked(validateApiKey).mockResolvedValueOnce({ + userId: 'user-1', + companyId: '11111111-1111-4111-8111-111111111111', + // Deliberately omits transactions:read, which the inner tool requires. + scopes: ['reports:read'], + apiKeyId: 'key-1', + apiKeyName: 'Narrow Key', + mode: 'live', + } as Awaited>) + const eventPromise = captureNextToolCalled() + + const response = await handleMcpRequest( + mcpToolCall('gnubok_call_tool', { tool: 'gnubok_list_cash_accounts' }), + ) + const { isError } = await parsedToolResult(response) + + expect(isError).toBe(true) + const event = await eventPromise + expect(event.errorKind).toBe('scope_denied') + expect(event.tool).toBe('gnubok_list_cash_accounts') + }) + + it('applies the unknown-argument guard to the inner tool', async () => { + const response = await handleMcpRequest( + mcpToolCall('gnubok_call_tool', { + tool: 'gnubok_list_skills', + arguments: { nonexistent_parameter: 1 }, + }), + ) + const { isError, payload } = await parsedToolResult(response) + + expect(isError).toBe(true) + expect(JSON.stringify(payload)).toContain('nonexistent_parameter') + }) + + it('is closed to anonymous callers: the pre-auth gate keys on the outer name', async () => { + // gnubok_call_tool is deliberately absent from PUBLIC_TOOLS, so an + // unauthenticated client cannot use it as a lever at all. Nothing is lost: + // all three public tools are in the default catalog already. + vi.mocked(extractBearerToken).mockReturnValueOnce(null) + + const response = await handleMcpRequest( + mcpToolCall('gnubok_call_tool', { tool: 'gnubok_list_skills' }), + ) + expect(response.status).toBe(401) + }) + + it('reports an unknown inner tool through the normal unknown-tool path', async () => { + const response = await handleMcpRequest( + mcpToolCall('gnubok_call_tool', { tool: 'gnubok_not_a_real_tool' }), + ) + const json = (await response.json()) as { error?: { message?: string } } + expect(json.error?.message).toContain('gnubok_not_a_real_tool') + }) +}) diff --git a/extensions/general/mcp-server/__tests__/payload-size.bench.test.ts b/extensions/general/mcp-server/__tests__/payload-size.bench.test.ts index 7bad2616..f0a36f72 100644 --- a/extensions/general/mcp-server/__tests__/payload-size.bench.test.ts +++ b/extensions/general/mcp-server/__tests__/payload-size.bench.test.ts @@ -270,10 +270,20 @@ describe('tools/list payload size guard', () => { // nothing tested it, so this bump buys no new catalog surface. It // re-points an existing ceiling at the payload new installs actually // receive; the gnubok projection still sits ~320 tokens under it. - // Long-term answer to growth is leaning harder on gnubok_search_tools: if this - // fires again, prefer trimming descriptions or making a tool opt-in via search - // before bumping further. - expect(approxTokens).toBeLessThan(63_600) + // * 63.6K DOWN to 63.1K, the first tightening in this ledger. Two + // changes, net -549 tokens on the guarded (accounted) projection: + // gnubok_get_agent_briefing's outputSchema went 7,743 to 4,565 chars + // by condensing four sub-schemas whose interiors were documentation + // rather than contract (ledger_context, dimensions, + // skatteverket_connection, recommended_tools: agent-briefing.test.ts + // pins their RUNTIME shape, so nothing was left unguarded), against + // +~245 for the new gnubok_call_tool. + // Long-term answer to growth is no longer a ceiling bump. gnubok_call_tool + // makes `catalogVisibility: 'search'` usable for READ tools on hosts that + // can only invoke what tools/list showed them, which is the constraint that + // forced gnubok_reconcile_match back into the default catalog on + // 2026-08-26. Demote a read to search-only before proposing a bump. + expect(approxTokens).toBeLessThan(63_100) }) it('keeps the accounted_* namespace as the measured worst case', () => { diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index 24b99caa..22da645a 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -2863,6 +2863,64 @@ function projectMcpPayload(value: T, namespace: McpToolNamespace): T { // ── Tools ──────────────────────────────────────────────────── export const tools: McpTool[] = [ + { + // The bridge that makes `catalogVisibility: 'search'` usable on hosts that + // can only invoke what tools/list showed them. + // + // Server-side, every tool has always been callable: the tools/call + // dispatcher resolves the name against the whole `tools` array and + // `isDefaultCatalogTool` gates only what tools/list SHOWS. The failure was + // purely client-side (Claude.ai cannot name a tool it never saw), which is + // why search-only tools shipped unreachable there. + // + // This tool is never executed. `tools/call` rewrites a + // gnubok_call_tool({tool, arguments}) request into a direct call on the + // inner tool BEFORE resolution, so scope checks, the unknown-argument + // guard, company routing, the staging contract and telemetry all apply to + // the real target rather than to a wrapper that would have bypassed them. + name: 'gnubok_call_tool', + title: 'Call a Read Tool by Name', + description: + 'Invoke any read-only tool by name, including ones absent from tools/list. Find the name with gnubok_search_tools first. Writes are refused: call a write tool directly so its approval contract stays visible.', + inputSchema: { + type: 'object', + additionalProperties: false, + properties: { + tool: { + type: 'string', + description: 'Canonical name of the read-only tool to invoke, e.g. "gnubok_get_reconciliation_status".', + }, + arguments: { + type: 'object', + description: "Arguments for that tool, validated against its own inputSchema. Omit for a tool that takes none.", + }, + }, + required: ['tool'], + }, + // The response is whatever the inner tool returns, so no fixed shape can + // be declared. Every tool must carry an object outputSchema, and an open + // object is the only honest one here. + outputSchema: { + type: 'object', + additionalProperties: true, + description: 'The inner tool\'s own result, unchanged.', + }, + annotations: { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: false, + }, + async execute() { + // Unreachable: the dispatcher rewrites the call before resolution. If + // this ever throws, the rewrite was removed and every call would have + // silently skipped the read-only check. + throw codedError( + 'VALIDATION_ERROR', + 'gnubok_call_tool is resolved by the dispatcher and has no direct implementation.', + ) + }, + }, { name: 'gnubok_search_tools', title: 'Search MCP Tools', @@ -4403,127 +4461,19 @@ export const tools: McpTool[] = [ }, dimensions: { type: 'object', - additionalProperties: false, - description: 'Dimension registry snapshot (kostnadsställe/projekt). OMITTED when the company has none registered; presence means lines can be tagged via the dims bag on gnubok_create_voucher.', - properties: { - enabled: { type: 'boolean', description: 'When true, dims-bag values are validated against the registry.' }, - dimensions: { - type: 'array', - items: { - type: 'object', - additionalProperties: false, - properties: { - sie_dim_no: { type: 'number' }, - name: { type: 'string' }, - active_value_count: { type: 'number' }, - required_on_accounts: { - type: 'array', - description: 'BAS accounts with an active required-rule: postings there are refused without a value for this dimension.', - items: { type: 'string' }, - }, - default_on_accounts: { - type: 'array', - description: 'BAS accounts where a default/fixed rule auto-applies a value at draft creation.', - items: { type: 'string' }, - }, - top_values: { - type: 'array', - description: 'Up to 10 active values; full list via gnubok_list_dimension_values.', - items: { - type: 'object', - additionalProperties: false, - properties: { - code: { type: 'string' }, - name: { type: 'string' }, - }, - required: ['code', 'name'], - }, - }, - }, - required: ['sie_dim_no', 'name', 'active_value_count', 'required_on_accounts', 'default_on_accounts', 'top_values'], - }, - }, - }, - required: ['enabled', 'dimensions'], + description: + 'Dimension registry snapshot (kostnadsställe/projekt): an enabled flag plus the registered dimensions with their codes, counts and top values. OMITTED when the company has none registered; presence means lines can be tagged via the dims bag on gnubok_create_voucher, and enabled=true means dims-bag values are validated against the registry.', }, ledger_context: { type: 'object', - additionalProperties: false, - description: 'Digest of how this company books things: top-5 counterparty + top-3 supplier patterns. Full picture (account usage, explicit rules, VAT profile, conventions) in the Accounted://ledger/context resource. Evidence is historical frequency, NOT permission to auto-book: weigh seen count AND recency, never a ratio alone. OMITTED when not computable.', - properties: { - resource_uri: { type: 'string', description: 'URI of the full ledger-context resource.' }, - window_from: { type: 'string', description: 'Start of the rolling stats window (ISO date).' }, - posted_entries_window: { type: 'number', description: 'Posted journal entries in the window. Low = thin evidence: treat patterns as weak.' }, - top_counterparty_patterns: { - type: 'array', - description: 'Most frequent booked bank-feed counterparties with dominant booking. evidence = seen N in 12m, M agreed, last booked; below 0.7 agreement excluded.', - items: { - type: 'object', - additionalProperties: false, - properties: { - counterparty: { type: 'string' }, - dominant_category: { type: 'string' }, - dominant_account_number: { type: ['string', 'null'] }, - evidence: { - type: 'object', - additionalProperties: false, - properties: { - seen_12m: { type: 'number' }, - agree: { type: 'number' }, - last_booked: { type: 'string' }, - }, - required: ['seen_12m', 'agree', 'last_booked'], - }, - }, - required: ['counterparty', 'dominant_category', 'dominant_account_number', 'evidence'], - }, - }, - top_supplier_patterns: { - type: 'array', - description: 'Most invoiced suppliers (AP side) with dominant expense account and VAT treatment. Same evidence semantics.', - items: { - type: 'object', - additionalProperties: false, - properties: { - supplier: { type: 'string' }, - dominant_account_number: { type: 'string' }, - vat_treatment: { type: ['string', 'null'] }, - evidence: { - type: 'object', - additionalProperties: false, - properties: { - seen_12m: { type: 'number' }, - agree: { type: 'number' }, - last_booked: { type: 'string' }, - }, - required: ['seen_12m', 'agree', 'last_booked'], - }, - }, - required: ['supplier', 'dominant_account_number', 'vat_treatment', 'evidence'], - }, - }, - }, - required: ['resource_uri', 'window_from', 'posted_entries_window', 'top_counterparty_patterns', 'top_supplier_patterns'], + description: + 'Digest of how this company books things: top-5 counterparty + top-3 supplier patterns, each with an evidence block (seen_12m, agree, share, last_booked) and the rolling window it was computed over. Evidence is historical frequency, NOT permission to auto-book: weigh seen count AND recency, never a ratio alone. OMITTED when not computable. Field-by-field detail, plus account usage, explicit rules, VAT profile and conventions, is in the Accounted://ledger/context resource.', }, recommended_tools: { type: 'array', + items: { type: 'object' }, description: - 'Per-workflow tool loadouts, ordered by call sequence. Deferred-loading harnesses batch-load a whole cluster in one call (ToolSearch select:a,b,c). Static; validated against the registry.', - items: { - type: 'object', - additionalProperties: false, - properties: { - workflow: { type: 'string', description: 'Stable workflow key.' }, - description: { type: 'string' }, - skill: { type: 'string', description: 'Slug for gnubok_load_skill (full playbook).' }, - tools: { - type: 'array', - items: { type: 'string' }, - description: 'Exact tool names, ordered.', - }, - }, - required: ['workflow', 'description', 'skill', 'tools'], - }, + 'Per-workflow tool loadouts, ordered by call sequence: each entry names a workflow, describes it, and lists the exact registry tools it needs. Deferred-loading harnesses batch-load a whole cluster in one call (ToolSearch select:a,b,c). Static; validated against the registry at module load.', }, feedback_channel: { type: 'object', @@ -4538,19 +4488,8 @@ export const tools: McpTool[] = [ }, skatteverket_connection: { type: 'object', - additionalProperties: false, description: - 'Present only when a Skatteverket connection exists. needs_reconsent: only a person can fix it (BankID under Inställningar → Skatteverket); warn the user before starting SKV work.', - properties: { - status: { type: 'string', enum: ['active', 'needs_reconsent'] }, - source: { type: 'string', enum: ['user', 'system'] }, - connected_at: { - type: ['string', 'null'], - description: 'Personal sessions last ~65 min from this time; absent for system (ombud) connections.', - }, - message: { type: 'string' }, - }, - required: ['status', 'source'], + 'Present only when a Skatteverket connection exists. Carries status ("active" or "needs_reconsent") and the grant detail behind it. needs_reconsent: only a person can fix it (BankID under Inställningar → Skatteverket); warn the user before starting SKV work.', }, }, required: ['company', 'user_name', 'profile_summary', 'atoms', 'memory', 'recommended_tools'], @@ -19075,7 +19014,7 @@ function emitToolCallTelemetry(payload: { success: boolean isError: boolean errorCode: string | null - errorKind: 'execution' | 'scope_denied' | 'capability_denied' | 'company_access_denied' | 'unknown_tool' | 'test_key_write_blocked' | null + errorKind: 'execution' | 'scope_denied' | 'capability_denied' | 'company_access_denied' | 'unknown_tool' | 'test_key_write_blocked' | 'bridge_refused' | null errorMessage: string | null requestId: string | number | null userId: string @@ -19543,7 +19482,7 @@ export async function handleMcpRequest(request: Request): Promise { ] : []), 'Discovery:', - '• tools/list returns common tool schemas. Call gnubok_search_tools(query="…") for specialized tools: it ranks all capabilities; pass detail="name"|"summary"|"full" to control payload size.', + '• tools/list returns common tool schemas. Call gnubok_search_tools(query="…") for specialized tools: it ranks all capabilities; pass detail="name"|"summary"|"full" to control payload size. If your client cannot invoke a tool that is not in tools/list, reach any READ tool through gnubok_call_tool({tool, arguments}); writes must be named directly.', '• gnubok_get_agent_briefing returns recommended_tools: ordered per-workflow tool loadouts (categorize_month, close_period, invoice_run, vat_declaration, payroll_month). If your harness defers tool loading, batch-load a whole workflow in one call (e.g. Claude Code ToolSearch select:a,b,c) instead of searching cluster by cluster.', `• This connection can work with every non-archived company the API-key user belongs to. Call gnubok_list_companies to discover company_id values. Omit company_id to use the API key default (${companyId ?? 'none yet: this account has no company. Create it with gnubok_create_company (preview first, then confirm=true); the "onboarding" skill walks the whole setup'}); when selecting another company, repeat company_id on every company-data call, including approval.`, '• MCP resources use the API key default company. For a selected non-default company, call gnubok_get_agent_briefing with company_id instead of relying on Accounted://company/current or other company-data resources.', @@ -19653,18 +19592,85 @@ export async function handleMcpRequest(request: Request): Promise { case 'tools/call': { const rawRequestedToolName = (params as Record)?.name - const requestedToolName = + const outerToolName = typeof rawRequestedToolName === 'string' ? rawRequestedToolName : '' - const toolName = toCanonicalToolName(requestedToolName) - const rawToolArgs = ((params as Record)?.arguments ?? {}) as Record< + const outerToolArgs = ((params as Record)?.arguments ?? {}) as Record< string, unknown > + // gnubok_call_tool bridge. Rewrite {tool, arguments} into a direct call + // on the inner tool BEFORE resolution, so the scope check, the + // unknown-argument guard, company routing, the test-key write block, the + // staging _meta and telemetry below all apply to the real target. A + // wrapper that called the inner tool's execute() itself would have + // skipped every one of them. + const viaBridge = toCanonicalToolName(outerToolName) === 'gnubok_call_tool' + const requestedToolName = viaBridge + ? typeof outerToolArgs.tool === 'string' + ? outerToolArgs.tool + : '' + : outerToolName + const toolName = toCanonicalToolName(requestedToolName) + const rawToolArgs = viaBridge + ? ((outerToolArgs.arguments ?? {}) as Record) + : outerToolArgs + const tool = tools.find((t) => t.name === toolName) + // The pre-auth gate already refused anonymous calls to anything outside // PUBLIC_TOOLS; re-checked here so the dispatcher never depends on it. + // Ordered ahead of the bridge refusal below so an anonymous caller is + // turned away before it learns whether a named tool is read-only. + // + // The bridge cannot widen anonymous reach, and is doubly closed: the + // pre-auth gate keys on the OUTER name, and gnubok_call_tool is not in + // PUBLIC_TOOLS, so an anonymous bridged call is refused before it gets + // here; this line then re-checks the INNER name. Nothing is lost by + // that, because all three public tools are in the default catalog and + // an anonymous caller never needs the bridge to name them. if (isAnonymous && !isPublicTool(toolName)) return unauthorized() + + // The bridge reaches reads only. A write must be named directly so the + // client sees its own annotations and its staging/approval contract + // rather than a generic wrapper's. An unknown-but-named target falls + // through to the unknown-tool handler below, which lists what exists. + if (viaBridge && (!requestedToolName || (tool && tool.annotations.readOnlyHint !== true))) { + const bridgeError = toToolError( + codedError( + 'VALIDATION_ERROR', + requestedToolName + ? `${requestedToolName} is not a read-only tool, so gnubok_call_tool will not invoke it. Call ${requestedToolName} directly by name.` + : 'gnubok_call_tool requires a "tool" argument naming the read-only tool to invoke.', + ), + { toolName: 'gnubok_call_tool' }, + ) + emitToolCallTelemetry({ + tool: 'gnubok_call_tool', + requiredScope: null, + actor, + latencyMs: 0, + success: false, + isError: true, + errorCode: bridgeError.error.code, + errorKind: 'bridge_refused', + errorMessage: bridgeError.error.message_sv, + requestId: id ?? null, + userId, + companyId, + }) + return NextResponse.json( + jsonRpc(id ?? null, decorate({ + content: [ + { + type: 'text', + text: JSON.stringify(projectMcpPayload(bridgeError, toolNamespace), null, 2), + }, + ], + isError: true, + })) + ) + } if (!tool) { emitToolCallTelemetry({ tool: toolName ?? '', diff --git a/lib/events/types.ts b/lib/events/types.ts index 5d5b3efc..4945a5a5 100644 --- a/lib/events/types.ts +++ b/lib/events/types.ts @@ -242,7 +242,8 @@ export type CoreEvent = success: boolean // true iff the tool returned without throwing AND was invoked (not denied) isError: boolean // matches the JSON-RPC tool-result isError flag returned to the client errorCode: string | null // structured error code from tool-result.toToolError when applicable - errorKind: 'execution' | 'scope_denied' | 'capability_denied' | 'company_access_denied' | 'unknown_tool' | 'test_key_write_blocked' | null + errorKind: 'execution' | 'scope_denied' | 'capability_denied' | 'company_access_denied' | 'unknown_tool' | 'test_key_write_blocked' | 'bridge_refused' | null + // bridge_refused: gnubok_call_tool was pointed at a write tool, or at nothing. errorMessage: string | null // human-readable error message (truncated to 500 chars), null on success. // Raw material for clustering real agent failures into curated gotchas: // errorCode alone can't distinguish "period locked" from "unbalanced".