From 864a8689f8a44f335444c53ac7e071ff2e9aaa8f Mon Sep 17 00:00:00 2001 From: Simone Date: Sat, 25 Jul 2026 19:52:49 +0200 Subject: [PATCH] fix(web): sort Executions by run id/attempt, not started timestamp Sorting by started_at let a manually re-run old workflow jump above a genuinely newer run: the re-run's started_at bumps to now, but its run number never changes, so the Run column stopped reading strictly descending. run_id is assigned once at creation and never changes across attempts, so (run_id, attempt) descending is both correct and simpler -- it also handles a queued run (no started_at yet) for free, no datetime.max special case needed. --- docs/WEB-UI.md | 9 ++++++-- src/ci_orchestrator/operations/executions.py | 15 +++++++------ tests/python/test_operations_executions.py | 22 ++++++++++++++++++++ 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/docs/WEB-UI.md b/docs/WEB-UI.md index 570f730..70792cc 100644 --- a/docs/WEB-UI.md +++ b/docs/WEB-UI.md @@ -368,8 +368,13 @@ run. Every pill in the column — outcome pills and the VM-only `orphan`/ sized to fit `cancelled`, the longest word) with left-aligned content, so the column reads as a clean list instead of ragged pill widths. -Rows are ordered **newest first** (a queued run, which has no start time yet, -sorts above everything; ties break on Gitea's run id). The row-count dropdown at +Rows are ordered **newest first by run id, then attempt** — not by +`started` timestamp. A queued run (no `started` yet) sorts above everything +for free, since it already holds the highest id of anything currently +listed. Sorting by time instead would let a manually re-run old workflow (its +`started_at` bumped to now, its id unchanged) jump above a genuinely newer +run; sorting by id/attempt keeps the Run column reading strictly descending. +The row-count dropdown at the right end of the card's header bar picks how many to list — 10 / 25 / 50 / 100, default from `[web].executions_limit` (10). The choice rides on the card's own polling URL (`/executions?limit=N`), so it survives every 5s refresh with no client-side diff --git a/src/ci_orchestrator/operations/executions.py b/src/ci_orchestrator/operations/executions.py index 0b3b5b6..fd6ce17 100644 --- a/src/ci_orchestrator/operations/executions.py +++ b/src/ci_orchestrator/operations/executions.py @@ -350,15 +350,18 @@ def _cached_runs( return runs, source, error, auth_failed, refused -def _recency_key(run: Execution) -> tuple[datetime, int]: +def _recency_key(run: Execution) -> tuple[int, int]: """Sort key for "newest first" (used with ``sorted(..., reverse=True)``). - A run that has not started yet (queued) is the newest thing there is, so it - sorts above every started run. ``run_id`` breaks ties — Gitea hands ids out - in creation order — which keeps the order stable when several runs share a - timestamp, as matrix jobs routinely do. + Ordered by run id, then attempt -- not by ``started`` timestamp. Gitea + hands ``run_id`` out in creation order and it never changes across + re-runs, unlike ``started``: manually re-running an old workflow bumps + its ``started_at`` to now, which would sort that stale run above a + genuinely newer one under a time-based key. This also handles a queued + run (no ``started`` yet) for free -- it already has the highest id of + anything currently listed, no ``datetime.max`` special case needed. """ - return (run.started or datetime.max.replace(tzinfo=UTC), run.run_id) + return (run.run_id, run.attempt) def _attach_vms( diff --git a/tests/python/test_operations_executions.py b/tests/python/test_operations_executions.py index 0aec7c6..aded64e 100644 --- a/tests/python/test_operations_executions.py +++ b/tests/python/test_operations_executions.py @@ -432,6 +432,28 @@ def test_queued_run_sorts_above_started_runs( assert [r.status for r in view.runs] == ["queued", "success"] +def test_a_rerun_does_not_jump_above_a_newer_run( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Re-running an old workflow bumps its ``started_at`` to now, but its + run id/number never change -- sorting by wall-clock time would put that + stale run above a genuinely newer one it should still trail. The Run + column must read strictly by run, descending, not by whichever attempt + was most recently kicked off.""" + payload = [ + _run(run_id=12, number=12, attempt=1, started="2026-07-25T10:00:00Z"), + _run(run_id=13, number=13, attempt=1, started="2026-07-25T10:05:00Z"), + # #12 gets manually re-run after #13 already started -- its + # started_at is now the latest timestamp of the three. + _run(run_id=12, number=12, attempt=2, started="2026-07-25T10:10:00Z"), + ] + _install_urlopen(monkeypatch, _admin_ok(payload)) + + view = list_executions(_make_config(tmp_path), [], token="tok", now=_NOW) + + assert [(r.number, r.attempt) for r in view.runs] == [(13, 1), (12, 2), (12, 1)] + + def test_same_timestamp_falls_back_to_run_id( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: