Skip to content

Commit b724ead

Browse files
committed
fix(tools): stop provider timeout params from becoming the request deadline
The request transport and the internal-operation path read params.timeout as a millisecond deadline for every tool. Twilio make_call, New Relic NRQL, Apify (3 tools), Daytona (2 tools), and Trigger.dev waitpoint tokens declare their own timeout param in seconds or as a duration, so a 60-second setting aborted the call after 60 ms. A declared timeout param is now the deadline only when the tool sets timeoutParamIsDeadline (http_request, firecrawl_map, firecrawl_parse); callers can still bound tools that declare none. No param ids change. Redis and Upstash coerced params with Number() inside tools.config.tool, which runs at serialization on the serialized params object, turning <Block.output> references into NaN. The coercions now run in tools.config.params. Guardrails: check-block-registry rejects coerced assignments to params inside an inline tools.config.tool; check-tool-param-reachability rejects a method param on a fixed-verb external tool, which the transport would send as the HTTP verb.
1 parent ba485c6 commit b724ead

11 files changed

Lines changed: 205 additions & 27 deletions

File tree

‎.agents/skills/add-tools/SKILL.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,8 @@ fallback, or caller-controlled `_context` authority.
190190

191191
A required `'hidden'` param needs an `oauth` declaration or `hosting.apiKeyParam` to supply it (`bun run check:tool-param-reachability`).
192192

193+
A declared `timeout` param goes to the provider as-is; it becomes Sim's millisecond request deadline only when the tool sets `timeoutParamIsDeadline: true` (`http_request`). A `method` param on a tool with a fixed `request.method` would be sent as the HTTP verb, so the same audit rejects it.
194+
193195
### Parameter Types
194196
- `'string'` - Text values
195197
- `'number'` - Numeric values

‎apps/sim/blocks/blocks/redis.ts‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -255,23 +255,25 @@ export const RedisBlock: BlockConfig = {
255255
'redis_setnx',
256256
],
257257
config: {
258-
tool: (params) => {
258+
tool: (params) => `redis_${params.operation}`,
259+
params: (params) => {
260+
const coerced: Record<string, number> = {}
259261
if (params.ex) {
260-
params.ex = Number(params.ex)
262+
coerced.ex = Number(params.ex)
261263
}
262264
if (params.seconds !== undefined) {
263-
params.seconds = Number(params.seconds)
265+
coerced.seconds = Number(params.seconds)
264266
}
265267
if (params.start !== undefined) {
266-
params.start = Number(params.start)
268+
coerced.start = Number(params.start)
267269
}
268270
if (params.stop !== undefined) {
269-
params.stop = Number(params.stop)
271+
coerced.stop = Number(params.stop)
270272
}
271273
if (params.increment !== undefined) {
272-
params.increment = Number(params.increment)
274+
coerced.increment = Number(params.increment)
273275
}
274-
return `redis_${params.operation}`
276+
return coerced
275277
},
276278
},
277279
},

