Skip to content

Commit 7e76c02

Browse files
ARHAEEMclaude
andcommitted
fix(mcp): address 4 open bugs from bulk-delete report
Bug 6 (HIGH): Silent success masked as timeout - Wrap both destroyMultipleColumns calls in try/catch in deleteFields() - On network/timeout error, call _verifyFieldsDeletion() which invalidates cache, re-fetches schema, and checks which fields are actually gone - Fields missing from fresh schema → succeeded (timedOut:true); still present → failed with honest error message Bug 7 (MEDIUM): Name-verification timeout on 300+ field tables - Thread evalTimeoutMs through auth._rawApiCall / _apiCall / get() - getApplicationData() now passes evalTimeoutMs:60_000 (was hardcoded 15s) - Add getApplicationDataForTable(appId, tableId) using includeDataForTableIds for targeted single-table fetches — avoids downloading the full base schema when the caller already knows the tableId - resolveTable() uses targeted fetch when tableIdOrName starts with 'tbl', falling back to full fetch for name-based lookups - getApplicationDataForTable() detects if server ignored the filter (returned all tables) and stores the result in the full cache to avoid redundant fetches Bug 5 (Partial→Fixed): Checkpoint not written on timeout - Flows from Bug 6 fix: post-timeout verification now correctly populates succeeded[] so the checkpoint handler has accurate data to write Bug 3 (MEDIUM): list_fields returns full 2.7MB schema - Add optional fieldType param: filter fields by type ("formula", "text", …) - Add optional nameContains param: case-insensitive substring match on name - Update tool description to mention filtering on large tables Bug 4 (Airtable cursor behavior — not actionable) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ab7ed70 commit 7e76c02

4 files changed

Lines changed: 147 additions & 26 deletions

File tree

