From aff334e82b003cd375e449dd148ceaf12a0faed8 Mon Sep 17 00:00:00 2001 From: claude-bot Date: Mon, 8 Jun 2026 16:21:48 +0300 Subject: [PATCH] fix(merge-gate): SHA-in-main as sole merge-verify criterion + main regression guard Root-cause fix for main erosion (phantom merge): code of ORCH-067/069 reached `done` while absent from origin/main (only their auto docs-PRs landed). - FR-1: verify_merged_to_main confirms merge ONLY by `git merge-base --is-ancestor origin/main`; the OR-branch pr_already_merged is removed (a merged PR no longer confirms). Empty SHA / git error -> False. - FR-2: pr_already_merged demoted to merge_pr idempotency-guard; counts a PR only when merged & head.ref== & base.ref=="main" (explicit in-loop filter). - FR-3: merge_pr selects the open code-PR by head== AND base==main. - FR-5: new deterministic check_main_regression in _handle_merge_verify (after confirmed SHA-in-main, before done) verifies MAIN_REGRESSION_MARKERS still in origin/main; deterministic count==0 -> alert "main regressed" + HOLD (NOT done, no rollback); git error of the grep -> fail-open. Kill-switch ORCH_REGRESSION_GUARD_ENABLED; non-self -> no-op. - FR-4: root .gitattributes `CHANGELOG.md merge=union` so Unreleased edits auto-merge on rebase without conflict (branch not rolled back). Invariants unchanged (STAGE_TRANSITIONS, QG_CHECKS, deploy-status, merge-gate, image-freshness, DB schema, external HTTP API); non-self repos no-op (INV-5); never-raise (INV-1); merge only via Gitea PR-API (INV-2). Docs: CHANGELOG, .env.example (README/ADR updated by architect). Tests: tests/test_orch073_*.py (TC-01..18); existing merge-gate tests updated for the new code-PR filter. Refs: ORCH-073 Co-Authored-By: Claude Opus 4.7 --- .env.example | 21 +++ .gitattributes | 13 ++ CHANGELOG.md | 1 + src/config.py | 13 ++ src/merge_gate.py | 195 +++++++++++++++++++++---- src/stage_engine.py | 58 ++++++++ tests/conftest.py | 4 + tests/test_merge_actor.py | 6 +- tests/test_merge_gate.py | 11 +- tests/test_merge_gate_race.py | 4 +- tests/test_merge_verify.py | 33 +++-- tests/test_orch073_conditionality.py | 93 ++++++++++++ tests/test_orch073_gitattributes.py | 85 +++++++++++ tests/test_orch073_merge_pr.py | 106 ++++++++++++++ tests/test_orch073_merge_verify.py | 99 +++++++++++++ tests/test_orch073_pr_classify.py | 78 ++++++++++ tests/test_orch073_regression_guard.py | 114 +++++++++++++++ 17 files changed, 887 insertions(+), 47 deletions(-) create mode 100644 .gitattributes create mode 100644 tests/test_orch073_conditionality.py create mode 100644 tests/test_orch073_gitattributes.py create mode 100644 tests/test_orch073_merge_pr.py create mode 100644 tests/test_orch073_merge_verify.py create mode 100644 tests/test_orch073_pr_classify.py create mode 100644 tests/test_orch073_regression_guard.py diff --git a/.env.example b/.env.example index 6c81285..55cd373 100644 --- a/.env.example +++ b/.env.example @@ -50,6 +50,27 @@ ORCH_MERGE_RETEST_TARGET=tests/ ORCH_MERGE_LOCK_TIMEOUT_S=300 ORCH_MERGE_DEFER_DELAY_S=60 ORCH_MERGE_DEFER_MAX_ATTEMPTS=5 +# ORCH-071/073: merge-verify under-gate on the `deploy -> done` edge (врезка in +# advance_stage, NOT a new STAGE_TRANSITIONS edge / registered QG). A deterministic +# merge-actor merges the feature code-PR via the Gitea PR-merge API (never push/ +# force-push to main), then `done` is allowed ONLY when the deployed SHA is proven an +# ancestor of origin/main (ORCH-073 FR-1: SHA-in-main is the single criterion; a +# merged PR alone no longer confirms). A secondary regression guard then checks a +# declarative marker set (MAIN_REGRESSION_MARKERS) is still in origin/main; a missing +# marker -> alert + HOLD (NOT done), a git error of the grep itself -> fail-open. +# MERGE_VERIFY_ENABLED -> global kill-switch (false -> strictly pre-ORCH-071). +# MERGE_VERIFY_REPOS -> CSV of repos where the under-gate is REAL; empty -> +# only the self-hosting repo (orchestrator); non-self -> no-op. +# MERGE_PR_TIMEOUT_S -> per Gitea list/merge HTTP call timeout. +# MERGE_VERIFY_TIMEOUT_S -> git fetch/merge-base timeout for the ancestor + marker checks. +# REGRESSION_GUARD_ENABLED -> kill-switch for the ORCH-073 main-integrity regression +# guard (false -> SHA-in-main alone gates done); reuses the +# merge-verify scope, so non-self repos are a no-op. +ORCH_MERGE_VERIFY_ENABLED=true +ORCH_MERGE_VERIFY_REPOS= +ORCH_MERGE_PR_TIMEOUT_S=60 +ORCH_MERGE_VERIFY_TIMEOUT_S=60 +ORCH_REGRESSION_GUARD_ENABLED=true # ORCH-036: executable self-deploy of the `deploy` stage. For the self-hosting repo # (orchestrator) the stage REALLY restarts prod (8500) via a detached host hook; # deploy_status: SUCCESS means proven health-ok, not an LLM declaration. Three diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..a3d703c --- /dev/null +++ b/.gitattributes @@ -0,0 +1,13 @@ +# ORCH-073 (ADR-001 Р-5 / FR-4): union merge for the append-only changelog. +# +# CHANGELOG.md is append-only at the top (## [Unreleased]). Without a merge driver, +# two branches that both add an Unreleased entry collide on auto_rebase_onto_main +# (merge_gate), which rolls the branch back to `development` and can drag in stale +# neighbouring code (a phantom-merge amplifier — see ADR-001 root cause #3). The +# built-in `union` driver keeps BOTH sides' lines instead of conflicting, so both +# changelog entries survive and the branch is not rolled back. +# +# Scope is INTENTIONALLY limited to CHANGELOG.md: `union` only suits strictly +# append-only files. docs/**/*.md (README, ADR, internals) are rewritten line-by-line, +# where `union` would silently duplicate edited lines — so they are NOT included. +CHANGELOG.md merge=union diff --git a/CHANGELOG.md b/CHANGELOG.md index 5184332..531cd63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Формат: [Keep a Changelog](https://keepachangelog.com/). Записи — на смысловой PR/задачу. ## [Unreleased] +- **CRIT: системный фикс эрозии `main` — SHA-в-main как единственный критерий merge-verify + регресс-гард + `.gitattributes`** (ORCH-073): устранён корень фантомного merge, из-за которого код задач ORCH-067 (`plane_issue_link`) и ORCH-069 (`qg0_title_max`) дошёл до `done`, но физически отсутствовал в `origin/main` (в `main` попадали только их авто docs-PR). **(FR-1)** `merge_gate.verify_merged_to_main` подтверждает merge **ТОЛЬКО** прямым фактом `git merge-base --is-ancestor origin/main` — OR-ветка `pr_already_merged` удалена (merged PR больше не подтверждает merge); пустой SHA / git-ошибка → `False` (fail-closed, never-raise). **(FR-2)** `pr_already_merged` понижен до idempotency-guard для `merge_pr` и засчитывает PR лишь при `merged & head.ref== & base.ref=="main"` (явный in-loop фильтр вместо ненадёжного query-параметра `head` — исключает авто docs-PR). **(FR-3)** `merge_pr` выбирает open code-PR строго по `head.ref==` И `base.ref=="main"`; merge только через Gitea PR-merge API, никогда push/force-push в `main`. **(FR-5)** новый детерминированный регресс-гард `merge_gate.check_main_regression` в `_handle_merge_verify` ПОСЛЕ подтверждённого SHA-в-main и ДО `done` проверяет, что `origin/main` содержит декларативный append-only набор маркеров ранее-merged задач (`MAIN_REGRESSION_MARKERS`, `git grep -c origin/main -- `); детерминированный `count==0` → alert «main regressed» + HOLD (`set_issue_blocked` + Telegram + Plane, задача НЕ `done`, БЕЗ авто-отката на `development`), git-ошибка самого грепа → fail-OPEN (не блокирует, SHA-в-main остаётся первичным гейтом). Kill-switch `ORCH_REGRESSION_GUARD_ENABLED` (дефолт `true`), область — `merge_verify_applies` (self-hosting / `merge_verify_repos`), non-self → no-op. **(FR-4)** корневой `.gitattributes` с `CHANGELOG.md merge=union` — правки `## [Unreleased]` авто-сливаются при `auto_rebase_onto_main` без конфликта (обе записи сохраняются), ветка не откатывается в `development` и не тащит устаревший код-сосед; `docs/**` под union НЕ ставится. `GET /queue::merge_verify_status` дополнен счётчиком `main_regressed_alerts_total` (read-only). Инварианты НЕ менялись: `STAGE_TRANSITIONS`, реестр `QG_CHECKS` (под-гейт — врезка в `advance_stage`), `check_deploy_status`/`_parse_deploy_status`, merge-gate, image-freshness, схема БД, внешние HTTP-эндпоинты; non-self (enduro) merge/verify/гард — no-op (INV-5); ручной `Confirm Deploy` сохранён. ADR `docs/work-items/ORCH-073/06-adr/ADR-001-merge-verify-sha-truth-and-regression-guard.md` (+ сквозной `adr-0014`). Документация: `docs/architecture/README.md`, `.env.example`. Тесты: `tests/test_orch073_*.py` (TC-01..18). - **Конфигурируемый верхний лимит длины заголовка QG-0 (`ORCH_QG0_TITLE_MAX`, дефолт 200)** (ORCH-069): хардкод `if len(name) > 80` во входной валидации `_qg0_errors` (`src/webhooks/plane.py`) вынесен в настраиваемый параметр `Settings.qg0_title_max` (env `ORCH_QG0_TITLE_MAX`, дефолт 200). Лимит 80 был гигиеническим, а не структурным (slug режется независимо `[:30]`, `tasks.title TEXT` без ограничения), поэтому валидные заголовки 81–200 символов отклонялись на входе без бизнес-причины. Лимит читается из `settings.qg0_title_max` динамически на каждый вызов (тесты патчат значение), текст ошибки подставляет актуальное число; граница строгая (`len > limit` → FAIL, `len == limit` → PASS). **Graceful-деградация (AC-3, self-hosting safety):** пустое/нечисловое значение env не роняет процесс на старте — `field_validator(mode="before")` `_qg0_title_max_default` в `src/config.py` перехватывает сырое env ДО `int`-парсинга pydantic и при невалидном/пустом входе возвращает дефолт 200 (never-raise), гася `ValidationError`. Чисто аддитивно и обратносовместимо: дефолт 200 > прежних 80 → все ранее проходившие заголовки проходят (AC-7). Инварианты НЕ менялись: `STAGE_TRANSITIONS`, реестр `QG_CHECKS` (QG-0 — inline-валидация входа, не зарегистрированный stage-gate), схема БД, slug-логика `[:30]`, нижние лимиты (`< 5` title, `< 20` description), soft-QG-0 поведение (warning на `work_item.created`), API. ADR `docs/work-items/ORCH-069/06-adr/ADR-001-configurable-qg0-title-limit.md`. Документация: `.env.example`, `.env.staging.example`. Тесты: `tests/test_qg0_title_limit.py`. ### Added diff --git a/src/config.py b/src/config.py index 48fd249..65e078e 100644 --- a/src/config.py +++ b/src/config.py @@ -396,6 +396,19 @@ class Settings(BaseSettings): merge_pr_timeout_s: int = 60 merge_verify_timeout_s: int = 60 + # ORCH-073 (ADR-001 Р-4): main-integrity regression guard. After the merge-verify + # under-gate confirms the deployed SHA is an ancestor of origin/main (FR-1), a + # secondary deterministic (no-LLM) guard checks that a declarative set of markers + # for recently-merged tasks (MAIN_REGRESSION_MARKERS in merge_gate.py) is still + # present in origin/main — i.e. a CHANGELOG-rebase or phantom-merge did not silently + # roll back a neighbouring task's code. A missing marker (deterministic count==0) -> + # ALERT + HOLD (task stays on `deploy`, NOT done); an infra/git error on the grep + # itself -> fail-OPEN (do not block done; SHA-in-main remains the primary gate). + # regression_guard_enabled -> kill-switch (env ORCH_REGRESSION_GUARD_ENABLED); + # reuses the merge_verify_applies scope (self-hosting / + # merge_verify_repos), so non-self repos are a no-op. + regression_guard_enabled: bool = True + # Telegram notifications telegram_bot_token: str = "" telegram_chat_id: str = "" diff --git a/src/merge_gate.py b/src/merge_gate.py index 6b3eb7a..b8a51fe 100644 --- a/src/merge_gate.py +++ b/src/merge_gate.py @@ -445,25 +445,30 @@ def reclaim_stale_lease(repo: str) -> bool: # ORCH-065: idempotent merge finalization guard (Problem C) # --------------------------------------------------------------------------- def pr_already_merged(repo: str, branch: str) -> bool: - """Return True iff the PR for ``branch`` is ALREADY merged (ADR-001 Р-3, FR-3.2). + """Return True iff the **code-PR of ``branch``** is ALREADY merged (idempotency-guard). - A deterministic, read-only guard the merge path consults BEFORE attempting a - (second) merge so a re-driven / reaped task is idempotent: an already-merged - PR -> no-op, never a duplicate merge and never an error. This is the ONLY new - merge-related helper and it does NOT merge — it only READS the PR state via - the existing Gitea client, so it does not introduce duplicate merge logic. + ORCH-073 ADR-001 Р-2 (FR-2): this is an **idempotency-guard for ``merge_pr``**, NOT + a source of truth for ``done`` (the only proof of merge is SHA-in-main, FR-1 / + ``verify_merged_to_main``). It lets a re-driven / reaped ``merge_pr`` be idempotent: + the code-PR is already merged -> no-op, never a duplicate merge. - Consultation point: the actual merge actor is the **deployer agent** (it merges - the feature PR at the start of the ``deploy`` stage — see webhooks/gitea.py), - so the wiring lives in the deployer prompt (``.openclaw/agents/deployer.md``), - which runs this exact function before any (re-)merge. The merge-gate quality - check (``qg.checks.check_branch_mergeable``) is intentionally NOT modified - (ORCH-065 AC-13: ``check_*`` behaviour unchanged) — it runs on the FIRST - deploy-staging -> deploy edge and does not re-run on a ``deploy``-stage re-drive, - which is exactly where the second-merge risk lives. + Root-cause fix (G4 audit): the previous implementation returned True for ANY + ``merged == True`` PR returned by ``GET /pulls?state=all&head=``. Gitea's + ``head`` query-param filters unreliably for a bare branch name, so auto docs-PRs + (staging/deploy logs, ``head=docs/*``) leaked into the result and were counted as + "merged" — the ORCH-067/069 phantom-merge. We now apply an EXPLICIT in-loop filter + instead of trusting the query-param: a PR counts only when it carries the code of + THIS feature-branch into ``main``: + + * ``pr.merged is True`` AND + * ``pr.head.ref == branch`` (the code of exactly this feature-branch) AND + * ``pr.base.ref == "main"`` (target is main, not a docs/other base). + + This excludes auto docs-PRs (different ``head.ref``) and PRs onto a non-``main`` + base, so a merged docs-PR can no longer make ``merge_pr`` skip a real code merge. Queries Gitea ``GET /repos/{owner}/{repo}/pulls?state=all&head=`` and - reports True when any matching PR has ``merged == True``. Never raises (AC-9): + reports True only when a matching PR passes the filter above. Never raises (AC-9): any HTTP/parse error -> ``False`` (conservative: "not known-merged" lets the normal gate re-evaluate rather than silently skipping a real merge). """ @@ -479,7 +484,11 @@ def pr_already_merged(repo: str, branch: str) -> bool: if resp.status_code != 200: return False for pr in resp.json() or []: - if pr.get("merged") is True: + if ( + pr.get("merged") is True + and pr.get("head", {}).get("ref") == branch + and pr.get("base", {}).get("ref") == "main" + ): return True return False except Exception as e: # noqa: BLE001 - never-raise contract @@ -505,6 +514,7 @@ def pr_already_merged(repo: str, branch: str) -> bool: _MERGE_VERIFY_COUNTERS: dict = { "merge_verified_total": 0, "not_merged_alerts_total": 0, + "main_regressed_alerts_total": 0, # ORCH-073 Р-4: regression-guard HOLD+alert count. "last_alert_wi": None, } @@ -526,6 +536,15 @@ def note_not_merged_alert(work_item_id: str | None) -> None: pass +def note_main_regressed_alert(work_item_id: str | None) -> None: + """Bump the 'main regressed (marker missing)' counter (ORCH-073 Р-4). Never raises.""" + try: + _MERGE_VERIFY_COUNTERS["main_regressed_alerts_total"] += 1 + _MERGE_VERIFY_COUNTERS["last_alert_wi"] = work_item_id + except Exception: # noqa: BLE001 - observability must never break a decision + pass + + def merge_verify_status() -> dict: """Snapshot of the merge-verify under-gate for GET /queue. Never raises.""" try: @@ -534,6 +553,7 @@ def merge_verify_status() -> dict: "repos": settings.merge_verify_repos or "", "merge_verified_total": _MERGE_VERIFY_COUNTERS["merge_verified_total"], "not_merged_alerts_total": _MERGE_VERIFY_COUNTERS["not_merged_alerts_total"], + "main_regressed_alerts_total": _MERGE_VERIFY_COUNTERS["main_regressed_alerts_total"], "last_alert_wi": _MERGE_VERIFY_COUNTERS["last_alert_wi"], } except Exception as e: # noqa: BLE001 - never-raise contract @@ -578,7 +598,10 @@ def merge_pr(repo: str, branch: str) -> tuple[bool, str]: Algorithm: 1. ``pr_already_merged`` -> True -> no-op ``(True, "already-merged")`` (INV-5/AC-9). 2. ``GET /repos/{owner}/{repo}/pulls?state=open`` -> the open PR whose head ref - == ``branch`` -> its index. No open PR -> ``(False, "no open PR")``. + == ``branch`` AND base ref == ``main`` -> its index. ORCH-073 ADR-001 Р-3 + (FR-3) adds the ``base == main`` filter so the actor merges exactly the + feature code-PR and never an auto docs-PR / a PR onto a foreign base. No + such open PR -> ``(False, "no open PR")``. 3. ``POST /repos/{owner}/{repo}/pulls/{index}/merge`` (Do: ``merge``) -> 200/201 -> ``(True, "merged PR #")``; otherwise ``(False, "")``. @@ -602,7 +625,10 @@ def merge_pr(repo: str, branch: str) -> tuple[bool, str]: return False, f"list PRs failed: HTTP {resp.status_code}" index = None for pr in resp.json() or []: - if pr.get("head", {}).get("ref") == branch: + if ( + pr.get("head", {}).get("ref") == branch + and pr.get("base", {}).get("ref") == "main" + ): index = pr.get("number") break if index is None: @@ -631,26 +657,32 @@ def merge_pr(repo: str, branch: str) -> tuple[bool, str]: def verify_merged_to_main(repo: str, branch: str, sha: str) -> bool: """Return True iff the deployed commit is confirmed merged into ``origin/main``. - Post-deploy verification (FR-2 / D4): the merge is confirmed when EITHER - * ``pr_already_merged(repo, branch)`` is True (Gitea ``PR.merged == true``), OR - * ``git merge-base --is-ancestor origin/main`` succeeds in the per-branch - worktree (after ``git fetch origin main``), i.e. the validated SHA is an - ancestor of the current ``origin/main``. + Post-deploy verification — ORCH-073 ADR-001 Р-1 (FR-1): the merge is confirmed by + the SINGLE, authoritative fact "the deployed commit IS an ancestor of the current + ``origin/main``": + + * after ``git fetch origin main`` (in the per-branch worktree), + ``git merge-base --is-ancestor origin/main`` returns ``rc == 0``. + + The former OR-branch ``pr_already_merged(repo, branch)`` was REMOVED: a merged + ``PR.merged == true`` is no longer sufficient to confirm a merge. That branch was + the ORCH-067/069 phantom-merge root cause — an auto docs-PR (staging/deploy logs) + counted as "merged" via the unreliable Gitea ``head`` query, turning merge-verify + falsely GREEN while the code-PR was never merged. ``pr_already_merged`` now serves + ONLY as an idempotency-guard inside ``merge_pr`` (Р-2/Р-3), never as proof of merge. ``sha`` is the validated commit (``image_freshness.validated_revision`` = - worktree ``git rev-parse HEAD``). An empty ``sha`` makes the git branch - inconclusive (only the PR-merged branch can then confirm). + worktree ``git rev-parse HEAD``). An empty ``sha`` is inconclusive -> ``False`` + (fail-closed: alert + HOLD), since the SHA-in-main check cannot run without it. Never-raise (INV-1/AC-7 / TC-04): any git/HTTP error -> ``False`` (= "not confirmed" -> fail-closed for ``done``: alert + HOLD). The exception is NEVER propagated into ``advance_stage``. """ try: - if pr_already_merged(repo, branch): - return True if not sha: logger.warning( - "verify_merged_to_main: empty SHA for %s/%s and PR not known-merged", + "verify_merged_to_main: empty SHA for %s/%s -> cannot confirm SHA-in-main", repo, branch, ) return False @@ -675,3 +707,110 @@ def verify_merged_to_main(repo: str, branch: str, sha: str) -> bool: "verify_merged_to_main unexpected error for %s/%s: %s", repo, branch, e ) return False + + +# --------------------------------------------------------------------------- +# ORCH-073 (ADR-001 Р-4): main-integrity regression guard. +# +# A secondary, deterministic (no-LLM) guard that runs in `_handle_merge_verify` +# AFTER the SHA-in-main check (verify_merged_to_main, FR-1) confirms the deployed +# commit, and BEFORE the task is stamped `done`. It checks that a DECLARATIVE set +# of markers for recently-merged tasks is still present in `origin/main` — i.e. a +# CHANGELOG-rebase / phantom-merge did not silently roll back a neighbouring task's +# code (the ORCH-067/069 failure mode, which SHA-in-main alone would not catch when +# the deployed SHA itself IS in main but a sibling's code is gone). +# --------------------------------------------------------------------------- + +# Declarative, append-only marker set (ADR-001 Р-4). Each future task that lands +# significant code SHOULD append its own (task, marker_substring, path) row so the +# guard protects it from a later phantom-merge / rebase rollback. Kept in code (not +# DB / Plane — a non-goal) so it versions together with the fix it protects. +MAIN_REGRESSION_MARKERS: list[tuple[str, str, str]] = [ + ("ORCH-067", "plane_issue_link", "src/notifications.py"), + ("ORCH-069", "qg0_title_max", "src/config.py"), + ("ORCH-071", "verify_merged_to_main", "src/merge_gate.py"), + ("ORCH-073", "check_main_regression", "src/merge_gate.py"), +] + + +def check_main_regression(repo: str, branch: str) -> tuple[bool, str]: + """Verify the declarative marker set is still present in ``origin/main``. + + ORCH-073 ADR-001 Р-4 (FR-5). For each ``(task, marker, path)`` in + ``MAIN_REGRESSION_MARKERS`` run ``git grep -c origin/main -- `` in + the per-branch worktree (after ``git fetch origin main``). A DETERMINISTIC count + of ``0`` for any marker means a neighbouring task's code was rolled back -> + regression. + + Returns ``(ok, reason)``: + * ``(True, "markers intact ()")`` — every marker present -> proceed. + * ``(False, "main regressed: ...")`` — a marker is deterministically + absent (count==0) -> caller HOLDs the task (NOT done) + alerts. + + **Fail-OPEN on infra error** (intentional trade-off, ADR-001 Р-4): any git/OS + error on the grep itself -> ``(True, "guard inconclusive: ")`` so a flaky + git never produces a false HOLD. "Regressed" is asserted ONLY on a deterministic + ``count == 0``, never on "could not determine". The PRIMARY fail-closed gate is + SHA-in-main (FR-1); this marker-grep is a secondary, best-effort guard. + + Never raises (INV-1): any unexpected error -> ``(True, "guard error: ...")``. + """ + try: + try: + wt = ensure_worktree(repo, branch) + except Exception as e: # noqa: BLE001 - never-raise contract -> fail-open + logger.warning( + "check_main_regression: worktree error for %s/%s: %s (fail-open)", + repo, branch, e, + ) + return True, f"guard inconclusive: worktree error: {e}" + + try: + subprocess.run( + ["git", "-C", wt, "fetch", "origin", "main"], + capture_output=True, timeout=settings.merge_verify_timeout_s, + ) + except (subprocess.SubprocessError, OSError) as e: + logger.warning( + "check_main_regression: fetch error for %s/%s: %s (fail-open)", + repo, branch, e, + ) + return True, f"guard inconclusive: fetch error: {e}" + + for task, marker, path in MAIN_REGRESSION_MARKERS: + try: + r = subprocess.run( + ["git", "-C", wt, "grep", "-c", marker, "origin/main", "--", path], + capture_output=True, text=True, timeout=_SHORT_TIMEOUT, + ) + except (subprocess.SubprocessError, OSError) as e: + # Infra error on this marker -> fail-open (do NOT assert regression). + logger.warning( + "check_main_regression: grep error for %s (%s @ %s): %s (fail-open)", + task, marker, path, e, + ) + return True, f"guard inconclusive: grep error for {task}: {e}" + # git grep exit codes: 0 = match(es) found, 1 = no match, >1 = real error. + if r.returncode == 0: + continue + if r.returncode == 1: + # Deterministic absence -> regression of a neighbouring task's code. + logger.warning( + "check_main_regression: marker MISSING in origin/main for %s " + "(%s @ %s) -> main regressed", task, marker, path, + ) + return False, f"main regressed: {task} code missing ({marker} @ {path})" + # rc > 1 -> git error (e.g. bad path/ref) -> inconclusive -> fail-open. + logger.warning( + "check_main_regression: ambiguous git grep rc=%s for %s (%s @ %s) " + "(fail-open)", r.returncode, task, marker, path, + ) + return True, f"guard inconclusive: git grep rc={r.returncode} for {task}" + + return True, f"markers intact ({len(MAIN_REGRESSION_MARKERS)})" + except Exception as e: # noqa: BLE001 - never-raise contract -> fail-open + logger.warning( + "check_main_regression unexpected error for %s/%s: %s (fail-open)", + repo, branch, e, + ) + return True, f"guard error: {e}" diff --git a/src/stage_engine.py b/src/stage_engine.py index c7bf165..c8427c8 100644 --- a/src/stage_engine.py +++ b/src/stage_engine.py @@ -1277,6 +1277,50 @@ def _deploy_finalize_defer_count(task_id: int) -> int: return n +def _hold_main_regressed( + task_id, repo, work_item_id, branch, guard_msg: str, result: AdvanceResult +) -> bool: + """HOLD the task because the regression guard found neighbouring code missing. + + ORCH-073 Р-4 (FR-5 / AC-5): the deployed SHA IS in `main` (FR-1 passed) but a + declarative marker of a recently-merged task is gone -> a phantom-merge / rebase + rolled back sibling code. Reaction is ALERT-only + HOLD (Telegram + Plane + ``set_issue_blocked`` + comment), task stays on `deploy` (NOT done), NO rollback + to development (an infra defect, not a code fault — symmetric to the not-merged + HOLD). Returns ``True`` (INTERVENED). Never breaks the HOLD on a notify error. + """ + merge_gate.note_main_regressed_alert(work_item_id) + msg = ( + f"main regressed: {guard_msg} (repo={repo}, branch={branch}, " + f"wi={work_item_id}). Соседний код пропал из `main` — задача удержана на " + f"`deploy` (НЕ done). Нужно ручное восстановление кода." + ) + logger.warning(f"Task {task_id}: {msg}") + if work_item_id: + try: + set_issue_blocked(work_item_id) + except Exception as e: # noqa: BLE001 - never break the HOLD + logger.warning(f"Task {task_id}: set_issue_blocked failed: {e}") + try: + plane_add_comment( + work_item_id, + "\U0001f6a8 Регресс `main`: " + guard_msg + ". Код соседней задачи " + "пропал из `main`. Задача удержана на `deploy` (НЕ done) — нужно " + "восстановить код и повторить approve.", + author="deployer", + ) + except Exception as e: # noqa: BLE001 - never break the HOLD + logger.warning(f"Task {task_id}: plane regressed comment failed: {e}") + try: + send_telegram(f"\U0001f6a8 {msg}") + except Exception as e: # noqa: BLE001 - never break the HOLD + logger.warning(f"Task {task_id}: main-regressed telegram failed: {e}") + result.alerted = True + result.note = "main-regressed-hold" + result.advanced = False + return True + + def _handle_merge_verify(task_id, repo, work_item_id, branch, result: AdvanceResult) -> bool: """ORCH-071 merge-verify under-gate on the `deploy -> done` edge. @@ -1317,6 +1361,20 @@ def _handle_merge_verify(task_id, repo, work_item_id, branch, result: AdvanceRes confirmed = merge_gate.verify_merged_to_main(repo, branch, sha) if confirmed: + # ORCH-073 Р-4 (FR-5): secondary main-integrity regression guard. The + # deployed SHA is in main (FR-1), but a CHANGELOG-rebase / phantom-merge + # could still have rolled back a NEIGHBOURING task's code. Verify the + # declarative marker set is intact; a deterministic miss -> HOLD + alert + # (NOT done, no rollback — an infra defect, not a code fault). Fail-OPEN + # on a git error of the guard itself (SHA-in-main remains the primary + # gate). Honours the same scope/kill-switch as the under-gate. + if settings.regression_guard_enabled: + guard_ok, guard_msg = merge_gate.check_main_regression(repo, branch) + if not guard_ok: + return _hold_main_regressed( + task_id, repo, work_item_id, branch, guard_msg, result + ) + merge_gate.note_merge_verified() try: self_deploy.record_merged_to_main(repo, work_item_id, branch, True) diff --git a/tests/conftest.py b/tests/conftest.py index 70cd388..26b8255 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -94,4 +94,8 @@ def _disable_merge_verify(monkeypatch): """ from src import config as _cfg monkeypatch.setattr(_cfg.settings, "merge_verify_enabled", False, raising=False) + # ORCH-073: the regression guard (check_main_regression) runs real git in + # _handle_merge_verify's confirmed branch. Default it OFF too so unrelated + # deploy->done tests stay 1:1; the dedicated ORCH-073 tests re-enable it. + monkeypatch.setattr(_cfg.settings, "regression_guard_enabled", False, raising=False) yield diff --git a/tests/test_merge_actor.py b/tests/test_merge_actor.py index 5065d35..01925da 100644 --- a/tests/test_merge_actor.py +++ b/tests/test_merge_actor.py @@ -42,7 +42,7 @@ def test_tc07_merge_actor_calls_gitea_merge(monkeypatch): def fake_get(url, params=None, headers=None, timeout=None): get_calls.append((url, params)) - return _Resp(200, [{"head": {"ref": branch}, "number": 7}]) + return _Resp(200, [{"head": {"ref": branch}, "base": {"ref": "main"}, "number": 7}]) def fake_post(url, json=None, headers=None, timeout=None): post_calls.append((url, json)) @@ -104,7 +104,7 @@ def test_tc09_never_raise_on_http_error(monkeypatch): def test_tc09_merge_endpoint_non_2xx_is_false(monkeypatch): monkeypatch.setattr(merge_gate, "pr_already_merged", lambda r, b: False) monkeypatch.setattr( - httpx, "get", lambda *a, **k: _Resp(200, [{"head": {"ref": "feature/ORCH-071-x"}, "number": 3}]) + httpx, "get", lambda *a, **k: _Resp(200, [{"head": {"ref": "feature/ORCH-071-x"}, "base": {"ref": "main"}, "number": 3}]) ) monkeypatch.setattr(httpx, "post", lambda *a, **k: _Resp(409, text="conflict")) ok, msg = merge_gate.merge_pr("orchestrator", "feature/ORCH-071-x") @@ -119,7 +119,7 @@ def test_tc09_merge_endpoint_non_2xx_is_false(monkeypatch): def test_tc13_no_shell_out_no_force_push(monkeypatch): monkeypatch.setattr(merge_gate, "pr_already_merged", lambda r, b: False) monkeypatch.setattr( - httpx, "get", lambda *a, **k: _Resp(200, [{"head": {"ref": "feature/ORCH-071-x"}, "number": 9}]) + httpx, "get", lambda *a, **k: _Resp(200, [{"head": {"ref": "feature/ORCH-071-x"}, "base": {"ref": "main"}, "number": 9}]) ) monkeypatch.setattr(httpx, "post", lambda *a, **k: _Resp(200)) diff --git a/tests/test_merge_gate.py b/tests/test_merge_gate.py index 6c692bf..0a12c4c 100644 --- a/tests/test_merge_gate.py +++ b/tests/test_merge_gate.py @@ -315,10 +315,17 @@ class _FakeResp: def test_tc16_pr_already_merged_true(monkeypatch): - """A merged PR -> True so a re-driven/reaped task is a no-op (no second merge).""" + """A merged code-PR -> True so a re-driven/reaped task is a no-op (no second merge). + + ORCH-073 FR-2: the guard now counts a PR only when it carries THIS branch's code + into main (merged & head.ref==branch & base.ref=="main"). + """ monkeypatch.setattr( httpx, "get", - lambda *a, **k: _FakeResp(200, [{"number": 7, "merged": True}]), + lambda *a, **k: _FakeResp( + 200, + [{"number": 7, "merged": True, "head": {"ref": "feature/x"}, "base": {"ref": "main"}}], + ), ) assert merge_gate.pr_already_merged("orchestrator", "feature/x") is True diff --git a/tests/test_merge_gate_race.py b/tests/test_merge_gate_race.py index 0e65fc6..8a885f6 100644 --- a/tests/test_merge_gate_race.py +++ b/tests/test_merge_gate_race.py @@ -204,7 +204,9 @@ def test_tc17_pr_already_merged_makes_redrive_a_noop(race_repo, monkeypatch): @staticmethod def json(): - return [{"merged": True}] + # ORCH-073 FR-2: the guard counts a PR only when it carries THIS branch's + # code into main (merged & head.ref==branch & base.ref=="main"). + return [{"merged": True, "head": {"ref": "feature/B"}, "base": {"ref": "main"}}] monkeypatch.setattr(httpx, "get", lambda *a, **k: _R()) assert merge_gate.pr_already_merged(repo, "feature/B") is True diff --git a/tests/test_merge_verify.py b/tests/test_merge_verify.py index 1d9f9b4..4d29b14 100644 --- a/tests/test_merge_verify.py +++ b/tests/test_merge_verify.py @@ -49,17 +49,22 @@ def test_tc01_verify_true_when_sha_is_ancestor(monkeypatch): # --------------------------------------------------------------------------- -# TC-02: PR.merged==true short-circuits to True even if git is unavailable. +# TC-02 (ORCH-073 FR-1): PR.merged==true NO LONGER confirms a merge. The former +# OR-branch was the phantom-merge root cause (a merged docs-PR turned verify green). +# SHA-in-main is now the SINGLE criterion; an empty SHA -> inconclusive -> False. # --------------------------------------------------------------------------- -def test_tc02_verify_true_when_pr_merged_even_without_git(monkeypatch): +def test_tc02_pr_merged_does_not_confirm_without_sha_in_main(monkeypatch): + # Even if a (docs-)PR is reported merged, that must NOT short-circuit to True. monkeypatch.setattr(merge_gate, "pr_already_merged", lambda r, b: True) - - def boom(*a, **k): - raise RuntimeError("git must NOT be consulted when PR is already merged") - - monkeypatch.setattr(merge_gate, "ensure_worktree", boom) - monkeypatch.setattr(merge_gate.subprocess, "run", boom) - assert merge_gate.verify_merged_to_main("orchestrator", "feature/ORCH-071-x", "") is True + monkeypatch.setattr(merge_gate, "ensure_worktree", lambda r, b: "/wt") + # SHA not an ancestor of origin/main (rc=1) -> not confirmed despite merged PR. + monkeypatch.setattr( + merge_gate.subprocess, "run", + lambda cmd, *a, **k: _R(1) if "merge-base" in cmd else _R(0), + ) + assert merge_gate.verify_merged_to_main("orchestrator", "feature/ORCH-071-x", "abc123") is False + # And an empty SHA is inconclusive -> False (cannot prove SHA-in-main). + assert merge_gate.verify_merged_to_main("orchestrator", "feature/ORCH-071-x", "") is False # --------------------------------------------------------------------------- @@ -93,11 +98,13 @@ def test_tc04_verify_never_raises_on_git_error(monkeypatch): assert merge_gate.verify_merged_to_main("orchestrator", "feature/ORCH-071-x", "abc123") is False -def test_tc04_verify_never_raises_on_http_error(monkeypatch): - def boom(r, b): - raise RuntimeError("gitea down") +def test_tc04_verify_never_raises_on_worktree_error(monkeypatch): + # ORCH-073: verify no longer consults pr_already_merged; a worktree/git error + # on the SHA-in-main path is the failure to swallow -> conservative False. + def boom(*a, **k): + raise RuntimeError("worktree exploded") - monkeypatch.setattr(merge_gate, "pr_already_merged", boom) + monkeypatch.setattr(merge_gate, "ensure_worktree", boom) assert merge_gate.verify_merged_to_main("orchestrator", "feature/ORCH-071-x", "abc123") is False diff --git a/tests/test_orch073_conditionality.py b/tests/test_orch073_conditionality.py new file mode 100644 index 0000000..c943bfc --- /dev/null +++ b/tests/test_orch073_conditionality.py @@ -0,0 +1,93 @@ +"""ORCH-073 — conditionality / backward-compat (INV-5). + +Covers TC-17/18 / AC-6. The whole under-gate and the regression guard are no-ops for +non-self repos and when their kill-switches are off, so enduro-trails and a disabled +self-host behave exactly as before. +""" + +import os +import tempfile + +os.environ.setdefault("ORCH_GITEA_TOKEN", "test-token") +os.environ.setdefault("ORCH_PLANE_API_TOKEN", "test-token") +os.environ.setdefault("ORCH_DB_PATH", os.path.join(tempfile.gettempdir(), "test_orch073_cond.db")) + +from unittest.mock import MagicMock # noqa: E402 + +from src import merge_gate, stage_engine, image_freshness # noqa: E402 +from src.stage_engine import AdvanceResult, _handle_merge_verify # noqa: E402 + +REPO = "orchestrator" +WI = "ORCH-073" +BRANCH = "feature/ORCH-073-x" + + +# --------------------------------------------------------------------------- +# TC-17 (AC-6/INV-5): non-self repo / kill-switch off -> under-gate is a no-op. +# --------------------------------------------------------------------------- +def test_tc17_merge_verify_applies_scope(monkeypatch): + monkeypatch.setattr(merge_gate.settings, "merge_verify_enabled", True) + monkeypatch.setattr(merge_gate.settings, "merge_verify_repos", "") + # Empty CSV -> only the self-hosting repo. + assert merge_gate.merge_verify_applies("orchestrator") is True + assert merge_gate.merge_verify_applies("enduro-trails") is False + # Kill-switch off -> no-op for everyone. + monkeypatch.setattr(merge_gate.settings, "merge_verify_enabled", False) + assert merge_gate.merge_verify_applies("orchestrator") is False + + +def test_tc17_under_gate_noop_for_non_self(monkeypatch): + # When the under-gate does not apply, _handle_merge_verify advances (False) and + # never touches the merge-actor / verifier / guard. + monkeypatch.setattr(stage_engine.merge_gate, "merge_verify_applies", lambda r: False) + + def must_not_call(*a, **k): + raise AssertionError("under-gate must be a no-op for non-self repos") + + monkeypatch.setattr(stage_engine.merge_gate, "merge_pr", must_not_call) + monkeypatch.setattr(stage_engine.merge_gate, "verify_merged_to_main", must_not_call) + monkeypatch.setattr(stage_engine.merge_gate, "check_main_regression", must_not_call) + + res = AdvanceResult() + assert _handle_merge_verify(1, "enduro-trails", WI, BRANCH, res) is False + assert res.alerted is False + + +# --------------------------------------------------------------------------- +# TC-18 (INV-5): regression guard respects its kill-switch -> no-op; SHA-in-main +# alone still advances the task. +# --------------------------------------------------------------------------- +def test_tc18_guard_kill_switch_skips_guard(monkeypatch): + monkeypatch.setattr(stage_engine.merge_gate, "merge_verify_applies", lambda r: True) + monkeypatch.setattr(stage_engine.settings, "regression_guard_enabled", False) + monkeypatch.setattr(image_freshness, "validated_revision", lambda r, b: "deadbeef") + monkeypatch.setattr(stage_engine.merge_gate, "merge_pr", lambda r, b: (True, "merged PR #1")) + monkeypatch.setattr(stage_engine.merge_gate, "verify_merged_to_main", lambda r, b, s: True) + monkeypatch.setattr( + stage_engine.merge_gate, "check_main_regression", + lambda r, b: (_ for _ in ()).throw(AssertionError("guard must not run when disabled")), + ) + monkeypatch.setattr( + stage_engine.self_deploy, "record_merged_to_main", MagicMock(return_value=True) + ) + for name in ("set_issue_blocked", "plane_add_comment", "send_telegram", "link_for"): + monkeypatch.setattr(stage_engine, name, MagicMock()) + + res = AdvanceResult() + # Guard disabled -> confirmed SHA-in-main advances straight to done (return False). + assert _handle_merge_verify(1, REPO, WI, BRANCH, res) is False + assert res.alerted is False + assert not stage_engine.set_issue_blocked.called + + +def test_tc18_guard_noop_for_non_self_repo(monkeypatch): + # check_main_regression is only invoked inside the confirmed branch which itself + # only runs when merge_verify_applies is True (self-hosting / CSV). For a non-self + # repo the guard is never reached. + monkeypatch.setattr(stage_engine.merge_gate, "merge_verify_applies", lambda r: False) + monkeypatch.setattr( + stage_engine.merge_gate, "check_main_regression", + lambda r, b: (_ for _ in ()).throw(AssertionError("guard must not run for non-self")), + ) + res = AdvanceResult() + assert _handle_merge_verify(1, "enduro-trails", WI, BRANCH, res) is False diff --git a/tests/test_orch073_gitattributes.py b/tests/test_orch073_gitattributes.py new file mode 100644 index 0000000..64dfb3a --- /dev/null +++ b/tests/test_orch073_gitattributes.py @@ -0,0 +1,85 @@ +"""ORCH-073 FR-4 — .gitattributes: CHANGELOG.md merge=union. + +Covers TC-11/TC-12 / AC-4. TC-11 asserts the repo-root .gitattributes declares the +union driver (git check-attr). TC-12 proves, in a throwaway git repo, that two +branches both editing '## [Unreleased]' merge WITHOUT a conflict and BOTH entries +survive — exactly what stops auto_rebase_onto_main from rolling a branch back. +""" + +import subprocess +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] + + +def _git(cwd, *args, env=None): + return subprocess.run( + ["git", *args], cwd=str(cwd), capture_output=True, text=True, env=env, + ) + + +# --------------------------------------------------------------------------- +# TC-11 (AC-4): the repo-root .gitattributes declares CHANGELOG.md merge=union. +# --------------------------------------------------------------------------- +def test_tc11_gitattributes_declares_union(): + ga = REPO_ROOT / ".gitattributes" + assert ga.is_file(), ".gitattributes must exist at the repo root" + assert "CHANGELOG.md merge=union" in ga.read_text(encoding="utf-8") + + r = _git(REPO_ROOT, "check-attr", "merge", "CHANGELOG.md") + assert r.returncode == 0, r.stderr + # Output form: 'CHANGELOG.md: merge: union' + assert "merge: union" in r.stdout, r.stdout + + +# --------------------------------------------------------------------------- +# TC-12 (AC-4): two Unreleased edits merge with no conflict; both kept. +# --------------------------------------------------------------------------- +def _init_repo(tmp_path): + env = { + "GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@t", + "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@t", + "GIT_CONFIG_GLOBAL": "/dev/null", "GIT_CONFIG_SYSTEM": "/dev/null", + "PATH": __import__("os").environ.get("PATH", ""), + "HOME": str(tmp_path), + } + repo = tmp_path / "repo" + repo.mkdir() + assert _git(repo, "init", "-b", "main", env=env).returncode == 0 + (repo / ".gitattributes").write_text("CHANGELOG.md merge=union\n", encoding="utf-8") + base = ( + "# Changelog\n\n## [Unreleased]\n\n### Common\n\n## [0.1.0]\n- initial\n" + ) + (repo / "CHANGELOG.md").write_text(base, encoding="utf-8") + _git(repo, "add", ".", env=env) + assert _git(repo, "commit", "-m", "base", env=env).returncode == 0 + return repo, env + + +def test_tc12_union_merge_keeps_both_entries(tmp_path): + repo, env = _init_repo(tmp_path) + + # Branch A adds its Unreleased line. + _git(repo, "checkout", "-b", "task-a", env=env) + txt = (repo / "CHANGELOG.md").read_text(encoding="utf-8") + (repo / "CHANGELOG.md").write_text( + txt.replace("### Common\n", "### Common\n- ORCH-A: feature A\n"), encoding="utf-8" + ) + _git(repo, "commit", "-am", "task A changelog", env=env) + + # Branch B (from main) adds a DIFFERENT Unreleased line at the same spot. + _git(repo, "checkout", "main", env=env) + _git(repo, "checkout", "-b", "task-b", env=env) + txt = (repo / "CHANGELOG.md").read_text(encoding="utf-8") + (repo / "CHANGELOG.md").write_text( + txt.replace("### Common\n", "### Common\n- ORCH-B: feature B\n"), encoding="utf-8" + ) + _git(repo, "commit", "-am", "task B changelog", env=env) + + # Merge A into B — union must avoid a conflict and keep BOTH lines. + m = _git(repo, "merge", "--no-edit", "task-a", env=env) + result = (repo / "CHANGELOG.md").read_text(encoding="utf-8") + assert m.returncode == 0, f"union merge must not conflict: {m.stdout}\n{m.stderr}" + assert "<<<<<<<" not in result and ">>>>>>>" not in result + assert "ORCH-A: feature A" in result + assert "ORCH-B: feature B" in result diff --git a/tests/test_orch073_merge_pr.py b/tests/test_orch073_merge_pr.py new file mode 100644 index 0000000..1f5be74 --- /dev/null +++ b/tests/test_orch073_merge_pr.py @@ -0,0 +1,106 @@ +"""ORCH-073 FR-3 — merge_pr merges exactly the feature code-PR (base==main). + +Covers TC-08..10 / AC-7 / INV-2/INV-4. The actor selects the open PR with +head==branch AND base==main (never an auto docs-PR / foreign base), merges via the +Gitea PR-merge API only (no push/force-push), and is idempotent on an already-merged +code-PR. Gitea HTTP is mocked; never-raise -> (False, reason). +""" + +import httpx +import pytest + +from src import merge_gate + +BRANCH = "feature/ORCH-073-x" + + +class _Resp: + def __init__(self, status_code, payload=None, text=""): + self.status_code = status_code + self._payload = payload if payload is not None else [] + self.text = text + + def json(self): + return self._payload + + +@pytest.fixture(autouse=True) +def _settings(monkeypatch): + monkeypatch.setattr(merge_gate.settings, "gitea_url", "http://gitea.test") + monkeypatch.setattr(merge_gate.settings, "gitea_owner", "admin") + monkeypatch.setattr(merge_gate.settings, "gitea_token", "tok") + monkeypatch.setattr(merge_gate.settings, "merge_pr_timeout_s", 5) + + +# --------------------------------------------------------------------------- +# TC-08: open code-PR (head==branch, base==main) -> POST /pulls/{n}/merge. +# A concurrently-open docs-PR (head=docs/*) must be skipped. +# --------------------------------------------------------------------------- +def test_tc08_merges_code_pr_not_docs_pr(monkeypatch): + monkeypatch.setattr(merge_gate, "pr_already_merged", lambda r, b: False) + post_calls = [] + + def fake_get(url, params=None, headers=None, timeout=None): + return _Resp(200, [ + {"head": {"ref": "docs/ORCH-073-log"}, "base": {"ref": "main"}, "number": 4}, + {"head": {"ref": BRANCH}, "base": {"ref": "main"}, "number": 7}, + ]) + + def fake_post(url, json=None, headers=None, timeout=None): + post_calls.append((url, json)) + return _Resp(200) + + monkeypatch.setattr(httpx, "get", fake_get) + monkeypatch.setattr(httpx, "post", fake_post) + + ok, msg = merge_gate.merge_pr("orchestrator", BRANCH) + assert ok is True and "PR #7" in msg + assert len(post_calls) == 1 + url, body = post_calls[0] + assert url.endswith("/repos/admin/orchestrator/pulls/7/merge") + assert body == {"Do": "merge"} + + +def test_tc08_skips_pr_onto_non_main_base(monkeypatch): + # Right head but base != main -> not a merge-to-main code-PR -> no open PR. + monkeypatch.setattr(merge_gate, "pr_already_merged", lambda r, b: False) + monkeypatch.setattr( + httpx, "get", + lambda *a, **k: _Resp(200, [{"head": {"ref": BRANCH}, "base": {"ref": "develop"}, "number": 9}]), + ) + monkeypatch.setattr(httpx, "post", lambda *a, **k: (_ for _ in ()).throw( + AssertionError("must not POST merge for a non-main base PR"))) + ok, msg = merge_gate.merge_pr("orchestrator", BRANCH) + assert ok is False and msg == "no open PR" + + +# --------------------------------------------------------------------------- +# TC-09 (INV-2): no open code-PR -> (False, "no open PR"); never shells out. +# --------------------------------------------------------------------------- +def test_tc09_no_open_pr_no_shell_out(monkeypatch): + monkeypatch.setattr(merge_gate, "pr_already_merged", lambda r, b: False) + monkeypatch.setattr(httpx, "get", lambda *a, **k: _Resp(200, [])) + subprocess_calls = [] + monkeypatch.setattr( + merge_gate.subprocess, "run", + lambda cmd, *a, **k: subprocess_calls.append(cmd), + ) + ok, msg = merge_gate.merge_pr("orchestrator", BRANCH) + assert ok is False and msg == "no open PR" + # No git push/force-push (or any subprocess) for the merge-actor. + assert subprocess_calls == [] + + +# --------------------------------------------------------------------------- +# TC-10 (AC-7/INV-4): already-merged code-PR -> no-op, no second POST merge. +# --------------------------------------------------------------------------- +def test_tc10_idempotent_already_merged(monkeypatch): + monkeypatch.setattr(merge_gate, "pr_already_merged", lambda r, b: True) + + def must_not_call(*a, **k): + raise AssertionError("no Gitea call when the code-PR is already merged") + + monkeypatch.setattr(httpx, "get", must_not_call) + monkeypatch.setattr(httpx, "post", must_not_call) + ok, msg = merge_gate.merge_pr("orchestrator", BRANCH) + assert ok is True and msg == "already-merged" diff --git a/tests/test_orch073_merge_verify.py b/tests/test_orch073_merge_verify.py new file mode 100644 index 0000000..8034738 --- /dev/null +++ b/tests/test_orch073_merge_verify.py @@ -0,0 +1,99 @@ +"""ORCH-073 FR-1 — verify_merged_to_main: SHA-in-main is the SINGLE criterion. + +Covers TC-01..04 / AC-2 / AC-6. The former OR-branch `pr_already_merged` was the +phantom-merge root cause and is removed: a merged docs-PR must NOT confirm a merge. +git/HTTP are mocked; the verifier honours the never-raise contract (INV-1). +""" + +import pytest + +from src import merge_gate + + +class _R: + """Minimal completed-subprocess stand-in (returncode only).""" + + def __init__(self, rc): + self.returncode = rc + self.stdout = "" + self.stderr = "" + + +@pytest.fixture(autouse=True) +def _settings(monkeypatch): + monkeypatch.setattr(merge_gate.settings, "merge_verify_enabled", True) + monkeypatch.setattr(merge_gate.settings, "merge_verify_timeout_s", 5) + + +# --------------------------------------------------------------------------- +# TC-01 (AC-6): sha is an ancestor of origin/main (merge-base rc=0) -> True. +# --------------------------------------------------------------------------- +def test_tc01_true_when_sha_is_ancestor(monkeypatch): + monkeypatch.setattr(merge_gate, "ensure_worktree", lambda r, b: "/wt") + calls = [] + + def fake_run(cmd, *a, **k): + calls.append(cmd) + return _R(0) # fetch ok; merge-base --is-ancestor -> 0 (ancestor) + + monkeypatch.setattr(merge_gate.subprocess, "run", fake_run) + assert merge_gate.verify_merged_to_main("orchestrator", "feature/ORCH-073-x", "abc123") is True + assert any( + "merge-base" in c and "--is-ancestor" in c and "origin/main" in c for c in calls + ) + + +# --------------------------------------------------------------------------- +# TC-02 (AC-2): sha NOT in main AND a merged docs-PR exists -> False. +# This is the exact ORCH-067/069 bug: a merged docs-PR must not confirm. +# --------------------------------------------------------------------------- +def test_tc02_false_when_sha_not_in_main_even_with_merged_docs_pr(monkeypatch): + # A merged docs-PR is present (mock returns True), but it must be IGNORED. + called = {"pr": False} + + def fake_pr_already_merged(r, b): + called["pr"] = True + return True + + monkeypatch.setattr(merge_gate, "pr_already_merged", fake_pr_already_merged) + monkeypatch.setattr(merge_gate, "ensure_worktree", lambda r, b: "/wt") + monkeypatch.setattr( + merge_gate.subprocess, "run", + lambda cmd, *a, **k: _R(1) if "merge-base" in cmd else _R(0), + ) + assert merge_gate.verify_merged_to_main("orchestrator", "feature/ORCH-073-x", "abc123") is False + # The merged-PR signal is no longer consulted by the verifier at all. + assert called["pr"] is False + + +# --------------------------------------------------------------------------- +# TC-03: empty sha -> inconclusive -> False (fail-closed), no git consulted. +# --------------------------------------------------------------------------- +def test_tc03_empty_sha_is_false(monkeypatch): + def boom(*a, **k): + raise AssertionError("git must NOT run for an empty SHA") + + monkeypatch.setattr(merge_gate, "ensure_worktree", boom) + monkeypatch.setattr(merge_gate.subprocess, "run", boom) + assert merge_gate.verify_merged_to_main("orchestrator", "feature/ORCH-073-x", "") is False + + +# --------------------------------------------------------------------------- +# TC-04 (INV-1): a git/OS error -> False, exception never propagated. +# --------------------------------------------------------------------------- +def test_tc04_never_raises_on_git_error(monkeypatch): + monkeypatch.setattr(merge_gate, "ensure_worktree", lambda r, b: "/wt") + + def boom(*a, **k): + raise OSError("git exploded") + + monkeypatch.setattr(merge_gate.subprocess, "run", boom) + assert merge_gate.verify_merged_to_main("orchestrator", "feature/ORCH-073-x", "abc123") is False + + +def test_tc04_never_raises_on_worktree_error(monkeypatch): + def boom(*a, **k): + raise RuntimeError("worktree down") + + monkeypatch.setattr(merge_gate, "ensure_worktree", boom) + assert merge_gate.verify_merged_to_main("orchestrator", "feature/ORCH-073-x", "abc123") is False diff --git a/tests/test_orch073_pr_classify.py b/tests/test_orch073_pr_classify.py new file mode 100644 index 0000000..c9570c5 --- /dev/null +++ b/tests/test_orch073_pr_classify.py @@ -0,0 +1,78 @@ +"""ORCH-073 FR-2 — pr_already_merged distinguishes code-PR from docs-PR. + +Covers TC-05..07. pr_already_merged is now an idempotency-guard: it counts a PR as +"merged" ONLY when it carries the code of THIS feature-branch into main +(merged & head.ref==branch & base.ref=="main"), excluding auto docs-PRs. Gitea HTTP +is mocked; never-raise -> False (INV-1). +""" + +import httpx +import pytest + +from src import merge_gate + +BRANCH = "feature/ORCH-073-x" + + +class _Resp: + def __init__(self, status_code, payload=None): + self.status_code = status_code + self._payload = payload if payload is not None else [] + + def json(self): + return self._payload + + +@pytest.fixture(autouse=True) +def _settings(monkeypatch): + monkeypatch.setattr(merge_gate.settings, "gitea_url", "http://gitea.test") + monkeypatch.setattr(merge_gate.settings, "gitea_owner", "admin") + monkeypatch.setattr(merge_gate.settings, "gitea_token", "tok") + + +# --------------------------------------------------------------------------- +# TC-05: a merged docs-PR (head=docs/*, base=main) is NOT counted as code-merge. +# --------------------------------------------------------------------------- +def test_tc05_merged_docs_pr_not_counted(monkeypatch): + payload = [ + {"merged": True, "head": {"ref": "docs/ORCH-073-staging-log"}, "base": {"ref": "main"}}, + {"merged": False, "head": {"ref": BRANCH}, "base": {"ref": "main"}}, + ] + monkeypatch.setattr(httpx, "get", lambda *a, **k: _Resp(200, payload)) + assert merge_gate.pr_already_merged("orchestrator", BRANCH) is False + + +# --------------------------------------------------------------------------- +# TC-06: a merged code-PR (head==branch, base==main) IS recognised. +# --------------------------------------------------------------------------- +def test_tc06_merged_code_pr_recognised(monkeypatch): + payload = [ + {"merged": True, "head": {"ref": BRANCH}, "base": {"ref": "main"}}, + ] + monkeypatch.setattr(httpx, "get", lambda *a, **k: _Resp(200, payload)) + assert merge_gate.pr_already_merged("orchestrator", BRANCH) is True + + +def test_tc06_merged_code_pr_onto_non_main_base_not_counted(monkeypatch): + # Right head but a foreign base (not main) must NOT count. + payload = [ + {"merged": True, "head": {"ref": BRANCH}, "base": {"ref": "develop"}}, + ] + monkeypatch.setattr(httpx, "get", lambda *a, **k: _Resp(200, payload)) + assert merge_gate.pr_already_merged("orchestrator", BRANCH) is False + + +# --------------------------------------------------------------------------- +# TC-07: HTTP error / non-200 -> False (never-raise, conservative). +# --------------------------------------------------------------------------- +def test_tc07_non_200_is_false(monkeypatch): + monkeypatch.setattr(httpx, "get", lambda *a, **k: _Resp(500, [])) + assert merge_gate.pr_already_merged("orchestrator", BRANCH) is False + + +def test_tc07_http_exception_is_false(monkeypatch): + def boom(*a, **k): + raise httpx.ConnectError("gitea unreachable") + + monkeypatch.setattr(httpx, "get", boom) + assert merge_gate.pr_already_merged("orchestrator", BRANCH) is False diff --git a/tests/test_orch073_regression_guard.py b/tests/test_orch073_regression_guard.py new file mode 100644 index 0000000..f19e63e --- /dev/null +++ b/tests/test_orch073_regression_guard.py @@ -0,0 +1,114 @@ +"""ORCH-073 FR-5 — main-integrity regression guard wired into _handle_merge_verify. + +Covers TC-13..16 / AC-3 / AC-5 / AC-6 / INV-1. Calls the under-gate handler directly +with mocked merge_gate primitives + side effects (Plane/Telegram). Asserts the +return contract: False == advance to `done`, True == HOLD (alert, NOT done). +""" + +import os +import tempfile + +os.environ.setdefault("ORCH_GITEA_TOKEN", "test-token") +os.environ.setdefault("ORCH_PLANE_API_TOKEN", "test-token") +os.environ.setdefault("ORCH_DB_PATH", os.path.join(tempfile.gettempdir(), "test_orch073_rg.db")) + +from unittest.mock import MagicMock # noqa: E402 + +import pytest # noqa: E402 + +from src import stage_engine, image_freshness # noqa: E402 +from src.stage_engine import AdvanceResult, _handle_merge_verify # noqa: E402 + +REPO = "orchestrator" +WI = "ORCH-073" +BRANCH = "feature/ORCH-073-x" + + +@pytest.fixture(autouse=True) +def _wire(monkeypatch): + # Under-gate is in scope for the self-hosting repo; guard enabled. + monkeypatch.setattr(stage_engine.merge_gate, "merge_verify_applies", lambda r: True) + monkeypatch.setattr(stage_engine.settings, "regression_guard_enabled", True) + monkeypatch.setattr(image_freshness, "validated_revision", lambda r, b: "deadbeef") + monkeypatch.setattr(stage_engine.merge_gate, "merge_pr", lambda r, b: (True, "merged PR #1")) + # Silence Plane/Telegram side effects (assert on .called where relevant). + for name in ("set_issue_blocked", "plane_add_comment", "send_telegram", "link_for"): + monkeypatch.setattr(stage_engine, name, MagicMock()) + monkeypatch.setattr( + stage_engine.self_deploy, "record_merged_to_main", MagicMock(return_value=True) + ) + + +# --------------------------------------------------------------------------- +# TC-13 (AC-6): SHA in main AND markers intact -> advance (return False), no alert. +# --------------------------------------------------------------------------- +def test_tc13_confirmed_and_intact_advances(monkeypatch): + monkeypatch.setattr(stage_engine.merge_gate, "verify_merged_to_main", lambda r, b, s: True) + monkeypatch.setattr(stage_engine.merge_gate, "check_main_regression", lambda r, b: (True, "markers intact (4)")) + + res = AdvanceResult() + intervened = _handle_merge_verify(1, REPO, WI, BRANCH, res) + + assert intervened is False # advance to done + assert res.alerted is False + assert not stage_engine.set_issue_blocked.called + + +# --------------------------------------------------------------------------- +# TC-14 (AC-3): SHA NOT in main (docs-only merge) -> HOLD + alert + Blocked. +# --------------------------------------------------------------------------- +def test_tc14_sha_not_in_main_holds(monkeypatch): + monkeypatch.setattr(stage_engine.merge_gate, "verify_merged_to_main", lambda r, b, s: False) + # Guard must never even run when SHA is not confirmed. + monkeypatch.setattr( + stage_engine.merge_gate, "check_main_regression", + lambda r, b: (_ for _ in ()).throw(AssertionError("guard must not run when not confirmed")), + ) + + res = AdvanceResult() + intervened = _handle_merge_verify(1, REPO, WI, BRANCH, res) + + assert intervened is True # HOLD + assert res.advanced is False + assert res.note == "merge-not-verified-hold" + assert stage_engine.set_issue_blocked.called + assert stage_engine.send_telegram.called + + +# --------------------------------------------------------------------------- +# TC-15 (AC-5): SHA in main BUT a marker missing -> HOLD + 'main regressed' alert. +# --------------------------------------------------------------------------- +def test_tc15_marker_missing_holds(monkeypatch): + monkeypatch.setattr(stage_engine.merge_gate, "verify_merged_to_main", lambda r, b, s: True) + monkeypatch.setattr( + stage_engine.merge_gate, "check_main_regression", + lambda r, b: (False, "main regressed: ORCH-067 code missing (plane_issue_link @ src/notifications.py)"), + ) + + res = AdvanceResult() + intervened = _handle_merge_verify(1, REPO, WI, BRANCH, res) + + assert intervened is True # HOLD, NOT done + assert res.advanced is False + assert res.note == "main-regressed-hold" + assert stage_engine.set_issue_blocked.called + assert stage_engine.send_telegram.called + + +# --------------------------------------------------------------------------- +# TC-16 (INV-1): an internal verifier error -> HOLD + alert, no exception escapes. +# --------------------------------------------------------------------------- +def test_tc16_internal_error_holds_never_raises(monkeypatch): + def boom(r, b, s): + raise RuntimeError("verifier exploded") + + monkeypatch.setattr(stage_engine.merge_gate, "verify_merged_to_main", boom) + + res = AdvanceResult() + # Must NOT raise. + intervened = _handle_merge_verify(1, REPO, WI, BRANCH, res) + + assert intervened is True # HOLD + assert res.advanced is False + assert res.alerted is True + assert "merge-verify-error" in (res.note or "")