fix(webapp): keep paused environments paused when concurrency limits are pushed - #4625
Merged
Merged
Conversation
…are pushed `updateEnvConcurrencyLimits` is the only enforcement of an environment pause: pausing writes a 0 env concurrency limit to the run queue, which is what stops dequeueing. Callers that push the limit without an explicit value (finalizing a deployment, creating a background worker, the admin concurrency/burst-factor routes) rewrote the real limit, silently resuming an environment the dashboard still showed as paused. Clamp the pushed limit to 0 when the environment is paused and no explicit limit is given, so every caller is covered. An explicit limit still wins. Resuming now passes the post-update state, and the helper no longer mutates the caller's environment object (which made a pause + resume on the same object write 0 twice). Co-Authored-By: Claude <noreply@anthropic.com>
|
matt-aitken
marked this pull request as ready for review
August 14, 2026 19:07
…currency limits The clamp added in the previous commit read `paused` off the environment object the caller passed, which is captured when the request authenticates and can be read from a replica. A stale `paused: false` still silently resumes a paused environment, and a stale `paused: true` writes 0 over a limit that was just restored - leaving the environment stalled with `paused: false` and nothing to put the limit back. Resolve `paused` and `maximumConcurrencyLimit` for the environment id inside `updateEnvConcurrencyLimits` when no explicit limit is given, and let callers hand in the client they wrote with so the read is primary- and transaction-consistent. The explicit-limit path is unchanged: pausing and billing-limit converge still decide the value with no read. Move the RunQueue limit cases into their own test file so the pre-existing suite keeps the shared engine stub instead of a file-level mock, and cover the resumed-while-in-flight ordering. Co-Authored-By: Claude <noreply@anthropic.com>
matt-aitken
approved these changes
Aug 14, 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.
Requested by Matt Aitken · Slack thread
Before: you pause an environment, then a deploy lands (or a background worker is created, or an admin changes the concurrency/burst-factor). The environment starts picking up runs again even though the dashboard still shows it as paused.
After: a paused environment stays paused until it is resumed, no matter what else pushes its concurrency limit.
Pausing an environment sets
pausedin the database and writes a0env concurrency limit into the run queue — the0is the only thing that actually stops dequeueing. Any caller that pushed the limit without an explicit value (finalizeDeployment,createBackgroundWorker, the two admin environment routes) rewrote the real limit and silently un-paused the environment.✅ Checklist
Testing
apps/webapp/test/pauseEnvironment.server.test.tsgains twocontainerTestcases that wire a realRunEngine(real Redis) in place of the stubbed app singleton and assert the actual run-queue env limit:0→ run the realFinalizeDeploymentService→ limit is still0, plus a control on a running env in the same test proving that deploy path really does push the limit (so the0can't just mean "nothing happened").Both cases fail on
main(expected 17 to be +0andexpected +0 to be 17) and pass with this change.pnpm run typecheck --filter webappis clean.Changelog
Fix paused environments starting to run work again after a deploy.
How
The clamp lives in the shared
updateEnvConcurrencyLimitshelper inapps/webapp/app/v3/runQueue.server.ts, so every present and future caller is covered: when no explicit limit is passed and the environment is paused,0is written instead of the stored maximum. An explicitly-passed limit still wins, which is what pausing itself relies on. The resume path now passes the post-update environment state (its in-memory copy was read before the un-pause and would otherwise be clamped back to0), and the helper no longer mutates the caller's environment object — that aliasing made a pause followed by a resume on the same object write0twice. The existing!pausedguards inallocateConcurrencyand the queue-level guard increateBackgroundWorkerare left in place as defence in depth, and queue-levelTaskQueue.pausedbehaviour is untouched.