feat: add a queue reset so a repeated push cannot enqueue work twice - #27
Merged
Conversation
The queue key is scoped by CI run, not by CI attempt. That is deliberate: re-running a single failed runner does not re-run the job that pushed, so the runner must still find the queue and the published marker the first attempt created. The cost is that the producer is not idempotent. A producer that pushes and then fails, or that is re-run with the whole workflow, appends a second copy of the work list. Every file is then enqueued twice, the suite runs twice, and any two copies that reach the same worker are loaded twice in one process. That last part is fatal for test files that define constants at file scope. Add RedisQueue#clear, which removes the queue and its published marker in one DEL, and expose it two ways: - `specbandit push --reset` empties the key immediately before the RPUSH. - `specbandit reset --key KEY` does the same as a standalone command. Reset is opt-in, so `push` keeps its current meaning and appending to a queue stays possible. Per-runner rerun and failed keys are left alone, so a single-runner re-run can still replay its own files. A runner that finds data in both the shared queue and its rerun key is already handled by the full-rerun path. Nothing is cleared when there is nothing to push in its place: dropping the marker on its own would make every worker on that key crash as "never published".
jacobobq
approved these changes
Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A backend spec failed in factorial CI with a
T::Structprop redefinition error. It looked like a Sorbet/Tapioca problem. It was not: the same spec file was loaded twice in one process.The queue key is scoped by CI run, not by CI attempt. That is deliberate. Re-running a single failed runner does not re-run the job that pushed, so the runner must still find the queue and the published marker that the first attempt created.
The cost is that the producer is not idempotent:
Every file is then enqueued twice. The suite runs twice, and any two copies that reach the same worker are loaded twice in one process. For test files that define constants at file scope, the second load is fatal.
Change
RedisQueue#clearremoves the queue and its<key>:publishedmarker in a singleDEL. It is exposed two ways:Reset is opt-in.
pushkeeps its current meaning and appending to a queue stays possible.What reset does not touch
Per-runner rerun keys and failed keys. A runner's replay memory has to survive a re-run of that runner. A runner that finds data in both the shared queue and its rerun key is the full-rerun case, already handled in v1.2.0: it resets its own memory and steals.
Nothing is cleared when there is nothing to push in its place. Dropping the marker on its own would make every worker on that key crash as "never published".
Tests
RedisQueue#clearissues oneDELwith both key names.reset: true, never on an empty push, and the leftover count is reported.--resetleaves 7 and the marker survives.Also fixes a leak in the existing failed-key integration test, which deleted
<key>-failedbut not its:publishedmarker, so every run left a key behind.Follow-up, not in this PR
The flaky-retry pass consumes
<key>-failed-<n>, which is written bywork, notpush, so a re-run can still leave stale entries there. The blast radius is re-running an already-failed spec, not a double load.Next
Needs a
Releasedispatch (bump: minor→ v1.3.0) before factorial can adopt it. The npm package needs the same change; that PR is separate.