Skip to content

Commit 102dbd6

Browse files
committed
fix(docs): report a spread-only subBlocks array as unknown, not empty
extractUserSettableParamIds answered [] for a subBlocks array whose every element spreads a fields array it cannot follow (NotionV2Block's `[...NotionBlock.subBlocks, ...getTrigger(x).subBlocks]`). [] asserts the block supplies nothing, so the hidden-param filter stripped every hidden param from every tool the block owns - silently, with no parseError and so no warning. That is the exact false-drop the null UNKNOWN state exists to prevent. Return null in that case and propagate it: extractBlockSuppliedParamIds no longer folds it into [], and the block pass no longer collapses it with `supplied.ids ?? []`. A config-level spread base still narrows the filter to its readable fields plus the mapper's renames; with no base the filter is switched off. An array with at least one inline id, a genuinely empty array, and the existing throw/warn paths are unchanged - all 8 warned blocks warn identically and every generated page is byte-identical. Also pin the hidden-param filter on extractToolInfo's source-parsing path, which had no coverage at all: deleting it outright left the suite green.
1 parent 8d828cc commit 102dbd6

2 files changed

Lines changed: 127 additions & 13 deletions

File tree

scripts/generate-docs.test.ts

Lines changed: 88 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,56 @@ describe('documentation input parameter parsing', () => {
111111
])
112112
})
113113

