feat(reaper): job-reaper + stale merge-lease reclaim + idempotent merge finalization
Closes the "zombie jobs" incident class: job status was set only inside the live launcher process, so a process death left jobs.status='running' forever; at max_concurrency=1 one zombie blocked ALL projects' queue (self-hosting risk). Adds a background daemon (src/job_reaper.py) with three-tier liveness (dead-pid streak / known exit_code / max-running backstop) whose only mutating write is an atomic terminal flip guarded by WHERE status='running' (no double-process). For exit0 the canonical QG is the source of truth via gate-driven advance, not "exit0". Also proactively reclaims stale merge-lease (dead pid OR TTL) via file delete only (no git ops), and makes merge finalization idempotent (pr_already_merged guard + up-to-date short-circuit on re-drive). New jobs.pid column via idempotent _ensure_column (no migration); pid stamped in launcher._spawn after Popen. Reaper start/stop in lifespan; "reaper" snapshot in GET /queue. Kill-switches: ORCH_REAPER_ENABLED, ORCH_REAPER_INTERVAL_S, ORCH_REAPER_DEAD_TICKS, ORCH_REAPER_MAX_RUNNING_S, ORCH_LEASE_RECLAIM_ENABLED. Invariants unchanged (AC-13): STAGE_TRANSITIONS, QG_CHECKS registry, check_branch_mergeable signature/behaviour, BUG-8 rollback, hook exit codes. restart-safe, never-raise per unit of background work. Docs: docs/architecture/README.md, CHANGELOG.md, .env.example. Tests: tests/test_job_reaper.py, tests/test_merge_lease_reclaim.py, tests/test_merge_gate.py (TC-16), tests/test_merge_gate_race.py (TC-17), tests/test_queue.py, tests/test_config.py (TC-19/TC-20). 742 passed. Refs: ORCH-065 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -165,3 +165,82 @@ def test_staging_infra_tolerance_env_override_true(monkeypatch):
|
||||
"""The field is read verbatim from its ORCH_* env var."""
|
||||
monkeypatch.setenv("ORCH_STAGING_INFRA_TOLERANCE_ENABLED", "true")
|
||||
assert Settings().staging_infra_tolerance_enabled is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# ORCH-065 / TC-20: reaper_* + lease_reclaim_* settings defaults + env override.
|
||||
# ---------------------------------------------------------------------------
|
||||
_REAPER_ENV = (
|
||||
"ORCH_REAPER_ENABLED",
|
||||
"ORCH_REAPER_INTERVAL_S",
|
||||
"ORCH_REAPER_DEAD_TICKS",
|
||||
"ORCH_REAPER_MAX_RUNNING_S",
|
||||
"ORCH_LEASE_RECLAIM_ENABLED",
|
||||
)
|
||||
|
||||
|
||||
def test_reaper_settings_defaults(monkeypatch):
|
||||
"""TC-20 / §5: documented defaults when no env is set."""
|
||||
for name in _REAPER_ENV:
|
||||
monkeypatch.delenv(name, raising=False)
|
||||
s = Settings()
|
||||
assert s.reaper_enabled is True
|
||||
assert s.reaper_interval_s == 60
|
||||
assert s.reaper_dead_ticks == 2
|
||||
assert s.reaper_max_running_s == 3600
|
||||
assert s.lease_reclaim_enabled is True
|
||||
|
||||
|
||||
def test_reaper_settings_env_override(monkeypatch):
|
||||
"""TC-20 / §5 / AC-14: each field is read from its ORCH_* env var."""
|
||||
monkeypatch.setenv("ORCH_REAPER_ENABLED", "false")
|
||||
monkeypatch.setenv("ORCH_REAPER_INTERVAL_S", "30")
|
||||
monkeypatch.setenv("ORCH_REAPER_DEAD_TICKS", "5")
|
||||
monkeypatch.setenv("ORCH_REAPER_MAX_RUNNING_S", "1200")
|
||||
monkeypatch.setenv("ORCH_LEASE_RECLAIM_ENABLED", "false")
|
||||
s = Settings()
|
||||
assert s.reaper_enabled is False
|
||||
assert s.reaper_interval_s == 30
|
||||
assert s.reaper_dead_ticks == 5
|
||||
assert s.reaper_max_running_s == 1200
|
||||
assert s.lease_reclaim_enabled is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# ORCH-065 / TC-19: contracts unchanged — no new stages / QG checks; the
|
||||
# check_branch_mergeable signature is intact (AC-13).
|
||||
# ---------------------------------------------------------------------------
|
||||
def test_tc19_stage_transitions_unchanged():
|
||||
"""No new pipeline stage was introduced by ORCH-065."""
|
||||
from src.stages import STAGE_TRANSITIONS
|
||||
assert set(STAGE_TRANSITIONS) == {
|
||||
"created", "analysis", "architecture", "development", "review",
|
||||
"testing", "deploy-staging", "deploy", "done",
|
||||
}
|
||||
|
||||
|
||||
def test_tc19_qg_checks_registry_unchanged():
|
||||
"""No new quality-gate check was added to the registry by ORCH-065."""
|
||||
from src.qg.checks import QG_CHECKS
|
||||
assert set(QG_CHECKS) == {
|
||||
"check_analysis_approved",
|
||||
"check_analysis_complete",
|
||||
"check_architecture_done",
|
||||
"check_ci_green",
|
||||
"check_review_approved",
|
||||
"check_tests_passed",
|
||||
"check_reviewer_verdict",
|
||||
"check_tests_local",
|
||||
"check_deploy_status",
|
||||
"check_staging_status",
|
||||
"check_branch_mergeable",
|
||||
"check_staging_image_fresh",
|
||||
}
|
||||
|
||||
|
||||
def test_tc19_check_branch_mergeable_signature_intact():
|
||||
"""check_branch_mergeable still takes exactly (repo, work_item_id, branch)."""
|
||||
import inspect
|
||||
from src.qg.checks import check_branch_mergeable
|
||||
params = list(inspect.signature(check_branch_mergeable).parameters)
|
||||
assert params == ["repo", "work_item_id", "branch"]
|
||||
|
||||
285
tests/test_job_reaper.py
Normal file
285
tests/test_job_reaper.py
Normal file
@@ -0,0 +1,285 @@
|
||||
"""ORCH-065: job-reaper unit tests (TC-01..TC-08, TC-21).
|
||||
|
||||
The reaper never spawns claude; we drive the DB directly (a 'running' jobs row +
|
||||
optional agent_runs exit_code/pid) and assert the terminal flip + side-effects.
|
||||
``os.kill`` liveness is monkeypatched so a 'dead'/'alive' pid is deterministic.
|
||||
"""
|
||||
import os
|
||||
import tempfile
|
||||
|
||||
import pytest
|
||||
|
||||
# Override env before importing app modules (same convention as test_queue.py).
|
||||
os.environ["ORCH_DB_PATH"] = os.path.join(tempfile.gettempdir(), "test_orch_reaper.db")
|
||||
os.environ["ORCH_REPOS_DIR"] = tempfile.gettempdir()
|
||||
os.environ["ORCH_GITEA_TOKEN"] = "test-token"
|
||||
os.environ["ORCH_PLANE_API_TOKEN"] = "test-token"
|
||||
|
||||
import src.db as db
|
||||
from src.db import init_db, get_db, enqueue_job, get_job
|
||||
import src.job_reaper as jr
|
||||
from src.job_reaper import JobReaper
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def fresh_db(tmp_path, monkeypatch):
|
||||
dbfile = tmp_path / "reaper.db"
|
||||
monkeypatch.setattr(db.settings, "db_path", str(dbfile))
|
||||
init_db()
|
||||
yield
|
||||
|
||||
|
||||
# --- helpers ----------------------------------------------------------------
|
||||
def _make_running_job(agent="developer", repo="orchestrator", task_id=None,
|
||||
pid=None, age_s=0, attempts=0, max_attempts=2,
|
||||
run_id=None, exit_code=None):
|
||||
"""Insert a job already in 'running' with the given pid/age/attempts.
|
||||
|
||||
started_at is back-dated by ``age_s`` seconds so running_age_s reflects it.
|
||||
When ``exit_code`` is given an agent_runs row is created and linked (Tier-2).
|
||||
"""
|
||||
conn = get_db()
|
||||
if run_id is None and exit_code is not None:
|
||||
cur = conn.execute(
|
||||
"INSERT INTO agent_runs (task_id, agent, finished_at, exit_code) "
|
||||
"VALUES (?, ?, datetime('now'), ?)",
|
||||
(task_id, agent, exit_code),
|
||||
)
|
||||
run_id = cur.lastrowid
|
||||
cur = conn.execute(
|
||||
"INSERT INTO jobs (agent, repo, task_id, status, attempts, max_attempts, "
|
||||
"run_id, pid, started_at) "
|
||||
"VALUES (?, ?, ?, 'running', ?, ?, ?, ?, datetime('now', ?))",
|
||||
(agent, repo, task_id, attempts, max_attempts, run_id, pid,
|
||||
f"-{int(age_s)} seconds"),
|
||||
)
|
||||
job_id = cur.lastrowid
|
||||
conn.commit()
|
||||
conn.close()
|
||||
return job_id
|
||||
|
||||
|
||||
def _make_task(repo="orchestrator", branch="feature/x", stage="development",
|
||||
work_item_id="ORCH-1"):
|
||||
conn = get_db()
|
||||
cur = conn.execute(
|
||||
"INSERT INTO tasks (plane_id, work_item_id, repo, branch, stage) "
|
||||
"VALUES (?, ?, ?, ?, ?)",
|
||||
(work_item_id, work_item_id, repo, branch, stage),
|
||||
)
|
||||
tid = cur.lastrowid
|
||||
conn.commit()
|
||||
conn.close()
|
||||
return tid
|
||||
|
||||
|
||||
def _dead_pid(monkeypatch):
|
||||
"""Force merge_gate.pid_alive -> False (process gone) for the reaper."""
|
||||
import src.merge_gate as mg
|
||||
monkeypatch.setattr(mg, "pid_alive", lambda pid: False)
|
||||
|
||||
|
||||
def _alive_pid(monkeypatch):
|
||||
import src.merge_gate as mg
|
||||
monkeypatch.setattr(mg, "pid_alive", lambda pid: True)
|
||||
|
||||
|
||||
# --- TC-01: dead executor -> reaped without process restart -----------------
|
||||
def test_tc01_dead_pid_reaped_to_queued(monkeypatch):
|
||||
_dead_pid(monkeypatch)
|
||||
jid = _make_running_job(pid=999999, attempts=0, max_attempts=2)
|
||||
r = JobReaper()
|
||||
r.reap_once() # tick 1 (streak=1, dead_ticks default 2 -> not yet)
|
||||
assert get_job(jid)["status"] == "running"
|
||||
r.reap_once() # tick 2 -> reaped
|
||||
assert get_job(jid)["status"] == "queued"
|
||||
assert r.reaped_total == 1
|
||||
assert r.last_reaped["job_id"] == jid
|
||||
|
||||
|
||||
# --- TC-02: live agent within timeout is NEVER reaped -----------------------
|
||||
def test_tc02_alive_pid_never_reaped(monkeypatch):
|
||||
_alive_pid(monkeypatch)
|
||||
jid = _make_running_job(pid=4321, age_s=10)
|
||||
r = JobReaper()
|
||||
for _ in range(5):
|
||||
r.reap_once()
|
||||
assert get_job(jid)["status"] == "running"
|
||||
assert r.reaped_total == 0
|
||||
|
||||
|
||||
def test_tc02_alive_within_max_running_not_reaped(monkeypatch):
|
||||
_alive_pid(monkeypatch)
|
||||
monkeypatch.setattr(db.settings, "reaper_max_running_s", 3600)
|
||||
jid = _make_running_job(pid=4321, age_s=1800) # < ceiling, alive
|
||||
r = JobReaper()
|
||||
r.reap_once()
|
||||
assert get_job(jid)["status"] == "running"
|
||||
|
||||
|
||||
# --- TC-03: zombie only after reaper_dead_ticks consecutive ticks -----------
|
||||
def test_tc03_requires_consecutive_dead_ticks(monkeypatch):
|
||||
monkeypatch.setattr(db.settings, "reaper_dead_ticks", 3)
|
||||
import src.merge_gate as mg
|
||||
# Dead, dead, ALIVE (resets), dead, dead, dead -> reaped only on the 6th tick.
|
||||
seq = iter([False, False, True, False, False, False])
|
||||
monkeypatch.setattr(mg, "pid_alive", lambda pid: next(seq))
|
||||
jid = _make_running_job(pid=999998)
|
||||
r = JobReaper()
|
||||
for _ in range(5):
|
||||
r.reap_once()
|
||||
assert get_job(jid)["status"] == "running"
|
||||
r.reap_once() # 6th tick: third CONSECUTIVE dead -> reaped
|
||||
assert get_job(jid)["status"] == "queued"
|
||||
|
||||
|
||||
# --- TC-04: backstop ceiling reaps even when liveness is unknown ------------
|
||||
def test_tc04_backstop_ceiling(monkeypatch):
|
||||
_alive_pid(monkeypatch) # liveness says "alive", but age exceeds the ceiling
|
||||
monkeypatch.setattr(db.settings, "reaper_max_running_s", 100)
|
||||
jid = _make_running_job(pid=4321, age_s=500)
|
||||
r = JobReaper()
|
||||
r.reap_once()
|
||||
assert get_job(jid)["status"] == "queued"
|
||||
assert r.reaped_total == 1
|
||||
|
||||
|
||||
def test_tc04_backstop_no_pid(monkeypatch):
|
||||
monkeypatch.setattr(db.settings, "reaper_max_running_s", 100)
|
||||
jid = _make_running_job(pid=None, age_s=500)
|
||||
r = JobReaper()
|
||||
r.reap_once()
|
||||
assert get_job(jid)["status"] == "queued"
|
||||
|
||||
|
||||
# --- TC-05: correct outcome by exit_code (Tier-2) ---------------------------
|
||||
def test_tc05_exit0_gate_green_done(monkeypatch):
|
||||
# A developer job runs to LEAVE the 'architecture' stage (-> 'development').
|
||||
tid = _make_task(stage="architecture")
|
||||
jid = _make_running_job(agent="developer", task_id=tid, exit_code=0)
|
||||
# gate green -> advance succeeds (stage leaves the developer candidate set).
|
||||
import src.agents.launcher as L
|
||||
monkeypatch.setattr(
|
||||
L.launcher, "_try_advance_stage",
|
||||
lambda run_id, agent, repo, branch: db.update_task_stage(tid, "development"),
|
||||
)
|
||||
r = JobReaper()
|
||||
r.reap_once()
|
||||
assert get_job(jid)["status"] == "done"
|
||||
|
||||
|
||||
def test_tc05_exit0_gate_red_requeues(monkeypatch):
|
||||
tid = _make_task(stage="architecture")
|
||||
jid = _make_running_job(agent="developer", task_id=tid, exit_code=0,
|
||||
attempts=0, max_attempts=2)
|
||||
# gate red -> _try_advance_stage is a no-op (stage stays 'architecture').
|
||||
import src.agents.launcher as L
|
||||
monkeypatch.setattr(L.launcher, "_try_advance_stage",
|
||||
lambda run_id, agent, repo, branch: None)
|
||||
r = JobReaper()
|
||||
r.reap_once()
|
||||
assert get_job(jid)["status"] == "queued" # exit0 but gate red -> not 'done'
|
||||
|
||||
|
||||
def test_tc05_nonzero_exit_requeue_then_failed(monkeypatch):
|
||||
sent = []
|
||||
monkeypatch.setattr(jr, "JobReaper", JobReaper)
|
||||
tid = _make_task(stage="development")
|
||||
jid = _make_running_job(agent="developer", task_id=tid, exit_code=1,
|
||||
attempts=1, max_attempts=2)
|
||||
r = JobReaper()
|
||||
import src.notifications as notif
|
||||
monkeypatch.setattr(notif, "send_telegram", lambda *a, **k: sent.append(a))
|
||||
r.reap_once() # attempts(1) < max(2) -> queued
|
||||
assert get_job(jid)["status"] == "queued"
|
||||
|
||||
# Now exhaust the budget.
|
||||
jid2 = _make_running_job(agent="developer", task_id=tid, exit_code=1,
|
||||
attempts=2, max_attempts=2)
|
||||
r.reap_once()
|
||||
assert get_job(jid2)["status"] == "failed"
|
||||
assert sent, "failed reap must send a Telegram alert"
|
||||
|
||||
|
||||
# --- TC-06: atomicity — reaper vs requeue_running_jobs (status guard) --------
|
||||
def test_tc06_atomic_no_double_reap(monkeypatch):
|
||||
_dead_pid(monkeypatch)
|
||||
monkeypatch.setattr(db.settings, "reaper_dead_ticks", 1)
|
||||
jid = _make_running_job(pid=999997, attempts=0, max_attempts=2)
|
||||
# Simulate the startup requeue winning the row first.
|
||||
n = db.requeue_running_jobs()
|
||||
assert n == 1
|
||||
assert get_job(jid)["status"] == "queued"
|
||||
# The reaper now scans: the row is no longer 'running' -> reap_running_job's
|
||||
# WHERE status='running' guard yields rowcount 0 -> no second processing.
|
||||
r = JobReaper()
|
||||
r.reap_once()
|
||||
assert get_job(jid)["status"] == "queued"
|
||||
assert r.reaped_total == 0
|
||||
|
||||
|
||||
def test_tc06_reap_running_job_guard_returns_false_when_not_running():
|
||||
jid = enqueue_job("developer", "orchestrator") # status 'queued', not running
|
||||
assert db.reap_running_job(jid, "done") is False
|
||||
assert get_job(jid)["status"] == "queued"
|
||||
|
||||
|
||||
# --- TC-07: kill-switch reaper_enabled=False -> no-op -----------------------
|
||||
def test_tc07_kill_switch(monkeypatch):
|
||||
_dead_pid(monkeypatch)
|
||||
monkeypatch.setattr(db.settings, "reaper_enabled", False)
|
||||
monkeypatch.setattr(db.settings, "lease_reclaim_enabled", False)
|
||||
jid = _make_running_job(pid=999996, age_s=99999)
|
||||
r = JobReaper()
|
||||
for _ in range(3):
|
||||
r.reap_once()
|
||||
assert get_job(jid)["status"] == "running"
|
||||
assert r.reaped_total == 0
|
||||
|
||||
|
||||
# --- TC-08: never-raise — a DB/OS error in one tick does not propagate -------
|
||||
def test_tc08_never_raise_isolates_per_job(monkeypatch):
|
||||
_dead_pid(monkeypatch)
|
||||
monkeypatch.setattr(db.settings, "reaper_dead_ticks", 1)
|
||||
good = _make_running_job(pid=111, attempts=0, max_attempts=2)
|
||||
bad = _make_running_job(pid=222, attempts=0, max_attempts=2)
|
||||
|
||||
r = JobReaper()
|
||||
orig = r._reap_job
|
||||
|
||||
def boom(job):
|
||||
if job["id"] == bad:
|
||||
raise RuntimeError("simulated per-job failure")
|
||||
return orig(job)
|
||||
|
||||
monkeypatch.setattr(r, "_reap_job", boom)
|
||||
# Must not raise despite the bad job blowing up.
|
||||
r.reap_once()
|
||||
# The good job is still reaped; the bad one is isolated (stays running).
|
||||
assert get_job(good)["status"] == "queued"
|
||||
assert get_job(bad)["status"] == "running"
|
||||
|
||||
|
||||
def test_tc08_reap_once_outer_never_raises(monkeypatch):
|
||||
monkeypatch.setattr(jr, "get_running_jobs",
|
||||
lambda: (_ for _ in ()).throw(RuntimeError("db down")))
|
||||
r = JobReaper()
|
||||
# reap_once swallows... actually get_running_jobs is iterated in the for; the
|
||||
# _tick wrapper guarantees the loop never dies. Assert _tick is safe.
|
||||
r._tick()
|
||||
assert r.last_run_ts is not None
|
||||
|
||||
|
||||
# --- TC-21: startup lease-reclaim + reaper start/stop smoke -----------------
|
||||
def test_tc21_reaper_start_stop_smoke():
|
||||
r = JobReaper(interval_s=0.05)
|
||||
r.start()
|
||||
assert r._thread is not None and r._thread.is_alive()
|
||||
r.stop(timeout=2)
|
||||
assert not r._thread.is_alive()
|
||||
|
||||
|
||||
def test_tc21_reclaim_all_stale_leases_callable(monkeypatch):
|
||||
# No lease files present -> 0 reclaimed, never raises (registration smoke).
|
||||
monkeypatch.setattr(db.settings, "lease_reclaim_enabled", True)
|
||||
assert jr.reclaim_all_stale_leases() == 0
|
||||
@@ -11,6 +11,7 @@ import subprocess
|
||||
import tempfile
|
||||
import time
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
# Env before importing app modules (same convention as the other suites).
|
||||
@@ -299,3 +300,56 @@ def test_tc11_release_missing_is_noop(lease_dir):
|
||||
# Releasing a non-existent lease never raises.
|
||||
merge_gate.release_merge_lease("orchestrator", "feature/none")
|
||||
merge_gate.release_merge_lease("orchestrator") # force form
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# ORCH-065 / TC-16: idempotent merge finalization — pr_already_merged guard.
|
||||
# ---------------------------------------------------------------------------
|
||||
class _FakeResp:
|
||||
def __init__(self, status_code, payload):
|
||||
self.status_code = status_code
|
||||
self._payload = payload
|
||||
|
||||
def json(self):
|
||||
return self._payload
|
||||
|
||||
|
||||
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)."""
|
||||
monkeypatch.setattr(
|
||||
httpx, "get",
|
||||
lambda *a, **k: _FakeResp(200, [{"number": 7, "merged": True}]),
|
||||
)
|
||||
assert merge_gate.pr_already_merged("orchestrator", "feature/x") is True
|
||||
|
||||
|
||||
def test_tc16_pr_open_not_merged_false(monkeypatch):
|
||||
"""An open / not-yet-merged PR -> False (the normal merge path proceeds)."""
|
||||
monkeypatch.setattr(
|
||||
httpx, "get",
|
||||
lambda *a, **k: _FakeResp(200, [{"number": 7, "merged": False}]),
|
||||
)
|
||||
assert merge_gate.pr_already_merged("orchestrator", "feature/x") is False
|
||||
|
||||
|
||||
def test_tc16_pr_no_pr_false(monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
httpx, "get", lambda *a, **k: _FakeResp(200, []),
|
||||
)
|
||||
assert merge_gate.pr_already_merged("orchestrator", "feature/x") is False
|
||||
|
||||
|
||||
def test_tc16_pr_already_merged_never_raises(monkeypatch):
|
||||
"""Any HTTP/parse error -> False (conservative), never an exception (AC-9)."""
|
||||
def boom(*a, **k):
|
||||
raise RuntimeError("gitea down")
|
||||
|
||||
monkeypatch.setattr(httpx, "get", boom)
|
||||
assert merge_gate.pr_already_merged("orchestrator", "feature/x") is False
|
||||
|
||||
|
||||
def test_tc16_pr_non_200_false(monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
httpx, "get", lambda *a, **k: _FakeResp(500, None),
|
||||
)
|
||||
assert merge_gate.pr_already_merged("orchestrator", "feature/x") is False
|
||||
|
||||
@@ -148,3 +148,63 @@ def test_tc24_red_catch_up_fails_and_releases_main_stays_green(race_repo, monkey
|
||||
assert _origin_main_sha(origin) == main_before
|
||||
# The lease was released on failure (a later task can proceed).
|
||||
assert merge_gate._read_lease(merge_gate._lease_path(repo)) is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# ORCH-065 / TC-17: recovery — "rebase+re-test green, merge not done, process
|
||||
# died" -> reaper requeues -> the merge re-drives the STANDARD path WITHOUT a
|
||||
# second expensive re-test when safe (the branch is already up-to-date). AC-10.
|
||||
# ---------------------------------------------------------------------------
|
||||
def test_tc17_redrive_skips_expensive_retest_when_already_caught_up(
|
||||
race_repo, monkeypatch
|
||||
):
|
||||
repo, origin = race_repo
|
||||
main_before = _origin_main_sha(origin)
|
||||
|
||||
# First pass: B catches up (real rebase onto C1) with a GREEN re-test. This is
|
||||
# the work that completed before the process died — the lease is held, the
|
||||
# branch is now caught up on origin.
|
||||
retest_calls = []
|
||||
|
||||
def _retest(r, b):
|
||||
retest_calls.append((r, b))
|
||||
return True, "re-test green"
|
||||
|
||||
monkeypatch.setattr(merge_gate, "retest_branch", _retest)
|
||||
passed, reason = check_branch_mergeable(repo, "ORCH-B", "feature/B")
|
||||
assert passed is True
|
||||
assert reason == "rebased onto main, re-test green"
|
||||
assert len(retest_calls) == 1 # the expensive re-test ran ONCE
|
||||
|
||||
# The process "died" before the merge: release the lease the way the reaper /
|
||||
# reconciler recovery path would (the row is requeued; the branch stays caught
|
||||
# up because the rebase was already pushed).
|
||||
merge_gate.release_merge_lease(repo, "feature/B")
|
||||
|
||||
# Re-drive (standard path) after recovery: the branch already contains
|
||||
# origin/main, so branch_is_behind_main is False and the gate short-circuits to
|
||||
# the up-to-date pass WITHOUT re-running the expensive rebase+re-test.
|
||||
assert merge_gate.branch_is_behind_main(repo, "feature/B") is False
|
||||
passed2, reason2 = check_branch_mergeable(repo, "ORCH-B", "feature/B")
|
||||
assert passed2 is True
|
||||
assert reason2 == "branch up-to-date with main"
|
||||
assert len(retest_calls) == 1 # NOT re-run on the re-drive (no double cost)
|
||||
# origin/main was never pushed by the gate across the whole recovery.
|
||||
assert _origin_main_sha(origin) == main_before
|
||||
|
||||
|
||||
def test_tc17_pr_already_merged_makes_redrive_a_noop(race_repo, monkeypatch):
|
||||
"""If the PR actually merged before the process died, the idempotency guard
|
||||
reports it so the re-drive is a no-op (no second merge)."""
|
||||
import httpx
|
||||
repo, _ = race_repo
|
||||
|
||||
class _R:
|
||||
status_code = 200
|
||||
|
||||
@staticmethod
|
||||
def json():
|
||||
return [{"merged": True}]
|
||||
|
||||
monkeypatch.setattr(httpx, "get", lambda *a, **k: _R())
|
||||
assert merge_gate.pr_already_merged(repo, "feature/B") is True
|
||||
|
||||
138
tests/test_merge_lease_reclaim.py
Normal file
138
tests/test_merge_lease_reclaim.py
Normal file
@@ -0,0 +1,138 @@
|
||||
"""ORCH-065: proactive stale/dead merge-lease reclaim (TC-10..TC-15).
|
||||
|
||||
Exercises merge_gate.reclaim_stale_lease / pid_alive directly with lease files
|
||||
written into a tmp repos_dir. No git ops run (reclaim only removes the lease
|
||||
file). pid liveness is monkeypatched so 'dead'/'alive' are deterministic.
|
||||
"""
|
||||
import json
|
||||
import os
|
||||
import tempfile
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
os.environ["ORCH_DB_PATH"] = os.path.join(tempfile.gettempdir(), "test_orch_lease.db")
|
||||
os.environ["ORCH_REPOS_DIR"] = tempfile.gettempdir()
|
||||
os.environ["ORCH_GITEA_TOKEN"] = "test-token"
|
||||
os.environ["ORCH_PLANE_API_TOKEN"] = "test-token"
|
||||
|
||||
from src import merge_gate
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def repos_dir(tmp_path, monkeypatch):
|
||||
d = tmp_path / "repos"
|
||||
d.mkdir()
|
||||
monkeypatch.setattr(merge_gate.settings, "repos_dir", str(d))
|
||||
monkeypatch.setattr(merge_gate.settings, "lease_reclaim_enabled", True)
|
||||
monkeypatch.setattr(merge_gate.settings, "merge_gate_repos", "") # self-hosting only
|
||||
monkeypatch.setattr(merge_gate.settings, "merge_lock_timeout_s", 300)
|
||||
return d
|
||||
|
||||
|
||||
def _write_lease(repos_dir, repo, branch="feature/x", pid=1234, age_s=0):
|
||||
path = os.path.join(str(repos_dir), f".merge-lease-{repo}.json")
|
||||
holder = {
|
||||
"branch": branch,
|
||||
"work_item_id": "ORCH-1",
|
||||
"task_id": 1,
|
||||
"acquired_at": time.time() - age_s,
|
||||
"pid": pid,
|
||||
}
|
||||
with open(path, "w", encoding="utf-8") as f:
|
||||
f.write(json.dumps(holder))
|
||||
return path
|
||||
|
||||
|
||||
def _no_telegram(monkeypatch):
|
||||
import src.notifications as notif
|
||||
monkeypatch.setattr(notif, "send_telegram", lambda *a, **k: None)
|
||||
|
||||
|
||||
# --- TC-10: reclaim a lease with a DEAD pid, proactively --------------------
|
||||
def test_tc10_reclaim_dead_pid(repos_dir, monkeypatch):
|
||||
_no_telegram(monkeypatch)
|
||||
path = _write_lease(repos_dir, "orchestrator", pid=999999, age_s=0)
|
||||
monkeypatch.setattr(merge_gate, "pid_alive", lambda pid: False)
|
||||
assert merge_gate.reclaim_stale_lease("orchestrator") is True
|
||||
assert not os.path.exists(path) # lease removed
|
||||
|
||||
|
||||
# --- TC-11: reclaim by TTL is preserved -------------------------------------
|
||||
def test_tc11_reclaim_by_ttl(repos_dir, monkeypatch):
|
||||
_no_telegram(monkeypatch)
|
||||
# pid alive, but the lease is older than the TTL -> still reclaimed.
|
||||
path = _write_lease(repos_dir, "orchestrator", pid=4321, age_s=999)
|
||||
monkeypatch.setattr(merge_gate, "pid_alive", lambda pid: True)
|
||||
assert merge_gate.reclaim_stale_lease("orchestrator") is True
|
||||
assert not os.path.exists(path)
|
||||
|
||||
|
||||
# --- TC-12: a LIVE lease within TTL is NOT released -------------------------
|
||||
def test_tc12_live_lease_protected(repos_dir, monkeypatch):
|
||||
_no_telegram(monkeypatch)
|
||||
path = _write_lease(repos_dir, "orchestrator", pid=4321, age_s=10)
|
||||
monkeypatch.setattr(merge_gate, "pid_alive", lambda pid: True)
|
||||
assert merge_gate.reclaim_stale_lease("orchestrator") is False
|
||||
assert os.path.exists(path) # untouched
|
||||
|
||||
|
||||
# --- TC-13: conditional — non-self-hosting repos are a no-op ----------------
|
||||
def test_tc13_non_scope_repo_noop(repos_dir, monkeypatch):
|
||||
_no_telegram(monkeypatch)
|
||||
path = _write_lease(repos_dir, "enduro-trails", pid=999999, age_s=999)
|
||||
monkeypatch.setattr(merge_gate, "pid_alive", lambda pid: False)
|
||||
assert merge_gate.reclaim_stale_lease("enduro-trails") is False
|
||||
assert os.path.exists(path) # out of scope -> untouched
|
||||
|
||||
|
||||
def test_tc13_merge_gate_repos_csv_scope(repos_dir, monkeypatch):
|
||||
_no_telegram(monkeypatch)
|
||||
monkeypatch.setattr(merge_gate.settings, "merge_gate_repos", "enduro-trails")
|
||||
path = _write_lease(repos_dir, "enduro-trails", pid=999999, age_s=0)
|
||||
monkeypatch.setattr(merge_gate, "pid_alive", lambda pid: False)
|
||||
assert merge_gate.reclaim_stale_lease("enduro-trails") is True
|
||||
assert not os.path.exists(path)
|
||||
|
||||
|
||||
# --- TC-14: never-raise on a read/remove error ------------------------------
|
||||
def test_tc14_never_raise_on_read_error(repos_dir, monkeypatch):
|
||||
_no_telegram(monkeypatch)
|
||||
_write_lease(repos_dir, "orchestrator", pid=1, age_s=999)
|
||||
|
||||
def boom(path):
|
||||
raise OSError("simulated read failure")
|
||||
|
||||
monkeypatch.setattr(merge_gate, "_read_lease", boom)
|
||||
# Must not raise; returns False (could not reclaim).
|
||||
assert merge_gate.reclaim_stale_lease("orchestrator") is False
|
||||
|
||||
|
||||
def test_tc14_no_lease_file_is_noop(repos_dir, monkeypatch):
|
||||
_no_telegram(monkeypatch)
|
||||
assert merge_gate.reclaim_stale_lease("orchestrator") is False
|
||||
|
||||
|
||||
# --- TC-15: kill-switch lease_reclaim_enabled=False -------------------------
|
||||
def test_tc15_kill_switch(repos_dir, monkeypatch):
|
||||
_no_telegram(monkeypatch)
|
||||
monkeypatch.setattr(merge_gate.settings, "lease_reclaim_enabled", False)
|
||||
path = _write_lease(repos_dir, "orchestrator", pid=999999, age_s=999)
|
||||
monkeypatch.setattr(merge_gate, "pid_alive", lambda pid: False)
|
||||
assert merge_gate.reclaim_stale_lease("orchestrator") is False
|
||||
assert os.path.exists(path) # proactive reclaim off -> untouched
|
||||
|
||||
|
||||
# --- pid_alive semantics ----------------------------------------------------
|
||||
def test_pid_alive_dead_process():
|
||||
# PID 999999999 almost certainly does not exist.
|
||||
assert merge_gate.pid_alive(999999999) is False
|
||||
|
||||
|
||||
def test_pid_alive_self():
|
||||
assert merge_gate.pid_alive(os.getpid()) is True
|
||||
|
||||
|
||||
def test_pid_alive_missing_pid_conservative():
|
||||
assert merge_gate.pid_alive(None) is True
|
||||
assert merge_gate.pid_alive(0) is True
|
||||
@@ -302,3 +302,58 @@ class TestWorkerConcurrency:
|
||||
assert count_running_jobs() == 0
|
||||
counts = job_status_counts()
|
||||
assert counts["failed"] == 1
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# ORCH-065: job-reaper unblocks the shared queue (TC-09) + /queue block (TC-18)
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestReaperUnblocksQueue:
|
||||
def test_tc09_reap_unblocks_claim_at_concurrency_1(self, monkeypatch):
|
||||
"""A zombie 'running' row at max_concurrency=1 blocks every claim; once the
|
||||
reaper reaps it the next queued job can be claimed (AC-2)."""
|
||||
import src.merge_gate as mg
|
||||
from src.job_reaper import JobReaper
|
||||
|
||||
monkeypatch.setattr(db.settings, "reaper_dead_ticks", 1)
|
||||
monkeypatch.setattr(mg, "pid_alive", lambda pid: False) # zombie pid dead
|
||||
|
||||
# A zombie row stuck 'running' with a dead pid.
|
||||
conn = db.get_db()
|
||||
cur = conn.execute(
|
||||
"INSERT INTO jobs (agent, repo, status, attempts, max_attempts, pid, "
|
||||
"started_at) VALUES ('developer','r','running',2,2,999999,datetime('now'))"
|
||||
)
|
||||
zombie = cur.lastrowid
|
||||
conn.commit()
|
||||
conn.close()
|
||||
|
||||
# A second job waits in the queue behind it.
|
||||
nxt = enqueue_job("analyst", "r")
|
||||
|
||||
# At concurrency 1 the slot is fully occupied -> nothing else can run.
|
||||
assert count_running_jobs() == 1
|
||||
|
||||
monkeypatch.setattr("src.notifications.send_telegram", lambda *a, **k: None)
|
||||
JobReaper().reap_once() # dead pid, attempts>=max -> failed
|
||||
|
||||
assert get_job(zombie)["status"] == "failed"
|
||||
assert count_running_jobs() == 0
|
||||
# Queue is unblocked: the next job claims successfully.
|
||||
claimed = claim_next_job()
|
||||
assert claimed is not None and claimed["id"] == nxt
|
||||
|
||||
def test_tc18_queue_endpoint_has_reaper_block(self):
|
||||
"""GET /queue exposes the reaper observability block (AC-15).
|
||||
|
||||
Calls the endpoint coroutine directly (no lifespan / no background
|
||||
threads / no network) so the test stays hermetic.
|
||||
"""
|
||||
import asyncio
|
||||
import src.main as main
|
||||
|
||||
body = asyncio.run(main.queue())
|
||||
assert "reaper" in body
|
||||
reaper = body["reaper"]
|
||||
for key in ("enabled", "interval", "last_run_ts", "reaped_total",
|
||||
"last_reaped", "lease_reclaimed_total"):
|
||||
assert key in reaper
|
||||
|
||||
Reference in New Issue
Block a user