Skip to content

Commit b13c7dc

Browse files
committed
fix(docs): never abort the generator on an unreadable subBlocks array
An unreadable `subBlocks` value used to throw, and with no spread base to fall back on the failure was fatal: the pre-scan recorded it and `generateAllBlockDocs` returned false, so `main` exited 1 and nothing was written at all. Nine shipped blocks already use the non-literal form and are saved only because they happen to spread a base — the first block authored as `subBlocks: myFields` without one would brick `generate-docs` and `docs:check` for the whole repository. The author's reason for aborting was sound: an empty `userSettableParamIds` is indistinguishable from "nothing is settable", which strips every hidden param and publishes a wrong page. So the fix is not to treat the failure as empty — it is to represent UNKNOWN distinctly. `extractBlockSuppliedParamIds` now returns `{ ids, mapperIds, parseError }` with `ids: null` for UNKNOWN, that `null` flows through `BlockConfig.userSettableParamIds`, `getToolInfo` and `extractToolInfo`, and the filter site skips filtering entirely when it sees it — restoring the pre-filter behaviour for that one block instead of killing the run. `getToolInfo`'s default is `null` for the same reason: `[]` as a default silently meant "strip everything". The mapper scan now runs before the subBlocks scan, so a spread-inheriting block keeps its mapper's renames when only the subBlocks scan fails. With nothing left that can record a fatal, the dry pre-scan and its reporting are removed. Also fixes a silent blind spot in the mapper scan: both key regexes require a literal `:`, so a mapper returning a shorthand property (`{ file }`) or writing a computed key (`result['file'] = …`) dropped a real user input from the docs with no warning. Shorthand names are read from the depth-1 comma segments of brace-matched regions, which keeps call argument lists from contributing names. Verified byte-identical output: `scripts/generate-docs.ts` and `tool-metadata:generate` reproduce all 302 generated files unchanged, the credential-shaped hidden params stay stripped, and `check:audits` passes.
1 parent 4782daa commit b13c7dc

2 files changed

Lines changed: 303 additions & 103 deletions

File tree

scripts/generate-docs.test.ts

