Skip to content

Commit 2783606

Browse files
author
octo-patch
committed
fix: address Cursor and Greptile bot review comments
- Pass preResolvedContextVariables through to shellEnvs for Shell language (Cursor: shell loses pre-resolved block refs, executes against undefined vars) - Remove duplicate CodeExecutionOutput interface declaration (Cursor + Greptile: dead duplicate declaration in tools/function/types.ts) - Deduplicate identical block references in resolveCodeWithContextVars so the same <BlockA.result> reused multiple times shares one __blockRef_N slot (Greptile P2: avoid duplicating large payloads across the wire)
1 parent 6734052 commit 2783606

3 files changed

Lines changed: 21 additions & 11 deletions

File tree

apps/sim/app/api/function/execute/route.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,10 @@ export async function POST(req: NextRequest) {
744744
// For shell, env vars are injected as OS env vars via shellEnvs.
745745
// Replace {{VAR}} placeholders with $VAR so the shell can access them natively.
746746
resolvedCode = code.replace(/\{\{([A-Za-z_][A-Za-z0-9_]*)\}\}/g, '$$$1')
747+
// Carry pre-resolved block output variables (e.g. __blockRef_N) so they can be
748+
// injected as shell env vars below. The executor replaces block references in the
749+
// code with these names, so the values must be present at runtime.
750+
contextVariables = { ...preResolvedContextVariables }
747751
} else {
748752
const codeResolution = resolveCodeVariables(
749753
code,

apps/sim/executor/variables/resolver.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,22 +237,35 @@ export class VariableResolver {
237237

238238
let replacementError: Error | null = null
239239

240+
const blockRefByMatch = new Map<string, string>()
241+
240242
let result = replaceValidReferences(template, (match) => {
241243
if (replacementError) return match
242244

243245
try {
244-
const resolved = this.resolveReference(match, resolutionContext)
245-
if (resolved === undefined) return match
246+
if (this.blockResolver.canResolve(match)) {
247+
// Deduplicate: identical references in the same template share a single
248+
// accumulator slot so we do not duplicate large payloads.
249+
const existing = blockRefByMatch.get(match)
250+
if (existing !== undefined) return existing
246251

247-
const effectiveValue = resolved === RESOLVED_EMPTY ? null : resolved
252+
const resolved = this.resolveReference(match, resolutionContext)
253+
if (resolved === undefined) return match
254+
255+
const effectiveValue = resolved === RESOLVED_EMPTY ? null : resolved
248256

249-
if (this.blockResolver.canResolve(match)) {
250257
// Block output: store in contextVarAccumulator, replace with variable name
251258
const varName = `__blockRef_${Object.keys(contextVarAccumulator).length}`
252259
contextVarAccumulator[varName] = effectiveValue
260+
blockRefByMatch.set(match, varName)
253261
return varName
254262
}
255263

264+
const resolved = this.resolveReference(match, resolutionContext)
265+
if (resolved === undefined) return match
266+
267+
const effectiveValue = resolved === RESOLVED_EMPTY ? null : resolved
268+
256269
// Non-block reference (loop, parallel, workflow, env): embed as literal
257270
return this.blockResolver.formatValueForBlock(effectiveValue, BlockType.FUNCTION, language)
258271
} catch (error) {

apps/sim/tools/function/types.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,3 @@ export interface CodeExecutionOutput extends ToolResponse {
3434
stdout: string
3535
}
3636
}
37-
38-
export interface CodeExecutionOutput extends ToolResponse {
39-
output: {
40-
result: any
41-
stdout: string
42-
}
43-
}

0 commit comments

Comments
 (0)