Skip to content

Conversation

@icecrasher321
Copy link
Collaborator

@icecrasher321 icecrasher321 commented Jan 23, 2026

Summary

  • Resolution varied between trigger paths causing confusion.
  • Double resolution of env vars in some, and special handling for response format that was redundant.
  • Client/Server side mixing of responsibilities hidden by dynamic imports.
  • Highlighting of env vars should be in line with variable resolution. If we're "trying" to resolve -- should be blue, otherwise not highlighted.

Type of Change

  • Bug fix
  • Other: Refactor

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Jan 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Jan 23, 2026 10:20pm

Request Review

@icecrasher321
Copy link
Collaborator Author

@cursor review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 23, 2026

Greptile Summary

Standardized environment variable resolution across webhook, workflow, and MCP execution paths by consolidating duplicate resolution logic into shared utilities.

Key improvements:

  • Introduced ENV_VAR_RESOLVE_DEFAULTS constant in executor/utils/reference-validation.ts to ensure consistent resolution behavior across all contexts (exact match, embedded patterns, whitespace tolerance, missing key handling)
  • Fixed double resolution bug in webhook creation flow by preserving original config and only merging external subscription additions (apps/sim/app/api/webhooks/route.ts:303-308)
  • Removed redundant response format special handling in serializer that was causing extra resolution passes
  • Created server-only lib/mcp/resolve-config.ts and client-safe lib/mcp/shared.ts for proper client/server separation in MCP code
  • Deleted preflight env var validation (lib/workflows/executor/preflight.ts) which was causing confusion and unnecessary validation passes
  • Enhanced frontend highlighting to only highlight env vars that actually exist in workspace/personal settings

Client/server responsibility clarification:

  • Server-side: resolveMcpConfigEnvVars, resolveEnvVarsInObject handle database access and resolution
  • Client-side: useAvailableEnvVarKeys hook provides env var key lists for UI highlighting without needing decrypted values
  • MCP service and test-connection endpoint now share the same resolution utility, with test endpoint using non-strict mode

The refactor removes 724 lines and adds 382 lines, eliminating inconsistencies where some paths double-resolved env vars while others had special handling.

Confidence Score: 4/5

  • Safe to merge with minor style improvement recommended for closure ordering in lib/mcp/resolve-config.ts
  • The refactor successfully consolidates env var resolution logic and removes problematic double-resolution paths. Code is well-tested manually according to the PR checklist. The only concern is a confusing closure scope ordering pattern in resolveMcpConfigEnvVars where a function references a variable declared later (technically valid JS but poor readability). All other changes follow clear architectural improvements: standardized defaults, proper client/server separation, and simplified resolution paths.
  • Pay attention to apps/sim/lib/mcp/resolve-config.ts for the closure scope ordering pattern. Consider moving the envVars initialization before the resolveValue function definition for better readability.

Important Files Changed

Filename Overview
apps/sim/lib/mcp/resolve-config.ts New server-side MCP config resolver with shared utility for env var resolution, has closure scope ordering issue
apps/sim/executor/utils/reference-validation.ts Added standardized defaults for env var resolution across all contexts, improving consistency
apps/sim/lib/webhooks/env-resolver.ts Simplified to delegate to shared resolveEnvVarReferences utility with deep:true for nested structures
apps/sim/lib/environment/utils.ts Removed preflight validation functions, added decryption failure tracking with proper logging
apps/sim/lib/execution/preprocessing.ts Removed preflightEnvVars option and related validation, simplifying the preprocessing flow
apps/sim/app/api/webhooks/route.ts Fixed double resolution of env vars by preserving original config and only merging external subscription additions

Sequence Diagram

