feat(mcp): use native destroyMultipleColumns batch endpoint in deleteFields

ARHAEEM · claude · ARHAEEM · commit d1737a753fd4 · 2026-05-21T01:32:36.000+03:00
Replaces the per-field loop (N×3 browser evals) with 1-2 batch API calls
per table group via POST /v0.3/table/{tableId}/destroyMultipleColumns.
Safety checks (expectedName) still validated before any destructive call.
Fields from different tables are batched independently.

Co-Authored-By: Claude Sonnet 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/packages/mcp-server/src/client.js b/packages/mcp-server/src/client.js
@@ -692,9 +692,9 @@ export class AirtableClient {
   }
 
   /**
-   * Delete multiple fields sequentially. Returns { succeeded, failed }.
-   * Each failed entry includes the fieldId and error message but does NOT
-   * abort the rest of the batch — all fields are attempted.
+   * Delete multiple fields using the native batch endpoint (destroyMultipleColumns).
+   * Fields are grouped by tableId and each table's fields are deleted in 1-2 API
+   * calls instead of N×3. Returns { succeeded, failed }.
    *
    * @param {string} appId
    * @param {{fieldId: string, expectedName: string}[]} fields
@@ -704,21 +704,82 @@ export class AirtableClient {
     const succeeded = [];
     const failed = [];
 
-    for (let i = 0; i < fields.length; i++) {
-      const { fieldId, expectedName } = fields[i];
+    // Step 1: Validate expectedName safety checks and group by tableId.
+    // getApplicationData is cached, so N resolveField calls cost one network round-trip.
+    const byTable = new Map(); // tableId → [{ fieldId, expectedName }]
+
+    for (const { fieldId, expectedName } of fields) {
       try {
-        const result = await this.deleteField(appId, fieldId, expectedName, { force });
-        if (!result.deleted) {
-          failed.push({ fieldId, name: expectedName, error: result.message ?? 'Field was not deleted' });
+        const { field, table } = await this.resolveField(appId, fieldId);
+        if (field.name !== expectedName) {
+          failed.push({
+            fieldId,
+            name: expectedName,
+            error: `Safety check failed: field is named "${field.name}" but expectedName was "${expectedName}"`,
+          });
         } else {
-          succeeded.push({ fieldId, name: expectedName, deleted: true, forced: result.forced ?? false });
+          if (!byTable.has(table.id)) byTable.set(table.id, []);
+          byTable.get(table.id).push({ fieldId, expectedName });
         }
-      } catch (error) {
-        failed.push({ fieldId, name: expectedName, error: error.message });
+      } catch (err) {
+        failed.push({ fieldId, name: expectedName, error: err.message });
       }
-      onProgress?.({ index: i, total: fields.length, succeeded: succeeded.length, failed: failed.length });
     }
 
+    // Step 2: Batch-delete each table's fields via destroyMultipleColumns (1-2 calls per table).
+    for (const [tableId, tableFields] of byTable) {
+      const columnIds = tableFields.map(f => f.fieldId);
+
+      const checkRes = await this.auth.postForm(
+        `https://airtable.com/v0.3/table/${tableId}/destroyMultipleColumns`,
+        this._mutationParams({ columnIds, checkSchemaDependencies: true }, appId),
+        appId,
+      );
+
+      if (checkRes.ok) {
+        this.cache.invalidate(appId);
+        for (const f of tableFields) {
+          succeeded.push({ fieldId: f.fieldId, name: f.expectedName, deleted: true, forced: false });
+        }
+        continue;
+      }
+
+      const checkBody = await checkRes.json().catch(() => null);
+      const isDependencyError = checkBody?.error?.type === 'SCHEMA_DEPENDENCIES_VALIDATION_FAILED';
+
+      if (!isDependencyError || !force) {
+        const errMsg = isDependencyError
+          ? 'Fields have schema dependencies and force=false'
+          : `destroyMultipleColumns failed (${checkRes.status})`;
+        for (const f of tableFields) {
+          failed.push({ fieldId: f.fieldId, name: f.expectedName, error: errMsg });
+        }
+        continue;
+      }
+
+      // Deps exist and force=true: second call without checkSchemaDependencies
+      const forceRes = await this.auth.postForm(
+        `https://airtable.com/v0.3/table/${tableId}/destroyMultipleColumns`,
+        this._mutationParams({ columnIds }, appId),
+        appId,
+      );
+
+      if (!forceRes.ok) {
+        const errText = await forceRes.text().catch(() => '');
+        for (const f of tableFields) {
+          failed.push({ fieldId: f.fieldId, name: f.expectedName, error: `destroyMultipleColumns force-delete failed (${forceRes.status}): ${errText}` });
+        }
+      } else {
+        this.cache.invalidate(appId);
+        for (const f of tableFields) {
+          succeeded.push({ fieldId: f.fieldId, name: f.expectedName, deleted: true, forced: true });
+        }
+      }
+    }
+
+    if (fields.length > 0) {
+      onProgress?.({ index: fields.length - 1, total: fields.length, succeeded: succeeded.length, failed: failed.length });
+    }
     return { succeeded, failed };
   }
 
diff --git a/packages/mcp-server/test/test-bulk-delete.test.js b/packages/mcp-server/test/test-bulk-delete.test.js
@@ -59,129 +59,126 @@ describe('tool-call semaphore helpers', () => {
 import { AirtableClient } from '../src/client.js';
 
 describe('AirtableClient.deleteFields', () => {
-  it('processes all fields and reports succeeded/failed counts', async () => {
-    // Schema contains all three fields so resolveField succeeds for each.
-    // The cache is invalidated after each successful delete, causing a re-fetch
-    // that always returns the full schema. postForm fails only for fldBAD.
-    const SCHEMA = {
-      data: {
-        tableSchemas: [{
-          id: 'tblAAA',
-          columns: [
-            { id: 'fld001', name: 'Field A', type: 'text', typeOptions: {} },
-            { id: 'fldBAD', name: 'Bad Field', type: 'text', typeOptions: {} },
-            { id: 'fld003', name: 'Field C', type: 'text', typeOptions: {} },
-          ],
-          views: [],
-        }],
-      },
-    };
+  it('batch-deletes all fields with no dependencies in one API call', async () => {
+    const SCHEMA = { data: { tableSchemas: [{ id: 'tblAAA', columns: [
+      { id: 'fld001', name: 'Field A', type: 'text', typeOptions: {} },
+      { id: 'fld002', name: 'Field B', type: 'text', typeOptions: {} },
+    ], views: [] }] } };
     const auth = createMockAuth({
-      get() {
-        return { ok: true, status: 200, json: async () => SCHEMA, text: async () => '{}' };
+      get() { return { ok: true, status: 200, json: async () => SCHEMA, text: async () => '{}' }; },
+      postForm(url) {
+        if (url.includes('destroyMultipleColumns')) {
+          return { ok: true, status: 200, json: async () => ({}), text: async () => '{}' };
+        }
+        return { ok: false, status: 500, json: async () => ({}), text: async () => '' };
       },
+    });
+    const client = new AirtableClient(auth);
+    const result = await client.deleteFields('appTEST', [
+      { fieldId: 'fld001', expectedName: 'Field A' },
+      { fieldId: 'fld002', expectedName: 'Field B' },
+    ]);
+    assert.equal(result.succeeded.length, 2);
+    assert.equal(result.failed.length, 0);
+    const batchCalls = auth.calls.filter(c => c.url.includes('destroyMultipleColumns'));
+    assert.equal(batchCalls.length, 1, 'should make exactly one batch call when no deps');
+  });
+
+  it('makes two batch calls when force=true and deps exist', async () => {
+    const SCHEMA = { data: { tableSchemas: [{ id: 'tblAAA', columns: [
+      { id: 'fld001', name: 'HasDeps', type: 'text', typeOptions: {} },
+    ], views: [] }] } };
+    let batchCallCount = 0;
+    const auth = createMockAuth({
+      get() { return { ok: true, status: 200, json: async () => SCHEMA, text: async () => '{}' }; },
       postForm(url) {
-        // fldBAD destroy call fails with a non-dependency error → throws in deleteField
-        if (url.includes('fldBAD')) {
-          return { ok: false, status: 422, json: async () => ({ error: { type: 'NOT_FOUND' } }), text: async () => 'not found' };
+        if (!url.includes('destroyMultipleColumns')) return { ok: true, status: 200, json: async () => ({}), text: async () => '{}' };
+        batchCallCount++;
+        if (batchCallCount === 1) {
+          return { ok: false, status: 422, json: async () => ({ error: { type: 'SCHEMA_DEPENDENCIES_VALIDATION_FAILED' } }), text: async () => '' };
         }
         return { ok: true, status: 200, json: async () => ({}), text: async () => '{}' };
       },
     });
     const client = new AirtableClient(auth);
-    const fields = [
-      { fieldId: 'fld001', expectedName: 'Field A' },
-      { fieldId: 'fldBAD', expectedName: 'Bad Field' },
-      { fieldId: 'fld003', expectedName: 'Field C' },
-    ];
-    const result = await client.deleteFields('appTEST', fields, { force: true });
-    assert.equal(result.succeeded.length, 2, 'two fields should succeed');
-    assert.equal(result.failed.length, 1, 'one field should fail');
-    assert.equal(result.failed[0].fieldId, 'fldBAD');
-    assert.ok(typeof result.failed[0].error === 'string', 'error must be a string');
+    const result = await client.deleteFields('appTEST', [{ fieldId: 'fld001', expectedName: 'HasDeps' }], { force: true });
+    assert.equal(result.succeeded.length, 1);
+    assert.equal(result.failed.length, 0);
+    assert.equal(result.succeeded[0].forced, true);
+    assert.equal(batchCallCount, 2, 'should make two batch calls for force=true + deps');
   });
 
-  it('calls onProgress once per field', async () => {
+  it('routes to failed when deps exist and force=false', async () => {
+    const SCHEMA = { data: { tableSchemas: [{ id: 'tblAAA', columns: [
+      { id: 'fld001', name: 'HasDeps', type: 'text', typeOptions: {} },
+    ], views: [] }] } };
     const auth = createMockAuth({
-      get() {
-        return {
-          ok: true, status: 200,
-          json: async () => ({
-            data: { tableSchemas: [{ id: 'tbl1', columns: [{ id: 'fld001', name: 'A', type: 'text', typeOptions: {} }], views: [] }] },
-          }),
-          text: async () => '{}',
-        };
+      get() { return { ok: true, status: 200, json: async () => SCHEMA, text: async () => '{}' }; },
+      postForm() {
+        return { ok: false, status: 422, json: async () => ({ error: { type: 'SCHEMA_DEPENDENCIES_VALIDATION_FAILED' } }), text: async () => '' };
       },
     });
     const client = new AirtableClient(auth);
-    const progressLog = [];
-    await client.deleteFields('appTEST', [{ fieldId: 'fld001', expectedName: 'A' }], {
-      onProgress: (info) => progressLog.push(info),
+    const result = await client.deleteFields('appTEST', [{ fieldId: 'fld001', expectedName: 'HasDeps' }], { force: false });
+    assert.equal(result.succeeded.length, 0);
+    assert.equal(result.failed.length, 1);
+    assert.ok(result.failed[0].error.includes('dependencies'), `expected dep error, got: "${result.failed[0].error}"`);
+    const batchCalls = auth.calls.filter(c => c.url.includes('destroyMultipleColumns'));
+    assert.equal(batchCalls.length, 1, 'should stop after first call when force=false');
+  });
+
+  it('routes to failed when expectedName does not match', async () => {
+    const SCHEMA = { data: { tableSchemas: [{ id: 'tblAAA', columns: [
+      { id: 'fld001', name: 'Actual Name', type: 'text', typeOptions: {} },
+    ], views: [] }] } };
+    const auth = createMockAuth({
+      get() { return { ok: true, status: 200, json: async () => SCHEMA, text: async () => '{}' }; },
     });
-    assert.equal(progressLog.length, 1);
-    assert.equal(progressLog[0].index, 0);
-    assert.equal(progressLog[0].total, 1);
+    const client = new AirtableClient(auth);
+    const result = await client.deleteFields('appTEST', [{ fieldId: 'fld001', expectedName: 'Wrong Name' }]);
+    assert.equal(result.succeeded.length, 0);
+    assert.equal(result.failed.length, 1);
+    assert.ok(result.failed[0].error.includes('Safety check'), `expected safety check error, got: "${result.failed[0].error}"`);
+    assert.equal(auth.calls.filter(c => c.url.includes('destroyMultipleColumns')).length, 0, 'no batch call if safety check fails');
   });
 
-  it('continues processing after a per-field failure', async () => {
-    // Schema contains both fields. fld001 postForm fails (non-dep error → throws),
-    // fld002 succeeds. Cache is invalidated only on success, but the schema mock
-    // always returns both fields so both resolveField calls succeed regardless.
-    const SCHEMA2 = {
-      data: { tableSchemas: [{ id: 'tbl1', columns: [
-        { id: 'fld001', name: 'A', type: 'text', typeOptions: {} },
-        { id: 'fld002', name: 'B', type: 'text', typeOptions: {} },
-      ], views: [] }] },
-    };
+  it('processes fields from different tables independently', async () => {
+    const SCHEMA = { data: { tableSchemas: [
+      { id: 'tbl1', columns: [{ id: 'fld001', name: 'A', type: 'text', typeOptions: {} }], views: [] },
+      { id: 'tbl2', columns: [{ id: 'fld002', name: 'B', type: 'text', typeOptions: {} }], views: [] },
+    ] } };
     const auth = createMockAuth({
-      get() {
-        return { ok: true, status: 200, json: async () => SCHEMA2, text: async () => '{}' };
-      },
+      get() { return { ok: true, status: 200, json: async () => SCHEMA, text: async () => '{}' }; },
       postForm(url) {
-        if (url.includes('fld001')) return { ok: false, status: 422, json: async () => ({ error: { type: 'ERR' } }), text: async () => 'err' };
-        return { ok: true, status: 200, json: async () => ({}), text: async () => '{}' };
+        if (url.includes('tbl1')) return { ok: true, status: 200, json: async () => ({}), text: async () => '{}' };
+        return { ok: false, status: 500, json: async () => ({ error: { type: 'INTERNAL' } }), text: async () => 'error' };
       },
     });
     const client = new AirtableClient(auth);
     const result = await client.deleteFields('appTEST', [
       { fieldId: 'fld001', expectedName: 'A' },
       { fieldId: 'fld002', expectedName: 'B' },
-    ], { force: true });
+    ]);
     assert.equal(result.succeeded.length, 1);
     assert.equal(result.failed.length, 1);
+    assert.equal(result.succeeded[0].fieldId, 'fld001');
+    assert.equal(result.failed[0].fieldId, 'fld002');
   });
 
-  it('routes deleted:false (dependency-blocked) to failed[] when force=false', async () => {
-    const DEPENDENCY_RESPONSE = {
-      ok: false,
-      status: 400,
-      json: async () => ({
-        error: {
-          type: 'SCHEMA_DEPENDENCIES_VALIDATION_FAILED',
-          details: { dependentColumns: [{ id: 'fldDEP', type: 'formula' }] },
-        },
-      }),
-      text: async () => JSON.stringify({ error: { type: 'SCHEMA_DEPENDENCIES_VALIDATION_FAILED' } }),
-    };
+  it('calls onProgress once at end with final counts', async () => {
+    const SCHEMA = { data: { tableSchemas: [{ id: 'tbl1', columns: [
+      { id: 'fld001', name: 'A', type: 'text', typeOptions: {} },
+    ], views: [] }] } };
     const auth = createMockAuth({
-      get() {
-        return {
-          ok: true, status: 200,
-          json: async () => ({
-            data: { tableSchemas: [{ id: 'tbl1', columns: [{ id: 'fld001', name: 'HasDeps', type: 'formula', typeOptions: {} }], views: [] }] },
-          }),
-          text: async () => '{}',
-        };
-      },
-      postForm() {
-        return DEPENDENCY_RESPONSE;
-      },
+      get() { return { ok: true, status: 200, json: async () => SCHEMA, text: async () => '{}' }; },
     });
     const client = new AirtableClient(auth);
-    const result = await client.deleteFields('appTEST', [{ fieldId: 'fld001', expectedName: 'HasDeps' }], { force: false });
-    assert.equal(result.succeeded.length, 0, 'no fields should be in succeeded');
-    assert.equal(result.failed.length, 1, 'dependency-blocked field should be in failed');
-    assert.ok(result.failed[0].error.includes('dependencies') || result.failed[0].error.includes('not deleted'),
-      `error message should mention dependencies or not deleted, got: "${result.failed[0].error}"`);
+    const progressLog = [];
+    await client.deleteFields('appTEST', [{ fieldId: 'fld001', expectedName: 'A' }], {
+      onProgress: (info) => progressLog.push(info),
+    });
+    assert.equal(progressLog.length, 1);
+    assert.equal(progressLog[0].index, 0);
+    assert.equal(progressLog[0].total, 1);
   });
 });
