cdp: rerun over a historical window mishandles already-replayed invocations
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 39.9k
- Forks
- 3.4k
- Avg merge
- 6h 51m
- Merged PRs (30d)
- 232
Description
Two findings from review on #85156, deliberately deferred there to keep that PR out of core rerun logic. Both stem from the same property: replay lifecycle rows land in the ClickHouse partition of their own scheduled_at, outside a historical rerun window, while the paginator reads several values from the window-bounded collapse.
1. Stale skips exhaust the cap
Skipped invocations count against max_count (remainingBudget = cap - queued - skipped), and a new rerun job restarts at the top of the window with the same DESC ordering. On a workflow with more matching failures than the cap: run 1 queues the newest cap-worth and stops; run 2 walks those same rows, finds them stale (already succeeded), charges them all against the cap, and finishes with queued=0. Every later run repeats this, so the leftover below the cap boundary is unreachable.
Sketch of a fix: charge only queued invocations against the cap; make isDone compare the budget to the page's queued count rather than its processed count.
2. Rehydration reuses the in-window snapshot
The cross-partition stale check added in #85156 validates only status. An invocation that a prior replay advanced and then failed again keeps latest status failed, so it correctly reruns - but invocation_globals, attempts, parent_run_id, and first_scheduled_at all come from the window-bounded argMax, i.e. the pre-replay state. For hog flows the stored currentAction is the resume point, so resuming from the old snapshot re-runs actions that already completed - re-sending emails that already went out. The max_attempts guard also reads the stale (lower) attempts value.
Sketch of a fix: extend the stale-status query into a latest-state fetch (status, is_deleted, attempts, invocation_globals, parent_run_id, first_scheduled_at via argMax without a partition bound) and rehydrate from that instead of the in-window row.
Both have review-comment threads with full analysis on #85156. The integration suite in nodejs/src/cdp/rerun/rerun-paginator.service.test.ts only uses windows that include now, which is why neither shows up there - regression tests for both should seed a succeeded/failed-again row outside the window.
🤖 Generated with Claude Code
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the rerun paginator and nodejs/src/cdp/rerun/rerun-paginator.service.test.ts, then read the linked review analysis on #85156. Add regression coverage using succeeded and failed-again rows outside the rerun window; done means stale rows no longer consume the cap and rehydration uses the latest state, including attempts and invocation globals.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- clickhouse, node.js, python
- Domain
- backend, databases, testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100