Skip to content

fix(DesignerV2): Use ARM /run endpoint for published workflow triggers instead of listCallbackUrl#9033

Merged
rllyy97 merged 3 commits intomainfrom
riley/fixed-published-wf-run-path
Apr 8, 2026
Merged

fix(DesignerV2): Use ARM /run endpoint for published workflow triggers instead of listCallbackUrl#9033
rllyy97 merged 3 commits intomainfrom
riley/fixed-published-wf-run-path

Conversation

@rllyy97
Copy link
Copy Markdown
Contributor

@rllyy97 rllyy97 commented Apr 8, 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

What & Why

Fixes #8870 - The V2 designer was using the listCallbackUrl endpoint to get a SAS-signed URL, then invoking the trigger directly via that URL (no ARM auth). This differs from V1, which POSTs directly to the /triggers/{triggerName}/run ARM endpoint with ARM authentication.

This changes the V2 FloatingRunButton to construct the ARM /run URL directly for published workflows, matching V1 behavior.

Impact of Change

  • Users: Published workflow "Run" button in the V2 designer now uses the same ARM-authenticated /run endpoint as V1, improving reliability and consistency.
  • Developers: WorkflowService().getCallbackUrl() is no longer called for published workflow runs in FloatingRunButton. The WorkflowService import was removed from this file.
  • System: One fewer network call per run (no longer needs listCallbackUrl before invoking). Auth is now ARM-based instead of SAS token.

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in:

Manually tested, behavior matches V1 behavior now.

Contributors

@rllyy97

Screenshots/Videos

N/A

Copilot AI review requested due to automatic review settings April 8, 2026 16:24
@rllyy97 rllyy97 added the risk:low Low risk change with minimal impact label Apr 8, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 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(DesignerV2): Use ARM /run endpoint for published workflow triggers instead of listCallbackUrl
  • Issue: None — title is clear, follows conventional commit style, and accurately describes the change.
  • Recommendation: Keep as-is. (Optionally shorten to fix(DesignerV2): Use ARM /run endpoint for published workflow triggers if you prefer brevity.)

Commit Type

  • Properly selected (fix).
  • Only one commit type is selected in the PR body which is correct.

⚠️ Risk Level

  • The PR body and labels indicate: Low (label risk:low).
  • Assessment: I recommend Medium instead. Rationale: this change alters how the UI invokes published workflows (switching from SAS/listCallbackUrl to direct ARM /run paths). That changes auth flow and runtime invocation semantics — a subtle change that can affect production runs and permissions, even though the code change is small and unit tests were added. Because of the potential for user-visible failures in published workflow runs, label this PR risk:medium (or provide justification why risk:low is safe).

What & Why


Impact of Change

  • Impact section present and appropriate.
  • Recommendation: (Optional) Add a short note about whether any backend RBAC or Caller role changes are required or any service principal implications, if applicable.
    • Users: Published workflow "Run" button now uses ARM-authenticated /run endpoint (consistency + reliability).
    • Developers: WorkflowService().getCallbackUrl() no longer used for published runs in FloatingRunButton.
    • System: One fewer network call; auth changes to ARM.

Test Plan

  • Claim: Unit tests added/updated, manual testing completed.
  • Verified: The code diff includes new unit tests for getPublishedRunUrl and changes in FloatingRunButton; unit tests appear added.
  • Recommendation: Please consider adding an integration or E2E test to validate authenticated ARM invocation (covering published workflow run with proper ARM auth) or add a short rationale explaining why E2E is unnecessary. This change touches authentication and runtime invocation; an E2E/integration test would reduce risk.

⚠️ Contributors

  • PR lists @rllyy97 in Contributors — good.
  • Recommendation: If PMs/designers or other reviewers contributed, consider listing them; otherwise this is acceptable.

⚠️ Screenshots/Videos

  • N/A — appropriate since this is a behavioral/backend-shift change.
  • Recommendation: None required.

Summary Table

Section Status Recommendation
Title Keep as-is or slightly shorten
Commit Type No change needed
Risk Level ⚠️ Change to risk:medium or add strong justification for low
What & Why Fine; optionally call out auth change explicitly
Impact of Change Add backend/auth note if relevant
Test Plan Add/instrument E2E/integration test for ARM run if possible
Contributors ⚠️ Optional: list others if applicable
Screenshots/Videos ⚠️ N/A is fine

Final notes:

  • The PR body is well-formed and follows the required template. Unit tests for the URL generation were added and the change is small and well-contained.
  • I advise upgrading the risk label to risk:medium due to the change in invocation/authentication surface for published workflow runs. If you believe the change is indeed low-risk, please add a clear justification in the PR body explaining why (for example, detail why ARM auth will behave identically for all consumers and why no production impact is expected).
  • Please consider adding an integration or E2E test to exercise the authenticated ARM /run invocation path, or add an explanation for why such a test is unnecessary.

Please update the PR labels/body to reflect the chosen risk level (or provide the rationale for keeping risk:low), and optionally add an integration/E2E test (or justification) before merging. Thank you for the clear PR and thorough unit tests!


Last updated: Wed, 08 Apr 2026 18:08:46 GMT

Copy link
Copy Markdown
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

Updates the Designer V2 published-workflow “Run” behavior to match V1 by invoking triggers via an ARM-authenticated /run endpoint rather than retrieving and invoking a SAS-signed callback URL via listCallbackUrl.

Changes:

  • Removes WorkflowService().getCallbackUrl() usage from FloatingRunButton for published runs.
  • Constructs an ARM /run URL for both consumption and standard workflows and passes it to RunService().runTrigger.
  • Reduces one network call per published run (no pre-fetch of callback URL).

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

📊 Coverage Check

No source files changed in this PR.

@rllyy97 rllyy97 enabled auto-merge (squash) April 8, 2026 17:03
@rllyy97 rllyy97 merged commit 80771a4 into main Apr 8, 2026
14 checks passed
@rllyy97 rllyy97 deleted the riley/fixed-published-wf-run-path branch April 8, 2026 18:22
rllyy97 added a commit that referenced this pull request Apr 8, 2026
…s instead of listCallbackUrl (#9033)

Fixed published wf run path
rllyy97 added a commit that referenced this pull request Apr 8, 2026
…s instead of listCallbackUrl (#9034)

fix(DesignerV2): Use ARM /run endpoint for published workflow triggers instead of listCallbackUrl (#9033)

Fixed published wf run path
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.

[NewDesigner]:When identity is in off and run the workflow ,Run is getting error instead of run fail.

3 participants