From 27ef39862369cc01e6994db1660d10fd3c6f17d8 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com> Date: Wed, 22 Jul 2026 18:31:39 +0200 Subject: [PATCH] feat(mcp): briefing returns a recommended tool loadout per workflow (#1098) (#1109) With deferred tool loading on the client side (Claude Code ToolSearch, claude.ai connector search), an agent starting a bookkeeping session burns 4-6 round-trips discovering tools cluster by cluster. The briefing now returns recommended_tools: five per-workflow loadouts (categorize_month, close_period, invoice_run, vat_declaration, payroll_month), each with a stable key, one-line description, the gnubok_load_skill slug for the full playbook, and the exact registry tool names ordered by typical call sequence, so a batch-selection harness loads a whole workflow cluster in ONE ToolSearch select call. Drift protection: assertRecommendedLoadoutsValid() runs at module init in server.ts right after the tools array is built, failing module load (and every test importing the server) if a loadout names a tool absent from the registry or a skill slug absent from workflowSkills; the agent-briefing test suite pins the same checks. The list is static per company: the briefing does not query workflow state today, so gating inclusion would add reads to the bootstrap hot path. tools/list payload ceiling bumped 57K to 57.5K with a documented progression entry (schema prose trimmed to the floor first; headroom before the change was ~15 tokens). Fixes #1098 Co-authored-by: Claude Fable 5 --- DECISIONS.md | 1 + .../__tests__/agent-briefing.test.ts | 83 +++++++++++ .../__tests__/payload-size.bench.test.ts | 9 +- .../general/mcp-server/recommended-tools.ts | 141 ++++++++++++++++++ extensions/general/mcp-server/server.ts | 42 +++++- 5 files changed, 273 insertions(+), 3 deletions(-) create mode 100644 extensions/general/mcp-server/recommended-tools.ts diff --git a/DECISIONS.md b/DECISIONS.md index e66aa1ab..f3b30e93 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -274,3 +274,4 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-07-22] Issue #313 fix limited to the meals warning; left "Representationsgåvor max 180 kr" on the gåvor line untouched: scope rule (only the inverted-VAT claim and repealed ML 8:9 reference), even though the swedish-vat skill lists 300 SEK as the representationsgåvor base; flagged as follow-up in the PR. [2026-07-22] LEGACY_DISCOVERY_HOSTS drift guard (#1093) is an exported validateLegacyDiscoveryHosts() returning a violations list, exercised only by a unit test that pins the registered prod config (app.accounted.se canonical + app.gnubok.se SKV pin), not a startup assertion: CI does not set the prod env vars, so a runtime assertion would either no-op in CI or crash self-hosted deploys with different domains; the test-pinned constants make any allowlist or pin change a deliberate, reviewed edit. [2026-07-22] Stuck-committing recovery sweep (#843) rejects rows without positive evidence instead of reverting to pending, and only three op types (categorize_transaction, link_transaction_journal_entry, match_transaction_invoice) can recover to committed: no generic side-effect -> pending_op linkage exists yet (that is #842's posted-ids work), so evidence is limited to types whose params identify a target row with an unambiguous posted state; reverting to pending risks re-executing side-effects that posted without a trace (duplicate entries/emails). +[2026-07-22] MCP briefing recommended_tools (#1098) ships as a STATIC per-workflow loadout list, not state-gated: the briefing does not query workflow state (unbooked counts, open periods) today, so gating would add reads to the session-bootstrap hot path for marginal honesty; drift protection is a module-init assert against the tool registry + workflow-skill slugs, pinned by tests. diff --git a/extensions/general/mcp-server/__tests__/agent-briefing.test.ts b/extensions/general/mcp-server/__tests__/agent-briefing.test.ts index db9090ce..66afd463 100644 --- a/extensions/general/mcp-server/__tests__/agent-briefing.test.ts +++ b/extensions/general/mcp-server/__tests__/agent-briefing.test.ts @@ -4,6 +4,8 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest' import { tools } from '../server' +import { RECOMMENDED_WORKFLOW_LOADOUTS, assertRecommendedLoadoutsValid } from '../recommended-tools' +import { workflowSkills } from '../skills' vi.mock('@/lib/supabase/server', () => ({ createClient: vi.fn(), @@ -452,3 +454,84 @@ describe('gnubok_get_agent_briefing tool', () => { expect(result.dimensions?.enabled).toBe(false) }) }) + +describe('recommended_tools workflow loadouts (issue #1098)', () => { + it('every tool name in every loadout exists in the actual tool registry (drift guard)', () => { + const known = new Set(tools.map((t) => t.name)) + for (const loadout of RECOMMENDED_WORKFLOW_LOADOUTS) { + const unknown = loadout.tools.filter((name) => !known.has(name)) + expect(unknown, `workflow "${loadout.workflow}" references unknown tools`).toEqual([]) + } + }) + + it('every loadout skill slug exists in the workflow-skill registry', () => { + const slugs = new Set(workflowSkills.map((s) => s.slug)) + for (const loadout of RECOMMENDED_WORKFLOW_LOADOUTS) { + expect(slugs.has(loadout.skill), `workflow "${loadout.workflow}" skill "${loadout.skill}"`).toBe(true) + } + }) + + it('workflow keys are unique, stable snake_case, and loadouts are non-empty with a description', () => { + const keys = RECOMMENDED_WORKFLOW_LOADOUTS.map((w) => w.workflow) + expect(new Set(keys).size).toBe(keys.length) + for (const loadout of RECOMMENDED_WORKFLOW_LOADOUTS) { + expect(loadout.workflow).toMatch(/^[a-z][a-z0-9_]*$/) + expect(loadout.description.length).toBeGreaterThan(0) + expect(loadout.tools.length).toBeGreaterThan(0) + // No duplicate tool names within a single loadout. + expect(new Set(loadout.tools).size).toBe(loadout.tools.length) + } + }) + + it('assertRecommendedLoadoutsValid throws on a tool name missing from the registry', () => { + const known = new Set(tools.map((t) => t.name)) + known.delete('gnubok_categorize_transaction') + expect(() => assertRecommendedLoadoutsValid(known)).toThrow( + /categorize_month.*unknown tool "gnubok_categorize_transaction"/ + ) + }) + + it('assertRecommendedLoadoutsValid passes against the real registry (mirrors the module-init guard)', () => { + expect(() => assertRecommendedLoadoutsValid(new Set(tools.map((t) => t.name)))).not.toThrow() + }) + + it('the briefing returns recommended_tools with the expected shape, even for an empty company', async () => { + const tool = tools.find((t) => t.name === 'gnubok_get_agent_briefing')! + const supabase = mockSupabase({ profile: null }) + const result = (await tool.execute( + {}, + 'company-1', + 'user-1', + supabase as never, + { type: 'api_key' } + )) as { + recommended_tools: Array<{ workflow: string; description: string; skill: string; tools: string[] }> + } + expect(Array.isArray(result.recommended_tools)).toBe(true) + expect(result.recommended_tools.length).toBe(RECOMMENDED_WORKFLOW_LOADOUTS.length) + for (const entry of result.recommended_tools) { + expect(typeof entry.workflow).toBe('string') + expect(typeof entry.description).toBe('string') + expect(typeof entry.skill).toBe('string') + expect(Array.isArray(entry.tools)).toBe(true) + expect(entry.tools.every((n) => typeof n === 'string')).toBe(true) + } + // The static list is returned verbatim, in registry order. + expect(result.recommended_tools.map((w) => w.workflow)).toEqual( + RECOMMENDED_WORKFLOW_LOADOUTS.map((w) => w.workflow) + ) + }) + + it('the briefing outputSchema declares recommended_tools as required', () => { + const tool = tools.find((t) => t.name === 'gnubok_get_agent_briefing')! + const output = tool.outputSchema as { properties: Record; required: string[] } + expect(output.properties.recommended_tools).toBeDefined() + expect(output.required).toContain('recommended_tools') + }) + + it('the briefing description advertises the loadout and the one-call batch-load pattern', () => { + const tool = tools.find((t) => t.name === 'gnubok_get_agent_briefing')! + expect(tool.description).toContain('recommended_tools') + expect(tool.description).toContain('ToolSearch') + }) +}) 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 df4f153f..13559599 100644 --- a/extensions/general/mcp-server/__tests__/payload-size.bench.test.ts +++ b/extensions/general/mcp-server/__tests__/payload-size.bench.test.ts @@ -129,9 +129,16 @@ describe('tools/list payload size guard', () => { // gnubok_delete_absence (inverse of register_absence), both inlining // STAGED_OPERATION_SCHEMA + _meta + company_id routing. Descriptions // trimmed to the floor first; the remainder is wire contract. + // * 57K → 57.5K with recommended_tools on gnubok_get_agent_briefing + // (#1098): the per-workflow tool-loadout array in the outputSchema + // (~175 tokens) lets deferred-loading harnesses batch-load a whole + // workflow cluster in one ToolSearch select call instead of 4-6 + // discovery round-trips. Schema prose trimmed to the floor first; + // headroom before the change was ~15 tokens, and the remainder is + // the wire contract agents read the loadout through. // 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(57_000) + expect(approxTokens).toBeLessThan(57_500) }) }) diff --git a/extensions/general/mcp-server/recommended-tools.ts b/extensions/general/mcp-server/recommended-tools.ts new file mode 100644 index 00000000..123a1272 --- /dev/null +++ b/extensions/general/mcp-server/recommended-tools.ts @@ -0,0 +1,141 @@ +/** + * Recommended tool loadouts per workflow, surfaced by gnubok_get_agent_briefing + * as `recommended_tools`. + * + * Why: client harnesses with deferred tool loading (Claude Code ToolSearch, + * claude.ai connector search) otherwise burn 4-6 round-trips discovering tools + * cluster by cluster before any work happens. Each loadout names the exact + * registry tools a workflow needs, ordered by typical call sequence, so a + * harness that supports batch selection (ToolSearch select:a,b,c) loads the + * whole cluster in ONE call, and connector agents stop guessing search + * keywords. + * + * Drift protection (same spirit as deriveToolMeta): the loadouts are validated + * against the real tool registry and the workflow-skill registry via + * assertRecommendedLoadoutsValid(), called at module init in server.ts right + * after the tools array is defined. A loadout naming a tool or skill that does + * not exist fails module load, and therefore every test that imports the + * server. __tests__/agent-briefing.test.ts additionally pins the check. + * + * The list is static per issue #1098: the briefing does not currently query + * workflow state (unbooked counts, open periods), so gating inclusion on state + * would mean new reads in the hot bootstrap path. Every loadout is returned + * for every company; applicability is the agent's judgment call. + */ +import { workflowSkills } from './skills' + +export interface WorkflowLoadout { + /** Stable snake_case workflow key (e.g. "categorize_month"). */ + workflow: string + /** One-line English description of what the workflow accomplishes. */ + description: string + /** Workflow-skill slug: pass to gnubok_load_skill for the full playbook. */ + skill: string + /** Exact registry tool names, ordered by typical call sequence. */ + tools: readonly string[] +} + +export const RECOMMENDED_WORKFLOW_LOADOUTS: readonly WorkflowLoadout[] = [ + { + workflow: 'categorize_month', + description: 'Categorize and book a month of bank transactions.', + skill: 'bank-reconciliation', + tools: [ + 'gnubok_list_uncategorized_transactions', + 'gnubok_suggest_categories', + 'gnubok_categorize_transaction', + 'gnubok_match_transaction_to_invoice', + 'gnubok_load_skill', + 'gnubok_approve_pending_operation', + ], + }, + { + workflow: 'close_period', + description: 'Reconcile, document voucher gaps, and lock a fiscal period.', + skill: 'month-end-close', + tools: [ + 'gnubok_list_fiscal_periods', + 'gnubok_list_uncategorized_transactions', + 'gnubok_get_reconciliation_status', + 'gnubok_list_voucher_gaps', + 'gnubok_explain_voucher_gap', + 'gnubok_lock_period', + 'gnubok_approve_pending_operation', + ], + }, + { + workflow: 'invoice_run', + description: 'Create and send customer invoices.', + skill: 'invoicing-rules', + tools: [ + 'gnubok_list_customers', + 'gnubok_create_customer', + 'gnubok_list_articles', + 'gnubok_create_invoice', + 'gnubok_send_invoice', + 'gnubok_mark_invoice_as_sent', + 'gnubok_approve_pending_operation', + ], + }, + { + workflow: 'vat_declaration', + description: 'Compute, review, and file the VAT declaration.', + skill: 'quarterly-vat-review', + tools: [ + 'gnubok_get_vat_report', + 'gnubok_vat_close_check', + 'gnubok_get_general_ledger', + 'gnubok_vat_declaration_validate', + 'gnubok_vat_declaration_submit', + 'gnubok_vat_declaration_status', + 'gnubok_approve_pending_operation', + ], + }, + { + workflow: 'payroll_month', + description: 'Run monthly payroll and generate the AGI.', + skill: 'payroll-monthly', + tools: [ + 'gnubok_list_employees', + 'gnubok_create_salary_run', + 'gnubok_calculate_salary_run', + 'gnubok_get_salary_run', + 'gnubok_book_salary_run', + 'gnubok_generate_agi', + 'gnubok_approve_pending_operation', + ], + }, +] + +/** + * Fails fast when a loadout references a tool or workflow skill that does not + * exist. Called at module init in server.ts (after the tools array is built) + * so any rename/removal in the registry breaks the build and the test suite + * immediately instead of shipping a briefing that recommends phantom tools. + */ +export function assertRecommendedLoadoutsValid(knownToolNames: ReadonlySet): void { + const knownSkillSlugs = new Set(workflowSkills.map((s) => s.slug)) + const seenWorkflows = new Set() + for (const loadout of RECOMMENDED_WORKFLOW_LOADOUTS) { + if (seenWorkflows.has(loadout.workflow)) { + throw new Error( + `recommended_tools: duplicate workflow key "${loadout.workflow}" in RECOMMENDED_WORKFLOW_LOADOUTS.` + ) + } + seenWorkflows.add(loadout.workflow) + if (!knownSkillSlugs.has(loadout.skill)) { + throw new Error( + `recommended_tools: workflow "${loadout.workflow}" references unknown skill slug "${loadout.skill}". ` + + 'Update RECOMMENDED_WORKFLOW_LOADOUTS in recommended-tools.ts.' + ) + } + for (const toolName of loadout.tools) { + if (!knownToolNames.has(toolName)) { + throw new Error( + `recommended_tools: workflow "${loadout.workflow}" references unknown tool "${toolName}". ` + + 'Update RECOMMENDED_WORKFLOW_LOADOUTS in recommended-tools.ts when renaming or removing tools.' + ) + } + } + } +} diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index 68f6737f..d271b89f 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -36,6 +36,7 @@ import { buildLedgerContext } from '@/lib/agent-context/ledger-context' import { prompts, findPrompt } from './prompts' import { findSkill, loadAllSkills, toSummary, SKILL_MIME_TYPE, SKILL_URI_PREFIX, skillUri, skillSlugFromUri } from './skills' import type { SkillTier } from './skills' +import { RECOMMENDED_WORKFLOW_LOADOUTS, assertRecommendedLoadoutsValid } from './recommended-tools' import { getRiskLevel } from '@/lib/pending-operations/risk-tiers' import { normalizeVatRateToDecimal } from '@/lib/vat/supplier-invoice-line-checks' import { CreateSupplierParamsSchema } from '@/lib/pending-operations/schemas/create-supplier' @@ -2534,7 +2535,7 @@ export const tools: McpTool[] = [ { name: 'gnubok_get_agent_briefing', title: 'Get Agent Briefing', - description: 'Bootstrap this company\'s accountant context in one call: user_name, profile_summary, loaded atoms (metadata only, gnubok_load_skill for bodies), top-30 active memories, dimensions snapshot (when registered). Call once at session start.', + description: 'Bootstrap this company\'s accountant context in one call: user_name, profile_summary, atoms (gnubok_load_skill for bodies), top-30 memories, dimensions, and recommended_tools: per-workflow loadouts to batch-load in one ToolSearch select:a,b,c call. Call once at session start.', inputSchema: { type: 'object', additionalProperties: false, @@ -2708,8 +2709,28 @@ export const tools: McpTool[] = [ }, required: ['resource_uri', 'window_from', 'posted_entries_window', 'top_counterparty_patterns', 'top_supplier_patterns'], }, + recommended_tools: { + type: 'array', + 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'], + }, + }, }, - required: ['company', 'user_name', 'profile_summary', 'atoms', 'memory'], + required: ['company', 'user_name', 'profile_summary', 'atoms', 'memory', 'recommended_tools'], }, annotations: { readOnlyHint: true, @@ -2979,6 +3000,15 @@ export const tools: McpTool[] = [ })), ...(dimensionsBlock ? { dimensions: dimensionsBlock } : {}), ...(ledgerDigest ? { ledger_context: ledgerDigest } : {}), + // Static per-workflow loadouts (issue #1098): lets a deferred-loading + // harness batch-load a whole workflow cluster in one call. Validated + // against the tool registry at module init (assertRecommendedLoadoutsValid). + recommended_tools: RECOMMENDED_WORKFLOW_LOADOUTS.map((w) => ({ + workflow: w.workflow, + description: w.description, + skill: w.skill, + tools: [...w.tools], + })), } }, }, @@ -13571,6 +13601,13 @@ export const tools: McpTool[] = [ }, ] +// Drift guard for the gnubok_get_agent_briefing recommended_tools loadouts: +// every referenced tool must exist in the registry above and every referenced +// skill must be a real workflow skill. Runs at module init so a rename or +// removal fails the build (and every test importing this module) instead of +// shipping a briefing that recommends phantom tools. +assertRecommendedLoadoutsValid(new Set(tools.map((t) => t.name))) + // ── MCP Protocol Handler ───────────────────────────────────── const SERVER_INFO = { @@ -13954,6 +13991,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.', + '• 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}); 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.', '• When the user asks "how do I do X" or you\'re unsure of the correct sequence (month-end close, VAT review, year-end, invoicing, payroll), call gnubok_list_skills first: domain workflows are documented as loadable skills with tool references.',