fix(mcp): issues #9 and #10 — list_tables empty array bug + queryRecords wrong response path

Issue #9 — list_tables returned [] for bases where the getApplicationScaffoldingData
endpoint returns tableById + visibleTableOrder alongside an empty tables:[] stub.
Empty arrays are truthy in JS, so the || chain short-circuited at tables:[] before
reaching Object.values(tableById).

Fix: extracted parsing into AirtableClient.parseScaffoldingTables() which guards
each candidate with .length > 0, handles visibleTableOrder in addition to tableOrder,
and uses Object.values(tableById) as the final fallback. list_tables handler now
delegates to this method. 5 regression tests added.

Issue #10 — query_records returned {count:0, rows:[]} for views with real records.
Two bugs:
  1. Wrong response field: code read queryResults[queryId].rows but the readQueries
     endpoint returns querySlices[0].rowIds + tableDataById[tableId].partialRowById
     (confirmed from capture 2026-05-21T14-16-09-125Z.json).
  2. Silent failure: res.json().catch(()=>({})) swallowed non-JSON (msgpack) parse
     errors, masking the problem entirely.

Fix: rewrote response parsing to use the correct querySlices + tableDataById path.
res.json() replaced with res.text() + explicit JSON.parse that throws a descriptive
error on msgpack/binary responses instead of returning empty. failureReasonByQueryId
now surfaces query-level errors. Debug mode now exposes the raw API response instead
of the already-processed result. 4 regression tests added.

249 tests pass (245 before + 9 new).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: ARHAEEM

packages/mcp-server/src/client.js

@@ -494,6 +494,20 @@ export class AirtableClient {
     return data;
   }
 
