Skip to content

fix(designer): Fix infinite recursive nesting in monitoring run history values [hotfix]#9125

Closed
takyyon wants to merge 1 commit into
hotfix/v5.294from
hotfix/v5.294-monitoring-fix-v2
Closed

fix(designer): Fix infinite recursive nesting in monitoring run history values [hotfix]#9125
takyyon wants to merge 1 commit into
hotfix/v5.294from
hotfix/v5.294-monitoring-fix-v2

Conversation

@takyyon
Copy link
Copy Markdown
Contributor

@takyyon takyyon commented Apr 29, 2026

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

Cherry-pick of #9123 onto hotfix/v5.294 for immediate release.

What & Why

Problem

Multiple customers (including US Government tenants) reported that action input/output values in the workflow run history view scroll infinitely, making them unreadable. The values display as deeply nested recursive JSON with {displayName, value} objects nested inside each other.

This affects both Consumption and Standard SKUs, across new and historical runs. First reported April 27, 2026. The bug was introduced in v5.294 (not present in v5.273).

Root Cause

Regression introduced in PR #8875 (commit 3e71bcde8, March 4) which added code_interpreter built-in tool support. The commit added a shortcut in the v1 designer's MonitoringPanel that returned already-bound BoundParameters as raw inputs, creating an infinite feedback loop with useEffect(() => refetch(), [runMetaData]).

Fix (3 changes)

  1. Remove the shortcut — always call getActionLinks() for fresh raw data
  2. Stable React Query key — include actionTrackingId, startTime, endTime (matching v2 pattern)
  3. Remove the refetch loop — remove useEffect(() => refetch(), [runMetaData])

Impact of Change

  • Users: Fixes infinite scrolling in run history for all Logic App types (Consumption + Standard).
  • Developers: No API changes. Internal data fetching pattern in v1 MonitoringPanel now matches v2.
  • System: Eliminates infinite refetch loop that caused unnecessary API calls and re-renders.

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in: Standalone designer with ARM token against live Standard Logic App

Test details:

  • monitoringTab.spec.tsx — 6 unit tests all passing on hotfix branch
  • Regression test should not re-feed already-bound BoundParameters as raw inputs verifies the exact bug is fixed
  • Clean cherry-pick, no conflicts

Contributors

Before (Bug) - Infinite recursive nesting

Values in the monitoring panel display as infinitely nested {displayName, value} objects, continuously scrolling:

Before - Bug

After (Fix) - Clean value display

Values display correctly as flat, readable content:

After - Fix

…ry values

The code_interpreter built-in tool support (3e71bcd, PR #8875) introduced
a shortcut that returned already-bound BoundParameters as raw inputs when
runMetaData.inputs existed. Combined with a refetch() triggered on every
runMetaData change, this created an infinite loop:

1. Raw inputs fetched → bound to {displayName, value} → stored on runData
2. runData change → refetch → shortcut returns already-bound data as 'raw'
3. Re-binding wraps {displayName, value} inside another {displayName, value}
4. Repeat infinitely → values scroll endlessly in the action boxes

Fix:
- Always call getActionLinks() for fresh raw data (never short-circuit)
- Null-safe getActionLinks return value via ?? {} fallback
- Use stable query key (actionTrackingId, startTime, endTime) so React Query
  caches properly and doesn't refetch when bound data changes
- Remove refetch() on runMetaData change that drove the infinite loop
- Use placeholderData instead of initialData to prevent premature dispatch
- Add enabled guard to prevent query when runMetaData is null

Affects both Consumption and Standard SKUs across all run history views.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Elaina-Lee Elaina-Lee enabled auto-merge (squash) April 29, 2026 17:57
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 29, 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(designer): Fix infinite recursive nesting in monitoring run history values [hotfix]
  • Issue: Title is clear and follows conventional format, but it repeats the verb "Fix" (once as the type, once in the message). It's a small nit.
  • Recommendation: Keep as-is if you prefer, or slightly tighten for clarity. Example: fix(designer): Prevent infinite recursive nesting in monitoring run history values [hotfix].

Commit Type

  • Properly selected (fix). Only one option is selected which is correct for this change.
  • Note: This is a hotfix cherry-pick — you already mention the cherry-pick onto hotfix/v5.294 in the body which is good.

⚠️ Risk Level

  • Assessment: The PR body selects Medium which matches the scope of changes (removing a refetch loop, changing query keys and data fetching behavior across monitoring). From the code diff, the changes touch runtime fetching/behavior and are therefore appropriately Medium.
  • Issue: There is NO repository label in the PR's labels array (e.g. risk:medium). Every PR must include a risk label that matches the Risk Level selected in the PR body. Please add the corresponding label: risk:medium (or the repo's canonical label name for Medium).

What & Why


Impact of Change

  • The Impact section clearly lists Users, Developers, and System impact.
  • Recommendation: Keep as-is. If you want to be extra explicit, mention whether any downstream consumers rely on the previous refetch behavior (unlikely, but helpful to reassure reviewers).

Test Plan

  • Assessment: The PR claims unit tests were added/updated and the diff shows updates to monitoringTab.spec.tsx with several tests exercising the new behavior. Unit tests presence is confirmed in the diff.
  • E2E tests: None added (unchecked) — acceptable for this scoped hotfix because the unit tests and manual verification against live Standard Logic App are provided. You explained manual testing steps and listed the unit test name that checks the regression.
  • Recommendation: Ensure CI passes and include the test run / CI link in the PR if available.

⚠️ Contributors

  • Assessment: A contributor is listed (- @takyyon). The Contributors section exists but is minimal and the section marker formatting merges into the next header in the body — consider adding a newline or explicit formatting.
  • Recommendation: Optional — add any additional reviewers or contributors (PMs, designers) who helped validate the fix.

Screenshots/Videos

  • Assessment: Before/After screenshots are included and linked, which is appropriate for a UI bugfix that affects rendering/scroll behavior.
  • Recommendation: None.

Summary Table

Section Status Recommendation
Title Consider removing duplicate verb (optional)
Commit Type None
Risk Level ⚠️ Add the repo risk label risk:medium to match the PR body
What & Why None
Impact of Change None
Test Plan Provide CI/test run link if available
Contributors ⚠️ Fix minor formatting and optionally list more contributors
Screenshots/Videos None

Final notes:

  • Advised risk level based on the code diff: Medium — this matches the risk level chosen in your PR body. No escalation required.
  • Required action before final merge: Add the repository risk label that corresponds to Medium (commonly risk:medium) to the PR. The PR body already selects Medium, but the missing label must be present on the PR itself so automation and reviewers see it.
  • Optional suggestions:
    • Slightly reword the PR title to avoid repeating the word "Fix" (cosmetic).
    • Fix the small formatting issue between Contributors and the Screenshots header (add a blank line) so the section renders cleanly in GitHub.
    • If available, include a link to CI results for the added unit tests to speed approval.

Please update the PR to add the risk:medium label and consider the minor title/formatting suggestions above. Thank you for the clear description, tests, and screenshots — this is in good shape for a hotfix release.


Last updated: Wed, 29 Apr 2026 18:02:14 GMT

@github-actions
Copy link
Copy Markdown
Contributor

📊 Coverage Check

🎉 All changed files have adequate test coverage!

@takyyon
Copy link
Copy Markdown
Contributor Author

takyyon commented Apr 29, 2026

Closing — recreating with clean commit (removed accidental .gitignore change).

@takyyon takyyon closed this Apr 29, 2026
auto-merge was automatically disabled April 29, 2026 18:41

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants