Skip to content

fix: detect nested variable references and hide tracked properties on triggers#8804

Merged
hartra344 merged 2 commits intomainfrom
fix/variable-reference-and-tracked-properties
Feb 11, 2026
Merged

fix: detect nested variable references and hide tracked properties on triggers#8804
hartra344 merged 2 commits intomainfrom
fix/variable-reference-and-tracked-properties

Conversation

@hartra344
Copy link
Collaborator

Commit Type

  • fix - Bug fix

Risk Level

  • Low - Minor changes, limited scope

What & Why

Two bug fixes:

  1. Nested variable reference detection (Fixes combine initialize variables logicapps#1447): The \hasVariableReference\ regex only matched @variables(...)\ directly, missing nested calls like @{substring(variables('x'), ...)}. This caused variables with inter-dependencies to be incorrectly combined into a single InitializeVariable action, breaking workflows at runtime.

  2. Tracked properties on triggers (Fixes Some triggers show tracked properties in the settings, but saving errors out when set logicapps#798): Some connector manifests declared \ rackedProperties: {}\ without scope restrictions, causing the tracked properties setting to appear for triggers in the designer. The backend never supports tracked properties on triggers, so we now return \ alse\ early for triggers.

Impact of Change

  • Users: Variables that reference other variables inside nested functions (e.g. \substring(variables('x'), ...)) will no longer be broken by the combine-variables optimization. Tracked properties will no longer incorrectly appear on trigger nodes.
  • Developers: No API changes.
  • System: No performance impact. The regex change is more permissive (conservative behavior - prevents combining when in doubt).

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in: Unit tests for both designer v1 and designer-v2

Contributors

Screenshots/Videos

N/A - logic-only changes

… triggers

- Fix hasVariableReference regex to detect variables() calls nested inside other functions like substring(), length(), concat(), etc. Previously the regex required @ directly before variables, missing patterns like @{substring(variables('x'), ...)}.

- Prevent tracked properties from showing on triggers. The backend never supports tracked properties on triggers regardless of what the manifest declares.

- Added unit tests for nested variable reference patterns in both designer v1 and v2. Changes mirrored in both libs/designer and libs/designer-v2.
Copilot AI review requested due to automatic review settings February 10, 2026 22:46
@github-actions
Copy link

github-actions bot commented Feb 10, 2026

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix: detect nested variable references and hide tracked properties on triggers
  • Issue: None — title is clear, follows conventional commit style, and accurately summarizes the two fixes in this PR.
  • Recommendation: Keep as-is.

Commit Type

  • Properly selected (fix).
  • Only one commit type selected which is correct for this change.

Risk Level

  • The PR selects Low and the repository has the risk:low label applied.
  • Assessment from the diff: the changes are localized logic fixes (regex and a guarded check) with unit tests added. Advised risk: low (matches submitter).

What & Why

  • Current: Two concise bug fix descriptions: 1) nested variable reference detection, 2) hiding tracked properties on triggers. (Includes relevant issue links / references.)
  • Issue: None — explanation is clear and tied to issues.
  • Recommendation: Keep as-is; optionally add the exact issue numbers as links (you already referenced them by number which is good).

Impact of Change

  • The Impact section clearly lists Users, Developers, and System impact.
  • Recommendation: The section is good. No changes required.

Test Plan

  • Tests claimed: Unit tests added/updated. Verified in diff: unit tests were added/updated in both libs/designer and libs/designer-v2 test files for the variable-detection behavior and sequential combine behavior. Good.
  • E2E tests: not added (reasonable for logic-only changes). Manual testing: not selected (fine given unit tests exist).
  • Recommendation: Add a brief note in the Test Plan indicating which test files were changed (e.g., path) or mention CI status if available — this makes it easier for reviewers to find the tests quickly.

⚠️ Contributors

  • Assessment: Blank in the PR body. This is optional and allowed.
  • Recommendation: Consider adding a short contributors section to acknowledge reviewers, PM, or designers if they contributed. If no additional contributors, adding a short comment like None or leaving as-is is fine.

Screenshots/Videos

  • Assessment: N/A - logic-only changes — appropriate.
  • Recommendation: None.

Summary Table

Section Status Recommendation
Title Keep the current title
Commit Type fix is correct
Risk Level risk:low matches code changes
What & Why Clear and adequate
Impact of Change Clear and adequate
Test Plan Unit tests added — consider calling out file paths or CI results
Contributors ⚠️ Optional: add contributors or acknowledge nobody contributed
Screenshots/Videos N/A is appropriate

Final notes

  • The PR follows the required template and includes unit tests that match the changes in the diff. The risk label (risk:low) correctly reflects the changes.
  • Small recommendations: add file paths or brief pointers to the tests you changed in the Test Plan to speed review (e.g., libs/designer/src/lib/core/parsers/__test__/combineSequentialInitializeVariables.spec.ts and libs/designer-v2/...), and optionally list contributors.

Please update only if you want to add the optional contributors information or a short pointer to the modified test files; otherwise this PR is ready from a PR-title/body/template perspective. Thanks!


Last updated: Wed, 11 Feb 2026 18:35:34 GMT

@hartra344 hartra344 added the risk:low Low risk change with minimal impact label Feb 10, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes two designer-side behaviors in the Logic Apps UX monorepo: (1) more reliably detecting variable-to-variable references inside nested expression functions to prevent invalid “combine InitializeVariable actions” optimizations, and (2) ensuring tracked properties settings never appear for trigger nodes (backend doesn’t support them).

Changes:

  • Broaden hasVariableReference detection to catch variables() when nested inside other expression functions.
  • Add/extend unit tests for nested variable reference detection and for preventing sequential InitializeVariable combining in such cases.
  • Force trackedProperties setting support to false for triggers regardless of what connector manifests declare.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
libs/designer/src/lib/core/parsers/test/combineSequentialInitializeVariables.spec.ts Adds unit tests covering nested variables() detection and prevents combining when nested dependencies exist.
libs/designer/src/lib/core/parsers/ParseReduxAction.ts Updates hasVariableReference logic used to block unsafe InitializeVariable combining.
libs/designer/src/lib/core/actions/bjsworkflow/settings.ts Disables tracked properties support for triggers unconditionally.
libs/designer-v2/src/lib/core/parsers/test/combineSequentialInitializeVariables.spec.ts Mirrors v1 tests for nested variable reference detection and non-combining behavior.
libs/designer-v2/src/lib/core/parsers/ParseReduxAction.ts Mirrors v1 hasVariableReference update for designer-v2.
libs/designer-v2/src/lib/core/actions/bjsworkflow/settings.ts Mirrors v1 tracked properties trigger restriction for designer-v2.

@hartra344 hartra344 enabled auto-merge (squash) February 10, 2026 22:57
@github-actions
Copy link

📊 Coverage check completed. See workflow run for details.

@github-actions
Copy link

📊 Coverage check completed. See workflow run for details.

@hartra344 hartra344 merged commit cbeec45 into main Feb 11, 2026
13 checks passed
@hartra344 hartra344 deleted the fix/variable-reference-and-tracked-properties branch February 11, 2026 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:low Low risk change with minimal impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

combine initialize variables Some triggers show tracked properties in the settings, but saving errors out when set

2 participants