diff --git a/README.md b/README.md index f4f030d..2f5d9f5 100644 --- a/README.md +++ b/README.md @@ -300,7 +300,7 @@ The metrics server only starts in HTTP mode (not stdio) and binds to `0.0.0.0` - **`get_row_activities`** — Get change history of a row (who changed what, when, old/new values) - **`create_snapshot`** — Create a snapshot of the current base (10 min cooldown) -- **`add_select_options`** — Add new options to single-select or multi-select columns +- **`add_select_options`** — Add new options to single-select or multi-select columns (existing options are skipped, no duplicates) - **`ping_seatable`** — Health check with latency monitoring ## Supported Column Types diff --git a/package.json b/package.json index 513d3c5..aa3fa84 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@seatable/mcp-seatable", - "version": "1.6.1", + "version": "1.6.2", "type": "module", "license": "MIT", "mcpName": "io.github.seatable/seatable", diff --git a/src/mcp/tools/addSelectOption.ts b/src/mcp/tools/addSelectOption.ts index 14a46f2..68d0a29 100644 --- a/src/mcp/tools/addSelectOption.ts +++ b/src/mcp/tools/addSelectOption.ts @@ -1,5 +1,6 @@ import { z } from 'zod' +import { makeError } from '../../errors.js' import { ToolRegistrar } from './types.js' const InputSchema = z.object({ @@ -12,25 +13,67 @@ const InputSchema = z.object({ })).min(1).describe('Array of options to add'), }) +/** Names of the options a select column already has. Empty if the column has none yet. */ +function existingOptionNames(metadata: any, table: string, column: string): Set { + const tableObj = (metadata?.tables ?? []).find((t: any) => t.name === table) + if (!tableObj) { + throw makeError('ERR_SCHEMA_UNKNOWN_TABLE', `Table "${table}" not found`, { table }) + } + const columnObj = (tableObj.columns ?? []).find((c: any) => c.name === column) + if (!columnObj) { + throw makeError('ERR_SCHEMA_UNKNOWN_COLUMN', `Column "${column}" not found in table "${table}"`, { + table, + column, + }) + } + const opts = columnObj.data?.options + if (!Array.isArray(opts)) return new Set() + return new Set( + opts.map((o: any) => o?.name).filter((n: unknown): n is string => typeof n === 'string') + ) +} + export const registerAddSelectOptions: ToolRegistrar = (server, { client, getInputSchema }) => { server.registerTool( 'add_select_options', { title: 'Add Select Options', - description: 'Add new options to a single-select or multi-select column. Use this before writing rows with option values that do not exist yet.', + description: 'Add new options to a single-select or multi-select column. Use this before writing rows with option values that do not exist yet. Options that already exist are skipped, so this will not create duplicates.', inputSchema: getInputSchema(InputSchema), annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: true, openWorldHint: false }, }, async (args: unknown) => { const parsed = InputSchema.parse(args) + // Skip options the column already has — the SeaTable API happily creates + // a second option with the same name, which is never what the caller wants. + const existing = existingOptionNames(await client.getMetadata(), parsed.table, parsed.column) + const skipped: string[] = [] + const seen = new Set() + const toAdd: typeof parsed.options = [] + for (const opt of parsed.options) { + if (existing.has(opt.name)) { + skipped.push(opt.name) + } else if (!seen.has(opt.name)) { + seen.add(opt.name) + toAdd.push(opt) + } + } + + const added = toAdd.map((o) => o.name) + if (toAdd.length === 0) { + return { + content: [{ type: 'text', text: JSON.stringify({ added, skipped, success: true }) }], + } + } + const result = await client.addColumnOptions({ table: parsed.table, column: parsed.column, - options: parsed.options, + options: toAdd, }) - return { content: [{ type: 'text', text: JSON.stringify(result) }] } + return { content: [{ type: 'text', text: JSON.stringify({ added, skipped, ...result }) }] } } ) } diff --git a/src/schema/validate.ts b/src/schema/validate.ts index 0eebb71..2a58d36 100644 --- a/src/schema/validate.ts +++ b/src/schema/validate.ts @@ -132,9 +132,14 @@ function validateDate(colName: string, value: unknown): void { } function getSelectOptions(col: GenericColumn): Set { - const opts = col.options?.options as Array<{ name: string }> | undefined + // Options arrive in two shapes: the raw SeaTable form ([{ name, id, color }]) + // and the flattened form produced by mapMetadataToGeneric (["open", "closed"]). + const opts = col.options?.options as Array<{ name?: unknown } | string> | undefined if (!Array.isArray(opts)) return new Set() - return new Set(opts.map((o) => o.name)) + const names = opts + .map((o) => (typeof o === 'string' ? o : (o as { name?: unknown })?.name)) + .filter((n): n is string => typeof n === 'string' && n !== '') + return new Set(names) } function validateSingleSelect(colName: string, value: unknown, col: GenericColumn): void { diff --git a/tests/addSelectOptions.spec.ts b/tests/addSelectOptions.spec.ts new file mode 100644 index 0000000..2ee39f9 --- /dev/null +++ b/tests/addSelectOptions.spec.ts @@ -0,0 +1,131 @@ +import { describe, expect, it } from 'vitest' + +import { registerAddSelectOptions } from '../src/mcp/tools/addSelectOption.js' +import type { ClientLike } from '../src/mcp/tools/types.js' + +type Handler = (args: unknown) => Promise + +/** Metadata with one single-select column that already has two options. */ +function metadataWithOptions(names: string[]) { + return { + tables: [ + { + _id: 'tbl1', + name: 'Tasks', + columns: [ + { key: 'col1', name: 'Title', type: 'text' }, + { + key: 'col2', + name: 'Statut', + type: 'single-select', + data: { options: names.map((n, i) => ({ id: `o${i}`, name: n, color: '#fff' })) }, + }, + ], + }, + ], + } +} + +/** Wire the registrar to a fake client and return the handler plus recorded calls. */ +function setup(existing: string[]) { + const calls: Array<{ table: string; column: string; options: Array<{ name: string }> }> = [] + const client = { + getMetadata: async () => metadataWithOptions(existing), + addColumnOptions: async (args: any) => { + calls.push(args) + return { success: true } + }, + } as unknown as ClientLike + + let handler: Handler | undefined + const server = { + registerTool: (_name: string, _cfg: unknown, h: Handler) => { + handler = h + }, + } + + registerAddSelectOptions(server as any, { + client, + env: {} as any, + getInputSchema: (s: any) => s, + }) + + return { handler: handler as Handler, calls } +} + +function parse(result: any): any { + return JSON.parse(result.content[0].text) +} + +describe('add_select_options', () => { + it('adds options that do not exist yet', async () => { + const { handler, calls } = setup(['En cours']) + const result = await handler({ + table: 'Tasks', + column: 'Statut', + options: [{ name: 'Reconnu' }], + }) + + expect(calls).toHaveLength(1) + expect(calls[0].options.map((o) => o.name)).toEqual(['Reconnu']) + expect(parse(result).added).toEqual(['Reconnu']) + }) + + it('skips options that already exist instead of duplicating them', async () => { + const { handler, calls } = setup(['En cours', 'Reconnu']) + const result = await handler({ + table: 'Tasks', + column: 'Statut', + options: [{ name: 'En cours' }, { name: 'Non reconnu' }], + }) + + expect(calls).toHaveLength(1) + expect(calls[0].options.map((o) => o.name)).toEqual(['Non reconnu']) + const data = parse(result) + expect(data.added).toEqual(['Non reconnu']) + expect(data.skipped).toEqual(['En cours']) + }) + + it('makes no API call when every option already exists', async () => { + const { handler, calls } = setup(['En cours', 'Reconnu']) + const result = await handler({ + table: 'Tasks', + column: 'Statut', + options: [{ name: 'En cours' }, { name: 'Reconnu' }], + }) + + expect(calls).toHaveLength(0) + const data = parse(result) + expect(data.added).toEqual([]) + expect(data.skipped).toEqual(['En cours', 'Reconnu']) + }) + + it('collapses duplicates within a single request', async () => { + const { handler, calls } = setup([]) + await handler({ + table: 'Tasks', + column: 'Statut', + options: [{ name: 'Nouveau' }, { name: 'Nouveau' }], + }) + + expect(calls[0].options.map((o) => o.name)).toEqual(['Nouveau']) + }) + + it('treats option names as case-sensitive, like SeaTable does', async () => { + const { handler, calls } = setup(['En cours']) + await handler({ + table: 'Tasks', + column: 'Statut', + options: [{ name: 'EN COURS' }], + }) + + expect(calls[0].options.map((o) => o.name)).toEqual(['EN COURS']) + }) + + it('fails clearly when the column does not exist', async () => { + const { handler } = setup(['En cours']) + await expect( + handler({ table: 'Tasks', column: 'Nope', options: [{ name: 'x' }] }) + ).rejects.toThrow(/Nope/) + }) +}) diff --git a/tests/validate.spec.ts b/tests/validate.spec.ts index 94083ed..5689d85 100644 --- a/tests/validate.spec.ts +++ b/tests/validate.spec.ts @@ -311,3 +311,41 @@ describe('validateRowsAgainstSchema', () => { }) }) }) + +// Regression: the shape produced by mapMetadataToGeneric must validate correctly. +// mapMetadataToGeneric flattens select options to a string array ({ options: ['open'] }), +// while the fixtures above use the raw SeaTable object form ({ options: [{ name: 'open' }] }). +// Both must be accepted — see ticket "unknown option ... Valid options: ". +describe('select validation against mapped schema shape', () => { + const mappedSchema: GenericSchema = { + base_id: 'base1', + tables: [ + { + id: 'tbl1', + name: 'Tasks', + columns: [ + { id: 'col1', name: 'Title', type: 'text' }, + { id: 'col2', name: 'Statut', type: 'single_select', options: { options: ['En cours', 'Reconnu'] } }, + { id: 'col3', name: 'Tags', type: 'multi_select', options: { options: ['urgent', 'low'] } }, + ], + }, + ], + } + + it('accepts a valid single-select option', () => { + const rows = [{ Title: 'A', Statut: 'En cours' }] + validateRowsAgainstSchema(mappedSchema, 'Tasks', rows) + }) + + it('accepts valid multi-select options', () => { + const rows = [{ Title: 'A', Tags: ['urgent'] }] + validateRowsAgainstSchema(mappedSchema, 'Tasks', rows) + }) + + it('still rejects an unknown option and lists the valid ones', () => { + const rows = [{ Title: 'A', Statut: 'Nope' }] + expect(() => validateRowsAgainstSchema(mappedSchema, 'Tasks', rows)).toThrowError( + 'unknown option "Nope". Valid options: En cours, Reconnu' + ) + }) +})