Public Access
DEVX-49: fix: remove auto-rebase from auto-merge to prevent CI feedback loop
Post-merge / detect-type (push) Successful in 10s
Post-merge / validate-commit-msg (push) Successful in 16s
Post-merge / configure-repo (push) Successful in 17s
Post-merge / release (push) Successful in 1m13s
Post-merge / vikunja (push) Successful in 14s
Post-merge / badges (push) Successful in 57s
Post-merge / sync-wiki (push) Successful in 1m5s
Post-merge / detect-type (push) Successful in 10s
Post-merge / validate-commit-msg (push) Successful in 16s
Post-merge / configure-repo (push) Successful in 17s
Post-merge / release (push) Successful in 1m13s
Post-merge / vikunja (push) Successful in 14s
Post-merge / badges (push) Successful in 57s
Post-merge / sync-wiki (push) Successful in 1m5s
This commit was merged in pull request #75.
This commit is contained in:
@@ -237,21 +237,15 @@ def main(branch: str, pr_title: str, repo: str, pr_number: str) -> None:
|
|||||||
client.merge_pr(pr_num, merge_title)
|
client.merge_pr(pr_num, merge_title)
|
||||||
except APIError as e:
|
except APIError as e:
|
||||||
if e.status == 405 and "behind" in e.message.lower():
|
if e.status == 405 and "behind" in e.message.lower():
|
||||||
# Head branch is behind master — pull master and rebase, then retry
|
# Head branch is behind master — do NOT auto-rebase.
|
||||||
click.echo(_("Head branch is behind master. Pulling and rebasing..."))
|
# Auto-rebasing creates a feedback loop: the force-push triggers
|
||||||
try:
|
# a new pull_request synchronize event, which starts a new CI run,
|
||||||
run_cmd(["git", "config", "user.name", "devx-ci-bot"])
|
# which runs auto-merge again, which rebases again, etc.
|
||||||
run_cmd(["git", "config", "user.email", "devx-ci-bot@oblachno.fyi"])
|
|
||||||
run_cmd(["git", "fetch", "origin", "master"])
|
|
||||||
run_cmd(["git", "rebase", "origin/master"])
|
|
||||||
run_cmd(["git", "push", "--force-with-lease", "origin", f"HEAD:{branch}"])
|
|
||||||
click.echo(_("Rebased and pushed. Retrying merge..."))
|
|
||||||
client.merge_pr(pr_num, merge_title)
|
|
||||||
except (APIError, Exception) as retry_err:
|
|
||||||
raise click.ClickException(
|
raise click.ClickException(
|
||||||
_(
|
_(
|
||||||
"Merge failed after rebase retry: {error}\nPlease rebase the PR manually.",
|
"Branch is behind master. Rebase manually:\n"
|
||||||
error=str(retry_err),
|
" git fetch origin master && git rebase origin/master && git push --force-with-lease\n"
|
||||||
|
"Then re-add the ready-to-merge label.",
|
||||||
)
|
)
|
||||||
) from None
|
) from None
|
||||||
else:
|
else:
|
||||||
|
|||||||
@@ -383,6 +383,14 @@
|
|||||||
"ru": "Another molecule runner failed. Stopping this runner early.",
|
"ru": "Another molecule runner failed. Stopping this runner early.",
|
||||||
"zh": "Another molecule runner failed. Stopping this runner early."
|
"zh": "Another molecule runner failed. Stopping this runner early."
|
||||||
},
|
},
|
||||||
|
"Branch is behind master. Rebase manually:\n git fetch origin master && git rebase origin/master && git push --force-with-lease\nThen re-add the ready-to-merge label.": {
|
||||||
|
"bg": "Branch is behind master. Rebase manually:\n git fetch origin master && git rebase origin/master && git push --force-with-lease\nThen re-add the ready-to-merge label.",
|
||||||
|
"de": "Branch is behind master. Rebase manually:\n git fetch origin master && git rebase origin/master && git push --force-with-lease\nThen re-add the ready-to-merge label.",
|
||||||
|
"en": "Branch is behind master. Rebase manually:\n git fetch origin master && git rebase origin/master && git push --force-with-lease\nThen re-add the ready-to-merge label.",
|
||||||
|
"pl": "Gałąź jest w tyle za master. Wykonaj rebase ręcznie:\n git fetch origin master && git rebase origin/master && git push --force-with-lease\nNastępnie dodaj ponownie etykietę ready-to-merge.",
|
||||||
|
"ru": "Branch is behind master. Rebase manually:\n git fetch origin master && git rebase origin/master && git push --force-with-lease\nThen re-add the ready-to-merge label.",
|
||||||
|
"zh": "Branch is behind master. Rebase manually:\n git fetch origin master && git rebase origin/master && git push --force-with-lease\nThen re-add the ready-to-merge label."
|
||||||
|
},
|
||||||
"Bumping version: {current} -> v{new_version}": {
|
"Bumping version: {current} -> v{new_version}": {
|
||||||
"bg": "Bumping version: {current} -> v{new_version}",
|
"bg": "Bumping version: {current} -> v{new_version}",
|
||||||
"de": "Bumping version: {current} -> v{new_version}",
|
"de": "Bumping version: {current} -> v{new_version}",
|
||||||
@@ -639,14 +647,6 @@
|
|||||||
"ru": "HTTP {status} Запрещено — у вашего токена нет прав администратора.\nУбедитесь, что токен принадлежит владельцу репозитория или администратору организации.\nЛибо настройте защиту ветки вручную в разделе Настройки → Ветки.",
|
"ru": "HTTP {status} Запрещено — у вашего токена нет прав администратора.\nУбедитесь, что токен принадлежит владельцу репозитория или администратору организации.\nЛибо настройте защиту ветки вручную в разделе Настройки → Ветки.",
|
||||||
"zh": "HTTP {status} 禁止访问 — 您的令牌缺少管理员权限。\n请确保令牌属于仓库所有者或组织管理员。\n或者,您可以在 设置 → 分支 中手动配置分支保护。"
|
"zh": "HTTP {status} 禁止访问 — 您的令牌缺少管理员权限。\n请确保令牌属于仓库所有者或组织管理员。\n或者,您可以在 设置 → 分支 中手动配置分支保护。"
|
||||||
},
|
},
|
||||||
"Head branch is behind master. Pulling and rebasing...": {
|
|
||||||
"bg": "Head branch is behind master. Pulling and rebasing...",
|
|
||||||
"de": "Head branch is behind master. Pulling and rebasing...",
|
|
||||||
"en": "Head branch is behind master. Pulling and rebasing...",
|
|
||||||
"pl": "Gałąź head jest w tyle za master. Pobieranie i rebasing...",
|
|
||||||
"ru": "Head branch is behind master. Pulling and rebasing...",
|
|
||||||
"zh": "Head branch is behind master. Pulling and rebasing..."
|
|
||||||
},
|
|
||||||
"Host Docker not available, starting local dockerd...": {
|
"Host Docker not available, starting local dockerd...": {
|
||||||
"bg": "Хост Docker не е наличен, стартиране на локален dockerd...",
|
"bg": "Хост Docker не е наличен, стартиране на локален dockerd...",
|
||||||
"de": "Host-Docker nicht verfügbar, lokaler dockerd wird gestartet...",
|
"de": "Host-Docker nicht verfügbar, lokaler dockerd wird gestartet...",
|
||||||
@@ -719,14 +719,6 @@
|
|||||||
"ru": "Mapped file {file} not found. Update mapping.json or create the file.",
|
"ru": "Mapped file {file} not found. Update mapping.json or create the file.",
|
||||||
"zh": "Mapped file {file} not found. Update mapping.json or create the file."
|
"zh": "Mapped file {file} not found. Update mapping.json or create the file."
|
||||||
},
|
},
|
||||||
"Merge failed after rebase retry: {error}\nPlease rebase the PR manually.": {
|
|
||||||
"bg": "Merge failed after rebase retry: {error}\nPlease rebase the PR manually.",
|
|
||||||
"de": "Merge failed after rebase retry: {error}\nPlease rebase the PR manually.",
|
|
||||||
"en": "Merge failed after rebase retry: {error}\nPlease rebase the PR manually.",
|
|
||||||
"pl": "Scalanie nie powiodło się po ponownej próbie rebase: {error}\nProszę wykonać rebase PR ręcznie.",
|
|
||||||
"ru": "Merge failed after rebase retry: {error}\nPlease rebase the PR manually.",
|
|
||||||
"zh": "Merge failed after rebase retry: {error}\nPlease rebase the PR manually."
|
|
||||||
},
|
|
||||||
"Merge failed with HTTP {status}: {message}\nPlease check the PR is ready and you have merge rights.": {
|
"Merge failed with HTTP {status}: {message}\nPlease check the PR is ready and you have merge rights.": {
|
||||||
"bg": "Сливането неуспешно с HTTP {status}: {message}\nПроверете дали PR е готов и имате права за сливане.",
|
"bg": "Сливането неуспешно с HTTP {status}: {message}\nПроверете дали PR е готов и имате права за сливане.",
|
||||||
"de": "Merge fehlgeschlagen mit HTTP {status}: {message}\nBitte prüfen Sie, ob der PR bereit ist und Sie Merge-Rechte haben.",
|
"de": "Merge fehlgeschlagen mit HTTP {status}: {message}\nBitte prüfen Sie, ob der PR bereit ist und Sie Merge-Rechte haben.",
|
||||||
@@ -1007,14 +999,6 @@
|
|||||||
"ru": "Pushed release commit to master.",
|
"ru": "Pushed release commit to master.",
|
||||||
"zh": "Pushed release commit to master."
|
"zh": "Pushed release commit to master."
|
||||||
},
|
},
|
||||||
"Rebased and pushed. Retrying merge...": {
|
|
||||||
"bg": "Rebased and pushed. Retrying merge...",
|
|
||||||
"de": "Rebased and pushed. Retrying merge...",
|
|
||||||
"en": "Rebased and pushed. Retrying merge...",
|
|
||||||
"pl": "Rebase i wypchnięto. Ponowna próba scalenia...",
|
|
||||||
"ru": "Rebased and pushed. Retrying merge...",
|
|
||||||
"zh": "Rebased and pushed. Retrying merge..."
|
|
||||||
},
|
|
||||||
"Release creation failed: {error}": {
|
"Release creation failed: {error}": {
|
||||||
"bg": "Release creation failed: {error}",
|
"bg": "Release creation failed: {error}",
|
||||||
"de": "Release creation failed: {error}",
|
"de": "Release creation failed: {error}",
|
||||||
|
|||||||
@@ -253,31 +253,34 @@ class TestMain:
|
|||||||
@patch.dict("os.environ", {"REPO_TOKEN": "tok", "VIKUNJA_TOKEN": "tok"}, clear=True)
|
@patch.dict("os.environ", {"REPO_TOKEN": "tok", "VIKUNJA_TOKEN": "tok"}, clear=True)
|
||||||
@patch("devx.ci.auto_merge.validate_pr_title_matches_vikunja")
|
@patch("devx.ci.auto_merge.validate_pr_title_matches_vikunja")
|
||||||
@patch("devx.ci.auto_merge.GiteaClient")
|
@patch("devx.ci.auto_merge.GiteaClient")
|
||||||
def test_merge_behind_master_rebases(
|
def test_merge_behind_master_raises_no_rebase(
|
||||||
self, mock_client_cls: MagicMock, _mock_validate: MagicMock, tmp_path, monkeypatch
|
self, mock_client_cls: MagicMock, _mock_validate: MagicMock, tmp_path, monkeypatch
|
||||||
) -> None: # type: ignore[no-untyped-def]
|
) -> None: # type: ignore[no-untyped-def]
|
||||||
|
"""When branch is behind master, auto-merge should NOT rebase.
|
||||||
|
|
||||||
|
Auto-rebasing creates a feedback loop: the force-push triggers a new
|
||||||
|
pull_request synchronize event, which starts a new CI run, which runs
|
||||||
|
auto-merge again, which rebases again, etc.
|
||||||
|
"""
|
||||||
monkeypatch.chdir(tmp_path)
|
monkeypatch.chdir(tmp_path)
|
||||||
|
|
||||||
mock_client = MagicMock()
|
mock_client = MagicMock()
|
||||||
mock_client.get_pr_commits.return_value = [
|
mock_client.get_pr_commits.return_value = [
|
||||||
{"commit": {"message": "fix: resolve timeout"}},
|
{"commit": {"message": "fix: resolve timeout"}},
|
||||||
]
|
]
|
||||||
mock_client.merge_pr.side_effect = [
|
mock_client.merge_pr.side_effect = APIError(405, "HEAD branch is behind master")
|
||||||
APIError(405, "HEAD branch is behind master"),
|
|
||||||
None, # Second call succeeds
|
|
||||||
]
|
|
||||||
mock_client_cls.return_value = mock_client
|
mock_client_cls.return_value = mock_client
|
||||||
|
|
||||||
with patch("devx.ci.auto_merge.run_cmd") as mock_run:
|
|
||||||
runner = CliRunner()
|
runner = CliRunner()
|
||||||
result = runner.invoke(
|
result = runner.invoke(
|
||||||
main,
|
main,
|
||||||
["DEVX-19-fix-bug", "DEVX-19: Fix timeout", "owner/repo", "7"],
|
["DEVX-19-fix-bug", "DEVX-19: Fix timeout", "owner/repo", "7"],
|
||||||
)
|
)
|
||||||
assert result.exit_code == 0, result.output
|
assert result.exit_code != 0
|
||||||
assert mock_client.merge_pr.call_count == 2
|
assert "behind master" in result.output.lower()
|
||||||
# Should have fetched, rebased, and pushed
|
assert "rebase manually" in result.output.lower()
|
||||||
assert mock_run.call_count == 5 # config name, config email, fetch, rebase, push
|
# Must NOT have called merge_pr twice (no retry after rebase)
|
||||||
|
assert mock_client.merge_pr.call_count == 1
|
||||||
|
|
||||||
@patch.dict("os.environ", {"REPO_TOKEN": "tok", "VIKUNJA_TOKEN": "tok"}, clear=True)
|
@patch.dict("os.environ", {"REPO_TOKEN": "tok", "VIKUNJA_TOKEN": "tok"}, clear=True)
|
||||||
@patch("devx.ci.auto_merge.validate_pr_title_matches_vikunja")
|
@patch("devx.ci.auto_merge.validate_pr_title_matches_vikunja")
|
||||||
@@ -344,10 +347,10 @@ class TestMain:
|
|||||||
@patch.dict("os.environ", {"REPO_TOKEN": "tok", "VIKUNJA_TOKEN": "tok"}, clear=True)
|
@patch.dict("os.environ", {"REPO_TOKEN": "tok", "VIKUNJA_TOKEN": "tok"}, clear=True)
|
||||||
@patch("devx.ci.auto_merge.validate_pr_title_matches_vikunja")
|
@patch("devx.ci.auto_merge.validate_pr_title_matches_vikunja")
|
||||||
@patch("devx.ci.auto_merge.GiteaClient")
|
@patch("devx.ci.auto_merge.GiteaClient")
|
||||||
def test_rebase_retry_failure_raises(
|
def test_merge_behind_master_does_not_force_push(
|
||||||
self, mock_client_cls: MagicMock, _mock_validate: MagicMock, tmp_path, monkeypatch
|
self, mock_client_cls: MagicMock, _mock_validate: MagicMock, tmp_path, monkeypatch
|
||||||
) -> None: # type: ignore[no-untyped-def]
|
) -> None: # type: ignore[no-untyped-def]
|
||||||
"""When rebase retry also fails, raises with helpful message."""
|
"""Verify no git commands are run when branch is behind master."""
|
||||||
monkeypatch.chdir(tmp_path)
|
monkeypatch.chdir(tmp_path)
|
||||||
|
|
||||||
mock_client = MagicMock()
|
mock_client = MagicMock()
|
||||||
@@ -358,14 +361,14 @@ class TestMain:
|
|||||||
mock_client_cls.return_value = mock_client
|
mock_client_cls.return_value = mock_client
|
||||||
|
|
||||||
with patch("devx.ci.auto_merge.run_cmd") as mock_run:
|
with patch("devx.ci.auto_merge.run_cmd") as mock_run:
|
||||||
mock_run.side_effect = click.ClickException("git rebase failed")
|
|
||||||
runner = CliRunner()
|
runner = CliRunner()
|
||||||
result = runner.invoke(
|
result = runner.invoke(
|
||||||
main,
|
main,
|
||||||
["DEVX-19-fix-bug", "DEVX-19: Fix timeout", "owner/repo", "7"],
|
["DEVX-19-fix-bug", "DEVX-19: Fix timeout", "owner/repo", "7"],
|
||||||
)
|
)
|
||||||
assert result.exit_code != 0
|
assert result.exit_code != 0
|
||||||
assert "rebase" in result.output.lower()
|
# No git commands should be run (no rebase, no push)
|
||||||
|
mock_run.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
def test_main_module_block() -> None:
|
def test_main_module_block() -> None:
|
||||||
|
|||||||
Reference in New Issue
Block a user