Skip to content

Verify all destination remotes before pushing artifact - #2022

Open
mglaman wants to merge 4 commits into
acquia:mainfrom
mglaman:fix/push-artifact-multi-remote-verify
Open

Verify all destination remotes before pushing artifact#2022
mglaman wants to merge 4 commits into
acquia:mainfrom
mglaman:fix/push-artifact-multi-remote-verify

Conversation

@mglaman

@mglaman mglaman commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

push:artifact only cloned and fetched from the first destination git URL, then pushed to the rest blind. Because every run generates a fresh commit SHA, any drift between destinations (branch left over on one remote, concurrent builds, a partial push failure) made later pushes fail with a non-fast-forward rejection that only a force push could clear. This hit PR/build branches constantly while main survived because merges serialize its pushes.

Now the branch tip is fetched from every destination before the artifact is built. If tips differ but are ancestor-related, the build bases on the most advanced tip so every remote can fast-forward. If tips have truly diverged, the command aborts with an error naming each remote and its tip before anything is built. A push failure on one remote no longer skips the remaining remotes, which was how the drift started in the first place.

Motivation

Fixes #NNN

Proposed changes

Alternatives considered

Testing steps

  1. Follow the contribution guide to set up your development environment or download a pre-built acli.phar for this PR.
  2. If running from source, clear the kernel cache to pick up new and changed commands: ./bin/acli ckc
  3. Check for regressions: (add specific steps for this pr)
  4. Check new functionality: (add specific steps for this pr)

push:artifact only cloned and fetched from the first destination git
URL, then pushed to the rest blind. Because every run generates a fresh
commit SHA, any drift between destinations (branch left over on one
remote, concurrent builds, a partial push failure) made later pushes
fail with a non-fast-forward rejection that only a force push could
clear. This hit PR/build branches constantly while main survived
because merges serialize its pushes.

Now the branch tip is fetched from every destination before the
artifact is built. If tips differ but are ancestor-related, the build
bases on the most advanced tip so every remote can fast-forward. If
tips have truly diverged, the command aborts with an error naming each
remote and its tip before anything is built. A push failure on one
remote no longer skips the remaining remotes, which was how the drift
started in the first place.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 15, 2026 14:01
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.67%. Comparing base (92c4337) to head (ab67f31).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2022      +/-   ##
============================================
+ Coverage     92.49%   92.67%   +0.17%     
- Complexity     1995     2010      +15     
============================================
  Files           123      123              
  Lines          7238     7314      +76     
============================================
+ Hits           6695     6778      +83     
+ Misses          543      536       -7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

Copy link
Copy Markdown

Try the dev build for this PR: https://acquia-cli.s3.amazonaws.com/build/pr/2022/acli.phar

curl -OL https://acquia-cli.s3.amazonaws.com/build/pr/2022/acli.phar
chmod +x acli.phar

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.

Pull request overview

This PR improves push:artifact reliability when pushing to multiple destination Git remotes by verifying the destination branch tips across all remotes before building, selecting an appropriate base tip when remotes are ancestor-related, and aborting early when remotes have truly diverged.

Changes:

  • Fetch destination branch tips from every configured remote and select a base tip that allows fast-forward pushes to all remotes (or abort if diverged).
  • Continue pushing to remaining remotes even if one push fails, and report all push failures together.
  • Add PHPUnit coverage for diverged remotes, missing branches, behind remotes, new branches, and partial push failures.

Reviewed changes

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

File Description
src/Command/Push/PushArtifactCommand.php Fetch and compare tips across all destination remotes before building; choose a base tip; aggregate push failures.
tests/phpunit/src/Commands/Push/PushArtifactCommandTest.php Adds/extends tests and mocking helpers to cover multi-remote tip verification and partial push failure behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Command/Push/PushArtifactCommand.php
Comment thread src/Command/Push/PushArtifactCommand.php
Comment thread src/Command/Push/PushArtifactCommand.php
Comment thread tests/phpunit/src/Commands/Push/PushArtifactCommandTest.php Outdated
@acquia-stalebot-platauto

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Please remove the stale label to avoid it being closed. Thank you for your contributions. More info: https://github.com/acquia/devops-github-administration/blob/main/docs/operations_related_to_repositories.md#acquia-stale-bot

mglaman and others added 3 commits July 31, 2026 09:55
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Resolve setUpPushArtifact() signature conflict: keep both the $tips
parameter from this branch and $hasAutoloadRuntime from main.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add tests for unresolvable and empty fetched tips, failed history
deepening, and unexpected merge-base exit codes. Assert the full
out-of-sync message and the fetch progress output so the covered
mutation score stays at 100%.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

2 participants