From a3dbf6ad45e6569ad694f771102010759e1715e5 Mon Sep 17 00:00:00 2001 From: Simone Date: Sat, 25 Jul 2026 19:56:57 +0200 Subject: [PATCH] fix(web): sort Executions by run number, not the instance-wide run id The previous fix (sort by run_id/attempt) traded one bug for another: run_id is Gitea's instance-wide id, shared across every repo/workflow the admin endpoint pulls from, not a per-workflow counter. A rarely-triggered workflow's #53 could land a run_id between two ids from a busier workflow's #141/#142, putting #53's row between them in the Run column even though 53 isn't close to either -- exactly the reported symptom, now reproduced as a regression test before the fix and passing after it. run.number is the value the Run column actually displays; sort by that, then attempt, then run_id only to keep a genuine same-number tie (two different workflows landing on the same number independently) stable. --- docs/WEB-UI.md | 18 ++++++--- src/ci_orchestrator/operations/executions.py | 27 ++++++++----- tests/python/test_operations_executions.py | 40 ++++++++++++++++---- 3 files changed, 63 insertions(+), 22 deletions(-) diff --git a/docs/WEB-UI.md b/docs/WEB-UI.md index 70792cc..29d04ad 100644 --- a/docs/WEB-UI.md +++ b/docs/WEB-UI.md @@ -368,12 +368,18 @@ 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 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. +Rows are ordered **newest first by run number, then attempt** — the `#N` the +Run column actually shows, not `started` timestamp and not `run_id` either. +Both were tried and both break once more than one repo/workflow is in the +mix (the admin endpoint pulls from every repo on the instance): sorting by +time lets a manually re-run old workflow (its `started_at` bumped to now, +its number unchanged) jump above a genuinely newer run; sorting by `run_id` +looks right for a single workflow but `run_id` is an *instance-wide* +sequence shared by every repo/workflow, not a per-workflow counter, so a +rarely-triggered workflow's #53 can land a `run_id` between two ids from a +busier workflow's #141/#142 — putting #53's row between them even though 53 +is nowhere near 141. `run.number` is the only one of the three that means +what the column header says. 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 diff --git a/src/ci_orchestrator/operations/executions.py b/src/ci_orchestrator/operations/executions.py index fd6ce17..3fdf7b9 100644 --- a/src/ci_orchestrator/operations/executions.py +++ b/src/ci_orchestrator/operations/executions.py @@ -350,18 +350,27 @@ def _cached_runs( return runs, source, error, auth_failed, refused -def _recency_key(run: Execution) -> tuple[int, int]: +def _recency_key(run: Execution) -> tuple[int, int, int]: """Sort key for "newest first" (used with ``sorted(..., reverse=True)``). - 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. + Ordered by ``number`` -- the ``#N`` the Run column actually shows -- then + ``attempt``, then ``run_id`` only to keep ties stable. ``run_id`` is + Gitea's *instance-wide* id (shared across every repo/workflow the admin + endpoint pulls from), not a per-workflow counter, so it does not track + ``number`` at all once more than one workflow is in the mix: a run #53 + from a rarely-triggered workflow can easily land a run id between two + #141/#142 runs from a busier one, putting #53's row between them even + though the column reads 142, 141, ..., 53. ``number`` is what "newest + first" has to mean here. + + Two things this key deliberately does NOT sort by, both tried before: + ``started`` timestamp lets a manually re-run old workflow's ``started_at`` + (bumped to now) jump it above a genuinely newer run even though its + number/id never changed; ``run_id`` alone is the instance-wide id + described above. ``attempt`` still breaks a same-number re-run correctly + (attempt 2 outranks attempt 1) without either flaw. """ - return (run.run_id, run.attempt) + return (run.number, run.attempt, run.run_id) def _attach_vms( diff --git a/tests/python/test_operations_executions.py b/tests/python/test_operations_executions.py index aded64e..1a460d6 100644 --- a/tests/python/test_operations_executions.py +++ b/tests/python/test_operations_executions.py @@ -454,21 +454,47 @@ def test_a_rerun_does_not_jump_above_a_newer_run( 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( +def test_a_lower_numbered_run_from_a_busier_workflow_does_not_land_in_between( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """Matrix runs share a timestamp; ids keep the order stable, not arbitrary.""" - stamp = "2026-07-25T11:00:00Z" + """``run_id`` is instance-wide (every repo/workflow the admin endpoint + pulls from shares one sequence), not a per-workflow counter -- a rarely + triggered workflow's #53 can easily get a run id that falls between two + ids from a busier workflow's #141/#142, even though 53 is nowhere near + 141 or 142. The Run column has to read by the number it shows, not by + that shared id.""" payload = [ - _run(run_id=10, number=10, started=stamp), - _run(run_id=12, number=12, started=stamp), - _run(run_id=11, number=11, started=stamp), + _run(run_id=9001, number=142, started="2026-07-25T12:00:00Z"), + # A different, rarely-run workflow's id landed between the two above + # purely because of when it happened to fire -- not because #53 is + # newer than #141. + _run(run_id=9000, number=53, path=".gitea/workflows/rare.yml", started="2026-07-25T09:00:00Z"), + _run(run_id=8000, number=141, started="2026-07-25T11:00:00Z"), ] _install_urlopen(monkeypatch, _admin_ok(payload)) view = list_executions(_make_config(tmp_path), [], token="tok", now=_NOW) - assert [r.number for r in view.runs] == [12, 11, 10] + assert [r.number for r in view.runs] == [142, 141, 53] + + +def test_same_number_and_attempt_falls_back_to_run_id( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Two different workflows can land on the same run number independently + -- a real tie, not a bug. ``run_id`` just keeps the order stable/repeatable + rather than left to chance.""" + stamp = "2026-07-25T11:00:00Z" + payload = [ + _run(run_id=10, number=12, started=stamp), + _run(run_id=12, number=12, path=".gitea/workflows/other.yml", started=stamp), + _run(run_id=11, number=12, path=".gitea/workflows/third.yml", started=stamp), + ] + _install_urlopen(monkeypatch, _admin_ok(payload)) + + view = list_executions(_make_config(tmp_path), [], token="tok", now=_NOW) + + assert [r.run_id for r in view.runs] == [12, 11, 10] # ── endpoint fallback ───────────────────────────────────────────────────────