sequenceDiagram
    participant Client as Client Component
    participant API as API Route
    participant Resolver as Env Resolver
    participant Utils as Env Utils
    participant DB as Database
    participant Core as resolveEnvVarReferences

    Note over Client,Core: Standardized Flow (After PR)

    rect rgb(220, 240, 220)
    Note right of Client: Webhook Creation Path
    Client->>API: POST /api/webhooks
    API->>Resolver: resolveEnvVarsInObject(config)
    Resolver->>Utils: getEffectiveDecryptedEnv(userId, workspaceId)
    Utils->>DB: Query personal + workspace env vars
    DB-->>Utils: Encrypted env vars
    Utils->>Utils: Decrypt all env vars
    Utils-->>Resolver: Decrypted env vars map
    Resolver->>Core: resolveEnvVarReferences(config, envVars, {deep:true})
    Core->>Core: Apply ENV_VAR_RESOLVE_DEFAULTS
    Core-->>Resolver: Resolved config
    Resolver-->>API: Resolved config (single pass)
    API->>API: Store original + external additions
    Note over API: No double resolution
    end

    rect rgb(220, 220, 240)
    Note right of Client: MCP Config Resolution Path
    Client->>API: POST /api/mcp/servers/test-connection
    API->>Utils: resolveMcpConfigEnvVars(config, {strict:false})
    Utils->>Utils: getEffectiveDecryptedEnv(userId, workspaceId)
    Utils->>Core: resolveEnvVarReferences(url, envVars)
    Core->>Core: Apply ENV_VAR_RESOLVE_DEFAULTS
    Core-->>Utils: Resolved url
    Utils->>Core: resolveEnvVarReferences(headers[key], envVars)
    Core-->>Utils: Resolved header value
    Utils-->>API: {config, missingVars}
    Note over API: Consistent resolution across MCP service & test endpoint
    end

    rect rgb(240, 220, 220)
    Note right of Core: Removed Paths (Before PR)
    Note over API,Core: ❌ Preflight env var validation (deleted)
    Note over API,Core: ❌ Double resolution in serializer (removed)
    Note over API,Core: ❌ Special response format handling (removed)
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

27 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@icecrasher321
Copy link
Collaborator Author

@cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

@icecrasher321
Copy link
Collaborator Author

@greptile

@icecrasher321
Copy link
Collaborator Author

@cursor review

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

33 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@icecrasher321
Copy link
Collaborator Author

@cursor review

@icecrasher321
Copy link
Collaborator Author

@cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

@icecrasher321 icecrasher321 merged commit b913cff into staging Jan 24, 2026
11 checks passed
@icecrasher321 icecrasher321 deleted the fix/preflight-env-vars branch January 24, 2026 02:59
waleedlatif1 added a commit that referenced this pull request Jan 24, 2026
…2973)

