Skip to content

fix: don't abort on failure for 'comp update' and 'comp refresh-upstream-commit' - #359

Merged
Nan Liu (liunan-ms) merged 2 commits into
microsoft:mainfrom
ddstreet:dont-abort-on-failure
Sep 21, 2026
Merged

Nan Liu (liunan-ms) merged 2 commits into
microsoft:mainfrom
ddstreet:dont-abort-on-failure

Conversation

@ddstreet

Copy link
Copy Markdown
Contributor

When using 'comp update -a' or '--without-lockfile comp refresh-upstream-commit -a', a failure of any single component will abort the entire process, instead of completing as many as possible and then reporting which components failed. This fixes that behavior to process all components that don't fail the update/refresh, and report any failing components.

Copilot AI lite review requested due to automatic review settings September 21, 2026 15:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

A couple of newly introduced user-facing messages/help text are misleading in edge cases (e.g., “successful files were updated” when all components fail, and ambiguity with '--check-only').

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 Low severity

Open (2)
What changed in this PR

This PR changes the azldev component update and azldev --without-lockfile component refresh-upstream-commit workflows so that, when running across many components (-a / include-all), a failure in one component no longer aborts the entire run. Instead, the commands preserve successful per-component outputs, then return an aggregated error listing the failing components (and skip orphan pruning on partial failure).

Changes:

  • Stop cancelling parallel resolution on first component failure; continue resolving remaining components.
  • Save successful lock/TOML outputs even when other components fail, then return an aggregate error.
  • Add unit tests validating partial-success behavior; update user-facing text/docs to describe the new semantics.
File Description
internal/​app/​azldev/​cmds/​component/​update.go Writes successful lock updates before returning aggregate error; no longer cancels remaining work on first failure.
internal/​app/​azldev/​cmds/​component/​update_test.go Adds coverage for continuing after per-component resolution errors and preserving successful lock writes.
internal/​app/​azldev/​cmds/​component/​refreshupstreamcommit.go Mirrors update behavior for upstream-commit TOML generation (partial save + aggregate error).
internal/​app/​azldev/​cmds/​component/​refreshupstreamcommit_resolution_test.go Adds tests ensuring refresh continues after component errors and saves successes.
docs/​user/​reference/​cli/​azldev_component_update.md Updates CLI reference text to reflect partial-success behavior.
docs/​user/​how-to/​preview-without-lockfile.md Documents partial-success refresh behavior when resolving multiple components.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/app/azldev/cmds/component/refreshupstreamcommit.go Outdated
Comment thread internal/app/azldev/cmds/component/update.go
Copilot AI review requested due to automatic review settings September 21, 2026 16:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Several newly added/modified lines are likely to fail the repo’s enabled lll linter and a couple of updated aggregate error messages can be misleading in common “no files actually written” cases.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 High severity

Open (2)
Resolved since last review (2)

Comment thread internal/app/azldev/cmds/component/refreshupstreamcommit.go Outdated
Comment thread internal/app/azldev/cmds/component/update.go
Copilot AI review requested due to automatic review settings September 21, 2026 16:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The new aggregate error messages can incorrectly claim files “were updated” even when no writes occurred (e.g., all non-failing components were already up-to-date).

Review effort: Lite
Findings: None

Resolved since last review (2)
Previously missed (2)

In code that hasn't changed since last review

Low severity Avoid claiming upstream commit TOML files were updated unconditionally

internal/​app/​azldev/​cmds/​component/​refreshupstreamcommit.go:458

This message unconditionally states successful upstream commit TOML files were updated, but if the non-failing components were already current there may be nothing to write. Consider using wording that doesn’t claim disk updates definitely happened.

Low severity Avoid claiming lock files were updated when no writes occurred

internal/​app/​azldev/​cmds/​component/​update.go:647

The error message here unconditionally says successful lock files were updated, but this function doesn't know whether any writes actually occurred (e.g., the only non-failing components could already be up-to-date or unchanged). Consider wording this more defensively to avoid misleading users.

Copilot AI added 2 commits September 21, 2026 13:01
Record per-component upstream resolution failures without cancelling the
remaining refresh workers. Successful component TOML files are created or
updated before the command returns the aggregate error, while failed
components remain unchanged and orphan pruning is skipped.

External cancellation still stops pending work. Add regression coverage for
continued processing and partial-success persistence, and document the
behavior.
Record per-component source resolution failures without cancelling the
remaining update workers. Successful component lock files are written before
the command returns the aggregate error, while failed component locks remain
unchanged and orphan pruning is skipped.

External cancellation still stops pending work. Preserve write-free
--check-only behavior, add regression coverage for partial-success updates,
and regenerate the command documentation.
@liunan-ms
Nan Liu (liunan-ms) merged commit 6afcd87 into microsoft:main Sep 21, 2026
19 checks passed
@ddstreet
Dan Streetman (ddstreet) deleted the dont-abort-on-failure branch September 21, 2026 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants