fix(web): Executions sorts by actual global recency, not per-repo run number
number is a per-repo/workflow counter with no meaning once the admin endpoint mixes every repo on the instance into one feed -- a rarely-run workflow's low #53 that just fired outranked a busier workflow's #142 from hours earlier, but sorting by number buried it instead. Sort by started (globally comparable) with run_id/attempt as tiebreaks; a re-run bumping started_at back to the top is correct, not a bug, under "most recent globally".
This commit is contained in:
+10
-12
@@ -386,18 +386,16 @@ 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
|
sized to fit `cancelled`, the longest word) with left-aligned content, so the
|
||||||
column reads as a clean list instead of ragged pill widths.
|
column reads as a clean list instead of ragged pill widths.
|
||||||
|
|
||||||
Rows are ordered **newest first by run number, then attempt** — the `#N` the
|
Rows are ordered **newest first by actual start time, globally** —
|
||||||
Run column actually shows, not `started` timestamp and not `run_id` either.
|
`run.started` (a run still queued reads as "now", since it was only just
|
||||||
Both were tried and both break once more than one repo/workflow is in the
|
created), with `run_id` then `attempt` as tiebreakers for runs sharing the
|
||||||
mix (the admin endpoint pulls from every repo on the instance): sorting by
|
exact same timestamp. `run.number` is deliberately *not* the sort key: it is
|
||||||
time lets a manually re-run old workflow (its `started_at` bumped to now,
|
a per-repo/workflow counter, and the admin endpoint mixes every repo/workflow
|
||||||
its number unchanged) jump above a genuinely newer run; sorting by `run_id`
|
on the instance into one feed, so a rarely-triggered workflow's #53 that just
|
||||||
looks right for a single workflow but `run_id` is an *instance-wide*
|
fired legitimately outranks a #142 from a busier workflow that ran hours
|
||||||
sequence shared by every repo/workflow, not a per-workflow counter, so a
|
earlier — sorting by `number` would bury it. A manual re-run bumping
|
||||||
rarely-triggered workflow's #53 can land a `run_id` between two ids from a
|
`started_at` and jumping its row back to the top is not a bug either: it
|
||||||
busier workflow's #141/#142 — putting #53's row between them even though 53
|
really is the most recent activity on that run.
|
||||||
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 row-count dropdown at
|
||||||
the right end of the card's header bar picks how many to list — 10 / 25 / 50 /
|
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
|
100, default from `[web].executions_limit` (10). The choice rides on the card's own polling URL
|
||||||
|
|||||||
@@ -350,27 +350,21 @@ def _cached_runs(
|
|||||||
return runs, source, error, auth_failed, refused
|
return runs, source, error, auth_failed, refused
|
||||||
|
|
||||||
|
|
||||||
def _recency_key(run: Execution) -> tuple[int, int, int]:
|
def _recency_key(run: Execution, now: datetime) -> tuple[datetime, int, int]:
|
||||||
"""Sort key for "newest first" (used with ``sorted(..., reverse=True)``).
|
"""Sort key for "newest first" (used with ``sorted(..., reverse=True)``).
|
||||||
|
|
||||||
Ordered by ``number`` -- the ``#N`` the Run column actually shows -- then
|
Global recency: the admin endpoint mixes every repo/workflow on the
|
||||||
``attempt``, then ``run_id`` only to keep ties stable. ``run_id`` is
|
instance, and each one keeps its own ``number`` counter, so there is no
|
||||||
Gitea's *instance-wide* id (shared across every repo/workflow the admin
|
way to read "most recent across the whole instance" off a per-repo
|
||||||
endpoint pulls from), not a per-workflow counter, so it does not track
|
number -- a #53 from a rarely-triggered workflow that just ran outranks a
|
||||||
``number`` at all once more than one workflow is in the mix: a run #53
|
#142 from a busier one that ran hours ago. ``started`` is what actually
|
||||||
from a rarely-triggered workflow can easily land a run id between two
|
answers "when did this happen"; a run still queued (no ``started`` yet)
|
||||||
#141/#142 runs from a busier one, putting #53's row between them even
|
reads as ``now`` since it was only just created. A manual re-run bumping
|
||||||
though the column reads 142, 141, ..., 53. ``number`` is what "newest
|
``started_at`` and jumping the row back to the top is not a bug here --
|
||||||
first" has to mean here.
|
it really is the most recent activity on that run. ``run_id``/``attempt``
|
||||||
|
only break a tie between runs sharing the exact same timestamp.
|
||||||
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.number, run.attempt, run.run_id)
|
return (run.started or now, run.run_id, run.attempt)
|
||||||
|
|
||||||
|
|
||||||
def _attach_vms(
|
def _attach_vms(
|
||||||
@@ -462,7 +456,7 @@ def list_executions(
|
|||||||
runs, unmatched = _attach_vms(
|
runs, unmatched = _attach_vms(
|
||||||
sorted(
|
sorted(
|
||||||
(_to_execution(item, moment) for item in raw_runs),
|
(_to_execution(item, moment) for item in raw_runs),
|
||||||
key=_recency_key,
|
key=lambda run: _recency_key(run, moment),
|
||||||
reverse=True,
|
reverse=True,
|
||||||
),
|
),
|
||||||
vms,
|
vms,
|
||||||
|
|||||||
@@ -432,50 +432,43 @@ def test_queued_run_sorts_above_started_runs(
|
|||||||
assert [r.status for r in view.runs] == ["queued", "success"]
|
assert [r.status for r in view.runs] == ["queued", "success"]
|
||||||
|
|
||||||
|
|
||||||
def test_a_rerun_does_not_jump_above_a_newer_run(
|
def test_a_rerun_sorts_above_a_run_that_started_earlier(
|
||||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Re-running an old workflow bumps its ``started_at`` to now, but its
|
"""Re-running an old workflow really is fresh activity -- it belongs at
|
||||||
run id/number never change -- sorting by wall-clock time would put that
|
the top of a "most recent globally" feed, same as any other run that just
|
||||||
stale run above a genuinely newer one it should still trail. The Run
|
started."""
|
||||||
column must read strictly by run, descending, not by whichever attempt
|
|
||||||
was most recently kicked off."""
|
|
||||||
payload = [
|
payload = [
|
||||||
_run(run_id=12, number=12, attempt=1, started="2026-07-25T10:00:00Z"),
|
_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"),
|
_run(run_id=13, number=13, attempt=1, started="2026-07-25T10:05:00Z"),
|
||||||
# #12 gets manually re-run after #13 already started -- its
|
# #12 gets manually re-run after #13 already started.
|
||||||
# started_at is now the latest timestamp of the three.
|
|
||||||
_run(run_id=12, number=12, attempt=2, started="2026-07-25T10:10:00Z"),
|
_run(run_id=12, number=12, attempt=2, started="2026-07-25T10:10:00Z"),
|
||||||
]
|
]
|
||||||
_install_urlopen(monkeypatch, _admin_ok(payload))
|
_install_urlopen(monkeypatch, _admin_ok(payload))
|
||||||
|
|
||||||
view = list_executions(_make_config(tmp_path), [], token="tok", now=_NOW)
|
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)]
|
assert [(r.number, r.attempt) for r in view.runs] == [(12, 2), (13, 1), (12, 1)]
|
||||||
|
|
||||||
|
|
||||||
def test_a_lower_numbered_run_from_a_busier_workflow_does_not_land_in_between(
|
def test_a_more_recent_run_from_a_quiet_workflow_sorts_above_an_older_higher_numbered_run(
|
||||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||||
) -> None:
|
) -> None:
|
||||||
"""``run_id`` is instance-wide (every repo/workflow the admin endpoint
|
"""``number`` is a per-repo/workflow counter -- it cannot answer "most
|
||||||
pulls from shares one sequence), not a per-workflow counter -- a rarely
|
recent across the whole instance" at all once more than one repo is in
|
||||||
triggered workflow's #53 can easily get a run id that falls between two
|
the mix. A rarely-triggered workflow's #53 that just ran outranks a
|
||||||
ids from a busier workflow's #141/#142, even though 53 is nowhere near
|
#141/#142 from a busier workflow that ran hours earlier."""
|
||||||
141 or 142. The Run column has to read by the number it shows, not by
|
|
||||||
that shared id."""
|
|
||||||
payload = [
|
payload = [
|
||||||
_run(run_id=9001, number=142, started="2026-07-25T12:00:00Z"),
|
_run(run_id=8000, number=141, started="2026-07-25T09:00:00Z"),
|
||||||
# A different, rarely-run workflow's id landed between the two above
|
_run(run_id=9001, number=142, started="2026-07-25T10:00:00Z"),
|
||||||
# purely because of when it happened to fire -- not because #53 is
|
# A different, rarely-run workflow -- but it just fired, after both.
|
||||||
# newer than #141.
|
_run(run_id=1, number=53, path=".gitea/workflows/rare.yml", started="2026-07-25T11:00:00Z"),
|
||||||
_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))
|
_install_urlopen(monkeypatch, _admin_ok(payload))
|
||||||
|
|
||||||
view = list_executions(_make_config(tmp_path), [], token="tok", now=_NOW)
|
view = list_executions(_make_config(tmp_path), [], token="tok", now=_NOW)
|
||||||
|
|
||||||
assert [r.number for r in view.runs] == [142, 141, 53]
|
assert [r.number for r in view.runs] == [53, 142, 141]
|
||||||
|
|
||||||
|
|
||||||
def test_same_number_and_attempt_falls_back_to_run_id(
|
def test_same_number_and_attempt_falls_back_to_run_id(
|
||||||
|
|||||||
Reference in New Issue
Block a user