Skip to content

Commit 39bb39d

Browse files
icecrasher321claude
andcommitted
fix(executor): step over comments instead of reading them as tokens
Treating a comment end as the answer was too blunt: `/* c */ if (x)` is a control-flow head, and calling it a method left a statement-position regex scanned as division — the failure the comment guard was added to prevent, moved one shape over. The scan now steps back over comments to the token that precedes them, so the dot in `p./* c */catch(fn)` is still found and a keyword after a comment is still a head. A condition's environment read is placed the same way references are: an occurrence inside a string, a template, or a regex is text and mounts nothing, while any executable read — whatever its shape — keeps the map. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 82a80c3 commit 39bb39d

2 files changed

Lines changed: 90 additions & 11 deletions

File tree

apps/sim/executor/variables/resolver.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,33 @@ describe('VariableResolver function block inputs', () => {
280280
expect(resolvedConditions[1].value).toContain('environmentVariables.OPENAI_API_KEY')
281281
})
282282

283+
it('counts an environment read only where it can execute', async () => {
284+
const { ctx, resolver } = createResolver()
285+
const conditionBlock = createBlock('condition', 'Condition', BlockType.CONDITION)
286+
const conditions = [
287+
// Reads, in the shapes a pattern would have to anticipate.
288+
{ id: 'c1', title: 'if', value: `environmentVariables?.FLAG === 'on'` },
289+
{ id: 'c2', title: 'else if', value: 'Object.keys(environmentVariables).length > 0' },
290+
// Mentions: text, not code.
291+
{ id: 'c3', title: 'else if', value: `'environmentVariables.FLAG' === 'x'` },
292+
{ id: 'c4', title: 'else if', value: '`environmentVariables` === "x"' },
293+
{ id: 'c5', title: 'else if', value: `/environmentVariables/.test('x')` },
294+
]
295+
296+
const result = await resolver.resolveInputs(
297+
ctx,
298+
conditionBlock.id,
299+
{ conditions: JSON.stringify(conditions) },
300+
conditionBlock
301+
)
302+
303+
expect(
304+
(result.conditions as Array<Record<string, unknown>>).map(
305+
(condition) => condition._readsEnvironmentVariables
306+
)
307+
).toEqual([true, true, false, false, false])
308+
})
309+
283310
it('preserves legacy condition outcomes end to end through the boundary compiler', async () => {
284311
const environmentVariables = {
285312
API_KEY: 'token',
@@ -947,6 +974,28 @@ describe('VariableResolver function block inputs', () => {
947974
expect(code).toContain(`Number('' + JSON.stringify(globalThis["__blockRef_1"]) + '')`)
948975
})
949976

977+
it('steps over a comment rather than reading it as the preceding token', async () => {
978+
const { block, ctx, resolver } = createResolver('javascript')
979+
980+
const result = await resolver.resolveInputsForFunctionBlock(
981+
ctx,
982+
'function',
983+
{
984+
code: [
985+
`/* lead */ if (params.a) /['"]/.test('<producer.result>')`,
986+
`const n = params.p./* mid */catch(() => 0) / 2 + Number('<producer.result>')`,
987+
].join('\n'),
988+
},
989+
block
990+
)
991+
992+
// A comment before a control-flow keyword leaves it a head; one hiding a property dot
993+
// still leaves the call a call.
994+
const code = result.resolvedInputs.code as string
995+
expect(code).toContain(`.test('' + JSON.stringify(globalThis["__blockRef_0"]) + '')`)
996+
expect(code).toContain(`Number('' + JSON.stringify(globalThis["__blockRef_1"]) + '')`)
997+
})
998+
950999
it('does not read a method named after a keyword as a control-flow head', async () => {
9511000
const { block, ctx, resolver } = createResolver('javascript')
9521001

apps/sim/executor/variables/resolver.ts

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ import type { SerializedBlock, SerializedWorkflow } from '@/serializer/types'
4646
export const CONDITION_READS_ENVIRONMENT_KEY = '_readsEnvironmentVariables'
4747

4848
/** The sandbox global holding the run's secrets, in whatever shape an expression reaches it. */
49-
const ENVIRONMENT_MAP_IDENTIFIER = /\benvironmentVariables\b/
49+
const ENVIRONMENT_MAP_IDENTIFIER = /\benvironmentVariables\b/g
5050

5151
/** Key used to carry pre-resolved context variables through the inputs map. */
5252
export const FUNCTION_BLOCK_CONTEXT_VARS_KEY = '_runtimeContextVars'
@@ -362,7 +362,7 @@ export class VariableResolver {
362362
// the environment map is indistinguishable from one that merely quotes trigger
363363
// data containing the word, and the handler decides what to mount from this.
364364
[CONDITION_READS_ENVIRONMENT_KEY]:
365-
typeof value === 'string' && ENVIRONMENT_MAP_IDENTIFIER.test(value),
365+
typeof value === 'string' && this.readsEnvironmentMap(value),
366366
value:
367367
typeof value === 'string'
368368
? await this.resolveTemplateWithoutConditionFormatting(
@@ -1181,6 +1181,28 @@ export class VariableResolver {
11811181
)
11821182
}
11831183

1184+
/**
1185+
* Whether a Condition expression reads the run's secrets off the environment map.
1186+
*
1187+
* The name is only a read where it can execute, so each occurrence is placed with the same
1188+
* scanner that decides how references are spliced — a mention inside a string, a template,
1189+
* or a regex is text and mounts nothing. No shape of the read itself is assumed:
1190+
* `environmentVariables?.FLAG` and `Object.keys(environmentVariables)` both count, because
1191+
* narrowing an expression that does reach the map would route the run down a branch the
1192+
* author did not write, silently, while admitting one too many only costs the narrowing.
1193+
*/
1194+
private readsEnvironmentMap(expression: string): boolean {
1195+
ENVIRONMENT_MAP_IDENTIFIER.lastIndex = 0
1196+
let match = ENVIRONMENT_MAP_IDENTIFIER.exec(expression)
1197+
while (match !== null) {
1198+
if (this.getCodeStringQuoteContext(expression, match.index, 'javascript') === null) {
1199+
return true
1200+
}
1201+
match = ENVIRONMENT_MAP_IDENTIFIER.exec(expression)
1202+
}
1203+
return false
1204+
}
1205+
11841206
/**
11851207
* Whether the `(` at this index opens a control-flow head rather than a value.
11861208
*
@@ -1204,19 +1226,27 @@ export class VariableResolver {
12041226

12051227
// `p.catch(fn)` is a method call whose name happens to be a keyword, and what follows its
12061228
// `)` is an operator, not a statement. A control-flow head can never be a property access,
1207-
// and a comment can hide the dot (`p./* c */catch(fn)`), so a comment ending here is read
1208-
// as the method call it usually is: the wrong guess there costs a division scanned as a
1209-
// regex, while this way it costs nothing a regex-free line would notice.
1210-
let before = start
1211-
while (before > 0 && WHITESPACE_CHAR.test(template[before - 1])) {
1212-
before--
1213-
}
1214-
if (template[before - 1] === '/' && template[before - 2] === '*') {
1215-
return false
1229+
// and a comment can stand between the two (`p./* c */catch(fn)`), so a comment is stepped
1230+
// over rather than treated as an answer — `/* c */ if (x)` is still a head.
1231+
let before = this.skipWhitespaceBackward(template, start)
1232+
while (template[before - 1] === '/' && template[before - 2] === '*') {
1233+
const opening = template.lastIndexOf('/*', before - 2)
1234+
if (opening < 0) {
1235+
return true
1236+
}
1237+
before = this.skipWhitespaceBackward(template, opening)
12161238
}
12171239
return template[before - 1] !== '.'
12181240
}
12191241

1242+
private skipWhitespaceBackward(template: string, index: number): number {
1243+
let cursor = index
1244+
while (cursor > 0 && WHITESPACE_CHAR.test(template[cursor - 1])) {
1245+
cursor--
1246+
}
1247+
return cursor
1248+
}
1249+
12201250
private matchesKeywordAt(template: string, index: number, keyword: string): boolean {
12211251
if (!template.startsWith(keyword, index)) {
12221252
return false

0 commit comments

Comments
 (0)