packages/mcp-server/src/auth.js

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ export class AirtableAuth {
368368

369369
// ─── Raw API Call (no queue, no recovery) ─────────────────────
370370

371-
async _rawApiCall(method, urlPath, body = null, appId = null, contentType = 'form') {
371+
async _rawApiCall(method, urlPath, body = null, appId = null, contentType = 'form', evalTimeoutMs = 15_000) {
372372
const url = urlPath.startsWith('http') ? urlPath : `https://airtable.com${urlPath}`;
373373
const timeZone = Intl.DateTimeFormat().resolvedOptions().timeZone;
374374
const csrfToken = this.csrfToken;
@@ -379,13 +379,14 @@ export class AirtableAuth {
379379

380380
// H3 — per-evaluate timeout. Playwright has no built-in timeout on
381381
// page.evaluate; a CDP-dead Chromium can hang forever. We race against
382-
// a 15s timer and let _apiCall's catch trigger _recoverSession.
383-
const EVAL_TIMEOUT_MS = 15_000;
382+
// a configurable timer (default 15s) and let _apiCall's catch trigger
383+
// _recoverSession. Callers handling large responses (e.g. getApplicationData)
384+
// should pass a higher evalTimeoutMs to avoid spurious recovery cycles.
384385
const withTimeout = (promise) => Promise.race([
385386
promise,
386387
new Promise((_, reject) => setTimeout(
387-
() => reject(new Error(`page.evaluate exceeded ${EVAL_TIMEOUT_MS / 1000}s; browser may be unresponsive`)),
388-
EVAL_TIMEOUT_MS,
388+
() => reject(new Error(`page.evaluate exceeded ${evalTimeoutMs / 1000}s; browser may be unresponsive`)),
389+
evalTimeoutMs,
389390
)),
390391
]);
391392

@@ -448,7 +449,7 @@ export class AirtableAuth {
448449

449450
// ─── Queued API Call (with session recovery & rate-limit backoff) ──────────────────
450451

451-
async _apiCall(method, urlPath, body = null, appId = null, contentType = 'form') {
452+
async _apiCall(method, urlPath, body = null, appId = null, contentType = 'form', evalTimeoutMs = 15_000) {
452453
return this._enqueue(async () => {
453454
await this.ensureLoggedIn();
454455

@@ -464,7 +465,7 @@ export class AirtableAuth {
464465
}
465466
let result;
466467
try {
467-
result = await this._rawApiCall(method, urlPath, body, appId, contentType);
468+
result = await this._rawApiCall(method, urlPath, body, appId, contentType, evalTimeoutMs);
468469
} catch (evalError) {
469470
// page.evaluate itself failed (browser crashed, context destroyed)
470471
if (!this._recovering && attempt < MAX_RETRIES) {
@@ -527,11 +528,11 @@ export class AirtableAuth {
527528
}
528529
}
529530

530-
async get(url, appId) {
531+
async get(url, appId, { evalTimeoutMs = 15_000 } = {}) {
531532
const pattern = url.replace(/.*v0\.3\//, '').replace(/(app|tbl|viw|fld|rec|usr|wsp|sel|flt|blk|ext|col)[A-Za-z0-9]{10,}/g, '$1*');
532533
trace('http', 'http:request', { method: 'GET', endpoint_pattern: pattern, has_payload: false });
533534
const start = Date.now();
534-
const result = await this._apiCall('GET', url, null, appId);
535+
const result = await this._apiCall('GET', url, null, appId, 'form', evalTimeoutMs);
535536
trace('http', 'http:response', { endpoint_pattern: pattern, status: result.status, duration_ms: Date.now() - start });
536537
return this._wrapResponse(result);
537538
}

packages/mcp-server/src/cache.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ export class SchemaCache {
1919
// we use for simple LRU-ish eviction (evict oldest when size exceeds cap).
2020
this._scaffolding = new Map();
2121
this._full = new Map();
22+
// Map<`${appId}:${tableId}`, { data, timestamp }> — single-table targeted fetches
23+
this._table = new Map();
2224
}
2325

2426
// ─── Scaffolding Cache ────────────────────────────────────────
@@ -41,20 +43,34 @@ export class SchemaCache {
4143
this._setEntry(this._full, appId, data);
4244
}
4345

46+
// ─── Per-Table Cache ──────────────────────────────────────────
47+
48+
getTable(appId, tableId) {
49+
return this._getEntry(this._table, `${appId}:${tableId}`);
50+
}
51+
52+
setTable(appId, tableId, data) {
53+
this._setEntry(this._table, `${appId}:${tableId}`, data);
54+
}
55+
4456
// ─── Invalidation ─────────────────────────────────────────────
4557

4658
/**
4759
* Invalidate caches for an app after a mutation.
48-
* Clears both full and scaffolding since field changes affect both.
60+
* Clears full, scaffolding, and all per-table entries for this app.
4961
*/
5062
invalidate(appId) {
5163
this._full.delete(appId);
5264
this._scaffolding.delete(appId);
65+
for (const key of this._table.keys()) {
66+
if (key.startsWith(`${appId}:`)) this._table.delete(key);
67+
}
5368
}
5469

5570
invalidateAll() {
5671
this._full.clear();
5772
this._scaffolding.clear();
73+
this._table.clear();
5874
}
5975

6076
// ─── Internals ────────────────────────────────────────────────

packages/mcp-server/src/client.js

Lines changed: 109 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,8 @@ export class AirtableClient {
424424
requestId: this._genRequestId(),
425425
});
426426
const url = `https://airtable.com/v0.3/application/${appId}/read?${params}`;
427-
const res = await this.auth.get(url, appId);
427+
// Use a generous timeout — full-base responses can exceed 1MB on large bases.
428+
const res = await this.auth.get(url, appId, { evalTimeoutMs: 60_000 });
428429
if (!res.ok) {
429430
const text = await res.text();
430431
throw new Error(`getApplicationData failed (${res.status}): ${text}`);
@@ -434,6 +435,49 @@ export class AirtableClient {
434435
return data;
435436
}
436437

438+
/**
439+
* Fetch schema data for a single table using includeDataForTableIds.
440+
* Dramatically reduces response size on large bases (avoids downloading all tables).
441+
* Falls back to caching full data if the endpoint returns all tables anyway.
442+
*/
443+
async getApplicationDataForTable(appId, tableId) {
444+
assertAirtableId(appId, 'appId');
445+
assertAirtableId(tableId, 'tableId');
446+
447+
const cachedTable = this.cache.getTable(appId, tableId);
448+
if (cachedTable) return cachedTable;
449+
450+
// Also accept a warm full-cache hit — no extra fetch needed.
451+
const cachedFull = this.cache.getFull(appId);
452+
if (cachedFull) return cachedFull;
453+
454+
const params = new URLSearchParams({
455+
stringifiedObjectParams: JSON.stringify({
456+
includeDataForTableIds: [tableId],
457+
includeDataForViewIds: null,
458+
shouldIncludeSchemaChecksum: false,
459+
}),
460+
requestId: this._genRequestId(),
461+
});
462+
const url = `https://airtable.com/v0.3/application/${appId}/read?${params}`;
463+
const res = await this.auth.get(url, appId, { evalTimeoutMs: 60_000 });
464+
if (!res.ok) {
465+
const text = await res.text();
466+
throw new Error(`getApplicationDataForTable failed (${res.status}): ${text}`);
467+
}
468+
const data = await res.json();
469+
const tables = data?.data?.tableSchemas || data?.data?.tables || [];
470+
471+
if (tables.length === 1 && tables[0].id === tableId) {
472+
// Targeted response — cache per-table only (don't pollute the full cache).
473+
this.cache.setTable(appId, tableId, data);
474+
} else {
475+
// Endpoint returned the full base (no server-side filtering) — cache as full.
476+
this.cache.setFull(appId, data);
477+
}
478+
return data;
479+
}
480+
437481
async getScaffoldingData(appId) {
438482
assertAirtableId(appId, 'appId');
439483
const cached = this.cache.getScaffolding(appId);
@@ -462,7 +506,11 @@ export class AirtableClient {
462506
// ─── Schema Resolution Helpers ────────────────────────────────
463507

464508
async resolveTable(appId, tableIdOrName) {
465-
const data = await this.getApplicationData(appId);
509+
// When caller passes a table ID we can do a targeted single-table fetch,
510+
// avoiding a full-base download on large bases.
511+
const data = tableIdOrName.startsWith('tbl')
512+
? await this.getApplicationDataForTable(appId, tableIdOrName)
513+
: await this.getApplicationData(appId);
466514
const tables = data?.data?.tableSchemas || data?.data?.tables || [];
467515

468516
// Exact ID match is always unambiguous.
@@ -691,6 +739,31 @@ export class AirtableClient {
691739
return { deleted: true, fieldId, name: expectedName, forced: true, ...data };
692740
}
693741

742+
/**
743+
* After a destroyMultipleColumns network error, verify which fields are actually
744+
* gone from a fresh schema fetch. This handles the case where the request timed
745+
* out at the MCP layer but Airtable had already deleted the fields (Bug 6).
746+
*/
747+
async _verifyFieldsDeletion(appId, tableFields) {
748+
this.cache.invalidate(appId);
749+
const freshData = await this.getApplicationData(appId);
750+
const tables = freshData?.data?.tableSchemas || freshData?.data?.tables || [];
751+
const stillExist = new Set();
752+
for (const t of tables) {
753+
for (const f of (t.columns || t.fields || [])) stillExist.add(f.id);
754+
}
755+
const succeeded = [];
756+
const failed = [];
757+
for (const { fieldId, expectedName } of tableFields) {
758+
if (!stillExist.has(fieldId)) {
759+
succeeded.push({ fieldId, name: expectedName, deleted: true, forced: false, timedOut: true });
760+
} else {
761+
failed.push({ fieldId, name: expectedName, error: 'destroyMultipleColumns timed out; field still exists' });
762+
}
763+
}
764+
return { succeeded, failed };
765+
}
766+
694767
/**
695768
* Delete multiple fields using the native batch endpoint (destroyMultipleColumns).
696769
* Fields are grouped by tableId and each table's fields are deleted in 1-2 API
@@ -730,11 +803,24 @@ export class AirtableClient {
730803
for (const [tableId, tableFields] of byTable) {
731804
const columnIds = tableFields.map(f => f.fieldId);
732805

733-
const checkRes = await this.auth.postForm(
734-
`https://airtable.com/v0.3/table/${tableId}/destroyMultipleColumns`,
735-
this._mutationParams({ columnIds, checkSchemaDependencies: true }, appId),
736-
appId,
737-
);
806+
let checkRes;
807+
try {
808+
checkRes = await this.auth.postForm(
809+
`https://airtable.com/v0.3/table/${tableId}/destroyMultipleColumns`,
810+
this._mutationParams({ columnIds, checkSchemaDependencies: true }, appId),
811+
appId,
812+
);
813+
} catch (err) {
814+
// Network/timeout — Airtable may have already deleted the fields.
815+
// Verify by re-fetching the schema (Bug 6: silent success masking).
816+
const verified = await this._verifyFieldsDeletion(appId, tableFields).catch(ve => ({
817+
succeeded: [],
818+
failed: tableFields.map(f => ({ fieldId: f.fieldId, name: f.expectedName, error: `destroyMultipleColumns network error: ${err.message}; verification also failed: ${ve.message}` })),
819+
}));
820+
succeeded.push(...verified.succeeded);
821+
failed.push(...verified.failed);
822+
continue;
823+
}
738824

739825
if (checkRes.ok) {
740826
this.cache.invalidate(appId);
@@ -758,11 +844,22 @@ export class AirtableClient {
758844
}
759845

760846
// Deps exist and force=true: second call without checkSchemaDependencies
761-
const forceRes = await this.auth.postForm(
762-
`https://airtable.com/v0.3/table/${tableId}/destroyMultipleColumns`,
763-
this._mutationParams({ columnIds }, appId),
764-
appId,
765-
);
847+
let forceRes;
848+
try {
849+
forceRes = await this.auth.postForm(
850+
`https://airtable.com/v0.3/table/${tableId}/destroyMultipleColumns`,
851+
this._mutationParams({ columnIds }, appId),
852+
appId,
853+
);
854+
} catch (err) {
855+
const verified = await this._verifyFieldsDeletion(appId, tableFields).catch(ve => ({
856+
succeeded: [],
857+
failed: tableFields.map(f => ({ fieldId: f.fieldId, name: f.expectedName, error: `destroyMultipleColumns force network error: ${err.message}; verification also failed: ${ve.message}` })),
858+
}));
859+
succeeded.push(...verified.succeeded);
860+
failed.push(...verified.failed);
861+
continue;
862+
}
766863

767864
if (!forceRes.ok) {
768865
const errText = await forceRes.text().catch(() => '');

packages/mcp-server/src/index.js

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,13 +212,15 @@ const TOOLS = [
212212
},
213213
{
214214
name: 'list_fields',
215-
description: 'List all fields (columns) in a specific table of an Airtable base.',
215+
description: 'List fields (columns) in a table. Returns id, name, type, and typeOptions for each field. On large tables (100+ fields) use fieldType or nameContains to filter the response and reduce context size.',
216216
annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false },
217217
inputSchema: {
218218
type: 'object',
219219
properties: {
220220
appId: { type: 'string', description: 'The Airtable base/application ID' },
221-
tableIdOrName: { type: 'string', description: 'The table ID or name to list fields for' },
221+
tableIdOrName: { type: 'string', description: 'The table ID (tblXXX) or exact name' },
222+
fieldType: { type: 'string', description: 'Return only fields of this type, e.g. "formula", "text", "number", "checkbox"' },
223+
nameContains: { type: 'string', description: 'Return only fields whose name contains this substring (case-insensitive)' },
222224
debug: debugProp,
223225
},
224226
required: ['appId', 'tableIdOrName'],
@@ -1545,14 +1547,19 @@ const handlers = {
15451547
return ok(summary, table, debug);
15461548
},
15471549

1548-
async list_fields({ appId, tableIdOrName, debug }) {
1550+
async list_fields({ appId, tableIdOrName, fieldType, nameContains, debug }) {
15491551
const table = await client.resolveTable(appId, tableIdOrName);
1550-
const fields = (table.columns || table.fields || []).map(f => ({
1552+
let fields = (table.columns || table.fields || []).map(f => ({
15511553
id: f.id,
15521554
name: f.name,
15531555
type: f.type,
15541556
typeOptions: f.typeOptions,
15551557
}));
1558+
if (fieldType) fields = fields.filter(f => f.type === fieldType);
1559+
if (nameContains) {
1560+
const needle = nameContains.toLowerCase();
1561+
fields = fields.filter(f => f.name.toLowerCase().includes(needle));
1562+
}
15561563
return ok(fields, table, debug);
15571564
},
15581565

0 commit comments

Comments
 (0)