diff --git a/src/core/tracking/origin_ledger.cpp b/src/core/tracking/origin_ledger.cpp index 5e7067d..dbcafd4 100644 --- a/src/core/tracking/origin_ledger.cpp +++ b/src/core/tracking/origin_ledger.cpp @@ -141,6 +141,8 @@ bool parseRecordArray(json::Reader& r, OriginLedger& out) { // `versionOut` stays 0 when no "v" key was present — the legacy path-only shape, or // an empty object. Read regardless of key order, so it is validated after the close. +// A duplicate "v" key (never emitted by serialize()) is last-wins, same as every +// other duplicate key here — not reachable except by hand-editing the blob. bool parseLedger(json::Reader& r, OriginLedger& out, int& versionOut) { if (!r.consume('{')) return false; r.skipWs(); @@ -174,7 +176,14 @@ ParseOutcome parseStored(const std::string& text, OriginLedger& out) { OriginLedger parsed; int version = 0; ::reasampler::json::Reader r(text); - if (!parseLedger(r, parsed, version)) return ParseOutcome::Malformed; + if (!parseLedger(r, parsed, version)) { + // A future record shape (e.g. v3 retyping "kind" or "records") fails these v2 + // parse rules on the very field the version bump changed. "v" is read before + // any record parsing in our own writer's key order, so `version` already holds + // it here: report FutureVersion, not Malformed, or the operator gets the + // corrupt-blob "clear it" advice against a newer build's ledger. + return version > kLedgerVersion ? ParseOutcome::FutureVersion : ParseOutcome::Malformed; + } // Trailing garbage means the blob is not what it claims; accepting it would turn // a detectably-corrupt value into a silently-partial ledger. r.skipWs(); diff --git a/src/shell/actions/prune_action.cpp b/src/shell/actions/prune_action.cpp index f163442..1e9a36b 100644 --- a/src/shell/actions/prune_action.cpp +++ b/src/shell/actions/prune_action.cpp @@ -50,7 +50,9 @@ void doBankPruneFolder(ReaSamplerSession& session) { "ReaSampler than this one, so its records cannot be read safely. It " "has been left intact and will NOT be overwritten. Reopen the project " "with that newer version -- do NOT clear this key from here, that " - "would discard tracking records this build cannot see.\n"; + "would discard tracking records this build cannot see. The block is " + "held for the rest of this session, and until then no new capture is " + "persisted to the ledger either.\n"; } if (!report.unreadableUsageKeys.empty()) { msg += "One or more instance usage records could not be read or decoded. " diff --git a/src/shell/capture/realtime_lifecycle.cpp b/src/shell/capture/realtime_lifecycle.cpp index fed3eb3..ab2c70c 100644 --- a/src/shell/capture/realtime_lifecycle.cpp +++ b/src/shell/capture/realtime_lifecycle.cpp @@ -11,6 +11,7 @@ #define REAPERAPI_MINIMAL #define REAPERAPI_WANT_EnumProjects +#define REAPERAPI_WANT_GetProjectName #define REAPERAPI_WANT_ShowConsoleMsg #include "reaper_plugin_functions.h" @@ -64,21 +65,30 @@ void DriveRealtimeCapture(ReaSamplerSession& session) // folder, so it survives here untracked — deleting a user's just-recorded // audio is the destructive direction and is not this shell's call, so the // path is NAMED instead and the operator decides (docs/TODO.md). - if (r.status == RealtimeTickStatus::Done && r.result.status == CaptureStatus::Ok) + // + // RealtimeTickStatus::Done implies CaptureStatus::Ok (RealtimeRecordBackend:: + // abort only sets Done on that path) — the Ok branch below is therefore the + // whole Done case; anything else is Failed and falls to the last branch. + if (r.status == RealtimeTickStatus::Done && r.result.status == CaptureStatus::Ok) { + // g_rtCaptureProject is still valid here (reset only below) — it names the + // ORIGINAL project, not whatever is active now, which is the whole point: + // the user has already switched away from it. + char nameBuf[512] = {0}; + GetProjectName(g_rtCaptureProject, nameBuf, sizeof(nameBuf)); + const std::string projName = nameBuf[0] ? nameBuf : "(unsaved project)"; ShowConsoleMsg(("ReaSampler realtime capture: project switched mid-record -- " - "captured audio restored into the original project; not " - "persisted to avoid crossing projects. The recorded file was " - "left in the original project's bank folder as '" + + "captured audio restored into the original project (" + + projName + "); not persisted to avoid crossing projects. The " + "recorded file was left in that project's bank folder as '" + r.result.sample.relativePath + - "', untracked -- reopen that project and re-import it, or " - "delete it by hand.\n").c_str()); - else if (r.status == RealtimeTickStatus::Done) - ShowConsoleMsg(("ReaSampler realtime capture: project switched mid-record -- " - "the capture was aborted and produced no usable file: " + - r.result.message + "\n").c_str()); - else + "', untracked. Re-importing it there does NOT adopt this file " + "-- it copies the audio in under a new name -- so after " + "re-importing, delete this untracked original by hand.\n") + .c_str()); + } else { ShowConsoleMsg(("ReaSampler realtime capture: project changed mid-record -- " + r.result.message + "\n").c_str()); + } g_rtCapture.reset(); g_rtCaptureProject = nullptr; return; diff --git a/src/shell/persist/session.cpp b/src/shell/persist/session.cpp index 8babf88..3329051 100644 --- a/src/shell/persist/session.cpp +++ b/src/shell/persist/session.cpp @@ -61,9 +61,11 @@ void ReaSamplerSession::recordCreated(const model::Sample& sample, const tracking::RecordResult result = tracking_.record(rec); // A rejection means a file exists that nothing attributes to us — invisible // otherwise, and exactly the gap this ledger exists to close. AlreadyPresent is - // the normal dedup outcome, not a gap. - if (result == tracking::RecordResult::RejectedEmptyPath || - result == tracking::RecordResult::RejectedAbsolutePath) { + // the normal dedup outcome, not a gap. Written as "anything but the two OK + // outcomes" rather than a rejection whitelist, so a future RecordResult value + // is reported by default instead of silently passing through unrecognized. + if (result != tracking::RecordResult::Recorded && + result != tracking::RecordResult::AlreadyPresent) { ShowConsoleMsg(("ReaSampler: could not record the origin of '" + sample.relativePath + "' -- the path is empty or absolute. The file is NOT tracked and " diff --git a/tests/test_origin_ledger.cpp b/tests/test_origin_ledger.cpp index f14efd1..ff2fb8f 100644 --- a/tests/test_origin_ledger.cpp +++ b/tests/test_origin_ledger.cpp @@ -258,6 +258,17 @@ static void testFutureVersionIsNeitherLoadedNorMalformed() { CHECK(loadLedger("{\"v\":2,\"records\":[]}").status == LedgerStatus::Loaded); CHECK(loadLedger("{\"v\":1,\"owned\":[\"a.wav\"]}").status == LedgerStatus::Loaded); CHECK(loadLedger("{\"v\":-1,\"records\":[]}").status == LedgerStatus::Unreadable); + + // A future version whose record shape actually changed — not just grew — fails + // parseRecord under these v2 rules ("kind" retyped from int to string). That must + // still classify as FutureVersion, not Unreadable: this is the exact "changed + // record shape" case the version ladder comment says v3 means, and Unreadable + // would steer the operator into clearing a newer build's ledger. + CHECK(loadLedger("{\"v\":3,\"records\":[{\"path\":\"a.wav\",\"kind\":\"loud\"}]}") + .status == LedgerStatus::FutureVersion); + // Same, with the array itself retyped to an object. + CHECK(loadLedger("{\"v\":3,\"records\":{\"path\":\"a.wav\"}}").status == + LedgerStatus::FutureVersion); } // A hand-edited or corrupt blob cannot smuggle an absolute or duplicate path past @@ -315,8 +326,11 @@ static void testLegacyPathOnlyManifestLiftsIn() { // A recapture regenerates the audio behind ONE bank id under a NEW file name // (makeUniqueTag guarantees it), so the ledger legitimately ends up holding two -// records with the same sampleId and different paths. The original's lineage survives -// untouched — the Sample was rewritten in place, the birth record was not. +// records with the same sampleId and different paths. This proves that shape and +// that take-1's record is untouched by take-2's insert (record() never overwrites — +// see testLineageIsNeverBackfilled for the same-path case). It does NOT exercise the +// ledger-wins-over-the-mutable-Sample claim: nothing here reads Sample.provenance, and +// no production code reads OriginRecord.parentSampleId today. static void testRecaptureAddsSecondRecordUnderTheSameSampleId() { OriginLedger l; l.record(rec("bank/take-1.wav", OriginKind::Capture, "S-1", "S-parent")); @@ -326,8 +340,6 @@ static void testRecaptureAddsSecondRecordUnderTheSameSampleId() { CHECK(l.size() == 2); CHECK(l.find("bank/take-1.wav")->sampleId == "S-1"); CHECK(l.find("bank/take-2.wav")->sampleId == "S-1"); - // The ledger, not the (mutable) Sample, is authoritative for lineage: the - // recapture's differing parent did not rewrite the original's. CHECK(l.find("bank/take-1.wav")->parentSampleId == "S-parent"); CHECK(l.find("bank/take-1.wav")->kind == OriginKind::Capture); // Both stay owned, so the superseded file is reclaimable rather than foreign. diff --git a/tests/test_tracking_authority.cpp b/tests/test_tracking_authority.cpp index b700fbb..05d53be 100644 --- a/tests/test_tracking_authority.cpp +++ b/tests/test_tracking_authority.cpp @@ -213,12 +213,16 @@ static void testUnreadableLedgerBlocksAndYieldsNoOrphans() { // Both blockers at once must both be reported — the operator needs to fix both. static void testBothBlockersReported() { - const OriginLedger empty; + // Deliberately NON-empty, like testUnreadableLedgerBlocksAndYieldsNoOrphans: an + // empty fixture asserts nothing about ownedPaths being withheld, only that it + // started empty. + OriginLedger populated; + populated.record(originOf("bank/would-be-orphan.wav", OriginKind::Capture, "S-1")); const UsageFoldResult fold = foldLive( {usage("rsusage_A", "{T1}", {UsageHold{"S-held", "bank/held.wav"}}), unreadable("rsusage_BROKEN")}, {"{T1}"}); - const TrackingState state{LedgerStatus::Unreadable, empty, fold}; + const TrackingState state{LedgerStatus::Unreadable, populated, fold}; const ProtectionAnswer answer = pruneProtection(state); CHECK(answer.blocked); CHECK(answer.ledgerUnreadable); @@ -227,6 +231,12 @@ static void testBothBlockersReported() { // what prune protects, so withholding it would be the unsafe direction. CHECK(contains(answer.heldPaths, "bank/held.wav")); CHECK(answer.ownedPaths.empty()); + + // Belt-and-braces: the orphan set computed from this answer is empty even though + // the file is present, unreferenced, and recorded as owned. + const std::vector orphans = reclaim::pruneOrphans( + {"bank/would-be-orphan.wav"}, {}, answer.ownedPaths); + CHECK(orphans.empty()); } // A record that exists but whose track hosts no identified instance still protects