fix(replication): back off and report once when a round finds no holder

[jacderida]

Linear issue

V2-1049

Risk tier

• T0 — docs / tooling / CI / pure UX-output. Repo CI only.
• T1 — client-only, no network-facing behavior change. CI + prod compat smoke.
• T2 — node/client logic with behavioral surface, no protocol/format/economics change. Dev testnet + ADR.
• T3 — protocol / storage format / payments / routing. T2 evidence + adversarial testing.

Proposed T2 because the retry cadence for unresolved keys is network-facing: it changes how often this node sends verification requests to its close group. The surface is narrow — it only affects keys that have already failed at least one round, and only lengthens the interval — so I'd suggest the tier's dev-testnet requirement is satisfied by the beta evidence below rather than a fresh run, but that is the reviewer's call.

Compatibility

• Wire: none. No message type, field, or protocol change. Fewer verification requests are sent; each is unchanged.
• Storage: none. unresolved_retries lives on the in-memory VerificationEntry; nothing is persisted.
• API: ReplicationQueues::defer_pending keeps its signature and behaviour. New defer_unresolved, claim_no_holder_report, clear_unresolved and DeferralOutcome are additive. VerificationEntry gains two public fields, which is the breaking part — see below.

Log wording: the two no-holder messages are unified on has no responding holders yet. The network-verification site previously read has no holders yet. Any saved search matching the old string needs updating.

Semver impact

• breaking
• feature
• fix

Changed from feature after review. VerificationEntry is pub, all its fields are pub, and it carries no #[non_exhaustive], so adding unresolved_retries and no_holder_reported breaks any downstream struct literal — the two in-repo integration tests this PR had to fix are the demonstration. Nothing outside this repository is known to construct one, so if the lib target is considered internal a reviewer may reasonably downgrade this to feature; I have marked it by the letter of the rule rather than by the expected blast radius.

Worth a separate decision: #[non_exhaustive] on VerificationEntry would stop this recurring. It is itself breaking, so it belongs with a deliberate bump rather than smuggled in here.

Test evidence

cargo test — 946 lib tests pass, plus all integration and doc tests. cargo fmt --all --check clean. Both clippy configurations clean: --all-targets --all-features -D warnings, and the repo-standard -D clippy::panic -D clippy::unwrap_used -D clippy::expect_used.

Nine new unit tests in replication::scheduling, plus one in replication::config:

• repeated_deferrals_back_off_and_saturate_at_the_cap — the 15/30/60/120/240 sequence, then 64 further attempts pinned at the cap (guards the shift against overflowing into a short delay).
• re_admission_after_eviction_restarts_the_backoff — eviction ends the episode, so the next hint warns again.
• flat_defer_does_not_advance_the_unresolved_backoff — ten capacity-gate deferrals do not consume the first-failure warning. This is the interaction with fix(replication): stop a write-blocked node probing its close group #207 and is the test I would most want reviewed.
• backoff_never_retries_faster_than_the_caller_base — a base above the cap is not shortened.
• defer_unresolved_reports_none_for_unknown_key.
• The existing deferred_pending_key_is_not_ready_until_retry_time is unchanged and still asserts flat behaviour for defer_pending.

Added in the review round:

• a_non_reporting_round_does_not_consume_the_no_holder_warning — an inconclusive quorum advances the count, and the first actual no-holder result still warns at attempt 2.
• finding_a_holder_clears_the_backoff_and_rearms_the_warning — a successful round resets both, so a key held up only by a full fetch queue does not inherit an earlier backoff.
• a_duplicate_hint_does_not_reset_the_backoff_or_the_retry_time — pins the merge-not-replace behaviour of add_pending_verify that the whole fix rests on.
• verification_retry_backoff_max_is_between_the_request_timeout_and_the_entry_lifetime in config.rs, beside the CAPACITY_BLOCKED_RETRY invariant.

Field evidence (V2-1049). Measured on the beta cohort, beta-nodes-*, 24h to 2026-08-21 18:00 UTC:

Lines from this message family, cohort-wide	1,287,248 (98.8% of all WARN)
One node's share	1,153,502
Distinct keys behind it	2,072
Lines per key	557

Measured re-log cadence was 16–25s, matching the flat 15s defer plus a 0.6–10s probe round. Runs between gaps were exactly 30 minutes — PENDING_VERIFY_MAX_AGE — with one sampled key running through fifteen residencies. Replaying is_responsible against that node's reconstructed routing table shows it claimed ~1,451 of those keys at bootstrap and only 98 once the table converged, which matches the observed residual stuck set exactly. None of the 2,072 keys was ever fetched.

Expected effect at that shape: ~10 probes per key per residency instead of ~110, and 1 WARN per key per episode instead of ~110.

New dependency

none

ADR

https://github.com/jacderida/ant-node/blob/fix/unresolved-verification-retry-backoff/docs/adr/ADR-0012-unresolved-verification-retry-backoff.md

