Repository navigation
id(waiting): WaitingPool::insert's duplicate guard misses the ready queue — the same key inserted twice before take_ready is handed out twice #2001
Description
Summary
WaitingPool::insert in pkgs/id/src/waiting.rs promises that a duplicate is ignored — "An event whose key is already held or has arrived is a duplicate, and it is ignored" (insert docs, waiting.rs:180-184) — but the guard misses the one state an event can be in that is neither held nor arrived: ready. An event whose dependencies have all arrived is pushed onto the ready Vec (waiting.rs:206-211) without recording its key anywhere. Inserting the same key a second time before take_ready() is drained passes both guard checks (arrived.contains(&key) and by_key.contains_key(&key) are both false) and pushes a second copy, so take_ready() returns the same event twice and the caller applies / decides it twice.
Evidence (verified by code reading on main 77918e0, 2026-10-28)
insert(waiting.rs:193-212): the duplicate guard checks onlyself.arrivedandself.by_key. The ready path (missing.is_empty()) doesself.ready.push(event)and records the key in no index — notby_key, notarrived, no ready-key set exists in the struct (waiting.rs:124-141).- Deterministic trace with the pool's own test vocabulary (
WaitingPool<u32, &str>):pool.arrived(10, 0)— dependency 10 is present.pool.insert(author(1), 1, [10], "first", 0)→ ready =["first"].pool.insert(author(1), 1, [10], "second", 0)→ guard passes again → ready =["first", "second"].pool.take_ready()returns both copies of key 1.
The no-deps form (insert(author, 1, [], …)twice) behaves the same.
- The existing duplicate test does not cover this state:
test_a_duplicate_key_is_ignored(waiting.rs:578-589) pins the held-key duplicate and the arrived-key duplicate only; no test inserts a duplicate while the first copy is sitting inready. - Caller impact today is bounded but real: the event store's
drain(src/event_store/ingest.rs:725, 742) extends its work queue withtake_ready()after every settle, so a duplicated ready event is decided a second time and only the store's own duplicate detection (Settled::Duplicate) keeps it harmless. The pool's documented contract is what other callers (the module ispub, and its caller-loop protocol in the module docs, waiting.rs:16-24, tells callers to apply whattake_readyreturns) rely on.
Related but distinct: #1730 (the arrived set grows unboundedly) and #1567 (no total cap on held events) — neither covers the ready-queue duplicate.
Why it matters
The waiting pool is the causal-order gate for event ingest: everything it returns is treated as "apply this, once". A pool that can hand the same event out twice breaks its own stated contract and pushes duplicate defence onto every present and future caller. The fix is small and the module is pure (no I/O, explicit clock), so a regression test is cheap and deterministic.
Acceptance criteria
-
WaitingPool::insert(andpark, if it shares the path) ignores a second insert of a key that is already in the ready queue, matching the documented duplicate rule for held and arrived keys. - A regression test alongside
test_a_duplicate_key_is_ignoredcovers the ready-state duplicate (deps already arrived, and the no-deps form), assertingtake_ready()returns the event exactly once. - Existing waiting-pool tests still pass (
cargo test --lib waitingor the crate's equivalent targeted run). - No change to held / parked / restore semantics or to the per-author cap.
Filed by new-issue-creator-code-a (Lane A), 2026-10-28 run. Verification: code reading + deterministic trace on main 77918e0; no probe crate was compiled (per the laptop-only probe rule, and the trace needs no execution to settle — every step is a direct read of insert's guard and push).
Activity
developing-today commentedon Oct 28, 2026
CODE-925
dezren39 commentedon Oct 28, 2026
🤖 Agent note · filed
77918e0fby code reading + deterministic trace (no probe compiled; every step is a direct read ofinsert's guard and push).WaitingPool::insert(pkgs/id/src/waiting.rs:193-212) guards duplicates againstarrivedandby_keyonly; the ready path pushes the event with its key recorded in no index, so a second insert of the same key beforetake_ready()returns the event twice — contradicting insert's own documented duplicate rule (waiting.rs:180-184). Existing duplicate test (waiting.rs:578-589) pins held/arrived duplicates only. Store impact bounded today by ingest drain's ownSettled::Duplicate(ingest.rs:725, 742); the pool contract is the defect.26 remaining items
dezren39 commentedon Nov 1, 2026
agent-note:v1 — queue task 2918 (lane A, shift close-out) — targeted test PASSED
Update after the park: the laptop run left going at park time has finished —
cargo test --features web --lib waitingin~/git/fleet/2918-2001-waiting-ready: test result: ok. 38 passed; 0 failed (log: laptop/tmp/2918-waiting-test.log, EXIT:0).The resuming lane can go straight to the merge step: PR #3357 (head 20a8d5b, Codex clean, no findings) → merge → close #2001 → complete task 2918 as fixed_merged.
dezren39 commentedon Nov 1, 2026
🤖 Agent note · picked up (resume)
Merge pull request #3357 from developing-today/queue/2918-2001-ready-…
dezren39 commentedon Nov 1, 2026
🤖 Agent note · done