Skip to content

feat(worker): retry routine dispatch, shut down gracefully, configure cadence - #639

Open
Ayush7614 wants to merge 1 commit into
CopilotKit:mainfrom
Ayush7614:feat/worker-dispatch-retry-shutdown
Open

Ayush7614 wants to merge 1 commit into
CopilotKit:mainfrom
Ayush7614:feat/worker-dispatch-retry-shutdown

Conversation

@Ayush7614

@Ayush7614 Ayush7614 commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

A single transient failure while handing a claimed routine run to the server used to fail the handoff outright.

Evidence (upstream/main):

  • worker/src/index.ts dispatch threw on the first non-202 with zero retries, so one 502/503/429 during a sweep marked the run failed.
  • TICK_MS (30s), PURGE_EVERY_N_TICKS (120) and PURGE_OLDER_THAN_MS (24h) were hardcoded with no env override and no jitter.
  • The loop was for(;;) with no SIGTERM/SIGINT handling, so Docker/K8s killed it mid-sweep and claims leaked until lease expiry.
  • worker/src/env.ts validated only 3 vars.

What this changes:

  • dispatchWithRetry (worker/src/retry.ts): retries 408/429/502/503/504 and transport failures with exponential backoff + jitter; 400/401/404 throw immediately. Last failure carries the status for last_error.
  • New WORKER_TICK_MS, WORKER_PURGE_EVERY_N_TICKS, WORKER_PURGE_OLDER_THAN_MS, WORKER_DISPATCH_RETRIES/TIMEOUT_MS/RETRY_BASE_MS, parsed and fail-fast validated in loadWorkerEnv (blank = default, non-numeric/out-of-range refused with the variable named).
  • Graceful shutdown (worker/src/shutdown.ts): SIGTERM/SIGINT lets the in-flight tick finish; the inter-tick sleep wakes early so containers meet their grace period.
  • import.meta.main guard so importing the module never starts the loop.
  • .env.example documents the new knobs.

Verification (all real, run locally):

  • bun test worker/tests/: 75 pass, 0 fail (env matrix, retry decision/backoff/attempt-count matrix, shutdown controller + interruptible sleep).
  • bun run --filter worker typecheck: clean.
  • biome lint + format on worker/src and worker/tests: clean.

… cadence

A single 502/503/429 while handing a claimed run to the server used to fail
the handoff outright, the 30s tick and purge cadences were hardcoded, and
SIGTERM killed the loop mid-sweep leaving claims to expire on their lease.

- dispatchWithRetry retries transient statuses (408/429/502/503/504) and
  transport failures with exponential backoff and jitter; 400/401/404 still
  throw immediately.
- WORKER_TICK_MS, WORKER_PURGE_EVERY_N_TICKS, WORKER_PURGE_OLDER_THAN_MS,
  WORKER_DISPATCH_RETRIES/TIMEOUT_MS/RETRY_BASE_MS parsed and fail-fast
  validated in loadWorkerEnv.
- SIGTERM/SIGINT lets the in-flight tick finish; the меж-tick sleep wakes
  early so containers meet their grace period.
- import.meta.main guard so importing the module never starts the loop.
- Tests: extended env matrix, retry decision/backoff/attempt-count matrix,
  shutdown controller and interruptible sleep.
@davidmckayv

Copy link
Copy Markdown
Contributor

Needs changes before merge, and it should be three PRs:

  • worker/src/index.ts is the laptop stand-in for the sweep. Deployments fire routines from the chart's CronJob running scripts/fire-routines.ts, which this PR does not touch, and docker-compose runs no worker. The Docker and Kubernetes case in the description does not apply.
  • Drop the retry. A dispatch that throws is already released and retried after 60 seconds, up to 5 attempts (server/src/routines/sweep.ts). /internal/routines/run does not de-duplicate a run id, so retrying a request whose response was lost runs the Bot's turn twice. The retry makes that more likely and stretches one dispatch past the 60-second lease.
  • Shutdown: a second SIGTERM or SIGINT does not force an exit, because requestShutdown returns early, and there is no deadline, so Ctrl+C can hang for minutes. Add a hard deadline that exits non-zero, make a second signal exit immediately, stop at the next item boundary, and correct the header comment that says a second signal exits promptly.
  • Cadence: WORKER_TICK_MS is allowed up to 600,000 ms, the same as DEFAULT_GRACE_MS. A routine that falls due just after a tick is more than ten minutes late by the next one and is skipped without firing. Cap it well below the grace window, for example 300,000, and raise the floor on WORKER_PURGE_OLDER_THAN_MS to at least an hour.
  • Document the new variables in docs/configuration.md and add a CHANGELOG entry.

This branch has not been deployed

No deployments
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.

2 participants