fix(mcp): query_journal accepts scalar/number accounts instead of silently dropping the filter (#1803)

* fix(mcp): query_journal accepts scalar/number accounts instead of silently dropping the filter

Hosts don't always enforce inputSchema. `accounts: 1630` (JSON number) has no
`.length`, so the account filter was skipped while applied_filters still
echoed it, returning every account's lines as if filtered. A bare string
"1630" was spread into its digits by postgrest-js `.in()` and matched nothing.

Normalize accounts (array / bare string / comma list / integer) to string[]
and account_from/account_to to strings; reject values that aren't account
numbers with a clear error instead of ignoring them. Tests assert the actual
`.in('account_number', [...])` predicate, not just the echo.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(mcp): reuse shared ACCOUNT_NUMBER_RE, drop null entries, normalize tag_journal_lines filters

Review fixes: the local ACCOUNT_NUMBER_RE collided with the import from
lib/invariants/account-number (build break); null/undefined array entries
now drop out instead of reaching .in(); gnubok_tag_journal_lines (a bulk
write path) gets the same accounts/account_from/account_to normalization so
a mangled filter fails fast instead of widening the retag scope.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

extensions/general/mcp-server/__tests__/query-journal.test.ts

@@ -769,3 +769,98 @@ describe('gnubok_query_journal: free-text search', () => {
     ).rejects.not.toThrow(/private_internal/)
   })
 })
+
+/**
+ * Hosts don't always enforce inputSchema, so `accounts` arrives as a bare
+ * number or string in the wild. Before the normalizer, `accounts: 1630`
+ * silently dropped the filter (a number has no `.length`) while
+ * applied_filters still echoed it, and `accounts: "1630"` was spread into its
+ * digits by `.in()`. Record every `.in()` call on journal_entry_lines so the
+ * assertion is on the actual predicate, not just the echo.
+ */
+function makeRecordingEntryLinesMock() {
+  const inCalls: Array<{ table: string; column: string; values: unknown }> = []
+  // One parent entry so the two-step fetch actually issues the lines query
+  // (fetchEntryLines short-circuits on zero entries); the lines page is empty.
+  const rowsFor = (table: string) =>
+    table === 'journal_entries'
+      ? [{ id: 'je-1', entry_date: '2026-01-15', voucher_series: 'A', voucher_number: 1, status: 'posted' }]
+      : []
+  const chain = (table: string): unknown =>
+    new Proxy(
+      {},
+      {
+        get(_t, prop) {
+          if (prop === 'then') {
+            return (resolve: (v: unknown) => void) =>
+              resolve({ data: rowsFor(table), error: null, count: rowsFor(table).length })
+          }
+          if (prop === 'range') return () => ({ data: rowsFor(table), error: null, count: rowsFor(table).length })
+          if (prop === 'in') {
+            return (column: string, values: unknown) => {
+              inCalls.push({ table, column, values })
+              return chain(table)
+            }
+          }
+          return () => chain(table)
+        },
+      },
+    )
+  const supabase = { from: vi.fn().mockImplementation((table: string) => chain(table)) } as never
+  return { supabase, inCalls }
+}
+
+describe('gnubok_query_journal: accounts argument normalization', () => {
+  const tool = () => tools.find((t) => t.name === 'gnubok_query_journal')!
+  const lineAccountFilters = (calls: Array<{ table: string; column: string; values: unknown }>) =>
+    calls.filter((c) => c.table === 'journal_entry_lines' && c.column === 'account_number')
+
+  it('applies a bare number `accounts: 1630` as ["1630"] and echoes the normalized value', async () => {
+    const { supabase, inCalls } = makeRecordingEntryLinesMock()
+    const result = (await tool().execute(
+      { accounts: 1630 },
+      'company-1', 'user-1', supabase,
+    )) as { applied_filters: { accounts: unknown } }
+
+    const filters = lineAccountFilters(inCalls)
+    expect(filters.length).toBeGreaterThan(0)
+    for (const f of filters) expect(f.values).toEqual(['1630'])
+    expect(result.applied_filters.accounts).toEqual(['1630'])
+  })
+
+  it('applies a bare string `accounts: "1630"` as ["1630"], not as its digits', async () => {
+    const { supabase, inCalls } = makeRecordingEntryLinesMock()
+    await tool().execute({ accounts: '1630' }, 'company-1', 'user-1', supabase)
+    const filters = lineAccountFilters(inCalls)
+    expect(filters.length).toBeGreaterThan(0)
+    for (const f of filters) expect(f.values).toEqual(['1630'])
+  })
+
+  it('accepts a comma-separated string and integer array items', async () => {
+    const { supabase, inCalls } = makeRecordingEntryLinesMock()
+    await tool().execute({ accounts: '1930, 1940' }, 'company-1', 'user-1', supabase)
+    expect(lineAccountFilters(inCalls)[0]?.values).toEqual(['1930', '1940'])
+
+    const second = makeRecordingEntryLinesMock()
+    await tool().execute({ accounts: [1930, '1940'] }, 'company-1', 'user-1', second.supabase)
+    expect(lineAccountFilters(second.inCalls)[0]?.values).toEqual(['1930', '1940'])
+  })
+
+  it('rejects non-account values instead of dropping the filter', async () => {
+    const { supabase } = makeRecordingEntryLinesMock()
+    await expect(
+      tool().execute({ accounts: 'kontorsmaterial' }, 'company-1', 'user-1', supabase),
+    ).rejects.toThrow(/accounts must be an account number/)
+    await expect(
+      tool().execute({ account_from: 4000 as unknown as string, account_to: { x: 1 } }, 'company-1', 'user-1', supabase),
+    ).rejects.toThrow(/account_to must be an account number/)
+  })
+
+  it('still caps accounts at 50 after normalization', async () => {
+    const { supabase } = makeRecordingEntryLinesMock()
+    const accounts = Array.from({ length: 51 }, (_, i) => 1000 + i).join(',')
+    await expect(
+      tool().execute({ accounts }, 'company-1', 'user-1', supabase),
+    ).rejects.toThrow(/capped at 50/)
+  })
+})

