Phase 2 — Report tools (wrapping existing upstream queries)

packages/mcp-server/README.md

@@ -18,16 +18,18 @@ Phase 1 (read-only) is feature-complete. The server provides: strict startup env
 transport with `/health`, `/metrics`, the OAuth protected-resource metadata endpoint, and the MCP
 route (`POST /mcp`, JSON-RPC 2.0) with graceful shutdown; Auth0 bearer-token verification; identity
 mapping to an internal user + business-membership context with memberships resolved from the
-Accounter GraphQL server; a curated registry of nine read-only tools
-(`accounter_list_business_memberships`, `accounter_search_charges`, `accounter_get_charges`,
-`accounter_get_transactions`, `accounter_get_documents`, `accounter_list_tags`,
-`accounter_list_tax_categories`, `accounter_list_businesses`, `accounter_balance_report`) each gated
-by strict input validation, a per-tool authorization policy, and business-scope narrowing forwarded
-upstream as `x-business-scope`; a hardened upstream GraphQL client (timeout, bounded retries, header
-propagation, sanitized errors); a unified error taxonomy; per-`tools/call` rate limiting; in-process
-operational metrics (request/outcome counters, a latency histogram, auth-failure counters) exposed
-at `GET /metrics`; and OpenTelemetry tracing exported to Grafana Tempo (opt-in), correlated with the
-backend via `traceparent` and `X-Correlation-Id`.
+Accounter GraphQL server; a curated registry of fifteen read-only tools
+(`accounter_list_business_memberships`, `accounter_list_accounts`,
+`accounter_income_expense_summary`, `accounter_profit_and_loss`, `accounter_vat_report`,
+`accounter_counterparty_totals`, `accounter_search_charges`, `accounter_get_charges`,
+`accounter_get_transactions`, `accounter_get_documents`, `accounter_ledger_records`,
+`accounter_list_tags`, `accounter_list_tax_categories`, `accounter_list_businesses`,
+`accounter_balance_report`) each gated by strict input validation, a per-tool authorization policy,
+and business-scope narrowing forwarded upstream as `x-business-scope`; a hardened upstream GraphQL
+client (timeout, bounded retries, header propagation, sanitized errors); a unified error taxonomy;
+per-`tools/call` rate limiting; in-process operational metrics (request/outcome counters, a latency
+histogram, auth-failure counters) exposed at `GET /metrics`; and OpenTelemetry tracing exported to
+Grafana Tempo (opt-in), correlated with the backend via `traceparent` and `X-Correlation-Id`.
 
 Phase 2 (write scope) is **not** implemented — see
 [Known limitations & phase 2](#known-limitations--phase-2-write-scope).
@@ -57,8 +59,8 @@ a `RATE_LIMIT_ERROR` with `retryAfterMs`. Limits are configured via `MCP_RATE_LI
 Every business-scoped tool follows one convention, so the model learns it once:
 
 - **Discover, then scope.** `accounter_list_business_memberships` returns
-  `{ businessId, name, role }`. Pass those ids back as `businessIds` (or, for the balance report,
-  the singular required `businessId`).
+  `{ businessId, name, role }`. Pass those ids back as `businessIds` (or, for the single-business
+  report tools, the singular required `businessId`).
 - **`businessIds` is optional and means "narrow".** Omitting it covers every business the caller
   belongs to. Any id outside the caller's memberships is **rejected**, never silently dropped.
 - **The resolved scope is forwarded upstream** as `x-business-scope`, so RLS on the Accounter server
@@ -76,26 +78,63 @@ scope, because it _is_ the scope.
   businesses last. Pure: memberships are already on the auth context, so it makes no upstream call.
   A caller with no memberships gets an empty list, not an error. This is the scope-discovery entry
   point; to browse the full business directory use `accounter_list_businesses`.
+
+The **report tools** are registered ahead of the row-level ones on purpose: a model asked "how much
+did we make this year" should assemble and explain a report, not derive one from raw bank rows. All
+five are single-business (required `businessId`) and require the `business_owner`/`accountant` role.
+
+- **`accounter_list_accounts`** — one business's financial accounts: id, name, number, `type`
+  (`BANK_ACCOUNT`, `BANK_DEPOSIT_ACCOUNT`, `CREDIT_CARD`, `CRYPTO_WALLET`, `FOREIGN_SECURITIES`),
+  currencies, and bank/card identifiers. `type` is what makes transaction data interpretable —
+  credit-card rows are settled again by a matching bank row, deposits receive checking-account
+  sweeps, and securities rows have no mirror bank leg. **Balances are deliberately not reported:**
+  upstream stores a placeholder `0` for card, deposit and SWIFT rows in a non-null column, so a
+  reported balance would be wrong more often than right.
+- **`accounter_income_expense_summary`** — monthly income/expense totals over a date range (≤ 3660
+  days, so a decade fits one call), converted to a single `currency` using historical rates, plus
+  period totals. `cumulativeNet` is a running sum of the period's own flows from zero — **not** an
+  account balance, and it counts card rows alongside the bank rows that settle them.
+- **`accounter_profit_and_loss`** — the ledger-computed P&L for one calendar year: revenue, cost of
+  sales, gross profit, R&D/marketing/G&A, operating profit, financial expenses, other income, profit
+  before tax, tax, net profit. Optional `referenceYears` (≤ 5) for year-over-year. Line items are
+  flattened to totals; the per-sort-code breakdown is dropped.
+- **`accounter_vat_report`** — monthly VAT: output VAT on income, input VAT on expenses, and net VAT
+  due (positive) or refundable (negative), in local currency. Per-document `records` are opt-in via
+  `includeRecords`; counts are always reported.
+- **`accounter_counterparty_totals`** — per-counterparty credit/debit/net totals from the
+  double-entry ledger, ordered by absolute total, with a per-currency breakdown. This is the direct
+  answer to "who are our biggest customers/suppliers". Revaluation entries are excluded by default.
+- **`accounter_ledger_records`** — individual ledger records (date, counterparty, counter account,
+  local and foreign amounts, `chargeId`, reference) over a bounded range (≤ 1096 days). The ledger,
+  not the raw bank feed, is authoritative about what a movement was. Both ledger tools surface an
+  upstream `CommonError` as a tool error rather than an empty list, which would misread as "no
+  activity".
+
 - **`accounter_search_charges`** — read-only charges search/browse within the caller's authorized
   businesses. Optional `businessIds` (subset of memberships), `fromDate`/`toDate` (bounded to 366
   days), `tags`, `freeText`, and `flow` (`ALL`/`INCOME`/`EXPENSE`), with bounded pagination
-  (`pageSize` ≤ 50). Returns normalized charges — each carrying `ownerId`/`ownerName` — plus
-  pagination metadata and the echoed `scope`. Scoping uses the `byOwners` predicate upstream (the
-  owner), never `byBusinesses` (the counterparty).
+  (`pageSize` ≤ 50). Returns normalized charges — each carrying `ownerId`/`ownerName`, plus
+  `chargeType` and `flowKind` — pagination metadata, and the echoed `scope`. Scoping uses the
+  `byOwners` predicate upstream (the owner), never `byBusinesses` (the counterparty).
 - **`accounter_get_charges`** — read-only charge **detail** by id (1–25 `chargeIds`). Returns each
   charge with owner, counterparty, amounts (total, VAT, withholding), the full set of dates, tags,
-  and `metadata` counts, plus — by default — its linked `transactions` and `documents` nested inline
-  (toggle with `includeTransactions` / `includeDocuments`). This is the drill-down for
+  `metadata` counts, `chargeType`, and `flowKind`. Linked `transactions` and `documents` are
+  **opt-in** via `includeTransactions` / `includeDocuments` (both default `false` — nesting them by
+  default is what forced nearly every response over the byte budget). This is the drill-down for
   `accounter_search_charges`. A charge whose `owner` falls outside the resolved scope is dropped as
   defense-in-depth on top of RLS.
 - **`accounter_get_transactions`** — read-only bank/card **transactions** by id (1–50
   `transactionIds`). Each row carries direction, amount, event/effective dates, source description,
-  `isFee`, `chargeId`, counterparty, and account. Scope is enforced upstream by RLS (transactions
-  carry no owner field for a client-side filter).
+  `isFee`, `chargeId`, counterparty, and account, plus `amountLocal` and `exchangeRate` — the amount
+  converted to ILS at that transaction's own event-date rate, so historical rows are never valued at
+  today's rates. Both are `null` when no rate is on file. Scope is enforced upstream by RLS
+  (transactions carry no owner field for a client-side filter).
 - **`accounter_get_documents`** — read-only **documents** by id (1–50 `documentIds`). Each row
-  carries `documentType`, serial number, date, amount, VAT, creditor/debtor, `chargeId`, and
-  `file`/`image` links. A document whose owning charge falls outside the resolved scope is dropped
-  as defense-in-depth on top of RLS.
+  carries `documentType`, serial number, date, amount, `amountExVat`, VAT, creditor/debtor,
+  `chargeId`, `file`/`image` links, and `direction` (`issued` = the business raised it, `received` =
+  it was billed). `direction` is `null` when the owner is neither party — credit invoices and
+  documents with a missing creditor — rather than guessed. A document whose owning charge falls
+  outside the resolved scope is dropped as defense-in-depth on top of RLS.
 - **`accounter_list_tags`** — list tags for categorizing charges, optionally filtered by name and by
   `businessIds`. Rows carry `ownerId`. Deterministically sorted (name, then id) and size-capped (≤
   500).

packages/mcp-server/docs/todo.md

@@ -10,7 +10,7 @@ Last consolidated 2026-08-03, after I1/I2/I4 (#4103–#4105) merged into `main`.
 owner-scoping work (#4089–#4097) is also in.
 
 **Status of the connector today:** working end-to-end against Claude Desktop with a pre-registered
-Auth0 client, read-only, five curated tools, business scope enforced by RLS upstream. Not
+Auth0 client, read-only, fifteen curated tools, business scope enforced by RLS upstream. Not
 publishable — see B1.
 
 ---
@@ -56,6 +56,39 @@ D3 (`MCP_TOOL_ALLOWLIST`) and D6 (rate-limit keying) are settled — see the clo
 
 Ranked by when they start to hurt, not by size.
 
+### I6. Server-side gaps behind the connector-UX work — **high**
+
+Raised by the agent-session feedback (`accounter_mcp_feedback.md`) and scoped out of the MCP-only
+Phases 1–2 (`docs/mcp-extension/plan.md`). These need `packages/server` / `packages/migrations`
+changes.
+
+- [ ] **`transactions.current_balance` is a placeholder for most sources — the blocker for
+      `balanceAfter`.** `resolvers/common.ts:57` passes the column straight through, but only the
+      Poalim ILS/foreign and Discount triggers write a real value; the SWIFT, deposit, and all four
+      credit-card triggers insert a literal `0`. Since `Transaction.balance` is non-null, a
+      placeholder is indistinguishable from a genuine zero balance, and `Transaction` exposes no
+      `sourceOrigin` for the MCP layer to filter on — so per-transaction balances and
+      `accounter_list_accounts`' `includeCurrentBalance` both stay unshipped. Fix: write `NULL`
+      instead of `0`, make the GraphQL field nullable, return `null` from the resolver, and backfill
+      keyed off `source_origin` / account type (**not** off the value — a real `0` is legitimate).
+      Longer term, capture the balance the card and deposit sources do expose.
+- [ ] **No sorting or pagination on `transactionsByFilters`.** `TransactionsFilters` has no `sortBy`
+      and the query returns an unbounded `[Transaction!]!`, which is why the feedback session
+      hand-rolled cursor pagination over ~100 calls. Wants `sortBy` + `limit`/`offset` (or a cursor)
+      and, ideally, a bulk export for the "fetch once, work locally" pattern.
+- [ ] **`Charge.totalAmount` reported `null` on every income charge**, which is why revenue had to
+      be computed from documents. Verify against `packages/server/src/modules/charges`; compute
+      server-side if it is derivable.
+- [ ] **`myMemberships.businessName` is nullable** and came back `null`, so the agent could not tell
+      which business was which without fetching a charge. Fix the resolver's join, or fall back
+      through `businesses(ids:)` in `src/upstream/memberships.ts`.
+- [ ] **Not yet exposed, and asked for:** document → payment status (open/partially paid/paid with
+      matched transaction ids) for invoiced-vs-collected, securities positions (holdings, cost
+      basis, market value, realized/unrealized P&L), and a payroll breakdown. `allDeposits` already
+      exists — a thin wrapper is cheap if deposits become a priority.
+
+Source: `accounter_mcp_feedback.md` §§1–3, 6–7.
+
 ### I5. Stable tunnel for local development — **low (friction)**
 
 Production is settled (see the closed list — `https://mcp.accounter.tax`). Local development still

packages/mcp-server/src/tools/__tests__/charges.test.ts

@@ -70,13 +70,16 @@ describe('searchChargesTool — successful read', () => {
       totalCount: number;
       truncated: boolean;
     };
-    // This fixture omits `owner` on purpose — it predates the field. Owner
-    // tagging must degrade to nulls rather than throwing, so older fixtures and
-    // any upstream that stops returning the field keep working.
+    // This fixture omits `owner` and `__typename` on purpose — it predates both
+    // fields. Owner tagging and charge classification must degrade to
+    // nulls/`unknown` rather than throwing, so older fixtures and any upstream
+    // that stops returning a field keep working.
     expect(structured.charges).toEqual([
       {
         id: 'c1',
         description: 'Coffee',
+        chargeType: null,
+        flowKind: 'unknown',
         ownerId: null,
         ownerName: null,
         amount: { value: 12.5, formatted: '₪12.50', currency: 'ILS' },
@@ -88,6 +91,30 @@ describe('searchChargesTool — successful read', () => {
     expect(structured.pagination.hasNextPage).toBe(false);
   });
 
+  it('tags each charge with its type and flow kind', async () => {
+    const client = clientReturning({
+      allCharges: {
+        nodes: [
+          {
+            __typename: 'InternalTransferCharge',
+            id: 'c1',
+            userDescription: 'Sweep to deposit',
+            owner: { id: 'b1', name: 'Acme' },
+            totalAmount: { raw: -50_000, formatted: '₪-50,000.00', currency: 'ILS' },
+            minEventDate: '2026-01-05',
+          },
+        ],
+        pageInfo: { totalPages: 1, totalRecords: 1, currentPage: 1, pageSize: 25 },
+      },
+    });
+    const result = await run(client, authContext(['b1']), {});
+    const { charges } = result.structuredContent as {
+      charges: Array<{ chargeType: string; flowKind: string }>;
+    };
+    // Negative amount, but it is not an expense — the money stayed in-house.
+    expect(charges[0]).toMatchObject({ chargeType: 'INTERNAL', flowKind: 'internal_transfer' });
+  });
+
   it('tags each charge with its owning business', async () => {
     const client = clientReturning({
       allCharges: {

packages/mcp-server/src/tools/__tests__/detail-tools.test.ts

@@ -198,7 +198,7 @@ describe('getChargesTool', () => {
     const client = clientReturning(chargeFixture, body => (sentBody = body));
     await run(getChargesTool, client, authContext([B1]), {
       chargeIds: ['c1', 'c2'],
-      includeDocuments: false,
+      includeTransactions: true,
     });
     const variables = (
       sentBody as {
@@ -210,6 +210,19 @@ describe('getChargesTool', () => {
     expect(variables.includeDocuments).toBe(false);
   });
 
+  // Nesting transactions/documents by default is what forced nearly every call
+  // to spill over the payload budget, so both are opt-in.
+  it('omits nested transactions and documents unless asked', async () => {
+    let sentBody: unknown;
+    const client = clientReturning(chargeFixture, body => (sentBody = body));
+    await run(getChargesTool, client, authContext([B1]), { chargeIds: ['c1'] });
+    const variables = (
+      sentBody as { variables: { includeTransactions: boolean; includeDocuments: boolean } }
+    ).variables;
+    expect(variables.includeTransactions).toBe(false);
+    expect(variables.includeDocuments).toBe(false);
+  });
+
   it('forwards all available filters to allCharges', async () => {
     let sentBody: unknown;
     const client = clientReturning(filteredChargeFixture, body => (sentBody = body));
@@ -312,6 +325,32 @@ describe('getChargesTool', () => {
     expect(structured.charges[0]?.id).toBe('c1');
   });
 
+  it('classifies a charge from its typename', async () => {
+    const client = clientReturning({
+      chargesByIDs: [
+        { ...chargeFixture.chargesByIDs[0], __typename: 'CreditcardBankCharge' },
+        {
+          ...chargeFixture.chargesByIDs[0],
+          id: 'c2',
+          __typename: 'CommonCharge',
+          totalAmount: { raw: 5000, formatted: '₪5,000.00', currency: 'ILS' },
+        },
+      ],
+    });
+    const result = await run(getChargesTool, client, authContext([B1]), {
+      chargeIds: ['c1', 'c2'],
+    });
+    const { charges } = result.structuredContent as {
+      charges: Array<{ chargeType: string | null; flowKind: string }>;
+    };
+    // A card settlement moves money between the owner's own accounts.
+    expect(charges[0]).toMatchObject({
+      chargeType: 'CREDITCARD_BANK',
+      flowKind: 'internal_transfer',
+    });
+    expect(charges[1]).toMatchObject({ chargeType: 'COMMON', flowKind: 'income' });
+  });
+
   it('drops a charge whose owner is outside the resolved scope (defense-in-depth)', async () => {
     const client = clientReturning(chargeFixture);
     // Caller is authorized for B2 only; the fixture charge is owned by B1.
@@ -420,6 +459,31 @@ describe('getTransactionsTool', () => {
     expect(variables.transactionIDs).toEqual(['tx1', 'tx2']);
   });
 
+  it('converts a foreign-currency row using its own event-date rate', async () => {
+    const client = clientReturning({
+      transactionsByIDs: [
+        {
+          ...fixture.transactionsByIDs[0],
+          amount: { raw: -100, formatted: '$-100.00', currency: 'USD' },
+          eventExchangeRates: { date: '2026-01-05', ils: 1, usd: 3.5 },
+        },
+      ],
+    });
+    const result = await run(getTransactionsTool, client, authContext([B1]), {
+      transactionIds: ['tx1'],
+    });
+    const [transaction] = (
+      result.structuredContent as {
+        transactions: Array<{
+          amountLocal: { value: number; currency: string } | null;
+          exchangeRate: number | null;
+        }>;
+      }
+    ).transactions;
+    expect(transaction!.amountLocal).toEqual({ value: -350, currency: 'ILS' });
+    expect(transaction!.exchangeRate).toBe(3.5);
+  });
+
   it('reports no matches for an empty upstream result', async () => {
     const client = clientReturning({ transactionsByIDs: [] });
     const result = await run(getTransactionsTool, client, authContext([B1]), {
@@ -605,6 +669,19 @@ describe('getDocumentsTool', () => {
     expect(structured.documents[0]!.vat).toEqual({ value: -17, formatted: '₪-17.00', currency: 'ILS' });
   });
 
+  it('exposes direction and amountExVat on each row', async () => {
+    const client = clientReturning({ documentsByIds: [doc()] });
+    const result = await run(getDocumentsTool, client, authContext([B1]), { documentIds: ['d1'] });
+    const [document] = (
+      result.structuredContent as {
+        documents: Array<{ direction: string | null; amountExVat: { value: number } | null }>;
+      }
+    ).documents;
+    // B1 is the debtor on this fixture, so the business was billed.
+    expect(document!.direction).toBe('received');
+    expect(document!.amountExVat?.value).toBe(-103);
+  });
+
   it('drops a document whose owning charge is outside scope', async () => {
     const client = clientReturning({
       documentsByIds: [doc({ charge: { id: 'c9', owner: { id: B2 } } })],