alkblobs/tasks/smells-n5-n7.md

id, name, status, depends_on, scope, risk, impact, level, tags

id	name	status	depends_on	scope	risk	impact	level	tags
smells-n5-n7	N-5 + N-7 — put_in_batch's foreign-Batch assert; delete_everywhere's partial-failure reporting	completed		narrow	low	isolated	implementation	wave-9 review-001 correctness smells

Description

Two one-spot neighborhood smells from review-001 (the plan's §Wave 9 keeps them in the correctness round; they are too small for their own sessions but belong somewhere owned):

• N-5 — put_in_batch accepts a foreign Batch (src/store/put.rs:133): no identity check on the batch's registry Arc; cross-store use leaks pins into the other store's in-process registry (release is silently a no-op in the wrong one). The review's close: a debug_assert!(Arc::ptr_eq) on the batch's registry against the store's. A test drives the assert (debug builds run it); if the test round wants the refusal drivable in release too, a named error is the accepted alternative — the chosen point recorded.
• N-7 — delete_everywhere leaves the two homes divergent on partial failure (src/store/sweep.rs:761): kv deleted, large failed ⇒ Err with half the entry gone, unprotected (self-healing by the next sweep's re-mark — the mechanism stays; what's missing is observability). The Err should report which homes moved. The reporting shape is the task's design point (an error payload variant, a wrapped error, or a report field — recorded), chosen to not break the error family's altitude discipline (ADR-012 §6.5: operational errors stay honest without becoming diagnostics dumps). A test drives the partial-failure shape (large-tier failure injected; the kv home's deletion observable in the report).

No other behavior change: the self-healing re-mark posture is reviewed-correct and stays.

Acceptance Criteria

• put_in_batch debug-asserts the Batch's registry identity (or the named-error alternative); the chosen point recorded; a test drives the foreign-Batch case
• delete_everywhere's partial failure reports which homes moved; the shape recorded; a test drives the kv-deleted/ large-failed divergence
• No other behavior change; the error family's altitude discipline held
• Gates green: cargo test (default), cargo test --all-features server-less, clippy --all-targets -- -D warnings, cargo fmt --check

References

• docs/reviews/001-storage-surface-general-review.md (N-5, N-7)
• docs/plans/implementation.md (§Wave 9 — "the neighborhood smells")
• src/store/put.rs (put_in_batch)
• src/store/sweep.rs (delete_everywhere; adjacent to F-1's arbitrated_delete — land after tasks/f1-arbitrate-conclude.md if the diffs would tangle, otherwise order-free)
• docs/architecture/decisions/012-pre-decomposition-consistency-rulings.md §6.5 (the error altitude discipline)

Notes

N-5 — chosen point: debug_assert!(Arc::ptr_eq) (the review's close), not a named error. put_in_batch (src/store/put.rs) opens with the identity check — the batch's registry Arc against the store's (Batch::registry accessor vs GcHome::core accessor, both crate-internal) — before any pin is minted. Rationale recorded in the seam's doc comment: the mis-use is developer-error territory, and the error family is ADR-pinned (ADR-012 §6.5 — new variants are new ADRs), so a named refusal would have been a new ADR for a mis-use a debug tripwire catches in the round that wants it (debug builds run the assert; release builds run on — the assert's window exists to catch the leak, not to gate consumers). The test (store::tests::put_in_batch_refuses_a_foreign_batch_in_debug_builds) drives the foreign-Batch case against two mem_only stores with #[should_panic(expected = "foreign Store")].

N-7 — chosen reporting shape: wrapped Error::Io carrying a one-line homes report, not a payload variant and not a report field. At the large-failure arm the underlying media failure is re-raised as Error::Io(format!("delete partial failure: large home delete failed ({err}); {homes}")), where homes is "kv home deleted — the entry's homes diverged (residue; the next sweep's re-mark reconciles)" or "kv home absent (nothing moved there)" — which homes moved, plus the failure cause, verbatim the ADR-012 §6.5 honesty without a diagnostics dump. The kv-failure short-circuit arm propagates unwrapped (nothing has moved yet — no divergence to report). No variant added; the family stays ADR-pinned. The self-healing re-mark posture is unchanged — the observability is the fix. The test (store::tests::delete_everywhere_partial_failure_reports_which_homes_moved) builds the degenerate-duplicate shape (the same key planted in both homes by hand — the only shape where kv-deleted + large-failed can diverge), drives it through public delete() with a large-home delete-failure injection, and asserts the report names the failed home, the kv move, and the injected cause, plus the real divergence: kv has false, large has true.

Riding change (no behavior change): LocalBackend gained a fail_delete cfg(test) injection knob (mirroring the engine knobs' pattern — inject_fail_delete), to drive the large-tier media failure the filesystem cannot reliably produce in tests.

Summary

Both neighborhood smells closed with their tests. N-5: put_in_batch now debug-asserts the Batch's registry identity (Arc::ptr_eq, accessors Batch::registry/GcHome::core) before any pin is minted — the debug-assert point recorded over the named error (family is ADR-pinned, ADR-012 §6.5); test drives the foreign batch against two stores. N-7: delete_everywhere's large-failure arm wraps the media failure in an Error::Io message that reports which homes moved (kv deleted / kv absent vs large failed) — the wrapped shape recorded over a payload variant or report field; test drives the kv-deleted/large-failed divergence through public delete() and pins both has observables. No other behavior change; the re-mark self-healing posture stays. Gates: cargo test (default) green, cargo test --all-features green against the live pglo-poc harness (the pg columns ran for real), both cargo clippy --all-targets -- -D warnings runs green (default + all-features), cargo fmt --check clean.