Add Pr URL in subscription outcome table by adamzip · Pull Request #6395 · dotnet/arcade-services

adamzip · 2026-06-15T10:21:50Z

Adds a PrUrl column to the subscription outcome DB table and the BarViz table
Removes Pr URLs from outcome messages
Retrieves InProgressPullRequest while recording subscription outcome to get the ongoing PR url. If this fails, logs an error message and continues with recording the outcome - without the pr url.

Copilot

Pull request overview

This PR adds a Pull Request URL field to recorded subscription outcomes, exposes it via the PCS API/client model, and displays it as a dedicated “Pull Request” column in the BarViz subscription trigger history table (rather than embedding the URL in the outcome message text).

Changes:

Persist PrUrl on SubscriptionOutcome (EF model + migration) and populate it when recording outcomes by reading the tracked PR state from Redis.
Extend API and generated client SubscriptionTriggerOutcome models to include prUrl.
Update BarViz UI to show a Pull Request link column; normalize several outcome messages to remove inline URLs.

Reviewed changes

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

Show a summary per file

File	Description
src/ProductConstructionService/ProductConstructionService.DependencyFlow/WorkItemProcessors/SubscriptionUpdateProcessor.cs	Removes unused subscription lookup during update processing.
src/ProductConstructionService/ProductConstructionService.DependencyFlow/WorkItemProcessors/SubscriptionTriggerProcessor.cs	Improves failure message when no matching subscription/build is found.
src/ProductConstructionService/ProductConstructionService.DependencyFlow/SubscriptionUpdateOutcomeRecorder.cs	Adds Redis lookup to capture PR URL and store it on outcomes.
src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdaters/PullRequestUpdater.cs	Removes PR URL from certain outcome messages now that it’s stored separately.
src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdaters/DependencyPullRequestUpdater.cs	Updates outcome messages to be URL-free / more generic.
src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdaters/CodeFlowPullRequestUpdater.cs	Updates outcome messages to be URL-free / more generic.
src/ProductConstructionService/ProductConstructionService.BarViz/Pages/SubscriptionTriggerHistory.razor	Adds “Pull Request” column linking to `PrUrl`.
src/ProductConstructionService/ProductConstructionService.Api/Api/v2020_02_20/Models/SubscriptionTriggerOutcome.cs	Adds `PrUrl` to API response model.
src/ProductConstructionService/Microsoft.DotNet.ProductConstructionService.Client/Generated/Models/SubscriptionTriggerOutcome.cs	Regenerates client model to include `prUrl`.
src/Maestro/Maestro.Data/Models/SubscriptionOutcome.cs	Adds `PrUrl` to EF entity.
src/Maestro/Maestro.Data/Migrations/BuildAssetRegistryContextModelSnapshot.cs	Updates snapshot to include `PrUrl` column.
src/Maestro/Maestro.Data/Migrations/20260610160212_AddSubscriptionOutcomeRepoAndPrUrls.cs	Adds migration introducing `SubscriptionOutcomes.PrUrl` column.

This reverts commit 4708ea5.

…operationId

Copilot

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 4 comments.

Files not reviewed (1)

src/Maestro/Maestro.Data/Migrations/20260615142342_AddSubscriptionOutcomePrUrl.Designer.cs: Generated file

                return new SubscriptionUpdateResult(
-                    $"Skipping codeflow update because an update with a newer build {pr.NextBuildsToProcess} has already been queued."
-                     + (pr != null ? $"PR url: {GitRepoUrlUtils.TurnApiUrlToWebsite(pr.Url)}" : ""),
+                    $"Skipping codeflow update because an update with a newer build {pr.NextBuildsToProcess.Values.First().ToString() ?? "(N/A)"} has already been queued",
                    SubscriptionOutcomeType.NoUpdate);


premun

I think my original question stands - why wouldn't it be enough in those 3 places?

premun · 2026-06-17T15:37:25Z

        {
            oldPrUrl = pr.Url;
            await _stateManager.ClearAllStateAsync(isCodeFlow: true, clearPendingUpdates: true);
+            _outcomeRecorder.SetPullRequestUrl(null);


Isn't it better to not to do this? The URL will get overwritten later. And if we blow up before then, don't we want to have that old PR tied to that exception?

I think it's misleading to put the URL in that case.
When we clear the PR state and start a codeflow on a new branch, failures won't have anything to do with the old PR, right? it would be misleading to put the URL there with the old codeflows and whatever user commits or comments might be there

premun · 2026-06-17T15:38:34Z

            {
                oldPrUrl = pr.Url;
                await _stateManager.ClearAllStateAsync(isCodeFlow: true, clearPendingUpdates: true);
+                _outcomeRecorder.SetPullRequestUrl(null);


Same here, we will create a new PR and override this. And if we fail before then, let's log the old URI?

premun · 2026-06-17T15:41:06Z

        _commentCollector.AddComment(PullRequestCommentBuilder.BuildOppositeCodeflowMergedNotification(), CommentType.Warning);
        pr.BlockedFromFutureUpdates = true;
        await _stateManager.SetInProgressPullRequestAsync(pr);
+        _outcomeRecorder.SetPullRequestUrl(pr.Url);


Why do we need this one? Wouldn't it already be set from when we created/fetched the PR info?

Applies to other places too

Yep there were a few cases where copilot only added it because of the interaction with the state manager even though it wasn't needed (i.e. we only call the state manager to persist the pr's new "blocked" state or something like that). removed.

Copilot

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

src/Maestro/Maestro.Data/Migrations/20260615142342_AddSubscriptionOutcomePrUrl.Designer.cs: Generated file

Comments suppressed due to low confidence (1)

src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdaters/PullRequestUpdater.cs:427

pr.NextBuildsToProcess.Values.First() can throw when the dictionary is empty, and even when non-empty it may show the wrong build id (it ignores which subscription key was matched). You already have the queued build id in the TryGetValue(..., out int buildId) result; use that for both logging and the outcome message to avoid exceptions and incorrect output.

                    $"Skipping codeflow update because an update with a newer build {pr.NextBuildsToProcess.Values.First().ToString() ?? "(N/A)"} has already been queued",
                    SubscriptionOutcomeType.NoUpdate);
            }

            (var status, prInfo) = await GetPullRequestStatusAsync(pr, isCodeFlow, tryingToUpdate: true);

