From 98ddcd35955e1440d4726d54f0c138ded4ae36b4 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 16 Jul 2026 12:43:34 +0200 Subject: [PATCH] =?UTF-8?q?audit:=20cycle-close=20tidy=20for=20#276=20?= =?UTF-8?q?=E2=80=94=20guard=20the=20third=20append=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Architect drift review for the #276 cycle (b048923..HEAD). What holds: C1 is preserved (the append_write_lock sits at the results-persistence edge, outside the lock-free sim loop); the mechanism is correct (the guard spans the full read-counter-then-write critical section in both locked functions, no re-entrancy via the load paths, poison entered by into_inner); both fixed paths are pinned by property tests. One [medium] drift item, resolved here on the fix path: Registry::append (the flat runs store, runs.jsonl) was the third identical unsynchronized OpenOptions::append + writeln! path, left standing unguarded beside its two newly-guarded siblings — and it is public API with external consumers (C18), so a concurrent caller would hit exactly the #276 corruption. The append_write_lock doc also enumerated the two guarded stores as if exhaustive. Fixed RED-first: the sibling property test failed 2/2 against the unguarded path (same "trailing characters" merged-line corruption), then append takes the same lock and the doc now names all three JSONL append paths. No other finding: put_blueprint / put_doc (content-addressed idempotent writes) and TraceStore::write (per-name whole-file writes) are race-tolerant by shape and correctly outside the append scope. Regression gate: no dedicated regression scripts configured; the project-facts gates all green — cargo test --workspace no failures (the three concurrency property tests among them), clippy --workspace --all-targets -D warnings clean, cargo doc --workspace --no-deps clean. Cycle #276 is drift-clean. refs #276 --- crates/aura-registry/src/lib.rs | 59 +++++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/crates/aura-registry/src/lib.rs b/crates/aura-registry/src/lib.rs index 03024ce..0b1f983 100644 --- a/crates/aura-registry/src/lib.rs +++ b/crates/aura-registry/src/lib.rs @@ -49,8 +49,9 @@ pub struct Registry { /// read-the-counter-then-write critical section (`append_family`, /// `append_campaign_run` in `lineage.rs`), so concurrent writers through /// one handle also see a consistent per-name run counter. Store-agnostic: - /// it guards BOTH `families.jsonl` and the separate `campaign_runs.jsonl` - /// store (they never contend on the same file, but share this one lock). + /// it guards all three JSONL append paths — the flat runs store + /// ([`Registry::append`]), `families.jsonl`, and `campaign_runs.jsonl` + /// (they never contend on the same file, but share this one lock). /// In-process only; cross-process locking stays out of contract. append_write_lock: std::sync::Mutex<()>, } @@ -73,8 +74,11 @@ impl Registry { } /// Append one record as a single JSON line, creating the file (and its parent - /// directory) if absent. + /// directory) if absent. The write is serialized by `Registry`'s internal + /// `append_write_lock` (#276) — safe for concurrent writers sharing one + /// `&Registry`. pub fn append(&self, report: &RunReport) -> Result<(), RegistryError> { + let _guard = self.append_write_lock.lock().unwrap_or_else(std::sync::PoisonError::into_inner); if let Some(parent) = self.path.parent().filter(|p| !p.as_os_str().is_empty()) { fs::create_dir_all(parent)?; } @@ -1242,6 +1246,55 @@ mod tests { let _ = fs::remove_dir_all(path.parent().expect("temp family dir")); } + /// Third sibling of the #276 concurrency property, for the flat runs store: + /// `Registry::append` (→ `runs.jsonl`) shares the same + /// `OpenOptions::append` + `writeln!` pattern as the family and + /// campaign-run appends, and it is public API with external consumers + /// (C18), so N threads appending through one shared `&Registry` must leave + /// `runs.jsonl` well-formed — every successful append's record intact and + /// parseable, none torn, merged, or lost. + #[test] + fn concurrent_flat_appends_keep_the_runs_store_well_formed() { + let path = temp_family_dir("concurrent_flat"); + let reg = Registry::open(&path); + let report = report_with(1.0, 0.5, 1); + let n_threads = 8usize; + let iters = 50usize; + + let total_ok: usize = std::thread::scope(|scope| { + let handles: Vec<_> = (0..n_threads) + .map(|_| { + let reg = ® + let report = &report; + scope.spawn(move || { + let mut ok = 0usize; + for _ in 0..iters { + if reg.append(report).is_ok() { + ok += 1; + } + } + ok + }) + }) + .collect(); + handles.into_iter().map(|h| h.join().expect("worker thread joined")).sum() + }); + + let loaded = reg.load(); + assert!( + loaded.is_ok(), + "concurrent flat appends must leave a parseable store; the writes corrupted it: {:?}", + loaded.err(), + ); + assert_eq!( + loaded.expect("store parses").len(), + total_ok, + "every successful flat append must survive as its own intact line", + ); + + let _ = fs::remove_dir_all(path.parent().expect("temp family dir")); + } + fn member(total_pips: f64, net_trade_rs: Vec) -> SweepPoint { // Every RMetrics field is derived from the R slice by `r_metrics_from_rs` // (the same arithmetic production members carry), then the per-trade