-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix(env-vars): remove regex parsing from table subblock, add formatDisplayText to various subblocks that didn't have it #1582
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Summary
This PR simplifies environment variable resolution across the application by removing complex regex-based parsing logic and standardizing on a consistent `{{}}` format approach. The changes affect both the executor's resolver logic and multiple UI components that display variable references.Key Changes:
- Executor Resolver: Removed complex API key detection methods (
isApiKeyField,containsProperEnvVarReference,isValidVariableReference) and simplifiedresolveEnvVariablesto parse all{{ENV_VAR}}patterns uniformly without contextual validation - UI Components: Added
formatDisplayTextfunctionality to several subblock components (knowledge tag filters, trigger config, table, MCP dynamic args, document tag entry) that were missing consistent variable highlighting - Display Pattern: Components now use an overlay approach where input fields have transparent text with formatted overlays showing highlighted variables and block references
The changes integrate with the existing formatDisplayText utility and useAccessibleReferencePrefixes hook to provide context-aware highlighting of variables ({{var}}) and block references (<block.value>) based on workflow accessibility rules. This creates a more predictable and maintainable system where environment variable resolution behavior is consistent across all contexts rather than having special cases for different field types.
Important Files Changed
Changed Files
| Filename | Score | Overview |
|---|---|---|
| apps/sim/executor/resolver/resolver.ts | 4/5 | Simplified environment variable resolution by removing complex API key detection and contextual parsing logic |
| apps/sim/executor/resolver/resolver.test.ts | 5/5 | Updated tests to reflect new uniform environment variable resolution behavior across all contexts |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/components/sub-block/components/table.tsx | 4/5 | Added formatDisplayText with accessible prefixes for variable highlighting in table cells |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/components/sub-block/components/mcp-dynamic-args/mcp-dynamic-args.tsx | 4/5 | Implemented overlay-based variable highlighting for text inputs using formatDisplayText |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/components/sub-block/components/trigger-config/components/trigger-config-section.tsx | 4/5 | Added variable highlighting to non-secret trigger configuration input fields |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/components/sub-block/components/trigger-config/components/trigger-modal.tsx | 5/5 | Added blockId prop to TriggerConfigSection for proper variable reference context |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/components/sub-block/components/knowledge-tag-filters/knowledge-tag-filters.tsx | 5/5 | Added accessiblePrefixes and highlightAll parameters to formatDisplayText calls |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/components/sub-block/components/document-tag-entry/document-tag-entry.tsx | 4/5 | Updated formatDisplayText calls to include proper variable highlighting configuration |
Confidence score: 4/5
- This PR is generally safe to merge but requires attention to the executor resolver changes due to their critical nature
- Score reflects the significant behavioral change in environment variable resolution that could affect existing workflows
- Pay close attention to apps/sim/executor/resolver/resolver.ts as it removes complex validation logic that may have been preventing edge cases
Sequence Diagram
sequenceDiagram
participant User
participant TableSubblock
participant InputResolver
participant VariableResolver
participant EnvVarHandler
participant FormattedText
User->>TableSubblock: "Edit cell value with {{API_KEY}}"
TableSubblock->>TableSubblock: "handleCellChange(rowIndex, column, value)"
TableSubblock->>InputResolver: "resolveInputs(block, context)"
InputResolver->>InputResolver: "processStringValue(value, key, context, block)"
InputResolver->>VariableResolver: "resolveVariableReferences(value, block)"
InputResolver->>InputResolver: "resolveBlockReferences(resolvedVars, context, block)"
InputResolver->>EnvVarHandler: "resolveEnvVariables(resolvedReferences)"
EnvVarHandler->>EnvVarHandler: "Check for {{ENV_VAR}} pattern"
EnvVarHandler->>EnvVarHandler: "Replace with environment variable value"
EnvVarHandler-->>InputResolver: "Return resolved string"
InputResolver-->>TableSubblock: "Return resolved inputs"
TableSubblock->>FormattedText: "formatDisplayText(cellValue, options)"
FormattedText->>FormattedText: "Highlight {{}} format variables"
FormattedText-->>TableSubblock: "Return formatted display"
TableSubblock-->>User: "Display resolved value in cell"
8 files reviewed, no comments
…splayText to various subblocks that didn't have it
416e094 to
dd6e1f2
Compare
…splayText to various subblocks that didn't have it (#1582)
Summary
{{}}formatType of Change
Testing
Tested manually.
Checklist