Copilot

Pull request overview

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

Files not reviewed (1)

src/Maestro/Maestro.Data/Migrations/20260615142342_AddSubscriptionOutcomePrUrl.Designer.cs: Generated file

Comments suppressed due to low confidence (1)

src/ProductConstructionService/ProductConstructionService.DependencyFlow/WorkItemProcessors/SubscriptionUpdateProcessor.cs:44

This processor no longer validates that the subscription exists before dispatching to the updater. If the subscription was deleted while a work item is queued, DependencyPullRequestUpdater currently throws an InvalidOperationException (retriable) when it can’t load the subscription, causing unnecessary retries. Consider keeping the early non-retriable validation here (as before) so deleted subscriptions fail fast and don’t churn the queue.

        var build = await _sqlClient.GetBuildAsync(workItem.BuildId)
            ?? throw new NonRetriableException($"Build with buildId {workItem.BuildId} not found in the DB.");

        var updater = _updaterFactory.CreatePullRequestUpdater(
            PullRequestUpdaterId.Parse(
                workItem.UpdaterId,
                workItem.SubscriptionType == SubscriptionType.DependenciesAndSources));

                return new SubscriptionUpdateResult(
-                    $"Skipping codeflow update because an update with a newer build {pr.NextBuildsToProcess} has already been queued."
-                     + (pr != null ? $"PR url: {GitRepoUrlUtils.TurnApiUrlToWebsite(pr.Url)}" : ""),
+                    $"Skipping codeflow update because an update with a newer build {pr.NextBuildsToProcess.Values.First().ToString() ?? "(N/A)"} has already been queued",
                    SubscriptionOutcomeType.NoUpdate);


Add Pr URL in subscription outcome table

47504c5

Copilot AI review requested due to automatic review settings June 15, 2026 10:21

adamzip commented Jun 15, 2026

View reviewed changes

Comment thread .../ProductConstructionService.DependencyFlow/WorkItemProcessors/SubscriptionUpdateProcessor.cs

Copilot started reviewing on behalf of adamzip June 15, 2026 10:22 View session

Copilot AI reviewed Jun 15, 2026

View reviewed changes

Comment thread ...uctionService/ProductConstructionService.DependencyFlow/SubscriptionUpdateOutcomeRecorder.cs Outdated

Comment thread ...ConstructionService/ProductConstructionService.BarViz/Pages/SubscriptionTriggerHistory.razor Outdated

adamzip added 2 commits June 15, 2026 13:28

CR changes

daf4bb5

try fix migration

6eeb848

adamzip requested review from dkurepa and premun June 15, 2026 12:12

fix broken outcome message

4fcbf99

premun reviewed Jun 15, 2026

View reviewed changes

Comment thread ...uctionService/ProductConstructionService.DependencyFlow/SubscriptionUpdateOutcomeRecorder.cs Outdated

premun reviewed Jun 15, 2026

View reviewed changes

Comment thread ...ConstructionService/ProductConstructionService.BarViz/Pages/SubscriptionTriggerHistory.razor Outdated

adamzip added 3 commits June 15, 2026 16:38

Display PR number hypertext, add URL in darc result

f2bca68

Notify about missing opposite-dir subs

4708ea5

try fix migration

adcf572

premun reviewed Jun 16, 2026

View reviewed changes

Comment thread ...ctConstructionService/ProductConstructionService.DependencyFlow/PullRequestCommentBuilder.cs Outdated

adamzip added 2 commits June 16, 2026 10:17

Revert "Notify about missing opposite-dir subs"

ac4e3b9

This reverts commit 4708ea5.

Save the PR URL state via PRUpdater, and dont record outcome without …

86ac0e7

…operationId

Copilot AI review requested due to automatic review settings June 17, 2026 15:18

Copilot started reviewing on behalf of adamzip June 17, 2026 15:19 View session

remove questionable comment

e5e0a7a

Copilot AI reviewed Jun 17, 2026

View reviewed changes

adamzip requested a review from premun June 17, 2026 15:27

premun reviewed Jun 17, 2026

View reviewed changes

bring back operationId fallback

c3004fc

Copilot AI review requested due to automatic review settings June 17, 2026 16:03

Copilot started reviewing on behalf of adamzip June 17, 2026 16:04 View session

Remove extraenous url setters

3d326a0

Copilot AI reviewed Jun 17, 2026

View reviewed changes

Comment thread ...roductConstructionService/ProductConstructionService.BarViz/Code/Helpers/RepoUrlConverter.cs

remove unnecessary url setter

d3abc5e

Copilot AI review requested due to automatic review settings June 17, 2026 16:10

Copilot started reviewing on behalf of adamzip June 17, 2026 16:11 View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

Conversation

adamzip commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

premun left a comment

Choose a reason for hiding this comment

Uh oh!

premun Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

adamzip Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

premun Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

premun Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

premun Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

adamzip Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adamzip commented Jun 15, 2026 •

edited

Loading