enforce maxConcurrentJobs with a per-assignee spawn lock

[bborbe]

Problem

maxConcurrentJobs (v0.5.0) was never actually enforced. It is a check-then-act race:
spawnIfNeeded is reached from two goroutines — the Kafka consumer (consumer.Consume)
and the deferred-respawn loop (RunDeferredRespawnLoop), both started by service.Run
(main.go:148-159) — with no lock spanning the count → create sequence. Each reads the same
live Job count before either has created its Job, so both conclude they are under the cap.
The number admitted tracks handler concurrency, not the cap.

Measured in prod with maxConcurrentJobs: 1:

Released at once	Jobs admitted
5	2
36	17
15	15

Every over-cap Job is then rejected by the agent's pods: 1 ResourceQuota, loops on
FailedCreate, and burns its full 1800s activeDeadlineSeconds while merely queued —
then is killed without ever running. All 15 from the last burst died deadline_exceeded
having never run. This is the mechanism behind the 2026-08-14 pipeline outage that
maxConcurrentJobs was written to prevent.

An earlier hypothesis blamed informer/cache lag. Disproven: CountActiveJobs issues a
live Jobs().List(), not a cached lister, and a quota-blocked Job is counted. There is no
cache to lag — the lock was simply missing.

Fix

lockAssigneeSpawn(assignee) — a per-assignee mutex held across the whole cap-check →
SpawnJob sequence, taken via defer h.lockAssigneeSpawn(...)() so error paths cannot leak it.

• Per-assignee, not global: a fleet-wide lock would make one agent's spawn latency
  everyone's. pr-reviewer-agent routinely runs several at once and must not queue behind
  github-update-go-agent.
• Only taken when a cap is configured — uncapped agents have nothing to serialize.
• Live count stays the source of truth, inside the lock: it self-corrects across executor
  restarts and externally deleted Jobs. An in-process counter alone would drift.
• ⚠️Correct only at replicas: 1 (verified in quant dev + prod). Scaling the executor
  reinstates the race across processes and would need a lease or leader election. Documented
  in the function comment.
• The never-dropped deferral path is untouched — deferring rather than skipping is what lets
  a saturated queue drain instead of losing tasks.

Also adds event=concurrency_admit (V(1)) logging the admit decision. Only deferrals
logged before, which is why the overshoot stayed invisible until 17 Jobs existed against a
cap of 1.

Test

10 concurrent ConsumeMessage calls against maxConcurrentJobs: 1, with the fake counting
Jobs the way the cluster does. Verified in both directions:

• lock disabled → 5 spawns (FAIL)
• lock enabled → 1 spawn (PASS)

The first version of this test passed even against the broken code — the real window is
sub-millisecond — so the stub now widens it deliberately. Under the lock those sleeps
serialize rather than overlap, so the delay cannot mask a regression; it only makes one
visible.

Unrelated but required for green CI

make precommit fails on master without these:

• golang.org/x/mod → v0.40.0 (GO-2026-6179, GO-2026-6180) — vulncheck gate
• Go 1.26.5 → 1.26.6, go.mod + Dockerfile (GO-2026-5026, -5972, -6089, -6090, -6218) — osv-scanner gate

make precommit green.

[ben-s-pull-request-reviewer]

Now I have all context needed. Let me compile the full report.

Summary of findings from the diff and mechanical funnel:

The PR adds a per-assignee mutex (spawnLocks map + lockAssigneeSpawn) to serialize the count→spawn sequence in spawnIfNeeded, fixing a check-then-act race that caused MaxConcurrentJobs to be vastly exceeded in production. It also adds a concurrent spawn test and bumps Go/tooling versions.

The mechanical funnel findings (174 total) are pre-existing violations in the unchanged parts of task_event_handler.go and throughout the test file. The diff itself introduces no new violations.

---

PR Review: fix/concurrency-cap-race → master

Must Fix (Critical)

None — no new violations introduced by this PR.

Should Fix (Important)

None attributable to this diff.

Nice to Have (Optional)

None.

---

Notes:

• The 174 mechanical findings are pre-existing (flagged in unchanged code). The go-composition/no-package-function-calls-in-business-logic (60+ findings) and go-testing/no-bare-error-call (60+ findings) were present before this PR — the diff only touches 2 Go files and the additions are the mutex, the defer call, the admit log, and the concurrent test, none of which violate those rules.
• The new concurrent test (pkg/handler/task_event_handler_test.go:305) is correctly structured: SpawnJobStub/CountActiveJobsStub assign function values (not bare calls) in BeforeEach/It scope; spawned.Add(1) operates on atomic.Int64 with no return value to discard.
• The lock is correctly implemented: lazy map allocation under spawnLocksMu, per-assignee mu.Lock()/mu.Unlock() pair with defer guaranteeing no leak on any early return path (lines 471–472).
• The event=concurrency_admit log at V(1) is appropriate — V(0) is reserved for operator-significant events; admit decisions are debug-shaped.
• Go 1.26.5→1.26.6 and golang.org/x/mod bump are correct hygiene per the CVE references.
• changelog/conventional-prefix-required is satisfied: all 4 ## Unreleased bullets use fix:, feat:, chore:.

---

Traceability (selector mode)

• Candidates: ~100 judgment rules matched by glob filter
• Applicable: 2 (go-context/cancel-check-in-loop, changelog/conventional-prefix-required)
• Skipped: go-licensing/copyright-year-discipline → no copyright edits in diff; go-build-args/three-args-required → Dockerfile ARG unchanged; go-mod-dependency-fix/tidy-after-get-update → go mod tidy was run (consistent version bumps across go.mod/go.sum); go-testing/no-bare-error-call → pre-existing in unchanged test lines; go-context/list-checks-ctx-done → pre-existing in unchanged code

{
"verdict": "approve",
"summary": "The per-assignee spawn lock is correctly implemented: lazy map allocation under a dedicated mutex, lock held across the count→spawn sequence with no early-return leak paths, and the new concurrent test exercises the race window with deterministic timing. The 174 mechanical findings are pre-existing violations in unchanged code. No new violations introduced.",
"comments": [],
"concerns_addressed": [
"correctness: race condition fixed by lockAssigneeSpawn in task_event_handler.go:172 — mutex serializes count-check and SpawnJob",
"correctness: test widens race window with 20ms stub sleep to make race visible and deterministic",
"security: maxConcurrentJobs cap now enforced before SpawnJob, preventing quota exhaustion from over-cap Jobs",
"performance: per-assignee (not global) mutex avoids cross-assignee contention"
]
}