Merged
Conversation
Put required arguments first, optional args after
Since the Workloop inteface is just a static object, we should just create it at source in the parser. This removes some confusing typings and helps simplify the code. This miight enable a further refactor
josephjclark
commented
Mar 12, 2026
A pretty big change that seriously simplifies the worklook handling. Each workloop is its own instance, created straight from the string config, which manages its own state
Collaborator
Author
|
that last commit has made integration tests go beserk. Bit concerned about this Also seeing occasionally flaky fails on main actually - didI know about this? Right now I don't recall. |
josephjclark
pushed a commit
that referenced
this pull request
Mar 13, 2026
* Add --queues CLI option for slot group configuration (#1288) Introduce queue-aware slot allocation to support fast lanes. Workers can now dedicate specific slots to specific queues via a new `--queues` flag (e.g. `--queues "fast_lane:1 manual,*:4"`). - Add parseQueues() with SlotGroup type and full validation - Add --queues CLI option with WORKER_QUEUES env var support - Enforce mutual exclusivity between --queues and --capacity - Derive effectiveCapacity from slot groups in start.ts - Add queues field to ClaimPayload in lexicon * Review fixes: remove dead code, add TODO and duplicate queue warnings - Remove unreachable ?? 5 fallback in start.ts (capacity always has default) - Add TODO(#1289) for passing slotGroups to createWorker - Warn on duplicate queue names within a preference chain - Warn on identical queue configurations across slot groups - Add test for WORKER_QUEUES + WORKER_CAPACITY env var mutual exclusivity * Add changeset for queues CLI option * Fix ws-worker build: make capacity non-optional in Args type capacity always receives a default value via setArg, so the type should reflect that rather than requiring a runtime fallback. * Fix formatting for prettier * Implement per-group workloops and queue-aware claiming (#1289) Each slot group now gets its own independent workloop, tracks its own active runs and capacity, and sends queue-scoped claims to Lightning. The join payload includes a queues map so Lightning knows the slot distribution. Default behavior (no --queues) is preserved with a single manual,* group. * Extract groupHasCapacity helper to consolidate repeated capacity checks The pattern of computing pending claims and comparing against maxSlots was duplicated 4 times across server.ts. Extracts it into a single groupHasCapacity() function in parse-queues.ts with 5 unit tests. * Consolidate redundant claim tests Fold run-to-group tracking assertions into the existing execute test, remove the redundant workloop-stop test (already covered), and drop the now-covered todo. * Fix formatting for prettier * Fix CLI args ignored when invoked via pnpm start pnpm v7+ passes the '--' separator through to process.argv, causing yargs to treat all subsequent flags as positional arguments. Strip a leading '--' before parsing so --queues and other flags work correctly whether invoked via pnpm, npm, or directly. * Add ES2021 lib to ws-worker tsconfig for Promise.any The per-group workloops use Promise.any which requires ES2021 or later in the TypeScript lib setting. * Add JUnit XML test reporting for CircleCI Tests tab Pipes AVA TAP output through tap-xunit to generate JUnit XML, enabling test results to display in CircleCI's Tests UI. * Fix tap-xunit not found by using npx * Add per-package test:ci scripts for clean JUnit XML output Each package now produces its own XML file instead of piping combined pnpm -r output through tap-xunit, which was corrupted by pnpm's interleaved logging. * Fix "no tests found" warning in worker.test.ts Remove redundant env var guard wrapping an already-skipped test. AVA now sees the test as registered but skipped instead of finding zero tests and emitting a warning that causes a non-zero exit. * Refactor claim() to accept group as a standalone parameter Move group from ClaimOptions to a required positional parameter, making the function signature clearer about its dependencies. * Rename slots/queues terminology to workloops and merge runtime types - Rename --queues CLI option to --workloops (env: WORKER_WORKLOOPS) - Change queue separator from comma to angle bracket (fast_lane>*:4) - Rename SlotGroup → WorkloopConfig, maxSlots → capacity - Merge RuntimeSlotGroup + Workloop into single Workloop interface with stub stop/isStopped that get overwritten by startWorkloop() - Rename parse-queues.ts → parse-workloops.ts - Update all imports, types, variable names, and tests * Separate workloop parsing from runtime with WorkloopHandle pattern Move Workloop, createWorkloop, and workloopHasCapacity out of parse-workloops.ts (pure parsing) into api/workloop.ts (runtime). startWorkloop now returns a WorkloopHandle { stop, isStopped } instead of mutating the Workloop object. ServerApp stores handles in a workloopHandles Map, keeping Workloop as pure state with no lifecycle methods. Also fixes syntax error on claim.ts L81 and updates error message expectations in tests. * Fastlanes simpler workloops (#1297) * remove app.openClaims, which is not useful anymore
josephjclark
added a commit
that referenced
this pull request
Mar 13, 2026
* cli: ensure --endpoint flag is respected in deploy * fastlanes * Add --queues CLI option for slot group configuration (#1288) Introduce queue-aware slot allocation to support fast lanes. Workers can now dedicate specific slots to specific queues via a new `--queues` flag (e.g. `--queues "fast_lane:1 manual,*:4"`). - Add parseQueues() with SlotGroup type and full validation - Add --queues CLI option with WORKER_QUEUES env var support - Enforce mutual exclusivity between --queues and --capacity - Derive effectiveCapacity from slot groups in start.ts - Add queues field to ClaimPayload in lexicon * Review fixes: remove dead code, add TODO and duplicate queue warnings - Remove unreachable ?? 5 fallback in start.ts (capacity always has default) - Add TODO(#1289) for passing slotGroups to createWorker - Warn on duplicate queue names within a preference chain - Warn on identical queue configurations across slot groups - Add test for WORKER_QUEUES + WORKER_CAPACITY env var mutual exclusivity * Add changeset for queues CLI option * Fix ws-worker build: make capacity non-optional in Args type capacity always receives a default value via setArg, so the type should reflect that rather than requiring a runtime fallback. * Fix formatting for prettier * Implement per-group workloops and queue-aware claiming (#1289) Each slot group now gets its own independent workloop, tracks its own active runs and capacity, and sends queue-scoped claims to Lightning. The join payload includes a queues map so Lightning knows the slot distribution. Default behavior (no --queues) is preserved with a single manual,* group. * Extract groupHasCapacity helper to consolidate repeated capacity checks The pattern of computing pending claims and comparing against maxSlots was duplicated 4 times across server.ts. Extracts it into a single groupHasCapacity() function in parse-queues.ts with 5 unit tests. * Consolidate redundant claim tests Fold run-to-group tracking assertions into the existing execute test, remove the redundant workloop-stop test (already covered), and drop the now-covered todo. * Fix formatting for prettier * Fix CLI args ignored when invoked via pnpm start pnpm v7+ passes the '--' separator through to process.argv, causing yargs to treat all subsequent flags as positional arguments. Strip a leading '--' before parsing so --queues and other flags work correctly whether invoked via pnpm, npm, or directly. * Add ES2021 lib to ws-worker tsconfig for Promise.any The per-group workloops use Promise.any which requires ES2021 or later in the TypeScript lib setting. * Add JUnit XML test reporting for CircleCI Tests tab Pipes AVA TAP output through tap-xunit to generate JUnit XML, enabling test results to display in CircleCI's Tests UI. * Fix tap-xunit not found by using npx * Add per-package test:ci scripts for clean JUnit XML output Each package now produces its own XML file instead of piping combined pnpm -r output through tap-xunit, which was corrupted by pnpm's interleaved logging. * Fix "no tests found" warning in worker.test.ts Remove redundant env var guard wrapping an already-skipped test. AVA now sees the test as registered but skipped instead of finding zero tests and emitting a warning that causes a non-zero exit. * Refactor claim() to accept group as a standalone parameter Move group from ClaimOptions to a required positional parameter, making the function signature clearer about its dependencies. * Rename slots/queues terminology to workloops and merge runtime types - Rename --queues CLI option to --workloops (env: WORKER_WORKLOOPS) - Change queue separator from comma to angle bracket (fast_lane>*:4) - Rename SlotGroup → WorkloopConfig, maxSlots → capacity - Merge RuntimeSlotGroup + Workloop into single Workloop interface with stub stop/isStopped that get overwritten by startWorkloop() - Rename parse-queues.ts → parse-workloops.ts - Update all imports, types, variable names, and tests * Separate workloop parsing from runtime with WorkloopHandle pattern Move Workloop, createWorkloop, and workloopHasCapacity out of parse-workloops.ts (pure parsing) into api/workloop.ts (runtime). startWorkloop now returns a WorkloopHandle { stop, isStopped } instead of mutating the Workloop object. ServerApp stores handles in a workloopHandles Map, keeping Workloop as pure state with no lifecycle methods. Also fixes syntax error on claim.ts L81 and updates error message expectations in tests. * Fastlanes simpler workloops (#1297) * remove app.openClaims, which is not useful anymore * possible flaky test fix * CLI: Fix issue when running deploy with no locally changed workflows (#1287) * added a unit test for deploying a new project * remove only * enable mock lightning to load a project from yaml * update tests * failed attempt to repro * fix issue where a project with no local diffs can wrongly report diffs on deploy]# * fix types * fix tests * approve builds * fix test * remove dev log * revert dev log * fix lockfile * tweak test code for stability * another stability test tweak * versions --------- Co-authored-by: Stuart Corbishley <corbish@gmail.com>
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
1 participant
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.
Simplifying the
workloopsimplementation with the help of claude code.The base branch is a bit too complex - too many moving parts - which makes it harder to think about.
Changes made:
AI Usage
Please disclose whether you've used AI anywhere in this PR (it's cool, we just
want to know!):
You can read more details in our
Responsible AI Policy