diff --git a/nisse-review.example.yaml b/nisse-review.example.yaml index f51eb93..bb6466d 100644 --- a/nisse-review.example.yaml +++ b/nisse-review.example.yaml @@ -35,6 +35,7 @@ gitea: # notes: ro # tuning: # docs/Tuning-Guide.md -# review: {poll_s: 60, max_turns: 40, diff_chars: 60000, attempts: 2} +# review: {poll_s: 60, max_turns: 40, diff_chars: 60000, attempts: 2, +# ci_wait_s: 3600, ci_grace_s: 120} # expert: {consult_budget: 3} # context: {keep_recent_turns: 6, safety_margin: 0.1} diff --git a/nisse-review/src/nisse_review/scheduler.py b/nisse-review/src/nisse_review/scheduler.py index 61c1861..dd865f7 100644 --- a/nisse-review/src/nisse_review/scheduler.py +++ b/nisse-review/src/nisse_review/scheduler.py @@ -212,6 +212,12 @@ class Scheduler: return False return not (state == "none" and waited < self._ci_grace_s) + def _seen(self, known: PR, head: str) -> float: + """When ``head`` was first seen: kept for the head the item knew, now + for one pushed during the review, so a retry still waits for CI. A + retry at a head already past ``ci_wait_s`` doesn't wait again.""" + return known.seen_at if head == known.head else self._clock() + # -- a review --------------------------------------------------------------- def review(self, item: WorkItem) -> None: @@ -254,7 +260,16 @@ class Scheduler: self._store.update_work_item( item.id, position=REVIEWED, - data=data(PR(known.repo, known.number, pr.head_sha, pr.head_sha, 0)), + data=data( + PR( + known.repo, + known.number, + pr.head_sha, + pr.head_sha, + 0, + self._seen(known, pr.head_sha), + ) + ), cause="review posted", ) @@ -263,7 +278,14 @@ class Scheduler: ) -> None: attempts = known.attempts + 1 head = pr.head_sha if pr else known.head - new = PR(known.repo, known.number, head, known.reviewed, attempts) + new = PR( + known.repo, + known.number, + head, + known.reviewed, + attempts, + self._seen(known, head), + ) self._oplog.emit("review.failed", cause=why[:300], work_item=item.id) if attempts < self._attempts: self._store.update_work_item( @@ -313,6 +335,7 @@ class Scheduler: head, head if posted else known.reviewed, known.attempts, + self._seen(known, head), ) self._store.update_work_item( item.id, diff --git a/nisse-review/tests/test_review.py b/nisse-review/tests/test_review.py index ed6f91a..5b85c95 100644 --- a/nisse-review/tests/test_review.py +++ b/nisse-review/tests/test_review.py @@ -394,6 +394,47 @@ def test_a_pr_waiting_on_ci_holds_up_no_other(tmp_path: Path) -> None: assert (item(w, 7).position, item(w, 8).position) == (READY, REVIEWED) +def test_a_failed_review_keeps_its_heads_wait(tmp_path: Path) -> None: + # nisse-review on #155: a retry lost seen_at and skipped the wait. + clock = Clock() + gitea = FakeGitea(repos={REPO: [pr()]}) + prose = [AssistantText(text="I think it's fine.", model="m")] + w = world(tmp_path, Scripted(prose, prose), gitea, clock=clock, attempts=2) + w["scheduler"].round() + assert (item(w).position, pr_of(item(w)).seen_at) == (READY, 1000.0) + + +def test_a_head_pushed_during_a_failed_review_waits_for_its_ci( + tmp_path: Path, +) -> None: + clock = Clock() + + class Pushed(FakeGitea): + def post_review(self, *args: Any, **kwargs: Any) -> int: + clock.now += 50 + self.repos[REPO] = [pr(head_sha="fff000")] + self.ci["fff000"] = "pending" + raise GiteaError("422: the head moved") + + gitea = Pushed(repos={REPO: [pr()]}) + w = world(tmp_path, Scripted(*REVIEW), gitea, clock=clock) + w["scheduler"].round() + assert w["scheduler"].round() is False # the new head's CI is pending + + +def test_checks_gitea_cannot_report_wait_a_round(tmp_path: Path) -> None: + class NoChecks(FakeGitea): + def checks(self, repo: str, sha: str) -> str: + raise GiteaError("502: bad gateway") + + out = io.StringIO() + w = world(tmp_path, Scripted(*REVIEW), NoChecks(repos={REPO: [pr()]})) + w["scheduler"]._oplog = OpLog("t", out) + assert w["scheduler"].round() is False + assert item(w).position == READY + assert '"review.checks_unknown"' in out.getvalue() + + def test_a_configured_repository_it_cannot_read_is_left_out() -> None: gitea = FakeGitea(repos={REPO: []}) out = io.StringIO()