Add --force flag to re-dispatch reviewed PRs

- Add review --force CLI flag; requires --pr, rejects --loop
- Thread force through run() into _evaluate_pr, bypassing decide
- Forced dispatch tags reason "forced", keeps fingerprint sha/hash
- Cover both fingerprint paths (--max capped and uncapped)
- Add CLI usage-error and force-binding tests

Force short-circuits the decide() land-guard so an already-decided
PR (terminal LAND, unchanged hash) is re-dispatched on demand. It
short-circuits only decide, not the fingerprint try/except: a
fingerprint failure is still caught, recorded as failed, and raised
as the aggregate scan error, so force never dispatches a failed PR
or bypasses failure handling.

Signed-off-by: Jean Schmidt <contato@jschmidt.me>

Author: jeanschmidt

greenlight/src/greenlight/cli.py

@@ -61,6 +61,11 @@ def build_parser() -> argparse.ArgumentParser:
         default=DEFAULT_TIMEOUT_MINUTES,
         help="minutes before an in-flight or failed review is re-dispatched",
     )
+    review_parser.add_argument(
+        "--force",
+        action="store_true",
+        help="re-dispatch even if already reviewed; requires --pr, not allowed with --loop",
+    )
 
     verdict_parser = subparsers.add_parser(
         "verdict",
@@ -167,11 +172,16 @@ def main(argv: Sequence[str] | None = None) -> int:
     lock_path = config.lock_path
     if lock_path is not None:
         logger.info("using single-instance lock path %s", lock_path)
+    if args.force and args.pr is None:
+        parser.error("--force requires --pr")
+    if args.force and args.loop:
+        parser.error("--force cannot be combined with --loop")
     run = functools.partial(
         review.run,
         pr=args.pr,
         max_dispatches=args.max,
         ref=args.ref,
         timeout_minutes=args.timeout_minutes,
+        force=args.force,
     )
     return _dispatch(config, run, loop=args.loop, lock_path=lock_path)

greenlight/src/greenlight/review.py

@@ -144,10 +144,14 @@ def _evaluate_pr(
     now: datetime,
     timeout: timedelta,
     failed: list[int],
+    force: bool,
 ) -> _Candidate | None:
     try:
         head_sha, eval_hash = future.result()
         recorded = states.get(number)
+        if force:
+            logger.info("PR #%d: DISPATCH (forced)", number)
+            return _Candidate(number, head_sha, eval_hash, recorded, "forced")
         outcome = decide(
             current_eval_hash=eval_hash,
             latest_status=recorded.status if recorded is not None else None,
@@ -176,6 +180,7 @@ def _fingerprint_all(
     now: datetime,
     timeout: timedelta,
     failed: list[int],
+    force: bool,
 ) -> list[_Candidate]:
     futures: dict[int, Future[tuple[str, str]]] = {}
     if worker_count:
@@ -185,7 +190,7 @@ def _fingerprint_all(
             }
     pending: list[_Candidate] = []
     for number in pr_numbers:
-        candidate = _evaluate_pr(number, futures[number], states, now=now, timeout=timeout, failed=failed)
+        candidate = _evaluate_pr(number, futures[number], states, now=now, timeout=timeout, failed=failed, force=force)
         if candidate is not None:
             pending.append(candidate)
     return pending
@@ -202,6 +207,7 @@ def _fingerprint_until_dispatchable(
     now: datetime,
     timeout: timedelta,
     failed: list[int],
+    force: bool,
 ) -> list[_Candidate]:
     ranked = sorted(pr_numbers, key=lambda number: _staleness_key_for_state(states.get(number)))
     pending: list[_Candidate] = []
@@ -213,7 +219,9 @@ def _fingerprint_until_dispatchable(
                 batch = ranked[start : start + worker_count]
                 futures = {number: pool.submit(_fingerprint_task, fingerprint, client_pool, number) for number in batch}
                 for number in batch:
-                    candidate = _evaluate_pr(number, futures[number], states, now=now, timeout=timeout, failed=failed)
+                    candidate = _evaluate_pr(
+                        number, futures[number], states, now=now, timeout=timeout, failed=failed, force=force
+                    )
                     if candidate is not None:
                         pending.append(candidate)
     return pending
@@ -226,6 +234,7 @@ def run(
     max_dispatches: int | None = None,
     ref: str = DEFAULT_DISPATCH_REF,
     timeout_minutes: int = DEFAULT_TIMEOUT_MINUTES,
+    force: bool = False,
     build_github: Callable[[str], Github] = github_client.build_client,
     fetch: Callable[[Github], list[OpenPR]] = _default_fetch,
     fingerprint: Callable[[Github, int], tuple[str, str]] = _default_fingerprint,
@@ -265,6 +274,7 @@ def run(
                 now=evaluated_at,
                 timeout=timeout,
                 failed=failed,
+                force=force,
             )
         else:
             pending = _fingerprint_until_dispatchable(
@@ -277,6 +287,7 @@ def run(
                 now=evaluated_at,
                 timeout=timeout,
                 failed=failed,
+                force=force,
             )
         _dispatch_pending(client, pending, ref=ref, max_dispatches=max_dispatches, dispatch=dispatch)
         if failed:

greenlight/tests/test_cli.py

@@ -82,7 +82,13 @@ def fake_run_forever(config, *, run):
     bound = captured["run"]
     assert isinstance(bound, functools.partial)
     assert bound.func is review.run
-    assert bound.keywords == {"pr": None, "max_dispatches": None, "ref": "main", "timeout_minutes": 30}
+    assert bound.keywords == {
+        "pr": None,
+        "max_dispatches": None,
+        "ref": "main",
+        "timeout_minutes": 30,
+        "force": False,
+    }
 
 
 def test_main_oneshot_failure_returns_exit_failure(monkeypatch):
@@ -454,11 +460,14 @@ def fake_run(request, config):
 
 def test_review_parser_parses_scan_flags():
     parser = cli.build_parser()
-    args = parser.parse_args(["review", "--pr", "5", "--max", "3", "--ref", "release/2.9", "--timeout-minutes", "45"])
+    args = parser.parse_args(
+        ["review", "--pr", "5", "--max", "3", "--ref", "release/2.9", "--timeout-minutes", "45", "--force"]
+    )
     assert args.pr == 5
     assert args.max == 3
     assert args.ref == "release/2.9"
     assert args.timeout_minutes == 45
+    assert args.force is True
 
 
 def test_review_parser_scan_flag_defaults():
@@ -468,6 +477,7 @@ def test_review_parser_scan_flag_defaults():
     assert args.max is None
     assert args.ref == DEFAULT_DISPATCH_REF
     assert args.timeout_minutes == DEFAULT_TIMEOUT_MINUTES
+    assert args.force is False
 
 
 def test_main_review_binds_scan_flags_into_run(monkeypatch):
@@ -486,6 +496,7 @@ def test_main_review_binds_scan_flags_into_run(monkeypatch):
         "max_dispatches": 2,
         "ref": "release/2.9",
         "timeout_minutes": 45,
+        "force": False,
     }
 
 
@@ -503,4 +514,43 @@ def test_main_review_defaults_bind_into_run(monkeypatch):
         "max_dispatches": None,
         "ref": DEFAULT_DISPATCH_REF,
         "timeout_minutes": DEFAULT_TIMEOUT_MINUTES,
+        "force": False,
+    }
+
+
+def test_main_force_without_pr_is_usage_error(monkeypatch):
+    monkeypatch.setattr(cli, "configure_logging", Mock())
+    run_mock = Mock()
+    monkeypatch.setattr(review, "run", run_mock)
+    with pytest.raises(SystemExit) as excinfo:
+        cli.main(["review", "--force"])
+    assert excinfo.value.code == 2
+    run_mock.assert_not_called()
+
+
+def test_main_force_with_loop_is_usage_error(monkeypatch):
+    monkeypatch.setattr(cli, "configure_logging", Mock())
+    run_forever_mock = Mock()
+    monkeypatch.setattr(cli, "run_forever", run_forever_mock)
+    with pytest.raises(SystemExit) as excinfo:
+        cli.main(["review", "--pr", "5", "--force", "--loop"])
+    assert excinfo.value.code == 2
+    run_forever_mock.assert_not_called()
+
+
+def test_main_review_force_binds_into_run(monkeypatch):
+    review_mock = Mock()
+    monkeypatch.setattr(review, "run", review_mock)
+    monkeypatch.setattr(cli, "single_instance_lock", _noop_lock)
+    monkeypatch.setattr(cli, "configure_logging", Mock())
+
+    rc = cli.main(["review", "--pr", "5", "--force"])
+
+    assert rc == EXIT_OK
+    assert review_mock.call_args.kwargs == {
+        "pr": 5,
+        "max_dispatches": None,
+        "ref": DEFAULT_DISPATCH_REF,
+        "timeout_minutes": DEFAULT_TIMEOUT_MINUTES,
+        "force": True,
     }

greenlight/tests/test_review.py

@@ -72,6 +72,7 @@ def _run_scan(
     max_dispatches: int | None = None,
     ref: str = DEFAULT_DISPATCH_REF,
     timeout_minutes: int = DEFAULT_TIMEOUT_MINUTES,
+    force: bool = False,
     now: datetime = _NOW,
 ) -> _Scan:
     states = states or {}
@@ -102,6 +103,7 @@ def fake_dispatch(_client, number, head_sha, eval_hash, dispatch_ref):
         max_dispatches=max_dispatches,
         ref=ref,
         timeout_minutes=timeout_minutes,
+        force=force,
         build_github=lambda _token: _CLIENT,
         fetch=fake_fetch,
         fingerprint=fake_fingerprint,
@@ -330,6 +332,74 @@ def test_pr_decided_still_skips(make_config):
     assert scan.listed_calls == 0
 
 
+def test_force_dispatches_decided_pr_via_fingerprint_all(make_config, caplog):
+    with caplog.at_level(logging.INFO, logger="greenlight"):
+        scan = _run_scan(
+            make_config,
+            pr=5,
+            fingerprints={5: ("headsha5", _HASH_A)},
+            states={5: _state(5, STATUS_LAND, _HASH_A, _NEW)},
+            force=True,
+        )
+
+    # The decided PR from test_pr_decided_still_skips (terminal LAND, unchanged hash) normally SKIPs;
+    # force bypasses decide on the max_dispatches=None path (_fingerprint_all) and dispatches it,
+    # still using the fingerprint's head_sha/eval_hash and tagging the reason "forced".
+    assert scan.dispatched == [(5, "headsha5", _HASH_A, DEFAULT_DISPATCH_REF)]
+    assert "PR #5: DISPATCH (forced)" in caplog.text
+    assert "dispatched review for PR #5 (forced)" in caplog.text
+
+
+def test_force_dispatches_decided_pr_via_until_dispatchable(make_config, caplog):
+    with caplog.at_level(logging.INFO, logger="greenlight"):
+        scan = _run_scan(
+            make_config,
+            pr=5,
+            fingerprints={5: ("headsha5", _HASH_A)},
+            states={5: _state(5, STATUS_LAND, _HASH_A, _NEW)},
+            max_dispatches=1,
+            force=True,
+        )
+
+    # The capped path (_fingerprint_until_dispatchable, selected by a non-None --max) must honor
+    # force identically: the same decided PR is dispatched with reason "forced".
+    assert scan.dispatched == [(5, "headsha5", _HASH_A, DEFAULT_DISPATCH_REF)]
+    assert "PR #5: DISPATCH (forced)" in caplog.text
+
+
+def test_force_fingerprint_failure_still_raises(make_config, caplog):
+    dispatched: list[int] = []
+
+    def boom_fingerprint(_client, _number):
+        raise RuntimeError("fingerprint boom")
+
+    def fake_dispatch(_client, number, head_sha, eval_hash, dispatch_ref):
+        dispatched.append(number)
+
+    with (
+        caplog.at_level(logging.ERROR, logger="greenlight"),
+        pytest.raises(RuntimeError, match=r"1 PR\(s\) failed during scan: \[7\]"),
+    ):
+        review.run(
+            make_config(github_token="t"),
+            pr=7,
+            force=True,
+            build_github=lambda _token: _CLIENT,
+            fetch=lambda _client: [],
+            fingerprint=boom_fingerprint,
+            read_state=lambda _repo, _numbers: {},
+            dispatch=fake_dispatch,
+            now=lambda: _NOW,
+        )
+
+    # force short-circuits decide, not the try/except: the fingerprint failure is raised before the
+    # force branch is reached, so PR7 is still caught, recorded as failed, and surfaces as the
+    # aggregate RuntimeError -- force never bypasses failure handling nor dispatches a failed PR.
+    assert dispatched == []
+    assert "skipping PR #7" in caplog.text
+    assert any(record.exc_info is not None for record in caplog.records)
+
+
 def test_ref_forwarded_to_dispatch(make_config):
     scan = _run_scan(
         make_config,