Fix darc update-subscription to error when immutable fields are changed by Copilot · Pull Request #5348 · dotnet/arcade-services

Copilot · 2025-10-08T10:03:12Z

Problem

When updating a subscription using darc update-subscription with the popup editor, changes to immutable fields were silently ignored. The command would report success, but fields like TargetBranch, TargetRepository, and SourceEnabled would not be updated, leading to user confusion.

As noted in the documentation, these fields cannot be modified because of how Maestro++ tracks pull requests, but the tool was not validating or enforcing this restriction.

Example of the problem:

> darc update-subscription --id 58cbbaad-a57e-4327-865a-609fa1c10968
# User changes target branch in the popup from 'main' to 'dev'
Successfully updated subscription with id '58cbbaad-a57e-4327-865a-609fa1c10968'.
# But the target branch is still 'main' - change was silently ignored

Solution

Added validation in UpdateSubscriptionOperation after popup values are extracted to detect and reject changes to immutable fields:

Target Repository URL - The repository receiving updates
Target Branch - The branch receiving updates
Source Enabled - Whether the subscription uses source-enabled (VMR) flow

When users attempt to modify any of these fields, the operation now fails with a clear error message listing all immutable fields that were changed.

After this fix:

> darc update-subscription --id 58cbbaad-a57e-4327-865a-609fa1c10968
# User changes target branch in the popup from 'main' to 'dev'
The following immutable fields cannot be modified:
  - Target Branch (cannot be changed from 'main')

Changes

UpdateSubscriptionOperation.cs: Added validation logic after popup values are extracted to compare against the original subscription and detect immutable field changes
Darc.md: Updated documentation to explicitly list all immutable fields and clarify that changes will result in an error

Testing

All existing tests continue to pass (117 passed, 1 skipped)
Build succeeds with no warnings
Documentation updated to reflect the new behavior

Fixes #4248

Fixes #4725

Original prompt

This section details on the original issue you should resolve

<issue_title>darc update-subscription target branch doesn't work but also doesn't error</issue_title>
<issue_description>If I update a subscription's target branch, darc tells me that it successfully updated the subscription but the change is not respected:

C:\git\arcade-services\tools\FlatFlowMigrationCli>darc update-subscription --id 58cbbaad-a57e-4327-865a-609fa1c10968
Successfully updated subscription with id '58cbbaad-a57e-4327-865a-609fa1c10968'.

If we can't update target branches, we should at least error out during the save.</issue_description>

Comments on the Issue (you are @copilot in this section)
@dkurepa @ViktorHofer when this happened, did you use the `update-subscription` with the pop-up? @ViktorHofer Yes. Pop-up in notepad on Windows. @dkurepa Ah I see, thanks @dkurepa The best thing to do here is probably to error out. Ideally we wouldn't do this just for the target branch, but for other non mutable fields: - Id - targetRepository - target branch - source enabled
So after a popup is closed, we'd check if any of these are changed, and error out with all of them</comment_new>

Fixes #4725

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

When updating a subscription via popup, validate that immutable fields (Id, TargetRepository, TargetBranch, SourceEnabled) have not been changed. If any of these fields are modified, the operation will fail with an error listing all the immutable fields that were changed. Fixes the issue where changes to these fields were silently ignored. Co-authored-by: dkurepa <[email protected]>

Tests verify that the validation correctly: - Detects changes to Id, TargetRepository, TargetBranch, and SourceEnabled - Returns error code when immutable fields are changed - Logs clear error messages for each immutable field violation - Allows updates when only mutable fields are changed - Handles multiple immutable field changes simultaneously Co-authored-by: dkurepa <[email protected]>

dkurepa · 2025-10-08T10:32:17Z

@@ -0,0 +1,275 @@
+// Licensed to the .NET Foundation under one or more agreements.


@copilot delete this file

Done in commit 0b7fc8d

dkurepa · 2025-10-08T10:35:49Z

@@ -1,7 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.


@copilot it's easier to do these checks in the UpdateSubscriptionOperation rather than in the popup, because you have the original subscription there already. So remove all of the changes, and just add the checks in UpdateSubscriptionOperation before the assignments to values that are used to create the SubscriptionUpdate

