Skip to content

Commit dae9eb8

Browse files
committed
fix(mcp): use formulaTextParsed key + preserve typeOptions on formula updates
- Read formula text from typeOptions.formulaTextParsed (live API key) with fallback to formulaText/formula - Preserve existing typeOptions (format, precision, etc.) when updating formulas to prevent wiping PercentV2/precision settings - Fix parseScaffoldingTables to only map visibleTableOrder/tableOrder when non-empty (avoid mapping undefined)
1 parent 802b0f8 commit dae9eb8

3 files changed

Lines changed: 208 additions & 7 deletions

File tree

packages/mcp-server/src/client.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -515,12 +515,14 @@ export class AirtableClient {
515515
// would short-circuit the chain before reaching tableById (the scaffolding
516516
// endpoint returns tableById + visibleTableOrder, not tableSchemas/tables).
517517
parseScaffoldingTables(d) {
518+
const order = (d?.visibleTableOrder?.length > 0 && d.visibleTableOrder) ||
519+
(d?.tableOrder?.length > 0 && d.tableOrder);
518520
return (d?.tableSchemas?.length > 0 && d.tableSchemas) ||
519521
(d?.tables?.length > 0 && d.tables) ||
520-
(d?.visibleTableOrder || d?.tableOrder)?.map(id => {
522+
(order && order.map(id => {
521523
const t = d?.tableById?.[id] || d?.tableDatas?.[id] || {};
522524
return { id, name: t.name || id };
523-
}) ||
525+
})) ||
524526
Object.values(d?.tableById || {});
525527
}
526528

packages/mcp-server/src/index.js

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1778,9 +1778,11 @@ const handlers = {
17781778
if (foundField.type !== 'formula') {
17791779
return { content: [{ type: 'text', text: JSON.stringify({ error: `Field ${fieldId} is type "${foundField.type}", not formula` }) }], isError: true };
17801780
}
1781-
// Airtable's internal API has stored formula text at typeOptions.formulaText historically.
1782-
// Defensive fallback chain handles potential key renames in future API responses.
1783-
const formulaText = foundField.typeOptions?.formulaText
1781+
// Live Airtable internal API stores formula text under typeOptions.formulaTextParsed.
1782+
// typeOptions.formulaText is the historical key kept as a fallback.
1783+
// Note: formulaTextParsed encodes field refs as {column_value_fldXXX} placeholders.
1784+
const formulaText = foundField.typeOptions?.formulaTextParsed
1785+
|| foundField.typeOptions?.formulaText
17841786
|| foundField.typeOptions?.formula
17851787
|| foundField.formula
17861788
|| '';
@@ -1817,7 +1819,8 @@ const handlers = {
18171819
await mkdir(tableDir, { recursive: true });
18181820

18191821
for (const field of formulaFields) {
1820-
const formulaText = field.typeOptions?.formulaText
1822+
const formulaText = field.typeOptions?.formulaTextParsed
1823+
|| field.typeOptions?.formulaText
18211824
|| field.typeOptions?.formula
18221825
|| field.formula
18231826
|| '';
@@ -1877,10 +1880,19 @@ const handlers = {
18771880
} else {
18781881
formula = stripHeader(formulaText, 'formula').text;
18791882
}
1883+
// Fetch current typeOptions so format/precision settings (e.g. PercentV2, precision)
1884+
// are preserved — sending only { formulaText } would wipe them (#17).
1885+
let existingTypeOptions = {};
1886+
try {
1887+
const { field } = await client.resolveField(appId, fieldId);
1888+
existingTypeOptions = field?.typeOptions || {};
1889+
} catch (_) {
1890+
// resolveField failure is non-fatal; proceed with formula update only
1891+
}
18801892
return handlers.update_field_config({
18811893
appId, fieldId,
18821894
fieldType: 'formula',
1883-
typeOptions: { formulaText: formula },
1895+
typeOptions: { ...existingTypeOptions, formulaText: formula },
18841896
debug,
18851897
});
18861898
},
Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
// Tests for round-5 bug fixes (issues #9, #15, #17 — 2026-05-28):
2+
// §1 — #9: parseScaffoldingTables empty visibleTableOrder short-circuits before tableById
3+
// §2 — #15: formulaTextParsed is the live key; fallback chain must check it first
4+
// §3 — #17: resolveField exposes existing typeOptions for update_formula_field merge
5+
6+
import { describe, it } from 'node:test';
7+
import assert from 'node:assert/strict';
8+
import { AirtableClient } from '../src/client.js';
9+
10+
function createMockAuth(schemaResponse) {
11+
return {
12+
getSecretSocketId: () => null,
13+
get: () => ({
14+
ok: true,
15+
status: 200,
16+
json: async () => schemaResponse,
17+
text: async () => JSON.stringify(schemaResponse),
18+
}),
19+
postForm: () => ({ ok: true, status: 200, json: async () => ({}), text: async () => '{}' }),
20+
};
21+
}
22+
23+
// ─── §1 — parseScaffoldingTables ──────────────────────────────────────────────
24+
25+
describe('#9 — parseScaffoldingTables', () => {
26+
const auth = createMockAuth({});
27+
const client = new AirtableClient(auth);
28+
const ps = d => client.parseScaffoldingTables(d);
29+
30+
it('returns tableSchemas when present and non-empty', () => {
31+
const d = { tableSchemas: [{ id: 'tbl1', name: 'A' }] };
32+
assert.deepEqual(ps(d), [{ id: 'tbl1', name: 'A' }]);
33+
});
34+
35+
it('returns tables when tableSchemas absent', () => {
36+
const d = { tables: [{ id: 'tbl1', name: 'B' }] };
37+
assert.deepEqual(ps(d), [{ id: 'tbl1', name: 'B' }]);
38+
});
39+
40+
it('empty tableSchemas [] does NOT short-circuit — falls through to visibleTableOrder', () => {
41+
const d = {
42+
tableSchemas: [],
43+
visibleTableOrder: ['tbl1'],
44+
tableById: { tbl1: { name: 'C' } },
45+
};
46+
const result = ps(d);
47+
assert.equal(result.length, 1);
48+
assert.equal(result[0].id, 'tbl1');
49+
assert.equal(result[0].name, 'C');
50+
});
51+
52+
it('empty visibleTableOrder [] does NOT short-circuit (#9 regression) — falls through to tableById', () => {
53+
const d = {
54+
visibleTableOrder: [], // empty — was returning [] instead of falling through
55+
tableById: {
56+
tbl1: { name: 'Table One' },
57+
tbl2: { name: 'Table Two' },
58+
},
59+
};
60+
const result = ps(d);
61+
assert.equal(result.length, 2, 'should return all tables from tableById, not []');
62+
});
63+
64+
it('non-empty visibleTableOrder maps to tableById entries in order', () => {
65+
const d = {
66+
visibleTableOrder: ['tbl2', 'tbl1'],
67+
tableById: {
68+
tbl1: { name: 'Alpha' },
69+
tbl2: { name: 'Beta' },
70+
},
71+
};
72+
const result = ps(d);
73+
assert.equal(result[0].id, 'tbl2');
74+
assert.equal(result[0].name, 'Beta');
75+
assert.equal(result[1].id, 'tbl1');
76+
assert.equal(result[1].name, 'Alpha');
77+
});
78+
79+
it('no order arrays at all — falls back to Object.values(tableById)', () => {
80+
const d = {
81+
tableById: {
82+
tblA: { name: 'Standalone' },
83+
},
84+
};
85+
const result = ps(d);
86+
assert.equal(result.length, 1);
87+
assert.equal(result[0].name, 'Standalone');
88+
});
89+
90+
it('null / undefined data returns []', () => {
91+
assert.deepEqual(ps(null), []);
92+
assert.deepEqual(ps(undefined), []);
93+
assert.deepEqual(ps({}), []);
94+
});
95+
});
96+
97+
// ─── §2 — formulaTextParsed fallback (#15) ───────────────────────────────────
98+
99+
describe('#15 — resolveField returns formulaTextParsed for formula fields', () => {
100+
it('resolveField exposes typeOptions.formulaTextParsed as the live key', async () => {
101+
const schema = {
102+
data: {
103+
tableSchemas: [{
104+
id: 'tblX',
105+
name: 'T',
106+
columns: [{
107+
id: 'fldFormula',
108+
name: 'Calc',
109+
type: 'formula',
110+
typeOptions: {
111+
formulaTextParsed: 'IF({column_value_fld123}, 1, 0)',
112+
resultType: 'number',
113+
},
114+
}],
115+
views: [],
116+
}],
117+
},
118+
};
119+
const client = new AirtableClient(createMockAuth(schema));
120+
const { field } = await client.resolveField('appX', 'fldFormula');
121+
// The fallback chain in download_formula_field should find this key first
122+
assert.equal(
123+
field.typeOptions?.formulaTextParsed,
124+
'IF({column_value_fld123}, 1, 0)',
125+
);
126+
});
127+
128+
it('resolveField still returns typeOptions.formulaText for older fields', async () => {
129+
const schema = {
130+
data: {
131+
tableSchemas: [{
132+
id: 'tblX',
133+
name: 'T',
134+
columns: [{
135+
id: 'fldOld',
136+
name: 'OldCalc',
137+
type: 'formula',
138+
typeOptions: {
139+
formulaText: 'NOW()',
140+
resultType: 'date',
141+
},
142+
}],
143+
views: [],
144+
}],
145+
},
146+
};
147+
const client = new AirtableClient(createMockAuth(schema));
148+
const { field } = await client.resolveField('appX', 'fldOld');
149+
assert.equal(field.typeOptions?.formulaText, 'NOW()');
150+
});
151+
});
152+
153+
// ─── §3 — resolveField provides existing typeOptions for update_formula_field merge (#17) ──
154+
155+
describe('#17 — resolveField exposes existing typeOptions for merge before formula update', () => {
156+
it('formula field with format typeOptions is accessible via resolveField', async () => {
157+
const schema = {
158+
data: {
159+
tableSchemas: [{
160+
id: 'tblX',
161+
name: 'T',
162+
columns: [{
163+
id: 'fldPct',
164+
name: 'Pct',
165+
type: 'formula',
166+
typeOptions: {
167+
formulaText: 'SUM(1,2)',
168+
resultType: 'percent',
169+
percentV2: true,
170+
precision: 0,
171+
},
172+
}],
173+
views: [],
174+
}],
175+
},
176+
};
177+
const client = new AirtableClient(createMockAuth(schema));
178+
const { field } = await client.resolveField('appX', 'fldPct');
179+
const existing = field?.typeOptions || {};
180+
// Simulates what update_formula_field does: merge existing into new formula update
181+
const merged = { ...existing, formulaText: 'SUM(3,4)' };
182+
assert.equal(merged.resultType, 'percent', 'resultType must survive merge');
183+
assert.equal(merged.precision, 0, 'precision must survive merge');
184+
assert.equal(merged.percentV2, true, 'percentV2 must survive merge');
185+
assert.equal(merged.formulaText, 'SUM(3,4)', 'formulaText must be updated');
186+
});
187+
});

0 commit comments

Comments
 (0)