Skip to content

fix(list-recent-runs): close the completion window at the cron tick - #850

Open
tend-agent wants to merge 1 commit into
mainfrom
hourly/review-31001852480
Open

fix(list-recent-runs): close the completion window at the cron tick#850
tend-agent wants to merge 1 commit into
mainfrom
hourly/review-31001852480

Conversation

@tend-agent

Copy link
Copy Markdown
Collaborator

list-recent-runs.sh anchors its completion window's floor to the intended cron tick, and advances that floor by exactly one period per cycle. It never sets a ceiling — the filter is updatedAt >= cutoff and nothing more. So each cycle lists floor .. now, which is wider than one period by however late the GHA scheduler was, and the next cycle — whose floor is exactly this cycle's tick — lists that tail a second time. The script's own header says the opposite: "Consecutive cycles then tile exactly: [intended-1h, intended], then [intended, intended+1h]." That property only holds with the ceiling this PR adds.

The cost is a duplicated agent survey, on the workflow that is the fleet's dominant token consumer.

Measured on this repo's last two cycles

review-reviewers ticks are landing 45–55 min late (09:41:26Z10:41:41Z11:32:14Z, against 47 * * * *), so the redundant share is close to the whole period:

Cycle Intended tick Floor Listed through Runs listed
30998444312 09:47Z 08:47Z 10:42Z (its own start) 21
31001852480 (this run) 10:47Z 09:47Z 11:32Z 21

19 of this cycle's 21 runs had already finished before the previous cycle began listing, so they were in that cycle's list too and were surveyed twice. Only two runs — 30998444312 itself and 30999831196 — were new. Both cycles independently reached the same all-clear on the same runs.

With the ceiling, this cycle's window closes at 10:47Z and the two post-tick runs move to the next cycle, whose floor is 10:47Z. Nothing is dropped, only deferred by one cycle: the deferral is bounded by the period, and the dropped-tick recovery below still reaches further back when a tick never fires.

Verification

New generator/tests/test_list_recent_runs.py drives the real script through a stub gh (no network), placing fixture runs either side of the tick:

  • test_window_is_half_open_at_the_cron_tick — pre-floor, in-window, post-tick and still-running runs; asserts only the in-window pair is listed. Mutation-checked: fails on main with [2, 3, 4] vs the expected [2, 3].
  • test_next_cycle_picks_up_what_the_ceiling_deferred — a run finishing just after the previous tick is listed by this cycle, i.e. deferred rather than dropped.
  • test_non_periodic_cron_keeps_the_now_anchored_window_uncapped — off the cron path (*/15 * * * *, workflow_dispatch) there is no next cycle to hand a tail to, so no ceiling is applied.

Each test derives the cron minute as (now.minute + 30) % 60, putting the tick ~30 min from the clock in either direction so it can't race a rollover. shellcheck -S warning clean, bash -n clean, full generator suite 330 passed.

Relationship to the other open PRs on this file

Neither of the two open PRs touching list-recent-runs.sh closes this gap; both keep the floor-only filter and both restate the "tile exactly / no overlap" claim in comments.

The edits are textually adjacent to both (a new assignment after COMPLETED_AFTER), so whichever lands second wants a small rebase; neither depends on the other.

Gate assessment

  • Evidence level: High. Mechanically determined, not sampled — with a non-zero scheduler delay the overlap occurs every cycle by construction. Measured at 19/21 runs this cycle; the evidence log records prior cycles noting runs "already surveyed by the prior run; listed by the run script because of window overlap", and the ~45–55 min lag has been confirmed in eight consecutive windows.
  • Structural: yes. No decision point — the same conditions produce the same duplication every cycle.
  • Change type: targeted fix (one assignment plus the filter clause it feeds), with a regression test.
  • Passes both gates.

Evidence log: https://gist.github.com/e08f6e62d6478163cb425a75648eb7e4

The window's floor advances one cron period per cycle but had no ceiling, so each cycle listed floor..now — wider than a period by the scheduler's delay — and the next cycle re-listed that tail. With ticks landing 45-55 min late, 19 of 21 runs in a cycle had already been surveyed by the previous one. Close the window at the tick, exclusive, so consecutive cycles tile as the header already claims; runs finishing after the tick are the next cycle's first items, not dropped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude-behavior Behavioral issues found by review-reviewers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant