Add operation_attempt secondary indexes, expiry sweep, and retention (#828)

[AcoPiper]

Summary

Adds the durable lookups, expiry sweep, and retention prune the operation_attempt ledger needs, all inside the single operation_attempt column family — no new CF name, no Store accessor, no format bump.

Three indexes as prefixed key spaces. A reserved leading tag byte (0xf8 non-terminal, 0xf9 owed-cleanup, 0xfa expires_at) separates each index from the record key space; no valid UTF-8 idempotency key starts that high, so a record key and an index key can never collide. Composite keys use length-prefixed segments, so (host = "ab", target = "c") and (host = "a", target = "bc") encode distinctly. The reserved bytes are documented in one table at the top of the module, and each index has one encoder used by every write, delete, and lookup path. Every entry is written and removed in the same transaction as its row, over one OptimisticTransactionDB transaction with the retry-on-"Resource busy" loop, so an entry can never outlive its record. Nothing can write the row on its own: the generic write API on Table is bounded by UniqueKey and Value, and OperationAttempt implements neither, so put, insert, update_with_transaction and delete_with_transaction do not exist for this table at all and the index-aware operations are the only writers the compiler admits.

• Non-terminal (host, target, instance) — the single-flight guard. upsert refuses a second live attempt for a triple; the instance is part of the key, so a second instance may install alongside the first. live_attempt(host, target, instance) answers the lookup.
• Owed-cleanup (target, host, instance) — over rows whose cleanup_state is Some(..), terminal ones included. attempts_owing_cleanup(target, host, instance) answers it.
• expires_at — over all rows whatever their action, timestamp-encoded so byte order is time order. expired_attempts(instant) returns them earliest deadline first.

sweep_expired(instant) scans the expires_at index and finalizes every overdue non-terminal attempt with outcome = Some(Outcome::Failed), touching no other field. The row leaves the non-terminal index — the single-flight slot is freed — while any owed cleanup_state stays intact and visible in the owed-cleanup index, so a host that never returns leaks neither the slot nor the record of an identity still owed a teardown. Idempotent: a second run finalizes nothing.

prune(bound, instant) takes RetentionBound { max_age, max_terminal_per_triple } — both required, OR'd — and the instant to measure against. It keeps the most recent terminal attempt per (host, target, instance), where "most recent" is the greatest (started_at, idempotency_key) pair, plus every attempt that is still non-terminal or still owes a cleanup; those keep-rules beat the bound unconditionally. Age is measured from started_at, never from expires_at. The boundary at each bound is specified in rustdoc and tested: an attempt exactly at the age cutoff is kept, and a triple holding exactly the count loses none.

Neither operation reads the wall clock — both always take a chrono::DateTime<Utc>, with no Utc::now() fallback.

RetentionBound is re-exported as OperationRetentionBound, and CHANGELOG.md folds the new capabilities into the existing unreleased OperationAttempt entry, since no released version ever saw the record.

Closes #828

Test plan

• Single flight: a second live attempt for (host-a, piglet, Some(1)) is refused while the first is live, and accepted once the first becomes terminal
• A second live attempt racing the first is refused inside the transaction, not only on a serial re-read
• Instance is part of the key: (host-a, piglet, Some(1)) does not block (host-a, piglet, Some(2))
• Two live attempts for the same target on different hosts coexist
• Every attempt expires: an Install attempt whose deadline passed is finalized Failed, leaves the non-terminal index, and keeps its owed cleanup_state visible in the owed-cleanup index
• The sweep leaves a deadline that has not passed alone, and a second run over swept state changes nothing
• An Onboard attempt expires by the same path as an Install attempt
• Owed-cleanup lookup returns exactly the rows whose cleanup_state is Some(..), terminal ones included, and drops a row once its cleanup is cleared
• An Onboard attempt owes its teardown under an empty target
• Key-space separation: a record and an index entry that would collide under naive concatenation are both stored and independently readable; iterating records yields no index entries
• A row leaves the non-terminal index on becoming terminal and the owed-cleanup index on its cleanup_state being cleared
• Deleting an undecodable row takes its index entries with it
• The generic write API cannot reach this record: a compile_fail doctest pins that OperationAttempt is not admitted by it, paired with a passing one over a record that is, so the negative test cannot go vacuous
• Retention — recency is started_at: with started_at order deliberately the reverse of expires_at order, the greatest started_at survives; on a shared started_at, the greater idempotency_key survives
• Retention — keep-rules: one terminal attempt kept per triple, one record each for a module's two instances, and non-terminal or cleanup-owing attempts never pruned
• Retention — bound on each axis and on both: age alone fires; count alone fires; both fire and the union is removed; neither fires and nothing is pruned. Fixtures set expires_at so ordering by it would give a different answer
• Retention — boundary at each bound: an attempt exactly at the age cutoff is kept; a triple holding exactly the count loses none
• Retention — keep-rules beat the bound: under a bound every attempt exceeds, the most recent terminal attempt, a non-terminal attempt, and a terminal-but-cleanup-owing attempt all survive
• Retention — no wall clock, and idempotent: two calls with the same arguments produce the same table and the second removes nothing; advancing only the supplied instant, with real time unchanged, changes which attempts are eligible
• cargo fmt -- --check --config group_imports=StdExternalCrate passes
• cargo clippy --bins --tests --all-features -- -D warnings passes
• cargo test --all-features is green (585 passed)
• markdownlint-cli2 on CHANGELOG.md is clean
• CI green on ubuntu-latest and macOS-latest, plus the coverage job

[codecov]

Codecov Report

❌ Patch coverage is 97.76675% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.14%. Comparing base (8091212) to head (865b9c5).

Files with missing lines	Patch %	Lines
src/tables/operation_attempt.rs	97.76%	18 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #857      +/-   ##
==========================================
+ Coverage   83.83%   84.14%   +0.30%     
==========================================
  Files          92       92              
  Lines       35419    36216     +797     
==========================================
+ Hits        29695    30474     +779     
- Misses       5724     5742      +18     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[AcoPiper]

[Reviewer Round 1]

• [P1] Prevent OperationAttempt from being written through the inherited generic Table APIs. The new module explicitly notes that Table::put and Table::insert write only the row (src/tables/operation_attempt.rs:42), but those methods — as well as update_with_transaction and delete_with_transaction — are still public on Table (src/tables.rs:633). Once the planned Store accessor exposes this table, a caller can use table.insert(&attempt) or table.put(&attempt) and create a live row with no non-terminal or expiry entry; live_attempt will miss it, the sweep will never finalize it, and a raw transactional delete can leave stale entries behind. That violates the issue’s requirement that insert, upsert, finalize, and delete maintain indexes atomically. Please make the index-aware write paths the only usable ones for this record (rather than documenting the bypass) and add a regression test proving callers cannot create an unindexed attempt.

[AcoPiper]

[Review Verdict Round 1: NOT_APPROVED]

[AcoPiper]

[Author Round 1]

• [P1] Prevent OperationAttempt from being written through the inherited generic Table APIs — Fixed. The finding is right, and documenting the bypass was the wrong response to it: impl<R: UniqueKey + Value> Table<'_, R> applied to Table<'_, OperationAttempt>, so once the Store accessor lands a caller could put/insert a live row with no non-terminal and no expires_at entry — invisible to live_attempt, never finalized by the sweep, holding no single-flight slot — or delete_with_transaction a row and strand its entries, which for the non-terminal index is checked as raw presence and would hold its triple's slot for good.

  The fix is to make the record ineligible for that impl rather than to guard each method: OperationAttempt now implements neither UniqueKey nor Value, and reaches its key and serialized value through private inherent record_key / record_value instead. put, insert, update_with_transaction and delete_with_transaction therefore do not exist for this table at all, leaving upsert, delete, sweep_expired and prune as the only writers the compiler admits. UniqueKey is used as a bound in exactly one place in the crate (src/tables.rs:627, that write impl), so dropping it opts out of the generic write API and nothing else; no other record type and no read path is affected, and the type is unreleased, so nothing outside the crate can have depended on the impls.

  Regression test: a compile_fail doctest on OperationAttempt asserting the record is not admitted by the trait the generic writers are bounded by. Because a compile_fail block passes for any compile error, it is paired with a passing doctest that runs the same helper over TorExitNode, which does implement the trait — so a rename or a typo breaks the positive test rather than silently making the negative one vacuous. Verified both directions: substituting TorExitNode into the compile_fail block makes it fail with "Test compiled successfully, but it's marked compile_fail", which is the proof that it fails today for the missing impl and not for something incidental.

  Docs updated with it: the module's key-space section now states the restriction as compiler-enforced rather than as a caveat about which methods to avoid, and the type gained a # Writing section carrying the two doctests. No CHANGELOG.md change — the entry for this record is still unreleased, so a reader of the last release cannot observe the difference.

cargo fmt -- --check --config group_imports=StdExternalCrate and cargo clippy --bins --tests --all-features -- -D warnings are clean, and cargo test --all-features is green: 585 passed, plus 11 doctests and the compile_fail one.

[AcoPiper]

[Reviewer Round 2]

The Round 1 P1 is resolved. OperationAttempt now implements neither UniqueKey nor the table Value trait, so the inherited generic Table write methods are unavailable for this record. The private record encoding helpers are used only by the index-aware upsert, delete, sweep_expired, and prune paths, which keeps record and index changes transactional. The paired doctests make the intended restriction explicit and non-vacuous.

No new findings. The updated diff implements the three required prefixed key spaces, preserves owed cleanup during expiry finalization, and applies the retention floor and OR-bound semantics using the required (started_at, idempotency_key) ordering. The focused tests cover the key behavior that would have regressed otherwise.

[Review Verdict Round 2: APPROVED]

[AcoPiper]

[Review Verdict Round 2: APPROVED]

[AcoPiper]

Suggested squash commit

Title

Add operation attempt indexes, sweep, and prune

Body

The `operation_attempt` ledger is keyed by its idempotency key alone,
which deliberately gives no way to ask whether a live operation
already exists for a host, target, and instance. Four orchestration
guards need that answer, and need it to survive a REview restart: a
double-click followed by a restart would otherwise re-drive two live
attempts for one operation. Three secondary indexes answer it durably
— the non-terminal triple for single flight, the owed-cleanup triple
for teardown still owed, and `expires_at` for deadlines that have
passed.

They live as prefixed key spaces inside the one column family rather
than in column families of their own, so an index entry and its row
are written by a single transaction over a single handle, and nothing
new has to be registered at the later format bump. A reserved leading
byte separates the spaces, and composite keys are length-prefixed, so
neither a record key and an index key nor two different triples can
encode to the same bytes.

The record now implements neither `UniqueKey` nor `Value`, which
removes the generic write API from its table entirely. That makes the
index-aware operations the only writers the compiler admits, so no
caller can store a row the indexes never learn about or drop one and
leave its entries behind.

A host that never returns would otherwise hold its single-flight slot
forever, so the sweep finalizes every overdue non-terminal attempt as
failed. It leaves any owed `cleanup_state` intact and visible, since
clearing one here would orphan a minted bootroot identity that this
crate cannot tear down itself.

Terminal attempts are retained rather than deleted, because after a
self-update the record is the only thing that can still report the
outcome. The prune bounds that growth by age and by count per triple
while keeping the most recent terminal attempt of each, along with
everything unfinished or still owing. Both operations take the instant
to measure against and read no clock of their own, so their results
are determined entirely by their arguments.

Closes #828