Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .agents/skills/add-tools/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 9 additions & 7 deletions apps/sim/blocks/blocks/redis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
},
},
Expand Down
34 changes: 19 additions & 15 deletions apps/sim/blocks/blocks/upstash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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: {
Expand Down
152 changes: 150 additions & 2 deletions apps/sim/scripts/check-block-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]
*
Expand All @@ -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'
Expand Down Expand Up @@ -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}`)
Expand All @@ -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',
Expand All @@ -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)
2 changes: 2 additions & 0 deletions apps/sim/tools/firecrawl/map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ export const mapTool: ToolConfig<MapParams, MapResponse> = {

hosting: firecrawlHosting(),

timeoutParamIsDeadline: true,

request: {
method: 'POST',
url: 'https://api.firecrawl.dev/v2/map',
Expand Down
2 changes: 2 additions & 0 deletions apps/sim/tools/firecrawl/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ export const parseTool: InternalToolConfig<ParseParams, ParseResponse> = {

hosting: firecrawlHosting(),

timeoutParamIsDeadline: true,

operation: {
modelInput: {
mode: 'project',
Expand Down
2 changes: 2 additions & 0 deletions apps/sim/tools/http/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ export const requestTool: ToolConfig<RequestParams, RequestResponse> = {
},
},

timeoutParamIsDeadline: true,

request: {
allowSameOrigin: true,
url: (params: RequestParams) => {
Expand Down
3 changes: 2 additions & 1 deletion apps/sim/tools/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ import {
getOwnEnumerableDataEntries,
prepareToolRequest,
projectToolModelInputParams,
readRequestedDeadline,
} from '@/tools/request-transport'
import type {
BYOKProviderId,
Expand Down Expand Up @@ -2735,7 +2736,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())
Expand Down
14 changes: 13 additions & 1 deletion apps/sim/tools/request-transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions apps/sim/tools/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading
Loading