fix(webapp): keep paused environments paused when concurrency limits are pushed - #4624
fix(webapp): keep paused environments paused when concurrency limits are pushed#4624claude[bot] wants to merge 1 commit into
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>
|
|
Thanks for your contribution! We require all external PRs to be opened in draft status first so you can address CodeRabbit review comments and ensure CI passes before requesting a review. Please re-open this PR as a draft. See CONTRIBUTING.md for details. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| // A paused env is only enforced by a 0 limit in the RunQueue, so a push without an explicit | ||
| // limit has to stay 0 — otherwise it silently resumes an env the dashboard still shows as paused. | ||
| const limit = | ||
| maximumConcurrencyLimit ?? (environment.paused ? 0 : environment.maximumConcurrencyLimit); | ||
|
|
||
| await engine.runQueue.updateEnvConcurrencyLimits(updatedEnvironment); | ||
| await engine.runQueue.updateEnvConcurrencyLimits({ | ||
| ...environment, | ||
| maximumConcurrencyLimit: limit, | ||
| }); |
There was a problem hiding this comment.
🟡 An environment that was just resumed can be silently stopped from picking up work
The concurrency limit written into the queue is now forced to zero whenever the caller's copy of the environment says it is paused (environment.paused ? 0 : ... at apps/webapp/app/v3/runQueue.server.ts:11-12), and that copy is read earlier in the request (and from a read replica), so a deploy that started just before a resume can overwrite the restored limit with zero and leave a running environment unable to start any work.
Impact: An environment can appear active in the dashboard yet process nothing until someone pauses and resumes it again, or another concurrency update happens.
How a stale paused flag turns into a permanent zero limit
Deploy/worker-creation callers pass the environment object obtained at authentication time: finalizeDeployment.server.ts:126 uses authenticatedEnv, createBackgroundWorker.server.ts:241 uses environment. Those objects come from findEnvironmentByApiKey, which reads via $replica (apps/webapp/app/models/runtimeEnvironment.server.ts:275-289), so paused can be stale both because of replica lag and because the row can change during the request.
Sequence: (1) deploy request authenticates while the env is paused → paused: true captured; (2) user resumes → PauseEnvironmentService writes paused: false and pushes the real limit (apps/webapp/app/v3/services/pauseEnvironment.server.ts:123); (3) the in-flight deploy reaches updateEnvConcurrencyLimits(authenticatedEnv) and the new clamp writes 0. Nothing re-pushes the env limit afterwards, and dequeueing is gated only on that Redis value (internal-packages/run-engine/src/run-queue/index.ts:467-473), so the env stalls with paused: false in the database.
Before this change an argument-less push always wrote the stored limit, so this failure mode did not exist.
Prompt for agents
In apps/webapp/app/v3/runQueue.server.ts, updateEnvConcurrencyLimits now clamps the pushed env concurrency limit to 0 based on the `paused` flag of the environment object handed in by the caller. For the argument-less callers (finalizeDeployment.server.ts, createBackgroundWorker.server.ts, the admin environment routes) that object is captured at request authentication time and is read from the read replica, so `paused` can be stale. If the environment is resumed while such a request is in flight, the later argument-less push writes 0 over the restored limit and the environment stops dequeuing even though the database says paused: false, with nothing to restore it. Consider resolving the current paused state authoritatively inside the helper (e.g. a small primary-DB read of `runtimeEnvironment.paused` for the env id, or accepting an explicit `paused` argument that callers derive from a fresh row) so the clamp cannot be driven by a stale copy. Keep the explicit-limit behaviour (pause writes 0) unchanged.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
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.