knowledge_process_chunk: per-(group_id, chunk_id) locking to close concurrent-resubmission race

[verveguy]

Background

#284 (PR #286) documents a known, accepted concurrency limitation in handle_knowledge_process_chunk: the idempotency guarantee (identical chunk_text resubmission no-ops, changed chunk_text replaces) only holds for serialized calls per chunk_id. Two concurrent calls sharing a chunk_id — whether both are first-time submissions or one is a client retry racing the original in-flight request — can both observe the same PriorState and both proceed to insert, because state.write_lock is held only for the brief lookup and (if needed) delete steps, not across the intervening extraction/embedding work.

This was raised again during Validate-stage review of #284/PR #286 (review thread on handlers.rs:649), which asked that a tracking issue be filed so the follow-up doesn't get lost.

Documented shapes of the gap (see ADR-0052, "Consequences" section, in docs/adr/0052-chunk-splitting-and-chunk-id-idempotency.md)

1. Two concurrent first-time or retry submissions for the same chunk_id can both observe PriorState::None (or the same prior state) and both insert, producing duplicate/divergent episodes until a later resubmission self-heals via the mismatch/Anomalous path.
2. A concurrent knowledge_delete_chunk_episode interleaved with an in-flight split's per-unit inserts can remove already-committed units mid-split, since each unit acquires/releases write_lock independently rather than holding it for the whole split.
3. Two concurrent Replace-path resubmissions with different chunk_text can both run their own fresh ingest and both attempt to delete the same prior UUIDs — the loser's delete is a silent no-op, but both callers' new episode sets survive under the same chunk_id, with no stored signal for which is "current."

Recommended fix

A new per-(group_id, chunk_id) async lock held for the full request duration (lookup → extraction → insert/delete), rather than the current brief write_lock acquisitions. The global state.write_lock cannot be extended across extraction without serializing all concurrent ingestion (including unrelated chunk_ids) behind LLM latency — a much larger regression than the race itself.

This requires:

• A new AppState field (a lock map, likely DashMap<(String, String), Arc<tokio::sync::Mutex<()>>> or similar, with a cleanup/eviction strategy to avoid unbounded growth).
• AppState is hand-constructed via struct literal in 22 separate test files (rg -l "AppState {" as of knowledge_process_chunk: internal splitting for oversized chunk_text (follow-up to #282) #284) with no shared test constructor — this field addition touches all of them.
• Careful cleanup semantics: an unbounded lock-map leak (never removing entries for chunk_ids no longer in flight) would trade this race for a slow memory leak.

Scope note

Out of scope for #284/PR #286 itself — flagged repeatedly across that PR's Review and Validate passes as a materially larger, architecture-level change than fits a review-comment fix. #284 ships this as a documented, accepted limitation (surfaced to callers in README.md and the knowledge_process_chunk tool description in crates/service/src/mcp/tools.rs, in addition to ADR-0052).