Lines changed: 141 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import fs from 'fs'
22
import path from 'path'
33
import { describe, expect, it } from 'vitest'
44
import {
5+
extractAllBlockConfigs,
56
extractBlockSuppliedParamIds,
67
extractUserSettableParamIds,
78
getToolInfo,
@@ -248,17 +249,17 @@ describe('hidden params supplied by the block mapper', () => {
248249
fs.readFileSync(path.join(import.meta.dirname, '../apps/sim/blocks/blocks', blockFile), 'utf-8')
249250

250251
const paramNames = async (toolId: string, blockFile: string) => {
251-
const info = await getToolInfo(toolId, extractBlockSuppliedParamIds(blockSource(blockFile)))
252+
const info = await getToolInfo(toolId, extractBlockSuppliedParamIds(blockSource(blockFile)).ids)
252253
return info?.params.map((param) => param.name) ?? []
253254
}
254255

255256
it("keeps Cal.com's required attendee, assembled as result.attendee in the mapper", async () => {
256-
expect(extractBlockSuppliedParamIds(blockSource('calcom.ts'))).toContain('attendee')
257+
expect(extractBlockSuppliedParamIds(blockSource('calcom.ts')).ids).toContain('attendee')
257258
await expect(paramNames('calcom_create_booking', 'calcom.ts')).resolves.toContain('attendee')
258259
})
259260

260261
it("keeps JSM's workspaceId, renamed from assetWorkspaceId in the mapper", async () => {
261-
expect(extractBlockSuppliedParamIds(blockSource('jira_service_management.ts'))).toContain(
262+
expect(extractBlockSuppliedParamIds(blockSource('jira_service_management.ts')).ids).toContain(
262263
'workspaceId'
263264
)
264265
await expect(
@@ -267,7 +268,7 @@ describe('hidden params supplied by the block mapper', () => {
267268
})
268269

269270
it('keeps the file params Textract renames from its document field', async () => {
270-
const ids = extractBlockSuppliedParamIds(blockSource('textract.ts'))
271+
const ids = extractBlockSuppliedParamIds(blockSource('textract.ts')).ids
271272
expect(ids).toContain('file')
272273
expect(ids).toContain('fileBack')
273274
expect(ids).toContain('filePathBack')
@@ -307,7 +308,7 @@ describe('hidden params supplied by the block mapper', () => {
307308
})
308309

309310
it('finds the real mapper past a decoy params key that is not a mapper', () => {
310-
const ids = extractBlockSuppliedParamIds(`
311+
const { ids } = extractBlockSuppliedParamIds(`
311312
subBlocks: [{ id: 'operation' }],
312313
tools: {
313314
config: {
@@ -320,7 +321,7 @@ describe('hidden params supplied by the block mapper', () => {
320321
})
321322

322323
it('reads an async mapper body', () => {
323-
const ids = extractBlockSuppliedParamIds(`
324+
const { ids } = extractBlockSuppliedParamIds(`
324325
subBlocks: [{ id: 'operation' }],
325326
tools: {
326327
config: {
@@ -332,7 +333,7 @@ describe('hidden params supplied by the block mapper', () => {
332333
})
333334

334335
it('ignores a commented-out mapper assignment', () => {
335-
const ids = extractBlockSuppliedParamIds(`
336+
const { ids } = extractBlockSuppliedParamIds(`
336337
subBlocks: [{ id: 'operation' }],
337338
tools: {
338339
config: {
@@ -349,3 +350,136 @@ describe('hidden params supplied by the block mapper', () => {
349350
expect(ids).not.toContain('commentedOut')
350351
})
351352
})
353+
354+
describe('an unreadable subBlocks array', () => {
355+
/**
356+
* Every shape ships in the tree today (`SlackV2Block`, `VideoGeneratorV3Block`, the
357+
* `COMMON_SUBBLOCKS` spread and a backtick id), and each one used to end the run for the
358+
* whole repository unless the block happened to spread a base whose fields were readable.
359+
*/
360+
const unreadable: [string, string][] = [
361+
['a bare identifier', 'subBlocks: myFields,'],
362+
['a helper call', 'subBlocks: withFalAIModelOptions(Base.subBlocks, MODELS),'],
363+
['a spread of an opaque constant', 'subBlocks: [...COMMON_SUBBLOCKS],'],
364+
['a backtick id', 'subBlocks: [{ id: `operation` }],'],
365+
]
366+
367+
it.each(unreadable)('reports %s as UNKNOWN instead of throwing', (_label, source) => {
368+
const supplied = extractBlockSuppliedParamIds(source, 'Widget')
369+
370+
expect(supplied.ids).toBeNull()
371+
expect(supplied.parseError).toMatch(/Widget/)
372+
})
373+
374+
it('still collects the mapper-written ids when only the subBlocks scan failed', () => {
375+
const supplied = extractBlockSuppliedParamIds(
376+
`
377+
subBlocks: myFields,
378+
tools: {
379+
config: {
380+
params: (params) => ({ renamed: params.original }),
381+
},
382+
},
383+
`,
384+
'Widget'
385+
)
386+
387+
expect(supplied.ids).toBeNull()
388+
expect(supplied.mapperIds).toContain('renamed')
389+
})
390+
391+
const syntheticBlock = (name: string, body: string) => `
392+
import type { BlockConfig } from '@/blocks/types'
393+
394+
export const ${name}Block: BlockConfig = {
395+
type: '${name.toLowerCase()}',
396+
name: '${name}',
397+
description: 'A synthetic block',
398+
tools: { access: ['${name.toLowerCase()}_do'] },
399+
${body}
400+
}
401+
`
402+
403+
it('leaves userSettableParamIds UNKNOWN on the block config it produces', () => {
404+
const [unknownConfig] = extractAllBlockConfigs(syntheticBlock('Opaque', 'subBlocks: myFields,'))
405+
expect(unknownConfig.userSettableParamIds).toBeNull()
406+
407+
const [readableConfig] = extractAllBlockConfigs(
408+
syntheticBlock('Readable', `subBlocks: [{ id: 'query' }],`)
409+
)
410+
expect(readableConfig.userSettableParamIds).toEqual(['query'])
411+
})
412+
413+
/**
414+
* The whole point of the UNKNOWN state: `[]` asserts the block supplies nothing and strips
415+
* every hidden param, so the two must not be spelled the same way.
416+
*/
417+
it('disables the hidden-param filter, where an empty list applies it', async () => {
418+
const unfiltered = await getToolInfo('jira_retrieve', null)
419+
expect(unfiltered?.params.map((param) => param.name)).toContain('cloudId')
420+
421+
const filtered = await getToolInfo('jira_retrieve', [])
422+
expect(filtered?.params.map((param) => param.name)).not.toContain('cloudId')
423+
})
424+
425+
it('defaults to not filtering when no param ids are passed at all', async () => {
426+
const info = await getToolInfo('jira_retrieve')
427+
expect(info?.params.map((param) => param.name)).toContain('cloudId')
428+
})
429+
})
430+
431+
describe('mapper param shapes', () => {
432+
const mapperBlock = (body: string) => `
433+
subBlocks: [{ id: 'doc' }],
434+
tools: {
435+
config: {
436+
params: (params) => ${body},
437+
},
438+
},
439+
`
440+
441+
it('reads a shorthand property', () => {
442+
const { ids } = extractBlockSuppliedParamIds(
443+
mapperBlock('{\n const file = params.doc\n return { file }\n}')
444+
)
445+
expect(ids).toContain('doc')
446+
expect(ids).toContain('file')
447+
})
448+
449+
it('reads a shorthand property alongside a spread and a named key', () => {
450+
const { ids } = extractBlockSuppliedParamIds(mapperBlock('({ ...rest, file, other: 1 })'))
451+
expect(ids).toEqual(expect.arrayContaining(['file', 'other']))
452+
expect(ids).not.toContain('rest')
453+
})
454+
455+
it('reads a shorthand property listed after another shorthand', () => {
456+
const { ids } = extractBlockSuppliedParamIds(mapperBlock('({ first, file })'))
457+
expect(ids).toEqual(expect.arrayContaining(['first', 'file']))
458+
})
459+
460+
it('reads a computed string assignment', () => {
461+
const { ids } = extractBlockSuppliedParamIds(
462+
mapperBlock(
463+
"{\n const result: Record<string, unknown> = {}\n result['file'] = params.doc\n return result\n}"
464+
)
465+
)
466+
expect(ids).toContain('file')
467+
})
468+
469+
it('does not take a call argument list for a shorthand property', () => {
470+
const { ids } = extractBlockSuppliedParamIds(
471+
mapperBlock('({ file: buildFile(alpha, beta, gamma) })')
472+
)
473+
expect(ids).toContain('file')
474+
expect(ids).not.toContain('beta')
475+
})
476+
477+
it('ignores a shorthand property inside a comment or a string', () => {
478+
const { ids } = extractBlockSuppliedParamIds(
479+
mapperBlock("({\n // { ghostComment }\n note: '{ ghostString }',\n file,\n})")
480+
)
481+
expect(ids).toContain('file')
482+
expect(ids).not.toContain('ghostComment')
483+
expect(ids).not.toContain('ghostString')
484+
})
485+
})

0 commit comments

Comments
 (0)