alkblobs/tasks/f1-arbitrate-conclude.md

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

id	name	status	depends_on	scope	risk	impact	level	tags
f1-arbitrate-conclude	F-1 — arbitrated_delete concludes the in-flight delete before propagating the SQL-consult error (+ the contended-pin regression tests)	completed		narrow	medium	component	implementation	wave-9 review-001 correctness gc fleet

Description

Review-001 F-1 (docs/reviews/001-storage-surface-general-review.md — located in the review's quoted code; post-split home src/store/sweep.rs:733-753). In arbitrated_delete, after the in-process domain answers Proceed (the delete marker is inserted), the fleet SQL consult's operational error propagates with ? (EngineFleetState::arbitrate(&**host, &key.as_bytes()).await?) — without conclude_delete. The marker stays in RegistryState::deleting forever: every future GcHome::acquire(&key) blocks in its notify loop and every future arbitrate(&key) spins Plan::Wait on a deletion that will never conclude — a transient postgres failure wedges the key permanently, on both the direct-delete path and the sweep executor (execute_overdue_window → arbitrated_delete) under fleet: true. This violates gc.rs's own contract ("the caller must run conclude_delete on every path after its engine delete (success, failure, or absent)") — the delete_everywhere caller two lines below gets it right; the SQL consult is the one path that does not.

The fix: conclude_delete before propagating the SQL-consult error, so every arbitrated_delete arm honors the contract (verify by inspection on all arms after the change).

The regression tests (D-2's method — every fix ships with the tests that drive its previously-uncovered neighborhood):

1. An injected arbitrate failure mid-delete asserting the key stays put-able and delete-able afterwards (no wedged marker). The injection mechanism is this task's design point — a failing pool/ host, a dropped arbitrate table, or a test double at the seam — recorded in Notes. Cover the direct-delete path and (if the injection shape allows it cheaply) the sweep-executor path.
2. The Plan::Wait/protected contended-pin path exercised (gc.rs's wait loop and protected break — never driven by any test): a pin mint blocked during an in-flight delete proceeds against the live entry when arbitration refuses, and waits correctly when the delete concludes.

Live harness

The fleet shapes run against the local dockerized postgres container (pglo-poc, postgres:16-alpine on :15432, running for dev/testing):

export ALKBLOBS_PG_HOST=localhost
export ALKBLOBS_PG_PORT=15432
export ALKBLOBS_PG_USER=postgres
export ALKBLOBS_PG_PASSWORD=poc
export ALKBLOBS_PG_DB=blobs

Server-less, the pg tests skip cleanly; acceptance requires green against the live harness (the AGENTS verification matrix's shape).

Acceptance Criteria

• The SQL-consult error path runs conclude_delete before propagating; every arbitrated_delete arm concludes (verified by reading all arms, not just the fixed one)
• Regression test: an injected arbitrate failure mid-delete — the key remains put-able and delete-able after the failure; the injection mechanism recorded in Notes
• Regression test: the Plan::Wait loop and the protected break exercised (a blocked pin mint during an in-flight delete)
• The previously-uncovered arms (gc.rs's wait loop/protected break, the arbitrate error arm) are covered in the live-harness run
• Gates green: cargo test (default), cargo test --all-features server-less, cargo test --all-features + live harness, clippy --all-targets -- -D warnings across the feature variants, cargo fmt --check

References

• docs/reviews/001-storage-surface-general-review.md (F-1; D-2's uncovered-set correlation — gc.rs:594/607-608/634 in the review's pre-split coordinates)
• docs/plans/implementation.md (§Wave 9; §Review gates — the failure-path gate)
• src/store/sweep.rs (arbitrated_delete, delete_everywhere — the correct-path contrast)
• src/gc.rs (the conclude_delete contract; the Plan::Wait loop and the protected break)
• src/postgres/ (the SQL arbitrate the consult rides)

Notes

• The fix (src/store/sweep.rs arbitrated_delete): the SQL consult's await? became a match on Ok(true) / Ok(false) / Err — the Err arm runs conclude_delete(key) before returning the error. All four arms verified by inspection: Cancelled (no marker taken), consult-refusal Ok(true) (concludes), consult-failure Err (concludes — the fix), engine-delete (concludes before result? propagates). The arbitrated_delete doc comment records the posture.
• The injection mechanism (recorded per the task's design point): the dropped registered liveness table — a fleet-table registration points the SQL arbitrate at an embedder-owned table; dropping it mid-flight makes the consult fold fail with a real engine error (relation … does not exist, SQLSTATE 42P01) — no test double or engine flag needed, and the failure rides the exact production path (pg_err → Error::Io). The test re-creates the table for the delete-able/recovery assertions (same-shape CREATE TABLE; PG re-plans the invalidated prepared statements).
• The contended-pin tests drive the mid-delete window with the consult's own FOR UPDATE finger-lock: an independent session locks the key's gc_pins row ( finger_lock) or the whole table (ACCESS EXCLUSIVE, for the no-pin-row case), holding every delete-time consult with the marker already inserted; releasing the lock is the controlled conclusion point. Every blocking wait is bounded by a 10 s LATCH timeout helper, so a regression fails as a panic, never a hang.
• Five tests added to postgres_fleet_sweep_tests: consult failure on the direct path (put-able + delete-able after the failure), the same through the sweep executor (recovery across the next two sweeps), the refusal-form contended mint (foreign pin row → GcRefuse; blocked mint proceeds against the live entry; the concurrent delete's Plan::Wait re-check composes the mint's protection — the protected break), the conclude-form contended mint (ACCESS EXCLUSIVE hold → the delete really deletes; the blocked re-put lands as a fresh pinned put — ADR-005 case (a)), and a deterministic gc-seam drive of the wait-loop post-release re-check (a failing protect callback composed after the marker released → conservative GcRefuse; arbitrate/conclude_delete called directly at the crate-internal seam).
• One discovery en route: after a fleet re-put's guard drop, the durable pin row lives out its TTL (renewal stops, the row remains), so the "delete-able after the failure" assertion expires the store's own row explicitly (expire_pin_rows — the abandonment shape the sweeper's maintenance step would reap); otherwise the SQL consult correctly refuses the delete.
• The direct-path consult-failure and sweep-executor tests were verified red pre-fix (both latch-fail on the wedged marker) and green post-fix.

Summary

F-1 fixed: arbitrated_delete's fleet SQL-consult error arm now runs conclude_delete before propagating, so every arm past a Proceed arbitration concludes the in-flight delete marker — a transient SQL failure no longer wedges the key for every future pin mint and arbitration on the direct-delete path or the sweep executor. Five regression tests added (consult failure x two paths; the contended Plan::Wait/protected-break neighborhood x three forms), all under the live harness's env gate. Verified: cargo test (default, 127 lib + 54 + 4), cargo test --all-features server-less (skips clean, 275 + 125 + 4), cargo test --all-features against the live pglo-poc harness (275 + 125 + 4, 0 failed), clippy --all-targets -- -D warnings across both feature variants, cargo fmt --check. Scoped live-harness coverage run over the postgres_fleet_sweep_tests module confirms the previously-uncovered arms are now covered: no zero-count execution lines remain in gc.rs's arbitration/wait region (500-645 — the Plan::Wait loop, the notify loop, the protected re-checks), the postgres arbitrate consult including its ROLLBACK error arm, and every arm of arbitrated_delete.