fix(tools): stop provider timeout params from becoming the request deadline (#8878)

* 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.

* fix(audits): catch coercions under fallbacks in tools.config.tool, and clarify declared timeout params

* fix(audits): treat every value-deriving expression as a selector coercion

* fix(audits): reject compound assignments and increments on params in selectors

Author: waleedlatif1

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

@@ -190,6 +190,8 @@ fallback, or caller-controlled `_context` authority.
 
 A required `'hidden'` param needs an `oauth` declaration or `hosting.apiKeyParam` to supply it (`bun run check:tool-param-reachability`).
 
+A declared `timeout` param is an ordinary tool input — put it in the request body or URL yourself if the provider expects it; 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.
+
 ### Parameter Types
 - `'string'` - Text values
 - `'number'` - Numeric values

apps/sim/blocks/blocks/redis.ts

@@ -255,23 +255,25 @@ export const RedisBlock: BlockConfig = {
       'redis_setnx',
     ],
     config: {
-      tool: (params) => {
+      tool: (params) => `redis_${params.operation}`,
+      params: (params) => {
+        const coerced: Record<string, number> = {}
         if (params.ex) {
-          params.ex = Number(params.ex)
+          coerced.ex = Number(params.ex)
         }
         if (params.seconds !== undefined) {
-          params.seconds = Number(params.seconds)
+          coerced.seconds = Number(params.seconds)
         }
         if (params.start !== undefined) {
-          params.start = Number(params.start)
+          coerced.start = Number(params.start)
         }
         if (params.stop !== undefined) {
-          params.stop = Number(params.stop)
+          coerced.stop = Number(params.stop)
         }
         if (params.increment !== undefined) {
-          params.increment = Number(params.increment)
+          coerced.increment = Number(params.increment)
         }
-        return `redis_${params.operation}`
+        return coerced
       },
     },
   },

apps/sim/blocks/blocks/upstash.ts

@@ -255,21 +255,6 @@ export const UpstashBlock: BlockConfig = {
     ],
     config: {
       tool: (params) => {
-        if (params.ex) {
-          params.ex = Number(params.ex)
-        }
-        if (params.seconds !== undefined) {
-          params.seconds = Number(params.seconds)
-        }
-        if (params.start !== undefined) {
-          params.start = Number(params.start)
-        }
-        if (params.stop !== undefined) {
-          params.stop = Number(params.stop)
-        }
-        if (params.increment !== undefined) {
-          params.increment = Number(params.increment)
-        }
         switch (params.operation) {
           case 'get':
             return 'upstash_redis_get'
@@ -307,6 +292,25 @@ export const UpstashBlock: BlockConfig = {
             throw new Error(`Unknown operation: ${params.operation}`)
         }
       },
+      params: (params) => {
+        const coerced: Record<string, number> = {}
+        if (params.ex) {
+          coerced.ex = Number(params.ex)
+        }
+        if (params.seconds !== undefined) {
+          coerced.seconds = Number(params.seconds)
+        }
+        if (params.start !== undefined) {
+          coerced.start = Number(params.start)
+        }
+        if (params.stop !== undefined) {
+          coerced.stop = Number(params.stop)
+        }
+        if (params.increment !== undefined) {
+          coerced.increment = Number(params.increment)
+        }
+        return coerced
+      },
     },
   },
   inputs: {

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

@@ -3,7 +3,7 @@
 /**
  * CI check: enforces block-registry invariants that protect the runtime.
  *
- * Two checks run in sequence:
+ * Checks run in sequence:
  *
  * 1. **Subblock ID stability** — diffs the current registry against a base ref
  *    and fails if any subblock ID was removed without a corresponding entry in
@@ -17,6 +17,18 @@
  *    resolve values via direct lookup; mismatches false-flag fields as missing
  *    at submit time.
  *
+ * 3. **Selector coercions** — a block's inline `tools.config.tool` may not
+ *    assign a coerced value (`Number(...)`, `parseInt(...)`, `JSON.parse(...)`,
+ *    `!!x`, unary `+`, a template literal, a comparison, …) back onto its params,
+ *    nor write to a param with a compound assignment (`+=`, `??=`, …) or `++`/`--`.
+ *    `selectToolId` runs the selector during serialization on the object that
+ *    becomes the serialized block's `params`, before `<Block.output>` references
+ *    resolve, so `params.x = Number(params.x)` turns a reference into `NaN`.
+ *    Coercions belong in `tools.config.params`, which the executor runs on
+ *    resolved inputs.
+ *
+ * Integration BlockMeta coverage and sunset `replacedBy` targets are checked too.
+ *
  * Usage:
  *   bun run apps/sim/scripts/check-block-registry.ts [base-ref]
  *
@@ -25,6 +37,9 @@
  */
 
 import { execFileSync } from 'node:child_process'
+import { readdirSync, readFileSync } from 'node:fs'
+import { join } from 'node:path'
+import ts from '@typescript/typescript6'
 import { SUBBLOCK_ID_MIGRATIONS } from '@/lib/workflows/migrations/subblock-migrations'
 import { getAllBlocks, getBlock, getBlockMeta, getBlockRegistry } from '@/blocks/registry'
 import { readBlockRegistryAtRef } from '@/scripts/block-registry-snapshot'
@@ -231,6 +246,132 @@ function checkSunsetReplacedBy(): CheckResult {
   return { kind: 'fail', errors }
 }
 
+const COERCION_CALLEES = new Set([
+  'BigInt',
+  'Boolean',
+  'JSON.parse',
+  'Number',
+  'Number.parseFloat',
+  'Number.parseInt',
+  'String',
+  'parseFloat',
+  'parseInt',
+])
+
+const COERCING_UNARY = new Set([
+  ts.SyntaxKind.PlusToken,
+  ts.SyntaxKind.MinusToken,
+  ts.SyntaxKind.ExclamationToken,
+  ts.SyntaxKind.TildeToken,
+])
+
+const COMPARISON_OPERATORS = new Set([
+  ts.SyntaxKind.EqualsEqualsToken,
+  ts.SyntaxKind.EqualsEqualsEqualsToken,
+  ts.SyntaxKind.ExclamationEqualsToken,
+  ts.SyntaxKind.ExclamationEqualsEqualsToken,
+  ts.SyntaxKind.LessThanToken,
+  ts.SyntaxKind.LessThanEqualsToken,
+  ts.SyntaxKind.GreaterThanToken,
+  ts.SyntaxKind.GreaterThanEqualsToken,
+])
+
+/**
+ * Whether `expression` derives a new value anywhere in it — a coercing call, a unary `+ - ! ~`,
+ * a template literal with substitutions, or a comparison — including under `??`, `||`, or a
+ * conditional. Any of these turns an unresolved `<Block.output>` reference into a fixed value.
+ */
+function isCoercion(expression: ts.Node): boolean {
+  if (ts.isPrefixUnaryExpression(expression) && COERCING_UNARY.has(expression.operator)) return true
+  if (ts.isCallExpression(expression) && COERCION_CALLEES.has(expression.expression.getText())) {
+    return true
+  }
+  if (ts.isTemplateExpression(expression)) return true
+  if (
+    ts.isBinaryExpression(expression) &&
+    COMPARISON_OPERATORS.has(expression.operatorToken.kind)
+  ) {
+    return true
+  }
+  return ts.forEachChild(expression, (child) => (isCoercion(child) ? true : undefined)) ?? false
+}
+
+function findSelectorCoercions(file: string, source: ts.SourceFile): string[] {
+  const errors: string[] = []
+  const visit = (node: ts.Node) => {
+    const selector =
+      ts.isPropertyAssignment(node) || ts.isMethodDeclaration(node) ? node : undefined
+    const config = selector?.parent.parent
+    if (
+      selector &&
+      ts.isIdentifier(selector.name) &&
+      selector.name.text === 'tool' &&
+      config &&
+      ts.isPropertyAssignment(config) &&
+      ts.isIdentifier(config.name) &&
+      config.name.text === 'config'
+    ) {
+      const fn = ts.isMethodDeclaration(selector) ? selector : selector.initializer
+      const paramsName =
+        (ts.isArrowFunction(fn) || ts.isFunctionExpression(fn) || ts.isMethodDeclaration(fn)) &&
+        fn.parameters[0] &&
+        ts.isIdentifier(fn.parameters[0].name)
+          ? fn.parameters[0].name.text
+          : undefined
+      const isParamsMember = (target: ts.Expression) =>
+        (ts.isPropertyAccessExpression(target) || ts.isElementAccessExpression(target)) &&
+        ts.isIdentifier(target.expression) &&
+        target.expression.text === paramsName
+      const findAssignments = (inner: ts.Node) => {
+        const operator = ts.isBinaryExpression(inner) ? inner.operatorToken.kind : undefined
+        const derivedWrite =
+          (ts.isBinaryExpression(inner) &&
+            isParamsMember(inner.left) &&
+            ((operator === ts.SyntaxKind.EqualsToken && isCoercion(inner.right)) ||
+              (operator !== undefined &&
+                operator >= ts.SyntaxKind.FirstCompoundAssignment &&
+                operator <= ts.SyntaxKind.LastCompoundAssignment))) ||
+          ((ts.isPrefixUnaryExpression(inner) || ts.isPostfixUnaryExpression(inner)) &&
+            (inner.operator === ts.SyntaxKind.PlusPlusToken ||
+              inner.operator === ts.SyntaxKind.MinusMinusToken) &&
+            isParamsMember(inner.operand))
+        if (derivedWrite) {
+          const line = source.getLineAndCharacterOfPosition(inner.getStart()).line + 1
+          errors.push(
+            `${file}:${line}: \`${inner.getText()}\` coerces a param inside tools.config.tool.\n` +
+              '  → Return the coerced value from tools.config.params instead.'
+          )
+        }
+        ts.forEachChild(inner, findAssignments)
+      }
+      if (paramsName) findAssignments(fn)
+    }
+    ts.forEachChild(node, visit)
+  }
+  visit(source)
+  return errors
+}
+
+function checkSelectorCoercions(): CheckResult {
+  const blocksDir = join(gitRoot, 'apps/sim/blocks/blocks')
+  const errors: string[] = []
+  for (const name of readdirSync(blocksDir)) {
+    if (!name.endsWith('.ts') || name.endsWith('.test.ts')) continue
+    const path = join(blocksDir, name)
+    const source = ts.createSourceFile(
+      path,
+      readFileSync(path, 'utf-8'),
+      ts.ScriptTarget.Latest,
+      true
+    )
+    errors.push(...findSelectorCoercions(`apps/sim/blocks/blocks/${name}`, source))
+  }
+  if (errors.length === 0) {
+    return { kind: 'pass', message: 'Selector coercion check passed' }
+  }
+  return { kind: 'fail', errors }
+}
+
 function reportResult(label: string, failureHeader: string, result: CheckResult): boolean {
   if (result.kind === 'pass') {
     console.log(`✓ ${result.message}`)
@@ -252,6 +393,7 @@ const stabilityResult = checkSubblockIdStability()
 const canonicalResult = checkCanonicalIdContract()
 const metaCoverageResult = checkIntegrationMetaCoverage()
 const sunsetResult = checkSunsetReplacedBy()
+const selectorCoercionResult = checkSelectorCoercions()
 
 const stabilityOk = reportResult(
   'Subblock ID stability check',
@@ -277,4 +419,10 @@ const sunsetOk = reportResult(
   sunsetResult
 )
 
-process.exit(stabilityOk && canonicalOk && metaCoverageOk && sunsetOk ? 0 : 1)
+const selectorCoercionOk = reportResult(
+  'Selector coercion check',
+  'tools.config.tool runs at serialization, before references resolve, so a coerced reference becomes NaN.',
+  selectorCoercionResult
+)
+
+process.exit(stabilityOk && canonicalOk && metaCoverageOk && sunsetOk && selectorCoercionOk ? 0 : 1)

apps/sim/tools/firecrawl/map.ts

@@ -69,6 +69,8 @@ export const mapTool: ToolConfig<MapParams, MapResponse> = {
 
   hosting: firecrawlHosting(),
 
+  timeoutParamIsDeadline: true,
+
   request: {
     method: 'POST',
     url: 'https://api.firecrawl.dev/v2/map',

apps/sim/tools/firecrawl/parse.ts

@@ -94,6 +94,8 @@ export const parseTool: InternalToolConfig<ParseParams, ParseResponse> = {
 
   hosting: firecrawlHosting(),
 
+  timeoutParamIsDeadline: true,
+
   operation: {
     modelInput: {
       mode: 'project',

apps/sim/tools/http/request.ts

@@ -96,6 +96,8 @@ export const requestTool: ToolConfig<RequestParams, RequestResponse> = {
     },
   },
 
+  timeoutParamIsDeadline: true,
+
   request: {
     allowSameOrigin: true,
     url: (params: RequestParams) => {

apps/sim/tools/index.ts

@@ -113,6 +113,7 @@ import {
   getOwnEnumerableDataEntries,
   prepareToolRequest,
   projectToolModelInputParams,
+  readRequestedDeadline,
 } from '@/tools/request-transport'
 import type {
   BYOKProviderId,
@@ -2749,7 +2750,7 @@ async function executeDeclaredInternalOperation({
   } else {
     const handler = await getInternalToolOperationHandler(toolId)
     if (!handler) throw new Error(`No internal operation registered for ${toolId}`)
-    const requestedTimeout = Number(params.timeout)
+    const requestedTimeout = Number(readRequestedDeadline(tool, params))
     const operationTimeout =
       Number.isFinite(requestedTimeout) && requestedTimeout > 0
         ? Math.min(requestedTimeout, getMaxExecutionTimeout())

apps/sim/tools/request-transport.ts

@@ -155,6 +155,18 @@ function collectProvenanceSensitiveHeaders(
   return [...sensitiveHeaders]
 }
 
+/**
+ * Reads the caller's millisecond deadline from `params.timeout`, unless the tool declares a
+ * `timeout` param of its own without {@link ToolConfig.timeoutParamIsDeadline}.
+ */
+export function readRequestedDeadline(
+  tool: ExecutableToolConfig,
+  params: Record<string, unknown>
+): unknown {
+  if (tool.params?.timeout && !tool.timeoutParamIsDeadline) return undefined
+  return params.timeout
+}
+
 function formatToolRequest(
   tool: ToolConfig,
   params: Record<string, any>,
@@ -189,7 +201,7 @@ function formatToolRequest(
     }
   }
 
-  const rawTimeout = params.timeout
+  const rawTimeout = readRequestedDeadline(tool, params)
   const timeout = rawTimeout != null ? Number(rawTimeout) : undefined
   const validTimeout =
     timeout != null && Number.isFinite(timeout) && timeout > 0

apps/sim/tools/types.ts

@@ -321,6 +321,16 @@ export interface ToolConfig<P = any, R = any> {
    * Usage is billed according to the pricing config.
    */
   hosting?: ToolHostingConfig<P>
+
+  /**
+   * Makes this tool's declared `timeout` param the execution deadline, in milliseconds.
+   *
+   * A caller may pass `params.timeout` to bound any tool that does not declare one. A tool that
+   * declares its own `timeout` param owns that value instead — usually a provider field in
+   * seconds or a duration string, which as a millisecond deadline would abort the request almost
+   * immediately — so the executor treats it as the deadline only when this is set.
+   */
+  timeoutParamIsDeadline?: true
 }
 
 export interface TableRow {