extensions/general/mcp-server/__tests__/tag-journal-lines.test.ts

@@ -132,6 +132,43 @@ describe('gnubok_tag_journal_lines: filter gates', () => {
     ).rejects.toThrow(/Inga bokförda rader matchade filtret[\s\S]*gnubok_query_journal/)
   })
 
+  it('rejects non-account filter values instead of silently widening the retag scope', async () => {
+    // Hosts don't always enforce inputSchema. A mangled account filter on a
+    // bulk WRITE path must fail fast, never fall through to "no account
+    // filter" and retag more lines than asked.
+    const { supabase } = createQueuedMockSupabase()
+    await expect(
+      tagJournalLines.execute(
+        { dimensions: { '6': 'P01' }, reason: 'Retro-taggning', filters: { accounts: 'kontorsmaterial' } },
+        'company-1',
+        'user-1',
+        supabase as never,
+      ),
+    ).rejects.toThrow(/accounts must be an account number/)
+    await expect(
+      tagJournalLines.execute(
+        { dimensions: { '6': 'P01' }, reason: 'Retro-taggning', filters: { account_from: { x: 1 } } },
+        'company-1',
+        'user-1',
+        supabase as never,
+      ),
+    ).rejects.toThrow(/filters\.account_from must be an account number/)
+  })
+
+  it('accepts a bare number for filters.accounts and applies it as a filter', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: { dimensions_enabled: false }, error: null })
+    enqueue({ data: [], error: null }) // entry match query runs, so normalization passed
+    await expect(
+      tagJournalLines.execute(
+        { dimensions: { '6': 'P01' }, reason: 'Retro-taggning', filters: { accounts: 4010 } },
+        'company-1',
+        'user-1',
+        supabase as never,
+      ),
+    ).rejects.toThrow(/Inga bokförda rader matchade filtret/)
+  })
+
   it('throws asking to narrow the filter when more than 500 lines match', async () => {
     const { supabase, enqueue } = createQueuedMockSupabase()
     enqueue({ data: { dimensions_enabled: false }, error: null })

extensions/general/mcp-server/server.ts

@@ -590,6 +590,46 @@ const AUTO_PERIOD_DATE_KEYS = [
 
 const ISO_DATE_RE = /^\d{4}-\d{2}-\d{2}$/
 
+/**
+ * Coerce a single account-number argument to a trimmed string (validated
+ * against ACCOUNT_NUMBER_RE from lib/invariants/account-number). Accepts a
+ * string or a finite integer (hosts that skip inputSchema validation send
+ * `1930` as a JSON number); everything else is a validation error rather than
+ * a silently dropped filter.
+ */
+function normalizeAccountNumber(value: unknown, field: string): string | undefined {
+  if (value === undefined || value === null || value === '') return undefined
+  const str =
+    typeof value === 'number' && Number.isInteger(value)
+      ? String(value)
+      : typeof value === 'string'
+        ? value.trim()
+        : null
+  if (str === null || !ACCOUNT_NUMBER_RE.test(str)) {
+    throw new Error(`${field} must be an account number string like "1930" (got ${JSON.stringify(value)})`)
+  }
+  return str
+}
+
+/**
+ * Coerce an `accounts` argument to string[]. Accepts an array of
+ * strings/integers, a bare string ("1930" or comma-separated "1930,1940"), or
+ * a bare integer. Returns undefined for null/empty so callers fall through to
+ * account_from/account_to.
+ */
+function normalizeAccountList(value: unknown): string[] | undefined {
+  if (value === undefined || value === null) return undefined
+  const raw: unknown[] = Array.isArray(value)
+    ? value
+    : typeof value === 'string'
+      ? value.split(',')
+      : [value]
+  const list = raw
+    .filter((v) => v != null && !(typeof v === 'string' && v.trim() === ''))
+    .map((v) => normalizeAccountNumber(v, 'accounts') as string)
+  return list.length > 0 ? list : undefined
+}
+
 function autoExtractDateForPeriodCheck(params: Record<string, unknown>): string | undefined {
   for (const key of AUTO_PERIOD_DATE_KEYS) {
     const value = params[key]
@@ -7291,12 +7331,16 @@ export const tools: McpTool[] = [
 
       // ── Filters: validated before any DB work so bad input fails fast.
       const filters = (args.filters && typeof args.filters === 'object' ? args.filters : {}) as Record<string, unknown>
-      const accounts = Array.isArray(filters.accounts) ? (filters.accounts as string[]) : undefined
+      // Same normalization as gnubok_query_journal: a bare number or string
+      // must narrow the match set, never be silently dropped. This is a bulk
+      // WRITE path (retags up to 500 posted lines), so a dropped account
+      // filter would widen the scope of the retag.
+      const accounts = normalizeAccountList(filters.accounts)
       if (accounts && accounts.length > 50) {
         throw new Error('filters.accounts is capped at 50: use account_from/account_to for ranges')
       }
-      const accountFrom = typeof filters.account_from === 'string' ? filters.account_from : undefined
-      const accountTo = typeof filters.account_to === 'string' ? filters.account_to : undefined
+      const accountFrom = normalizeAccountNumber(filters.account_from, 'filters.account_from')
+      const accountTo = normalizeAccountNumber(filters.account_to, 'filters.account_to')
       const dateFrom = typeof filters.date_from === 'string' ? filters.date_from : undefined
       const dateTo = typeof filters.date_to === 'string' ? filters.date_to : undefined
       const text = typeof filters.text === 'string' ? filters.text.trim() : ''
@@ -7856,9 +7900,15 @@ export const tools: McpTool[] = [
     async execute(args, companyId, userId, supabase) {
       const limit = Math.min(Math.max(1, Number(args.limit) || 100), 500)
       const status = (args.status as string) || 'posted'
-      const accounts = args.accounts as string[] | undefined
-      const accountFrom = args.account_from as string | undefined
-      const accountTo = args.account_to as string | undefined
+      // Hosts don't always enforce inputSchema: `accounts: 1630` (a bare
+      // number) or `accounts: "1630"` both reached us as-is. A number has no
+      // `.length`, so the filter was silently skipped while applied_filters
+      // still echoed it; a bare string was spread into its digits by
+      // postgrest-js `.in()` and matched nothing. Normalize every shape to a
+      // string[] and reject anything that isn't an account number.
+      const accounts = normalizeAccountList(args.accounts)
+      const accountFrom = normalizeAccountNumber(args.account_from, 'account_from')
+      const accountTo = normalizeAccountNumber(args.account_to, 'account_to')
 
       if (accounts && accounts.length > 50) {
         throw new Error('accounts list capped at 50: use account_from/account_to for ranges')