114+
/**
115+
* Pins the hidden-param filter on the source-parsing path in {@link extractToolInfo}. Tools
116+
* with an entry in tool-metadata.ts never reach it, so it must be driven with synthetic
117+
* source rather than through `getToolInfo`. Without this the filter can be deleted outright
118+
* and the whole suite stays green.
119+
*/
120+
describe('the hidden-param filter on the source-parsing path', () => {
121+
const source = `
122+
export const exampleTool = {
123+
id: 'example_send',
124+
description: 'Send an example',
125+
params: {
126+
message: {
127+
type: 'string',
128+
required: true,
129+
description: 'The message',
130+
},
131+
apiKey: {
132+
type: 'string',
133+
required: true,
134+
visibility: 'hidden',
135+
description: 'The API key the block injects',
136+
},
137+
instanceUrl: {
138+
type: 'string',
139+
required: true,
140+
visibility: 'hidden',
141+
description: 'Resolved from the credential',
142+
},
143+
},
144+
outputs: {},
145+
}
146+
`
147+
148+
const paramNames = (ids: ReadonlySet<string> | null) =>
149+
extractToolInfo('example_send', source, '', '', '', ids)?.params.map(({ name }) => name)
150+
151+
it('drops a hidden param the block does not supply', () => {
152+
expect(paramNames(new Set(['message']))).toEqual(['message'])
153+
})
154+
155+
it('keeps a hidden param the block exposes as its own field', () => {
156+
expect(paramNames(new Set(['message', 'apiKey']))).toEqual(['message', 'apiKey'])
157+
})
158+
159+
it('keeps every param when the block-supplied set is UNKNOWN', () => {
160+
expect(paramNames(null)).toEqual(['message', 'apiKey', 'instanceUrl'])
161+
})
162+
})
163+
114164
it('stops at legacy request metadata after a comment', () => {
115165
const tool = extractToolInfo(
116166
'example_send',
@@ -275,18 +325,42 @@ describe('subBlock param extraction', () => {
275325
expect(ids).toContain('inputFormat')
276326
})
277327

278-
it('returns no ids for blocks whose subBlocks array holds only spreads', () => {
328+
it('returns no ids for blocks whose subBlocks array is genuinely empty', () => {
329+
for (const blockFile of ['chat_trigger.ts', 'manual_trigger.ts']) {
330+
expect(extractUserSettableParamIds(blockSource(blockFile))).toEqual([])
331+
}
332+
})
333+
334+
/**
335+
* The spreads name fields arrays this scanner never follows, so what the block supplies is
336+
* UNKNOWN. Answering `[]` asserts the block supplies nothing, and the hidden-param filter
337+
* reads that as licence to strip every hidden param from every tool the block owns — silently,
338+
* with no `parseError` and so no warning. `NotionV2Block` has exactly this shape and is only
339+
* harmless today because no `notion_*` tool carries a hidden param besides `accessToken`.
340+
*/
341+
it('reports a subBlocks array of only unfollowable spreads as UNKNOWN, not empty', () => {
279342
for (const blockFile of [
280343
'imap.ts',
281-
'chat_trigger.ts',
282344
'generic_webhook.ts',
283-
'manual_trigger.ts',
284345
'circleback.ts',
285346
'rss.ts',
286347
'sim_workspace_event.ts',
287348
]) {
288-
expect(extractUserSettableParamIds(blockSource(blockFile))).toEqual([])
349+
expect(extractUserSettableParamIds(blockSource(blockFile))).toBeNull()
289350
}
351+
352+
const supplied = extractBlockSuppliedParamIds(
353+
`subBlocks: [...NotionBlock.subBlocks],`,
354+
'NotionV2'
355+
)
356+
expect(supplied.ids).toBeNull()
357+
expect(supplied.parseError).toBeNull()
358+
})
359+
360+
it('still returns the inline ids when a spread sits alongside them', () => {
361+
expect(
362+
extractUserSettableParamIds(`subBlocks: [...Base.subBlocks, { id: 'operation' }],`)
363+
).toEqual(['operation'])
290364
})
291365

292366
it('ignores an id inside a comment or string literal at the top level of a subBlock', () => {
@@ -342,20 +416,26 @@ describe('subBlock param extraction', () => {
342416
).toThrow(/SlackV2: subBlocks/)
343417
})
344418

345-
it('still returns no ids for an array of nothing but named fields arrays', () => {
419+
/**
420+
* The elements name fields arrays, so the array parsed fine and there is nothing to warn
421+
* about — but this scanner never follows a spread, so the fields are UNKNOWN rather than
422+
* absent. `[]` would be a confident wrong answer that strips every hidden param the block's
423+
* tools declare.
424+
*/
425+
it('reports an array of nothing but named fields arrays as UNKNOWN', () => {
346426
expect(
347427
extractUserSettableParamIds(
348428
`subBlocks: [\n ...NotionBlock.subBlocks,\n ...getTrigger('notion_page_created').subBlocks,\n],`,
349429
'NotionV2'
350430
)
351-
).toEqual([])
431+
).toBeNull()
352432

353433
expect(
354434
extractUserSettableParamIds(
355435
`subBlocks: [\n ...LinearBlock.subBlocks.filter((sb) => !sb.id?.startsWith('webhookSecret')),\n],`,
356436
'LinearV2'
357437
)
358-
).toEqual([])
438+
).toBeNull()
359439
})
360440

361441
it('does not fail a block that overrides a spread subBlock instead of naming an id', () => {
@@ -364,7 +444,7 @@ describe('subBlock param extraction', () => {
364444
`subBlocks: [\n ...Base.subBlocks.map((sb) => (sb.id === 'x' ? { ...sb, required: true } : sb)),\n],`,
365445
'OverridingV2'
366446
)
367-
).toEqual([])
447+
).toBeNull()
368448
})
369449

370450
it('leaves a readable array alone even when it also spreads an opaque helper', () => {

scripts/generate-docs.ts

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -703,8 +703,16 @@ const subBlockParseWarnings = new Set<string>()
703703
* Brace matching runs on a blanked copy so braces inside string literals and comments
704704
* cannot skew it; only depth-1 properties of each subBlock are read, so `id` fields on
705705
* nested `options`/`condition` objects are never mistaken for the subBlock's own id.
706+
*
707+
* Returns `null` for UNKNOWN — an array whose elements are all spreads of fields arrays this
708+
* scanner cannot follow (`...NotionBlock.subBlocks`, `...getTrigger('x').subBlocks`). `[]` is
709+
* reserved for a block that genuinely exposes no fields, because `[]` strips every hidden param
710+
* from the page. Throws {@link SubBlockParseError} when the array is there but unreadable.
706711
*/
707-
export function extractUserSettableParamIds(blockContent: string, blockName = 'block'): string[] {
712+
export function extractUserSettableParamIds(
713+
blockContent: string,
714+
blockName = 'block'
715+
): string[] | null {
708716
const scannable = blankStringsAndComments(blockContent)
709717
const keyMatch = /\bsubBlocks\s*:/.exec(scannable)
710718
if (!keyMatch) return []
@@ -831,10 +839,11 @@ export function extractUserSettableParamIds(blockContent: string, blockName = 'b
831839
* bare helper call (`...getSlackV2ActionSubBlocks()`) hides whatever fields the helper builds,
832840
* and used to yield a silent empty array indistinguishable from a spread-only block.
833841
*/
834-
const opaque = elementHeads
842+
const segments = elementHeads
835843
.split(',')
836844
.map((segment) => segment.trim())
837-
.filter((segment) => segment.length > 0 && !segment.includes('.subBlocks'))
845+
.filter((segment) => segment.length > 0)
846+
const opaque = segments.filter((segment) => !segment.includes('.subBlocks'))
838847
if (opaque.length > 0) {
839848
throw new SubBlockParseError(
840849
`${blockName}: subBlocks array yielded no ids and element${
@@ -843,6 +852,15 @@ export function extractUserSettableParamIds(blockContent: string, blockName = 'b
843852
)
844853
}
845854

855+
/**
856+
* Every element named a fields array this scanner cannot follow, so the block's fields are
857+
* UNKNOWN, not empty. Returning `[]` here would assert the block supplies nothing and strip
858+
* every hidden param from its tools' Input tables with no warning — the silent false-drop the
859+
* `null` state exists to prevent. Only a genuinely empty array (`subBlocks: []`) reaches the
860+
* `[]` below.
861+
*/
862+
if (segments.length > 0) return null
863+
846864
return []
847865
}
848866

@@ -1026,7 +1044,9 @@ export function extractMapperWrittenParamIds(blockContent: string): string[] {
10261044
/** What {@link extractBlockSuppliedParamIds} could and could not read off a block. */
10271045
export interface BlockSuppliedParams {
10281046
/**
1029-
* Every param the block supplies, or `null` when its `subBlocks` array could not be read.
1047+
* Every param the block supplies, or `null` when what its `subBlocks` array contributes is
1048+
* unknown — either the array could not be read (`parseError` set) or it holds only spreads of
1049+
* fields arrays this scanner cannot follow (`parseError` null).
10301050
*
10311051
* `null` is UNKNOWN and is deliberately distinct from `[]`: an empty array asserts the block
10321052
* supplies nothing, which strips every hidden param from the page, while `null` says the scan
@@ -1060,6 +1080,7 @@ export function extractBlockSuppliedParamIds(
10601080

10611081
try {
10621082
const settableIds = extractUserSettableParamIds(blockContent, blockName)
1083+
if (settableIds === null) return { ids: null, mapperIds, parseError: null }
10631084
return { ids: [...new Set([...settableIds, ...mapperIds])], mapperIds, parseError: null }
10641085
} catch (error) {
10651086
if (!(error instanceof SubBlockParseError)) throw error
@@ -1806,8 +1827,21 @@ function extractBlockConfigFromContent(
18061827
userSettableParamIds = fallback
18071828
} else if (baseParamIds === null) {
18081829
userSettableParamIds = null
1830+
} else if (supplied.ids === null) {
1831+
/**
1832+
* The block's `subBlocks` array holds only spreads of fields arrays this scanner cannot
1833+
* follow. A config-level spread base still contributes its readable fields, so the filter
1834+
* stays on against those plus the mapper's renames; with no base there is nothing to
1835+
* filter against and the filter is switched off. No warning: unlike the `parseError`
1836+
* cases the array itself parsed fine, and every field it names is documented through the
1837+
* spread source's own page.
1838+
*/
1839+
userSettableParamIds =
1840+
baseSettableParamIds.length > 0
1841+
? [...new Set([...baseSettableParamIds, ...supplied.mapperIds])]
1842+
: null
18091843
} else {
1810-
userSettableParamIds = [...new Set([...(supplied.ids ?? []), ...baseSettableParamIds])]
1844+
userSettableParamIds = [...new Set([...supplied.ids, ...baseSettableParamIds])]
18111845
}
18121846
const docsLink =
18131847
extractStringPropertyFromContent(blockContent, 'docsLink', true) ||

0 commit comments

Comments
 (0)