* fix(subflows): tag dropdown + resolution logic (#2949)

* fix(subflows): tag dropdown + resolution logic

* fixes;

* revert parallel change

* chore(deps): bump posthog-js to 1.334.1 (#2948)

* fix(idempotency): add conflict target to atomicallyClaimDb query + remove redundant db namespace tracking (#2950)

* fix(idempotency): add conflict target to atomicallyClaimDb query

* delete needs to account for namespace

* simplify namespace filtering logic

* fix cleanup

* consistent target

* improvement(kb): add document filtering, select all, and React Query migration (#2951)

* improvement(kb): add document filtering, select all, and React Query migration

* test(kb): update tests for enabledFilter and removed userId params

* fix(kb): remove non-null assertion, add explicit guard

* improvement(logs): trace span, details (#2952)

* improvement(action-bar): ordering

* improvement(logs): details, trace span

* feat(blog): v0.5 release post (#2953)

* feat(blog): v0.5 post

* improvement(blog): simplify title and remove code block header

- Simplified blog title from "Introducing Sim Studio v0.5" to "Introducing Sim v0.5"
- Removed language label header and copy button from code blocks for cleaner appearance

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* ack PR comments

* small styling improvements

* created system to create post-specific components

* updated componnet

* cache invalidation

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* feat(admin): add credits endpoint to issue credits to users (#2954)

* feat(admin): add credits endpoint to issue credits to users

* fix(admin): use existing credit functions and handle enterprise seats

* fix(admin): reject NaN and Infinity in amount validation

* styling

* fix(admin): validate userId and email are strings

* improvement(copilot): fast mode, subagent tool responses and allow preferences (#2955)

* Improvements

* Fix actions mapping

* Remove console logs

* fix(billing): handle missing userStats and prevent crashes (#2956)

* fix(billing): handle missing userStats and prevent crashes

* fix(billing): correct import path for getFilledPillColor

* fix(billing): add Number.isFinite check to lastPeriodCost

* fix(logs): refresh logic to refresh logs details (#2958)

* fix(security): add authentication and input validation to API routes (#2959)

* fix(security): add authentication and input validation to API routes

* moved utils

* remove extraneous commetns

* removed unused dep

* improvement(helm): add internal ingress support and same-host path consolidation (#2960)

* improvement(helm): add internal ingress support and same-host path consolidation

* improvement(helm): clean up ingress template comments

Simplify verbose inline Helm comments and section dividers to match the
minimal style used in services.yaml.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(helm): add missing copilot path consolidation for realtime host

When copilot.host equals realtime.host but differs from app.host,
copilot paths were not being routed. Added logic to consolidate
copilot paths into the realtime rule for this scenario.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* improvement(helm): follow ingress best practices

- Remove orphan comments that appeared when services were disabled
- Add documentation about path ordering requirements
- Paths rendered in order: realtime, copilot, app (specific before catch-all)
- Clean template output matching industry Helm chart standards

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* feat(blog): enterprise post (#2961)

* feat(blog): enterprise post

* added more images, styling

* more content

* updated v0-5 post

* remove unused transition

---------

Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai>

* fix(envvars): resolution standardized (#2957)

* fix(envvars): resolution standardized

* remove comments

* address bugbot

* fix highlighting for env vars

* remove comments

* address greptile

* address bugbot

* fix(copilot): mask credentials fix (#2963)

* Fix copilot masking

* Clean up

* Lint

* improvement(webhooks): remove dead code (#2965)

* fix(webhooks): subscription recreation path

* improvement(webhooks): remove dead code

* fix tests

* address bugbot comments

* fix restoration edge case

* fix more edge cases

* address bugbot comments

* fix gmail polling

* add warnings for UI indication for credential sets

* fix(preview): subblock values (#2969)

* fix(child-workflow): nested spans handoff (#2966)

* fix(child-workflow): nested spans handoff

* remove overly defensive programming

* update type check

* type more code

* remove more dead code

* address bugbot comments

* fix(security): restrict API key access on internal-only routes (#2964)

* fix(security): restrict API key access on internal-only routes

* test(security): update function execute tests for checkInternalAuth

* updated agent handler

* move session check higher in checkSessionOrInternalAuth

* extracted duplicate code into helper for resolving user from jwt

* fix(copilot): update copilot chat title (#2968)

* fix(hitl): fix condition blocks after hitl (#2967)

* fix(notes): ghost edges (#2970)

* fix(notes): ghost edges

* fix deployed state fallback

* fallback

* remove UI level checks

* annotation missing from autoconnect source check

* improvement(docs): loop and parallel var reference syntax (#2975)

* fix(blog): slash actions description (#2976)

* improvement(docs): loop and parallel var reference syntax

* fix(blog): slash actions description

* fix(auth): copilot routes (#2977)

* Fix copilot auth

* Fix

* Fix

* Fix

* fix(copilot): fix edit summary for loops/parallels (#2978)

* fix(integrations): hide from tool bar (#2544)

* fix(landing): ui (#2979)

* fix(edge-validation): race condition on collaborative add (#2980)

* fix(variables): boolean type support and input improvements (#2981)

* fix(variables): boolean type support and input improvements

* fix formatting

---------

Co-authored-by: Vikhyath Mondreti <vikhyathvikku@gmail.com>
Co-authored-by: Emir Karabeg <78010029+emir-karabeg@users.noreply.github.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Siddharth Ganesan <33737564+Sg312@users.noreply.github.com>
Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai>
waleedlatif1 added a commit that referenced this pull request Jan 24, 2026
…2973)

* fix(subflows): tag dropdown + resolution logic (#2949)

* fix(subflows): tag dropdown + resolution logic

* fixes;

* revert parallel change

* chore(deps): bump posthog-js to 1.334.1 (#2948)

* fix(idempotency): add conflict target to atomicallyClaimDb query + remove redundant db namespace tracking (#2950)

* fix(idempotency): add conflict target to atomicallyClaimDb query

* delete needs to account for namespace

* simplify namespace filtering logic

* fix cleanup

* consistent target

* improvement(kb): add document filtering, select all, and React Query migration (#2951)

* improvement(kb): add document filtering, select all, and React Query migration

* test(kb): update tests for enabledFilter and removed userId params

* fix(kb): remove non-null assertion, add explicit guard

* improvement(logs): trace span, details (#2952)

* improvement(action-bar): ordering

* improvement(logs): details, trace span

* feat(blog): v0.5 release post (#2953)

* feat(blog): v0.5 post

* improvement(blog): simplify title and remove code block header

- Simplified blog title from Introducing Sim Studio v0.5 to Introducing Sim v0.5
- Removed language label header and copy button from code blocks for cleaner appearance

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* ack PR comments

* small styling improvements

* created system to create post-specific components

* updated componnet

* cache invalidation

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* feat(admin): add credits endpoint to issue credits to users (#2954)

* feat(admin): add credits endpoint to issue credits to users

* fix(admin): use existing credit functions and handle enterprise seats

* fix(admin): reject NaN and Infinity in amount validation

* styling

* fix(admin): validate userId and email are strings

* improvement(copilot): fast mode, subagent tool responses and allow preferences (#2955)

* Improvements

* Fix actions mapping

* Remove console logs

* fix(billing): handle missing userStats and prevent crashes (#2956)

* fix(billing): handle missing userStats and prevent crashes

* fix(billing): correct import path for getFilledPillColor

* fix(billing): add Number.isFinite check to lastPeriodCost

* fix(logs): refresh logic to refresh logs details (#2958)

* fix(security): add authentication and input validation to API routes (#2959)

* fix(security): add authentication and input validation to API routes

* moved utils

* remove extraneous commetns

* removed unused dep

* improvement(helm): add internal ingress support and same-host path consolidation (#2960)

* improvement(helm): add internal ingress support and same-host path consolidation

* improvement(helm): clean up ingress template comments

Simplify verbose inline Helm comments and section dividers to match the
minimal style used in services.yaml.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(helm): add missing copilot path consolidation for realtime host

When copilot.host equals realtime.host but differs from app.host,
copilot paths were not being routed. Added logic to consolidate
copilot paths into the realtime rule for this scenario.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* improvement(helm): follow ingress best practices

- Remove orphan comments that appeared when services were disabled
- Add documentation about path ordering requirements
- Paths rendered in order: realtime, copilot, app (specific before catch-all)
- Clean template output matching industry Helm chart standards

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* feat(blog): enterprise post (#2961)

* feat(blog): enterprise post

* added more images, styling

* more content

* updated v0-5 post

* remove unused transition

---------

Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai>

* fix(envvars): resolution standardized (#2957)

* fix(envvars): resolution standardized

* remove comments

* address bugbot

* fix highlighting for env vars

* remove comments

* address greptile

* address bugbot

* fix(copilot): mask credentials fix (#2963)

* Fix copilot masking

* Clean up

* Lint

* improvement(webhooks): remove dead code (#2965)

* fix(webhooks): subscription recreation path

* improvement(webhooks): remove dead code

* fix tests

* address bugbot comments

* fix restoration edge case

* fix more edge cases

* address bugbot comments

* fix gmail polling

* add warnings for UI indication for credential sets

* fix(preview): subblock values (#2969)

* fix(child-workflow): nested spans handoff (#2966)

* fix(child-workflow): nested spans handoff

* remove overly defensive programming

* update type check

* type more code

* remove more dead code

* address bugbot comments

* fix(security): restrict API key access on internal-only routes (#2964)

* fix(security): restrict API key access on internal-only routes

* test(security): update function execute tests for checkInternalAuth

* updated agent handler

* move session check higher in checkSessionOrInternalAuth

* extracted duplicate code into helper for resolving user from jwt

* fix(copilot): update copilot chat title (#2968)

* fix(hitl): fix condition blocks after hitl (#2967)

* fix(notes): ghost edges (#2970)

* fix(notes): ghost edges

* fix deployed state fallback

* fallback

* remove UI level checks

* annotation missing from autoconnect source check

* improvement(docs): loop and parallel var reference syntax (#2975)

* fix(blog): slash actions description (#2976)

* improvement(docs): loop and parallel var reference syntax

* fix(blog): slash actions description

* fix(auth): copilot routes (#2977)

* Fix copilot auth

* Fix

* Fix

* Fix

* fix(copilot): fix edit summary for loops/parallels (#2978)

* fix(integrations): hide from tool bar (#2544)

* fix(landing): ui (#2979)

* fix(edge-validation): race condition on collaborative add (#2980)

* fix(variables): boolean type support and input improvements (#2981)

* fix(variables): boolean type support and input improvements

* fix formatting

---------

Co-authored-by: Vikhyath Mondreti <vikhyathvikku@gmail.com>
Co-authored-by: Emir Karabeg <78010029+emir-karabeg@users.noreply.github.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Siddharth Ganesan <33737564+Sg312@users.noreply.github.com>
Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants