fix(dimse): assign sequential-mode instances by StudyInstanceUID (#71)

[alexluft]

Fixes #71.

Problem. In wado-rs.mode: sequential, every received instance went to whichever retrieve was active for the AET ((AET, None)). When a retrieve ended early (client disconnect, timeout), the C-MOVE kept running at the PACS while the per-AET lock was released. The next retrieve could then receive the previous study's instances.

Change

• Sequential subscriptions are keyed by (AET, StudyInstanceUID). publish delivers by Move Originator Message ID as before; otherwise it delivers only to the retrieve of the instance's own study. The (AET, None) catch-all is gone.
• The per-AET semaphore becomes a per-(AET, study) one. C-MOVEs of the same study stay serialized; different studies no longer wait for each other. That parallelism is capped by the AET's association pool size (pool.size).
• Instances without a StudyInstanceUID cannot be attributed in sequential mode and are dropped with the existing warning.
• Concurrent mode and the STORE-SCP are unchanged. Docs and CHANGELOG are updated.

Tests. 7 new mediator unit tests, plus 2 on the topic a retrieve subscribes to (subscription_topic() in wado.rs; one routes through the mediator, so it fails if a sequential retrieve stops subscribing to its requested study). The regression test an_abandoned_retrieve_does_not_leak_into_the_next_one fails on main (the same scenario against main's API) and passes here. cargo test passes on stable and on MSRV 1.91; the existing Orthanc tests are STOW-only, so they show no regression elsewhere rather than covering this path. cargo fmt --check is clean, and clippy reports the same warnings as main.

Known limitation, pre-existing and not changed here. publish holds the callbacks lock while it sends into a retrieve's 1-slot channel, so one slow client can delay delivery to the others until it reads or disconnects.

Not in this PR (happy to follow up):

• an end-to-end C-MOVE test (the Orthanc harness has no move destination configured);
• C-CANCEL on stream drop;
• de-duplicating instances when an abandoned retrieve of the same study overlaps a new one.

Drafted with an AI agent (Claude), checked against the source and tested as above.

[nickamzol]

Unfortunately, this doesn't fix the underlying problem: keying by StudyInstanceUID narrows the mixup down to the same study, but doesn't prevent it.

The per-study lock is still released when the HTTP stream is dropped, not when the C-MOVE ends. If a client disconnects or times out, the peer keeps pushing the rest of the study. A second WADO-RS request for the same study then receives those leftover instances in addition to its own, so its response contains duplicates and instances from the other C-MOVE.

The root cause is correlation. Without a Move Originator Message ID, a received C-STORE carries nothing that ties it to the C-MOVE that caused it. The StudyInstanceUID says which study an instance belongs to, not which request asked for it. No routing key can assign an instance to its original request reliably.

What we can do instead is accept that a retrieve may receive instances from another C-MOVE, and make that harmless:

• Filter: discard every received instance that doesn't match the requested UIDs. Instances of other studies (possibly other patients) never reach the client.
• Deduplicate: return each SOP Instance UID only once. Leftovers from an earlier C-MOVE of the same study are then indistinguishable from our own copies, so the response is still exactly the requested set of instances.
• Hold the lock until the C-MOVE ends, not until the HTTP stream is dropped, so an abandoned C-MOVE can't overlap the next one.

The only thing we give up is knowing which C-MOVE delivered an instance, which doesn't change what the client receives.

Implemented in #73

[alexluft]

Thanks. Agreed on all points.

1. Narrows, doesn't prevent: right. fix(dimse): assign sequential-mode instances by StudyInstanceUID (#71) #72 keeps instances of another study away from a retrieve, but a second retrieve of the same study still gets the leftovers of an abandoned C-MOVE.
2. Lock released on stream drop: confirmed. fix(dimse): assign sequential-mode instances by StudyInstanceUID (#71) #72 kept releasing the subscription with the HTTP stream, so an abandoned C-MOVE can still overlap the next one.
3. Correlation: agreed. A StudyInstanceUID identifies the study, not the request; without a Move Originator Message ID nothing can.
4. Validate and deduplicate instances received via C-MOVE #73: holding the subscription until the C-MOVE task ends removes the overlap itself, and filter + dedupe make any residue harmless. Publishing before the C-STORE-RSP also closes the race where the C-MOVE completes before the last instance is handed over. Closing fix(dimse): assign sequential-mode instances by StudyInstanceUID (#71) #72 in favour of Validate and deduplicate instances received via C-MOVE #73.

A few small notes on #73, none blocking:

• No test covers the part that fixes the root cause (the permit/subscription held until the C-MOVE task finishes after the stream is dropped). Happy to contribute one.
• retrieve()/render() still pass a study-only identifier (wado.rs L67, L95), so the new Series/SOP matching doesn't engage for DIMSE retrieves yet. Fine as a follow-up.
• After a client disconnect, each remaining instance of the abandoned C-MOVE now logs The subscription channel is closed at error! (storescp.rs L178), where main logged MissingCallback at warn!. With Sentry enabled, every ERROR is an event, so that is one event per leftover instance. It also happens on every DIMSE /rendered request, because rendering returns after the first image while the C-MOVE continues. warn/debug for a closed receiver would avoid that.
• Without a C-CANCEL, the sequential permit is now held for the peer's whole C-MOVE, so a sequential /rendered (one image) waits behind the previous full-study transfer. A C-CANCEL when the stream is dropped would release it early. A possible follow-up, not a blocker.

Drafted with an AI agent (Claude), checked against the code of both PRs.