CopilotKit / CopilotKit/outpost

worker: a default-configured Worker claims job types it has no handler for, and destroys them

Open
#246 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

roadmap: now
Dominant language
TypeScript
Stars
7
Forks
3
Avg merge
7d 16h
Merged PRs (30d)
15

Description

Found during a review round on #224. Latent in production today, one config edit away from live.

What happens

claimAndProcessJobs claims PENDING rows with no type predicate — unlike claimJobsForType, which has AND type = ${type}. processJob then finds no handler and writes a terminal status: 'FAILED' tombstone.

FAILED is not PROCESSING, so reclaimStaleJobs will never touch it. Nothing else in the queue package re-queues FAILED rows, and handlers/job-cleanup.ts deletes only COMPLETED and DEAD_LETTER, so the row is not even reaped. The job is simply gone, with one console.warn.

Why it is not live today

apps/worker/src/index.ts sets concurrencyByType for all ten types, so hasPerTypeLimits is true and only claimJobsByType runs — and that iterates this.handlers, so it never claims a type it cannot handle.

But concurrencyByType is optional in WorkerOptions. A Worker constructed without it — the constructor default, the shape used throughout the tests, and what any other consumer of the exported Worker gets — takes the destroying path.

Why it matters beyond the default

Heterogeneous replicas are an explicitly anticipated state. reclaimStaleJobs's own docstring reasons about "a crashed claim of a type this replica does not handle", and #224 made the sweep deliberately type-blind for exactly that reason. The claim side has the opposite requirement and says nothing about it. A rolling deploy where a new revision enqueues a type the old revision lacks loses those jobs on the default config.

Suggested fix

Either constrain the batch claim to registered types:

AND type = ANY(${Array.from(this.handlers.keys())})

or make the no-handler path release the row back to PENDING with no attempt consumed, rather than tombstoning it.

Note the second option is a deliberate behaviour change, not an oversight fix: handles missing handler gracefully in queue.test.ts currently asserts the tombstone, so that test has to change with it.

Related

  • FAILED rows are never reaped by JOB_CLEANUP — worth closing at the same time, since the tombstone is the only writer of FAILED.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with claimAndProcessJobs, claimJobsByType, and processJob in the worker queue implementation, then read handlers/job-cleanup.ts and the handles missing handler gracefully case in queue.test.ts. Confirm how a Worker without concurrencyByType handles unregistered job types, and align the tests with the chosen behavior: filtering claims to registered types or returning jobs to PENDING without consuming an attempt.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend, distributed-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.