fix(pS-usage): close delete-ward residuals — remint on corrupt, named abort keys, truncation protect-all, abort->protect-all set

src/actions.cpp

@@ -826,10 +826,18 @@ void doBankPruneFolder() {
     // than proceed with degraded protection. Distinct from "no orphans": the user must
     // know the prune refused to run and why.
     if (report.abortedUnreadableUsage) {
-        ShowConsoleMsg(
-            "ReaSampler prune: ABORTED -- an instance usage record could not be read.\n"
-            "Nothing was deleted. Re-opening the project usually clears this (instances "
-            "republish their usage records on load).\n");
+        std::string msg =
+            "ReaSampler prune: ABORTED -- one or more instance usage records could not "
+            "be read or decoded. Nothing was deleted.\n"
+            "If the owning instance is still loaded it will republish its record on the "
+            "next poll tick, clearing the abort. If the instance no longer exists (the "
+            "key is an orphaned corrupt record), clear it manually via ReaScript:\n"
+            "  reaper.SetProjExtState(0, \"reasampler\", \"<key>\", \"\")\n"
+            "Offending key(s):\n";
+        for (const std::string& key : report.offendingUsageKeys) {
+            msg += "  " + key + "\n";
+        }
+        ShowConsoleMsg(msg.c_str());
         return;
     }
 

src/persist.cpp

@@ -317,6 +317,7 @@ struct PruneScan {
     std::vector<std::string> orphans;
     std::unordered_map<std::string, std::uint64_t> sizeByRel;
     bool abortedUnreadableUsage = false;
+    std::vector<std::string> offendingUsageKeys;  // non-empty iff abortedUnreadableUsage
 };
 
 // Non-throwing: readActiveProject + resolveBankFile are pure/string; every filesystem
@@ -381,8 +382,10 @@ PruneScan scanPruneOrphans(const BankBook& book, const OwnedFileManifest& owned)
         // FAIL-SAFE ABORT: a present rsusage_* record could not be read/decoded, so the
         // protected set is unknowable. Compute NO orphans — every downstream consumer
         // (dry-run report, confirm set, fresh-recompute delete plan) then deletes
-        // nothing. The flag surfaces the reason to the action's console message.
+        // nothing. The flag + key names surface the reason so the action can name each
+        // offending key for operator recovery.
         scan.abortedUnreadableUsage = true;
+        scan.offendingUsageKeys = usage.offendingKeys;
         return scan;
     }
     scan.orphans = pruneOrphans(
@@ -402,7 +405,9 @@ PruneReport ReaSamplerSession::pruneDryRun() const {
     // pS-usage fail-safe: surface the unreadable-record abort so the action halts with
     // an explicit message instead of reporting "no orphaned files" (the count IS zero —
     // the scan computed nothing — but the user must know the prune refused to run).
+    // The offending key names propagate so the action can name each one for recovery.
     report.abortedUnreadableUsage = scan.abortedUnreadableUsage;
+    report.offendingUsageKeys = scan.offendingUsageKeys;
     return report;
 }
 

src/prune_reconcile.h

@@ -70,12 +70,20 @@ namespace reasampler {
 //                   must HALT — deleting with degraded protection is the data-loss
 //                   direction. Set by the session's scan shell, never by
 //                   buildPruneReport (which stays a pure tally).
+//   * offendingUsageKeys — the exact "rsusage_<guid>" ext-state key names that
+//                   triggered the abort (non-empty iff abortedUnreadableUsage). Named
+//                   so the action can print them for operator recovery: a corrupt/
+//                   oversized key whose owning instance no longer exists is never
+//                   automatically rewritten, so the abort would be permanent without
+//                   a way to clear it. The operator can clear each key via ReaScript:
+//                     reaper.SetProjExtState(0, "reasampler", "<key>", "")
 struct PruneReport {
     std::size_t              count = 0;
     std::uint64_t            totalBytes = 0;
     std::vector<std::string> orphans;
     bool                     truncated = false;
     bool                     abortedUnreadableUsage = false;
+    std::vector<std::string> offendingUsageKeys;  // non-empty iff abortedUnreadableUsage
 };
 
 // The outcome of an actual prune DELETION (Phase R, Wave 3 — R3). The prune shell fills

src/sample_usage.cpp

@@ -145,11 +145,20 @@ UsagePublishPlan planUsagePublish(const std::optional<std::string>& existing,
     }
     const std::optional<UsageRecord> theirs = decodeUsageRecord(*existing);
     if (!theirs) {
-        // Undecodable existing value under MY OWN key: a sibling sharing this key
-        // (copy) always writes decodable records, so this is corruption. Overwrite
-        // with mine — the self-heal restores correct protection for my holds; the
-        // prune side independently ABORTS while an unreadable record is present
-        // (foldUsageRecords), so the corrupt window can never cause a delete.
+        // Undecodable existing value under MY key: corruption (a sibling sharing
+        // this key via copy always writes decodable records). REMINT rather than
+        // overwrite: writing mine over the corrupt key would clear the prune-side
+        // abort, but a same-key sibling B's holds would then be unprotected until
+        // B publishes again. Leaving the corrupt key in place keeps the prune-side
+        // abort firing (foldUsageRecords.abortPrune) so the window where B's holds
+        // might be unprotected can never resolve toward delete. Mine is published
+        // under the new key that remint produces.
+        // NOTE (>16 MB gap): readReasamplerExtState returning nullopt for a value
+        // larger than 16 MB is indistinguishable from "absent" at the publish site;
+        // that narrow case takes the fresh-write branch above rather than remint.
+        // Both outcomes are safe (fresh write is also correct for a truly absent key);
+        // the gap is documented in the header's fail-safe list.
+        plan.remint = true;
         return plan;
     }
 
@@ -235,9 +244,24 @@ UsageFoldResult foldUsageRecords(
     for (const std::optional<UsageRecord>& rec : decoded) {
         if (!rec) {
             // A present-but-unreadable record: it may protect ANYTHING, so the prune
-            // must halt outright — heldPaths is irrelevant once abortPrune is set (the
-            // caller deletes nothing).
+            // must halt outright. Belt-and-braces: return the PROTECT-ALL set (all
+            // readable records' paths) so the fail-safe holds even under a future
+            // caller that forgets to check abortPrune before using heldPaths. The
+            // abort flag is still the authoritative signal; heldPaths is the
+            // maximum-protection fallback.
             result.abortPrune = true;
+            // Collect EVERY path from EVERY readable record, bypassing the liveness
+            // filter entirely (on abort the protected set is unknowable, so every
+            // decoded hold must be included regardless of track-guid membership).
+            std::unordered_set<std::string> seen;
+            for (const std::optional<UsageRecord>& r : decoded) {
+                if (!r) continue;
+                for (const UsageHold& h : r->holds) {
+                    if (h.relativePath.empty()) continue;
+                    if (seen.insert(h.relativePath).second)
+                        result.heldPaths.push_back(h.relativePath);
+                }
+            }
             return result;
         }
         records.push_back(*rec);

src/sample_usage.h

@@ -36,7 +36,10 @@
 //                           degrade toward delete);
 //   * unreadable record  -> ABORT the prune entirely (foldUsageRecords.abortPrune — a
 //                           record we cannot read may protect anything; halting deletes
-//                           nothing).
+//                           nothing). Residual: readReasamplerExtState returning nullopt
+//                           for a >16 MB value is indistinguishable from "absent" at the
+//                           publish site — that narrow case takes the fresh-write branch
+//                           (not remint), noted here for completeness.
 //
 // -- Liveness (no stale-key false-protect, no false-delete) --------------------
 //
@@ -170,10 +173,15 @@ struct UsagePublishPlan {
 // THIS lifetime's nonce; `mine.unioned` is ignored (the plan computes the written
 // flag). Branches, in order:
 //   * existing absent/empty        -> write mine (unioned=false — sole known writer).
-//   * existing undecodable         -> write mine, unioned=false (this is MY key — a
-//     sibling sharing it via copy always writes decodable records, so an undecodable
-//     value is corruption; overwriting restores correct protection for my holds, and
-//     the prune side independently ABORTS while an unreadable record is present).
+//   * existing undecodable         -> REMINT (mine, unioned=false, under a fresh key)
+//     rather than overwriting the corrupt key: overwriting would clear the prune-side
+//     abort, leaving a same-key sibling's holds unprotected until it republishes.
+//     Leaving the corrupt key in place keeps the prune-side abort (foldUsageRecords)
+//     firing so no delete-ward window opens. The sibling writes its own decodable
+//     record on the next publish tick; the corrupt key is eventually evicted once no
+//     live instance references it. Narrow gap: a >16 MB value reads back as nullopt
+//     (indistinguishable from absent), so it takes the fresh-write branch rather than
+//     remint — both outcomes are safe; the gap is noted in the header's fail-safe list.
 //   * nonce match AND !unioned     -> clean replace (sole writer, provably my content;
 //                                     released holds drop); skipWrite when
 //                                     byte-identical (idle reload tick).

src/usage_scan.cpp

@@ -69,8 +69,9 @@ using FxParmGetter =
 
 // True if any FX in the (possibly container-nested) sub-chain rooted at `fxId` is a
 // ReaSampler 9000. BOTH fx_ident and original_name are checked on BOTH chain kinds (a
-// renamed instance keeps its original_name; fx_ident carries the module path — the
-// review's take-path gap is closed by sharing this one walk). Containers are walked via
+// renamed instance may keep its original_name; fx_ident carries the module path — the
+// primary identification net is the module filename base via fx_ident, which holds even
+// after a user renames the FX instance). Containers are walked via
 // the documented container_count / container_item.X addressing (v7.06+); on a chain
 // kind or REAPER version without containers the parm read returns empty and recursion
 // is a no-op. `depth` bounds pathological nesting. fx_ident is queried per FX — chain
@@ -83,9 +84,17 @@ bool fxSubtreeHasInstance(const FxParmGetter& parm, int fxId,
         identityMatches(parm(fxId, "original_name"), id.uidHexUpper, id.nameUpper,
                         id.outputNameUpper))
         return true;
-    if (depth <= 0) return false;
     const std::string countStr = parm(fxId, "container_count");
-    if (countStr.empty()) return false;  // not a container
+    if (countStr.empty()) return false;  // not a container; no children to miss
+    if (depth <= 0) {
+        // This node IS a container but we have exhausted our descent budget. We cannot
+        // prove that none of its children is a ReaSampler 9000 instance — treat the
+        // incomplete walk as a positive identification (the protect direction). This is
+        // defense-in-depth: kMaxContainerDepth = 32 should prevent reaching this branch
+        // in any real project, but if it IS reached the fail-safe fires rather than
+        // silently missing a live nested instance.
+        return true;
+    }
     const int n = std::atoi(countStr.c_str());
     for (int k = 0; k < n; ++k) {
         const std::string item =
@@ -98,7 +107,10 @@ bool fxSubtreeHasInstance(const FxParmGetter& parm, int fxId,
     return false;
 }
 
-constexpr int kMaxContainerDepth = 8;
+// Raised from 8 to 32 (defense in depth against truncation). Real-world FX containers
+// are typically 2–4 levels deep; 32 is unreachable in practice while remaining finite.
+// Even at 32, the truncation→protect-all guard below is the primary protection.
+constexpr int kMaxContainerDepth = 32;
 
 std::string trackFxParm(MediaTrack* tr, int fxId, const char* parm) {
     char buf[2048] = {0};
@@ -183,6 +195,7 @@ UsageScanResult liveInstanceHeldPaths(void* projOpaque) {
     //    undecodable -> the pure fold ABORTS the prune.
     std::vector<std::string> usageKeys;
     {
+        const std::string prefix = kProjExtUsageKeyPrefix;  // hoisted: one alloc, not N
         char keyBuf[256];
         for (int idx = 0;; ++idx) {
             keyBuf[0] = '\0';
@@ -190,7 +203,6 @@ UsageScanResult liveInstanceHeldPaths(void* projOpaque) {
                                   static_cast<int>(sizeof(keyBuf)), nullptr, 0))
                 break;
             const std::string key(keyBuf);
-            const std::string prefix = kProjExtUsageKeyPrefix;
             if (key.compare(0, prefix.size(), prefix) == 0) usageKeys.push_back(key);
         }
     }
@@ -198,13 +210,17 @@ UsageScanResult liveInstanceHeldPaths(void* projOpaque) {
 
     std::vector<std::optional<UsageRecord>> decoded;
     decoded.reserve(usageKeys.size());
-    for (const std::string& key : usageKeys) {
+    for (std::size_t ki = 0; ki < usageKeys.size(); ++ki) {
+        const std::string& key = usageKeys[ki];
         const std::optional<std::string> value = readExtStateValue(proj, key.c_str());
         if (!value) {
             decoded.push_back(std::nullopt);  // unreadable -> abort (pure fold)
+            result.offendingKeys.push_back(key);
             continue;
         }
-        decoded.push_back(decodeUsageRecord(*value));  // undecodable -> nullopt -> abort
+        const std::optional<UsageRecord> rec = decodeUsageRecord(*value);
+        if (!rec) result.offendingKeys.push_back(key);
+        decoded.push_back(rec);  // undecodable nullopt -> abort
     }
 
     // 2. Enumerate live ReaSampler 9000 hosts. One channel-frozen needle set drives
@@ -250,6 +266,9 @@ UsageScanResult liveInstanceHeldPaths(void* projOpaque) {
     const UsageFoldResult fold = foldUsageRecords(decoded, liveTrackGuids, anyLive);
     result.abortPrune = fold.abortPrune;
     result.heldPaths = fold.heldPaths;
+    // offendingKeys already populated above (unreadable + undecodable entries);
+    // clear it on success so callers see it only when abortPrune is set.
+    if (!result.abortPrune) result.offendingKeys.clear();
     return result;
 }
 

src/usage_scan.h

@@ -37,12 +37,18 @@
 namespace reasampler {
 
 // The scan outcome. When abortPrune is true a present rsusage_* record could not be
-// read or decoded — the caller MUST halt the prune (delete nothing); heldPaths is then
-// meaningless (left empty). Otherwise heldPaths is every project-relative path held by
+// read or decoded — the caller MUST halt the prune (delete nothing). offendingKeys
+// names the exact "rsusage_<guid>" keys that triggered the abort so the action can
+// print them for operator recovery (clear via ReaScript:
+//   reaper.SetProjExtState(0, "reasampler", "<key>", "")
+// for each offending key). heldPaths on abort is the protect-all set (every readable
+// record's paths) — meaningful only as a belt-and-braces fallback; the abort flag is
+// the authoritative signal. Otherwise heldPaths is every project-relative path held by
 // a live ReaSampler 9000 instance, de-duped, in record order — empty in the common
 // no-records case (the FX enumeration is skipped entirely).
 struct UsageScanResult {
     bool abortPrune = false;
+    std::vector<std::string> offendingKeys;  // non-empty iff abortPrune
     std::vector<std::string> heldPaths;
 };
 

tests/test_sample_usage.cpp

@@ -7,15 +7,20 @@
 //
 // Covers: wire round-trip (nonce + unioned flag, empty / adversarial bytes),
 // malformed -> nullopt, the publish plan's branches (fresh / clean replace + skip /
-// sibling union with the sticky poison flag / cross-track re-mint / undecodable heal),
+// sibling union with the sticky poison flag / cross-track re-mint / undecodable remint),
 // the SAME-TRACK SIBLING repro (the review's 🔴#1 — byte-identical wire convergence
 // must never let one sibling clean-replace the other's still-held paths, including one
 // write later via the poison flag), the liveness fold (live, dead-track, empty-guid
 // fallback, de-dup), the ZERO-IDENTIFIED protect-all net (🔴#2 — an identity-matcher
 // failure must protect everything, not nothing), the UNREADABLE-record abort
-// (foldUsageRecords.abortPrune — prune halts, deletes nothing), the pure identity
-// matcher (UID hex / module filename base / display name, beta over-protect), and the
-// composed pruneOrphans exclusion proof.
+// (foldUsageRecords.abortPrune — prune halts, deletes nothing), the UNDECODABLE-EXISTING
+// REMINT (corrupt key left in place — prune-side abort keeps firing while sibling holds
+// unprotected), the ABORT→PROTECT-ALL belt-and-braces (foldUsageRecords.heldPaths is
+// the full protect-all set even when abortPrune is set), the TRUNCATED-WALK→PROTECT-ALL
+// proof (FX walk misses a nested instance → anyLive=false → usageHeldPaths protects
+// every record — the pure side of the depth-cap + depth-exhaustion-is-container fix),
+// the pure identity matcher (UID hex / module filename base / display name, beta
+// over-protect), and the composed pruneOrphans exclusion proof.
 
 #include "../src/sample_usage.h"
 
@@ -254,16 +259,20 @@ static void testPlanOwnRecordAfterTrackMove() {
     CHECK(back->trackGuid == "{T2}");
 }
 
-static void testPlanUndecodableExisting() {
-    // An undecodable existing value under MY key is corruption — overwrite with mine
-    // (the self-heal; the prune side independently aborts while it is unreadable).
+static void testPlanUndecodableExistingRemints() {
+    // An undecodable existing value under MY key must REMINT (not overwrite). Overwriting
+    // would clear the prune-side abort while a same-key sibling B's holds are unprotected
+    // until B republishes. Leaving the corrupt key in place keeps the prune-side abort
+    // (foldUsageRecords.abortPrune) firing so no delete-ward window opens.
     const UsageRecord mine = makeRecord("{T1}", "NA", {UsageHold{"a", "pa.wav"}});
     const UsagePublishPlan plan = planUsagePublish(std::string("corrupt"), mine);
-    CHECK(!plan.remint);
+    CHECK(plan.remint);         // fresh key — leave the corrupt key untouched
     CHECK(!plan.skipWrite);
+    // wire carries mine (to be written under the NEW key by the caller)
     auto back = decodeUsageRecord(plan.wire);
     CHECK(back.has_value());
     CHECK(back->holds.size() == 1);
+    CHECK(!back->unioned);  // fresh key, sole writer — un-poisoned
 }
 
 // --- liveness fold ---------------------------------------------------------------
@@ -370,6 +379,78 @@ static void testUnreadableRecordAbortsPrune() {
     CHECK(net.heldPaths.size() == 1);
 }
 
+// --- abort returns the protect-all set (belt-and-braces) ---------------------------
+// foldUsageRecords must return heldPaths = EVERY readable record's paths when
+// abortPrune is set, so a future caller that forgets to check the flag before using
+// heldPaths still gets maximum protection rather than an empty set (which would be
+// delete-ward).
+static void testAbortFoldReturnsProtectAllSet() {
+    // Two readable records + one unreadable (nullopt) in between.
+    std::vector<std::optional<UsageRecord>> decoded;
+    decoded.push_back(makeRecord("{T1}", "N1", {UsageHold{"a", "pa.wav"}}));
+    decoded.push_back(std::nullopt);  // triggers abort
+    decoded.push_back(makeRecord("{T2}", "N2", {UsageHold{"b", "pb.wav"}}));
+
+    // Live: only {T1} — so without protect-all, T2's path would be excluded.
+    const UsageFoldResult fold =
+        foldUsageRecords(decoded, std::unordered_set<std::string>{"{T1}"}, true);
+    CHECK(fold.abortPrune);
+    // heldPaths must contain BOTH paths (protect-all over all readable records),
+    // not just {T1}'s path.
+    CHECK(fold.heldPaths.size() == 2);
+    bool hasPA = false, hasPB = false;
+    for (const std::string& p : fold.heldPaths) {
+        if (p == "pa.wav") hasPA = true;
+        if (p == "pb.wav") hasPB = true;
+    }
+    CHECK(hasPA);
+    CHECK(hasPB);
+
+    // All nullopt (every key unreadable): abort + empty heldPaths (nothing readable).
+    std::vector<std::optional<UsageRecord>> allNull;
+    allNull.push_back(std::nullopt);
+    const UsageFoldResult allNullFold =
+        foldUsageRecords(allNull, std::unordered_set<std::string>{}, false);
+    CHECK(allNullFold.abortPrune);
+    CHECK(allNullFold.heldPaths.empty());  // no readable records to protect
+}
+
+// --- truncated enumeration → protect-all -------------------------------------------
+// The FX walk may truncate at kMaxContainerDepth, leaving a deeply-nested live instance
+// missed. At the PURE layer this is indistinguishable from a genuine identity-matcher
+// failure: anyInstanceLive stays false while records exist. The protect-all net in
+// usageHeldPaths guarantees this resolves toward PROTECT, never toward delete — the same
+// test shape as testZeroIdentifiedProtectsAll, stated here explicitly for the truncation
+// failure mode.
+static void testTruncatedWalkProtectsAll() {
+    // Records from two tracks that host instances; the FX walk (shell side) failed to
+    // identify ANY instance (e.g. truncated at depth, or a future matcher gap).
+    const std::vector<UsageRecord> records = {
+        makeRecord("{TRACK-A}", "N1", {UsageHold{"x", "nested-a.wav"}}),
+        makeRecord("{TRACK-B}", "N2", {UsageHold{"y", "nested-b.wav"}}),
+    };
+    // Shell reported anyLive=false (it couldn't identify any instance — truncated walk).
+    const std::vector<std::string> paths =
+        usageHeldPaths(records, std::unordered_set<std::string>{}, /*anyInstanceLive=*/false);
+    // Both paths must be protected — the protect-all net fires.
+    CHECK(paths.size() == 2);
+    bool hasA = false, hasB = false;
+    for (const std::string& p : paths) {
+        if (p == "nested-a.wav") hasA = true;
+        if (p == "nested-b.wav") hasB = true;
+    }
+    CHECK(hasA);
+    CHECK(hasB);
+
+    // Belt-and-braces: the same scenario through foldUsageRecords also protects all.
+    std::vector<std::optional<UsageRecord>> decoded;
+    for (const UsageRecord& r : records) decoded.push_back(r);
+    const UsageFoldResult fold =
+        foldUsageRecords(decoded, std::unordered_set<std::string>{}, false);
+    CHECK(!fold.abortPrune);
+    CHECK(fold.heldPaths.size() == 2);
+}
+
 // --- the identity matcher ----------------------------------------------------------
 // The common-case shapes: REAPER's fx_ident carries the .vst3 MODULE PATH (matched by
 // the output-name needle "REASAMPLER_9000" — the display name, space-separated, can
@@ -472,12 +553,14 @@ int main() {
     testPlanEmptyNonceNeverClaimsOwnership();
     testPlanCrossTrackRemint();
     testPlanOwnRecordAfterTrackMove();
-    testPlanUndecodableExisting();
+    testPlanUndecodableExistingRemints();
     testHeldPathsLiveness();
     testZeroIdentifiedProtectsAll();
     testHeldPathsDedupAndEmptyPathSkip();
     testTakeFxAttributedRecordIsProtected();
     testUnreadableRecordAbortsPrune();
+    testAbortFoldReturnsProtectAllSet();
+    testTruncatedWalkProtectsAll();
     testIdentityMatcher();
 
     testInstanceHoldMakesPathUnprunable();