feat: add a queue reset so a repeated push cannot enqueue work twice - #28
Merged
Conversation
Mirrors specbandit (Ruby) #27. 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. 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. `reset` is registered as a boolean flag in the argument parser. A flag missing from that list consumes the next token as its value, so `push --reset a.test.ts` would otherwise lose the file. Per-runner rerun and failed keys are left alone, so a single-runner re-run can still replay its own files. 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". Also closes a version drift. The VERSION constant said 0.10.0 while the package was at 1.3.0, so `--version` and the report JSON both reported a version nobody shipped. It cannot import package.json, because tsconfig rootDir is src, so the release workflow now rewrites it alongside the package.json bump.
beagleknight
marked this pull request as ready for review
August 12, 2026 08:25
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.
Mirrors specbandit#27 (Ruby gem). Both are needed: factorial pushes backend specs through the gem and everything else through this package.
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.
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.resetis registered as a boolean flag in the argument parser. A flag missing from that list consumes the next token as its value, sopush --reset a.test.tswould otherwise lose the file.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.3.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".
Version drift, fixed here too
VERSIONinsrc/configuration.tssaid0.10.0while package.json was at1.3.0, sospecbandit --versionand thespecbandit_versionfield in--reportJSON both reported a version nobody shipped. The constant cannot import package.json, because tsconfigrootDirissrc, so the release workflow now rewrites it alongside the package.json bump and fails loudly if the constant moves.Tests
RedisQueue.clearissues oneDELwith both key names.reset: true, never on an empty push, and the leftover count is reported.--resetleaves 7, the marker survives, a worker sees each file exactly once, and a standaloneresetremoves the marker. Verified locally against a real Redis before pushing.Two pre-existing test failures (
jestAdapter/cypressAdapter"throws when not installed") reproduce on unmodifiedmainin a local checkout where those optional peers are present. They pass in CI.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.4.0) before factorial can adopt it.