Done in commit 0b7fc8d. Moved validation to UpdateSubscriptionOperation after the popup values are extracted, where the original subscription is already available. The validation checks TargetRepository, TargetBranch, and SourceEnabled before creating the SubscriptionUpdate object.

…tion Per review feedback, moved validation logic to UpdateSubscriptionOperation where the original subscription is already available. This is simpler and avoids the need to store the subscription in the popup class. - Reverted all changes to UpdateSubscriptionPopUp.cs - Deleted UpdateSubscriptionPopUpTests.cs - Added validation in UpdateSubscriptionOperation after popup values are extracted - Validates TargetRepository, TargetBranch, and SourceEnabled fields - Returns error with clear messages if any immutable fields are changed Co-authored-by: dkurepa <[email protected]>

Copilot

Pull Request Overview

This PR fixes a confusing UX issue where darc update-subscription would silently ignore changes to immutable fields and report success. The fix adds proper validation to detect and reject attempts to modify fields that cannot be changed due to how Maestro++ tracks pull requests.

Added validation logic to detect changes to immutable subscription fields
Enhanced error reporting to clearly list which fields cannot be modified
Updated documentation to explicitly specify all immutable fields and the new error behavior

Reviewed Changes

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

File	Description
src/Microsoft.DotNet.Darc/Darc/Operations/UpdateSubscriptionOperation.cs	Added validation logic to check for changes to immutable fields (TargetRepository, TargetBranch, SourceEnabled) and return appropriate error messages
docs/Darc.md	Updated documentation to clarify all immutable fields and that changes will result in an error rather than being silently ignored

Initial plan

560b731

Copilot AI assigned Copilot and dkurepa Oct 8, 2025

Copilot started work on behalf of dkurepa October 8, 2025 10:03 View session

Copilot AI and others added 2 commits October 8, 2025 10:12

Copilot AI changed the title ~~[WIP] Fix target branch update for darc subscriptions~~ Fix darc update-subscription to error when immutable fields are changed Oct 8, 2025

Copilot AI requested a review from dkurepa October 8, 2025 10:21

Copilot finished work on behalf of dkurepa October 8, 2025 10:21

dkurepa reviewed Oct 8, 2025

View reviewed changes

Copilot started work on behalf of dkurepa October 8, 2025 10:36 View session

Copilot AI requested a review from dkurepa October 8, 2025 10:41

Copilot finished work on behalf of dkurepa October 8, 2025 10:41

premun approved these changes Oct 8, 2025

View reviewed changes

dkurepa marked this pull request as ready for review October 8, 2025 10:45

Copilot AI review requested due to automatic review settings October 8, 2025 10:45

Copilot AI reviewed Oct 8, 2025

View reviewed changes

dkurepa enabled auto-merge (squash) October 8, 2025 10:45

ViktorHofer approved these changes Oct 8, 2025

View reviewed changes

dkurepa merged commit f1747a4 into main Oct 8, 2025
9 checks passed

dkurepa deleted the copilot/fix-update-subscription-target-branch branch October 8, 2025 12:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix darc update-subscription to error when immutable fields are changed#5348

Fix darc update-subscription to error when immutable fields are changed#5348
dkurepa merged 4 commits into
mainfrom
copilot/fix-update-subscription-target-branch

Copilot AI commented Oct 8, 2025 •

edited

Loading

Uh oh!

dkurepa Oct 8, 2025 •

edited

Loading

Uh oh!

Copilot AI Oct 8, 2025

Uh oh!

dkurepa Oct 8, 2025

Uh oh!

Copilot AI Oct 8, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		@@ -0,0 +1,275 @@
		// Licensed to the .NET Foundation under one or more agreements.

		@@ -1,7 +1,8 @@
		// Licensed to the .NET Foundation under one or more agreements.
		// The .NET Foundation licenses this file to you under the MIT license.

Uh oh!

Conversation

Copilot AI commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Changes

Testing

Comments on the Issue (you are @copilot in this section)

Uh oh!

dkurepa Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

dkurepa Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Copilot AI commented Oct 8, 2025 •

edited

Loading

dkurepa Oct 8, 2025 •

edited

Loading