ADR-0012 — Back off and report once when a verification round finds no holder, added in this PR. Numbered 0012 because ADR-0011 is taken by capacity-gated source discovery (#207), which this cross-references: that ADR's write-blocked gate is deliberately left on the flat defer_pending path.

Mitigation / rollback

Revert the commit. There is no persisted state, no wire change, and no migration — a node running the previous binary behaves exactly as before. To soften rather than revert, lower VERIFICATION_RETRY_BACKOFF_MAX; setting it to VERIFICATION_REQUEST_TIMEOUT restores today's flat 15s cadence while keeping the log-once behaviour.

Note on scope

This makes the symptom proportionate; it does not fix the cause. The node in V2-1049 was a first start that claimed a slice of the keyspace its routing table could not resolve — the cold-start half of V2-883, which saorsa-core#152 explicitly left out of scope ("speeding up bucket refresh for a brand-new node with no snapshot"). That is tracked separately.

[dirvine]

Reviewed exact head cbf54072083e486aa09802d9088f2c3de5de1163.

The backoff implementation is correct across the lifecycle paths I checked: no-holder retries follow 15/30/60/120/240 seconds and then the 300-second cap; capacity deferrals remain flat and do not consume the unresolved counter; duplicate hints do not reset the timer; fetch failure preserves the entry state; and eviction/re-admission correctly starts a fresh episode. Queue selection excludes deferred entries, so the change reduces repeated traffic without consuming cycle capacity.

Local verification:

• cargo fmt --all --check — passed
• replication scheduling tests — 51/51 passed
• replication module tests — 577/577 passed
• focused PoC tests with test-utils — 10/10 passed
• cargo clippy --all-targets --all-features -- -D warnings — passed

The six-seat review produced no correctness blocker. Five seats completed; one release/API seat timed out. Two reviewers noted the same non-blocking observability edge case: if QuorumInconclusive consumes attempt 1, the first later no-holder result is logged at debug rather than warn, and its attempt count includes the inconclusive round. This follows the shared-counter design documented in ADR-0012 and does not affect retry behaviour or the aggregate no_holders= counter.

Minor documentation/API notes, not blockers:

• ADR-0012 says no_holders= appears only for slow cycles; it is also present in fast-cycle debug summaries. The intended info-level operational visibility is still accurate.
• VerificationEntry is public, so adding its public field can break downstream struct literals; the PR's semver note could state that more explicitly.

All completed GitHub checks are green. The Windows test job is still running at review time; merge should remain gated on it.

Verdict: APPROVE.

[grumbach]

Nice piece of work — the field evidence makes the case on its own, and the 15→300s ladder lands at roughly nine attempts per 30-minute residency, which feels like the right shape. The defer_pending / defer_unresolved split reads well, the shift saturation is handled properly, and leaving refresh_eviction_candidate out of the new path is right given EvictionOrder never looks at next_verify_at.

Two questions before it goes in:

1. QuorumInconclusive (mod.rs:8237) advances the counter via let _ = defer_unresolved(...) without logging. Does that mean a key whose first round is inconclusive then reaches the no-holder branch at attempt == 2, and only ever gets a debug! line — losing the one warning the design most wants to keep? If so, would a separate reported flag on the entry, set where the warn actually fires, be worth it so attempt-counting and reporting stay independent?

2. The fix rests on a re-hint not resetting unresolved_retries — and it doesn't, add_pending_verify merges into the existing entry and leaves both the counter and next_verify_at alone. Is that worth pinning with a test? A later refactor that replaced rather than merged would revert this silently with all six new tests still green.

Two smaller ones, take or leave: unresolved_retries isn't reset on the branches where a holder was found but promote_pending_to_fetch missed on a full queue, so "once per episode" is a little stronger than what actually happens; and VERIFICATION_RETRY_BACKOFF_MAX might want to join the invariant test at config.rs:1878 next to CAPACITY_BLOCKED_RETRY.

[jacderida]

Thanks — all four taken, in 1d1a616.

1. Inconclusive consuming the warning. You're right, and it's worse than a corner case: after a quorum round the key enters PaidForList, so the next cycle takes the local-paid fast path — inconclusive-then-no-holder is the ordinary sequence, not a rare one. Every such key would have lost its only warning.

Took the separate-flag route for the reason you gave: the two facts are independent. unresolved_retries answers "how many consecutive rounds failed" and an inconclusive quorum legitimately advances it; no_holder_reported answers "have we told anyone" and only a no-holder result may consume it. It's claimed through claim_no_holder_report at the two no-holder sites and by nothing else — not the inconclusive path, not the capacity gate.

The cheaper alternative — putting the inconclusive branch on flat defer_pending — I skipped deliberately, since it would drop backoff for exactly the keys generating repeat quorum rounds.

2. Pinning the re-hint. Added a_duplicate_hint_does_not_reset_the_backoff_or_the_retry_time, asserting all three of unresolved_retries, no_holder_reported and next_verify_at survive a second advertiser. Your silent-revert concern was the right instinct — I checked, and every one of the existing tests stays green under a replace-instead-of-merge refactor.

3. Reset on the promote-miss path. Taken rather than left — on inspection it was more than a doc gap. promote_pending_to_fetch returns false on a full fetch queue and leaves the entry intact, so a key that had found a holder kept its accumulated failure count and stayed stretched out at a 240s+ backoff while it was actually making progress. clear_unresolved now fires on both promote branches, clearing the counter and re-arming the warning, which makes "once per episode" literal instead of nearly true.

One related spot you didn't mention, which I've left alone on purpose: InFlightEntry.retry_verification restores the entry verbatim when every fetch source fails. That one I think is correct — the key genuinely is still unresolved — but say if you'd rather it reset too.

4. Config invariant. Added beside CAPACITY_BLOCKED_RETRY: above the request timeout, and at least four capped retries inside one PENDING_VERIFY_MAX_AGE.

Also picked up @dirvine's two: the ADR claimed no_holders= was slow-cycle only when it is in both summaries, and the semver note. On that second one I've gone further than "state it more explicitly" and flipped the PR to breaking — VerificationEntry is pub with pub fields and no #[non_exhaustive], so the two new fields break downstream struct literals, and the two integration tests I had to fix are the proof. Happy to be talked back down to feature if the lib target counts as internal; I marked it by the letter of the rule rather than by expected blast radius. Separately, #[non_exhaustive] would stop this recurring, but it is itself breaking so it wants its own bump rather than riding along here.

950 lib tests pass, fmt and both clippy configurations clean.