From 073c5d47af4254f267c7cf56705092022635019e Mon Sep 17 00:00:00 2001 From: Ricardo Canales Date: Mon, 15 Jun 2026 22:52:42 -0700 Subject: [PATCH 1/6] =?UTF-8?q?refactor(mcp):=20consolidate=20tool=20surfa?= =?UTF-8?q?ce=2043=20=E2=86=92=2027?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fold near-duplicate / sugar MCP tools into their compound siblings so an agent reaches for one obvious tool per entity, and the registry stays small. - metadata: set/remove/replace/clear_transaction_metadata → one set_transaction_metadata (set merge / unset / replace). - series: set_series_type + add/remove_series_tag → fields on update_series (type, tags_to_add, tags_to_remove); each sub-change reuses the same service method, so collision guards / sticky type / tag provenance are unchanged. - flag: flag_transaction + unflag_transaction → flagged *bool on an update_transactions op (reason rides in the op's comment). Adds a flag step to runUpdateOpInTx so it's atomic with the rest of the op. - count: count_transactions → query_transactions(count_only=true). - rules: batch_create_rules → create_transaction_rule now takes a rules[] of 1..100 specs (each may apply_retroactively). - reference: get_overview / list_accounts / list_categories / list_users / list_tags / get_sync_status / list_transaction_rules → one get_reference (kind=…) that dispatches to the existing per-kind handlers. Filtered rule analysis stays in query_transaction_rules. Updated the registry-scope contract test, rewrote the flag integration harness to drive update_transactions, and refreshed mcp-tools-reference, mcp-server, data-model, rule-dsl, api-reference, and the .claude/rules/mcp rule. Build + vet + unit + mcp/service integration all green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/rules/mcp.md | 19 +- docs/api-reference.md | 4 +- docs/data-model.md | 4 +- docs/mcp-server.md | 102 ++----- docs/mcp-tools-reference.md | 90 +++---- docs/rule-dsl.md | 2 +- ...ag_tool_response_shape_integration_test.go | 226 +++++----------- internal/mcp/server.go | 100 ++----- internal/mcp/server_test.go | 18 +- internal/mcp/tools.go | 251 +++++++----------- internal/mcp/tools_flag.go | 54 ---- internal/mcp/tools_reads.go | 35 +++ internal/mcp/tools_series.go | 117 +++----- internal/mcp/tools_transaction_metadata.go | 113 ++++---- internal/mcp/tools_update_transactions.go | 4 +- internal/service/update_transactions.go | 24 ++ 16 files changed, 420 insertions(+), 743 deletions(-) delete mode 100644 internal/mcp/tools_flag.go diff --git a/.claude/rules/mcp.md b/.claude/rules/mcp.md index c3ce10c66..9890fb53f 100644 --- a/.claude/rules/mcp.md +++ b/.claude/rules/mcp.md @@ -81,18 +81,29 @@ The "review queue" is just transactions tagged `needs-review`. A seeded system r Agents follow a uniform loop: `query_transactions(tags=["needs-review"])` to find work, `update_transactions(operations=[…])` to set category + remove the tag (and pair the change with a `comment` for the audit trail) atomically per transaction. Max 50 ops per call. -`update_transactions` is the universal per-row write — tag adds, tag removes, category sets, category resets (`reset_category: true`), and comments all flow through it. The bare-row and bulk variants (`add_transaction_tag`, `remove_transaction_tag`, `categorize_transaction`, `reset_transaction_category`, `add_transaction_comment`, `bulk_recategorize`, `batch_categorize_transactions`) were collapsed into it during the MCP overhaul. +`update_transactions` is the universal per-row write — tag adds, tag removes, category sets, category resets (`reset_category: true`), comments, and flag/unflag (`flagged: true|false`) all flow through it. The bare-row and bulk variants (`add_transaction_tag`, `remove_transaction_tag`, `categorize_transaction`, `reset_transaction_category`, `add_transaction_comment`, `bulk_recategorize`, `batch_categorize_transactions`, and the standalone `flag_transaction` / `unflag_transaction`) were collapsed into it. Annotations are read via `list_annotations`. Tag *vocabulary* admin (introducing, renaming, deleting tag definitions) goes through `create_tag`, `update_tag`, `delete_tag`. -## Reference data: dual surface (resources + tool mirrors) +## Consolidated write tools + +Several writes are deliberately compound so the registry stays small and an agent reaches for one obvious tool per entity: + +- `update_transactions` — see above (category, tags, comment, flag). +- `set_transaction_metadata` — the single op over the free-form metadata JSONB column: `set` (merge keys), `unset` (delete keys), `replace:true` (swap/clear the whole blob). Absorbed `remove_`/`replace_`/`clear_transaction_metadata`. +- `update_series` — edits a recurring series' attributes **plus** `type` (absorbed `set_series_type`) and tag membership via `tags_to_add` / `tags_to_remove` (absorbed `add_series_tag` / `remove_series_tag`). Each sub-change still calls the same service method, so the semantics (collision guards, sticky type, tag provenance) are unchanged. +- `create_transaction_rule` — takes a `rules` array of 1..N specs (absorbed `batch_create_rules`); each spec may carry `apply_retroactively`. + +When folding a tool, prefer reusing the existing service methods from the compound handler over re-implementing the write — the MCP layer is where the consolidation lives. + +## Reference data: dual surface (resources + one tool) Bounded reference data is exposed two ways: - **Resources (preferred)** — `breadbox://overview`, `://accounts`, `://categories`, `://tags`, `://users`, `://rules`, `://sync-status`. Surfaced in Claude.ai's paperclip menu and the Inspector resource picker. Application-driven, user-controlled. -- **Tool mirrors (compat)** — `get_overview`, `list_accounts`, `list_categories`, `list_tags`, `list_users`, `list_transaction_rules`, `get_sync_status`. Same payload, called as tools. Kept because not every MCP client implements the resources/* methods — without these, those clients can't read this data at all. +- **`get_reference` tool (compat)** — a single tool that dispatches on `kind` (`overview` \| `accounts` \| `categories` \| `tags` \| `users` \| `sync_status` \| `rules`) to the same per-kind handler the matching resource uses. Kept because not every MCP client implements the resources/* methods. Optional `user_id` (accounts) and `fields` (rules) ride through. Filtered/sorted rule analysis stays in `query_transaction_rules`; `get_reference kind=rules` returns the lean roster. -Both surfaces share the same service-layer call path (no logic duplication), so payload shape stays in sync. When adding a new bounded reference resource, register both: a resource handler in `resources.go` and a tool mirror in `tools_reads.go`. +Both surfaces share the same service-layer call path (no logic duplication), so payload shape stays in sync. The per-kind handlers (`handleGetOverview`, `handleListAccounts`, …) still live in `tools_reads.go` — `get_reference` dispatches to them. When adding a new bounded reference resource, add a resource handler in `resources.go` and a new `kind` branch in `handleGetReference`. ## Resource templates (drill-downs) diff --git a/docs/api-reference.md b/docs/api-reference.md index 865f5bfa9..6c43adabc 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -927,7 +927,7 @@ Rules auto-categorize transactions during sync by matching conditions on transac | GET | `/rules/{id}` | Read | Get a single rule | | GET | `/rules/{id}/sync-history` | Read | Last N sync runs that triggered this rule (default 10, max 100) | | POST | `/rules` | Write | Create a rule | -| POST | `/rules/batch` | Write | Bulk-create rules (mirrors MCP `batch_create_rules`, max 50 per call) | +| POST | `/rules/batch` | Write | Bulk-create rules (mirrors the MCP `create_transaction_rule` `rules` array, max 50 per call) | | PUT | `/rules/{id}` | Write | Update a rule | | DELETE | `/rules/{id}` | Write | Delete a rule | | POST | `/rules/{id}/apply` | Write | Apply a single rule retroactively (skips `category_override='user'` rows) | @@ -936,7 +936,7 @@ Rules auto-categorize transactions during sync by matching conditions on transac `POST /rules/batch` and the apply endpoints both have integration coverage in `internal/api/rules_batch_integration_test.go` and `internal/api/rules_apply_integration_test.go`. The apply tests assert the `category_override='user'` sacred-cow contract: a manually-overridden transaction is never recategorized by retroactive apply, even when its conditions match. -`POST /rules/batch` request body mirrors the MCP `batch_create_rules` shape: +`POST /rules/batch` request body mirrors the MCP `create_transaction_rule` `rules`-array shape: ```json { diff --git a/docs/data-model.md b/docs/data-model.md index 4f93260b2..8ff7cd607 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -800,7 +800,7 @@ CHECK (actor_type IN ('user', 'agent', 'system')) | `iso_currency_code` | `TEXT` | Yes | `NULL` | Currency for the amounts. Part of the dedup signature. | | `category_id` | `UUID` | Yes | `NULL` | FK → `categories(id)`. SET NULL on delete. Advisory suggested category. | | `status` | `TEXT` | No | `'active'` | `active`/`paused`/`cancelled`/`candidate`. Changed via verdicts, not raw edits. | -| `type` | `TEXT` | No | `'subscription'` | `subscription`/`bill`/`loan`/`other`. Inferred from members' dominant category at first detection; sticky override via `set_series_type`. | +| `type` | `TEXT` | No | `'subscription'` | `subscription`/`bill`/`loan`/`other`. Inferred from members' dominant category at first detection; sticky override via `update_series` (`type`). | | `detection_source` | `TEXT` | No | `'deterministic'` | `deterministic`/`agent`/`user`/`rule` — who last shaped the row. The precedence ladder (deterministic < rule < agent < user) protects higher-ranked writes from being clobbered by re-detection. | | `confidence` | `TEXT` | No | `'auto'` | `auto` (unreviewed candidate) / `confirmed` (adjudicated, fields frozen to rollups) / `rejected` (sticky — never re-proposed at this signature). | | `confirmed_by_type` | `TEXT` | Yes | `NULL` | `user` or `agent` — who adjudicated. A user's confirmation outranks a later agent write. | @@ -854,7 +854,7 @@ There is **no** unique constraint on the dedup signature `(merchant_key, iso_cur #### Field ownership (edit surface) -User/agent-editable (via `PATCH /series/{id}` / `update_series`, protected from re-detection): `name`, `expected_amount` (+ `iso_currency_code`, `amount_tolerance`), `cadence`, `expected_day`, `category_id`, `user_id`. `type` is edited via `set_series_type`; `merchant_key` via `rekey_series`; `status`/`confidence` via verdicts (`review_series`). Detector-owned (read-only): `detection_source`, `last_amount`, `last_seen_date`, `occurrence_count`, `detection_signals`. Derived (read-only): `next_expected_date`. +User/agent-editable (via `PATCH /series/{id}` / `update_series`, protected from re-detection): `name`, `expected_amount` (+ `iso_currency_code`, `amount_tolerance`), `cadence`, `expected_day`, `category_id`, `user_id`, `type`, and tag membership (`tags_to_add`/`tags_to_remove`). `merchant_key` is edited via `rekey_series`; `status`/`confidence` via verdicts (`review_series`). Detector-owned (read-only): `detection_source`, `last_amount`, `last_seen_date`, `occurrence_count`, `detection_signals`. Derived (read-only): `next_expected_date`. --- diff --git a/docs/mcp-server.md b/docs/mcp-server.md index fe30a1785..22d3d5710 100644 --- a/docs/mcp-server.md +++ b/docs/mcp-server.md @@ -225,7 +225,9 @@ On error, the MCP SDK's `IsError: true` flag is set and the content is: --- -### Tool: `list_accounts` +### Tool: `get_reference(kind=accounts)` + +Bank accounts are read via `get_reference(kind=accounts)` (optional `user_id` filter) — one of the bounded reference datasets behind the single `get_reference` tool. There is no standalone `list_accounts` tool. **Description (shown to LLM):** List all bank accounts with their current balances. Optionally filter by family member. Returns one object per account including institution, type, and balance. @@ -302,7 +304,7 @@ Calls `GET /api/v1/accounts` with the optional `user_id` query parameter. Return ### Tool: `query_transactions` -**Description (shown to LLM):** Search and filter transactions. Supports date ranges, account, category, amount bounds, free-text search, and pending status. Results are cursor-paginated — default 100 per page, maximum 500. Use `count_transactions` first to understand result size before querying. +**Description (shown to LLM):** Search and filter transactions. Supports date ranges, account, category, amount bounds, free-text search, and pending status. Results are cursor-paginated — default 100 per page, maximum 500. Pass `count_only: true` first to understand result size before querying. #### Input Schema @@ -389,7 +391,7 @@ When there are no more pages: Each transaction object is approximately 50 tokens of JSON. At the default page size of 100, a single call returns roughly **5,000 tokens of content** before any surrounding context. At the maximum page size of 500, a single call can return **25,000 tokens**. The LLM should: -1. Call `count_transactions` first when the result size is unknown. +1. Call `query_transactions({ ..., count_only: true })` first when the result size is unknown. 2. Apply date, category, and other filters to narrow results before paginating. 3. Prefer smaller `limit` values (25–50) when only a sample or aggregate is needed. 4. Only paginate through all results when the task genuinely requires complete data. @@ -408,71 +410,21 @@ Calls `GET /api/v1/transactions` with all provided filters as query parameters. --- -### Tool: `count_transactions` - -**Description (shown to LLM):** Count matching transactions without returning any transaction data. Accepts the same filters as `query_transactions`. Use this before calling `query_transactions` to understand how many results to expect and decide whether to add more filters or paginate. - -#### Input Schema - -Same as `query_transactions` **minus** `cursor` and `limit`. All parameters are optional. - -| Parameter | Type | Required | Description | -|---|---|---|---| -| `start_date` | string (YYYY-MM-DD) | No | Count transactions on or after this date. | -| `end_date` | string (YYYY-MM-DD) | No | Count transactions on or before this date. | -| `account_id` | string | No | Filter to a specific account. | -| `user_id` | string | No | Filter to accounts owned by this family member. | -| `category` | string | No | Filter by primary category. | -| `category_detailed` | string | No | Filter by detailed subcategory. | -| `min_amount` | number \| null | No | Minimum transaction amount. Zero is a valid filter value. | -| `max_amount` | number \| null | No | Maximum transaction amount. Zero is a valid filter value. | -| `pending` | boolean | No | Filter by pending status. | -| `search` | string | No | Full-text search over merchant name and description. | - -#### Example Input - -```json -{ - "start_date": "2025-01-01", - "end_date": "2025-01-31" -} -``` - -#### Output Format - -```json -{ "count": 347 } -``` - -#### Mapping to REST API - -Calls `GET /api/v1/transactions/count` with the same filter query parameters supported by `query_transactions`. The endpoint executes a `COUNT(*)` query rather than selecting rows, which is efficient even for large datasets — it does not load transaction data into memory. +### Counting transactions (`query_transactions` `count_only`) -#### Recommended Agent Workflow +Counting is folded into `query_transactions`: pass `count_only: true` with the same filters and it returns just `{ "count": N }` — no rows, no pagination (`cursor`/`limit`/`sort_*`/`fields` are ignored). There is no separate `count_transactions` tool. ``` -1. count_transactions({ start_date, end_date }) → { count: 347 } - - If count < 200: proceed to query_transactions directly - - If count >= 200: add more filters (category, account_id, etc.) and re-count - - If still large: query_transactions with pagination, processing page by page - -2. query_transactions({ start_date, end_date, category: "FOOD_AND_DRINK" }) - → { data: [...100 txns], next_cursor: "...", has_more: true } - -3. query_transactions({ ..., cursor: "..." }) - → { data: [...47 txns], next_cursor: null, has_more: false } +1. query_transactions({ start_date, end_date, count_only: true }) → { count: 347 } + - If small: re-call without count_only to fetch rows. + - If large: add filters (category_slug, account_id, …) and re-count, then paginate. ``` -#### Edge Cases - -- **No matching transactions:** Returns `{ "count": 0 }`. -- **Invalid parameters:** Returns an error in the same format as `query_transactions`. - --- -### Tool: `list_users` +### Tool: `get_reference(kind=users)` -**Description (shown to LLM):** List all family members tracked in Breadbox. Users are labels for account ownership — they are not login accounts. Use the returned IDs to filter `list_accounts` or `query_transactions` by family member. +**Description (shown to LLM):** List all family members tracked in Breadbox. Users are labels for account ownership — they are not login accounts. Read them via `get_reference(kind=users)`; use the returned IDs to filter accounts (`get_reference(kind=accounts)`) or `query_transactions` by family member. #### Input Schema @@ -513,7 +465,9 @@ Calls `GET /api/v1/users`. Returns all users without pagination (family sizes ar --- -### Tool: `get_sync_status` +### Tool: `get_reference(kind=sync_status)` + +Connection sync status is read via `get_reference(kind=sync_status)` — one of the bounded reference datasets behind the single `get_reference` tool. There is no standalone `get_sync_status` tool. **Description (shown to LLM):** Returns the health status of all bank connections — whether they are syncing successfully, when they last synced, and whether any connections need re-authentication. Use this to diagnose why transactions might be missing or stale. @@ -577,7 +531,7 @@ Calls `GET /api/v1/connections`. Each element in the response corresponds to one ### Tool: `trigger_sync` -**Description (shown to LLM):** Manually trigger a data sync to fetch the latest transactions and balances from the bank. Syncs all connections by default, or a specific connection if `connection_id` is provided. The sync runs asynchronously — this tool returns immediately after enqueuing the sync, not after it completes. Check `get_sync_status` afterward to monitor progress. +**Description (shown to LLM):** Manually trigger a data sync to fetch the latest transactions and balances from the bank. Syncs all connections by default, or a specific connection if `connection_id` is provided. The sync runs asynchronously — this tool returns immediately after enqueuing the sync, not after it completes. Check `get_reference(kind=sync_status)` afterward to monitor progress. #### Input Schema @@ -613,13 +567,15 @@ Calls `POST /api/v1/sync` with an optional `connection_id` body parameter. The R - **Unknown `connection_id`:** Returns an error: `{ "error": "Connection conn_xxx not found." }` with `IsError: true`. - **Sync already in progress:** The service layer handles deduplication. A second trigger for the same connection while one is running is a no-op; the tool returns a success message indicating the sync was already underway. -- **Connection in `error` state:** The sync is attempted. If re-authentication is required, the sync will fail again and `get_sync_status` will reflect the error. +- **Connection in `error` state:** The sync is attempted. If re-authentication is required, the sync will fail again and `get_reference(kind=sync_status)` will reflect the error. --- -### Tool: `list_categories` +### Tool: `get_reference(kind=categories)` + +The category taxonomy is read via `get_reference(kind=categories)` — one of the bounded reference datasets behind the single `get_reference` tool. There is no standalone `list_categories` tool. -**Description (shown to LLM):** List all distinct transaction category pairs (primary + detailed) that exist in the database. Useful for understanding the category taxonomy before filtering transactions. +**Description (shown to LLM):** List all transaction categories. Useful for understanding the category taxonomy before filtering transactions. #### Input Schema @@ -760,7 +716,7 @@ The standard pattern for any transaction-based task: ``` Step 1: Establish scope - → count_transactions({ start_date: "2025-01-01", end_date: "2025-01-31" }) + → query_transactions({ start_date: "2025-01-01", end_date: "2025-01-31", count_only: true }) ← { count: 347 } Step 2: Evaluate @@ -768,8 +724,8 @@ Step 2: Evaluate - count >= 200 → narrow filters, or plan multi-page processing Step 3a: Narrow filters (if count is large) - → count_transactions({ start_date: "2025-01-01", end_date: "2025-01-31", - category: "FOOD_AND_DRINK" }) + → query_transactions({ start_date: "2025-01-01", end_date: "2025-01-31", + category_slug: "food_and_drink", count_only: true }) ← { count: 52 } → query_transactions({ start_date: "2025-01-01", end_date: "2025-01-31", category: "FOOD_AND_DRINK" }) @@ -823,13 +779,13 @@ Both paths call the same `service.QueryTransactions` function. The MCP tool hand | MCP Tool | REST Endpoint | |---|---| -| `list_accounts` | `GET /api/v1/accounts` | +| `get_reference(kind=accounts)` | `GET /api/v1/accounts` | | `query_transactions` | `GET /api/v1/transactions` | -| `count_transactions` | `GET /api/v1/transactions/count` | -| `list_users` | `GET /api/v1/users` | -| `get_sync_status` | `GET /api/v1/connections` | +| `query_transactions(count_only=true)` | `GET /api/v1/transactions/count` | +| `get_reference(kind=users)` | `GET /api/v1/users` | +| `get_reference(kind=sync_status)` | `GET /api/v1/connections` | | `trigger_sync` | `POST /api/v1/sync` | -| `list_categories` | `GET /api/v1/categories` | +| `get_reference(kind=categories)` | `GET /api/v1/categories` | ### No MCP-Specific Data Access diff --git a/docs/mcp-tools-reference.md b/docs/mcp-tools-reference.md index 70294ea59..c32080567 100644 --- a/docs/mcp-tools-reference.md +++ b/docs/mcp-tools-reference.md @@ -50,10 +50,7 @@ Query transactions with composable filters and cursor pagination. | `fields` | string | Field selection. Aliases: `minimal`, `core`, `category`, `timestamps`. Omitted → `core,category` (lean default); `all` → every field. `id` always included. | | `cursor` | string | Pagination cursor | | `limit` | int | Results per page (default 50, max 500) | - -### count_transactions (Read) - -Count transactions matching filters. Same filter parameters as `query_transactions`. Returns count only — use before paginating large result sets. +| `count_only` | bool | When true, return just `{ "count": N }` for the same filters — no rows, no pagination. `cursor`/`limit`/`sort_*`/`fields` ignored. Use before paginating large result sets, or to compare counts across ranges. | ### transaction_summary (Read) @@ -72,25 +69,24 @@ Aggregated spending totals. Default date range: 30 days. ## Account & Status Tools -### list_accounts (Read) - -List all bank accounts across all connections. Returns account name, type, balances, connection info. - -### list_users (Read) - -List all family members in the household. - -### get_sync_status (Read) - -Get sync status for all connections — last sync time, status, errors. +### get_reference (Read) -### list_categories (Read) +One tool that reads any bounded reference dataset by `kind` — the single mirror of the `breadbox://` reference resources, for clients that don't support MCP resources. (Folds the former `get_overview`, `list_accounts`, `list_categories`, `list_tags`, `list_users`, `get_sync_status`, and `list_transaction_rules` tools.) -List all categories in the 2-level hierarchy. Returns slug, display name, parent, icon, color. - -### export_categories (Read) - -Export the full category tree as TSV. Useful for backup or transfer. +| Parameter | Type | Description | +|-----------|------|-------------| +| `kind` | string | **Required.** One of `overview`, `accounts`, `categories`, `tags`, `users`, `sync_status`, `rules`. | +| `user_id` | string | Only for `kind=accounts`: scope to one household member. | +| `fields` | string | Only for `kind=rules`: `summary` (default, omits conditions/actions) or `all`. | + +Per kind: +- `overview` — household snapshot (scope, freshness, pending-review backlog). Read once at the top of a session. +- `accounts` — bank accounts with name, type, balances, connection info. +- `categories` — the 2-level category hierarchy (slug, display name, parent, icon, color). +- `tags` — the registered tag vocabulary. +- `users` — household members; the `short_id` is the `user_id` on filters. +- `sync_status` — per-connection last-sync time, status, errors. +- `rules` — the transaction-rule roster (lean summary; `fields=all` for full definitions). For filtered/sorted rule analysis use `query_transaction_rules`. ### list_workflows (Read) @@ -152,7 +148,7 @@ Import categories from TSV format. Creates or updates categories. ### list_tags (Read) -List all registered tags. +The tag vocabulary is read via `get_reference(kind=tags)` (or the `breadbox://tags` resource). ### add_transaction_tag (Write) @@ -176,7 +172,7 @@ Remove a tag from a transaction. An optional `note` lands on the `tag_removed` a ### update_transactions (Write) -Compound batch write — set category, add tags, remove tags, and attach a comment per transaction in a single atomic call. Max 50 operations per request. +Compound batch write — set category, add tags, remove tags, attach a comment, and flag/unflag per transaction in a single atomic call. Max 50 operations per request. | Parameter | Type | Description | |-----------|------|-------------| @@ -191,7 +187,8 @@ Each operation: | `category_slug` | string | Optional category to set. Sets `category_override='user'`. | | `tags_to_add` | array | `[{slug, note?}, ...]`. Auto-creates tags if the slug is new. | | `tags_to_remove` | array | `[{slug, note?}, ...]`. `note` is optional — if provided, lands on the `tag_removed` annotation. | -| `comment` | string | Optional comment annotation. | +| `comment` | string | Optional comment annotation. When flagging, put the flag reason here. | +| `flagged` | bool | Optional. `true` flags the transaction for human attention (sets `flagged_at`); `false` clears the flag. Omit to leave it untouched. Retrieve flagged rows with `query_transactions(flagged=true)`. Folds the former `flag_transaction` / `unflag_transaction` tools. | Use this to close a review entry: `set category + remove needs-review (with note) + comment` in one call. @@ -246,7 +243,7 @@ Admin-only tag CRUD. Agents typically don't need these — `add_transaction_tag` ### list_series (Read) -List detected recurring series. Optional `status` filter (`active` | `candidate` | `paused` | `cancelled`). **Lean by default** (`fields` omitted → `overview` projection): each row carries `type` (`subscription` | `bill` | `loan` | `other` — inferred from category, set via `set_series_type`), `cadence`, `expected_amount` + `iso_currency_code` (never sum across currencies), `next_expected_date`, `occurrence_count`, and `confidence` (`auto` | `confirmed` | `rejected`). Active series also carry a derived `renewal_health` (`active` | `due_soon` | `overdue` | `stale` | `unknown`) and signed `days_until_renewal` (negative = overdue) so you can answer "what renews soon" and "what looks cancelled" without re-deriving cadence math — `stale` means a full cadence cycle elapsed past the expected charge. The verbose `detection_signals` evidence is **omitted** from the lean list — pass `fields=all`, or use `get_series` for one series' full detail. Read `status=candidate` to find series awaiting a verdict. +List detected recurring series. Optional `status` filter (`active` | `candidate` | `paused` | `cancelled`). **Lean by default** (`fields` omitted → `overview` projection): each row carries `type` (`subscription` | `bill` | `loan` | `other` — inferred from category, corrected via `update_series`), `cadence`, `expected_amount` + `iso_currency_code` (never sum across currencies), `next_expected_date`, `occurrence_count`, and `confidence` (`auto` | `confirmed` | `rejected`). Active series also carry a derived `renewal_health` (`active` | `due_soon` | `overdue` | `stale` | `unknown`) and signed `days_until_renewal` (negative = overdue) so you can answer "what renews soon" and "what looks cancelled" without re-deriving cadence math — `stale` means a full cadence cycle elapsed past the expected charge. The verbose `detection_signals` evidence is **omitted** from the lean list — pass `fields=all`, or use `get_series` for one series' full detail. Read `status=candidate` to find series awaiting a verdict. ### get_series (Read) @@ -266,11 +263,12 @@ Create a recurring series detection missed, or link transactions to an existing ### update_series (Write) -Edit a recurring series' user-owned attributes: `name`, `expected_amount` (+ `currency`, `amount_tolerance`), `cadence`, `expected_day`, `category_id`, `user_id` (owner). Every field is optional — omit to leave unchanged. This is a deliberate override, **not** a detection proposal: it bypasses the source-precedence ladder and protects the edited values from being reverted by the next sync's re-detect. Editing `cadence` re-derives `next_expected_date`; changing `currency` or `user_id` is collision-guarded (they're part of the dedup signature, so an edit can't silently merge two series). Use `review_series` for `confirm`/`pause`/`cancel`, `set_series_type` for the type axis, and `rekey_series` for the `merchant_key` — those have their own semantics and are not editable here. +Edit a recurring series' user-owned attributes: `name`, `expected_amount` (+ `currency`, `amount_tolerance`), `cadence`, `expected_day`, `category_id`, `user_id` (owner), `type`, and tag membership (`tags_to_add` / `tags_to_remove`). Every field is optional — omit to leave unchanged. This is a deliberate override, **not** a detection proposal: it bypasses the source-precedence ladder and protects the edited values from being reverted by the next sync's re-detect. -### set_series_type (Write) +- `type` — `subscription` (streaming/SaaS/memberships), `bill` (rent/utilities/insurance/telecom), `loan` (mortgage/auto/student/personal), or `other`. The detector infers it from the charges' dominant category at first detection; setting it here is a **sticky** override that re-detection won't revert. (Folds the former `set_series_type`; `assign_series` also accepts `type` when minting.) +- `tags_to_add` / `tags_to_remove` — slugs (must already exist; create with `create_tag`). An added tag is materialized onto every linked charge and applied to future members; removing one strips the series-inherited copies (a tag a user added directly to a charge survives). (Folds the former `add_series_tag` / `remove_series_tag`.) -Set a recurring series' `type`: `subscription` (streaming/SaaS/memberships), `bill` (rent/utilities/insurance/telecom), `loan` (mortgage/auto/student/personal), or `other`. The detector infers the type from the linked charges' dominant category at first detection; this is the correction handle. The override is **sticky** — re-detection won't revert it. (`assign_series` also accepts an optional `type` when minting.) +Editing `cadence` re-derives `next_expected_date`; changing `currency` or `user_id` is collision-guarded (they're part of the dedup signature, so an edit can't silently merge two series). Use `review_series` for `confirm`/`pause`/`cancel` and `rekey_series` for the `merchant_key` — those have their own semantics and are not editable here. ### rekey_series (Write) @@ -284,13 +282,7 @@ Break an over-grouped series in two: move `transaction_ids` (≤50, each a curre Detach `transaction_ids` (≤50, each a current member) from a recurring series — the inverse of `assign_series`' link path. Clears each charge's `series_id`, strips the series' inherited tags from them (a tag the user added directly survives), and recomputes the series' rollups + `next_expected_date`. Errors if any listed transaction isn't a current member, so it can't silently no-op or touch another series. Use to remove a charge the detector wrongly swept in; use `split_series` instead when the stray charges form their own series. -### add_series_tag (Write) - -Attach an existing tag to a recurring series. The tag is materialized onto every linked transaction (they inherit it) and applied to future members as they join — so tagging the Netflix series tags all its charges. The tag must already exist (create it with `create_tag` first). Returns the updated series (including its `tags`). - -### remove_series_tag (Write) - -Detach a tag from a recurring series and strip the series-inherited copies from its linked transactions. Provenance-scoped: a tag a user added directly to a transaction survives. +> Series **type** and **tag** edits fold into `update_series` (`type`, `tags_to_add`, `tags_to_remove`) — there are no standalone `set_series_type` / `add_series_tag` / `remove_series_tag` tools. --- @@ -302,33 +294,31 @@ The tools below are thin API skins over the DSL — the DSL doc is the source of ### create_transaction_rule (Write) -Create a rule that fires during sync. Actions compose (`set_category` + `add_tag` + `add_comment` in a single rule are all valid). +Create one or more rules that fire during sync. Pass `rules`: an array of 1..100 rule specs (a single rule is a one-element array). Authoring a chained pipeline in one call orders rules by stage so earlier-stage tag/category writes feed later-stage conditions. Actions compose within a rule (`set_category` + `add_tag` + `add_comment` are all valid). (Folds the former `batch_create_rules`.) | Parameter | Type | Description | |-----------|------|-------------| +| `rules` | array | **Required.** 1..100 rule specs (fields below). | + +Each rule spec: + +| Field | Type | Description | +|-------|------|-------------| | `name` | string | Human-readable rule name | | `conditions` | object | Condition tree. Omit or `{}` for match-all. Supports `and` / `or` / `not` nesting up to depth 10. | | `actions` | array | Typed actions: `set_category`, `add_tag`, `remove_tag`, `add_comment`. Either this or `category_slug` is required. | | `category_slug` | string | Shorthand for `actions=[{type:set_category,category_slug:...}]` | | `trigger` | string | `on_create` (default) / `on_change` / `always`. `on_update` accepted as legacy alias. | | `stage` | string | **Preferred.** Semantic pipeline stage: `baseline` / `standard` / `refinement` / `override`. Resolves to priority `0 / 10 / 50 / 100`. | -| `priority` | int | Raw pipeline-stage integer, 0–1000. Use for fine-grained slotting within a stage. If both `stage` and `priority` are supplied, `priority` wins. Defaults to `10` (standard) if neither is provided. | -| `enabled` | bool | Default true | +| `priority` | int | Raw pipeline-stage integer, 0–1000. If both `stage` and `priority` are supplied, `priority` wins. Defaults to `10` (standard). | | `expires_in` | string | Optional duration (e.g., `24h`, `30d`, `1w`) | | `apply_retroactively` | bool | Also back-fill matching existing transactions (materializes `set_category` / `add_tag` / `remove_tag`; `add_comment` is sync-only) | -### list_transaction_rules (Read) +Returns `{ created, failed, rules: [{rule, retroactive_matches?}], errors }` so a partial batch is recoverable. -List rules with optional filters and cursor pagination. **Lean by default** (`fields` omitted → `summary` projection): each row carries `name`, `enabled`, `priority`, `trigger`, `category_slug` / `category_display_name`, `hit_count`, `last_hit_at`, `created_by_type` — the roster view, **without** the `conditions` / `actions` trees. Pass `fields=all` to inspect or audit full rule definitions. Mirror of `breadbox://rules` (which always returns full). +### list_transaction_rules (Read) -| Parameter | Type | Description | -|-----------|------|-------------| -| `search` | string | Substring / words / fuzzy search on rule name | -| `category_slug` | string | Filter by target category | -| `enabled` | bool | Filter by enabled status | -| `fields` | string | Field selection. Alias: `summary` (default). `all` → full definition incl. `conditions`/`actions`. | -| `cursor` | string | Pagination cursor | -| `limit` | int | Results per page (default 50, max 500) | +The rule roster is read via `get_reference(kind=rules)` (lean `summary` projection; `fields=all` for full `conditions`/`actions`) or the `breadbox://rules` resource. For filtered/sorted analysis use `query_transaction_rules` below. ### query_transaction_rules (Read) @@ -386,9 +376,9 @@ The **inverse of `preview_rule`**: evaluates the full active rule set against a Response: `{ matched_count, rules: [{ short_id, name, sets_category, trigger, priority, hit_count, match_all }] }`. A rule with `sets_category` already handling the merchant means **don't** create a duplicate. `match_all=true` flags conditionless rules (e.g. the seeded `needs-review` tagger) that match everything — not merchant coverage. -### batch_create_rules (Write) +### create_transaction_rule — chained pipeline example -Create multiple rules in one call. Ideal for composable pipelines — use `stage` (preferred) or raw `priority` on each item to order rules so earlier-stage rules set up tags/categories that later-stage rules react to. `stage` resolves to priority `0 / 10 / 50 / 100`; if both `stage` and `priority` are supplied on an item, `priority` wins. Returns per-item success + errors. +`create_transaction_rule` takes a `rules` array, so a composable pipeline lands in one call — use `stage` (preferred) or raw `priority` on each item to order rules so earlier-stage rules set up tags/categories that later-stage rules react to. Returns per-item success + errors. Example pipeline (3 rules that chain): diff --git a/docs/rule-dsl.md b/docs/rule-dsl.md index 42c47671a..e4817c9a0 100644 --- a/docs/rule-dsl.md +++ b/docs/rule-dsl.md @@ -253,7 +253,7 @@ For `set_category`, the **last rule to match wins** (higher-priority stage has f ### Stage vs priority in API inputs -`create_transaction_rule`, `update_transaction_rule`, and `batch_create_rules` (both MCP and REST) accept a semantic `stage` string alongside the raw `priority` integer. Agents should prefer `stage` so rules from different sources compose predictably on the same shared values. +`create_transaction_rule` (MCP — takes a `rules` array of 1..N specs), `update_transaction_rule`, and the REST `POST /rules/batch` endpoint accept a semantic `stage` string alongside the raw `priority` integer. Agents should prefer `stage` so rules from different sources compose predictably on the same shared values. - Supply `stage` (`"baseline"` | `"standard"` | `"refinement"` | `"override"`) — resolves to `0 / 10 / 50 / 100`. - Supply raw `priority` — used as-is. Useful for fine-grained ordering inside a stage. diff --git a/internal/mcp/flag_tool_response_shape_integration_test.go b/internal/mcp/flag_tool_response_shape_integration_test.go index 17d4f0eff..d215bdee3 100644 --- a/internal/mcp/flag_tool_response_shape_integration_test.go +++ b/internal/mcp/flag_tool_response_shape_integration_test.go @@ -2,23 +2,20 @@ package mcp -// Regression harness for the flag_transaction / unflag_transaction MCP tool -// response shapes (T18). +// Regression harness for flagging through update_transactions (T18). // -// The goals mirror response_shapes_integration_test.go: lock the JSON envelope -// every caller / SDK generator relies on. The asserts are intentionally loose on -// values but strict on shape — if someone renames `flagged` → `is_flagged`, or -// drops `transaction_id` from the envelope, the test breaks in the same PR. +// flag_transaction / unflag_transaction were folded into update_transactions +// as the per-op `flagged` field. These tests lock that the folded path still +// has the same observable behavior: flagged_at is set/cleared in the DB, an +// accompanying comment lands as the flag reason, and query_transactions +// (flagged=…) filters on it. // // Coverage: -// - flag_transaction: required keys present (transaction_id, flagged=true) -// - flag_transaction with reason: reason recorded as a comment annotation -// - unflag_transaction: required keys present (transaction_id, flagged=false) -// - flag_transaction -> GetTransaction: flagged_at reflected in side-effect -// - unflag_transaction side-effect: flagged_at cleared (nil) -// - Missing transaction_id: error envelope returned -// - Missing transaction row: error envelope returned -// - StructuredContent / TextContent parity (via decodeToolResult) +// - flagged:true sets flagged_at (side-effect) +// - flagged:true + comment records the reason as a comment annotation +// - flagged:false clears flagged_at +// - flagged:false on an already-unflagged row is a no-op success +// - an op with a nonexistent transaction_id fails that op (not a panic) // - query_transactions(flagged=true) surfaces only the flagged row import ( @@ -29,13 +26,9 @@ import ( ) // T18FlagFreshTxn creates a fresh transaction on the seeded primary account -// and returns its UUID string. Keeps the test bodies from duplicating fixture -// plumbing and avoids naming collisions with the shared seedFixtures txn used -// by other parallel suites. +// and returns its UUID string. func T18FlagFreshTxn(t *testing.T, f *fixtures) string { t.Helper() - // Reuse the queries handle from the fixture server to stay within the same - // truncated DB state seedFixtures created. q := f.svc.svc.Queries accts, err := f.svc.svc.ListAccounts(f.ctx, nil) if err != nil || len(accts) == 0 { @@ -59,77 +52,59 @@ func T18FlagFreshTxn(t *testing.T, f *fixtures) string { return formatUUIDTest(t, txn.ID) } -// TestT18FlagTransactionResponseShape pins the flag_transaction tool's JSON -// envelope: required keys transaction_id and flagged=true must both be present -// for every successful call. Also locks the StructuredContent / TextContent -// parity contract (enforced by decodeToolResult). -func TestT18FlagTransactionResponseShape(t *testing.T) { - f := seedFixtures(t) - txnID := T18FlagFreshTxn(t, f) - - res, _, err := f.svc.handleFlagTransaction(f.ctx, nil, flagTransactionInput{ - TransactionID: txnID, - }) - out := decodeToolResult[map[string]any](t, "T18:flag_transaction", res, err) - - requireKeys(t, "T18:flag_transaction", out, "transaction_id", "flagged") - - if flagged, _ := out["flagged"].(bool); !flagged { - t.Errorf("T18:flag_transaction: flagged=%v, want true", out["flagged"]) - } - if tid, _ := out["transaction_id"].(string); tid == "" { - t.Errorf("T18:flag_transaction: transaction_id is empty") +// t18Flag drives a single flag/unflag op through update_transactions and +// returns the decoded response envelope. +func t18Flag(t *testing.T, f *fixtures, txnID string, flagged bool, comment string) map[string]any { + t.Helper() + op := transactionOperationInput{TransactionID: txnID, Flagged: &flagged} + if comment != "" { + op.Comment = &comment } + res, _, err := f.svc.handleUpdateTransactions(f.ctx, nil, updateTransactionsInput{ + Operations: []transactionOperationInput{op}, + }) + return decodeToolResult[map[string]any](t, "T18:update_transactions flag", res, err) } -// TestT18FlagTransactionSideEffect asserts that flagging a transaction actually -// sets flagged_at in the DB. A MCP wrapper that returned the success envelope -// without calling the service would look correct in the tool response but fail -// here when we read back the transaction. -func TestT18FlagTransactionSideEffect(t *testing.T) { +// TestT18FlagSetsFlaggedAt asserts that flagged:true via update_transactions +// actually sets flagged_at in the DB. +func TestT18FlagSetsFlaggedAt(t *testing.T) { f := seedFixtures(t) txnID := T18FlagFreshTxn(t, f) - // Confirm baseline: fresh transaction is unflagged. before, err := f.svc.svc.GetTransaction(f.ctx, txnID) if err != nil { - t.Fatalf("T18:side-effect: GetTransaction before: %v", err) + t.Fatalf("T18:flag: GetTransaction before: %v", err) } if before.FlaggedAt != nil { - t.Fatalf("T18:side-effect: fresh transaction FlaggedAt=%v, want nil", *before.FlaggedAt) + t.Fatalf("T18:flag: fresh transaction FlaggedAt=%v, want nil", *before.FlaggedAt) } - flagRes, _, flagErr := f.svc.handleFlagTransaction(f.ctx, nil, flagTransactionInput{ - TransactionID: txnID, - }) - decodeToolResult[map[string]any](t, "T18:flag_transaction side-effect", flagRes, flagErr) + out := t18Flag(t, f, txnID, true, "") + requireKeys(t, "T18:flag", out, "results", "succeeded", "failed") + if succeeded, _ := out["succeeded"].(float64); succeeded != 1 { + t.Errorf("T18:flag: succeeded=%v, want 1", out["succeeded"]) + } after, err := f.svc.svc.GetTransaction(f.ctx, txnID) if err != nil { - t.Fatalf("T18:side-effect: GetTransaction after: %v", err) + t.Fatalf("T18:flag: GetTransaction after: %v", err) } if after.FlaggedAt == nil { - t.Fatalf("T18:side-effect: FlaggedAt is nil after flag_transaction — wrapper likely dropped the service call") + t.Fatalf("T18:flag: FlaggedAt is nil after flagged:true — fold dropped the flag write") } } -// TestT18FlagTransactionWithReason ensures that passing a non-empty reason -// persists it as a comment annotation on the transaction timeline. The flag -// tool doc says "recorded as a comment annotation"; this test locks that the -// MCP wrapper actually forwards the reason string to the service. -func TestT18FlagTransactionWithReason(t *testing.T) { +// TestT18FlagWithReason ensures a comment paired with flagged:true persists as +// a comment annotation on the timeline (the fold's replacement for the old +// flag `reason` field). +func TestT18FlagWithReason(t *testing.T) { f := seedFixtures(t) txnID := T18FlagFreshTxn(t, f) const reason = "T18: amount looks high for this merchant" + t18Flag(t, f, txnID, true, reason) - flagRes, _, flagErr := f.svc.handleFlagTransaction(f.ctx, nil, flagTransactionInput{ - TransactionID: txnID, - Reason: reason, - }) - decodeToolResult[map[string]any](t, "T18:flag_with_reason", flagRes, flagErr) - - // The reason should surface as a comment annotation on the timeline. anns, err := f.svc.svc.ListAnnotations(f.ctx, txnID, service.ListAnnotationsParams{}) if err != nil { t.Fatalf("T18:flag_with_reason: ListAnnotations: %v", err) @@ -146,147 +121,67 @@ func TestT18FlagTransactionWithReason(t *testing.T) { } } -// TestT18UnflagTransactionResponseShape pins the unflag_transaction tool's JSON -// envelope: required keys transaction_id and flagged=false must both be present. -// Also locks the StructuredContent / TextContent parity contract. -func TestT18UnflagTransactionResponseShape(t *testing.T) { +// TestT18UnflagClearsFlaggedAt asserts flagged:false clears flagged_at. +func TestT18UnflagClearsFlaggedAt(t *testing.T) { f := seedFixtures(t) txnID := T18FlagFreshTxn(t, f) - // Flag first so there is something to unflag. - flagRes, _, flagErr := f.svc.handleFlagTransaction(f.ctx, nil, flagTransactionInput{ - TransactionID: txnID, - }) - decodeToolResult[map[string]any](t, "T18:unflag setup flag", flagRes, flagErr) - - res, _, err := f.svc.handleUnflagTransaction(f.ctx, nil, unflagTransactionInput{ - TransactionID: txnID, - }) - out := decodeToolResult[map[string]any](t, "T18:unflag_transaction", res, err) - - requireKeys(t, "T18:unflag_transaction", out, "transaction_id", "flagged") - - if flagged, _ := out["flagged"].(bool); flagged { - t.Errorf("T18:unflag_transaction: flagged=%v, want false", out["flagged"]) - } - if tid, _ := out["transaction_id"].(string); tid == "" { - t.Errorf("T18:unflag_transaction: transaction_id is empty") - } -} - -// TestT18UnflagTransactionSideEffect asserts that unflag_transaction actually -// clears flagged_at in the DB. A wrapper that returned flagged=false without -// calling the service would be silently broken. -func TestT18UnflagTransactionSideEffect(t *testing.T) { - f := seedFixtures(t) - txnID := T18FlagFreshTxn(t, f) - - // Seed a flag first via the tool handler. - flagRes, _, flagErr := f.svc.handleFlagTransaction(f.ctx, nil, flagTransactionInput{ - TransactionID: txnID, - }) - decodeToolResult[map[string]any](t, "T18:unflag side-effect: flag setup", flagRes, flagErr) - - // Confirm it is now flagged in the DB. + t18Flag(t, f, txnID, true, "") mid, err := f.svc.svc.GetTransaction(f.ctx, txnID) if err != nil { - t.Fatalf("T18:unflag side-effect: GetTransaction mid: %v", err) + t.Fatalf("T18:unflag: GetTransaction mid: %v", err) } if mid.FlaggedAt == nil { - t.Fatalf("T18:unflag side-effect: FlaggedAt is nil after flag — seeding failed") + t.Fatalf("T18:unflag: FlaggedAt is nil after flag — seeding failed") } - // Now unflag via the MCP tool handler. - unflagRes, _, unflagErr := f.svc.handleUnflagTransaction(f.ctx, nil, unflagTransactionInput{ - TransactionID: txnID, - }) - decodeToolResult[map[string]any](t, "T18:unflag side-effect", unflagRes, unflagErr) - + t18Flag(t, f, txnID, false, "") after, err := f.svc.svc.GetTransaction(f.ctx, txnID) if err != nil { - t.Fatalf("T18:unflag side-effect: GetTransaction after: %v", err) + t.Fatalf("T18:unflag: GetTransaction after: %v", err) } if after.FlaggedAt != nil { - t.Errorf("T18:unflag side-effect: FlaggedAt=%v after unflag, want nil — wrapper likely dropped the service call", *after.FlaggedAt) + t.Errorf("T18:unflag: FlaggedAt=%v after flagged:false, want nil", *after.FlaggedAt) } } -// TestT18UnflagIdempotent asserts that unflagging an already-unflagged -// transaction succeeds without error (the tool doc says "No-op if it isn't -// flagged"). The response shape contract still holds. +// TestT18UnflagIdempotent asserts flagged:false on a never-flagged row +// succeeds without error. func TestT18UnflagIdempotent(t *testing.T) { f := seedFixtures(t) txnID := T18FlagFreshTxn(t, f) - // txnID is already unflagged — calling unflag must succeed. - res, _, err := f.svc.handleUnflagTransaction(f.ctx, nil, unflagTransactionInput{ - TransactionID: txnID, - }) - out := decodeToolResult[map[string]any](t, "T18:unflag_idempotent", res, err) - requireKeys(t, "T18:unflag_idempotent", out, "transaction_id", "flagged") -} - -// TestT18FlagMissingTransactionID asserts that omitting transaction_id returns -// the error envelope (IsError=true) rather than a 500 or a nil crash. -func TestT18FlagMissingTransactionID(t *testing.T) { - f := seedFixtures(t) - - res, _, _ := f.svc.handleFlagTransaction(f.ctx, nil, flagTransactionInput{ - TransactionID: "", - }) - if res == nil || !res.IsError { - t.Fatalf("T18:flag_missing_id: expected error envelope for empty transaction_id, got %+v", res) - } -} - -// TestT18UnflagMissingTransactionID mirrors the same contract for the unflag -// tool. -func TestT18UnflagMissingTransactionID(t *testing.T) { - f := seedFixtures(t) - - res, _, _ := f.svc.handleUnflagTransaction(f.ctx, nil, unflagTransactionInput{ - TransactionID: "", - }) - if res == nil || !res.IsError { - t.Fatalf("T18:unflag_missing_id: expected error envelope for empty transaction_id, got %+v", res) + out := t18Flag(t, f, txnID, false, "") + if succeeded, _ := out["succeeded"].(float64); succeeded != 1 { + t.Errorf("T18:unflag_idempotent: succeeded=%v, want 1", out["succeeded"]) } } -// TestT18FlagNonexistentTransaction asserts that a syntactically valid but -// non-existent ID returns the error envelope instead of a nil result or panic. +// TestT18FlagNonexistentTransaction asserts that flagging an unknown id fails +// that op (failed=1) rather than panicking or silently succeeding. func TestT18FlagNonexistentTransaction(t *testing.T) { f := seedFixtures(t) - res, _, _ := f.svc.handleFlagTransaction(f.ctx, nil, flagTransactionInput{ - TransactionID: "zzzzzzzz", - }) - if res == nil || !res.IsError { - t.Fatalf("T18:flag_nonexistent: expected error envelope for unknown transaction, got %+v", res) + out := t18Flag(t, f, "zzzzzzzz", true, "") + if failed, _ := out["failed"].(float64); failed != 1 { + t.Errorf("T18:flag_nonexistent: failed=%v, want 1", out["failed"]) } } // TestT18QueryTransactionsFlaggedFilter exercises query_transactions with -// flagged=true after seeding a flagged and an unflagged transaction. The -// filter must return only the flagged row — locking that the flagged_at field -// is actually populated and filterable by agents using the MCP tool. +// flagged=true/false after flagging a fresh transaction via update_transactions. func TestT18QueryTransactionsFlaggedFilter(t *testing.T) { f := seedFixtures(t) txnID := T18FlagFreshTxn(t, f) - // Flag the fresh txn via the tool handler. - flagRes, _, flagErr := f.svc.handleFlagTransaction(f.ctx, nil, flagTransactionInput{ - TransactionID: txnID, - }) - decodeToolResult[map[string]any](t, "T18:query_flagged: flag setup", flagRes, flagErr) + t18Flag(t, f, txnID, true, "") - // Resolve the short_id so we can look for it in both result sets. flaggedRecord, err := f.svc.svc.GetTransaction(f.ctx, txnID) if err != nil { t.Fatalf("T18:query_flagged: GetTransaction: %v", err) } flaggedShort := flaggedRecord.ShortID - // query_transactions(flagged=true) — must include our flagged txn. tru := true flaggedQRes, _, flaggedQErr := f.svc.handleQueryTransactions(f.ctx, nil, queryTransactionsInput{ Flagged: &tru, @@ -311,7 +206,6 @@ func TestT18QueryTransactionsFlaggedFilter(t *testing.T) { t.Errorf("T18:query_flagged: short_id %q not found in flagged=true result set", flaggedShort) } - // query_transactions(flagged=false) — the flagged txn must NOT appear. fls := false unflaggedQRes, _, unflaggedQErr := f.svc.handleQueryTransactions(f.ctx, nil, queryTransactionsInput{ Flagged: &fls, diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 1f236297e..f88982788 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -338,38 +338,17 @@ func (s *MCPServer) buildToolRegistry() { // resolves its session via resolveTransportID + ensureAuditSession in // the dispatcher, so agents no longer need to call create_session. - // --- Reference data (mirrors resources for clients without resource support) --- + // --- Reference data (one tool that mirrors the bounded reference resources) --- // Resources are the preferred surface: breadbox://overview, ://accounts, - // ://categories, ://tags, ://users, ://sync-status, ://rules. These tool - // mirrors keep clients that don't implement resources/* unblocked. + // ://categories, ://tags, ://users, ://sync-status, ://rules. get_reference + // is the single tool fallback for clients that don't implement resources/* + // — it dispatches on `kind` so the seven bounded reads share one tool slot + // instead of seven. Filtered/sorted rule analysis still lives in + // query_transaction_rules; this returns the lean roster. makeToolDefLogged(ToolSpec{ - Name: "get_overview", Title: "Household Overview", Classification: ToolRead, - Description: "Get a household snapshot: scope (users, accounts, currencies), freshness (latest sync, errored connections, recent transactions), and backlog (pending review queue). Mirror of breadbox://overview — call this when your client doesn't support MCP resources, or when you want the snapshot inline as a tool result. Read once at the top of a session to ground every later filter (account ids, currency, attribution).", - }, s.handleGetOverview, s), - makeToolDefLogged(ToolSpec{ - Name: "list_accounts", Title: "List Accounts", Classification: ToolRead, - Description: "List bank accounts. Mirror of breadbox://accounts. Each account carries balance, type, currency, and the connection it belongs to. Filter by user_id to scope to a specific household member.", - }, s.handleListAccounts, s), - makeToolDefLogged(ToolSpec{ - Name: "list_categories", Title: "List Categories", Classification: ToolRead, - Description: "List the category taxonomy as a flat array. Mirror of breadbox://categories. Use the returned slugs (e.g. 'food_and_drink_groceries') as the canonical handle for category filters and category_slug fields on writes.", - }, s.handleListCategories, s), - makeToolDefLogged(ToolSpec{ - Name: "list_users", Title: "List Household Members", Classification: ToolRead, - Description: "List household members. Mirror of breadbox://users. Each user carries display name, role, and short_id — use the short_id as user_id on transaction filters and account scoping.", - }, s.handleListUsers, s), - makeToolDefLogged(ToolSpec{ - Name: "list_tags", Title: "List Tags", Classification: ToolRead, - Description: "List the tag vocabulary. Mirror of breadbox://tags. Tags are referenced by slug everywhere (filter, add, remove). New tag slugs auto-register the first time update_transactions adds them — read this list before authoring rules to avoid accidental near-duplicates.", - }, s.handleListTags, s), - makeToolDefLogged(ToolSpec{ - Name: "get_sync_status", Title: "Sync Status", Classification: ToolRead, - Description: "Get connection sync status: provider, status (active|error|pending_reauth|disconnected), last sync time, last error. Mirror of breadbox://sync-status. Call this before reasoning about freshness — an errored or pending_reauth connection means transactions you'd expect to be there might not be.", - }, s.handleGetSyncStatus, s), - makeToolDefLogged(ToolSpec{ - Name: "list_transaction_rules", Title: "List Rules", Classification: ToolRead, - Description: "List transaction rules (the roster). Filter by category_slug, enabled, or search by name. Lean by default: returns a summary projection (name, enabled, priority, trigger, category, hit_count) without the conditions/actions trees — pass fields=all to inspect or audit full rule definitions. Mirror of breadbox://rules. For richer analysis — filter by trigger/creator/hit-count or sort by impact — use query_transaction_rules; to check whether one specific merchant is already covered, use find_matching_rules.", - }, s.handleListTransactionRules, s), + Name: "get_reference", Title: "Get Reference Data", Classification: ToolRead, + Description: "Read a bounded reference dataset by `kind` — the single tool mirror of the breadbox:// reference resources, for clients that don't support MCP resources. kinds: 'overview' (household snapshot: scope, freshness, pending-review backlog — read once at the top of a session to ground later filters), 'accounts' (bank accounts with balance/type/currency/connection; optional user_id filter), 'categories' (the category taxonomy — use the slugs as the canonical handle on filters and writes), 'tags' (the tag vocabulary — slugs are referenced everywhere), 'users' (household members; the short_id is the user_id on filters), 'sync_status' (per-connection provider/status/last-sync/last-error — check freshness before trusting results), 'rules' (the transaction-rule roster, lean summary projection; pass fields=all for the full conditions/actions trees). For filtered/sorted rule analysis use query_transaction_rules; to check coverage for one merchant use find_matching_rules.", + }, s.handleGetReference, s), makeToolDefLogged(ToolSpec{ Name: "query_transaction_rules", Title: "Query Rules", Classification: ToolRead, Description: "Query and analyze the rule set — the rules analogue of query_transactions. Filter by category_slug, enabled, trigger (on_create|on_change|always), creator_type (user|agent|system), name search, min_hit_count, or only_unused (rules that have never fired). Sort by priority (default, pipeline order), hit_count, last_hit_at, created_at, or name. Lean by default (summary projection: name, enabled, priority, trigger, category, hit_count, last_hit_at — no conditions/actions trees); pass fields=all for the full definitions. Use this to audit coverage and prune dead rules (only_unused=true) without dumping the whole roster. To check coverage for ONE merchant before creating a rule, prefer find_matching_rules. Cursor pagination applies only to the default priority sort; an explicit sort_by returns a single top-N page (raise limit, max 500).", @@ -400,12 +379,8 @@ func (s *MCPServer) buildToolRegistry() { }, s.handleAssignSeries, s), makeToolDefLogged(ToolSpec{ Name: "update_series", Title: "Edit Recurring Series", Classification: ToolWrite, - Description: "Edit a recurring series' user-owned attributes: name, expected_amount (+ currency, amount_tolerance), cadence, expected_day, category_id, user_id (owner). Every field is optional — omit to leave unchanged. This is a deliberate override, not a detection proposal: it bypasses the precedence ladder and protects the edited values from being reverted by the next sync's re-detect. Editing cadence re-derives next_expected_date. Changing currency or owner is collision-guarded (they're part of the dedup signature). Use review_series for confirm/pause/cancel, set_series_type for the type axis, and rekey_series for the merchant_key — those have their own semantics and are NOT editable here.", + Description: "Edit a recurring series' user-owned attributes in one call: name, expected_amount (+ currency, amount_tolerance), cadence, expected_day, category_id, user_id (owner), type (subscription|bill|loan|other), and tag membership (tags_to_add / tags_to_remove). Every field is optional — omit to leave unchanged. This is a deliberate override, not a detection proposal: it bypasses the precedence ladder and protects the edited values from being reverted by the next sync's re-detect (the type override is likewise sticky). Editing cadence re-derives next_expected_date. Changing currency or owner is collision-guarded (they're part of the dedup signature). Tags must already exist (create_tag); an added tag is materialized onto every linked charge and applied to future members, a removed tag strips the series-inherited copies (a tag a user added directly survives). Use review_series for confirm/pause/cancel and rekey_series for the merchant_key — those have their own semantics and are NOT editable here.", }, s.handleUpdateSeries, s), - makeToolDefLogged(ToolSpec{ - Name: "set_series_type", Title: "Set Recurring Type", Classification: ToolWrite, - Description: "Set a recurring series' type: subscription (streaming/SaaS/memberships), bill (rent/utilities/insurance/telecom), loan (mortgage/auto/student/personal), or other. Detection infers the type from the charges' category on first detection; use this to correct it. The override is sticky — re-detection won't change it back.", - }, s.handleSetSeriesType, s), makeToolDefLogged(ToolSpec{ Name: "rekey_series", Title: "Re-key Recurring Series", Classification: ToolWrite, Description: "Correct a series' merchant_key when detection grouped it under a wrong or fallback key (e.g. 'payment' → 'spotify'). Repoints the series and its linked transactions to the new key. Refuses to silently merge: errors if a live series already exists at the new key, or that key is sticky-rejected. Corrects historical grouping — future charges still key off the provider name.", @@ -418,24 +393,12 @@ func (s *MCPServer) buildToolRegistry() { Name: "unlink_series_transactions", Title: "Unlink Charges from Series", Classification: ToolWrite, Description: "Detach transactions (≤50, each a current member) from a recurring series — the inverse of assign_series' link path. Clears each charge's series_id, strips the series' inherited tags from them (a tag the user added directly survives), and recomputes the series' rollups + next expected date. Errors if any listed transaction isn't a current member, so it can't silently no-op or touch another series. Use to remove a charge the detector wrongly swept in; use split_series instead when the stray charges form their own series.", }, s.handleUnlinkSeriesTransactions, s), - makeToolDefLogged(ToolSpec{ - Name: "add_series_tag", Title: "Tag Recurring Series", Classification: ToolWrite, - Description: "Attach an existing tag to a recurring series. The tag is materialized onto every linked transaction (they inherit it) and applied to future members as they join — so tagging the Netflix series tags all its charges. The tag must already exist (create it first with create_tag).", - }, s.handleAddSeriesTag, s), - makeToolDefLogged(ToolSpec{ - Name: "remove_series_tag", Title: "Untag Recurring Series", Classification: ToolWrite, - Description: "Detach a tag from a recurring series and strip the series-inherited copies from its linked transactions. Provenance-scoped: a tag a user added directly to a transaction survives.", - }, s.handleRemoveSeriesTag, s), // --- Query + aggregate --- makeToolDefLogged(ToolSpec{ Name: "query_transactions", Title: "Query Transactions", Classification: ToolRead, - Description: "Query bank transactions with optional filters and cursor-based pagination. Amounts: positive = money out (debit), negative = money in (credit). Dates: YYYY-MM-DD, start_date inclusive, end_date exclusive. Filter by category_slug (see breadbox://categories for the slug list); parent slugs include all children. Results ordered by date desc by default. Pagination: pass next_cursor from response. Responses are lean by default — a compact field set (core,category) is returned unless you pass fields=all or an explicit field/alias list. When every row shares one currency, iso_currency_code is returned once at the top level instead of on each row; otherwise each row carries its own.", + Description: "Query bank transactions with optional filters and cursor-based pagination. Amounts: positive = money out (debit), negative = money in (credit). Dates: YYYY-MM-DD, start_date inclusive, end_date exclusive. Filter by category_slug (see breadbox://categories for the slug list); parent slugs include all children. Results ordered by date desc by default. Pagination: pass next_cursor from response. Responses are lean by default — a compact field set (core,category) is returned unless you pass fields=all or an explicit field/alias list. When every row shares one currency, iso_currency_code is returned once at the top level instead of on each row; otherwise each row carries its own. Pass count_only=true to get just {\"count\": N} for the same filters (no rows) — use it to size a result set or compare counts across ranges before paginating.", }, s.handleQueryTransactions, s), - makeToolDefLogged(ToolSpec{ - Name: "count_transactions", Title: "Count Transactions", Classification: ToolRead, - Description: "Count transactions matching optional filters. Same filters as query_transactions except cursor, limit, sort_by, and sort_order. Use this to get totals before paginating, or to compare counts across date ranges or categories.", - }, s.handleCountTransactions, s), makeToolDefLogged(ToolSpec{ Name: "transaction_summary", Title: "Spending Summary", Classification: ToolRead, Description: "Get aggregated transaction totals grouped by category and/or time period. Replaces the need to paginate through thousands of individual transactions for spending analysis. Amounts follow the convention: positive = money out (debit), negative = money in (credit). Only includes non-deleted, non-pending transactions by default.", @@ -455,41 +418,14 @@ func (s *MCPServer) buildToolRegistry() { }, s.handleUpdateTransactions, s), // --- Transaction metadata (free-form JSONB enrichment store) --- - // Four deliberately-scoped ops; each touches ONLY the metadata column and - // names exactly what it does so an agent can't clobber sibling keys or - // other fields. Metadata is returned on every transaction read. + // One compound op (set / unset / replace) that touches ONLY the metadata + // column, so an agent can't clobber sibling keys or first-class fields. + // Metadata is returned on every transaction read. makeToolDefLogged(ToolSpec{ Name: "set_transaction_metadata", Title: "Set Transaction Metadata", Classification: ToolWrite, - Description: "Upsert ONE key in a transaction's free-form metadata JSONB store, leaving every other key untouched. Creates the key if absent, overwrites if present. The value may be any JSON value (string, number, boolean, object, array). Use slug-like keys, max 128 chars (e.g. 'tax_deductible', 'trip', 'reimbursable_by'). Metadata is a place for enrichment your household cares about that isn't a first-class field — it is NOT a substitute for category or tags. Returned on every transaction read (query_transactions, the transaction resource). Example: {\"transaction_id\":\"k7Xm9pQ2\",\"key\":\"tax_deductible\",\"value\":true}.", + Description: "Write a transaction's free-form metadata JSONB store. `set` upserts key→value pairs (MERGE — keys you don't list stay untouched); `unset` deletes keys (no-op if absent); `replace:true` makes the result EXACTLY the set object (clears every pre-existing key first), and replace:true with set omitted clears all metadata. Keys are slug-like, max 128 chars (e.g. 'tax_deductible', 'trip'); values may be any JSON. Metadata is for household enrichment that isn't a first-class field — it is NOT a substitute for category or tags. Returned on every transaction read (query_transactions, the transaction resource). Examples: merge → {\"transaction_id\":\"k7Xm9pQ2\",\"set\":{\"tax_deductible\":true}}; remove a key → {\"transaction_id\":\"k7Xm9pQ2\",\"unset\":[\"trip\"]}; replace all → {\"transaction_id\":\"k7Xm9pQ2\",\"replace\":true,\"set\":{\"trip\":\"q2\"}}; clear → {\"transaction_id\":\"k7Xm9pQ2\",\"replace\":true}.", Annotations: &mcpsdk.ToolAnnotations{DestructiveHint: boolPtr(false), IdempotentHint: true}, }, s.handleSetTransactionMetadata, s), - makeToolDefLogged(ToolSpec{ - Name: "remove_transaction_metadata", Title: "Remove Transaction Metadata Key", Classification: ToolWrite, - Description: "Delete ONE key from a transaction's metadata JSONB store. No-op (still succeeds) if the key isn't present. Other keys are untouched.", - Annotations: &mcpsdk.ToolAnnotations{DestructiveHint: boolPtr(false), IdempotentHint: true}, - }, s.handleRemoveTransactionMetadata, s), - makeToolDefLogged(ToolSpec{ - Name: "replace_transaction_metadata", Title: "Replace Transaction Metadata", Classification: ToolWrite, - Description: "Atomically replace the ENTIRE metadata object on a transaction. Use to write a structured payload in one call. Pass {} to clear all keys. Prefer set_transaction_metadata when you only mean to change one key — replace overwrites everything.", - Annotations: &mcpsdk.ToolAnnotations{DestructiveHint: boolPtr(false), IdempotentHint: true}, - }, s.handleReplaceTransactionMetadata, s), - makeToolDefLogged(ToolSpec{ - Name: "clear_transaction_metadata", Title: "Clear Transaction Metadata", Classification: ToolWrite, - Description: "Reset a transaction's metadata to the empty object {}, removing all keys. Equivalent to replace_transaction_metadata with {}.", - Annotations: &mcpsdk.ToolAnnotations{DestructiveHint: boolPtr(false), IdempotentHint: true}, - }, s.handleClearTransactionMetadata, s), - - // --- Flag (surface a transaction for human attention) --- - makeToolDefLogged(ToolSpec{ - Name: "flag_transaction", Title: "Flag Transaction", Classification: ToolWrite, - Description: "Flag a transaction for human attention (sets flagged_at) without changing its category. Pass an optional `reason` — it's recorded as a comment annotation on the timeline. This is the 'look at this' escape hatch: when you auto-categorize but are unsure, or spot something worth a human glance, flag it instead of guessing. Retrieve flagged transactions with query_transactions(flagged=true). Idempotent: re-flagging refreshes the timestamp.", - Annotations: &mcpsdk.ToolAnnotations{DestructiveHint: boolPtr(false), IdempotentHint: true}, - }, s.handleFlagTransaction, s), - makeToolDefLogged(ToolSpec{ - Name: "unflag_transaction", Title: "Unflag Transaction", Classification: ToolWrite, - Description: "Clear the flag on a transaction (sets flagged_at = NULL). No-op if it isn't flagged.", - Annotations: &mcpsdk.ToolAnnotations{DestructiveHint: boolPtr(false), IdempotentHint: true}, - }, s.handleUnflagTransaction, s), // --- Activity timeline --- makeToolDefLogged(ToolSpec{ @@ -502,12 +438,8 @@ func (s *MCPServer) buildToolRegistry() { // for the current ruleset. makeToolDefLogged(ToolSpec{ Name: "create_transaction_rule", Title: "Create Rule", Classification: ToolWrite, - Description: "Create a transaction rule for automatic categorization, tagging, or commenting. Rules match condition trees against transactions during sync and fire in pipeline-stage order (priority ASC — lower = earlier). Pass `stage` (one of baseline|standard|refinement|override) instead of a raw priority so rules from different agents compose predictably; stage resolves to priority 0/10/50/100. Earlier-stage rules' tag and category mutations feed later-stage rules' conditions, so rules compose: rule A tags 'coffee', rule B conditioned on tags-contains-coffee sets category. Before creating, read breadbox://rules to avoid duplicates; prefer `contains` over exact matches (bank feeds format merchant names inconsistently). Full DSL: breadbox://rule-dsl.", + Description: "Create one or more transaction rules for automatic categorization, tagging, or commenting. Pass `rules`: an array of 1..100 rule specs (a single rule is just a one-element array). Rules match condition trees against transactions during sync and fire in pipeline-stage order (priority ASC — lower = earlier). Pass `stage` (one of baseline|standard|refinement|override) per rule instead of a raw priority so rules from different agents compose predictably; stage resolves to priority 0/10/50/100. Earlier-stage rules' tag and category mutations feed later-stage rules' conditions, so rules compose: rule A tags 'coffee', rule B conditioned on tags-contains-coffee sets category — author such pipelines in one call. Set apply_retroactively=true on a rule to immediately back-fill it against existing transactions. Before creating, read the rules roster (get_reference kind=rules) to avoid duplicates; prefer `contains` over exact matches (bank feeds format merchant names inconsistently). Returns the created rules plus any per-item errors so a partial batch is recoverable. Full DSL: breadbox://rule-dsl.", }, s.handleCreateTransactionRule, s), - makeToolDefLogged(ToolSpec{ - Name: "batch_create_rules", Title: "Create Rules in Batch", Classification: ToolWrite, - Description: "Create multiple transaction rules at once. More efficient than looping create_transaction_rule. Ideal for composable pipelines — use `stage` (baseline|standard|refinement|override) on each item to order rules so earlier-stage rules set up tags/categories that later-stage rules react to. `stage` is preferred over raw `priority` for cross-agent consistency; if both are supplied, priority wins. Each item follows the same shape as create_transaction_rule. Returns created rules plus any per-item errors so partial success is recoverable.", - }, s.handleBatchCreateRules, s), makeToolDefLogged(ToolSpec{ Name: "update_transaction_rule", Title: "Update Rule", Classification: ToolWrite, Description: "Update a transaction rule's fields. Every field is optional; omit to leave unchanged. Pass conditions={} to explicitly clear conditions (match-all). Pass actions=[...] to replace the entire action set (rules must retain at least one action). Pass expires_at=\"\" to clear expiry. Pass `stage` (baseline|standard|refinement|override) to re-slot a rule into the pipeline without guessing a numeric priority. See breadbox://rule-dsl.", diff --git a/internal/mcp/server_test.go b/internal/mcp/server_test.go index c86beb52f..1cca4f4a8 100644 --- a/internal/mcp/server_test.go +++ b/internal/mcp/server_test.go @@ -124,24 +124,18 @@ func TestToolRegistryScopeContract(t *testing.T) { } // Anchor the explicit canonical set for read tools — this is the surface - // area read-only API keys are allowed to exercise. Includes the seven - // reference-data mirrors (get_overview / list_* / get_sync_status) that - // shadow the bounded reference resources for clients without resources - // support — see tools_reads.go. + // area read-only API keys are allowed to exercise. The seven bounded + // reference-data mirrors that shadow the breadbox:// resources for clients + // without resources support are folded behind get_reference(kind=…) — see + // tools_reads.go. count_transactions folded into query_transactions + // (count_only=true). wantReads := []string{ "query_transactions", - "count_transactions", "transaction_summary", "list_annotations", "preview_rule", "find_matching_rules", - "get_overview", - "list_accounts", - "list_categories", - "list_users", - "list_tags", - "get_sync_status", - "list_transaction_rules", + "get_reference", "query_transaction_rules", "list_series", "get_series", diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index eb0c64690..8d2736206 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -37,23 +37,7 @@ type queryTransactionsInput struct { SortBy string `json:"sort_by,omitempty" jsonschema:"Sort: date (default), amount, provider_name"` SortOrder string `json:"sort_order,omitempty" jsonschema:"Sort direction: desc (default) or asc"` Fields string `json:"fields,omitempty" jsonschema:"Comma-separated fields to include, to cut response size. Aliases: minimal (provider_name,amount,date), core (id,date,amount,provider_name,iso_currency_code), category (category,provider_category_primary,provider_category_detailed), timestamps (created_at,updated_at,datetime,authorized_datetime). Default when omitted: core,category (a compact projection). Pass fields=all for every field. id is always included."` -} - -type countTransactionsInput struct { - StartDate string `json:"start_date,omitempty" jsonschema:"Start date (YYYY-MM-DD) inclusive"` - EndDate string `json:"end_date,omitempty" jsonschema:"End date (YYYY-MM-DD) exclusive"` - AccountID string `json:"account_id,omitempty" jsonschema:"Filter by account ID"` - UserID string `json:"user_id,omitempty" jsonschema:"Filter by user ID"` - CategorySlug string `json:"category_slug,omitempty" jsonschema:"Filter by category slug"` - MinAmount *float64 `json:"min_amount,omitempty" jsonschema:"Minimum amount"` - MaxAmount *float64 `json:"max_amount,omitempty" jsonschema:"Maximum amount"` - Pending *bool `json:"pending,omitempty" jsonschema:"Filter by pending status"` - Flagged *bool `json:"flagged,omitempty" jsonschema:"Filter to flagged transactions (true) or unflagged (false). Omit to return both. Use flagged=true to retrieve transactions an agent or you have flagged for attention."` - Search string `json:"search,omitempty" jsonschema:"Search name or merchant. Comma-separated values are ORed."` - SearchMode string `json:"search_mode,omitempty" jsonschema:"Search mode: contains (default), words, fuzzy"` - ExcludeSearch string `json:"exclude_search,omitempty" jsonschema:"Exclude transactions matching this text"` - Tags []string `json:"tags,omitempty" jsonschema:"Filter to transactions that have EVERY tag slug in this list (AND semantics)."` - AnyTag []string `json:"any_tag,omitempty" jsonschema:"Filter to transactions that have AT LEAST ONE tag slug in this list (OR semantics)."` + CountOnly bool `json:"count_only,omitempty" jsonschema:"When true, return only {\"count\": N} for the given filters — no rows, no pagination. cursor/limit/sort_by/sort_order/fields are ignored. Use to size a result set or compare counts across date ranges or categories before paginating."` } type transactionSummaryInput struct { @@ -121,6 +105,31 @@ func (s *MCPServer) handleQueryTransactions(_ context.Context, _ *mcpsdk.CallToo return errorResult(err), nil, nil } + // count_only short-circuit: same filters, just the total. Absorbs the + // former standalone count_transactions tool. + if input.CountOnly { + count, err := s.svc.CountTransactionsFiltered(ctx, service.TransactionCountParams{ + StartDate: params.StartDate, + EndDate: params.EndDate, + AccountID: params.AccountID, + UserID: params.UserID, + CategorySlug: params.CategorySlug, + MinAmount: params.MinAmount, + MaxAmount: params.MaxAmount, + Pending: params.Pending, + Flagged: params.Flagged, + Search: params.Search, + SearchMode: params.SearchMode, + ExcludeSearch: params.ExcludeSearch, + Tags: params.Tags, + AnyTag: params.AnyTag, + }) + if err != nil { + return errorResult(err), nil, nil + } + return jsonResult(map[string]int64{"count": count}) + } + // Lean-by-default: an omitted fields param returns a compact projection // (core identity + category) rather than all ~22 fields. Agents rarely set // fields and the full row is mostly unused, so the default response is the @@ -232,38 +241,6 @@ func currencyString(v any) (string, bool) { return "", false } -func (s *MCPServer) handleCountTransactions(_ context.Context, _ *mcpsdk.CallToolRequest, input countTransactionsInput) (*mcpsdk.CallToolResult, any, error) { - ctx := context.Background() - params := service.TransactionCountParams{ - AccountID: optStr(input.AccountID), - UserID: optStr(input.UserID), - CategorySlug: optStr(input.CategorySlug), - MinAmount: input.MinAmount, - MaxAmount: input.MaxAmount, - Pending: input.Pending, - Flagged: input.Flagged, - Search: optStr(input.Search), - ExcludeSearch: optStr(input.ExcludeSearch), - Tags: input.Tags, - AnyTag: input.AnyTag, - } - - var err error - if params.StartDate, params.EndDate, err = parseDateRange(input.StartDate, input.EndDate); err != nil { - return errorResult(err), nil, nil - } - if params.SearchMode, err = parseSearchMode(input.SearchMode); err != nil { - return errorResult(err), nil, nil - } - - count, err := s.svc.CountTransactionsFiltered(ctx, params) - if err != nil { - return errorResult(err), nil, nil - } - - return jsonResult(map[string]int64{"count": count}) -} - // handleTransactionSummary aggregates totals over a window. // // Example call (last month grouped by category): @@ -314,6 +291,12 @@ func (s *MCPServer) handleTransactionSummary(_ context.Context, _ *mcpsdk.CallTo // --- Transaction Rules --- type createTransactionRuleInput struct { + Rules []ruleSpecInput `json:"rules" jsonschema:"required,Array of 1..100 rules to create (a single rule is a one-element array). Authoring a composable pipeline in one call orders rules by stage so earlier-stage tag/category writes feed later-stage conditions. Example — tag, then categorize the tagged, then flag the expensive: [{\"name\":\"Tag coffee shops\",\"stage\":\"baseline\",\"conditions\":{\"field\":\"provider_merchant_name\",\"op\":\"contains\",\"value\":\"starbucks\"},\"actions\":[{\"type\":\"add_tag\",\"tag_slug\":\"coffee\"}]},{\"name\":\"Categorize coffee\",\"stage\":\"standard\",\"conditions\":{\"field\":\"tags\",\"op\":\"contains\",\"value\":\"coffee\"},\"category_slug\":\"food_and_drink_coffee\"}]"` +} + +// ruleSpecInput is one rule in a create_transaction_rule call. Same shape the +// single-rule and batch tools used before they were folded into one array. +type ruleSpecInput struct { Name string `json:"name" jsonschema:"required,Name for this rule (human-readable description)"` Conditions map[string]any `json:"conditions,omitempty" jsonschema:"JSON condition tree. Omit or pass {} to match every transaction. Leaf: {\"field\":\"...\",\"op\":\"...\",\"value\":...}. Combinators: {\"and\":[...]}, {\"or\":[...]}, {\"not\":{...}} (nest freely, max depth 10). Fields: provider_name provider_merchant_name amount provider_category_primary provider_category_detailed category(assigned slug, live-updated by earlier-stage rules) pending provider account_id account_name user_id user_name tags. Ops: string/category=eq|neq|contains|not_contains|matches(RE2)|in; numeric=eq|neq|gt|gte|lt|lte; bool=eq|neq; tags=contains|not_contains|in. Nested example: {\"or\":[{\"and\":[{\"field\":\"provider_merchant_name\",\"op\":\"contains\",\"value\":\"starbucks\"},{\"field\":\"amount\",\"op\":\"gte\",\"value\":5}]},{\"field\":\"tags\",\"op\":\"contains\",\"value\":\"coffee\"}]}. Full spec: docs/rule-dsl.md."` Actions []map[string]string `json:"actions,omitempty" jsonschema:"Array of typed actions. {\"type\":\"set_category\",\"category_slug\":\"...\"} | {\"type\":\"add_tag\",\"tag_slug\":\"...\"} | {\"type\":\"remove_tag\",\"tag_slug\":\"...\"} | {\"type\":\"add_comment\",\"content\":\"...\"}. Actions compose: a rule can set a category AND add a tag AND add a comment in the same match. add_comment fires only at sync time (not on retroactive apply). remove_tag net-diffs against add_tag within the same sync pass. If omitted, use category_slug instead."` @@ -342,21 +325,6 @@ type deleteTransactionRuleInput struct { ID string `json:"id" jsonschema:"required,UUID of the rule to delete"` } -type batchCreateRulesInput struct { - Rules []batchRuleItem `json:"rules" jsonschema:"required,Array of rules to create. Ideal for composable pipelines. Example — tagging then categorizing then flagging: [{\"name\":\"Tag coffee shops\",\"priority\":0,\"conditions\":{\"field\":\"provider_merchant_name\",\"op\":\"contains\",\"value\":\"starbucks\"},\"actions\":[{\"type\":\"add_tag\",\"tag_slug\":\"coffee\"}]},{\"name\":\"Categorize coffee-tagged\",\"priority\":10,\"conditions\":{\"field\":\"tags\",\"op\":\"contains\",\"value\":\"coffee\"},\"actions\":[{\"type\":\"set_category\",\"category_slug\":\"food_and_drink_coffee\"}]},{\"name\":\"Flag expensive coffee\",\"priority\":50,\"conditions\":{\"and\":[{\"field\":\"tags\",\"op\":\"contains\",\"value\":\"coffee\"},{\"field\":\"amount\",\"op\":\"gt\",\"value\":15}]},\"actions\":[{\"type\":\"add_tag\",\"tag_slug\":\"expensive\"}]}]"` -} - -type batchRuleItem struct { - Name string `json:"name" jsonschema:"required,Human-readable rule name"` - Actions []map[string]string `json:"actions,omitempty" jsonschema:"Actions array (typed — same format as create_transaction_rule)"` - CategorySlug string `json:"category_slug,omitempty" jsonschema:"Shorthand for set_category action. Either actions or category_slug required."` - Conditions map[string]any `json:"conditions,omitempty" jsonschema:"Condition tree as JSON object. Omit or {} for match-all."` - Trigger string `json:"trigger,omitempty" jsonschema:"on_create (default), on_change, or always. 'on_update' accepted as alias for on_change."` - Stage string `json:"stage,omitempty" jsonschema:"Semantic pipeline stage: baseline | standard (default) | refinement | override — resolves to priority 0/10/50/100. Prefer over raw priority for cross-agent consistency. If both are supplied, priority wins."` - Priority int `json:"priority,omitempty" jsonschema:"Raw priority integer. Prefer 'stage' for shared vocabulary. Defaults to 10 (standard)."` - ExpiresIn string `json:"expires_in,omitempty" jsonschema:"Optional expiry duration"` -} - type applyRulesInput struct { RuleID string `json:"rule_id,omitempty" jsonschema:"Optional ID (UUID or short_id) of a specific rule to apply. When supplied, only that rule runs — no chaining. Omit to apply all active rules in pipeline-stage order (priority ASC); earlier rules' tag/category mutations feed later rules' conditions, exactly like sync-time. Materializes set_category / add_tag / remove_tag; add_comment stays sync-only. Ignores rule.trigger (retroactive is a bulk op)."` } @@ -403,45 +371,75 @@ func (s *MCPServer) handleCreateTransactionRule(ctx context.Context, _ *mcpsdk.C if err := s.checkWritePermission(ctx); err != nil { return errorResult(err), nil, nil } - if input.Name == "" { - return errorResult(fmt.Errorf("name is required")), nil, nil - } - if len(input.Actions) == 0 && input.CategorySlug == "" { - return errorResult(fmt.Errorf("either actions or category_slug is required")), nil, nil + if len(input.Rules) == 0 { + return errorResult(fmt.Errorf("rules array is required and must not be empty")), nil, nil } - - conditions, err := parseConditions(input.Conditions) - if err != nil { - return errorResult(err), nil, nil + if len(input.Rules) > 100 { + return errorResult(fmt.Errorf("maximum 100 rules per call")), nil, nil } actor := service.ActorFromContext(ctx) + created := make([]map[string]any, 0, len(input.Rules)) + var errs []map[string]string - rule, err := s.svc.CreateTransactionRule(ctx, service.CreateTransactionRuleParams{ - Name: input.Name, - Conditions: conditions, - Actions: convertMCPActions(input.Actions), - CategorySlug: input.CategorySlug, - Trigger: input.Trigger, - Priority: input.Priority, - Stage: input.Stage, - ExpiresIn: input.ExpiresIn, - Actor: actor, - }) - if err != nil { - return errorResult(err), nil, nil - } + for i, r := range input.Rules { + if r.Name == "" || (len(r.Actions) == 0 && r.CategorySlug == "") { + errs = append(errs, map[string]string{ + "index": strconv.Itoa(i), + "name": r.Name, + "error": "name and either actions or category_slug are required", + }) + continue + } - resp := map[string]any{"rule": rule} - if input.ApplyRetroactively { - count, err := s.svc.ApplyRuleRetroactively(ctx, rule.ID) + conditions, err := parseConditions(r.Conditions) if err != nil { - resp["retroactive_error"] = err.Error() - } else { - resp["retroactive_matches"] = count + errs = append(errs, map[string]string{ + "index": strconv.Itoa(i), + "name": r.Name, + "error": err.Error(), + }) + continue } + + rule, err := s.svc.CreateTransactionRule(ctx, service.CreateTransactionRuleParams{ + Name: r.Name, + Conditions: conditions, + Actions: convertMCPActions(r.Actions), + CategorySlug: r.CategorySlug, + Trigger: r.Trigger, + Priority: r.Priority, + Stage: r.Stage, + ExpiresIn: r.ExpiresIn, + Actor: actor, + }) + if err != nil { + errs = append(errs, map[string]string{ + "index": strconv.Itoa(i), + "name": r.Name, + "error": err.Error(), + }) + continue + } + + entry := map[string]any{"rule": rule} + if r.ApplyRetroactively { + count, aerr := s.svc.ApplyRuleRetroactively(ctx, rule.ID) + if aerr != nil { + entry["retroactive_error"] = aerr.Error() + } else { + entry["retroactive_matches"] = count + } + } + created = append(created, entry) } - return jsonResult(resp) + + return jsonResult(map[string]any{ + "created": len(created), + "failed": len(errs), + "rules": created, + "errors": errs, + }) } func (s *MCPServer) handleUpdateTransactionRule(ctx context.Context, _ *mcpsdk.CallToolRequest, input updateTransactionRuleInput) (*mcpsdk.CallToolResult, any, error) { @@ -565,71 +563,6 @@ func (s *MCPServer) handleFindMatchingRules(ctx context.Context, _ *mcpsdk.CallT return jsonResult(result) } -func (s *MCPServer) handleBatchCreateRules(ctx context.Context, _ *mcpsdk.CallToolRequest, input batchCreateRulesInput) (*mcpsdk.CallToolResult, any, error) { - if err := s.checkWritePermission(ctx); err != nil { - return errorResult(err), nil, nil - } - if len(input.Rules) == 0 { - return errorResult(fmt.Errorf("rules array is required and must not be empty")), nil, nil - } - if len(input.Rules) > 100 { - return errorResult(fmt.Errorf("maximum 100 rules per batch")), nil, nil - } - - actor := service.ActorFromContext(ctx) - var created []service.TransactionRuleResponse - var errors []map[string]string - - for i, r := range input.Rules { - if r.Name == "" || (len(r.Actions) == 0 && r.CategorySlug == "") { - errors = append(errors, map[string]string{ - "index": strconv.Itoa(i), - "error": "name and either actions or category_slug are required", - }) - continue - } - - // Empty conditions == match-all. - conditions, err := parseConditions(r.Conditions) - if err != nil { - errors = append(errors, map[string]string{ - "index": strconv.Itoa(i), - "name": r.Name, - "error": err.Error(), - }) - continue - } - - rule, err := s.svc.CreateTransactionRule(ctx, service.CreateTransactionRuleParams{ - Name: r.Name, - Conditions: conditions, - Actions: convertMCPActions(r.Actions), - CategorySlug: r.CategorySlug, - Trigger: r.Trigger, - Priority: r.Priority, - Stage: r.Stage, - ExpiresIn: r.ExpiresIn, - Actor: actor, - }) - if err != nil { - errors = append(errors, map[string]string{ - "index": strconv.Itoa(i), - "name": r.Name, - "error": err.Error(), - }) - continue - } - created = append(created, *rule) - } - - return jsonResult(map[string]any{ - "created": len(created), - "failed": len(errors), - "rules": created, - "errors": errors, - }) -} - // --- Agent Reports --- type submitReportInput struct { diff --git a/internal/mcp/tools_flag.go b/internal/mcp/tools_flag.go deleted file mode 100644 index efe7e266f..000000000 --- a/internal/mcp/tools_flag.go +++ /dev/null @@ -1,54 +0,0 @@ -//go:build !lite - -package mcp - -import ( - "context" - "fmt" - - "breadbox/internal/service" - - mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp" -) - -// The flag tools let an agent surface a transaction for human attention without -// changing its category. flag_transaction sets flagged_at; the reason is -// recorded as a comment annotation. Retrieve flagged rows with -// query_transactions(flagged=true). This is the "look at this" escape hatch the -// auto-apply workflow uses when it is unsure about a transaction. - -type flagTransactionInput struct { - TransactionID string `json:"transaction_id" jsonschema:"required,UUID or short ID of the transaction to flag."` - Reason string `json:"reason,omitempty" jsonschema:"Optional reason for the flag, recorded as a comment annotation on the transaction's timeline. Keep it short and specific (e.g. 'amount looks high for this merchant')."` -} - -type unflagTransactionInput struct { - TransactionID string `json:"transaction_id" jsonschema:"required,UUID or short ID of the transaction to unflag."` -} - -func (s *MCPServer) handleFlagTransaction(ctx context.Context, _ *mcpsdk.CallToolRequest, input flagTransactionInput) (*mcpsdk.CallToolResult, any, error) { - if err := s.checkWritePermission(ctx); err != nil { - return errorResult(err), nil, nil - } - if input.TransactionID == "" { - return errorResult(fmt.Errorf("transaction_id is required")), nil, nil - } - actor := service.ActorFromContext(ctx) - if err := s.svc.FlagTransaction(context.Background(), input.TransactionID, input.Reason, actor); err != nil { - return errorResult(err), nil, nil - } - return jsonResult(map[string]any{"transaction_id": input.TransactionID, "flagged": true}) -} - -func (s *MCPServer) handleUnflagTransaction(ctx context.Context, _ *mcpsdk.CallToolRequest, input unflagTransactionInput) (*mcpsdk.CallToolResult, any, error) { - if err := s.checkWritePermission(ctx); err != nil { - return errorResult(err), nil, nil - } - if input.TransactionID == "" { - return errorResult(fmt.Errorf("transaction_id is required")), nil, nil - } - if err := s.svc.UnflagTransaction(context.Background(), input.TransactionID); err != nil { - return errorResult(err), nil, nil - } - return jsonResult(map[string]any{"transaction_id": input.TransactionID, "flagged": false}) -} diff --git a/internal/mcp/tools_reads.go b/internal/mcp/tools_reads.go index 70235dd51..4879c6022 100644 --- a/internal/mcp/tools_reads.go +++ b/internal/mcp/tools_reads.go @@ -4,6 +4,7 @@ package mcp import ( "context" + "fmt" "breadbox/internal/service" @@ -72,6 +73,40 @@ type queryTransactionRulesInput struct { Fields string `json:"fields,omitempty" jsonschema:"Comma-separated fields to include, to cut response size. Alias: summary (name,enabled,priority,trigger,category,hit_count,last_hit_at; the default — omits the conditions and actions trees). Default when omitted: summary. Pass fields=all for every field including the full conditions/actions. id is always included."` } +// getReferenceInput is the single parameterized read over the bounded +// reference datasets. `kind` selects the dataset; the optional fields apply +// only to the kinds that accept them (user_id → accounts, fields → rules). +type getReferenceInput struct { + Kind string `json:"kind" jsonschema:"required,Which reference dataset to read: overview | accounts | categories | tags | users | sync_status | rules."` + UserID string `json:"user_id,omitempty" jsonschema:"Only for kind=accounts: scope accounts to one household member (short ID or UUID)."` + Fields string `json:"fields,omitempty" jsonschema:"Only for kind=rules: comma-separated fields/aliases. Default summary projection (omits conditions/actions trees); pass fields=all for full rule definitions."` +} + +// handleGetReference dispatches a get_reference call to the matching bounded +// reference handler. It reuses the per-kind handlers (which share the same +// service path as the breadbox:// resources), so payload shapes stay identical +// to the resources and to the pre-fold tool mirrors. +func (s *MCPServer) handleGetReference(ctx context.Context, req *mcpsdk.CallToolRequest, input getReferenceInput) (*mcpsdk.CallToolResult, any, error) { + switch input.Kind { + case "overview": + return s.handleGetOverview(ctx, req, getOverviewInput{}) + case "accounts": + return s.handleListAccounts(ctx, req, listAccountsInput{UserID: input.UserID}) + case "categories": + return s.handleListCategories(ctx, req, listCategoriesInput{}) + case "tags": + return s.handleListTags(ctx, req, listTagsInput{}) + case "users": + return s.handleListUsers(ctx, req, listUsersInput{}) + case "sync_status": + return s.handleGetSyncStatus(ctx, req, getSyncStatusInput{}) + case "rules": + return s.handleListTransactionRules(ctx, req, listTransactionRulesInput{Fields: input.Fields}) + default: + return errorResult(fmt.Errorf("unknown kind %q: must be one of overview, accounts, categories, tags, users, sync_status, rules", input.Kind)), nil, nil + } +} + // --- Handlers --- func (s *MCPServer) handleListAccounts(_ context.Context, _ *mcpsdk.CallToolRequest, input listAccountsInput) (*mcpsdk.CallToolResult, any, error) { diff --git a/internal/mcp/tools_series.go b/internal/mcp/tools_series.go index 014fcb167..5db3a5196 100644 --- a/internal/mcp/tools_series.go +++ b/internal/mcp/tools_series.go @@ -136,8 +136,17 @@ type updateSeriesInput struct { ExpectedDay *int32 `json:"expected_day,omitempty" jsonschema:"New day-of-month / day-of-week anchor (1–31). Send 0 to clear the anchor; omit to leave unchanged."` CategoryID string `json:"category_id,omitempty" jsonschema:"New suggested category short ID or UUID. Omit to leave unchanged."` UserID string `json:"user_id,omitempty" jsonschema:"New household-member owner short ID or UUID. Part of the dedup signature — a change is collision-guarded. Omit to leave unchanged."` + Type string `json:"type,omitempty" jsonschema:"New recurring-charge type: subscription (streaming/SaaS/memberships), bill (rent/utilities/insurance/telecom), loan (mortgage/auto/student/personal), or other. Detection infers it from the charges' category; set this to correct it. Sticky override — re-detection won't change it back. Omit to leave unchanged."` + TagsToAdd []string `json:"tags_to_add,omitempty" jsonschema:"Tag slugs to attach to the series (each must already exist — create_tag first). Each tag is materialized onto every linked charge and applied to future members as they join."` + TagsToRemove []string `json:"tags_to_remove,omitempty" jsonschema:"Tag slugs to detach from the series. Strips the series-inherited copies from linked charges; a tag a user added directly to a charge survives."` } +// handleUpdateSeries is the compound series editor — it absorbs the former +// set_series_type and add/remove_series_tag tools. Order: edit user-owned +// fields → set type (sticky) → add tags → remove tags, then return the fresh +// series. Each sub-change reuses the same service method the standalone tool +// did, so the semantics (collision guards, sticky type, tag provenance) are +// unchanged. func (s *MCPServer) handleUpdateSeries(ctx context.Context, _ *mcpsdk.CallToolRequest, input updateSeriesInput) (*mcpsdk.CallToolResult, any, error) { if err := s.checkWritePermission(ctx); err != nil { return errorResult(err), nil, nil @@ -155,111 +164,71 @@ func (s *MCPServer) handleUpdateSeries(ctx context.Context, _ *mcpsdk.CallToolRe CategoryID: optStr(input.CategoryID), UserID: optStr(input.UserID), } - if edit.Name == nil && edit.ExpectedAmount == nil && edit.AmountTolerance == nil && - edit.Currency == nil && edit.Cadence == nil && edit.ExpectedDay == nil && - edit.CategoryID == nil && edit.UserID == nil { - return errorResult(fmt.Errorf("provide at least one field to update")), nil, nil + hasEdit := edit.Name != nil || edit.ExpectedAmount != nil || edit.AmountTolerance != nil || + edit.Currency != nil || edit.Cadence != nil || edit.ExpectedDay != nil || + edit.CategoryID != nil || edit.UserID != nil + if !hasEdit && input.Type == "" && len(input.TagsToAdd) == 0 && len(input.TagsToRemove) == 0 { + return errorResult(fmt.Errorf("provide at least one field to update (a series attribute, type, or tags_to_add/tags_to_remove)")), nil, nil } + + bg := context.Background() actor := service.ActorFromContext(ctx) - series, err := s.svc.UpdateSeries(context.Background(), input.ID, edit, actor) - if err != nil { + + notFound := func(err error) (*mcpsdk.CallToolResult, any, error) { if errors.Is(err, service.ErrNotFound) { return errorResult(fmt.Errorf("series not found")), nil, nil } return errorResult(err), nil, nil } - return jsonResult(series) -} -type unlinkSeriesInput struct { - ID string `json:"id" jsonschema:"required,Series short ID or UUID."` - TransactionIDs []string `json:"transaction_ids" jsonschema:"required,Transactions (short ID or UUID) to detach from the series. Each must currently belong to it. Max 50."` -} - -func (s *MCPServer) handleUnlinkSeriesTransactions(ctx context.Context, _ *mcpsdk.CallToolRequest, input unlinkSeriesInput) (*mcpsdk.CallToolResult, any, error) { - if err := s.checkWritePermission(ctx); err != nil { - return errorResult(err), nil, nil - } - if input.ID == "" || len(input.TransactionIDs) == 0 { - return errorResult(fmt.Errorf("id and transaction_ids are required")), nil, nil + if hasEdit { + if _, err := s.svc.UpdateSeries(bg, input.ID, edit, actor); err != nil { + return notFound(err) + } } - actor := service.ActorFromContext(ctx) - series, err := s.svc.UnlinkSeriesTransactions(context.Background(), input.ID, input.TransactionIDs, actor) - if err != nil { - if errors.Is(err, service.ErrNotFound) { - return errorResult(fmt.Errorf("series not found")), nil, nil + if input.Type != "" { + if _, err := s.svc.SetSeriesType(bg, input.ID, input.Type, actor); err != nil { + return notFound(err) } - return errorResult(err), nil, nil } - return jsonResult(series) -} - -type setSeriesTypeInput struct { - ID string `json:"id" jsonschema:"required,Series short ID or UUID."` - Type string `json:"type" jsonschema:"required,One of: subscription, bill, loan, other."` -} - -func (s *MCPServer) handleSetSeriesType(ctx context.Context, _ *mcpsdk.CallToolRequest, input setSeriesTypeInput) (*mcpsdk.CallToolResult, any, error) { - if err := s.checkWritePermission(ctx); err != nil { - return errorResult(err), nil, nil + for _, slug := range input.TagsToAdd { + if err := s.svc.AddSeriesTag(bg, input.ID, slug, actor); err != nil { + return notFound(err) + } } - if input.ID == "" || input.Type == "" { - return errorResult(fmt.Errorf("id and type are required")), nil, nil + for _, slug := range input.TagsToRemove { + if err := s.svc.RemoveSeriesTag(bg, input.ID, slug); err != nil { + return notFound(err) + } } - actor := service.ActorFromContext(ctx) - series, err := s.svc.SetSeriesType(context.Background(), input.ID, input.Type, actor) + + series, err := s.svc.GetSeries(bg, input.ID) if err != nil { - if errors.Is(err, service.ErrNotFound) { - return errorResult(fmt.Errorf("series not found")), nil, nil - } - return errorResult(err), nil, nil + return notFound(err) } return jsonResult(series) } -type seriesTagInput struct { - ID string `json:"id" jsonschema:"required,Series short ID or UUID."` - TagSlug string `json:"tag_slug" jsonschema:"required,Tag slug (must already exist)."` +type unlinkSeriesInput struct { + ID string `json:"id" jsonschema:"required,Series short ID or UUID."` + TransactionIDs []string `json:"transaction_ids" jsonschema:"required,Transactions (short ID or UUID) to detach from the series. Each must currently belong to it. Max 50."` } -func (s *MCPServer) handleAddSeriesTag(ctx context.Context, _ *mcpsdk.CallToolRequest, input seriesTagInput) (*mcpsdk.CallToolResult, any, error) { +func (s *MCPServer) handleUnlinkSeriesTransactions(ctx context.Context, _ *mcpsdk.CallToolRequest, input unlinkSeriesInput) (*mcpsdk.CallToolResult, any, error) { if err := s.checkWritePermission(ctx); err != nil { return errorResult(err), nil, nil } - if input.ID == "" || input.TagSlug == "" { - return errorResult(fmt.Errorf("id and tag_slug are required")), nil, nil + if input.ID == "" || len(input.TransactionIDs) == 0 { + return errorResult(fmt.Errorf("id and transaction_ids are required")), nil, nil } actor := service.ActorFromContext(ctx) - if err := s.svc.AddSeriesTag(context.Background(), input.ID, input.TagSlug, actor); err != nil { - if errors.Is(err, service.ErrNotFound) { - return errorResult(fmt.Errorf("series not found")), nil, nil - } - return errorResult(err), nil, nil - } - series, err := s.svc.GetSeries(context.Background(), input.ID) + series, err := s.svc.UnlinkSeriesTransactions(context.Background(), input.ID, input.TransactionIDs, actor) if err != nil { - return errorResult(err), nil, nil - } - return jsonResult(series) -} - -func (s *MCPServer) handleRemoveSeriesTag(ctx context.Context, _ *mcpsdk.CallToolRequest, input seriesTagInput) (*mcpsdk.CallToolResult, any, error) { - if err := s.checkWritePermission(ctx); err != nil { - return errorResult(err), nil, nil - } - if input.ID == "" || input.TagSlug == "" { - return errorResult(fmt.Errorf("id and tag_slug are required")), nil, nil - } - if err := s.svc.RemoveSeriesTag(context.Background(), input.ID, input.TagSlug); err != nil { if errors.Is(err, service.ErrNotFound) { return errorResult(fmt.Errorf("series not found")), nil, nil } return errorResult(err), nil, nil } - series, err := s.svc.GetSeries(context.Background(), input.ID) - if err != nil { - return errorResult(err), nil, nil - } return jsonResult(series) } diff --git a/internal/mcp/tools_transaction_metadata.go b/internal/mcp/tools_transaction_metadata.go index d603d2caa..cad0dcb53 100644 --- a/internal/mcp/tools_transaction_metadata.go +++ b/internal/mcp/tools_transaction_metadata.go @@ -5,41 +5,28 @@ package mcp import ( "context" "fmt" + "strings" mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp" ) -// The transaction-metadata tools expose the free-form `metadata` JSONB store on -// a transaction through four deliberately-scoped operations. They touch ONLY the -// metadata column — none of them can write a first-class field (category, tags, -// amount). Each op states exactly what it does so an agent can't accidentally -// clobber sibling keys or other fields: -// - set_transaction_metadata → upsert ONE key (other keys untouched) -// - remove_transaction_metadata → delete ONE key -// - replace_transaction_metadata → replace the WHOLE object atomically -// - clear_transaction_metadata → reset to the empty object {} +// set_transaction_metadata is the single compound write over a transaction's +// free-form `metadata` JSONB store. It touches ONLY the metadata column — it +// can never write a first-class field (category, tags, amount). One tool covers +// every shape the four legacy ops did: +// - upsert keys → set:{...} (merge; other keys untouched) +// - delete keys → unset:[...] (no-op for absent keys) +// - replace the blob → replace:true + set:{} (clear every pre-existing key) +// - clear everything → replace:true (set omitted → {}) // // Metadata is read back on every transaction (query_transactions, the // breadbox://transaction/{id} resource, GET /transactions/{id}). type setTransactionMetadataInput struct { - TransactionID string `json:"transaction_id" jsonschema:"required,UUID or short ID of the transaction."` - Key string `json:"key" jsonschema:"required,Metadata key to set. Slug-like string, max 128 chars (e.g. 'tax_deductible', 'trip')."` - Value any `json:"value" jsonschema:"required,JSON value to store: string, number, boolean, object, or array. Replaces any existing value for this key."` -} - -type removeTransactionMetadataInput struct { - TransactionID string `json:"transaction_id" jsonschema:"required,UUID or short ID of the transaction."` - Key string `json:"key" jsonschema:"required,Metadata key to remove. No-op if absent."` -} - -type replaceTransactionMetadataInput struct { TransactionID string `json:"transaction_id" jsonschema:"required,UUID or short ID of the transaction."` - Metadata map[string]any `json:"metadata" jsonschema:"required,Complete metadata object. Replaces the entire blob atomically. Pass {} to clear all keys."` -} - -type clearTransactionMetadataInput struct { - TransactionID string `json:"transaction_id" jsonschema:"required,UUID or short ID of the transaction."` + Set map[string]any `json:"set,omitempty" jsonschema:"Key→value pairs to upsert. Each key is slug-like, max 128 chars (e.g. 'tax_deductible', 'trip'); each value may be any JSON (string, number, boolean, object, array). By default these are MERGED — keys you don't list are left untouched. Example: {\"tax_deductible\":true,\"trip\":\"q2-offsite\"}."` + Unset []string `json:"unset,omitempty" jsonschema:"Keys to delete from the metadata object. No-op (still succeeds) for keys that aren't present. Applied after set."` + Replace bool `json:"replace,omitempty" jsonschema:"When true, the resulting metadata is EXACTLY the set object — every pre-existing key is cleared first (then unset is applied to the new object). Pass replace:true with set omitted to clear all metadata. Prefer the default merge (replace:false) when you only mean to change a few keys."` } func (s *MCPServer) handleSetTransactionMetadata(ctx context.Context, _ *mcpsdk.CallToolRequest, input setTransactionMetadataInput) (*mcpsdk.CallToolResult, any, error) { @@ -49,47 +36,51 @@ func (s *MCPServer) handleSetTransactionMetadata(ctx context.Context, _ *mcpsdk. if input.TransactionID == "" { return errorResult(fmt.Errorf("transaction_id is required")), nil, nil } - if err := s.svc.SetTransactionMetadata(context.Background(), input.TransactionID, input.Key, input.Value); err != nil { - return errorResult(err), nil, nil + if !input.Replace && len(input.Set) == 0 && len(input.Unset) == 0 { + return errorResult(fmt.Errorf("nothing to do: provide set, unset, or replace")), nil, nil } - return jsonResult(map[string]any{"transaction_id": input.TransactionID, "key": input.Key, "set": true}) -} -func (s *MCPServer) handleRemoveTransactionMetadata(ctx context.Context, _ *mcpsdk.CallToolRequest, input removeTransactionMetadataInput) (*mcpsdk.CallToolResult, any, error) { - if err := s.checkWritePermission(ctx); err != nil { - return errorResult(err), nil, nil - } - if input.TransactionID == "" { - return errorResult(fmt.Errorf("transaction_id is required")), nil, nil - } - if err := s.svc.RemoveTransactionMetadata(context.Background(), input.TransactionID, input.Key); err != nil { - return errorResult(err), nil, nil - } - return jsonResult(map[string]any{"transaction_id": input.TransactionID, "key": input.Key, "removed": true}) -} + bg := context.Background() -func (s *MCPServer) handleReplaceTransactionMetadata(ctx context.Context, _ *mcpsdk.CallToolRequest, input replaceTransactionMetadataInput) (*mcpsdk.CallToolResult, any, error) { - if err := s.checkWritePermission(ctx); err != nil { - return errorResult(err), nil, nil - } - if input.TransactionID == "" { - return errorResult(fmt.Errorf("transaction_id is required")), nil, nil - } - if err := s.svc.ReplaceTransactionMetadata(context.Background(), input.TransactionID, input.Metadata); err != nil { - return errorResult(err), nil, nil + // Replace path: the resulting blob is exactly `set` (minus any unset keys). + // Written atomically as one object. + if input.Replace { + next := make(map[string]any, len(input.Set)) + for k, v := range input.Set { + if strings.TrimSpace(k) == "" { + return errorResult(fmt.Errorf("metadata key cannot be empty")), nil, nil + } + next[k] = v + } + for _, k := range input.Unset { + delete(next, k) + } + if err := s.svc.ReplaceTransactionMetadata(bg, input.TransactionID, next); err != nil { + return errorResult(err), nil, nil + } + return jsonResult(map[string]any{"transaction_id": input.TransactionID, "replaced": true, "keys": len(next)}) } - return jsonResult(map[string]any{"transaction_id": input.TransactionID, "replaced": true}) -} -func (s *MCPServer) handleClearTransactionMetadata(ctx context.Context, _ *mcpsdk.CallToolRequest, input clearTransactionMetadataInput) (*mcpsdk.CallToolResult, any, error) { - if err := s.checkWritePermission(ctx); err != nil { - return errorResult(err), nil, nil + // Merge path: upsert each set key, then delete each unset key. Keys not + // named are left untouched. + setKeys := make([]string, 0, len(input.Set)) + for k, v := range input.Set { + if strings.TrimSpace(k) == "" { + return errorResult(fmt.Errorf("metadata key cannot be empty")), nil, nil + } + if err := s.svc.SetTransactionMetadata(bg, input.TransactionID, k, v); err != nil { + return errorResult(err), nil, nil + } + setKeys = append(setKeys, k) } - if input.TransactionID == "" { - return errorResult(fmt.Errorf("transaction_id is required")), nil, nil - } - if err := s.svc.ClearTransactionMetadata(context.Background(), input.TransactionID); err != nil { - return errorResult(err), nil, nil + for _, k := range input.Unset { + if err := s.svc.RemoveTransactionMetadata(bg, input.TransactionID, k); err != nil { + return errorResult(err), nil, nil + } } - return jsonResult(map[string]any{"transaction_id": input.TransactionID, "cleared": true}) + return jsonResult(map[string]any{ + "transaction_id": input.TransactionID, + "set": setKeys, + "unset": input.Unset, + }) } diff --git a/internal/mcp/tools_update_transactions.go b/internal/mcp/tools_update_transactions.go index b00835e0d..8c7cfed0d 100644 --- a/internal/mcp/tools_update_transactions.go +++ b/internal/mcp/tools_update_transactions.go @@ -25,7 +25,8 @@ type transactionOperationInput struct { ResetCategory bool `json:"reset_category,omitempty" jsonschema:"Clear an existing manual category override and drop the transaction back to 'uncategorized' so rules can re-categorize it. Mutually exclusive with category_slug. Use this to undo a prior categorize/update_transactions decision."` TagsToAdd []tagOpEntryInput `json:"tags_to_add,omitempty" jsonschema:"List of tags to add. Each item: {slug}. Auto-creates persistent tags if the slug is unknown."` TagsToRemove []tagOpEntryInput `json:"tags_to_remove,omitempty" jsonschema:"List of tags to remove. Each item: {slug}."` - Comment *string `json:"comment,omitempty" jsonschema:"Free-form comment written as an annotation attributed to you. Max 10000 chars. This is the canonical place to record decision context (e.g. why a tag was added or a category set) — prefer the comment over a per-tag note."` + Comment *string `json:"comment,omitempty" jsonschema:"Free-form comment written as an annotation attributed to you. Max 10000 chars. This is the canonical place to record decision context (e.g. why a tag was added or a category set) — prefer the comment over a per-tag note. When you set flagged:true, put the flag's reason here — it lands as the same comment annotation."` + Flagged *bool `json:"flagged,omitempty" jsonschema:"Flag (true) or unflag (false) this transaction for human attention, without changing its category. flagged:true is the 'look at this' escape hatch when you're unsure — pair it with a comment for the reason; retrieve flagged rows later with query_transactions(flagged=true). flagged:false clears the flag. Omit to leave the flag untouched. Idempotent: re-flagging refreshes the timestamp."` } // tagOpEntryInput is a single tag add/remove entry. @@ -97,6 +98,7 @@ func (s *MCPServer) handleUpdateTransactions(ctx context.Context, _ *mcpsdk.Call CategorySlug: op.CategorySlug, ResetCategory: op.ResetCategory, Comment: op.Comment, + Flagged: op.Flagged, } for _, t := range op.TagsToAdd { ops[i].TagsToAdd = append(ops[i].TagsToAdd, service.UpdateTransactionsTagOp{Slug: t.Slug}) diff --git a/internal/service/update_transactions.go b/internal/service/update_transactions.go index 0394a6382..d4857d42d 100644 --- a/internal/service/update_transactions.go +++ b/internal/service/update_transactions.go @@ -28,6 +28,11 @@ type UpdateTransactionsOp struct { TagsToAdd []UpdateTransactionsTagOp TagsToRemove []UpdateTransactionsTagOp Comment *string + // Flagged toggles the transaction's flagged_at marker: true sets it to + // NOW(), false clears it. nil leaves the flag untouched. The flag's reason, + // if any, rides in Comment (the same comment annotation the standalone flag + // tool wrote before flag/unflag were folded into this op). + Flagged *bool } // UpdateTransactionsTagOp is a single tag add/remove entry. @@ -384,6 +389,25 @@ func (s *Service) runUpdateOpInTx(ctx context.Context, tx pgx.Tx, op UpdateTrans } } + // 5. Flag / unflag. flagged=true sets flagged_at=NOW(), false clears it. + // The reason (if any) rode in via op.Comment above. Idempotent: re-flagging + // refreshes the timestamp. + if op.Flagged != nil { + var sql string + if *op.Flagged { + sql = "UPDATE transactions SET flagged_at = NOW() WHERE id = $1 AND deleted_at IS NULL" + } else { + sql = "UPDATE transactions SET flagged_at = NULL WHERE id = $1 AND deleted_at IS NULL" + } + tag, err := tx.Exec(ctx, sql, txnUID) + if err != nil { + return fmt.Errorf("update flag: %w", err) + } + if tag.RowsAffected() == 0 { + return fmt.Errorf("%w: transaction not found", ErrNotFound) + } + } + return nil } From 6b09eeae90d820d6d1e5a58dbab0eab1f45cb4dd Mon Sep 17 00:00:00 2001 From: Ricardo Canales Date: Mon, 15 Jun 2026 23:33:59 -0700 Subject: [PATCH 2/6] refactor(mcp): retire categories/tags/users/rules reference resources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These four resources were exact duplicates of get_reference(kind=…) and, being pure agent-facing lookup tables (not human-attach material), carried no load on the resource surface — especially on clients that can't resources/list. Read them via the tool instead. Resource surface 13 → 9: kept overview / accounts / sync-status (a human plausibly attaches those snapshots), the three static-doc resources (rule-dsl / report-format / review-guidelines), and the three drilldown templates. - Drop the 4 AddResource registrations + their handlers (+ rulesResourceLimit). - Removed TestRulesResourceShape; trimmed the resource/tool parity test to the three pairs that still have both surfaces. - Scrubbed every "breadbox://{categories,tags,users,rules}" pointer in tool descriptions, input schemas, the MCP server instructions, the rule-dsl / review-guidelines prompts, and the docs → get_reference(kind=…). Build + vet + unit + mcp integration green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/rules/mcp.md | 10 +- docs/mcp-server.md | 10 +- docs/mcp-tools-reference.md | 6 +- internal/mcp/resources.go | 41 +----- .../mcp/response_shapes_integration_test.go | 137 ++---------------- internal/mcp/server.go | 50 ++----- internal/mcp/tools.go | 4 +- prompts/mcp/instructions.md | 19 ++- prompts/mcp/review-guidelines.md | 4 +- prompts/mcp/rule-dsl.md | 2 +- 10 files changed, 58 insertions(+), 225 deletions(-) diff --git a/.claude/rules/mcp.md b/.claude/rules/mcp.md index 9890fb53f..afab2e7bb 100644 --- a/.claude/rules/mcp.md +++ b/.claude/rules/mcp.md @@ -98,12 +98,14 @@ When folding a tool, prefer reusing the existing service methods from the compou ## Reference data: dual surface (resources + one tool) -Bounded reference data is exposed two ways: +Bounded reference data is read through **one tool**, `get_reference(kind=…)`, dispatching on `kind` (`overview` \| `accounts` \| `categories` \| `tags` \| `users` \| `sync_status` \| `rules`) to the per-kind handlers in `tools_reads.go` (`handleGetOverview`, `handleListAccounts`, …). Optional `user_id` (accounts) and `fields` (rules) ride through. Filtered/sorted rule analysis stays in `query_transaction_rules`; `get_reference kind=rules` returns the lean roster. -- **Resources (preferred)** — `breadbox://overview`, `://accounts`, `://categories`, `://tags`, `://users`, `://rules`, `://sync-status`. Surfaced in Claude.ai's paperclip menu and the Inspector resource picker. Application-driven, user-controlled. -- **`get_reference` tool (compat)** — a single tool that dispatches on `kind` (`overview` \| `accounts` \| `categories` \| `tags` \| `users` \| `sync_status` \| `rules`) to the same per-kind handler the matching resource uses. Kept because not every MCP client implements the resources/* methods. Optional `user_id` (accounts) and `fields` (rules) ride through. Filtered/sorted rule analysis stays in `query_transaction_rules`; `get_reference kind=rules` returns the lean roster. +A **subset** is also exposed as `resources/*` for clients with a resource/attach UI (Claude.ai's paperclip menu, the Inspector picker), sharing the same service-layer call path so payloads stay in sync: -Both surfaces share the same service-layer call path (no logic duplication), so payload shape stays in sync. The per-kind handlers (`handleGetOverview`, `handleListAccounts`, …) still live in `tools_reads.go` — `get_reference` dispatches to them. When adding a new bounded reference resource, add a resource handler in `resources.go` and a new `kind` branch in `handleGetReference`. +- `breadbox://overview`, `breadbox://accounts`, `breadbox://sync-status` — kept because a human plausibly attaches these snapshots. +- `breadbox://categories`, `://tags`, `://users`, `://rules` were **retired** — pure agent-facing lookup tables that no one attaches, and exact duplicates of the `get_reference` kinds. Read them via the tool. + +So a new reference dataset gets a `kind` branch in `handleGetReference` by default; add a parallel resource handler in `resources.go` only if it's something a user would attach in chat (snapshot-like), not for pure lookup tables. ## Resource templates (drill-downs) diff --git a/docs/mcp-server.md b/docs/mcp-server.md index 22d3d5710..d11458572 100644 --- a/docs/mcp-server.md +++ b/docs/mcp-server.md @@ -601,15 +601,19 @@ Calls `GET /api/v1/categories`. Returns the same `[]CategoryPair` response. ## 4. MCP Resources -MCP resources are passive context documents the LLM can read, similar to files. They do not execute queries at call time in the same on-demand way tools do. +MCP resources are passive context documents the LLM can read, similar to files. They do not execute queries at call time in the same on-demand way tools do. They're registered via `s.registerResources()` in `internal/mcp/server.go`, with handlers in `internal/mcp/resources.go`. -All three resources are registered via `s.registerResources()` in `internal/mcp/server.go`, with handlers in `internal/mcp/resources.go`. +Bounded reference data (accounts, categories, tags, users, sync status, rules) is read through the **`get_reference(kind=…)` tool**, not resources — only snapshot-like / static-doc resources remain (the lookup-table resources were retired as tool duplicates). The resources that stay: | Resource URI | MIME Type | Description | |---|---|---| -| `breadbox://overview` | `application/json` | Live dataset summary (users, connections, accounts, transactions, spending) | +| `breadbox://overview` | `application/json` | Live dataset summary (also `get_reference(kind=overview)`) | +| `breadbox://accounts` | `application/json` | Bank accounts (also `get_reference(kind=accounts)`) | +| `breadbox://sync-status` | `application/json` | Connection sync status (also `get_reference(kind=sync_status)`) | | `breadbox://review-guidelines` | `text/markdown` | Guidelines for reviewing transactions and creating rules | | `breadbox://report-format` | `text/markdown` | Report structure templates and formatting guidelines | +| `breadbox://rule-dsl` | `text/markdown` | Condition grammar / action types for authoring rules | +| `breadbox://transaction/{short_id}`, `breadbox://account/{short_id}`, `breadbox://user/{short_id}` | `application/json` | Per-entity drilldown templates | ### Resource: `breadbox://overview` diff --git a/docs/mcp-tools-reference.md b/docs/mcp-tools-reference.md index c32080567..5b018394a 100644 --- a/docs/mcp-tools-reference.md +++ b/docs/mcp-tools-reference.md @@ -71,7 +71,7 @@ Aggregated spending totals. Default date range: 30 days. ### get_reference (Read) -One tool that reads any bounded reference dataset by `kind` — the single mirror of the `breadbox://` reference resources, for clients that don't support MCP resources. (Folds the former `get_overview`, `list_accounts`, `list_categories`, `list_tags`, `list_users`, `get_sync_status`, and `list_transaction_rules` tools.) +One tool that reads any bounded reference dataset by `kind`. (Folds the former `get_overview`, `list_accounts`, `list_categories`, `list_tags`, `list_users`, `get_sync_status`, and `list_transaction_rules` tools.) `overview`, `accounts`, and `sync_status` are also exposed as `breadbox://` resources for clients with an attach UI; `categories`, `tags`, `users`, and `rules` are tool-only (their resources were retired as duplicates). | Parameter | Type | Description | |-----------|------|-------------| @@ -148,7 +148,7 @@ Import categories from TSV format. Creates or updates categories. ### list_tags (Read) -The tag vocabulary is read via `get_reference(kind=tags)` (or the `breadbox://tags` resource). +The tag vocabulary is read via `get_reference(kind=tags)`. ### add_transaction_tag (Write) @@ -318,7 +318,7 @@ Returns `{ created, failed, rules: [{rule, retroactive_matches?}], errors }` so ### list_transaction_rules (Read) -The rule roster is read via `get_reference(kind=rules)` (lean `summary` projection; `fields=all` for full `conditions`/`actions`) or the `breadbox://rules` resource. For filtered/sorted analysis use `query_transaction_rules` below. +The rule roster is read via `get_reference(kind=rules)` (lean `summary` projection; `fields=all` for full `conditions`/`actions`). For filtered/sorted analysis use `query_transaction_rules` below. ### query_transaction_rules (Read) diff --git a/internal/mcp/resources.go b/internal/mcp/resources.go index a9a1bf9af..93ef8e8a4 100644 --- a/internal/mcp/resources.go +++ b/internal/mcp/resources.go @@ -130,14 +130,6 @@ func jsonResourceResult(uri string, v any) (*mcpsdk.ReadResourceResult, error) { }, nil } -func (s *MCPServer) handleCategoriesResource(ctx context.Context, _ *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { - cats, err := s.svc.ListCategories(ctx) - if err != nil { - return nil, fmt.Errorf("list categories: %w", err) - } - return jsonResourceResult("breadbox://categories", map[string]any{"categories": cats}) -} - func (s *MCPServer) handleAccountsResource(ctx context.Context, _ *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { accts, err := s.svc.ListAccounts(ctx, nil) if err != nil { @@ -146,22 +138,6 @@ func (s *MCPServer) handleAccountsResource(ctx context.Context, _ *mcpsdk.ReadRe return jsonResourceResult("breadbox://accounts", map[string]any{"accounts": accts}) } -func (s *MCPServer) handleUsersResource(ctx context.Context, _ *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { - users, err := s.svc.ListUsers(ctx) - if err != nil { - return nil, fmt.Errorf("list users: %w", err) - } - return jsonResourceResult("breadbox://users", map[string]any{"users": users}) -} - -func (s *MCPServer) handleTagsResource(ctx context.Context, _ *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { - tags, err := s.svc.ListTags(ctx) - if err != nil { - return nil, fmt.Errorf("list tags: %w", err) - } - return jsonResourceResult("breadbox://tags", map[string]any{"tags": tags}) -} - func (s *MCPServer) handleSyncStatusResource(ctx context.Context, _ *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { conns, err := s.svc.ListConnections(ctx, nil) if err != nil { @@ -170,20 +146,9 @@ func (s *MCPServer) handleSyncStatusResource(ctx context.Context, _ *mcpsdk.Read return jsonResourceResult("breadbox://sync-status", map[string]any{"connections": conns}) } -// rulesResourceLimit caps the rules resource payload. The rule list is small -// in practice (households tend to have tens, not thousands of rules), but the -// cap keeps the resource bounded and predictable. -const rulesResourceLimit = 200 - -func (s *MCPServer) handleRulesResource(ctx context.Context, _ *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { - result, err := s.svc.ListTransactionRules(ctx, service.TransactionRuleListParams{ - Limit: rulesResourceLimit, - }) - if err != nil { - return nil, fmt.Errorf("list rules: %w", err) - } - return jsonResourceResult("breadbox://rules", result) -} +// The categories / tags / users / rules reference resources were retired as +// duplicates of get_reference(kind=…) — see registerResources. Their read +// paths still exist as the get_reference tool handlers in tools_reads.go. // resourceAnnotations builds the standard *mcpsdk.Annotations payload used on // every Breadbox resource. lastModifiedFn returns the timestamp the client diff --git a/internal/mcp/response_shapes_integration_test.go b/internal/mcp/response_shapes_integration_test.go index fa223683c..ca440e9b4 100644 --- a/internal/mcp/response_shapes_integration_test.go +++ b/internal/mcp/response_shapes_integration_test.go @@ -958,79 +958,16 @@ func TestUpdateTransactionsHandler_ResetCategoryShape(t *testing.T) { // optional service-layer params without copy-pasting the &s pattern. func ptrString(s string) *string { return &s } -// TestRulesResourceShape pins the breadbox://rules resource against the -// service.TransactionRuleListResult contract introduced when the rule listing -// moved from a tool to a live resource. The shape is what agents branch on -// when picking duplicate-detection logic, and the limit cap is what keeps the -// resource bounded under household-scale rule counts. The test asserts: -// - the JSON envelope mirrors TransactionRuleListResult (rules, has_more, total) -// - rule rows go through compactIDsBytes (id is the 8-char short, no short_id sibling) -// - the underlying ListTransactionRules call carries the rulesResourceLimit cap -func TestRulesResourceShape(t *testing.T) { - f := seedFixtures(t) - - // seedFixtures already inserted one rule. Read the resource directly. - res, err := f.svc.handleRulesResource(f.ctx, nil) - if err != nil { - t.Fatalf("handleRulesResource: %v", err) - } - if res == nil || len(res.Contents) == 0 { - t.Fatal("expected a content block") - } - c := res.Contents[0] - if c.URI != "breadbox://rules" { - t.Errorf("URI = %q, want breadbox://rules", c.URI) - } - if c.MIMEType != "application/json" { - t.Errorf("MIMEType = %q, want application/json", c.MIMEType) - } - - var out map[string]any - if err := json.Unmarshal([]byte(c.Text), &out); err != nil { - t.Fatalf("unmarshal rules resource: %v\nraw=%s", err, c.Text) - } - requireKeys(t, "breadbox://rules", out, "rules", "has_more", "total") - - rules := asArray(t, "breadbox://rules.rules", out["rules"]) - if len(rules) == 0 { - t.Fatal("expected at least the seeded rule") - } - rule := asObject(t, "breadbox://rules.rules[0]", rules[0]) - requireKeys(t, "breadbox://rules.rules[0]", rule, "id", "name", "trigger", "priority") - // compactIDsBytes must collapse the id/short_id pair: id should be the - // 8-char short, and short_id must not appear on the row. - requireAbsent(t, "breadbox://rules.rules[0]", rule, "short_id") - id, _ := rule["id"].(string) - if len(id) != 8 { - t.Errorf("rule id = %q (len=%d); expected 8-char short_id", id, len(id)) - } - - // Verify the resource handler honors the 200-cap by exercising the same - // service call with the same params and asserting the cap surfaces. We - // don't actually create 200+ rules (expensive); instead we confirm the - // limit value travels through to the service layer by calling the service - // directly with the documented cap and ensuring it doesn't raise. - if _, err := f.svc.svc.ListTransactionRules(f.ctx, service.TransactionRuleListParams{ - Limit: rulesResourceLimit, - }); err != nil { - t.Errorf("ListTransactionRules with rulesResourceLimit=%d failed: %v", rulesResourceLimit, err) - } - if rulesResourceLimit != 200 { - t.Errorf("rulesResourceLimit drift: got %d, want 200", rulesResourceLimit) - } -} - // TestReferenceMirrorTools_ParityWithResources locks the dual-surface -// contract: each bounded reference resource (breadbox://accounts, -// ://categories, ://tags, ://users, ://sync-status, ://rules, ://overview) has -// a tool mirror (list_accounts / list_categories / list_tags / list_users / -// get_sync_status / list_transaction_rules / get_overview) that returns the -// SAME payload via the SAME service call. A regression that diverges them — -// e.g. forgetting to wrap one in the resource envelope, or pointing the tool -// at a different service method — would let one surface drift from the other. -// Both surfaces are user-discoverable today (resources via Claude.ai's -// paperclip menu, tools via Inspector + clients without resource support), so -// drift is observable and bad. +// contract for the reference datasets that still have BOTH a resource and a +// get_reference tool path: breadbox://accounts, ://sync-status, ://overview. +// Each pair must return the SAME payload via the SAME service call. A +// regression that diverges them — e.g. forgetting the resource envelope, or +// pointing the tool at a different service method — would let one surface +// drift from the other. +// +// (categories / tags / users / rules were retired as resources — they're +// read-only via get_reference now, so there's no pair left to compare.) // // The parity test reads each pair, ignores envelope keys (resource handlers // always wrap in {"": [...]}), and compares the inner payload byte- @@ -1041,12 +978,12 @@ func TestReferenceMirrorTools_ParityWithResources(t *testing.T) { cases := []struct { name string - envelopeKey string // key inside the JSON envelope; "" means top-level (overview, rules) + envelopeKey string // key inside the JSON envelope; "" means top-level (overview) toolFn func() (*mcpsdk.CallToolResult, any, error) resourceFn func() (*mcpsdk.ReadResourceResult, error) }{ { - name: "list_accounts <-> breadbox://accounts", + name: "get_reference(accounts) <-> breadbox://accounts", envelopeKey: "accounts", toolFn: func() (*mcpsdk.CallToolResult, any, error) { return f.svc.handleListAccounts(f.ctx, nil, listAccountsInput{}) @@ -1056,37 +993,7 @@ func TestReferenceMirrorTools_ParityWithResources(t *testing.T) { }, }, { - name: "list_categories <-> breadbox://categories", - envelopeKey: "categories", - toolFn: func() (*mcpsdk.CallToolResult, any, error) { - return f.svc.handleListCategories(f.ctx, nil, listCategoriesInput{}) - }, - resourceFn: func() (*mcpsdk.ReadResourceResult, error) { - return f.svc.handleCategoriesResource(f.ctx, nil) - }, - }, - { - name: "list_tags <-> breadbox://tags", - envelopeKey: "tags", - toolFn: func() (*mcpsdk.CallToolResult, any, error) { - return f.svc.handleListTags(f.ctx, nil, listTagsInput{}) - }, - resourceFn: func() (*mcpsdk.ReadResourceResult, error) { - return f.svc.handleTagsResource(f.ctx, nil) - }, - }, - { - name: "list_users <-> breadbox://users", - envelopeKey: "users", - toolFn: func() (*mcpsdk.CallToolResult, any, error) { - return f.svc.handleListUsers(f.ctx, nil, listUsersInput{}) - }, - resourceFn: func() (*mcpsdk.ReadResourceResult, error) { - return f.svc.handleUsersResource(f.ctx, nil) - }, - }, - { - name: "get_sync_status <-> breadbox://sync-status", + name: "get_reference(sync_status) <-> breadbox://sync-status", envelopeKey: "connections", toolFn: func() (*mcpsdk.CallToolResult, any, error) { return f.svc.handleGetSyncStatus(f.ctx, nil, getSyncStatusInput{}) @@ -1096,25 +1003,7 @@ func TestReferenceMirrorTools_ParityWithResources(t *testing.T) { }, }, { - name: "list_transaction_rules <-> breadbox://rules", - envelopeKey: "", // both surfaces return the same {rules, has_more, total} object - toolFn: func() (*mcpsdk.CallToolResult, any, error) { - // list_transaction_rules is lean-by-default (summary projection, - // no conditions/actions trees). The resource has no fields knob - // and returns the full payload, so parity is asserted against - // the tool's full mode (fields=all) — that's the invariant that - // must hold: same service call, same full shape. - return f.svc.handleListTransactionRules(f.ctx, nil, listTransactionRulesInput{ - Limit: rulesResourceLimit, - Fields: "all", - }) - }, - resourceFn: func() (*mcpsdk.ReadResourceResult, error) { - return f.svc.handleRulesResource(f.ctx, nil) - }, - }, - { - name: "get_overview <-> breadbox://overview", + name: "get_reference(overview) <-> breadbox://overview", envelopeKey: "", // both surfaces return the same OverviewStats shape toolFn: func() (*mcpsdk.CallToolResult, any, error) { return f.svc.handleGetOverview(f.ctx, nil, getOverviewInput{}) diff --git a/internal/mcp/server.go b/internal/mcp/server.go index f88982788..3e2b4c745 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -397,7 +397,7 @@ func (s *MCPServer) buildToolRegistry() { // --- Query + aggregate --- makeToolDefLogged(ToolSpec{ Name: "query_transactions", Title: "Query Transactions", Classification: ToolRead, - Description: "Query bank transactions with optional filters and cursor-based pagination. Amounts: positive = money out (debit), negative = money in (credit). Dates: YYYY-MM-DD, start_date inclusive, end_date exclusive. Filter by category_slug (see breadbox://categories for the slug list); parent slugs include all children. Results ordered by date desc by default. Pagination: pass next_cursor from response. Responses are lean by default — a compact field set (core,category) is returned unless you pass fields=all or an explicit field/alias list. When every row shares one currency, iso_currency_code is returned once at the top level instead of on each row; otherwise each row carries its own. Pass count_only=true to get just {\"count\": N} for the same filters (no rows) — use it to size a result set or compare counts across ranges before paginating.", + Description: "Query bank transactions with optional filters and cursor-based pagination. Amounts: positive = money out (debit), negative = money in (credit). Dates: YYYY-MM-DD, start_date inclusive, end_date exclusive. Filter by category_slug (see get_reference(kind=categories) for the slug list); parent slugs include all children. Results ordered by date desc by default. Pagination: pass next_cursor from response. Responses are lean by default — a compact field set (core,category) is returned unless you pass fields=all or an explicit field/alias list. When every row shares one currency, iso_currency_code is returned once at the top level instead of on each row; otherwise each row carries its own. Pass count_only=true to get just {\"count\": N} for the same filters (no rows) — use it to size a result set or compare counts across ranges before paginating.", }, s.handleQueryTransactions, s), makeToolDefLogged(ToolSpec{ Name: "transaction_summary", Title: "Spending Summary", Classification: ToolRead, @@ -434,8 +434,8 @@ func (s *MCPServer) buildToolRegistry() { }, s.handleListAnnotations, s), // --- Rules --- - // See breadbox://rule-dsl for the condition grammar and breadbox://rules - // for the current ruleset. + // See breadbox://rule-dsl for the condition grammar and + // get_reference(kind=rules) for the current ruleset. makeToolDefLogged(ToolSpec{ Name: "create_transaction_rule", Title: "Create Rule", Classification: ToolWrite, Description: "Create one or more transaction rules for automatic categorization, tagging, or commenting. Pass `rules`: an array of 1..100 rule specs (a single rule is just a one-element array). Rules match condition trees against transactions during sync and fire in pipeline-stage order (priority ASC — lower = earlier). Pass `stage` (one of baseline|standard|refinement|override) per rule instead of a raw priority so rules from different agents compose predictably; stage resolves to priority 0/10/50/100. Earlier-stage rules' tag and category mutations feed later-stage rules' conditions, so rules compose: rule A tags 'coffee', rule B conditioned on tags-contains-coffee sets category — author such pipelines in one call. Set apply_retroactively=true on a rule to immediately back-fill it against existing transactions. Before creating, read the rules roster (get_reference kind=rules) to avoid duplicates; prefer `contains` over exact matches (bank feeds format merchant names inconsistently). Returns the created rules plus any per-item errors so a partial batch is recoverable. Full DSL: breadbox://rule-dsl.", @@ -674,33 +674,6 @@ func (s *MCPServer) registerResources(server *mcpsdk.Server) { Annotations: resourceAnnotations(audienceUserAndAssistant, 0.6, liveResourceModTime), }, s.handleAccountsResource) - server.AddResource(&mcpsdk.Resource{ - Name: "Categories", - Title: "Category Taxonomy", - URI: "breadbox://categories", - Description: "Two-level category taxonomy keyed by slug. Source for valid category_slug values when categorizing or authoring rules.", - MIMEType: "application/json", - Annotations: resourceAnnotations(audienceAssistantOnly, 0.5, liveResourceModTime), - }, s.handleCategoriesResource) - - server.AddResource(&mcpsdk.Resource{ - Name: "Tags", - Title: "Tag Vocabulary", - URI: "breadbox://tags", - Description: "Current tag vocabulary keyed by slug. The 'needs-review' tag is the review-queue handle.", - MIMEType: "application/json", - Annotations: resourceAnnotations(audienceAssistantOnly, 0.5, liveResourceModTime), - }, s.handleTagsResource) - - server.AddResource(&mcpsdk.Resource{ - Name: "Users", - Title: "Household Members", - URI: "breadbox://users", - Description: "Household members with their roles (admin, editor, viewer).", - MIMEType: "application/json", - Annotations: resourceAnnotations(audienceAssistantOnly, 0.5, liveResourceModTime), - }, s.handleUsersResource) - server.AddResource(&mcpsdk.Resource{ Name: "Sync Status", Title: "Connection Sync Status", @@ -710,14 +683,11 @@ func (s *MCPServer) registerResources(server *mcpsdk.Server) { Annotations: resourceAnnotations(audienceAssistantOnly, 0.6, liveResourceModTime), }, s.handleSyncStatusResource) - server.AddResource(&mcpsdk.Resource{ - Name: "Rules", - Title: "Transaction Rules", - URI: "breadbox://rules", - Description: "Currently registered transaction rules with their conditions, actions, trigger, priority, hit_count, and last_hit_at. Read before authoring new rules to avoid duplicates and to spot stale or never-matching rules.", - MIMEType: "application/json", - Annotations: resourceAnnotations(audienceAssistantOnly, 0.6, liveResourceModTime), - }, s.handleRulesResource) + // The categories / tags / users / rules reference resources were retired — + // they were exact duplicates of get_reference(kind=…) and, being pure + // agent-facing lookup tables (not human-attach material), carried no load on + // the resource surface. Read them via get_reference. overview / accounts / + // sync-status stay because a human plausibly attaches those snapshots. // Workflow guides — markdown, user-overridable via app_config. server.AddResource(&mcpsdk.Resource{ @@ -768,7 +738,7 @@ func (s *MCPServer) registerResources(server *mcpsdk.Server) { Name: "Account", Title: "Account Detail", URITemplate: "breadbox://account/{short_id}", - Description: "Single bank account with balance, currency, and the most recent 25 transactions. short_id is the 8-char base62 id surfaced by list_accounts.", + Description: "Single bank account with balance, currency, and the most recent 25 transactions. short_id is the 8-char base62 id surfaced by get_reference(kind=accounts).", MIMEType: "application/json", Annotations: resourceAnnotations(audienceUserAndAssistant, 0.7, liveResourceModTime), }, s.handleAccountTemplate) @@ -777,7 +747,7 @@ func (s *MCPServer) registerResources(server *mcpsdk.Server) { Name: "Household Member", Title: "Household Member Detail", URITemplate: "breadbox://user/{short_id}", - Description: "Single household member with their connected accounts. short_id is the 8-char base62 id surfaced by list_users.", + Description: "Single household member with their connected accounts. short_id is the 8-char base62 id surfaced by get_reference(kind=users).", MIMEType: "application/json", Annotations: resourceAnnotations(audienceUserAndAssistant, 0.7, liveResourceModTime), }, s.handleUserTemplate) diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index 8d2736206..1132b6498 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -22,7 +22,7 @@ type queryTransactionsInput struct { EndDate string `json:"end_date,omitempty" jsonschema:"End date (YYYY-MM-DD) exclusive"` AccountID string `json:"account_id,omitempty" jsonschema:"Filter by account ID"` UserID string `json:"user_id,omitempty" jsonschema:"Filter by user ID"` - CategorySlug string `json:"category_slug,omitempty" jsonschema:"Filter by category slug (parent slug includes all children). See breadbox://categories for the slug list."` + CategorySlug string `json:"category_slug,omitempty" jsonschema:"Filter by category slug (parent slug includes all children). See get_reference(kind=categories) for the slug list."` MinAmount *float64 `json:"min_amount,omitempty" jsonschema:"Minimum amount (positive=debit, negative=credit)"` MaxAmount *float64 `json:"max_amount,omitempty" jsonschema:"Maximum amount (positive=debit, negative=credit)"` Pending *bool `json:"pending,omitempty" jsonschema:"Filter by pending status"` @@ -30,7 +30,7 @@ type queryTransactionsInput struct { Search string `json:"search,omitempty" jsonschema:"Search transaction name or merchant. Comma-separated values are ORed (e.g. starbucks,amazon matches either)."` SearchMode string `json:"search_mode,omitempty" jsonschema:"How to match the search term: contains (default, substring match), words (all words must match, good for multi-word queries), fuzzy (typo-tolerant via trigram similarity)"` ExcludeSearch string `json:"exclude_search,omitempty" jsonschema:"Exclude transactions whose name or merchant matches this text. Comma-separated values are ORed. Use to filter out known merchants."` - Tags []string `json:"tags,omitempty" jsonschema:"Filter to transactions that have EVERY tag slug in this list (AND semantics). See breadbox://tags for the available vocabulary."` + Tags []string `json:"tags,omitempty" jsonschema:"Filter to transactions that have EVERY tag slug in this list (AND semantics). See get_reference(kind=tags) for the available vocabulary."` AnyTag []string `json:"any_tag,omitempty" jsonschema:"Filter to transactions that have AT LEAST ONE tag slug in this list (OR semantics)."` Limit int `json:"limit,omitempty" jsonschema:"Max results (default 50, max 500)"` Cursor string `json:"cursor,omitempty" jsonschema:"Pagination cursor from previous result"` diff --git a/prompts/mcp/instructions.md b/prompts/mcp/instructions.md index fd5f4a057..f567b4e53 100644 --- a/prompts/mcp/instructions.md +++ b/prompts/mcp/instructions.md @@ -1,15 +1,18 @@ Breadbox is a self-hosted financial-data aggregator server for households. It syncs transactions and other data via Plaid, Teller, or CSV imports into one unified database and exposes tools and resources for reviewing, enriching and interacting with financial data. ## Where to Start -Read `breadbox://overview` first (or call `get_overview` if your client doesn't support resources). Use resources and tools to interact with the data. +Read `get_reference(kind=overview)` first (or the `breadbox://overview` resource if your client supports resources) for a household snapshot, then use the tools and resources to interact with the data. -Bounded reference data has both a resource and a tool mirror with the same payload — pick whichever your client supports: -- `breadbox://accounts` ↔ `list_accounts` -- `breadbox://categories` ↔ `list_categories` -- `breadbox://tags` ↔ `list_tags` -- `breadbox://users` ↔ `list_users` -- `breadbox://sync-status` ↔ `get_sync_status` -- `breadbox://rules` ↔ `list_transaction_rules` +Bounded reference data is read through one tool — `get_reference(kind=…)`: +- `get_reference(kind=overview)` — household snapshot (scope, freshness, review backlog) +- `get_reference(kind=accounts)` — bank accounts (optional `user_id` filter) +- `get_reference(kind=categories)` — the category taxonomy (source of `category_slug` values) +- `get_reference(kind=tags)` — the tag vocabulary +- `get_reference(kind=users)` — household members +- `get_reference(kind=sync_status)` — per-connection sync status / freshness +- `get_reference(kind=rules)` — the transaction-rule roster (lean; `fields=all` for full) + +A few of these are also exposed as resources for clients with a resource/attach UI — `breadbox://overview`, `breadbox://accounts`, `breadbox://sync-status` — with the same payload as the matching `get_reference` kind. Per-entity drilldowns are exposed as resource templates (resolve a single short_id): - `breadbox://transaction/{short_id}` — full transaction + recent annotations diff --git a/prompts/mcp/review-guidelines.md b/prompts/mcp/review-guidelines.md index bcb83fae3..8f00b0081 100644 --- a/prompts/mcp/review-guidelines.md +++ b/prompts/mcp/review-guidelines.md @@ -25,7 +25,7 @@ RULE CREATION: - Fields: provider_name, provider_merchant_name, amount, provider_category_primary (raw provider category), provider_category_detailed, pending, provider, account_id, user_id, user_name BEFORE CREATING A RULE: -1. Read breadbox://rules to avoid duplicates +1. Read get_reference(kind=rules) to avoid duplicates 2. Use preview_rule to test your conditions — verify match count and review sample transactions 3. Query some transactions with fields=core,category to see what provider_category_primary values exist @@ -49,7 +49,7 @@ RULE PIPELINE STAGES & CONFLICTS: - Rules fire in priority-ASC order during sync (lower priority runs first; later stages observe earlier-stage tag/category mutations). - Pass `stage` (not raw `priority`) when authoring rules — `baseline` (broad defaults) → `standard` (default) → `refinement` (reacts to earlier stages) → `override` (last word). Stage resolves to priority 0/10/50/100. If both are supplied, raw priority wins. - Recommended mapping by pattern: `provider_category_primary` rules → `baseline` or `standard`; name-pattern rules (`contains` on `provider_name`) → `standard`; per-merchant rules → `refinement` or `override`. -- Check breadbox://rules before creating. If two rules can match the same transaction, the higher-stage one wins under last-writer semantics. +- Check get_reference(kind=rules) before creating. If two rules can match the same transaction, the higher-stage one wins under last-writer semantics. Use batch_create_rules (max 100) to create multiple rules efficiently. Prefer contains over exact match — bank feeds format merchant names inconsistently. diff --git a/prompts/mcp/rule-dsl.md b/prompts/mcp/rule-dsl.md index 300620163..4045202ed 100644 --- a/prompts/mcp/rule-dsl.md +++ b/prompts/mcp/rule-dsl.md @@ -91,7 +91,7 @@ If two rules can match the same transaction, the higher priority wins; ties reso ## Authoring checklist -1. Read `breadbox://rules` to avoid duplicates. +1. Read `get_reference(kind=rules)` to avoid duplicates. 2. `preview_rule` your conditions to verify match count and review a sample of matched transactions. 3. Pick the right priority band for the pattern type. 4. Use `category_slug` (never `category_id`). From 8c1866ff327de94ef82354119e58c5016a863178 Mon Sep 17 00:00:00 2001 From: Ricardo Canales Date: Mon, 15 Jun 2026 23:41:01 -0700 Subject: [PATCH 3/6] refactor(mcp): retire the accounts reference resource too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit breadbox://accounts was an exact duplicate of get_reference(kind=accounts) (where the old list_accounts tool folded). Agents read accounts via the tool; the per-account drilldown stays as the breadbox://account/{short_id} template. Resource surface 9 → 8: only overview + sync-status remain as data resources (human-attachable snapshots), alongside the 3 static-doc resources and 3 drilldown templates. - Drop the AddResource registration + handleAccountsResource. - Trim the resource/tool parity test to the two pairs left (sync_status, overview). - Scrub breadbox://accounts pointers from the server instructions and docs. Build + vet + unit + mcp integration green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/rules/mcp.md | 4 ++-- docs/mcp-server.md | 1 - docs/mcp-tools-reference.md | 2 +- internal/mcp/resources.go | 15 ++++-------- .../mcp/response_shapes_integration_test.go | 23 +++++-------------- internal/mcp/server.go | 20 ++++------------ internal/mcp/tools_reads.go | 16 ++++++------- prompts/mcp/instructions.md | 2 +- 8 files changed, 27 insertions(+), 56 deletions(-) diff --git a/.claude/rules/mcp.md b/.claude/rules/mcp.md index afab2e7bb..9e5a42871 100644 --- a/.claude/rules/mcp.md +++ b/.claude/rules/mcp.md @@ -102,8 +102,8 @@ Bounded reference data is read through **one tool**, `get_reference(kind=…)`, A **subset** is also exposed as `resources/*` for clients with a resource/attach UI (Claude.ai's paperclip menu, the Inspector picker), sharing the same service-layer call path so payloads stay in sync: -- `breadbox://overview`, `breadbox://accounts`, `breadbox://sync-status` — kept because a human plausibly attaches these snapshots. -- `breadbox://categories`, `://tags`, `://users`, `://rules` were **retired** — pure agent-facing lookup tables that no one attaches, and exact duplicates of the `get_reference` kinds. Read them via the tool. +- `breadbox://overview`, `breadbox://sync-status` — kept because a human plausibly attaches these snapshots. +- `breadbox://accounts`, `://categories`, `://tags`, `://users`, `://rules` were **retired** — exact duplicates of the `get_reference` kinds (and mostly pure agent-facing lookups no one attaches). Read them via the tool. So a new reference dataset gets a `kind` branch in `handleGetReference` by default; add a parallel resource handler in `resources.go` only if it's something a user would attach in chat (snapshot-like), not for pure lookup tables. diff --git a/docs/mcp-server.md b/docs/mcp-server.md index d11458572..66b46b13a 100644 --- a/docs/mcp-server.md +++ b/docs/mcp-server.md @@ -608,7 +608,6 @@ Bounded reference data (accounts, categories, tags, users, sync status, rules) i | Resource URI | MIME Type | Description | |---|---|---| | `breadbox://overview` | `application/json` | Live dataset summary (also `get_reference(kind=overview)`) | -| `breadbox://accounts` | `application/json` | Bank accounts (also `get_reference(kind=accounts)`) | | `breadbox://sync-status` | `application/json` | Connection sync status (also `get_reference(kind=sync_status)`) | | `breadbox://review-guidelines` | `text/markdown` | Guidelines for reviewing transactions and creating rules | | `breadbox://report-format` | `text/markdown` | Report structure templates and formatting guidelines | diff --git a/docs/mcp-tools-reference.md b/docs/mcp-tools-reference.md index 5b018394a..d3af2f632 100644 --- a/docs/mcp-tools-reference.md +++ b/docs/mcp-tools-reference.md @@ -71,7 +71,7 @@ Aggregated spending totals. Default date range: 30 days. ### get_reference (Read) -One tool that reads any bounded reference dataset by `kind`. (Folds the former `get_overview`, `list_accounts`, `list_categories`, `list_tags`, `list_users`, `get_sync_status`, and `list_transaction_rules` tools.) `overview`, `accounts`, and `sync_status` are also exposed as `breadbox://` resources for clients with an attach UI; `categories`, `tags`, `users`, and `rules` are tool-only (their resources were retired as duplicates). +One tool that reads any bounded reference dataset by `kind`. (Folds the former `get_overview`, `list_accounts`, `list_categories`, `list_tags`, `list_users`, `get_sync_status`, and `list_transaction_rules` tools.) `overview` and `sync_status` are also exposed as `breadbox://` resources for clients with an attach UI; `accounts`, `categories`, `tags`, `users`, and `rules` are tool-only (their resources were retired as duplicates). | Parameter | Type | Description | |-----------|------|-------------| diff --git a/internal/mcp/resources.go b/internal/mcp/resources.go index 93ef8e8a4..c842220dc 100644 --- a/internal/mcp/resources.go +++ b/internal/mcp/resources.go @@ -130,14 +130,6 @@ func jsonResourceResult(uri string, v any) (*mcpsdk.ReadResourceResult, error) { }, nil } -func (s *MCPServer) handleAccountsResource(ctx context.Context, _ *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { - accts, err := s.svc.ListAccounts(ctx, nil) - if err != nil { - return nil, fmt.Errorf("list accounts: %w", err) - } - return jsonResourceResult("breadbox://accounts", map[string]any{"accounts": accts}) -} - func (s *MCPServer) handleSyncStatusResource(ctx context.Context, _ *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { conns, err := s.svc.ListConnections(ctx, nil) if err != nil { @@ -146,9 +138,10 @@ func (s *MCPServer) handleSyncStatusResource(ctx context.Context, _ *mcpsdk.Read return jsonResourceResult("breadbox://sync-status", map[string]any{"connections": conns}) } -// The categories / tags / users / rules reference resources were retired as -// duplicates of get_reference(kind=…) — see registerResources. Their read -// paths still exist as the get_reference tool handlers in tools_reads.go. +// The accounts / categories / tags / users / rules reference resources were +// retired as duplicates of get_reference(kind=…) — see registerResources. +// Their read paths still exist as the get_reference tool handlers in +// tools_reads.go. // resourceAnnotations builds the standard *mcpsdk.Annotations payload used on // every Breadbox resource. lastModifiedFn returns the timestamp the client diff --git a/internal/mcp/response_shapes_integration_test.go b/internal/mcp/response_shapes_integration_test.go index ca440e9b4..d139ef2b6 100644 --- a/internal/mcp/response_shapes_integration_test.go +++ b/internal/mcp/response_shapes_integration_test.go @@ -960,14 +960,13 @@ func ptrString(s string) *string { return &s } // TestReferenceMirrorTools_ParityWithResources locks the dual-surface // contract for the reference datasets that still have BOTH a resource and a -// get_reference tool path: breadbox://accounts, ://sync-status, ://overview. -// Each pair must return the SAME payload via the SAME service call. A -// regression that diverges them — e.g. forgetting the resource envelope, or -// pointing the tool at a different service method — would let one surface -// drift from the other. +// get_reference tool path: breadbox://sync-status and ://overview. Each pair +// must return the SAME payload via the SAME service call. A regression that +// diverges them — e.g. forgetting the resource envelope, or pointing the tool +// at a different service method — would let one surface drift from the other. // -// (categories / tags / users / rules were retired as resources — they're -// read-only via get_reference now, so there's no pair left to compare.) +// (accounts / categories / tags / users / rules were retired as resources — +// they're read-only via get_reference now, so there's no pair left to compare.) // // The parity test reads each pair, ignores envelope keys (resource handlers // always wrap in {"": [...]}), and compares the inner payload byte- @@ -982,16 +981,6 @@ func TestReferenceMirrorTools_ParityWithResources(t *testing.T) { toolFn func() (*mcpsdk.CallToolResult, any, error) resourceFn func() (*mcpsdk.ReadResourceResult, error) }{ - { - name: "get_reference(accounts) <-> breadbox://accounts", - envelopeKey: "accounts", - toolFn: func() (*mcpsdk.CallToolResult, any, error) { - return f.svc.handleListAccounts(f.ctx, nil, listAccountsInput{}) - }, - resourceFn: func() (*mcpsdk.ReadResourceResult, error) { - return f.svc.handleAccountsResource(f.ctx, nil) - }, - }, { name: "get_reference(sync_status) <-> breadbox://sync-status", envelopeKey: "connections", diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 3e2b4c745..f6dfc7e9f 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -664,16 +664,6 @@ func (s *MCPServer) registerResources(server *mcpsdk.Server) { Annotations: resourceAnnotations(audienceUserAndAssistant, 1.0, liveResourceModTime), }, s.handleOverviewResource) - // Live state resources — replace what would otherwise be list_* tool calls. - server.AddResource(&mcpsdk.Resource{ - Name: "Accounts", - Title: "Bank Accounts", - URI: "breadbox://accounts", - Description: "Bank accounts (checking, savings, credit cards, loans, investments) with balances, institution names, and currency.", - MIMEType: "application/json", - Annotations: resourceAnnotations(audienceUserAndAssistant, 0.6, liveResourceModTime), - }, s.handleAccountsResource) - server.AddResource(&mcpsdk.Resource{ Name: "Sync Status", Title: "Connection Sync Status", @@ -683,11 +673,11 @@ func (s *MCPServer) registerResources(server *mcpsdk.Server) { Annotations: resourceAnnotations(audienceAssistantOnly, 0.6, liveResourceModTime), }, s.handleSyncStatusResource) - // The categories / tags / users / rules reference resources were retired — - // they were exact duplicates of get_reference(kind=…) and, being pure - // agent-facing lookup tables (not human-attach material), carried no load on - // the resource surface. Read them via get_reference. overview / accounts / - // sync-status stay because a human plausibly attaches those snapshots. + // The accounts / categories / tags / users / rules reference resources were + // retired — they were exact duplicates of get_reference(kind=…) and pure + // agent-facing lookups, carrying no load on the resource surface. Read them + // via get_reference. Only overview + sync-status stay as data resources + // (a human plausibly attaches those snapshots). // Workflow guides — markdown, user-overridable via app_config. server.AddResource(&mcpsdk.Resource{ diff --git a/internal/mcp/tools_reads.go b/internal/mcp/tools_reads.go index 4879c6022..05577ce97 100644 --- a/internal/mcp/tools_reads.go +++ b/internal/mcp/tools_reads.go @@ -11,14 +11,14 @@ import ( mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp" ) -// Tools in this file mirror the bounded reference resources registered in -// resources.go (breadbox://accounts, ://categories, ://tags, ://users, -// ://sync-status, ://rules, ://overview). Resources are the preferred surface -// for application-driven hosts (Claude.ai's paperclip menu, Inspector), but -// not every MCP client implements the resources/* methods — clients without -// resource support need tool fallbacks for the same reads. Each handler here -// calls into the same service path the matching resource handler uses, with -// a tool-shaped envelope. +// Tools in this file back the bounded reference reads, surfaced to agents +// through get_reference(kind=…). They're the canonical (and for most kinds, +// only) way to read this data: accounts, categories, tags, users, sync status, +// and the rule roster. A subset (overview, sync-status) is ALSO exposed as a +// breadbox:// resource for hosts with an attach UI (Claude.ai's paperclip menu, +// Inspector); those resource handlers in resources.go call the same service +// path, so the two surfaces never drift. Each handler here returns a +// tool-shaped envelope. // --- Input types --- diff --git a/prompts/mcp/instructions.md b/prompts/mcp/instructions.md index f567b4e53..f18058cee 100644 --- a/prompts/mcp/instructions.md +++ b/prompts/mcp/instructions.md @@ -12,7 +12,7 @@ Bounded reference data is read through one tool — `get_reference(kind=…)`: - `get_reference(kind=sync_status)` — per-connection sync status / freshness - `get_reference(kind=rules)` — the transaction-rule roster (lean; `fields=all` for full) -A few of these are also exposed as resources for clients with a resource/attach UI — `breadbox://overview`, `breadbox://accounts`, `breadbox://sync-status` — with the same payload as the matching `get_reference` kind. +A couple of these are also exposed as resources for clients with a resource/attach UI — `breadbox://overview` and `breadbox://sync-status` — with the same payload as the matching `get_reference` kind. Per-entity drilldowns are exposed as resource templates (resolve a single short_id): - `breadbox://transaction/{short_id}` — full transaction + recent annotations From 34d73998c183aa4a447785128033a8682ee4e014 Mon Sep 17 00:00:00 2001 From: Ricardo Canales Date: Tue, 16 Jun 2026 00:08:29 -0700 Subject: [PATCH 4/6] refactor(mcp): drop MCP resources; reads as tools; get_reference serves docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A deliberate reshape of the read surface: - Retire the MCP resources/* surface entirely (and the resource templates). Resources were invisible to clients that can't resources/list (e.g. Claude.ai), so everything is a tool now. Deleted resources.go, registerResources, and all resource/template handlers + their tests. - Re-introduce the seven bounded reference reads as individual tools: get_overview, list_accounts, list_categories, list_users, list_tags, get_sync_status, list_transaction_rules (handlers already lived in tools_reads.go; this just registers them again). - Repurpose get_reference to serve the operating-guidance docs by kind: instructions | rule-dsl | review-guidelines | report-format. Returns markdown; instructions/review-guidelines/report-format honor app_config overrides, rule-dsl is the fixed grammar. This is how resource-incapable clients now reach the guidance that used to live only in breadbox:// markdown resources. Registry: 34 tools, 0 resources. Per-entity drilldown templates are gone — reconstruct from query_transactions + list_annotations / list_accounts. Also scrubbed every breadbox:// / get_reference(kind=) / count_transactions / batch_create_rules reference across tool descriptions, input schemas, the MCP instructions + agent strategy prompts, the admin tool-group map, and the docs. Build + vet + unit + mcp/service/api integration all green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/rules/mcp.md | 28 +- docs/architecture.md | 11 +- docs/mcp-server.md | 84 ++--- docs/mcp-tools-reference.md | 69 ++-- internal/admin/settings_agents.go | 4 +- internal/api/rules.go | 10 +- .../resource_templates_integration_test.go | 276 --------------- internal/mcp/resource_templates_test.go | 67 ---- internal/mcp/resources.go | 321 ------------------ internal/mcp/resources_test.go | 69 ---- .../mcp/response_shapes_integration_test.go | 92 +---- internal/mcp/server.go | 165 +++------ internal/mcp/server_test.go | 17 +- internal/mcp/tools.go | 4 +- internal/mcp/tools_reads.go | 69 ++-- internal/mcp/tools_transaction_metadata.go | 2 +- internal/service/overview.go | 6 +- .../pages/design_prompt_preview.templ | 4 +- prompts/agents/strategy-anomaly-detection.md | 2 +- prompts/agents/strategy-bulk-catchup.md | 2 +- prompts/agents/strategy-bulk-review.md | 2 +- prompts/agents/strategy-initial-setup.md | 6 +- .../agents/strategy-large-charge-sentinel.md | 2 +- prompts/agents/strategy-quick-review.md | 2 +- prompts/agents/strategy-routine-review.md | 2 +- prompts/agents/strategy-rule-foundation.md | 4 +- prompts/agents/strategy-spending-report.md | 2 +- prompts/mcp/instructions.md | 33 +- prompts/mcp/review-guidelines.md | 6 +- prompts/mcp/rule-dsl.md | 4 +- 30 files changed, 247 insertions(+), 1118 deletions(-) delete mode 100644 internal/mcp/resource_templates_integration_test.go delete mode 100644 internal/mcp/resource_templates_test.go delete mode 100644 internal/mcp/resources.go diff --git a/.claude/rules/mcp.md b/.claude/rules/mcp.md index 9e5a42871..8cfe643f8 100644 --- a/.claude/rules/mcp.md +++ b/.claude/rules/mcp.md @@ -67,9 +67,9 @@ Good descriptions are load-bearing — they're the only documentation agents see `MCPServerConfig.Instructions` is domain-rich onboarding: data model overview, amount conventions, category system, recommended query patterns. Templates in `internal/mcp/templates.go` (`spend_review`, `monthly_analysis`, `reporting`) are presets users can load and edit. User-edited text stored as `mcp_custom_instructions` in `app_config`. -## Resource +## No MCP resources -`breadbox://overview` returns live stats: users, accounts by type, connections with counts, 30-day spending summary with top 5 categories, pending transaction count. +The `resources/*` surface (and resource templates) was retired entirely — it was invisible on clients that can't `resources/list` (e.g. Claude.ai), so everything is a tool. Don't re-add `AddResource` / `AddResourceTemplate`; add a read tool instead. See "Reference data" and "Guidance docs" below. ## Permissions admin page @@ -96,23 +96,17 @@ Several writes are deliberately compound so the registry stays small and an agen When folding a tool, prefer reusing the existing service methods from the compound handler over re-implementing the write — the MCP layer is where the consolidation lives. -## Reference data: dual surface (resources + one tool) +## Reference data: one tool per dataset -Bounded reference data is read through **one tool**, `get_reference(kind=…)`, dispatching on `kind` (`overview` \| `accounts` \| `categories` \| `tags` \| `users` \| `sync_status` \| `rules`) to the per-kind handlers in `tools_reads.go` (`handleGetOverview`, `handleListAccounts`, …). Optional `user_id` (accounts) and `fields` (rules) ride through. Filtered/sorted rule analysis stays in `query_transaction_rules`; `get_reference kind=rules` returns the lean roster. +Bounded reference data is read through dedicated tools, handlers in `tools_reads.go`: -A **subset** is also exposed as `resources/*` for clients with a resource/attach UI (Claude.ai's paperclip menu, the Inspector picker), sharing the same service-layer call path so payloads stay in sync: +- `get_overview` (`handleGetOverview`) +- `list_accounts` (`handleListAccounts`; optional `user_id`) +- `list_categories`, `list_tags`, `list_users`, `get_sync_status` +- `list_transaction_rules` (`handleListTransactionRules`; lean `summary` by default, `fields=all` for full). Filtered/sorted rule analysis lives in `query_transaction_rules`. -- `breadbox://overview`, `breadbox://sync-status` — kept because a human plausibly attaches these snapshots. -- `breadbox://accounts`, `://categories`, `://tags`, `://users`, `://rules` were **retired** — exact duplicates of the `get_reference` kinds (and mostly pure agent-facing lookups no one attaches). Read them via the tool. +To add a new reference dataset, register a new read tool here — there's no resource counterpart to keep in sync anymore. -So a new reference dataset gets a `kind` branch in `handleGetReference` by default; add a parallel resource handler in `resources.go` only if it's something a user would attach in chat (snapshot-like), not for pure lookup tables. +## Guidance docs: `get_reference(kind=…)` -## Resource templates (drill-downs) - -Per-entity detail views are exposed as MCP resource templates registered with `Server.AddResourceTemplate`: - -- `breadbox://transaction/{short_id}` — `{transaction, annotations}` -- `breadbox://account/{short_id}` — `{account, recent_transactions}` (capped at 25) -- `breadbox://user/{short_id}` — `{user, accounts}` - -Template handlers parse the trailing `{short_id}` via `extractTemplateParam`, resolve through the standard service `Get*` methods (which accept either UUID or short_id), and return `mcpsdk.ResourceNotFoundError(uri)` on miss. URIs come back to chat as clickable items via `resource_link` content blocks emitted by tools (planned in a follow-up PR). +The near-static markdown that teaches an agent how to drive the server — formerly `breadbox://` markdown resources — is served by the `get_reference` tool (`handleGetReference` in `tools_reads.go`). `kind` ∈ `instructions` | `rule-dsl` | `review-guidelines` | `report-format`; it returns a markdown text block. `instructions` / `review-guidelines` / `report-format` honor the operator's `app_config` overrides (falling back to the embedded `prompts/mcp/*.md` defaults via the `Default*` vars in `server.go`); `rule-dsl` is the fixed grammar. Per-entity drilldowns (transaction + annotations, account + recent txns, user + accounts) are no longer a single call — reconstruct them from `query_transactions` + `list_annotations` / `list_accounts`. diff --git a/docs/architecture.md b/docs/architecture.md index e909d0417..9ec378511 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -235,12 +235,11 @@ breadbox/ │ │ └── health.go GET /health │ │ │ ├── mcp/ -│ │ ├── server.go MCP server construction; registers tools + resources; mounts on router -│ │ ├── tools.go Tool handler functions (list_accounts, query_transactions, etc.) -│ │ └── resources.go MCP resource handlers: -│ │ - breadbox://overview (live dataset stats, JSON) -│ │ - breadbox://review-guidelines (review/rule guidelines, markdown) -│ │ - breadbox://report-format (report structure templates, markdown) +│ │ ├── server.go MCP server construction; registers tools; mounts on router (no MCP resources) +│ │ ├── tools.go Tool handler functions (query_transactions, update_transactions, etc.) +│ │ └── tools_reads.go Reference-data read tools (get_overview, list_accounts, …) + +│ │ get_reference (serves instructions / rule-dsl / +│ │ review-guidelines / report-format guidance docs as markdown) │ │ │ ├── admin/ │ │ ├── router.go Mounts all /admin/ routes; session auth middleware diff --git a/docs/mcp-server.md b/docs/mcp-server.md index 66b46b13a..859620d31 100644 --- a/docs/mcp-server.md +++ b/docs/mcp-server.md @@ -13,7 +13,7 @@ MCP tools are thin wrappers around the same service and query layer that backs t 1. [MCP Server Configuration](#1-mcp-server-configuration) 2. [Client Configuration](#2-client-configuration) 3. [Tool Definitions](#3-tool-definitions) -4. [MCP Resources](#4-mcp-resources) +4. [Operating-Guidance Docs](#4-operating-guidance-docs) 5. [Error Handling](#5-error-handling) 6. [Pagination Strategy](#6-pagination-strategy) 7. [Relationship to REST API](#7-relationship-to-rest-api) @@ -81,7 +81,7 @@ The MCP server does not manage its own database connection. It receives the same ### Server Capabilities -The server advertises tools and resources. Prompts are not used. Server instructions are provided via `ServerOptions.Instructions` (customizable from the MCP admin page, with a comprehensive default). +The server advertises tools only — MCP resources and prompts are not used. Server instructions are provided via `ServerOptions.Instructions` (customizable from the MCP admin page, with a comprehensive default). ```go server := mcpsdk.NewServer( @@ -90,7 +90,7 @@ server := mcpsdk.NewServer( ) ``` -Three resources are registered on every server instance via `registerResources()` (see [Section 4](#4-mcp-resources)). +The operating-guidance docs that previously shipped as resources are now served by the `get_reference` tool (see [Section 4](#4-operating-guidance-docs)). --- @@ -225,11 +225,9 @@ On error, the MCP SDK's `IsError: true` flag is set and the content is: --- -### Tool: `get_reference(kind=accounts)` +### Tool: `list_accounts` -Bank accounts are read via `get_reference(kind=accounts)` (optional `user_id` filter) — one of the bounded reference datasets behind the single `get_reference` tool. There is no standalone `list_accounts` tool. - -**Description (shown to LLM):** List all bank accounts with their current balances. Optionally filter by family member. Returns one object per account including institution, type, and balance. +**Description (shown to LLM):** List all bank accounts with their current balances. Optionally filter by family member (`user_id`). Returns one object per account including institution, type, and balance. #### Input Schema @@ -422,9 +420,9 @@ Counting is folded into `query_transactions`: pass `count_only: true` with the s --- -### Tool: `get_reference(kind=users)` +### Tool: `list_users` -**Description (shown to LLM):** List all family members tracked in Breadbox. Users are labels for account ownership — they are not login accounts. Read them via `get_reference(kind=users)`; use the returned IDs to filter accounts (`get_reference(kind=accounts)`) or `query_transactions` by family member. +**Description (shown to LLM):** List all family members tracked in Breadbox. Users are labels for account ownership — they are not login accounts. Use the returned IDs to filter `list_accounts` or `query_transactions` by family member. #### Input Schema @@ -465,9 +463,7 @@ Calls `GET /api/v1/users`. Returns all users without pagination (family sizes ar --- -### Tool: `get_reference(kind=sync_status)` - -Connection sync status is read via `get_reference(kind=sync_status)` — one of the bounded reference datasets behind the single `get_reference` tool. There is no standalone `get_sync_status` tool. +### Tool: `get_sync_status` **Description (shown to LLM):** Returns the health status of all bank connections — whether they are syncing successfully, when they last synced, and whether any connections need re-authentication. Use this to diagnose why transactions might be missing or stale. @@ -531,7 +527,7 @@ Calls `GET /api/v1/connections`. Each element in the response corresponds to one ### Tool: `trigger_sync` -**Description (shown to LLM):** Manually trigger a data sync to fetch the latest transactions and balances from the bank. Syncs all connections by default, or a specific connection if `connection_id` is provided. The sync runs asynchronously — this tool returns immediately after enqueuing the sync, not after it completes. Check `get_reference(kind=sync_status)` afterward to monitor progress. +**Description (shown to LLM):** Manually trigger a data sync to fetch the latest transactions and balances from the bank. Syncs all connections by default, or a specific connection if `connection_id` is provided. The sync runs asynchronously — this tool returns immediately after enqueuing the sync, not after it completes. Check `get_sync_status` afterward to monitor progress. #### Input Schema @@ -567,13 +563,11 @@ Calls `POST /api/v1/sync` with an optional `connection_id` body parameter. The R - **Unknown `connection_id`:** Returns an error: `{ "error": "Connection conn_xxx not found." }` with `IsError: true`. - **Sync already in progress:** The service layer handles deduplication. A second trigger for the same connection while one is running is a no-op; the tool returns a success message indicating the sync was already underway. -- **Connection in `error` state:** The sync is attempted. If re-authentication is required, the sync will fail again and `get_reference(kind=sync_status)` will reflect the error. +- **Connection in `error` state:** The sync is attempted. If re-authentication is required, the sync will fail again and `get_sync_status` will reflect the error. --- -### Tool: `get_reference(kind=categories)` - -The category taxonomy is read via `get_reference(kind=categories)` — one of the bounded reference datasets behind the single `get_reference` tool. There is no standalone `list_categories` tool. +### Tool: `list_categories` **Description (shown to LLM):** List all transaction categories. Useful for understanding the category taxonomy before filtering transactions. @@ -599,50 +593,22 @@ Calls `GET /api/v1/categories`. Returns the same `[]CategoryPair` response. --- -## 4. MCP Resources +## 4. Operating-Guidance Docs -MCP resources are passive context documents the LLM can read, similar to files. They do not execute queries at call time in the same on-demand way tools do. They're registered via `s.registerResources()` in `internal/mcp/server.go`, with handlers in `internal/mcp/resources.go`. +**MCP resources (`resources/*`) and resource templates were retired entirely.** They were invisible on clients that can't `resources/list` (e.g. Claude.ai), so everything an agent reads is now a tool. There is no `registerResources()`. -Bounded reference data (accounts, categories, tags, users, sync status, rules) is read through the **`get_reference(kind=…)` tool**, not resources — only snapshot-like / static-doc resources remain (the lookup-table resources were retired as tool duplicates). The resources that stay: +The near-static markdown that teaches an agent how to drive the server — previously the `breadbox://` markdown resources — is served by the **`get_reference(kind=…)` tool**, which returns a markdown text block: -| Resource URI | MIME Type | Description | +| `kind` | Source | Description | |---|---|---| -| `breadbox://overview` | `application/json` | Live dataset summary (also `get_reference(kind=overview)`) | -| `breadbox://sync-status` | `application/json` | Connection sync status (also `get_reference(kind=sync_status)`) | -| `breadbox://review-guidelines` | `text/markdown` | Guidelines for reviewing transactions and creating rules | -| `breadbox://report-format` | `text/markdown` | Report structure templates and formatting guidelines | -| `breadbox://rule-dsl` | `text/markdown` | Condition grammar / action types for authoring rules | -| `breadbox://transaction/{short_id}`, `breadbox://account/{short_id}`, `breadbox://user/{short_id}` | `application/json` | Per-entity drilldown templates | - -### Resource: `breadbox://overview` - -Returns a lightweight summary of the household's financial data — users, connections with account counts, accounts by type, transaction counts, date range, 30-day spending summary with top categories, and pending transaction count. This gives an LLM ambient context about the financial state without a round-trip tool call. - -Backed by `service.GetOverviewStats()`. - -```json -{ - "total_accounts": 5, - "total_transactions": 2847, - "date_range": { - "earliest": "2024-06-15", - "latest": "2026-03-07" - }, - "providers": ["plaid", "teller"] -} -``` - -### Resource: `breadbox://review-guidelines` - -Returns guidelines for reviewing transactions and creating transaction rules. Agents should read this before processing any reviews or creating rules. - -Content is user-editable via the MCP Settings admin page (`mcp_review_guidelines` in `app_config`). Falls back to `DefaultReviewGuidelines` (a comprehensive constant in `server.go`) when no custom guidelines are saved. - -### Resource: `breadbox://report-format` +| `instructions` | `mcp_custom_instructions` → `DefaultInstructions` | Data model + conventions overview (same text as `ServerOptions.Instructions`). | +| `rule-dsl` | `DefaultRuleDSL` (fixed) | Condition grammar, action types, pipeline-stage ordering, sync-vs-retroactive semantics. Read before authoring rules. | +| `review-guidelines` | `mcp_review_guidelines` → `DefaultReviewGuidelines` | Principles for reviewing transactions and creating rules. Read before working the needs-review queue. | +| `report-format` | `mcp_report_format` → `DefaultReportFormat` | Report structure + formatting for `submit_report`. | -Returns report structure templates and formatting guidelines. Agents should read this before submitting reports via the `submit_report` tool. +`instructions` / `review-guidelines` / `report-format` honor the operator's `app_config` override and fall back to the embedded `prompts/mcp/*.md` default (the `Default*` vars in `server.go`). The handler is `handleGetReference` in `internal/mcp/tools_reads.go`. -Content is user-editable via the MCP Settings admin page (`mcp_report_format` in `app_config`). Falls back to `DefaultReportFormat` (a comprehensive constant in `server.go`) when no custom format is saved. +Bounded reference **data** (overview, accounts, categories, tags, users, sync status, rules) is read through the dedicated read tools in [Section 3](#3-tool-definitions) (`get_overview`, `list_accounts`, …), not `get_reference`. The former per-entity drilldown templates are gone; reconstruct a transaction's detail from `query_transactions` + `list_annotations`, an account's from `list_accounts` + `query_transactions(account_id)`, etc. --- @@ -782,13 +748,13 @@ Both paths call the same `service.QueryTransactions` function. The MCP tool hand | MCP Tool | REST Endpoint | |---|---| -| `get_reference(kind=accounts)` | `GET /api/v1/accounts` | +| `list_accounts` | `GET /api/v1/accounts` | | `query_transactions` | `GET /api/v1/transactions` | | `query_transactions(count_only=true)` | `GET /api/v1/transactions/count` | -| `get_reference(kind=users)` | `GET /api/v1/users` | -| `get_reference(kind=sync_status)` | `GET /api/v1/connections` | +| `list_users` | `GET /api/v1/users` | +| `get_sync_status` | `GET /api/v1/connections` | | `trigger_sync` | `POST /api/v1/sync` | -| `get_reference(kind=categories)` | `GET /api/v1/categories` | +| `list_categories` | `GET /api/v1/categories` | ### No MCP-Specific Data Access diff --git a/docs/mcp-tools-reference.md b/docs/mcp-tools-reference.md index d3af2f632..f4836052d 100644 --- a/docs/mcp-tools-reference.md +++ b/docs/mcp-tools-reference.md @@ -67,26 +67,52 @@ Aggregated spending totals. Default date range: 30 days. --- -## Account & Status Tools +## Reference Data Tools + +> MCP resources were retired — these are plain tools (the only way to read this data). + +### get_overview (Read) + +Household snapshot: scope (users, accounts, currencies), freshness (latest sync, errored connections, recent transactions), pending-review backlog. Read once at the top of a session to ground later filters. + +### list_accounts (Read) + +Bank accounts with name, type, balances, currency, and connection. Optional `user_id` filter scopes to one household member. + +### list_categories (Read) + +The 2-level category taxonomy (slug, display name, parent, icon, color). Use the slugs as the canonical `category_slug` handle on filters and writes. + +### list_users (Read) + +Household members with role and `short_id` (the `short_id` is the `user_id` on filters). + +### list_tags (Read) + +The registered tag vocabulary (slugs). New slugs auto-register when `update_transactions` adds them. + +### get_sync_status (Read) + +Per-connection sync status (provider, `active`|`error`|`pending_reauth`|`disconnected`), last-sync time, last error. Check before trusting freshness. + +### list_transaction_rules (Read) + +The transaction-rule roster. Filter by `category_slug`, `enabled`, or name `search`. Lean `summary` projection by default (no conditions/actions trees); `fields=all` for full definitions. For trigger/creator/hit-count filters or sorting, use `query_transaction_rules`. ### get_reference (Read) -One tool that reads any bounded reference dataset by `kind`. (Folds the former `get_overview`, `list_accounts`, `list_categories`, `list_tags`, `list_users`, `get_sync_status`, and `list_transaction_rules` tools.) `overview` and `sync_status` are also exposed as `breadbox://` resources for clients with an attach UI; `accounts`, `categories`, `tags`, `users`, and `rules` are tool-only (their resources were retired as duplicates). +Read an operating-guidance doc by `kind` — the near-static markdown that explains how to drive the server (formerly `breadbox://` markdown resources). Returns markdown. | Parameter | Type | Description | |-----------|------|-------------| -| `kind` | string | **Required.** One of `overview`, `accounts`, `categories`, `tags`, `users`, `sync_status`, `rules`. | -| `user_id` | string | Only for `kind=accounts`: scope to one household member. | -| `fields` | string | Only for `kind=rules`: `summary` (default, omits conditions/actions) or `all`. | - -Per kind: -- `overview` — household snapshot (scope, freshness, pending-review backlog). Read once at the top of a session. -- `accounts` — bank accounts with name, type, balances, connection info. -- `categories` — the 2-level category hierarchy (slug, display name, parent, icon, color). -- `tags` — the registered tag vocabulary. -- `users` — household members; the `short_id` is the `user_id` on filters. -- `sync_status` — per-connection last-sync time, status, errors. -- `rules` — the transaction-rule roster (lean summary; `fields=all` for full definitions). For filtered/sorted rule analysis use `query_transaction_rules`. +| `kind` | string | **Required.** One of `instructions`, `rule-dsl`, `review-guidelines`, `report-format`. | + +- `instructions` — data model + conventions overview. +- `rule-dsl` — the transaction-rule condition grammar, action types, pipeline-stage ordering, sync-vs-retroactive semantics. Read before authoring rules. +- `review-guidelines` — principles for reviewing transactions and creating rules. Read before working the needs-review queue. +- `report-format` — structure + formatting conventions for `submit_report`. + +`instructions` / `review-guidelines` / `report-format` reflect operator customization (`app_config`); `rule-dsl` is the fixed grammar. ### list_workflows (Read) @@ -148,7 +174,7 @@ Import categories from TSV format. Creates or updates categories. ### list_tags (Read) -The tag vocabulary is read via `get_reference(kind=tags)`. +The tag vocabulary — see **Reference Data Tools → `list_tags`** above. ### add_transaction_tag (Write) @@ -318,7 +344,7 @@ Returns `{ created, failed, rules: [{rule, retroactive_matches?}], errors }` so ### list_transaction_rules (Read) -The rule roster is read via `get_reference(kind=rules)` (lean `summary` projection; `fields=all` for full `conditions`/`actions`). For filtered/sorted analysis use `query_transaction_rules` below. +The rule roster — see **Reference Data Tools → `list_transaction_rules`** above. For filtered/sorted analysis use `query_transaction_rules` below. ### query_transaction_rules (Read) @@ -499,12 +525,7 @@ Trigger a manual sync for all active connections. ## Resources -In addition to tools, Breadbox exposes three MCP resources that provide passive context: - -| URI | Description | -|-----|-------------| -| `breadbox://overview` | Live dataset summary (users, accounts, spending, pending transactions) | -| `breadbox://review-guidelines` | Guidelines for reviewing transactions and creating rules | -| `breadbox://report-format` | Report structure templates and formatting guidelines | +Breadbox exposes **no MCP resources** — they were retired entirely (invisible on clients that can't `resources/list`). Everything is a tool: -Agents should read `breadbox://overview` at the start of a session for ambient context about the household's financial data. +- Ambient context: call `get_overview` at the start of a session. +- Operating-guidance docs (formerly `breadbox://` markdown resources): `get_reference(kind=instructions|rule-dsl|review-guidelines|report-format)`. diff --git a/internal/admin/settings_agents.go b/internal/admin/settings_agents.go index 4b06ca16c..8e70b0cde 100644 --- a/internal/admin/settings_agents.go +++ b/internal/admin/settings_agents.go @@ -36,12 +36,13 @@ func AgentsSettingsHandler(svc *service.Service, mcpServer *breadboxmcp.MCPServe // Group ordering matches the previous MCP-settings layout // so user muscle memory carries over. toolGroups := map[string]string{ + "get_overview": "Accounts & Data", "list_accounts": "Accounts & Data", "list_users": "Accounts & Data", "get_sync_status": "Accounts & Data", + "get_reference": "Accounts & Data", "trigger_sync": "Accounts & Data", "query_transactions": "Transactions", - "count_transactions": "Transactions", "transaction_summary": "Transactions", "list_categories": "Categories", "export_categories": "Categories", @@ -63,7 +64,6 @@ func AgentsSettingsHandler(svc *service.Service, mcpServer *breadboxmcp.MCPServe "create_transaction_rule": "Rules", "update_transaction_rule": "Rules", "delete_transaction_rule": "Rules", - "batch_create_rules": "Rules", "apply_rules": "Rules", "preview_rule": "Rules", "list_account_links": "Account Links", diff --git a/internal/api/rules.go b/internal/api/rules.go index f6321f3af..18db1148e 100644 --- a/internal/api/rules.go +++ b/internal/api/rules.go @@ -247,8 +247,8 @@ func ApplyAllRulesHandler(svc *service.Service) http.HandlerFunc { } } -// batchCreateRulesRequest mirrors the MCP `batch_create_rules` tool's input -// shape 1:1 (snake_case, identical field names). +// batchCreateRulesRequest mirrors the MCP `create_transaction_rule` tool's +// `rules` array (snake_case, identical per-item field names). type batchCreateRulesRequest struct { Rules []batchCreateRuleItem `json:"rules"` OnError string `json:"on_error"` @@ -256,7 +256,7 @@ type batchCreateRulesRequest struct { // batchCreateRuleItem is a single rule create payload, mirroring the // per-item shape used by both the single-create REST handler and the MCP -// batch_create_rules tool. +// create_transaction_rule tool's `rules` array. type batchCreateRuleItem struct { Name string `json:"name"` Conditions *service.Condition `json:"conditions"` @@ -282,8 +282,8 @@ type batchCreateRuleResultError struct { Message string `json:"message"` } -// BatchCreateRulesHandler is the REST sibling of the MCP `batch_create_rules` -// tool. Up to 50 rules per call. Per-op errors live inside `results[]`; the +// BatchCreateRulesHandler is the REST sibling of the MCP +// `create_transaction_rule` `rules` array. Up to 50 rules per call. Per-op errors live inside `results[]`; the // top-level call returns 200 unless the input itself is malformed (empty/ // oversized rules array), in which case it returns `400 INVALID_PARAMETER`. // diff --git a/internal/mcp/resource_templates_integration_test.go b/internal/mcp/resource_templates_integration_test.go deleted file mode 100644 index d406963c2..000000000 --- a/internal/mcp/resource_templates_integration_test.go +++ /dev/null @@ -1,276 +0,0 @@ -//go:build integration && !lite - -package mcp - -// Integration tests for the per-entity resource templates introduced in -// stack/mcp-overhaul/04-resource-templates. Templates resolve URIs of the form -// breadbox:///{short_id} into a {entity, related} envelope agents can -// drill into. The asserts here lock: -// -// - the request URI is echoed onto Contents[0].URI -// - MIMEType is application/json (jsonResourceResult contract) -// - the envelope keys match the documented shape per template -// - the inner row's id field carries the 8-char short_id (via compactIDsBytes) -// - related slices are populated for the seeded scenario -// - missing entities surface the canonical -32002 ResourceNotFound error code -// -// The recent-transactions cap on the account template is exercised by seeding -// 30 transactions and asserting the response is truncated to -// templateRecentTransactionLimit. - -import ( - "encoding/json" - "errors" - "fmt" - "testing" - - mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp" - - "breadbox/internal/testutil" - - "github.com/modelcontextprotocol/go-sdk/jsonrpc" -) - -// --- Tests --- - -// TestTransactionTemplate_HappyPath exercises breadbox://transaction/{short_id}. -// Asserts envelope shape, URI echo, JSON mimetype, the 8-char compact id on -// the transaction row, and that the seeded tag/rule produced annotation rows. -func TestTransactionTemplate_HappyPath(t *testing.T) { - f := seedFixtures(t) - - // Resolve the seeded transaction's short_id via the service so the URI - // uses the public 8-char form (the template advertises {short_id}). - txn, err := f.svc.svc.GetTransaction(f.ctx, f.txnID) - if err != nil { - t.Fatalf("GetTransaction: %v", err) - } - uri := "breadbox://transaction/" + txn.ShortID - - res, err := f.svc.handleTransactionTemplate(f.ctx, &mcpsdk.ReadResourceRequest{ - Params: &mcpsdk.ReadResourceParams{URI: uri}, - }) - if err != nil { - t.Fatalf("handleTransactionTemplate: %v", err) - } - if len(res.Contents) != 1 { - t.Fatalf("expected 1 content block, got %d", len(res.Contents)) - } - c := res.Contents[0] - if c.URI != uri { - t.Errorf("URI = %q, want %q", c.URI, uri) - } - if c.MIMEType != "application/json" { - t.Errorf("MIMEType = %q, want application/json", c.MIMEType) - } - - var out map[string]any - if err := json.Unmarshal([]byte(c.Text), &out); err != nil { - t.Fatalf("unmarshal: %v\nraw=%s", err, c.Text) - } - requireKeys(t, "transaction-template", out, "transaction", "annotations") - - row := asObject(t, "transaction-template.transaction", out["transaction"]) - id, _ := row["id"].(string) - if len(id) != 8 { - t.Errorf("transaction.id = %q (len=%d); expected 8-char short", id, len(id)) - } - requireAbsent(t, "transaction-template.transaction", row, "short_id") - - anns := asArray(t, "transaction-template.annotations", out["annotations"]) - if len(anns) == 0 { - t.Fatal("expected at least one annotation (seeded tag + rule)") - } - for i, raw := range anns { - ann := asObject(t, fmt.Sprintf("annotations[%d]", i), raw) - requireKeys(t, fmt.Sprintf("annotations[%d]", i), ann, "kind", "action") - } -} - -// TestAccountTemplate_HappyPath exercises breadbox://account/{short_id}. -// Asserts the envelope, the compact id on the account row, and that the -// seeded primary transaction surfaces in recent_transactions. -func TestAccountTemplate_HappyPath(t *testing.T) { - f := seedFixtures(t) - - // The fixture seeds the primary account through UpsertAccount. Pull its - // short_id via the service so we exercise the public template URI shape. - accts, err := f.svc.svc.ListAccounts(f.ctx, nil) - if err != nil || len(accts) == 0 { - t.Fatalf("ListAccounts: err=%v, n=%d", err, len(accts)) - } - // Pick the primary account (the one with transactions). - var shortID string - for _, a := range accts { - if a.Name == "Primary Credit Card" { - shortID = a.ShortID - break - } - } - if shortID == "" { - t.Fatalf("could not find primary account in %+v", accts) - } - uri := "breadbox://account/" + shortID - - res, err := f.svc.handleAccountTemplate(f.ctx, &mcpsdk.ReadResourceRequest{ - Params: &mcpsdk.ReadResourceParams{URI: uri}, - }) - if err != nil { - t.Fatalf("handleAccountTemplate: %v", err) - } - c := res.Contents[0] - if c.URI != uri { - t.Errorf("URI = %q, want %q", c.URI, uri) - } - if c.MIMEType != "application/json" { - t.Errorf("MIMEType = %q, want application/json", c.MIMEType) - } - - var out map[string]any - if err := json.Unmarshal([]byte(c.Text), &out); err != nil { - t.Fatalf("unmarshal: %v\nraw=%s", err, c.Text) - } - requireKeys(t, "account-template", out, "account", "recent_transactions") - - row := asObject(t, "account-template.account", out["account"]) - id, _ := row["id"].(string) - if len(id) != 8 { - t.Errorf("account.id = %q (len=%d); expected 8-char short", id, len(id)) - } - requireAbsent(t, "account-template.account", row, "short_id") - - recent := asArray(t, "account-template.recent_transactions", out["recent_transactions"]) - if len(recent) == 0 { - t.Fatal("expected at least one recent transaction (seeded primary txn)") - } -} - -// TestUserTemplate_HappyPath exercises breadbox://user/{short_id}. Asserts the -// envelope and that the user's accounts are returned. -func TestUserTemplate_HappyPath(t *testing.T) { - f := seedFixtures(t) - - users, err := f.svc.svc.ListUsers(f.ctx) - if err != nil || len(users) == 0 { - t.Fatalf("ListUsers: err=%v, n=%d", err, len(users)) - } - shortID := users[0].ShortID - uri := "breadbox://user/" + shortID - - res, err := f.svc.handleUserTemplate(f.ctx, &mcpsdk.ReadResourceRequest{ - Params: &mcpsdk.ReadResourceParams{URI: uri}, - }) - if err != nil { - t.Fatalf("handleUserTemplate: %v", err) - } - c := res.Contents[0] - if c.URI != uri { - t.Errorf("URI = %q, want %q", c.URI, uri) - } - if c.MIMEType != "application/json" { - t.Errorf("MIMEType = %q, want application/json", c.MIMEType) - } - - var out map[string]any - if err := json.Unmarshal([]byte(c.Text), &out); err != nil { - t.Fatalf("unmarshal: %v\nraw=%s", err, c.Text) - } - requireKeys(t, "user-template", out, "user", "accounts") - - row := asObject(t, "user-template.user", out["user"]) - id, _ := row["id"].(string) - if len(id) != 8 { - t.Errorf("user.id = %q (len=%d); expected 8-char short", id, len(id)) - } - requireAbsent(t, "user-template.user", row, "short_id") - - accts := asArray(t, "user-template.accounts", out["accounts"]) - if len(accts) == 0 { - t.Fatal("expected at least one account for the seeded user") - } -} - -// TestTransactionTemplate_NotFound asserts that an unknown short_id surfaces -// the canonical MCP -32002 ResourceNotFound code so clients can branch on it. -func TestTransactionTemplate_NotFound(t *testing.T) { - f := seedFixtures(t) - - // Syntactically valid 8-char base62, but no row exists. - uri := "breadbox://transaction/zzzzzzzz" - _, err := f.svc.handleTransactionTemplate(f.ctx, &mcpsdk.ReadResourceRequest{ - Params: &mcpsdk.ReadResourceParams{URI: uri}, - }) - if err == nil { - t.Fatal("expected ResourceNotFound error, got nil") - } - var rpcErr *jsonrpc.Error - if !errors.As(err, &rpcErr) { - t.Fatalf("expected *jsonrpc.Error, got %T: %v", err, err) - } - if rpcErr.Code != mcpsdk.CodeResourceNotFound { - t.Errorf("error code = %d, want %d (CodeResourceNotFound / -32002)", - rpcErr.Code, mcpsdk.CodeResourceNotFound) - } -} - -// TestAccountTemplate_RecentTransactionsCap pins the cap at -// templateRecentTransactionLimit. We seed (cap + 5) transactions on the -// primary account and assert the response is truncated to exactly the cap. -func TestAccountTemplate_RecentTransactionsCap(t *testing.T) { - f := seedFixtures(t) - - // Reuse the queries handle attached to the fixture service. Calling - // testutil.ServicePool / testutil.Pool here would re-truncate the tables - // seeded by seedFixtures and wipe our primary account out from under us. - q := f.svc.svc.Queries - // Resolve the primary account back to its UUID — testutil.MustCreateTransaction - // takes a pgtype.UUID, and the fixture didn't expose the account ID. - accts, err := f.svc.svc.ListAccounts(f.ctx, nil) - if err != nil { - t.Fatalf("ListAccounts: %v", err) - } - var primaryShort string - for _, a := range accts { - if a.Name == "Primary Credit Card" { - primaryShort = a.ShortID - break - } - } - if primaryShort == "" { - t.Fatal("primary account not found") - } - - // Pull the row directly so we can grab the pgtype.UUID for the helper. - primaryUUID, err := q.GetAccountUUIDByShortID(f.ctx, primaryShort) - if err != nil { - t.Fatalf("GetAccountUUIDByShortID: %v", err) - } - - // Seed templateRecentTransactionLimit + 5 extra transactions. - extra := templateRecentTransactionLimit + 5 - for i := 0; i < extra; i++ { - testutil.MustCreateTransaction(t, q, primaryUUID, - fmt.Sprintf("txn_cap_%d", i), - fmt.Sprintf("Cap Test %d", i), - int64(100+i), - "2026-04-15", - ) - } - - uri := "breadbox://account/" + primaryShort - res, err := f.svc.handleAccountTemplate(f.ctx, &mcpsdk.ReadResourceRequest{ - Params: &mcpsdk.ReadResourceParams{URI: uri}, - }) - if err != nil { - t.Fatalf("handleAccountTemplate: %v", err) - } - var out map[string]any - if err := json.Unmarshal([]byte(res.Contents[0].Text), &out); err != nil { - t.Fatalf("unmarshal: %v", err) - } - recent := asArray(t, "recent_transactions", out["recent_transactions"]) - if len(recent) != templateRecentTransactionLimit { - t.Errorf("recent_transactions length = %d, want %d", - len(recent), templateRecentTransactionLimit) - } -} diff --git a/internal/mcp/resource_templates_test.go b/internal/mcp/resource_templates_test.go deleted file mode 100644 index 418fb0560..000000000 --- a/internal/mcp/resource_templates_test.go +++ /dev/null @@ -1,67 +0,0 @@ -//go:build !lite - -package mcp - -// Unit tests for the template URI parser. No DB needed — these stay outside -// the integration build tag so they run in `go test ./...`. - -import "testing" - -func TestExtractTemplateParam(t *testing.T) { - cases := []struct { - name string - uri string - prefix string - want string - wantErr bool - }{ - { - name: "happy path", - uri: "breadbox://transaction/abc12345", - prefix: "breadbox://transaction/", - want: "abc12345", - }, - { - name: "percent-encoded decodes", - uri: "breadbox://transaction/abc%20def", - prefix: "breadbox://transaction/", - want: "abc def", - }, - { - name: "empty tail", - uri: "breadbox://transaction/", - prefix: "breadbox://transaction/", - wantErr: true, - }, - { - name: "wrong prefix", - uri: "breadbox://account/abc", - prefix: "breadbox://transaction/", - wantErr: true, - }, - { - name: "extra path segment", - uri: "breadbox://transaction/abc/extra", - prefix: "breadbox://transaction/", - wantErr: true, - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - got, err := extractTemplateParam(tc.uri, tc.prefix) - if tc.wantErr { - if err == nil { - t.Fatalf("expected error, got %q", got) - } - return - } - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got != tc.want { - t.Errorf("got %q, want %q", got, tc.want) - } - }) - } -} diff --git a/internal/mcp/resources.go b/internal/mcp/resources.go deleted file mode 100644 index c842220dc..000000000 --- a/internal/mcp/resources.go +++ /dev/null @@ -1,321 +0,0 @@ -//go:build !lite - -package mcp - -import ( - "context" - "encoding/json" - "errors" - "fmt" - "net/url" - "strings" - "time" - - "breadbox/internal/service" - "breadbox/prompts" - - mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp" -) - -// Default static-resource bodies sourced from the embedded prompts package. -// Editable in prompts/mcp/*.md and rebuilt with the binary. -var ( - DefaultRuleDSL = prompts.MCP("rule-dsl") -) - -func (s *MCPServer) handleOverviewResource(ctx context.Context, _ *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { - stats, err := s.svc.GetOverviewStats(ctx) - if err != nil { - return nil, fmt.Errorf("get overview stats: %w", err) - } - return jsonResourceResult("breadbox://overview", stats) -} - -func (s *MCPServer) handleReviewGuidelinesResource(_ context.Context, _ *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { - ctx := context.Background() - cfg, err := s.svc.GetMCPConfig(ctx) - if err != nil { - return nil, fmt.Errorf("get mcp config: %w", err) - } - - content := cfg.ReviewGuidelines - if content == "" { - content = DefaultReviewGuidelines - } - - return &mcpsdk.ReadResourceResult{ - Contents: []*mcpsdk.ResourceContents{ - { - URI: "breadbox://review-guidelines", - MIMEType: "text/markdown", - Text: content, - }, - }, - }, nil -} - -func (s *MCPServer) handleReportFormatResource(_ context.Context, _ *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { - ctx := context.Background() - cfg, err := s.svc.GetMCPConfig(ctx) - if err != nil { - return nil, fmt.Errorf("get mcp config: %w", err) - } - - content := cfg.ReportFormat - if content == "" { - content = DefaultReportFormat - } - - return &mcpsdk.ReadResourceResult{ - Contents: []*mcpsdk.ResourceContents{ - { - URI: "breadbox://report-format", - MIMEType: "text/markdown", - Text: content, - }, - }, - }, nil -} - -// staticMarkdownResource returns a handler that serves a fixed markdown body. -// Used for rule-dsl — agent-facing reference that doesn't have an app_config -// override slot today. -func staticMarkdownResource(uri, content string) func(context.Context, *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { - return func(_ context.Context, _ *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { - return &mcpsdk.ReadResourceResult{ - Contents: []*mcpsdk.ResourceContents{ - { - URI: uri, - MIMEType: "text/markdown", - Text: content, - }, - }, - }, nil - } -} - -// marshalResourceJSON marshals v as compact JSON and runs compactIDsBytes -// so the resource payload follows the same compact-ID convention as MCP tool -// responses. -// -// Compact (no-indent) marshalling is load-bearing: the byte-scanner in -// compactIDsBytes was designed for the output of json.Marshal and does not -// handle the whitespace json.MarshalIndent inserts between values. On -// payloads with deeply nested objects (the rule list's conditions JSONB hit -// this), the scanner could mis-step into an infinite loop. Sticking to plain -// Marshal keeps the resource and tool surfaces using identical byte layouts. -func marshalResourceJSON(v any) ([]byte, error) { - raw, err := json.Marshal(v) - if err != nil { - return nil, err - } - return compactIDsBytes(raw), nil -} - -// jsonResourceResult is the standard wrapper for service-backed JSON resources. -// Keeps every handler's tail uniform: marshal → compact IDs → wrap. -func jsonResourceResult(uri string, v any) (*mcpsdk.ReadResourceResult, error) { - data, err := marshalResourceJSON(v) - if err != nil { - return nil, fmt.Errorf("marshal %s: %w", uri, err) - } - return &mcpsdk.ReadResourceResult{ - Contents: []*mcpsdk.ResourceContents{ - { - URI: uri, - MIMEType: "application/json", - Text: string(data), - }, - }, - }, nil -} - -func (s *MCPServer) handleSyncStatusResource(ctx context.Context, _ *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { - conns, err := s.svc.ListConnections(ctx, nil) - if err != nil { - return nil, fmt.Errorf("list connections: %w", err) - } - return jsonResourceResult("breadbox://sync-status", map[string]any{"connections": conns}) -} - -// The accounts / categories / tags / users / rules reference resources were -// retired as duplicates of get_reference(kind=…) — see registerResources. -// Their read paths still exist as the get_reference tool handlers in -// tools_reads.go. - -// resourceAnnotations builds the standard *mcpsdk.Annotations payload used on -// every Breadbox resource. lastModifiedFn returns the timestamp the client -// should treat as the resource's mtime; pass nil for resources that don't -// expose a meaningful mtime. -func resourceAnnotations(audience []mcpsdk.Role, priority float64, lastModifiedFn func() time.Time) *mcpsdk.Annotations { - a := &mcpsdk.Annotations{ - Audience: audience, - Priority: priority, - } - if lastModifiedFn != nil { - a.LastModified = lastModifiedFn().UTC().Format(time.RFC3339) - } - return a -} - -// staticPromptModTime returns the build time as a stable mtime for embedded -// markdown resources. Embed reads don't expose file timestamps; the binary -// build time is the closest meaningful proxy. -func staticPromptModTime() time.Time { - return buildStartTime -} - -// liveResourceModTime returns the current time so clients can observe that -// service-backed resources are recomputed on every read. -func liveResourceModTime() time.Time { - return time.Now() -} - -// buildStartTime is captured once at program start so successive reads of -// static markdown resources advertise a consistent mtime. -var buildStartTime = time.Now() - -// audienceUserAndAssistant marks resources that should appear in the user's -// attachment menu (paperclip in Claude.ai). audienceAssistantOnly hides them -// from user pickers in hosts that filter by audience — appropriate for -// agent-internal references. -var ( - audienceUserAndAssistant = []mcpsdk.Role{"user", "assistant"} - audienceAssistantOnly = []mcpsdk.Role{"assistant"} -) - -// --- Resource templates --- -// -// Templates resolve parameterized URIs that drill into a single entity. They -// are how `resource_link` content blocks emitted by tool results (PR 05) turn -// into clickable, attachable items in chat. The template URI format follows -// RFC 6570 ("breadbox://transaction/{short_id}"); the SDK matches incoming -// URIs against the template and routes to the handler. -// -// Discoverability today: most MCP hosts surface templates through the same -// attachment menu as plain resources, but template-parameter UI is uneven. -// The reliable path is `resource_link` blocks emitted from tool results — -// those render as clickable rows the user can attach. PR 03 of this stack -// adds those links; PR 04 (this PR) just makes the URIs resolvable. - -// templateRecentTransactionLimit caps the recent-transactions slice on -// account/user template responses. Keeps payloads bounded for UI rendering. -const templateRecentTransactionLimit = 25 - -// templateAnnotationLimit caps the annotation tail on transaction template -// responses. Most transaction timelines are short, but a noisy rule could -// produce dozens of rows; cap mirrors list_annotations' default-friendly -// budget. -const templateAnnotationLimit = 50 - -// extractTemplateParam pulls the trailing path segment from a templated URI. -// "breadbox://transaction/abc12345" → "abc12345". Returns the URL-decoded -// value (URLs may percent-encode special characters). -func extractTemplateParam(uri, prefix string) (string, error) { - if !strings.HasPrefix(uri, prefix) { - return "", fmt.Errorf("uri %q does not match template prefix %q", uri, prefix) - } - tail := strings.TrimPrefix(uri, prefix) - if tail == "" { - return "", fmt.Errorf("uri %q is missing template parameter", uri) - } - // Reject any further path segments — templates only support a single - // trailing identifier today. - if strings.Contains(tail, "/") { - return "", fmt.Errorf("uri %q has unexpected path segment", uri) - } - decoded, err := url.PathUnescape(tail) - if err != nil { - return "", fmt.Errorf("decode template parameter: %w", err) - } - return decoded, nil -} - -// templateNotFound returns the SDK's standard resource-not-found error so -// clients can branch on the canonical -32002 error code. -func templateNotFound(uri string) error { - return mcpsdk.ResourceNotFoundError(uri) -} - -func (s *MCPServer) handleTransactionTemplate(ctx context.Context, req *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { - uri := req.Params.URI - id, err := extractTemplateParam(uri, "breadbox://transaction/") - if err != nil { - return nil, err - } - - txn, err := s.svc.GetTransaction(ctx, id) - if err != nil { - if errors.Is(err, service.ErrNotFound) { - return nil, templateNotFound(uri) - } - return nil, fmt.Errorf("get transaction %s: %w", id, err) - } - - annotations, err := s.svc.ListAnnotations(ctx, id, service.ListAnnotationsParams{ - Limit: templateAnnotationLimit, - }) - if err != nil { - return nil, fmt.Errorf("list annotations for %s: %w", id, err) - } - - return jsonResourceResult(uri, map[string]any{ - "transaction": txn, - "annotations": toMCPAnnotations(annotations), - }) -} - -func (s *MCPServer) handleAccountTemplate(ctx context.Context, req *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { - uri := req.Params.URI - id, err := extractTemplateParam(uri, "breadbox://account/") - if err != nil { - return nil, err - } - - acct, err := s.svc.GetAccount(ctx, id) - if err != nil { - if errors.Is(err, service.ErrNotFound) { - return nil, templateNotFound(uri) - } - return nil, fmt.Errorf("get account %s: %w", id, err) - } - - recent, err := s.svc.ListTransactions(ctx, service.TransactionListParams{ - AccountID: &id, - Limit: templateRecentTransactionLimit, - }) - if err != nil { - return nil, fmt.Errorf("list transactions for %s: %w", id, err) - } - - return jsonResourceResult(uri, map[string]any{ - "account": acct, - "recent_transactions": recent.Transactions, - }) -} - -func (s *MCPServer) handleUserTemplate(ctx context.Context, req *mcpsdk.ReadResourceRequest) (*mcpsdk.ReadResourceResult, error) { - uri := req.Params.URI - id, err := extractTemplateParam(uri, "breadbox://user/") - if err != nil { - return nil, err - } - - user, err := s.svc.GetUser(ctx, id) - if err != nil { - if errors.Is(err, service.ErrNotFound) { - return nil, templateNotFound(uri) - } - return nil, fmt.Errorf("get user %s: %w", id, err) - } - - accounts, err := s.svc.ListAccounts(ctx, &id) - if err != nil { - return nil, fmt.Errorf("list accounts for %s: %w", id, err) - } - - return jsonResourceResult(uri, map[string]any{ - "user": user, - "accounts": accounts, - }) -} diff --git a/internal/mcp/resources_test.go b/internal/mcp/resources_test.go index 04c06be34..5609e8165 100644 --- a/internal/mcp/resources_test.go +++ b/internal/mcp/resources_test.go @@ -3,64 +3,10 @@ package mcp import ( - "context" "strings" "testing" - - mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp" ) -func TestStaticMarkdownResource_Shape(t *testing.T) { - cases := []struct { - uri string - body string - mustHit string - }{ - {"breadbox://rule-dsl", DefaultRuleDSL, "Transaction rule DSL"}, - } - - for _, tc := range cases { - t.Run(tc.uri, func(t *testing.T) { - handler := staticMarkdownResource(tc.uri, tc.body) - result, err := handler(context.Background(), &mcpsdk.ReadResourceRequest{}) - if err != nil { - t.Fatalf("handler error: %v", err) - } - if len(result.Contents) != 1 { - t.Fatalf("expected 1 content block, got %d", len(result.Contents)) - } - c := result.Contents[0] - if c.URI != tc.uri { - t.Errorf("URI mismatch: got %q want %q", c.URI, tc.uri) - } - if c.MIMEType != "text/markdown" { - t.Errorf("MIMEType mismatch: got %q want text/markdown", c.MIMEType) - } - if !strings.Contains(c.Text, tc.mustHit) { - t.Errorf("body missing expected marker %q (first 80 chars: %q)", tc.mustHit, head(c.Text, 80)) - } - }) - } -} - -func TestResourceAnnotations_FieldsPopulated(t *testing.T) { - ann := resourceAnnotations(audienceUserAndAssistant, 0.8, staticPromptModTime) - if len(ann.Audience) != 2 || ann.Audience[0] != "user" || ann.Audience[1] != "assistant" { - t.Errorf("Audience not set as expected: %v", ann.Audience) - } - if ann.Priority != 0.8 { - t.Errorf("Priority: got %v want 0.8", ann.Priority) - } - if ann.LastModified == "" { - t.Error("LastModified should be populated when lastModifiedFn is non-nil") - } - - annNoTime := resourceAnnotations(audienceAssistantOnly, 0.5, nil) - if annNoTime.LastModified != "" { - t.Errorf("LastModified should be empty when lastModifiedFn is nil, got %q", annNoTime.LastModified) - } -} - func TestBreadboxImplementation_IncludesMetadataAndIcon(t *testing.T) { impl := breadboxImplementation("1.2.3") if impl.Name != "breadbox" { @@ -90,21 +36,6 @@ func TestBreadboxImplementation_IncludesMetadataAndIcon(t *testing.T) { } } -func TestBuildServer_RegistersAllResources(t *testing.T) { - // Resources don't execute at registration time — they only need a non-nil - // MCPServer and BuildServer to wire them up successfully. The live - // resources call svc.* methods only at read time, so nil-svc registration - // is fine for this smoke check. - s := NewMCPServer(nil, "test") - server := s.BuildServer(MCPServerConfig{Mode: "read_write", APIKeyScope: "full_access"}) - if server == nil { - t.Fatal("BuildServer returned nil") - } - // If we reached here without panicking, every AddResource call accepted - // its handler signature. A deeper assertion (resources/list output) would - // require an in-process MCP transport; the integration tests cover that. -} - func head(s string, n int) string { if len(s) < n { return s diff --git a/internal/mcp/response_shapes_integration_test.go b/internal/mcp/response_shapes_integration_test.go index d139ef2b6..a3b5f119e 100644 --- a/internal/mcp/response_shapes_integration_test.go +++ b/internal/mcp/response_shapes_integration_test.go @@ -17,7 +17,6 @@ package mcp // PR as the service change. New/optional fields can still be added. import ( - "bytes" "context" "encoding/json" "fmt" @@ -958,90 +957,7 @@ func TestUpdateTransactionsHandler_ResetCategoryShape(t *testing.T) { // optional service-layer params without copy-pasting the &s pattern. func ptrString(s string) *string { return &s } -// TestReferenceMirrorTools_ParityWithResources locks the dual-surface -// contract for the reference datasets that still have BOTH a resource and a -// get_reference tool path: breadbox://sync-status and ://overview. Each pair -// must return the SAME payload via the SAME service call. A regression that -// diverges them — e.g. forgetting the resource envelope, or pointing the tool -// at a different service method — would let one surface drift from the other. -// -// (accounts / categories / tags / users / rules were retired as resources — -// they're read-only via get_reference now, so there's no pair left to compare.) -// -// The parity test reads each pair, ignores envelope keys (resource handlers -// always wrap in {"": [...]}), and compares the inner payload byte- -// for-byte after decompressing through json.Unmarshal — payload semantics, not -// formatting. -func TestReferenceMirrorTools_ParityWithResources(t *testing.T) { - f := seedFixtures(t) - - cases := []struct { - name string - envelopeKey string // key inside the JSON envelope; "" means top-level (overview) - toolFn func() (*mcpsdk.CallToolResult, any, error) - resourceFn func() (*mcpsdk.ReadResourceResult, error) - }{ - { - name: "get_reference(sync_status) <-> breadbox://sync-status", - envelopeKey: "connections", - toolFn: func() (*mcpsdk.CallToolResult, any, error) { - return f.svc.handleGetSyncStatus(f.ctx, nil, getSyncStatusInput{}) - }, - resourceFn: func() (*mcpsdk.ReadResourceResult, error) { - return f.svc.handleSyncStatusResource(f.ctx, nil) - }, - }, - { - name: "get_reference(overview) <-> breadbox://overview", - envelopeKey: "", // both surfaces return the same OverviewStats shape - toolFn: func() (*mcpsdk.CallToolResult, any, error) { - return f.svc.handleGetOverview(f.ctx, nil, getOverviewInput{}) - }, - resourceFn: func() (*mcpsdk.ReadResourceResult, error) { - return f.svc.handleOverviewResource(f.ctx, nil) - }, - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - toolRes, _, toolErr := tc.toolFn() - if toolErr != nil { - t.Fatalf("tool: %v", toolErr) - } - toolPayload := decodeToolResult[any](t, tc.name+" tool", toolRes, toolErr) - - resRes, resErr := tc.resourceFn() - if resErr != nil { - t.Fatalf("resource: %v", resErr) - } - if len(resRes.Contents) != 1 { - t.Fatalf("resource: expected 1 content block, got %d", len(resRes.Contents)) - } - var resPayload any - if err := json.Unmarshal([]byte(resRes.Contents[0].Text), &resPayload); err != nil { - t.Fatalf("resource: unmarshal: %v", err) - } - - if tc.envelopeKey != "" { - toolMap := asObject(t, "tool envelope", toolPayload) - resMap := asObject(t, "resource envelope", resPayload) - toolPayload = toolMap[tc.envelopeKey] - resPayload = resMap[tc.envelopeKey] - if toolPayload == nil { - t.Fatalf("tool envelope missing key %q", tc.envelopeKey) - } - if resPayload == nil { - t.Fatalf("resource envelope missing key %q", tc.envelopeKey) - } - } - - toolBytes := mustMarshal(t, toolPayload) - resBytes := mustMarshal(t, resPayload) - if !bytes.Equal(toolBytes, resBytes) { - t.Errorf("payload drift between tool and resource\n tool: %s\n resource: %s", - string(toolBytes), string(resBytes)) - } - }) - } -} +// The reference data reads (accounts / categories / tags / users / sync-status +// / rules / overview) are tool-only now — MCP resources were retired entirely, +// so there's no tool↔resource parity left to assert here. The individual tool +// response shapes are covered by the per-tool shape tests above. diff --git a/internal/mcp/server.go b/internal/mcp/server.go index f6dfc7e9f..371aa44f0 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -77,6 +77,7 @@ var ( DefaultInstructions = prompts.MCP("instructions") DefaultReviewGuidelines = prompts.MCP("review-guidelines") DefaultReportFormat = prompts.MCP("report-format") + DefaultRuleDSL = prompts.MCP("rule-dsl") ) // ToolClassification indicates whether a tool is read-only or performs writes. @@ -327,10 +328,10 @@ func auditSessionFromContext(ctx context.Context) string { // buildToolRegistry populates the allTools slice with all available tools and // their classifications. The registry carves around what an agent does // (query, decide, write, configure) rather than every underlying entity. -// Bounded reference data (accounts, categories, tags, users, sync status, -// rules, overview) is preferred via resources — see registerResources in this -// file — but each resource also has a tool mirror in tools_reads.go so MCP -// clients that don't implement the resources/* methods can still read it. +// Everything is a tool — MCP resources were retired. Bounded reference data +// (accounts, categories, tags, users, sync status, rules, overview) is read +// through dedicated read tools (handlers in tools_reads.go); the near-static +// operating-guidance docs are served by get_reference(kind=…). func (s *MCPServer) buildToolRegistry() { s.allTools = []ToolDef{ // Audit sessions are bound to the transport connection (MCP-Session-Id @@ -338,16 +339,46 @@ func (s *MCPServer) buildToolRegistry() { // resolves its session via resolveTransportID + ensureAuditSession in // the dispatcher, so agents no longer need to call create_session. - // --- Reference data (one tool that mirrors the bounded reference resources) --- - // Resources are the preferred surface: breadbox://overview, ://accounts, - // ://categories, ://tags, ://users, ://sync-status, ://rules. get_reference - // is the single tool fallback for clients that don't implement resources/* - // — it dispatches on `kind` so the seven bounded reads share one tool slot - // instead of seven. Filtered/sorted rule analysis still lives in - // query_transaction_rules; this returns the lean roster. + // --- Reference data reads --- + // Bounded lookup datasets, each its own tool. MCP resources were retired, + // so these tools are the only way to read this data. get_reference (below) + // is a separate tool that serves the operating-guidance docs, not data. makeToolDefLogged(ToolSpec{ - Name: "get_reference", Title: "Get Reference Data", Classification: ToolRead, - Description: "Read a bounded reference dataset by `kind` — the single tool mirror of the breadbox:// reference resources, for clients that don't support MCP resources. kinds: 'overview' (household snapshot: scope, freshness, pending-review backlog — read once at the top of a session to ground later filters), 'accounts' (bank accounts with balance/type/currency/connection; optional user_id filter), 'categories' (the category taxonomy — use the slugs as the canonical handle on filters and writes), 'tags' (the tag vocabulary — slugs are referenced everywhere), 'users' (household members; the short_id is the user_id on filters), 'sync_status' (per-connection provider/status/last-sync/last-error — check freshness before trusting results), 'rules' (the transaction-rule roster, lean summary projection; pass fields=all for the full conditions/actions trees). For filtered/sorted rule analysis use query_transaction_rules; to check coverage for one merchant use find_matching_rules.", + Name: "get_overview", Title: "Household Overview", Classification: ToolRead, + Description: "Get a household snapshot: scope (users, accounts, currencies), freshness (latest sync, errored connections, recent transactions), and backlog (pending review queue). Read once at the top of a session to ground every later filter (account ids, currency, attribution).", + }, s.handleGetOverview, s), + makeToolDefLogged(ToolSpec{ + Name: "list_accounts", Title: "List Accounts", Classification: ToolRead, + Description: "List bank accounts. Each account carries balance, type, currency, and the connection it belongs to. Filter by user_id to scope to a specific household member.", + }, s.handleListAccounts, s), + makeToolDefLogged(ToolSpec{ + Name: "list_categories", Title: "List Categories", Classification: ToolRead, + Description: "List the category taxonomy as a flat array. Use the returned slugs (e.g. 'food_and_drink_groceries') as the canonical handle for category filters and category_slug fields on writes.", + }, s.handleListCategories, s), + makeToolDefLogged(ToolSpec{ + Name: "list_users", Title: "List Household Members", Classification: ToolRead, + Description: "List household members. Each user carries display name, role, and short_id — use the short_id as user_id on transaction filters and account scoping.", + }, s.handleListUsers, s), + makeToolDefLogged(ToolSpec{ + Name: "list_tags", Title: "List Tags", Classification: ToolRead, + Description: "List the tag vocabulary. Tags are referenced by slug everywhere (filter, add, remove). New tag slugs auto-register the first time update_transactions adds them — read this list before authoring rules to avoid accidental near-duplicates.", + }, s.handleListTags, s), + makeToolDefLogged(ToolSpec{ + Name: "get_sync_status", Title: "Sync Status", Classification: ToolRead, + Description: "Get connection sync status: provider, status (active|error|pending_reauth|disconnected), last sync time, last error. Call this before reasoning about freshness — an errored or pending_reauth connection means transactions you'd expect to be there might not be.", + }, s.handleGetSyncStatus, s), + makeToolDefLogged(ToolSpec{ + Name: "list_transaction_rules", Title: "List Rules", Classification: ToolRead, + Description: "List transaction rules (the roster). Filter by category_slug, enabled, or search by name. Lean by default: returns a summary projection (name, enabled, priority, trigger, category, hit_count) without the conditions/actions trees — pass fields=all to inspect or audit full rule definitions. For richer analysis — filter by trigger/creator/hit-count or sort by impact — use query_transaction_rules; to check whether one specific merchant is already covered, use find_matching_rules.", + }, s.handleListTransactionRules, s), + + // --- Operating-guidance docs --- + // The near-static markdown that teaches an agent how to drive the server. + // Formerly breadbox:// markdown resources; served as a tool so clients + // that can't read MCP resources can still pull them. + makeToolDefLogged(ToolSpec{ + Name: "get_reference", Title: "Get Guidance Doc", Classification: ToolRead, + Description: "Read an operating-guidance doc by `kind` — the near-static markdown that explains how to drive this server. kinds: 'instructions' (data model + conventions overview, how the surface is organized), 'rule-dsl' (the full transaction-rule condition grammar, action types, pipeline-stage ordering, and sync-vs-retroactive semantics — read before authoring rules), 'review-guidelines' (principles for reviewing transactions and creating rules — read before working the needs-review queue), 'report-format' (structure + formatting conventions for submit_report). Returns markdown. instructions/review-guidelines/report-format reflect any operator customization; rule-dsl is the fixed grammar.", }, s.handleGetReference, s), makeToolDefLogged(ToolSpec{ Name: "query_transaction_rules", Title: "Query Rules", Classification: ToolRead, @@ -397,7 +428,7 @@ func (s *MCPServer) buildToolRegistry() { // --- Query + aggregate --- makeToolDefLogged(ToolSpec{ Name: "query_transactions", Title: "Query Transactions", Classification: ToolRead, - Description: "Query bank transactions with optional filters and cursor-based pagination. Amounts: positive = money out (debit), negative = money in (credit). Dates: YYYY-MM-DD, start_date inclusive, end_date exclusive. Filter by category_slug (see get_reference(kind=categories) for the slug list); parent slugs include all children. Results ordered by date desc by default. Pagination: pass next_cursor from response. Responses are lean by default — a compact field set (core,category) is returned unless you pass fields=all or an explicit field/alias list. When every row shares one currency, iso_currency_code is returned once at the top level instead of on each row; otherwise each row carries its own. Pass count_only=true to get just {\"count\": N} for the same filters (no rows) — use it to size a result set or compare counts across ranges before paginating.", + Description: "Query bank transactions with optional filters and cursor-based pagination. Amounts: positive = money out (debit), negative = money in (credit). Dates: YYYY-MM-DD, start_date inclusive, end_date exclusive. Filter by category_slug (see list_categories for the slug list); parent slugs include all children. Results ordered by date desc by default. Pagination: pass next_cursor from response. Responses are lean by default — a compact field set (core,category) is returned unless you pass fields=all or an explicit field/alias list. When every row shares one currency, iso_currency_code is returned once at the top level instead of on each row; otherwise each row carries its own. Pass count_only=true to get just {\"count\": N} for the same filters (no rows) — use it to size a result set or compare counts across ranges before paginating.", }, s.handleQueryTransactions, s), makeToolDefLogged(ToolSpec{ Name: "transaction_summary", Title: "Spending Summary", Classification: ToolRead, @@ -434,8 +465,8 @@ func (s *MCPServer) buildToolRegistry() { }, s.handleListAnnotations, s), // --- Rules --- - // See breadbox://rule-dsl for the condition grammar and - // get_reference(kind=rules) for the current ruleset. + // See get_reference(kind=rule-dsl) for the condition grammar and + // list_transaction_rules for the current ruleset. makeToolDefLogged(ToolSpec{ Name: "create_transaction_rule", Title: "Create Rule", Classification: ToolWrite, Description: "Create one or more transaction rules for automatic categorization, tagging, or commenting. Pass `rules`: an array of 1..100 rule specs (a single rule is just a one-element array). Rules match condition trees against transactions during sync and fire in pipeline-stage order (priority ASC — lower = earlier). Pass `stage` (one of baseline|standard|refinement|override) per rule instead of a raw priority so rules from different agents compose predictably; stage resolves to priority 0/10/50/100. Earlier-stage rules' tag and category mutations feed later-stage rules' conditions, so rules compose: rule A tags 'coffee', rule B conditioned on tags-contains-coffee sets category — author such pipelines in one call. Set apply_retroactively=true on a rule to immediately back-fill it against existing transactions. Before creating, read the rules roster (get_reference kind=rules) to avoid duplicates; prefer `contains` over exact matches (bank feeds format merchant names inconsistently). Returns the created rules plus any per-item errors so a partial batch is recoverable. Full DSL: breadbox://rule-dsl.", @@ -644,105 +675,15 @@ func (s *MCPServer) BuildServer(cfg MCPServerConfig) *mcpsdk.Server { td.register(server) } - s.registerResources(server) + // MCP resources (resources/* + resource templates) were retired entirely. + // They were invisible on clients that can't resources/list (e.g. Claude.ai), + // so every read goes through tools instead: the bounded data reads are + // standalone tools (get_overview, list_accounts, …) and the operating- + // guidance docs (instructions, rule-dsl, review-guidelines, report-format) + // are served by get_reference(kind=…). return server } -// registerResources adds MCP resources to a server. The catalog is -// agent-facing first: most resources are audienced to the assistant only. -// A handful (overview, accounts, review-guidelines, report-format) are -// dual-audience because users have a real "attach this in chat" flow for -// them — those show up in Claude.ai's paperclip / attachment menu. -func (s *MCPServer) registerResources(server *mcpsdk.Server) { - // Top-level live snapshot — read first. - server.AddResource(&mcpsdk.Resource{ - Name: "Overview", - Title: "Household Overview", - URI: "breadbox://overview", - Description: "Live summary of the dataset: users, connections, accounts, transaction counts and date range, recent spending. Read on connect for context.", - MIMEType: "application/json", - Annotations: resourceAnnotations(audienceUserAndAssistant, 1.0, liveResourceModTime), - }, s.handleOverviewResource) - - server.AddResource(&mcpsdk.Resource{ - Name: "Sync Status", - Title: "Connection Sync Status", - URI: "breadbox://sync-status", - Description: "Per-connection sync status and last-sync timestamps. Read to verify data freshness before answering questions about recent activity.", - MIMEType: "application/json", - Annotations: resourceAnnotations(audienceAssistantOnly, 0.6, liveResourceModTime), - }, s.handleSyncStatusResource) - - // The accounts / categories / tags / users / rules reference resources were - // retired — they were exact duplicates of get_reference(kind=…) and pure - // agent-facing lookups, carrying no load on the resource surface. Read them - // via get_reference. Only overview + sync-status stay as data resources - // (a human plausibly attaches those snapshots). - - // Workflow guides — markdown, user-overridable via app_config. - server.AddResource(&mcpsdk.Resource{ - Name: "Review Guidelines", - Title: "Transaction Review Guidelines", - URI: "breadbox://review-guidelines", - Description: "Principles for reviewing transactions and creating rules. Read before touching the review queue.", - MIMEType: "text/markdown", - Annotations: resourceAnnotations(audienceUserAndAssistant, 0.8, staticPromptModTime), - }, s.handleReviewGuidelinesResource) - - server.AddResource(&mcpsdk.Resource{ - Name: "Report Format", - Title: "Spending Report Format", - URI: "breadbox://report-format", - Description: "Report structure and formatting guidelines. Read before submit_report.", - MIMEType: "text/markdown", - Annotations: resourceAnnotations(audienceUserAndAssistant, 0.8, staticPromptModTime), - }, s.handleReportFormatResource) - - // Authoring reference — only relevant when the agent is creating rules. - // Carrying the grammar here instead of in create_transaction_rule's - // description keeps tools/list lean. - server.AddResource(&mcpsdk.Resource{ - Name: "Rule DSL", - Title: "Transaction Rule DSL", - URI: "breadbox://rule-dsl", - Description: "Condition grammar, action types, priority bands, sync-vs-retroactive semantics, provider quirks. Read before authoring rules.", - MIMEType: "text/markdown", - Annotations: resourceAnnotations(audienceAssistantOnly, 0.7, staticPromptModTime), - }, staticMarkdownResource("breadbox://rule-dsl", DefaultRuleDSL)) - - // --- Resource templates --- - // Drill-downs into a single entity. URIs come back from tool results as - // `resource_link` blocks (PR 05 in this stack) and resolve through these - // handlers. dual-audience so they appear in template-aware pickers as - // well; priority is below top-level resources. - server.AddResourceTemplate(&mcpsdk.ResourceTemplate{ - Name: "Transaction", - Title: "Transaction Detail", - URITemplate: "breadbox://transaction/{short_id}", - Description: "Single transaction with its activity timeline (annotations). short_id is the 8-char base62 id surfaced everywhere by query_transactions.", - MIMEType: "application/json", - Annotations: resourceAnnotations(audienceUserAndAssistant, 0.7, liveResourceModTime), - }, s.handleTransactionTemplate) - - server.AddResourceTemplate(&mcpsdk.ResourceTemplate{ - Name: "Account", - Title: "Account Detail", - URITemplate: "breadbox://account/{short_id}", - Description: "Single bank account with balance, currency, and the most recent 25 transactions. short_id is the 8-char base62 id surfaced by get_reference(kind=accounts).", - MIMEType: "application/json", - Annotations: resourceAnnotations(audienceUserAndAssistant, 0.7, liveResourceModTime), - }, s.handleAccountTemplate) - - server.AddResourceTemplate(&mcpsdk.ResourceTemplate{ - Name: "Household Member", - Title: "Household Member Detail", - URITemplate: "breadbox://user/{short_id}", - Description: "Single household member with their connected accounts. short_id is the 8-char base62 id surfaced by get_reference(kind=users).", - MIMEType: "application/json", - Annotations: resourceAnnotations(audienceUserAndAssistant, 0.7, liveResourceModTime), - }, s.handleUserTemplate) -} - // AllToolDefs returns the full tool registry for admin display. func (s *MCPServer) AllToolDefs() []ToolDef { return s.allTools diff --git a/internal/mcp/server_test.go b/internal/mcp/server_test.go index 1cca4f4a8..acda6c105 100644 --- a/internal/mcp/server_test.go +++ b/internal/mcp/server_test.go @@ -124,10 +124,12 @@ func TestToolRegistryScopeContract(t *testing.T) { } // Anchor the explicit canonical set for read tools — this is the surface - // area read-only API keys are allowed to exercise. The seven bounded - // reference-data mirrors that shadow the breadbox:// resources for clients - // without resources support are folded behind get_reference(kind=…) — see - // tools_reads.go. count_transactions folded into query_transactions + // area read-only API keys are allowed to exercise. MCP resources were + // retired, so the bounded reference datasets are each their own tool + // (get_overview / list_accounts / list_categories / list_users / list_tags / + // get_sync_status / list_transaction_rules), and get_reference serves the + // operating-guidance docs (instructions / rule-dsl / review-guidelines / + // report-format). count_transactions folded into query_transactions // (count_only=true). wantReads := []string{ "query_transactions", @@ -141,6 +143,13 @@ func TestToolRegistryScopeContract(t *testing.T) { "get_series", "explain_series_candidates", "list_workflows", + "get_overview", + "list_accounts", + "list_categories", + "list_users", + "list_tags", + "get_sync_status", + "list_transaction_rules", } if len(readNames) != len(wantReads) { t.Errorf("read tool count = %d, want %d (got %v)", len(readNames), len(wantReads), readNames) diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index 1132b6498..2f03d0641 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -22,7 +22,7 @@ type queryTransactionsInput struct { EndDate string `json:"end_date,omitempty" jsonschema:"End date (YYYY-MM-DD) exclusive"` AccountID string `json:"account_id,omitempty" jsonschema:"Filter by account ID"` UserID string `json:"user_id,omitempty" jsonschema:"Filter by user ID"` - CategorySlug string `json:"category_slug,omitempty" jsonschema:"Filter by category slug (parent slug includes all children). See get_reference(kind=categories) for the slug list."` + CategorySlug string `json:"category_slug,omitempty" jsonschema:"Filter by category slug (parent slug includes all children). See list_categories for the slug list."` MinAmount *float64 `json:"min_amount,omitempty" jsonschema:"Minimum amount (positive=debit, negative=credit)"` MaxAmount *float64 `json:"max_amount,omitempty" jsonschema:"Maximum amount (positive=debit, negative=credit)"` Pending *bool `json:"pending,omitempty" jsonschema:"Filter by pending status"` @@ -30,7 +30,7 @@ type queryTransactionsInput struct { Search string `json:"search,omitempty" jsonschema:"Search transaction name or merchant. Comma-separated values are ORed (e.g. starbucks,amazon matches either)."` SearchMode string `json:"search_mode,omitempty" jsonschema:"How to match the search term: contains (default, substring match), words (all words must match, good for multi-word queries), fuzzy (typo-tolerant via trigram similarity)"` ExcludeSearch string `json:"exclude_search,omitempty" jsonschema:"Exclude transactions whose name or merchant matches this text. Comma-separated values are ORed. Use to filter out known merchants."` - Tags []string `json:"tags,omitempty" jsonschema:"Filter to transactions that have EVERY tag slug in this list (AND semantics). See get_reference(kind=tags) for the available vocabulary."` + Tags []string `json:"tags,omitempty" jsonschema:"Filter to transactions that have EVERY tag slug in this list (AND semantics). See list_tags for the available vocabulary."` AnyTag []string `json:"any_tag,omitempty" jsonschema:"Filter to transactions that have AT LEAST ONE tag slug in this list (OR semantics)."` Limit int `json:"limit,omitempty" jsonschema:"Max results (default 50, max 500)"` Cursor string `json:"cursor,omitempty" jsonschema:"Pagination cursor from previous result"` diff --git a/internal/mcp/tools_reads.go b/internal/mcp/tools_reads.go index 05577ce97..7d329b9da 100644 --- a/internal/mcp/tools_reads.go +++ b/internal/mcp/tools_reads.go @@ -73,37 +73,54 @@ type queryTransactionRulesInput struct { Fields string `json:"fields,omitempty" jsonschema:"Comma-separated fields to include, to cut response size. Alias: summary (name,enabled,priority,trigger,category,hit_count,last_hit_at; the default — omits the conditions and actions trees). Default when omitted: summary. Pass fields=all for every field including the full conditions/actions. id is always included."` } -// getReferenceInput is the single parameterized read over the bounded -// reference datasets. `kind` selects the dataset; the optional fields apply -// only to the kinds that accept them (user_id → accounts, fields → rules). +// getReferenceInput selects which operating-guidance doc to read. These are the +// near-static markdown docs that teach an agent how to drive the server — the +// content that used to live in `breadbox://` markdown resources before resources +// were retired. Serving them as a tool means clients that can't read MCP +// resources (e.g. Claude.ai) can still pull the guidance on demand. type getReferenceInput struct { - Kind string `json:"kind" jsonschema:"required,Which reference dataset to read: overview | accounts | categories | tags | users | sync_status | rules."` - UserID string `json:"user_id,omitempty" jsonschema:"Only for kind=accounts: scope accounts to one household member (short ID or UUID)."` - Fields string `json:"fields,omitempty" jsonschema:"Only for kind=rules: comma-separated fields/aliases. Default summary projection (omits conditions/actions trees); pass fields=all for full rule definitions."` + Kind string `json:"kind" jsonschema:"required,Which guidance doc to read: 'instructions' (how the server is organized + conventions) | 'rule-dsl' (the full transaction-rule condition grammar, action types, and pipeline-stage semantics — read before authoring rules) | 'review-guidelines' (principles for reviewing transactions and creating rules — read before working the review queue) | 'report-format' (structure + formatting for submit_report)."` } -// handleGetReference dispatches a get_reference call to the matching bounded -// reference handler. It reuses the per-kind handlers (which share the same -// service path as the breadbox:// resources), so payload shapes stay identical -// to the resources and to the pre-fold tool mirrors. -func (s *MCPServer) handleGetReference(ctx context.Context, req *mcpsdk.CallToolRequest, input getReferenceInput) (*mcpsdk.CallToolResult, any, error) { +// handleGetReference returns the requested guidance doc as markdown. The +// instructions / review-guidelines / report-format docs honor the operator's +// app_config overrides (falling back to the embedded defaults); rule-dsl is the +// fixed embedded grammar. +func (s *MCPServer) handleGetReference(_ context.Context, _ *mcpsdk.CallToolRequest, input getReferenceInput) (*mcpsdk.CallToolResult, any, error) { + cfg, err := s.svc.GetMCPConfig(context.Background()) + if err != nil { + return errorResult(err), nil, nil + } + var content string switch input.Kind { - case "overview": - return s.handleGetOverview(ctx, req, getOverviewInput{}) - case "accounts": - return s.handleListAccounts(ctx, req, listAccountsInput{UserID: input.UserID}) - case "categories": - return s.handleListCategories(ctx, req, listCategoriesInput{}) - case "tags": - return s.handleListTags(ctx, req, listTagsInput{}) - case "users": - return s.handleListUsers(ctx, req, listUsersInput{}) - case "sync_status": - return s.handleGetSyncStatus(ctx, req, getSyncStatusInput{}) - case "rules": - return s.handleListTransactionRules(ctx, req, listTransactionRulesInput{Fields: input.Fields}) + case "instructions": + content = orDefault(cfg.Instructions, DefaultInstructions) + case "review-guidelines": + content = orDefault(cfg.ReviewGuidelines, DefaultReviewGuidelines) + case "report-format": + content = orDefault(cfg.ReportFormat, DefaultReportFormat) + case "rule-dsl": + content = DefaultRuleDSL default: - return errorResult(fmt.Errorf("unknown kind %q: must be one of overview, accounts, categories, tags, users, sync_status, rules", input.Kind)), nil, nil + return errorResult(fmt.Errorf("unknown kind %q: must be one of instructions, rule-dsl, review-guidelines, report-format", input.Kind)), nil, nil + } + return markdownResult(content), nil, nil +} + +// orDefault returns v, or def when v is empty. +func orDefault(v, def string) string { + if v == "" { + return def + } + return v +} + +// markdownResult wraps a guidance doc as a single markdown text content block. +// Unlike jsonResult it does not JSON-encode or ID-compact — the payload is +// human-readable markdown, not a data record. +func markdownResult(md string) *mcpsdk.CallToolResult { + return &mcpsdk.CallToolResult{ + Content: []mcpsdk.Content{&mcpsdk.TextContent{Text: md}}, } } diff --git a/internal/mcp/tools_transaction_metadata.go b/internal/mcp/tools_transaction_metadata.go index cad0dcb53..283a5029b 100644 --- a/internal/mcp/tools_transaction_metadata.go +++ b/internal/mcp/tools_transaction_metadata.go @@ -20,7 +20,7 @@ import ( // - clear everything → replace:true (set omitted → {}) // // Metadata is read back on every transaction (query_transactions, the -// breadbox://transaction/{id} resource, GET /transactions/{id}). +// GET /transactions/{id}). type setTransactionMetadataInput struct { TransactionID string `json:"transaction_id" jsonschema:"required,UUID or short ID of the transaction."` diff --git a/internal/service/overview.go b/internal/service/overview.go index abda2b705..9835ad4ba 100644 --- a/internal/service/overview.go +++ b/internal/service/overview.go @@ -68,9 +68,9 @@ const dependentExclusionWhere = ` )) ` -// GetOverviewStats returns the lightweight dataset overview served via -// `breadbox://overview`. Detail consumers (per-connection sync, per-account -// balances, full user/tag/category lists) read the dedicated resources. +// GetOverviewStats returns the lightweight dataset overview served by the +// get_overview MCP tool. Detail consumers (per-connection sync, per-account +// balances, full user/tag/category lists) call the dedicated read tools. func (s *Service) GetOverviewStats(ctx context.Context) (*OverviewStats, error) { stats := &OverviewStats{} diff --git a/internal/templates/components/pages/design_prompt_preview.templ b/internal/templates/components/pages/design_prompt_preview.templ index 4a77cecd8..07ca4763e 100644 --- a/internal/templates/components/pages/design_prompt_preview.templ +++ b/internal/templates/components/pages/design_prompt_preview.templ @@ -91,8 +91,8 @@ func designPromptPreviewSample() string { "## Objective\n\n" + "Shrink the `needs-review` queue as much as possible in one pass. Confidently categorize the clear-cut transactions and clear their `needs-review` tag; leave only the genuinely ambiguous ones tagged for a human.\n\n" + "## Step by step\n\n" + - "1. Read `breadbox://overview` for context (accounts, users, currency).\n" + - "2. `count_transactions{tags:[\"needs-review\"]}` to size the backlog up front.\n" + + "1. Call `get_overview` for context (accounts, users, currency).\n" + + "2. `query_transactions{tags:[\"needs-review\"], count_only:true}` to size the backlog up front.\n" + "3. Work in batches. `query_transactions` the largest raw-category groups together.\n" + "4. For each transaction, decide the category from its name, merchant, amount, and raw category fields.\n" + "5. Apply decisions in large compound writes — `update_transactions` (max 50 per call).\n\n" + diff --git a/prompts/agents/strategy-anomaly-detection.md b/prompts/agents/strategy-anomaly-detection.md index 095df2a00..173a9756e 100644 --- a/prompts/agents/strategy-anomaly-detection.md +++ b/prompts/agents/strategy-anomaly-detection.md @@ -12,7 +12,7 @@ Flag transactions or patterns that merit the family's attention. Every flagged i ## Steps -1. Read `breadbox://overview` for baseline context. +1. Call `get_overview` for baseline context. 2. Compare recent vs historical spending: - `transaction_summary` with `group_by=category_month` for the last 30 days vs the prior 30 days - Scan a `query_transactions` sample over the recent window for merchant names absent from the prior window diff --git a/prompts/agents/strategy-bulk-catchup.md b/prompts/agents/strategy-bulk-catchup.md index 60114efa8..3744713d1 100644 --- a/prompts/agents/strategy-bulk-catchup.md +++ b/prompts/agents/strategy-bulk-catchup.md @@ -12,7 +12,7 @@ Shrink the `needs-review` queue as much as possible in one pass. Confidently cat ## Steps -1. Read `breadbox://overview` for context (accounts, users, currency). `count_transactions(tags=["needs-review"])` to size the backlog up front so you know how much there is to clear. +1. Call `get_overview` for context (accounts, users, currency). `query_transactions(tags=["needs-review"], count_only=true)` to size the backlog up front so you know how much there is to clear. 2. **Work in batches.** `query_transactions(tags=["needs-review"], fields=minimal)` sorted by `category_primary` so you can clear the largest raw-category groups together. Page through the WHOLE backlog — don't stop at the first page. 3. For each transaction, decide the category from its name, merchant, amount, and raw category fields. Lean on obvious merchant→category signals — this is a fast pass, not a deliberation. 4. **Apply decisions in large compound writes** — `update_transactions` (max 50 operations per call): set `category_slug` AND remove the `needs-review` tag (with a terse rationale note) in one atomic op per transaction. For pre-categorized items whose category already looks right, just remove the tag with a short confirmation note. diff --git a/prompts/agents/strategy-bulk-review.md b/prompts/agents/strategy-bulk-review.md index 321ba6e60..4a9a4ee1a 100644 --- a/prompts/agents/strategy-bulk-review.md +++ b/prompts/agents/strategy-bulk-review.md @@ -12,7 +12,7 @@ Clear the backlog with high accuracy. Create rules for newly discovered patterns ## Steps -1. `count_transactions(tags=["needs-review"])` — understand backlog size. If zero, check `get_sync_status` for freshness, report "backlog clear" and exit. +1. `query_transactions(tags=["needs-review"], count_only=true)` — understand backlog size. If zero, check `get_sync_status` for freshness, report "backlog clear" and exit. 2. Skim `list_transaction_rules` once for a sense of overall coverage. But to decide whether a SPECIFIC merchant already has a rule, call `find_matching_rules(merchant="")` rather than re-scanning the full list per candidate — it returns just the rules that match, so you avoid creating duplicates without paying to re-read hundreds of rules each time. 3. Process by raw provider category group, starting with the largest groups: a. `query_transactions(tags=["needs-review"], fields=core,category, limit up to 500)` — you can iterate with cursor pagination. diff --git a/prompts/agents/strategy-initial-setup.md b/prompts/agents/strategy-initial-setup.md index 550c0c3f8..d721a345d 100644 --- a/prompts/agents/strategy-initial-setup.md +++ b/prompts/agents/strategy-initial-setup.md @@ -8,13 +8,13 @@ You are performing the very first categorization setup for a user who just conne > [!WARNING] > **Safety check — do this first.** -> 1. Read `breadbox://overview` and check `list_transaction_rules`. +> 1. Call `get_overview` and check `list_transaction_rules`. > 2. If rules already exist, this is NOT a first-time setup. Inform the user: "Rules already exist — this looks like a returning account. Consider using Bulk Review instead." > 3. If proceeding anyway: do NOT use `apply_retroactively=true` (it would re-categorize already-reviewed transactions), skip broad `category_primary` rule creation, and treat this as a bulk review instead. ## Steps (cold start — no existing rules) -1. Read `breadbox://overview` and `count_transactions(tags=["needs-review"])` to understand the backlog size. If empty, check `get_sync_status` — the account may not have synced yet. Report and exit if no data. +1. Call `get_overview` and `query_transactions(tags=["needs-review"], count_only=true)` to understand the backlog size. If empty, check `get_sync_status` — the account may not have synced yet. Report and exit if no data. 2. Create broad `category_primary` rules — one per raw provider category, scoped to the specific provider. Use `preview_rule` to verify each before creating. Use `apply_retroactively=true` since this is the initial cold start. Example: ```text @@ -29,7 +29,7 @@ You are performing the very first categorization setup for a user who just conne "Service Charge" → bank_fees ``` -4. `count_transactions(tags=["needs-review"])` again to see what remains uncovered after the retroactive rule pass. +4. `query_transactions(tags=["needs-review"], count_only=true)` again to see what remains uncovered after the retroactive rule pass. 5. Process remaining tagged transactions group by group: `query_transactions(tags=["needs-review"], fields=core,category)`. Review each and apply `update_transactions` with a compound op (`category_slug` + `tags_to_remove` `needs-review` with a note), batching up to 50 per call. 6. For miscategorized merchants, create per-merchant rules (without retroactive — they'll catch future instances). 7. Submit a report with an overview of coverage, not a full rule listing. diff --git a/prompts/agents/strategy-large-charge-sentinel.md b/prompts/agents/strategy-large-charge-sentinel.md index a310b3b6a..9083d40f5 100644 --- a/prompts/agents/strategy-large-charge-sentinel.md +++ b/prompts/agents/strategy-large-charge-sentinel.md @@ -12,7 +12,7 @@ Flag individual transactions whose amount is large enough, relative to the famil ## Steps -1. Read `breadbox://overview` for baseline context (accounts, users, currency, freshness). +1. Call `get_overview` for baseline context (accounts, users, currency, freshness). 2. List the largest recent debits: `query_transactions` with `sort_by=amount`, `sort_order=desc` over the lookback window (default: last 7 days). Remember amount sign — positive = money out. 3. Establish "normal" for context: `transaction_summary` with `group_by=category` gives a per-category baseline; `query_transactions(search="")` surfaces a merchant's recent charges so you can judge what's typical. A $400 grocery run is unusual; a $400 flight is not. 4. For each candidate, decide whether it is genuinely out of pattern: diff --git a/prompts/agents/strategy-quick-review.md b/prompts/agents/strategy-quick-review.md index 0cc5eabb9..d0c501140 100644 --- a/prompts/agents/strategy-quick-review.md +++ b/prompts/agents/strategy-quick-review.md @@ -12,7 +12,7 @@ Clear 80%+ of the backlog with reasonable accuracy. Leave uncertain items tagged ## Steps -1. `count_transactions(tags=["needs-review"])` — see queue size. If zero, report "queue clear" and exit. +1. `query_transactions(tags=["needs-review"], count_only=true)` — see queue size. If zero, report "queue clear" and exit. 2. Process the largest raw category groups first: a. `query_transactions(tags=["needs-review"], fields=minimal, limit up to 500)` — pull a large slice. b. Scan the transactions — if the category mapping is clear, call `update_transactions` in batches of up to 50 operations per call: diff --git a/prompts/agents/strategy-routine-review.md b/prompts/agents/strategy-routine-review.md index 972eeae98..243bb7f5a 100644 --- a/prompts/agents/strategy-routine-review.md +++ b/prompts/agents/strategy-routine-review.md @@ -12,7 +12,7 @@ Clear the `needs-review` backlog with care. Create rules for new recurring patte ## Steps -1. `count_transactions(tags=["needs-review"])` — if zero, check `get_sync_status` for data freshness and report accordingly. +1. `query_transactions(tags=["needs-review"], count_only=true)` — if zero, check `get_sync_status` for data freshness and report accordingly. 2. `query_transactions(tags=["needs-review"], fields=core,category, limit 30)` — fetch the backlog. 3. For each transaction with prior activity (existing category/rule applications), call `list_annotations` to see its history — respect human corrections that are recorded as prior comments. 4. Review each transaction: diff --git a/prompts/agents/strategy-rule-foundation.md b/prompts/agents/strategy-rule-foundation.md index 6d491f8b6..21fa856e2 100644 --- a/prompts/agents/strategy-rule-foundation.md +++ b/prompts/agents/strategy-rule-foundation.md @@ -12,11 +12,11 @@ Establish (or improve on) a high-precision set of transaction rules from the las ## Steps -1. Read `breadbox://overview` for context. `list_categories` for the taxonomy. `list_transaction_rules` to see what already exists — improve coverage and fill gaps; never duplicate an existing rule. +1. Call `get_overview` for context. `list_categories` for the taxonomy. `list_transaction_rules` to see what already exists — improve coverage and fill gaps; never duplicate an existing rule. 2. **Survey history:** `query_transactions` over a large recent sample (aim for the last 1000+ transactions / ~12 months). Use `transaction_summary` to surface the categories that dominate, and scan the sample for merchant names that recur most. 3. **Identify rule candidates.** A good candidate has STRONG, REPEATED evidence — the same merchant, or a stable name pattern, mapping consistently to one category across many transactions (prefer 3+ consistent occurrences). Favor specific, high-precision conditions (e.g. merchant name contains "X") over broad catch-alls. See the rule DSL for the condition grammar and operators. 4. **Dry-run EVERY candidate before creating it:** `preview_rule` reports how many transactions it would match and surfaces conflicts. Also call `find_matching_rules(merchant="")` per candidate to confirm no existing rule already covers it — a targeted check that beats re-scanning the whole list. Only proceed with rules whose preview is clean and high-precision and that aren't already covered — discard anything that over-matches, duplicates an existing rule, or would fight a category a human already set. -5. **Create the vetted rules** (`create_transaction_rule`, or `batch_create_rules` for several at once). Rules write the category source as `none` and must NEVER overwrite a user-locked category (`category_override='user'` is sacred). +5. **Create the vetted rules** (`create_transaction_rule` — pass a `rules` array for several at once). Rules write the category source as `none` and must NEVER overwrite a user-locked category (`category_override='user'` is sacred). 6. **Backfill carefully:** for rules you are confident in, use `apply_rules` to categorize the matching backlog. Apply conservatively — a clean dry run is the prerequisite, and a smaller set of precise rules beats a broad, fragile sweep. 7. **Submit a report** listing each rule created (its conditions, target category, and match count) and what was backfilled. diff --git a/prompts/agents/strategy-spending-report.md b/prompts/agents/strategy-spending-report.md index f7cf481b0..d24bb1bcf 100644 --- a/prompts/agents/strategy-spending-report.md +++ b/prompts/agents/strategy-spending-report.md @@ -12,7 +12,7 @@ Help the family understand where their money went, how spending changed, and whe ## Steps -1. Read `breadbox://overview` for context (accounts, users, date range, data freshness). +1. Call `get_overview` for context (accounts, users, date range, data freshness). 2. Use `transaction_summary` with `group_by=category` for the target period (default: last 30 days). 3. Use `transaction_summary` with `group_by=category_month` to compare against the prior period. 4. Query notable individual transactions if anything stands out (`query_transactions` with `fields=core,category`); scan that sample for merchants that recur or for unusually large charges. diff --git a/prompts/mcp/instructions.md b/prompts/mcp/instructions.md index f18058cee..50b4a015b 100644 --- a/prompts/mcp/instructions.md +++ b/prompts/mcp/instructions.md @@ -1,25 +1,24 @@ -Breadbox is a self-hosted financial-data aggregator server for households. It syncs transactions and other data via Plaid, Teller, or CSV imports into one unified database and exposes tools and resources for reviewing, enriching and interacting with financial data. +Breadbox is a self-hosted financial-data aggregator server for households. It syncs transactions and other data via Plaid, Teller, or CSV imports into one unified database and exposes tools for reviewing, enriching and interacting with financial data. (Everything is a tool — there are no MCP resources.) ## Where to Start -Read `get_reference(kind=overview)` first (or the `breadbox://overview` resource if your client supports resources) for a household snapshot, then use the tools and resources to interact with the data. +Call `get_overview` first for a household snapshot (scope, freshness, pending-review backlog), then use the tools below to interact with the data. -Bounded reference data is read through one tool — `get_reference(kind=…)`: -- `get_reference(kind=overview)` — household snapshot (scope, freshness, review backlog) -- `get_reference(kind=accounts)` — bank accounts (optional `user_id` filter) -- `get_reference(kind=categories)` — the category taxonomy (source of `category_slug` values) -- `get_reference(kind=tags)` — the tag vocabulary -- `get_reference(kind=users)` — household members -- `get_reference(kind=sync_status)` — per-connection sync status / freshness -- `get_reference(kind=rules)` — the transaction-rule roster (lean; `fields=all` for full) +Bounded reference data — each its own tool: +- `get_overview` — household snapshot +- `list_accounts` — bank accounts (optional `user_id` filter) +- `list_categories` — the category taxonomy (source of `category_slug` values) +- `list_tags` — the tag vocabulary +- `list_users` — household members +- `get_sync_status` — per-connection sync status / freshness +- `list_transaction_rules` — the transaction-rule roster (lean; `fields=all` for full) -A couple of these are also exposed as resources for clients with a resource/attach UI — `breadbox://overview` and `breadbox://sync-status` — with the same payload as the matching `get_reference` kind. - -Per-entity drilldowns are exposed as resource templates (resolve a single short_id): -- `breadbox://transaction/{short_id}` — full transaction + recent annotations -- `breadbox://account/{short_id}` — account detail + last 25 transactions -- `breadbox://user/{short_id}` — household member + connected accounts +Operating-guidance docs — read via `get_reference(kind=…)` when you need them: +- `get_reference(kind=instructions)` — this document +- `get_reference(kind=rule-dsl)` — the transaction-rule condition grammar (read before authoring rules) +- `get_reference(kind=review-guidelines)` — how to work the needs-review queue +- `get_reference(kind=report-format)` — structure/format for `submit_report` ## Conventions - **Amount sign**: positive = money out, negative = money in. Never sum across `iso_currency_code`. -- **Compact IDs**: to save on tokens, tools/resources use a 8-char base62 `short_id`; prefer over long form id (uuid) +- **Compact IDs**: to save on tokens, tools use a 8-char base62 `short_id`; prefer over long form id (uuid) - **Audit sessions are automatic.** Every tool call is logged under an audit-session row keyed off the transport connection (the `MCP-Session-Id` header for HTTP, a per-process id for stdio). Agents no longer need to call `create_session` — the row is lazy-created on first tool call and inherits `clientInfo` from the `initialize` request. To label a specific call, pass an optional `reason` string in `tools/call._meta` (the spec's per-request metadata slot); it surfaces in the audit timeline alongside the call. \ No newline at end of file diff --git a/prompts/mcp/review-guidelines.md b/prompts/mcp/review-guidelines.md index 8f00b0081..4a9c78b63 100644 --- a/prompts/mcp/review-guidelines.md +++ b/prompts/mcp/review-guidelines.md @@ -25,7 +25,7 @@ RULE CREATION: - Fields: provider_name, provider_merchant_name, amount, provider_category_primary (raw provider category), provider_category_detailed, pending, provider, account_id, user_id, user_name BEFORE CREATING A RULE: -1. Read get_reference(kind=rules) to avoid duplicates +1. Read list_transaction_rules to avoid duplicates 2. Use preview_rule to test your conditions — verify match count and review sample transactions 3. Query some transactions with fields=core,category to see what provider_category_primary values exist @@ -49,9 +49,9 @@ RULE PIPELINE STAGES & CONFLICTS: - Rules fire in priority-ASC order during sync (lower priority runs first; later stages observe earlier-stage tag/category mutations). - Pass `stage` (not raw `priority`) when authoring rules — `baseline` (broad defaults) → `standard` (default) → `refinement` (reacts to earlier stages) → `override` (last word). Stage resolves to priority 0/10/50/100. If both are supplied, raw priority wins. - Recommended mapping by pattern: `provider_category_primary` rules → `baseline` or `standard`; name-pattern rules (`contains` on `provider_name`) → `standard`; per-merchant rules → `refinement` or `override`. -- Check get_reference(kind=rules) before creating. If two rules can match the same transaction, the higher-stage one wins under last-writer semantics. +- Check list_transaction_rules before creating. If two rules can match the same transaction, the higher-stage one wins under last-writer semantics. -Use batch_create_rules (max 100) to create multiple rules efficiently. +Use `create_transaction_rule` with a `rules` array (max 100) to create multiple rules in one call. Prefer contains over exact match — bank feeds format merchant names inconsistently. Always use category_slug (not category_id) when creating rules. diff --git a/prompts/mcp/rule-dsl.md b/prompts/mcp/rule-dsl.md index 4045202ed..2a8888069 100644 --- a/prompts/mcp/rule-dsl.md +++ b/prompts/mcp/rule-dsl.md @@ -91,9 +91,9 @@ If two rules can match the same transaction, the higher priority wins; ties reso ## Authoring checklist -1. Read `get_reference(kind=rules)` to avoid duplicates. +1. Read `list_transaction_rules` to avoid duplicates. 2. `preview_rule` your conditions to verify match count and review a sample of matched transactions. 3. Pick the right priority band for the pattern type. 4. Use `category_slug` (never `category_id`). 5. Prefer `contains` over exact match — bank feeds format names inconsistently. -6. Use `batch_create_rules` (max 100) to land related rules in one call. +6. Use `create_transaction_rule` with a `rules` array (max 100) to land related rules in one call. From 1a66fd768779e12363785400ddcb1c8ce9bd1c93 Mon Sep 17 00:00:00 2001 From: Ricardo Canales Date: Tue, 23 Jun 2026 23:33:45 -0700 Subject: [PATCH 5/6] refactor(mcp): consolidate series + counterparty write tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Carry the PR's consolidation philosophy into the tools main added in the rules-as-substrate merge (#1906), so series and counterparties mirror each other and the transaction pattern (list/get/assign/update/unlink). - Fold add_series_tag + remove_series_tag into update_series via tags_to_add / tags_to_remove (same pattern as update_transactions; each tag sub-change still calls AddSeriesTag/RemoveSeriesTag, so inheritance/provenance is unchanged). - Fold create_counterparty into assign_counterparty: mint via create_if_missing, preserve strict-create as an optional fail_if_exists flag, and apply enrichment (website_url/logo_url/ category_id/mcc) inline — losing no capability. Mint a bare counterparty by passing name + create_if_missing with no transaction_ids. - Rename unlink_counterparty_transaction → unlink_counterparty_transactions for parity with unlink_series_transactions. Net: 38 → 35 tools. Updated docs/mcp-tools-reference.md and .claude/rules/mcp.md to match. Verified: go build ./..., go vet ./..., and the full internal/mcp integration suite green on a clean DB. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Y7qg4TN9ZPcHsXR714UrjG --- .claude/rules/mcp.md | 4 +- docs/mcp-tools-reference.md | 14 ++--- internal/api/counterparties.go | 2 +- internal/mcp/server.go | 20 ++----- internal/mcp/tools_counterparties.go | 79 +++++++++---------------- internal/mcp/tools_series.go | 88 +++++++++++----------------- 6 files changed, 76 insertions(+), 131 deletions(-) diff --git a/.claude/rules/mcp.md b/.claude/rules/mcp.md index bfa2bd8db..4cfd8dbd9 100644 --- a/.claude/rules/mcp.md +++ b/.claude/rules/mcp.md @@ -92,8 +92,10 @@ Several writes are deliberately compound so the registry stays small and an agen - `update_transactions` — see above (category, tags, comment, flag). - `set_transaction_metadata` — the single op over the free-form metadata JSONB column: `set` (merge keys), `unset` (delete keys), `replace:true` (swap/clear the whole blob). Absorbed `remove_`/`replace_`/`clear_transaction_metadata`. - `create_transaction_rule` — takes a `rules` array of 1..N specs (absorbed `batch_create_rules`); each spec may carry `apply_retroactively`. +- `update_series` — edits a recurring series' `name`, `type`, and tag membership via `tags_to_add` / `tags_to_remove` (absorbed `add_series_tag` / `remove_series_tag`). Each tag sub-change still calls the same service method, so inheritance/provenance is unchanged. +- `assign_counterparty` — mints-or-resolves a counterparty by name and links transactions; absorbed `create_counterparty` via `create_if_missing` (+ optional `fail_if_exists` for strict create) plus inline enrichment (`website_url` / `logo_url` / `category_id` / `mcc`). Mint a bare counterparty by passing `name` + `create_if_missing` with no `transaction_ids`. -The recurring-series subsystem (rules-as-substrate) keeps its writes split — `assign_series`, `update_series` (name/type), `unlink_series_transactions`, `add_series_tag`, `remove_series_tag` — to mirror the rule-driven membership model; don't fold those. +The series + counterparty subsystems (rules-as-substrate) otherwise mirror each other: `list_*` / `get_*` reads, `assign_*` (mints + links), `update_*` (edits), `unlink_*_transactions`. Membership for both comes from rules, not a shipped detector — don't add per-attribute write tools; fold into `update_*` or `assign_*`. When folding a tool, prefer reusing the existing service methods from the compound handler over re-implementing the write — the MCP layer is where the consolidation lives. diff --git a/docs/mcp-tools-reference.md b/docs/mcp-tools-reference.md index 985aaed52..52c6ddd0f 100644 --- a/docs/mcp-tools-reference.md +++ b/docs/mcp-tools-reference.md @@ -281,7 +281,7 @@ Link transactions to a recurring series, creating it if needed — the agent's p ### update_series (Write) -Edit a recurring series' `name` and/or `type` (`subscription` | `bill` | `loan` | `other`). Both optional — omit to leave unchanged. Renaming onto an existing live series name is rejected (the name is the series' unique mint key). +Edit a recurring series in one call: its `name`, `type` (`subscription` | `bill` | `loan` | `other`), and tag membership (`tags_to_add` / `tags_to_remove`). Every field optional — omit to leave unchanged. Renaming onto an existing live series name is rejected (the name is the series' unique mint key). Tags must already exist (`create_tag`); an added tag is materialized onto every linked charge and applied to future members, a removed tag strips the series-inherited copies (a tag a user added directly to a charge survives). ### unlink_series_transactions (Write) @@ -301,11 +301,7 @@ List every live counterparty with its `name` and enrichment fields. Get a counte ### get_counterparty (Read) -Get one counterparty by short ID or UUID: its `name` and enrichment fields. Its governing rules (the `assign_counterparty` rules that define its membership) are visible on the admin Counterparties detail page; its linked charges come from `query_transactions`. Also exposed as the `breadbox://counterparty/{short_id}` resource (detail + governing rules). - -### create_counterparty (Write) - -Create a new counterparty with a `name` and optional enrichment (`website_url`, `logo_url`, `category_id`, `mcc`). Creating onto an existing live name is rejected — edit that one instead. To bind charges, use `assign_counterparty` (one-off) or author an `assign_counterparty` **rule** (durable). +Get one counterparty by short ID or UUID: its `name` and enrichment fields. Its governing rules (the `assign_counterparty` rules that define its membership) are visible on the admin Counterparties detail page; its linked charges come from `query_transactions`. ### update_counterparty (Write) @@ -313,12 +309,14 @@ Enrich a counterparty: edit its `name`, `website_url`, `logo_url`, `category_id` ### assign_counterparty (Write) -Bind transactions to a counterparty, creating it if needed — a **one-off** assignment. For durable patterns, author an `assign_counterparty` **rule** so every future matching charge resolves automatically. Provide `counterparty_id` to bind to an existing counterparty, **or** `name` + `create_if_missing:true` to resolve-or-create one by name (surrogate-first; de-dupes on the live name). Pass `transaction_ids` (≤50) to link members (NULL-fill only — never steals a charge already bound elsewhere). +Bind transactions to a counterparty, minting it if needed — a **one-off** assignment. For durable patterns, author an `assign_counterparty` **rule** so every future matching charge resolves automatically. Provide `counterparty_id` to bind to an existing counterparty, **or** `name` + `create_if_missing:true` to resolve-or-create one by name (surrogate-first; de-dupes on the live name). Mint a **bare** counterparty by passing `name` + `create_if_missing` with no `transaction_ids`; pass `fail_if_exists:true` for a strict "make a brand-new one" intent. Optional enrichment (`website_url`, `logo_url`, `category_id`, `mcc`) is applied to the resolved/minted counterparty in the same call. Pass `transaction_ids` (≤50) to link members (NULL-fill only — never steals a charge already bound elsewhere). -### unlink_counterparty_transaction (Write) +### unlink_counterparty_transactions (Write) Detach `transaction_ids` (≤50, each a current member) from a counterparty — the inverse of `assign_counterparty`' link path. Clears each charge's `counterparty_id`. Errors if any listed transaction isn't a current member, so it can't silently no-op or touch another counterparty. +> Counterparty **creation** folds into `assign_counterparty` (`name` + `create_if_missing`, optional `fail_if_exists` + enrichment) — there is no standalone `create_counterparty` tool. + --- ## Transaction Rules Tools diff --git a/internal/api/counterparties.go b/internal/api/counterparties.go index 24de6089d..7cf940693 100644 --- a/internal/api/counterparties.go +++ b/internal/api/counterparties.go @@ -155,7 +155,7 @@ func AssignCounterpartyTransactionsHandler(svc *service.Service) http.HandlerFun // UnlinkCounterpartyTransactionHandler detaches a single transaction from a // counterparty. DELETE /api/v1/counterparties/{id}/transactions/{txid} — mirrors -// the unlink_counterparty_transaction MCP tool. Requires full_access scope. +// the unlink_counterparty_transactions MCP tool. Requires full_access scope. func UnlinkCounterpartyTransactionHandler(svc *service.Service) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { id := chi.URLParam(r, "id") diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 56cf0e5be..c8161ab34 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -402,20 +402,12 @@ func (s *MCPServer) buildToolRegistry() { }, s.handleAssignSeries, s), makeToolDefLogged(ToolSpec{ Name: "update_series", Title: "Edit Recurring Series", Classification: ToolWrite, - Description: "Edit a recurring series' name and/or type (subscription, bill, loan, other). Both optional — omit to leave unchanged. Renaming onto an existing live series name is rejected (the name is the series' unique mint key).", + Description: "Edit a recurring series in one call: its name, type (subscription, bill, loan, other), and tag membership (tags_to_add / tags_to_remove). Every field is optional — omit to leave unchanged. Renaming onto an existing live series name is rejected (the name is the series' unique mint key). Tags must already exist (create_tag); an added tag is materialized onto every linked charge and applied to future members, a removed tag strips the series-inherited copies (a tag a user added directly to a charge survives).", }, s.handleUpdateSeries, s), makeToolDefLogged(ToolSpec{ Name: "unlink_series_transactions", Title: "Unlink Charges from Series", Classification: ToolWrite, Description: "Detach transactions (≤50, each a current member) from a recurring series — the inverse of assign_series' link path. Clears each charge's series_id and strips the series' inherited tags from them (a tag the user added directly survives). Errors if any listed transaction isn't a current member, so it can't silently no-op or touch another series.", }, s.handleUnlinkSeriesTransactions, s), - makeToolDefLogged(ToolSpec{ - Name: "add_series_tag", Title: "Tag Recurring Series", Classification: ToolWrite, - Description: "Attach an existing tag to a recurring series. The tag is materialized onto every linked transaction (they inherit it) and applied to future members as they join — so tagging the Netflix series tags all its charges. The tag must already exist (create it first with create_tag).", - }, s.handleAddSeriesTag, s), - makeToolDefLogged(ToolSpec{ - Name: "remove_series_tag", Title: "Untag Recurring Series", Classification: ToolWrite, - Description: "Detach a tag from a recurring series and strip the series-inherited copies from its linked transactions. Provenance-scoped: a tag a user added directly to a transaction survives.", - }, s.handleRemoveSeriesTag, s), // --- Counterparties (rules-as-substrate, P4) --- makeToolDefLogged(ToolSpec{ @@ -426,20 +418,16 @@ func (s *MCPServer) buildToolRegistry() { Name: "get_counterparty", Title: "Get Counterparty", Classification: ToolRead, Description: "Get one counterparty by short ID or UUID: its name and enrichment fields. Its governing rules (the assign_counterparty rules that define its membership) are on the admin Counterparties detail page; its linked charges come from query_transactions.", }, s.handleGetCounterparty, s), - makeToolDefLogged(ToolSpec{ - Name: "create_counterparty", Title: "Create Counterparty", Classification: ToolWrite, - Description: "Create a new counterparty with a name and optional enrichment (website_url, logo_url, category_id, mcc). Creating onto an existing live name is rejected — edit that one instead. To bind charges, use assign_counterparty (one-off) or author an assign_counterparty RULE (durable).", - }, s.handleCreateCounterparty, s), makeToolDefLogged(ToolSpec{ Name: "update_counterparty", Title: "Enrich Counterparty", Classification: ToolWrite, Description: "Enrich a counterparty: edit its name, website_url, logo_url, category_id (slug or short ID), and/or mcc. Every field optional — omit to leave unchanged; an empty name is rejected. This is the enrichment lane (no auto-fetch).", }, s.handleUpdateCounterparty, s), makeToolDefLogged(ToolSpec{ - Name: "assign_counterparty", Title: "Assign Counterparty", Classification: ToolWrite, - Description: "Bind transactions to a counterparty, creating it if needed. This is a ONE-OFF assignment. For durable patterns, author an assign_counterparty RULE instead so every future matching charge resolves automatically. Provide counterparty_id to bind to an existing counterparty, OR name + create_if_missing:true to resolve-or-create one by name (surrogate-first; de-dupes on the live name). Pass transaction_ids (≤50) to link members (NULL-fill only — never steals a charge already bound to another counterparty).", + Name: "assign_counterparty", Title: "Assign / Create Counterparty", Classification: ToolWrite, + Description: "Bind transactions to a counterparty, minting it if needed — the agent's path for a one-off assignment (author an assign_counterparty RULE instead when you want future charges to resolve automatically). Provide counterparty_id to bind to an existing one, OR name + create_if_missing:true to resolve-or-create by name (surrogate-first; de-dupes on the live name). Mint a bare counterparty by passing name + create_if_missing with no transaction_ids; pass fail_if_exists:true for a strict 'make a brand-new one' intent. Optional enrichment (website_url, logo_url, category_id, mcc) is applied to the resolved/minted counterparty in the same call. Pass transaction_ids (≤50) to link members (NULL-fill only — never steals a charge already bound elsewhere).", }, s.handleAssignCounterparty, s), makeToolDefLogged(ToolSpec{ - Name: "unlink_counterparty_transaction", Title: "Unlink Charge from Counterparty", Classification: ToolWrite, + Name: "unlink_counterparty_transactions", Title: "Unlink Charges from Counterparty", Classification: ToolWrite, Description: "Detach transactions (≤50, each a current member) from a counterparty — the inverse of assign_counterparty's link path. Clears each charge's counterparty_id. Errors if any listed transaction isn't a current member, so it can't silently no-op or touch another counterparty.", }, s.handleUnlinkCounterpartyTransaction, s), diff --git a/internal/mcp/tools_counterparties.go b/internal/mcp/tools_counterparties.go index ba2fc797f..7605c2c2e 100644 --- a/internal/mcp/tools_counterparties.go +++ b/internal/mcp/tools_counterparties.go @@ -18,14 +18,6 @@ type getCounterpartyInput struct { ID string `json:"id" jsonschema:"required,Counterparty short ID or UUID."` } -type createCounterpartyInput struct { - Name string `json:"name" jsonschema:"required,Display name for the counterparty (the canonical, cross-provider 'other side' of a charge — a merchant, person, or employer). De-duped on the live name, but not unique."` - WebsiteURL string `json:"website_url,omitempty" jsonschema:"Optional canonical website URL."` - LogoURL string `json:"logo_url,omitempty" jsonschema:"Optional logo image URL."` - CategoryID string `json:"category_id,omitempty" jsonschema:"Optional default category (slug or short ID/UUID) for this counterparty's charges."` - MCC string `json:"mcc,omitempty" jsonschema:"Optional 4-digit merchant category code."` -} - type updateCounterpartyInput struct { ID string `json:"id" jsonschema:"required,Counterparty short ID or UUID to enrich."` Name *string `json:"name,omitempty" jsonschema:"New display name. Omit to leave unchanged; empty string is rejected."` @@ -37,9 +29,14 @@ type updateCounterpartyInput struct { type assignCounterpartyInput struct { CounterpartyID string `json:"counterparty_id,omitempty" jsonschema:"Existing counterparty short ID or UUID to bind transactions to. Provide this OR (name + create_if_missing)."` - Name string `json:"name,omitempty" jsonschema:"Name to resolve-or-create a counterparty under (requires create_if_missing). Surrogate-first: pick a clean canonical label — e.g. 'Amazon'."` + Name string `json:"name,omitempty" jsonschema:"Name to resolve-or-create a counterparty under (requires create_if_missing). Surrogate-first: pick a clean canonical label — e.g. 'Amazon'. To mint a bare counterparty (no transactions), pass name + create_if_missing with no transaction_ids."` CreateIfMissing bool `json:"create_if_missing,omitempty" jsonschema:"Resolve (or create) a counterparty by name when no counterparty_id is given."` + FailIfExists bool `json:"fail_if_exists,omitempty" jsonschema:"With create_if_missing, error instead of resolving when a counterparty already exists at this name — use for a strict 'make a brand-new counterparty' intent."` TransactionIDs []string `json:"transaction_ids,omitempty" jsonschema:"Transactions (short ID or UUID) to link to the counterparty. Max 50 per call. NULL-fill only — never steals a charge already bound elsewhere."` + WebsiteURL string `json:"website_url,omitempty" jsonschema:"Optional enrichment applied to the resolved/minted counterparty: canonical website URL."` + LogoURL string `json:"logo_url,omitempty" jsonschema:"Optional enrichment: logo image URL."` + CategoryID string `json:"category_id,omitempty" jsonschema:"Optional enrichment: default category (slug or short ID/UUID) for this counterparty's charges."` + MCC string `json:"mcc,omitempty" jsonschema:"Optional enrichment: 4-digit merchant category code."` } type unlinkCounterpartyTransactionInput struct { @@ -69,48 +66,6 @@ func (s *MCPServer) handleGetCounterparty(_ context.Context, _ *mcpsdk.CallToolR return jsonResult(cp) } -func (s *MCPServer) handleCreateCounterparty(ctx context.Context, _ *mcpsdk.CallToolRequest, input createCounterpartyInput) (*mcpsdk.CallToolResult, any, error) { - if err := s.checkWritePermission(ctx); err != nil { - return errorResult(err), nil, nil - } - if input.Name == "" { - return errorResult(fmt.Errorf("name is required")), nil, nil - } - actor := service.ActorFromContext(ctx) - // Strict create (FailIfExists) — an explicit "make a new counterparty" intent - // should surface a conflict rather than silently resolve an existing row. - cp, err := s.svc.AssignCounterparty(context.Background(), service.AssignCounterpartyInput{ - Name: input.Name, - CreateIfMissing: true, - FailIfExists: true, - }, actor) - if err != nil { - return errorResult(err), nil, nil - } - // Apply any enrichment fields supplied at create time. - if input.WebsiteURL != "" || input.LogoURL != "" || input.CategoryID != "" || input.MCC != "" { - edit := service.EditCounterpartyInput{} - if input.WebsiteURL != "" { - edit.WebsiteURL = &input.WebsiteURL - } - if input.LogoURL != "" { - edit.LogoURL = &input.LogoURL - } - if input.CategoryID != "" { - edit.CategoryID = &input.CategoryID - } - if input.MCC != "" { - edit.MCC = &input.MCC - } - enriched, err := s.svc.UpdateCounterparty(context.Background(), cp.ShortID, edit, actor) - if err != nil { - return errorResult(err), nil, nil - } - cp = enriched - } - return jsonResult(cp) -} - func (s *MCPServer) handleUpdateCounterparty(ctx context.Context, _ *mcpsdk.CallToolRequest, input updateCounterpartyInput) (*mcpsdk.CallToolResult, any, error) { if err := s.checkWritePermission(ctx); err != nil { return errorResult(err), nil, nil @@ -145,6 +100,7 @@ func (s *MCPServer) handleAssignCounterparty(ctx context.Context, _ *mcpsdk.Call CounterpartyShortID: optStr(input.CounterpartyID), Name: input.Name, CreateIfMissing: input.CreateIfMissing, + FailIfExists: input.FailIfExists, TransactionIDs: input.TransactionIDs, }, actor) if err != nil { @@ -153,6 +109,27 @@ func (s *MCPServer) handleAssignCounterparty(ctx context.Context, _ *mcpsdk.Call } return errorResult(err), nil, nil } + // Apply any enrichment fields in the same call (folds the former create_counterparty). + if input.WebsiteURL != "" || input.LogoURL != "" || input.CategoryID != "" || input.MCC != "" { + edit := service.EditCounterpartyInput{} + if input.WebsiteURL != "" { + edit.WebsiteURL = &input.WebsiteURL + } + if input.LogoURL != "" { + edit.LogoURL = &input.LogoURL + } + if input.CategoryID != "" { + edit.CategoryID = &input.CategoryID + } + if input.MCC != "" { + edit.MCC = &input.MCC + } + enriched, err := s.svc.UpdateCounterparty(context.Background(), cp.ShortID, edit, actor) + if err != nil { + return errorResult(err), nil, nil + } + cp = enriched + } return jsonResult(cp) } diff --git a/internal/mcp/tools_series.go b/internal/mcp/tools_series.go index 2ebc5da5e..0363bff1a 100644 --- a/internal/mcp/tools_series.go +++ b/internal/mcp/tools_series.go @@ -30,9 +30,11 @@ type assignSeriesInput struct { } type updateSeriesInput struct { - ID string `json:"id" jsonschema:"required,Series short ID or UUID to edit."` - Name string `json:"name,omitempty" jsonschema:"New display name. Renaming onto an existing live series name is rejected. Omit to leave unchanged."` - Type string `json:"type,omitempty" jsonschema:"New recurring-charge type: subscription, bill, loan, or other. Omit to leave unchanged."` + ID string `json:"id" jsonschema:"required,Series short ID or UUID to edit."` + Name string `json:"name,omitempty" jsonschema:"New display name. Renaming onto an existing live series name is rejected. Omit to leave unchanged."` + Type string `json:"type,omitempty" jsonschema:"New recurring-charge type: subscription, bill, loan, or other. Omit to leave unchanged."` + TagsToAdd []string `json:"tags_to_add,omitempty" jsonschema:"Tag slugs to attach to the series (each must already exist — create with create_tag). An added tag is materialized onto every linked charge and applied to future members as they join."` + TagsToRemove []string `json:"tags_to_remove,omitempty" jsonschema:"Tag slugs to detach from the series, stripping the series-inherited copies from its linked charges (a tag a user added directly to a charge survives)."` } type unlinkSeriesInput struct { @@ -40,11 +42,6 @@ type unlinkSeriesInput struct { TransactionIDs []string `json:"transaction_ids" jsonschema:"required,Transactions (short ID or UUID) to detach from the series. Each must currently belong to it. Max 50."` } -type seriesTagInput struct { - ID string `json:"id" jsonschema:"required,Series short ID or UUID."` - TagSlug string `json:"tag_slug" jsonschema:"required,Tag slug (must already exist)."` -} - func (s *MCPServer) handleListSeries(_ context.Context, _ *mcpsdk.CallToolRequest, input listSeriesInput) (*mcpsdk.CallToolResult, any, error) { ctx := context.Background() // Lean-by-default: list_series returns the overview projection (identity + @@ -122,51 +119,36 @@ func (s *MCPServer) handleUpdateSeries(ctx context.Context, _ *mcpsdk.CallToolRe Name: optStr(input.Name), Type: optStr(input.Type), } - if edit.Name == nil && edit.Type == nil { - return errorResult(fmt.Errorf("provide name and/or type to update")), nil, nil + if edit.Name == nil && edit.Type == nil && len(input.TagsToAdd) == 0 && len(input.TagsToRemove) == 0 { + return errorResult(fmt.Errorf("provide name, type, tags_to_add, or tags_to_remove to update")), nil, nil } actor := service.ActorFromContext(ctx) - series, err := s.svc.UpdateSeries(context.Background(), input.ID, edit, actor) - if err != nil { - if errors.Is(err, service.ErrNotFound) { - return errorResult(fmt.Errorf("series not found")), nil, nil + // Name/type edit first (only when supplied), then tag membership. Each + // sub-change calls the same service method the standalone tools used, so the + // semantics (rename collision guard, tag inheritance/provenance) are unchanged. + if edit.Name != nil || edit.Type != nil { + if _, err := s.svc.UpdateSeries(context.Background(), input.ID, edit, actor); err != nil { + if errors.Is(err, service.ErrNotFound) { + return errorResult(fmt.Errorf("series not found")), nil, nil + } + return errorResult(err), nil, nil } - return errorResult(err), nil, nil - } - return jsonResult(series) -} - -func (s *MCPServer) handleUnlinkSeriesTransactions(ctx context.Context, _ *mcpsdk.CallToolRequest, input unlinkSeriesInput) (*mcpsdk.CallToolResult, any, error) { - if err := s.checkWritePermission(ctx); err != nil { - return errorResult(err), nil, nil - } - if input.ID == "" || len(input.TransactionIDs) == 0 { - return errorResult(fmt.Errorf("id and transaction_ids are required")), nil, nil } - actor := service.ActorFromContext(ctx) - series, err := s.svc.UnlinkSeriesTransactions(context.Background(), input.ID, input.TransactionIDs, actor) - if err != nil { - if errors.Is(err, service.ErrNotFound) { - return errorResult(fmt.Errorf("series not found")), nil, nil + for _, slug := range input.TagsToAdd { + if err := s.svc.AddSeriesTag(context.Background(), input.ID, slug, actor); err != nil { + if errors.Is(err, service.ErrNotFound) { + return errorResult(fmt.Errorf("series not found")), nil, nil + } + return errorResult(err), nil, nil } - return errorResult(err), nil, nil } - return jsonResult(series) -} - -func (s *MCPServer) handleAddSeriesTag(ctx context.Context, _ *mcpsdk.CallToolRequest, input seriesTagInput) (*mcpsdk.CallToolResult, any, error) { - if err := s.checkWritePermission(ctx); err != nil { - return errorResult(err), nil, nil - } - if input.ID == "" || input.TagSlug == "" { - return errorResult(fmt.Errorf("id and tag_slug are required")), nil, nil - } - actor := service.ActorFromContext(ctx) - if err := s.svc.AddSeriesTag(context.Background(), input.ID, input.TagSlug, actor); err != nil { - if errors.Is(err, service.ErrNotFound) { - return errorResult(fmt.Errorf("series not found")), nil, nil + for _, slug := range input.TagsToRemove { + if err := s.svc.RemoveSeriesTag(context.Background(), input.ID, slug); err != nil { + if errors.Is(err, service.ErrNotFound) { + return errorResult(fmt.Errorf("series not found")), nil, nil + } + return errorResult(err), nil, nil } - return errorResult(err), nil, nil } series, err := s.svc.GetSeries(context.Background(), input.ID) if err != nil { @@ -175,22 +157,20 @@ func (s *MCPServer) handleAddSeriesTag(ctx context.Context, _ *mcpsdk.CallToolRe return jsonResult(series) } -func (s *MCPServer) handleRemoveSeriesTag(ctx context.Context, _ *mcpsdk.CallToolRequest, input seriesTagInput) (*mcpsdk.CallToolResult, any, error) { +func (s *MCPServer) handleUnlinkSeriesTransactions(ctx context.Context, _ *mcpsdk.CallToolRequest, input unlinkSeriesInput) (*mcpsdk.CallToolResult, any, error) { if err := s.checkWritePermission(ctx); err != nil { return errorResult(err), nil, nil } - if input.ID == "" || input.TagSlug == "" { - return errorResult(fmt.Errorf("id and tag_slug are required")), nil, nil + if input.ID == "" || len(input.TransactionIDs) == 0 { + return errorResult(fmt.Errorf("id and transaction_ids are required")), nil, nil } - if err := s.svc.RemoveSeriesTag(context.Background(), input.ID, input.TagSlug); err != nil { + actor := service.ActorFromContext(ctx) + series, err := s.svc.UnlinkSeriesTransactions(context.Background(), input.ID, input.TransactionIDs, actor) + if err != nil { if errors.Is(err, service.ErrNotFound) { return errorResult(fmt.Errorf("series not found")), nil, nil } return errorResult(err), nil, nil } - series, err := s.svc.GetSeries(context.Background(), input.ID) - if err != nil { - return errorResult(err), nil, nil - } return jsonResult(series) } From 8697fc40cd4a8aadecc9f19df99ff522060c9033 Mon Sep 17 00:00:00 2001 From: Ricardo Canales Date: Tue, 23 Jun 2026 23:38:54 -0700 Subject: [PATCH 6/6] docs(prompts): scrub stale tool refs from agent + MCP prompts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Align the workflow/MCP prompts main carried in with the consolidated tool surface — these were instructing agents to call tools that no longer exist: - category-system.md: categorize_transaction / batch_categorize_transactions / reset_transaction_category / bulk_recategorize → update_transactions (reset via reset_category; bulk via query_transactions + batches, or a rule). - strategy-transaction-reviewer.md, rules-curriculum.md: batch_create_rules → create_transaction_rule (rules array). - rules-curriculum.md: breadbox://rule-dsl → get_reference(kind=rule-dsl); "one-off MCP tools" → update_transactions / assign_series / assign_counterparty. - rule-dsl.md: flag/unflag now reference update_transactions(flagged), not the removed flag_transaction tool. - report-format.md: dropped the create_session instruction (sessions are automatic; reason travels via tools/call._meta). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Y7qg4TN9ZPcHsXR714UrjG --- internal/templates/components/nav.templ | 2 +- prompts/agents/category-system.md | 8 +++----- prompts/agents/rules-curriculum.md | 14 ++++++++------ prompts/agents/strategy-transaction-reviewer.md | 5 +++-- prompts/mcp/report-format.md | 7 ++----- prompts/mcp/rule-dsl.md | 2 +- 6 files changed, 18 insertions(+), 20 deletions(-) diff --git a/internal/templates/components/nav.templ b/internal/templates/components/nav.templ index dac6bef05..eb1d68e98 100644 --- a/internal/templates/components/nav.templ +++ b/internal/templates/components/nav.templ @@ -65,7 +65,7 @@ templ Nav(p NavProps) { } if p.IsAdmin { @navLink("/rules", "zap", "Rules", navActive(p.CurrentPage, "rules")) - @navLink("/categories", "folder", "Categories", navActive(p.CurrentPage, "categories")) + @navLink("/categories", "shapes", "Categories", navActive(p.CurrentPage, "categories")) } if p.IsEditor { @navLink("/counterparties", "store", "Counterparties", navActive(p.CurrentPage, "counterparties")) diff --git a/prompts/agents/category-system.md b/prompts/agents/category-system.md index 82c382fc9..1e1441a4d 100644 --- a/prompts/agents/category-system.md +++ b/prompts/agents/category-system.md @@ -15,11 +15,9 @@ icon: shapes ### Categorization -- `update_transactions`: the preferred write for routine work — combines set-category, tag changes, and a comment into one atomic operation per transaction (max 50 per call). Use this when finishing a review: set the category AND remove the `needs-review` tag in one write. -- `categorize_transaction`: set the category on a single transaction (writes `category_id`; last-writer-wins, no provenance). Use only when you don't need the compound op. -- `batch_categorize_transactions`: override multiple transactions at once (max 500) without touching tags. -- `reset_transaction_category`: remove a manual override, let rules re-resolve the category. -- `bulk_recategorize`: move ALL transactions matching filters from one category to another. +- `update_transactions`: the **only** per-row write — combines set-category, tag changes, comment, flag, and category reset into one atomic operation per transaction (max 50 per call). It writes `category_id` directly (last-writer-wins, no provenance). Use it for everything: set the category on one transaction, or set the category AND remove the `needs-review` tag in one write when finishing a review. + - Reset a category (let rules re-resolve it): `update_transactions` with `reset_category: true` on that row. + - Recategorize many: page through them with `query_transactions`, then `update_transactions` in batches of ≤50 — or, for a durable pattern, author a `create_transaction_rule` so future syncs categorize automatically. ### Taxonomy management diff --git a/prompts/agents/rules-curriculum.md b/prompts/agents/rules-curriculum.md index 6a7967356..6e2d9d94e 100644 --- a/prompts/agents/rules-curriculum.md +++ b/prompts/agents/rules-curriculum.md @@ -18,7 +18,7 @@ series, tags, flags, and metadata are all just rule actions. Learn this one mechanism and you can automate the whole review loop. > The grammar — every field, operator, and action shape — lives in -> `breadbox://rule-dsl`. Read it whenever you author a rule. This block +> `get_reference(kind=rule-dsl)`. Read it whenever you author a rule. This block > teaches the **decision framework**: *when* to write a rule and *which > fields* to write it on. It does not repeat the grammar. @@ -52,8 +52,8 @@ Never create a rule blind. Always: transactions would match and surfaces a sample. Reject anything that over-matches, fights a category a human already set, or matches zero rows (a typo in the condition). -3. **`create_transaction_rule`** (one) or **`batch_create_rules`** (several - related rules in one call, max 100). Only create what previewed clean. +3. **`create_transaction_rule`** — pass a `rules` array (one element, or + several related rules in one call, max 100). Only create what previewed clean. ## Stable-field doctrine @@ -129,8 +129,10 @@ fear of stomping a human's correction on settled history. ## Action catalog (summary) -Every action you might put on a rule — the same verbs you can also call as -one-off MCP tools. See `breadbox://rule-dsl` for exact shapes. +Every action you might put on a rule — the same effects you can also apply +one-off via `update_transactions` (category, tags, comment, metadata, flag), +`assign_series`, and `assign_counterparty`. See `get_reference(kind=rule-dsl)` +for exact shapes. | Action | What it does | |---|---| @@ -143,7 +145,7 @@ one-off MCP tools. See `breadbox://rule-dsl` for exact shapes. | `assign_counterparty` | Bind the transaction to a counterparty — the entity on the other side (the counterparties idiom below). | A transaction joins at most one series; a higher-priority `assign_series` -overrides a lower one. Pick priority bands per `breadbox://rule-dsl` so +overrides a lower one. Pick priority bands per `get_reference(kind=rule-dsl)` so specific rules outrank broad ones. ## The counterparties idiom — name the entity behind the charge diff --git a/prompts/agents/strategy-transaction-reviewer.md b/prompts/agents/strategy-transaction-reviewer.md index 0b5550ebd..3fc2e96a9 100644 --- a/prompts/agents/strategy-transaction-reviewer.md +++ b/prompts/agents/strategy-transaction-reviewer.md @@ -50,8 +50,9 @@ For each resolved transaction, apply the curriculum's core heuristic: expected again) → promote it to a rule. Follow the 3-step author flow every time: `find_matching_rules` (don't duplicate existing coverage) → `preview_rule` (verify the match count and sample) → - `create_transaction_rule` / `batch_create_rules`. Author conditions only on - the stable raw fields the curriculum names — never on mutable display fields. + `create_transaction_rule` (pass a `rules` array to author several at once). + Author conditions only on the stable raw fields the curriculum names — never + on mutable display fields. Rules you create here apply to **future** syncs. Do not run `apply_rules` or `apply_retroactively` during a routine review — just create the rule and let diff --git a/prompts/mcp/report-format.md b/prompts/mcp/report-format.md index 509013e84..0ff04da92 100644 --- a/prompts/mcp/report-format.md +++ b/prompts/mcp/report-format.md @@ -81,9 +81,6 @@ When referencing specific transactions, always use markdown links: [Transaction This makes transactions clickable in the dashboard for quick access. SESSION MANAGEMENT: -- Before performing write operations, call create_session with a purpose describing your task. -- Include the returned session_id and a brief reason on ALL write tool calls. -- Optionally include session_id on read calls to associate them with your session. -- One session per logical task (e.g. "weekly review", "rule cleanup for dining"). -- The reason should be informal and specific (e.g. "approving clearly valid grocery charge", "creating rule for recurring uber charges"). +- Audit sessions are automatic — there is no create_session tool. Every tool call is logged under a session keyed off your transport connection, lazy-created on your first call. +- To label a specific call, pass an optional `reason` string in `tools/call._meta` (the spec's per-request metadata slot) — informal and specific (e.g. "approving clearly valid grocery charge", "creating rule for recurring uber charges"). - Sessions and their tool calls are visible on the family's dashboard for transparency. \ No newline at end of file diff --git a/prompts/mcp/rule-dsl.md b/prompts/mcp/rule-dsl.md index 2355bae92..f328c712f 100644 --- a/prompts/mcp/rule-dsl.md +++ b/prompts/mcp/rule-dsl.md @@ -127,7 +127,7 @@ Rules replace the old recurring-charge detector. Express a subscription/recurrin - `add_tag` / `remove_tag` — tag auto-created on add; slug must match `^[a-z0-9][a-z0-9\-:]*[a-z0-9]$`. Same-slug add+remove in one pass cancels (net-diff). - `add_comment` — sync-only; retroactive apply does not materialize comments. - `set_metadata` / `remove_metadata` — write/clear a key on the transaction's metadata blob. `metadata_value` is any JSON value. -- `flag` / `unflag` — set/clear the transaction's flag (mirrors the `flag_transaction` tool, sans reason). +- `flag` / `unflag` — set/clear the transaction's flag (the same flag `update_transactions` writes via `flagged: true|false`, sans reason). - `assign_series` — **surrogate-first**: provide **exactly one** of `series_short_id` (existing series) OR `series_name` + `create_if_missing: true` (resolve-or-mint a thin series by name). There is **no `merchant_key`** — the legacy key is accepted only as a back-compat alias for `series_name`. Link-and-rollup only; never steals a charge already in another series; honors sticky-reject. - `assign_counterparty` — bind matching transactions to a **counterparty** (the other side of the transaction — merchants AND non-merchants: Venmo recipients, people, employers, landlords). Provide **exactly one** of `counterparty_short_id` (an existing counterparty) OR `counterparty_name` + `create_if_missing: true`. Default is **assign-to-existing**: a counterparty is **never** auto-created unless `create_if_missing` is set. By-name is a **resolve-or-create** that de-dupes on the live name (unlike series there is no UNIQUE on name) — so **reuse an existing counterparty across providers** (look one up first) rather than minting a duplicate. Link-only (NULL-fill).