‎apps/sim/blocks/blocks/upstash.ts‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -255,21 +255,6 @@ export const UpstashBlock: BlockConfig = {
255255
],
256256
config: {
257257
tool: (params) => {
258-
if (params.ex) {
259-
params.ex = Number(params.ex)
260-
}
261-
if (params.seconds !== undefined) {
262-
params.seconds = Number(params.seconds)
263-
}
264-
if (params.start !== undefined) {
265-
params.start = Number(params.start)
266-
}
267-
if (params.stop !== undefined) {
268-
params.stop = Number(params.stop)
269-
}
270-
if (params.increment !== undefined) {
271-
params.increment = Number(params.increment)
272-
}
273258
switch (params.operation) {
274259
case 'get':
275260
return 'upstash_redis_get'
@@ -307,6 +292,25 @@ export const UpstashBlock: BlockConfig = {
307292
throw new Error(`Unknown operation: ${params.operation}`)
308293
}
309294
},
295+
params: (params) => {
296+
const coerced: Record<string, number> = {}
297+
if (params.ex) {
298+
coerced.ex = Number(params.ex)
299+
}
300+
if (params.seconds !== undefined) {
301+
coerced.seconds = Number(params.seconds)
302+
}
303+
if (params.start !== undefined) {
304+
coerced.start = Number(params.start)
305+
}
306+
if (params.stop !== undefined) {
307+
coerced.stop = Number(params.stop)
308+
}
309+
if (params.increment !== undefined) {
310+
coerced.increment = Number(params.increment)
311+
}
312+
return coerced
313+
},
310314
},
311315
},
312316
inputs: {

‎apps/sim/scripts/check-block-registry.ts‎

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
/**
44
* CI check: enforces block-registry invariants that protect the runtime.
55
*
6-
* Two checks run in sequence:
6+
* Checks run in sequence:
77
*
88
* 1. **Subblock ID stability** — diffs the current registry against a base ref
99
* and fails if any subblock ID was removed without a corresponding entry in
@@ -17,6 +17,16 @@
1717
* resolve values via direct lookup; mismatches false-flag fields as missing
1818
* at submit time.
1919
*
20+
* 3. **Selector coercions** — a block's inline `tools.config.tool` may not
21+
* assign a coerced value (`Number(...)`, `parseInt(...)`, `JSON.parse(...)`,
22+
* unary `+`, …) back onto its params. `selectToolId` runs the selector during
23+
* serialization on the object that becomes the serialized block's `params`,
24+
* before `<Block.output>` references resolve, so `params.x = Number(params.x)`
25+
* turns a reference into `NaN`. Coercions belong in `tools.config.params`,
26+
* which the executor runs on resolved inputs.
27+
*
28+
* Integration BlockMeta coverage and sunset `replacedBy` targets are checked too.
29+
*
2030
* Usage:
2131
* bun run apps/sim/scripts/check-block-registry.ts [base-ref]
2232
*
@@ -25,6 +35,9 @@
2535
*/
2636

2737
import { execFileSync } from 'node:child_process'
38+
import { readdirSync, readFileSync } from 'node:fs'
39+
import { join } from 'node:path'
40+
import ts from '@typescript/typescript6'
2841
import { SUBBLOCK_ID_MIGRATIONS } from '@/lib/workflows/migrations/subblock-migrations'
2942
import { getAllBlocks, getBlock, getBlockMeta, getBlockRegistry } from '@/blocks/registry'
3043
import { readBlockRegistryAtRef } from '@/scripts/block-registry-snapshot'
@@ -231,6 +244,92 @@ function checkSunsetReplacedBy(): CheckResult {
231244
return { kind: 'fail', errors }
232245
}
233246

247+
const COERCION_CALLEES = new Set([
248+
'BigInt',
249+
'Boolean',
250+
'JSON.parse',
251+
'Number',
252+
'Number.parseFloat',
253+
'Number.parseInt',
254+
'String',
255+
'parseFloat',
256+
'parseInt',
257+
])
258+
259+
function isCoercion(expression: ts.Expression): boolean {
260+
let inner = expression
261+
while (ts.isParenthesizedExpression(inner) || ts.isAsExpression(inner)) inner = inner.expression
262+
if (ts.isPrefixUnaryExpression(inner)) return inner.operator === ts.SyntaxKind.PlusToken
263+
return ts.isCallExpression(inner) && COERCION_CALLEES.has(inner.expression.getText())
264+
}
265+
266+
function findSelectorCoercions(file: string, source: ts.SourceFile): string[] {
267+
const errors: string[] = []
268+
const visit = (node: ts.Node) => {
269+
const selector =
270+
ts.isPropertyAssignment(node) || ts.isMethodDeclaration(node) ? node : undefined
271+
const config = selector?.parent.parent
272+
if (
273+
selector &&
274+
ts.isIdentifier(selector.name) &&
275+
selector.name.text === 'tool' &&
276+
config &&
277+
ts.isPropertyAssignment(config) &&
278+
ts.isIdentifier(config.name) &&
279+
config.name.text === 'config'
280+
) {
281+
const fn = ts.isMethodDeclaration(selector) ? selector : selector.initializer
282+
const paramsName =
283+
(ts.isArrowFunction(fn) || ts.isFunctionExpression(fn) || ts.isMethodDeclaration(fn)) &&
284+
fn.parameters[0] &&
285+
ts.isIdentifier(fn.parameters[0].name)
286+
? fn.parameters[0].name.text
287+
: undefined
288+
const findAssignments = (inner: ts.Node) => {
289+
if (
290+
ts.isBinaryExpression(inner) &&
291+
inner.operatorToken.kind === ts.SyntaxKind.EqualsToken &&
292+
(ts.isPropertyAccessExpression(inner.left) || ts.isElementAccessExpression(inner.left)) &&
293+
ts.isIdentifier(inner.left.expression) &&
294+
inner.left.expression.text === paramsName &&
295+
isCoercion(inner.right)
296+
) {
297+
const line = source.getLineAndCharacterOfPosition(inner.getStart()).line + 1
298+
errors.push(
299+
`${file}:${line}: \`${inner.getText()}\` coerces a param inside tools.config.tool.\n` +
300+
' → Return the coerced value from tools.config.params instead.'
301+
)
302+
}
303+
ts.forEachChild(inner, findAssignments)
304+
}
305+
if (paramsName) findAssignments(fn)
306+
}
307+
ts.forEachChild(node, visit)
308+
}
309+
visit(source)
310+
return errors
311+
}
312+
313+
function checkSelectorCoercions(): CheckResult {
314+
const blocksDir = join(gitRoot, 'apps/sim/blocks/blocks')
315+
const errors: string[] = []
316+
for (const name of readdirSync(blocksDir)) {
317+
if (!name.endsWith('.ts') || name.endsWith('.test.ts')) continue
318+
const path = join(blocksDir, name)
319+
const source = ts.createSourceFile(
320+
path,
321+
readFileSync(path, 'utf-8'),
322+
ts.ScriptTarget.Latest,
323+
true
324+
)
325+
errors.push(...findSelectorCoercions(`apps/sim/blocks/blocks/${name}`, source))
326+
}
327+
if (errors.length === 0) {
328+
return { kind: 'pass', message: 'Selector coercion check passed' }
329+
}
330+
return { kind: 'fail', errors }
331+
}
332+
234333
function reportResult(label: string, failureHeader: string, result: CheckResult): boolean {
235334
if (result.kind === 'pass') {
236335
console.log(`✓ ${result.message}`)
@@ -252,6 +351,7 @@ const stabilityResult = checkSubblockIdStability()
252351
const canonicalResult = checkCanonicalIdContract()
253352
const metaCoverageResult = checkIntegrationMetaCoverage()
254353
const sunsetResult = checkSunsetReplacedBy()
354+
const selectorCoercionResult = checkSelectorCoercions()
255355

256356
const stabilityOk = reportResult(
257357
'Subblock ID stability check',
@@ -277,4 +377,10 @@ const sunsetOk = reportResult(
277377
sunsetResult
278378
)
279379

280-
process.exit(stabilityOk && canonicalOk && metaCoverageOk && sunsetOk ? 0 : 1)
380+
const selectorCoercionOk = reportResult(
381+
'Selector coercion check',
382+
'tools.config.tool runs at serialization, before references resolve, so a coerced reference becomes NaN.',
383+
selectorCoercionResult
384+
)
385+
386+
process.exit(stabilityOk && canonicalOk && metaCoverageOk && sunsetOk && selectorCoercionOk ? 0 : 1)

‎apps/sim/tools/firecrawl/map.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ export const mapTool: ToolConfig<MapParams, MapResponse> = {
6969

7070
hosting: firecrawlHosting(),
7171

72+
timeoutParamIsDeadline: true,
73+
7274
request: {
7375
method: 'POST',
7476
url: 'https://api.firecrawl.dev/v2/map',

‎apps/sim/tools/firecrawl/parse.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ export const parseTool: InternalToolConfig<ParseParams, ParseResponse> = {
9494

9595
hosting: firecrawlHosting(),
9696

97+
timeoutParamIsDeadline: true,
98+
9799
operation: {
98100
modelInput: {
99101
mode: 'project',

‎apps/sim/tools/http/request.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,8 @@ export const requestTool: ToolConfig<RequestParams, RequestResponse> = {
9696
},
9797
},
9898

99+
timeoutParamIsDeadline: true,
100+
99101
request: {
100102
allowSameOrigin: true,
101103
url: (params: RequestParams) => {

‎apps/sim/tools/index.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ import {
105105
getOwnEnumerableDataEntries,
106106
prepareToolRequest,
107107
projectToolModelInputParams,
108+
readRequestedDeadline,
108109
} from '@/tools/request-transport'
109110
import type {
110111
BYOKProviderId,
@@ -2735,7 +2736,7 @@ async function executeDeclaredInternalOperation({
27352736
} else {
27362737
const handler = await getInternalToolOperationHandler(toolId)
27372738
if (!handler) throw new Error(`No internal operation registered for ${toolId}`)
2738-
const requestedTimeout = Number(params.timeout)
2739+
const requestedTimeout = Number(readRequestedDeadline(tool, params))
27392740
const operationTimeout =
27402741
Number.isFinite(requestedTimeout) && requestedTimeout > 0
27412742
? Math.min(requestedTimeout, getMaxExecutionTimeout())

‎apps/sim/tools/request-transport.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,18 @@ function collectProvenanceSensitiveHeaders(
155155
return [...sensitiveHeaders]
156156
}
157157

158+
/**
159+
* Reads the caller's millisecond deadline from `params.timeout`, unless the tool declares a
160+
* `timeout` param of its own without {@link ToolConfig.timeoutParamIsDeadline}.
161+
*/
162+
export function readRequestedDeadline(
163+
tool: ExecutableToolConfig,
164+
params: Record<string, unknown>
165+
): unknown {
166+
if (tool.params?.timeout && !tool.timeoutParamIsDeadline) return undefined
167+
return params.timeout
168+
}
169+
158170
function formatToolRequest(
159171
tool: ToolConfig,
160172
params: Record<string, any>,
@@ -189,7 +201,7 @@ function formatToolRequest(
189201
}
190202
}
191203

192-
const rawTimeout = params.timeout
204+
const rawTimeout = readRequestedDeadline(tool, params)
193205
const timeout = rawTimeout != null ? Number(rawTimeout) : undefined
194206
const validTimeout =
195207
timeout != null && Number.isFinite(timeout) && timeout > 0

‎apps/sim/tools/types.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,16 @@ export interface ToolConfig<P = any, R = any> {
321321
* Usage is billed according to the pricing config.
322322
*/
323323
hosting?: ToolHostingConfig<P>
324+
325+
/**
326+
* Makes this tool's declared `timeout` param the execution deadline, in milliseconds.
327+
*
328+
* A caller may pass `params.timeout` to bound any tool that does not declare one. A tool that
329+
* declares its own `timeout` param owns that value instead — usually a provider field in
330+
* seconds or a duration string, which as a millisecond deadline would abort the request almost
331+
* immediately — so the executor treats it as the deadline only when this is set.
332+
*/
333+
timeoutParamIsDeadline?: true
324334
}
325335

326336
export interface TableRow {

0 commit comments

Comments
 (0)