Skip to content

feat(orchestration): admission queue with deferred pickup (#441)#544

Open
nizar-lahlali wants to merge 1 commit into
mainfrom
feat/441-admission-queue-deferred-pickup
Open

feat(orchestration): admission queue with deferred pickup (#441)#544
nizar-lahlali wants to merge 1 commit into
mainfrom
feat/441-admission-queue-deferred-pickup

Conversation

@nizar-lahlali

Copy link
Copy Markdown
Contributor

Closes #441. Successor to #331 (fan-out mass-fail against the concurrency cap).

What

When admission hits the per-user concurrency cap, the task is now queued, not failed: it transitions SUBMITTED → QUEUED (a new pre-active state that holds no concurrency slot) and a scheduled AdmissionQueuePickup Lambda drains the queue in FIFO order (by created_at) as slots free up — flipping QUEUED → SUBMITTED and re-invoking the orchestrator.

Design highlights

  • Single counter writer. The pickup Lambda does a read-only capacity pre-check; the orchestrator's atomic admissionControl conditional-increment remains the only writer of the concurrency counter. A pickup that loses the admission race harmlessly re-queues (created_at never changes, so FIFO position is preserved). This avoids introducing a second writer — the class of drift bug behind Fan-out orchestration mass-fails against the per-user concurrency cap (no queue/retry) #331.
  • Cancel semantics preserved. Every queue transition is conditional on the current status, so a user cancel cleanly wins any race. QUEUED is cancellable via REST (already status-generic) and Slack (list extended). No compute or slot to release.
  • Idempotency preserved. Replay with the same Idempotency-Key returns the existing QUEUED task (200) — dedupe path is status-agnostic and untouched.
  • Backstops. A task queued > QUEUE_MAX_AGE_SECONDS (default 24 h) is failed with a clear message. The stranded-task reconciler now ages tasks by time-in-current-status so a long queue wait isn't mistaken for a stranded SUBMITTED task right after pickup.
  • Orchestrator re-invoke carries a per-pickup nonce so no dedup layer mistakes it for a replay of the create-task invocation.
  • Channel feedback: Linear/Jira comment now says "queued — will start automatically", and only on first queue entry (no per-cycle spam).

API / CLI surface

  • TaskRecord: queued_at, admission_attempts (internal bookkeeping).
  • GET /tasks/{id}: queued_at, queue_position (1-based FIFO rank among the caller's queued tasks, computed at read time, fail-open), estimated_wait_s (coarse heuristic).
  • bgagent status / detail view render Queue: position N (est. wait ~Xm) while QUEUED.
  • CDK↔CLI types mirrored; check-types-sync passes.

#331 regression test

reconcile-admission-queue.test.ts includes a fan-out-burst regression: 5 children queued above a cap of 3 all survive as QUEUED, drain FIFO (3 picked up, 2 remain), and zero tasks fail.

Testing

  • mise run build passes end-to-end (agent quality, CDK compile + synth + 2265 tests across 125 suites, CLI 576 tests, docs build + sync, type/constants drift checks).
  • New coverage: state-machine invariants (QUEUED pre-active, disjointness, transitions), queueTask (first-entry vs re-queue, race, no counter writes), pickup handler (FIFO, capacity bounds, per-user isolation, races, invoke-failure, max-age backstop, pagination), get-task queue position (rank, fail-open, left-queue race), CLI rendering.

Notes for reviewers

  • The ETA is a deliberately coarse UX hint (ceil(position / cap) × avg-task-duration, default 600 s, env-tunable) — not a promise.
  • AdmissionQueuePickup runs every minute; an empty-queue cycle is a single GSI query.
  • Marked as potentially breaking in the issue: clients that treated the admission-cap FAILED + admission_rejected event as terminal will now see QUEUED + admission_queued instead.

When a task hits the per-user concurrency cap, park it in a new QUEUED
state instead of failing it (the #331 mass-fail mode). A scheduled
AdmissionQueuePickup Lambda drains the queue FIFO (by created_at) as
slots free up, flipping QUEUED -> SUBMITTED and re-invoking the
orchestrator, whose atomic admissionControl stays the single writer of
the concurrency counter — a lost pickup race harmlessly re-queues
without losing FIFO position.

- task-status: new QUEUED state (pre-active — holds no slot), with
  SUBMITTED <-> QUEUED transitions plus QUEUED -> CANCELLED/FAILED
- orchestrator: queueTask() replaces failTask on cap; queued_at is
  stamped once, admission_attempts increments per pass; Linear/Jira
  channel feedback now says 'queued' and fires only on first entry
- reconcile-admission-queue: FIFO drain per user with read-only
  capacity pre-check, conditional flips (cancel-safe), 24h max-age
  backstop, and orchestrator re-invoke carrying a pickup nonce
- reconcile-stranded-tasks: age by time-in-current-status so a task
  that waited in the queue longer than the stranded timeout is not
  killed right after pickup
- API/CLI: GET /tasks/{id} computes read-time queue_position +
  estimated_wait_s (fail-open); bgagent status/detail render a
  'Queue: position N (est. wait ~Xm)' line
- cancel: QUEUED is cancellable everywhere (REST already generic;
  Slack cancel list extended); no concurrency release needed
- idempotent replay returns the existing QUEUED task unchanged
- tests: state-machine invariants, queueTask, pickup handler (incl.
  #331 fan-out-burst regression: burst above cap survives and drains
  FIFO with zero failures), get-task queue position, CLI rendering

Closes #441
@nizar-lahlali nizar-lahlali requested review from isadeks, krokoko and scottschreckengaust and removed request for isadeks July 8, 2026 14:59
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.

feat(orchestration): admission queue with deferred pickup

1 participant