Skip to content

fix: let queued tasks start while a long task is running - #74

Open
LUCKYREDDY31 wants to merge 1 commit into
CopilotKit:mainfrom
LUCKYREDDY31:fix/worker-queued-tasks-starve
Open

LUCKYREDDY31 wants to merge 1 commit into
CopilotKit:mainfrom
LUCKYREDDY31:fix/worker-queued-tasks-starve

Conversation

@LUCKYREDDY31

Copy link
Copy Markdown
Contributor

Problem

TaskWorker.tick() claims up to three due tasks and then awaits all of their runs before it releases ticking. Every later tick returns early, so no new task starts until the slowest run in the batch finishes.

A model task can run for up to five minutes, and each browser request can take up to 45 seconds. While one runs, everything else waits: a quick job such as a spending summary stays queued, a watch's scheduled check runs late, and a task whose review was just approved does not resume. The worker keeps writing its heartbeat during this time, so the app reports it as running and the person sees no reason for the delay. Nothing is lost or run twice; the work is only late.

What changed

  • apps/server/src/engine/worker.ts: tick() now starts the runs it claims and releases ticking without waiting for them, so the next tick can start queued work. A new inFlight set tracks tasks this worker has started and not yet settled. It replaces the active check when picking tasks, and the limit of three now counts tasks already in flight, so a worker still runs at most three at once. tick() still awaits the runs it started, so direct callers behave as before. stop() waits for inFlight instead of active, which also covers a task that is still being claimed.
  • tests/engine.test.ts: a regression test next to "pending reviews do not starve queued work". It holds one task open, queues a second, ticks again, and asserts the second task finished while the first is still running.

Verification

  • The new test fails on main (the second task stays queued) and passes with the fix. With only the worker.ts change reverted, it is the one failing test out of 207.
  • The new test passed 20 of 20 repeated runs.
  • pnpm lint, pnpm typecheck, pnpm --dir apps/worker typecheck, pnpm test (207/207) and pnpm build:server pass on Node 24 and Node 22. On Node 24, the web, iOS and Android exports, pnpm test:browser, and both Docker container suites also pass.
  • End to end in sample mode with a scripted model: while a delegated task waited 15 seconds on its first model call, a finance task and a new watch added at 2.0s completed at 3.1s. On main both waited until 17.1s, when the slow task finished.
  • Local checks with two workers sharing one database and 20 tasks: neither worker ran more than three at once, every task ran exactly once, stop() still requeued an interrupted task, and cancelling a running task still stopped it.

Security review

  • No new data flows, dependencies, configuration or environment variables.
  • Task claiming is unchanged: each run still takes its lease through the same compare-and-swap, so two ticks or two workers cannot run the same task.
  • The per-worker limit of three concurrent runs is kept.
  • Lease, heartbeat, cancellation and approval checks are unchanged.
  • Tests use the in-memory store and a local fixture model only.

TaskWorker.tick() awaited every run it claimed before releasing its lock,
so one slow task held up all queued work, scheduled watch checks and
approved tasks until it finished. Start the claimed runs and release the
tick right away, tracking started tasks in an inFlight set so a worker
still runs at most three at once and never starts the same task twice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant