Repository navigation
fix(playback): recover stale pins on identity-less rows with guarded re-pin - #234
Conversation
|
Oracle production-readiness review of
The kill switch, empty-list refusal, identity gate, same-path input filtering, and copied-state clearing are present. The integrated disabled-recovery control is useful, but stronger collaborator coverage is needed. NEEDS_FIX |
…rows Finish the #227 partial: the version-fallback walk now declares its cold-start (unbound) intent, the recovery refuses a session-bound resolve explicitly, and a re-pinned row no longer carries the previous pin's stored URL, headers, durable identity, or probe evidence onto the matched candidate (clearing lifecycle verdict/delivery untouched). Rewrite the integrated walk test to be load-bearing: the primary resolve genuinely terminals before the recovery is consulted, and the recovery-disabled control proves the recovery is what recovers it. Add coverage for the session-bound refusal and the re-pin field hygiene.
Address the PR #234 review remainders: 1. Enforce an exact-candidate single attempt. The recovery no longer calls the general resolver (which iterates siblings and invokes the stale-source fallback, paying extra listings and breaking the promised retry bound). It resolves the re-pinned candidate once through resolveStalePinCandidateOnceV3, which has no sibling iteration, no fallback invocation, and rejects a substituted result id. 2. Validate before side effects. Identity and duration are checked on the probed result before anything is persisted or pinned; a rejected recovery leaves no new sticky selection and publishes no evidence. 3. Use the measured candidate duration. The probe copies the row with its duration zeroed and the fingerprint's duration tier is compared against the probe's own measured runtime; a no-prober or failed probe is a no-match rather than a comparison against the copied stale value. 4. Reserve time for alternatives. The recovery runs inside its own virtualStalePinRecoveryBudget (well under the walk's decision budget) so a slow recovery yields to the healthy alternates the walk still has to try. 5. Preserve session-bound intent. The walk threads the caller's binding intent instead of forcing it false, and the fresh-start path declares unbound at the call site; a session-bound caller reusing the walk keeps refusing. Tests cover the exact-candidate single attempt, substitution rejection, no-prober and probe-failure no-matches, measured-duration accept/reject with side-effect checks, a slow recovery yielding to a healthy alternate, and session-bound refusal through the walk. AI-assisted: implemented by AI coding agents on behalf of the repository owner.
|
Review addressed and pushed ( |
ef379f2 to
9b5a95b
Compare
Production-readiness: NOT READY — do not merge (review of 9b5a95b)Gate chain, single-retry bound, and rejection tests are solid. Four blockers remain, all in the new recovery file. Must-fix1. Recovery can rewrite the durable pin — playback_virtual_stale_pin.go:208 2. Sticky generation allocated too late — playback_virtual_stale_pin.go:207 3. Ambiguous fingerprints pick by rank — playback_virtual_stale_pin.go:332-344 4. 'Exact candidate' check incomplete — playback_virtual_stale_pin.go:259-266 Advisory (not blocking)
|
Recovery for an identity-less stale pin re-pinned in memory but then handed the adoption-capable evidence writer the original row with the replacement URI, which took the path-adoption branch and rewrote the durable ?result= pin it promised never to touch. Persist replacement evidence only against a row that already verifiably owns the matched candidate's concrete path, and then as metadata only; with no existing owner the recovery leaves no durable trace. Allocate the write generation before the listing instead of at publication, so an earlier-started, later-finishing recovery cannot overwrite a newer selection that finished first. Refuse when more than one live candidate identity carries the row's fingerprint rather than rank-picking the first: same-title releases share codec, size and runtime, so size+codec cannot separate language or edition variants. Veto known resolution and audio-language conflicts so those variants are excluded before the uniqueness count. Require a concrete matched candidate from the resolver: reject a provider-neutral URI, a different release under the same neutral key, a candidate id inconsistent with the returned URI, and an owner outside the matched release scope. Add DB-backed tests that read the whole media_files row back and prove the durable pin is byte-identical, plus unit tests for ambiguous listings in both orders, interleaved generation, and the substitution cases.
|
Review addressed and pushed ( |
2 participants
Problem
Closes #227
Cold starts on rows pinned to
?result=IDs that upstream renumbers within minutes fail terminally when the row carries no durable provider identity, so same-release rematch cannot apply — even while the live listing holds the release under a new ID.Approach
Cold-start-only guarded recovery in the version-fallback walk: pin absent from a NON-EMPTY listing + identity-less row + same-content fingerprint match (size±5%, codec, duration±2%) → re-pin in-memory (never the DB pin; old candidate URL/headers/identity/evidence cleared) and retry once. Session-bound path explicitly refuses; empty listings and cross-content adoption preserve terminal. Kill-switch:
SILO_DISABLE_VIRTUAL_STALE_PIN_RECOVERY=1(default on).Validation
-race; changed-lines lint 0 issues; gofmt/vet cleanRisks
Fingerprint re-pin can mismatch on volatile listings (guarded by strict tiers + non-empty-listing requirement + single retry). Verdict/delivery stamps untouched. No API, migration, or config impact.
Checklist
AI Disclosure