+  // Normalizes the data block from getScaffoldingData into a flat table array.
+  // Guards every candidate with a length check — empty arrays are truthy and
+  // would short-circuit the chain before reaching tableById (the scaffolding
+  // endpoint returns tableById + visibleTableOrder, not tableSchemas/tables).
+  parseScaffoldingTables(d) {
+    return (d?.tableSchemas?.length > 0 && d.tableSchemas) ||
+      (d?.tables?.length > 0 && d.tables) ||
+      (d?.visibleTableOrder || d?.tableOrder)?.map(id => {
+        const t = d?.tableById?.[id] || d?.tableDatas?.[id] || {};
+        return { id, name: t.name || id };
+      }) ||
+      Object.values(d?.tableById || {});
+  }
+
   async getUserProperties() {
     const res = await this.auth.get('https://airtable.com/v0.3/getUserProperties');
     if (!res.ok) {
@@ -2282,11 +2296,37 @@ export class AirtableClient {
       throw new Error(`readQueries failed (${res.status}): ${errText}`);
     }
 
-    const data = await res.json().catch(() => ({}));
-    const queryResult = data?.data?.queryResults?.[queryId] || {};
-    const rows = queryResult.rows || [];
+    // Fetch as text first so a non-JSON (msgpack) response surfaces as an error
+    // instead of silently returning empty results.
+    const text = await res.text().catch(() => '');
+    let data;
+    try {
+      data = JSON.parse(text);
+    } catch (e) {
+      throw new Error(
+        `readQueries returned non-JSON (${text.length} bytes, first byte 0x${text.charCodeAt(0).toString(16)}). ` +
+        `The server may not be honoring allowMsgpackOfResult:false. ` +
+        `Try re-authenticating or filing a bug.`,
+      );
+    }
 
-    const mapped = rows.map(r => ({ id: r.id, fields: r.cellValuesByColumnId || r.cellValues || {} }));
+    // Surface query-level failures (e.g. bad viewId, permissions).
+    const failure = data?.data?.failureReasonByQueryId?.[queryId];
+    if (failure) throw new Error(`readQueries query failed: ${JSON.stringify(failure)}`);
+
+    // Response structure (captured 2026-05-21):
+    //   data.querySlices[0].rowIds          — ordered record IDs for this query
+    //   data.tableDataById[tableId]
+    //     .partialRowById[rowId]
+    //     .cellValuesByColumnId             — resolved cell values per field
+    const slice = data?.data?.querySlices?.[0] || {};
+    const rowIds = slice.rowIds || [];
+    const tableData = data?.data?.tableDataById?.[tableId] || {};
+
+    const mapped = rowIds.map(rowId => {
+      const rowData = tableData.partialRowById?.[rowId] || {};
+      return { id: rowId, fields: rowData.cellValuesByColumnId || {} };
+    });
 
     // Client-side substring search across all resolved field values (including lookup fields).
     // This avoids the FIND()/SEARCH() formula limitation of the REST API that doesn't reach
@@ -2305,12 +2345,15 @@ export class AirtableClient {
       : mapped;
 
     return {
-      tableId,
-      viewId,
-      limit: clampedLimit,
-      search: search || null,
-      count: filtered.length,
-      rows: filtered,
+      summary: {
+        tableId,
+        viewId,
+        limit: clampedLimit,
+        search: search || null,
+        count: filtered.length,
+        rows: filtered,
+      },
+      raw: data,
     };
   }
 

packages/mcp-server/src/index.js

@@ -1570,12 +1570,7 @@ const handlers = {
 
   async list_tables({ appId, debug }) {
     const raw = await client.getScaffoldingData(appId);
-    const tables = raw?.data?.tableSchemas || raw?.data?.tables ||
-      raw?.data?.tableOrder?.map(id => {
-        const t = raw?.data?.tableDatas?.[id] || raw?.data?.tableById?.[id] || {};
-        return { id, name: t.name || id };
-      }) ||
-      Object.values(raw?.data?.tableById || {});
+    const tables = client.parseScaffoldingTables(raw?.data);
     const summary = tables.map(t => ({
       id: t.id,
       name: t.name,
@@ -2245,8 +2240,8 @@ const handlers = {
   // ── Record Read ──
 
   async query_records({ appId, tableId, viewId, columnIds, limit, search, debug }) {
-    const result = await client.queryRecords(appId, tableId, viewId, { columnIds, limit, search });
-    return ok(result, result, debug);
+    const { summary, raw } = await client.queryRecords(appId, tableId, viewId, { columnIds, limit, search });
+    return ok(summary, raw, debug);
   },
 
   // ── Record Write ──

packages/mcp-server/test/test-client.test.js

@@ -333,6 +333,59 @@ describe('AirtableClient', () => {
     });
   });
 
+  // Regression tests for issue #9 — list_tables returned [] when the scaffolding
+  // endpoint returned tableById + visibleTableOrder (empty arrays are truthy and
+  // previously short-circuited the fallback chain before reaching tableById).
+  describe('parseScaffoldingTables (issue #9 regression)', () => {
+    it('returns tables from tableById when tables:[] is present (empty-array truthy bug)', () => {
+      const result = client.parseScaffoldingTables({
+        tables: [],  // empty array — truthy, was short-circuiting the fallback chain
+        tableById: {
+          tblAAA: { id: 'tblAAA', name: 'Tasks' },
+          tblBBB: { id: 'tblBBB', name: 'Projects' },
+        },
+        visibleTableOrder: ['tblAAA', 'tblBBB'],
+      });
+      assert.equal(result.length, 2);
+      assert.deepEqual(result.map(t => t.id), ['tblAAA', 'tblBBB']);
+    });
+
+    it('respects visibleTableOrder for correct sort', () => {
+      const result = client.parseScaffoldingTables({
+        tableById: {
+          tblAAA: { id: 'tblAAA', name: 'Tasks' },
+          tblBBB: { id: 'tblBBB', name: 'Projects' },
+        },
+        visibleTableOrder: ['tblBBB', 'tblAAA'],
+      });
+      assert.equal(result[0].name, 'Projects');
+      assert.equal(result[1].name, 'Tasks');
+    });
+
+    it('prefers non-empty tableSchemas over tableById', () => {
+      const result = client.parseScaffoldingTables({
+        tableSchemas: [{ id: 'tblXXX', name: 'Schema Table' }],
+        tableById: { tblYYY: { id: 'tblYYY', name: 'Dict Table' } },
+      });
+      assert.equal(result.length, 1);
+      assert.equal(result[0].id, 'tblXXX');
+    });
+
+    it('skips empty tableSchemas and uses tableById', () => {
+      const result = client.parseScaffoldingTables({
+        tableSchemas: [],
+        tableById: { tblAAA: { id: 'tblAAA', name: 'Tasks' } },
+      });
+      assert.equal(result.length, 1);
+      assert.equal(result[0].name, 'Tasks');
+    });
+
+    it('returns [] for empty data', () => {
+      assert.deepEqual(client.parseScaffoldingTables({}), []);
+      assert.deepEqual(client.parseScaffoldingTables(null), []);
+    });
+  });
+
   describe('validateFormula', () => {
     it('returns valid result for good formula', async () => {
       mockAuth.postForm = () => ({
@@ -827,6 +880,106 @@ describe('AirtableClient', () => {
     });
   });
 
+  // Regression tests for issue #10 — queryRecords returned {count:0, rows:[]} because:
+  //   1. Wrong response field: code used queryResults[queryId] but API returns querySlices[0]
+  //   2. Cell values at tableDataById[tableId].partialRowById[rowId].cellValuesByColumnId
+  //   3. Silent failure: res.json().catch(()=>({})) hid msgpack parse errors
+  describe('queryRecords (issue #10 regression)', () => {
+    function makeReadQueriesAuth(querySlicesResponse) {
+      return createMockAuth({
+        get: (url) => {
+          if (url.includes('/read') || url.includes('getApplicationScaffoldingData')) {
+            return { ok: true, status: 200, json: async () => MOCK_SCHEMA, text: async () => JSON.stringify(MOCK_SCHEMA) };
+          }
+          return { ok: true, status: 200, json: async () => ({}), text: async () => '{}' };
+        },
+        postForm: () => ({
+          ok: true,
+          status: 200,
+          json: async () => querySlicesResponse,
+          text: async () => JSON.stringify(querySlicesResponse),
+        }),
+      });
+    }
+
+    it('parses querySlices+tableDataById response correctly', async () => {
+      const mockApiResponse = {
+        data: {
+          querySlices: [{
+            tableId: 'tblAAA',
+            rowIds: ['recAAA', 'recBBB'],
+            columnIds: ['fld111', 'fld222'],
+          }],
+          tableDataById: {
+            tblAAA: {
+              partialRowById: {
+                recAAA: { createdTime: '2026-01-01T00:00:00.000Z', cellValuesByColumnId: { fld111: 'Task 1', fld222: 'Done' } },
+                recBBB: { createdTime: '2026-01-02T00:00:00.000Z', cellValuesByColumnId: { fld111: 'Task 2', fld222: 'In Progress' } },
+              },
+            },
+          },
+          failureReasonByQueryId: {},
+        },
+      };
+      const auth = makeReadQueriesAuth(mockApiResponse);
+      const client = new AirtableClient(auth);
+      const { summary } = await client.queryRecords('appXXX', 'tblAAA', 'viwBBB', { limit: 10 });
+      assert.equal(summary.count, 2);
+      assert.equal(summary.rows[0].id, 'recAAA');
+      assert.equal(summary.rows[0].fields.fld111, 'Task 1');
+      assert.equal(summary.rows[1].fields.fld222, 'In Progress');
+    });
+
+    it('returns 0 rows when querySlices is absent (not an error)', async () => {
+      const auth = makeReadQueriesAuth({ data: { querySlices: [], tableDataById: {}, failureReasonByQueryId: {} } });
+      const client = new AirtableClient(auth);
+      const { summary } = await client.queryRecords('appXXX', 'tblAAA', 'viwBBB');
+      assert.equal(summary.count, 0);
+    });
+
+    it('throws on non-JSON response instead of silently returning empty', async () => {
+      const auth = createMockAuth({
+        get: (url) => ({ ok: true, status: 200, json: async () => MOCK_SCHEMA, text: async () => JSON.stringify(MOCK_SCHEMA) }),
+        postForm: () => ({
+          ok: true,
+          status: 200,
+          text: async () => '\xfd\x72\x40binary-not-json',  // msgpack-like binary
+        }),
+      });
+      const client = new AirtableClient(auth);
+      await assert.rejects(
+        () => client.queryRecords('appXXX', 'tblAAA', 'viwBBB'),
+        { message: /non-JSON/ },
+      );
+    });
+
+    it('throws on query-level failure', async () => {
+      const auth = makeReadQueriesAuth({
+        data: {
+          querySlices: [],
+          tableDataById: {},
+          failureReasonByQueryId: { 'qryANYID': { type: 'PERMISSION_DENIED' } },
+        },
+      });
+      // We need to override postForm to capture the dynamic queryId and inject it into failure map.
+      // Instead, test with a static failure that doesn't need the exact queryId:
+      const staticAuth = createMockAuth({
+        get: (url) => ({ ok: true, status: 200, json: async () => MOCK_SCHEMA, text: async () => JSON.stringify(MOCK_SCHEMA) }),
+        postForm: (url, params) => {
+          const payload = JSON.parse(params.stringifiedObjectParams);
+          const qid = payload.queries[0].id;
+          const resp = { data: { querySlices: [], tableDataById: {}, failureReasonByQueryId: { [qid]: { type: 'PERMISSION_DENIED' } } } };
+          return { ok: true, status: 200, text: async () => JSON.stringify(resp) };
+        },
+      });
+      const client = new AirtableClient(staticAuth);
+      await assert.rejects(
+        () => client.queryRecords('appXXX', 'tblAAA', 'viwBBB'),
+        { message: /PERMISSION_DENIED/ },
+      );
+    });
+  });
+
   describe('Airtable ID validation (defense-in-depth)', () => {
     // These tests guard against URL-injection regressions. Every client
     // method that interpolates an ID into a URL must validate it up-front.