fix(ingest): convert-on-import to 32f WAV, undo-group bank+assign, overflow guards

Non-WAV sources decode via PCM_source::GetSamples and land as canonical 32f
RIFF/WAVE; hash taken post-conversion so re-imports dedup. Undo block covers
bank mutation + assign_request atomically. Overflow guards + adversarial tests.

src/assignment_request.cpp

@@ -3,6 +3,7 @@
 #include "assignment_request.h"
 
 #include <cstddef>
+#include <limits>
 
 namespace reasampler {
 
@@ -29,27 +30,44 @@ public:
     bool atEnd() const { return pos_ >= s_.size(); }
 
     // Reads one length-prefixed field into `out`. Fails on a missing ':', an empty or
-    // non-numeric length, or a length that runs past the end.
+    // non-numeric length, a length that overflows SIZE_MAX, or a length that runs past
+    // the end. The digit count is capped at 20 (the decimal width of SIZE_MAX on a
+    // 64-bit host) so a crafted 200-digit length cannot accumulate past SIZE_MAX via
+    // repeated multiply. "never UB" promise from the header is upheld here.
     bool field(std::string& out) {
         if (!ok_) return false;
         const std::size_t colon = s_.find(':', pos_);
         if (colon == std::string::npos) return fail();
         if (colon == pos_) return fail();  // empty length token
+        // Cap: SIZE_MAX fits in at most 20 decimal digits; a longer run is bogus.
+        if (colon - pos_ > 20u) return fail();
         std::size_t len = 0;
         for (std::size_t i = pos_; i < colon; ++i) {
             const char c = s_[i];
             if (c < '0' || c > '9') return fail();
-            len = len * 10 + static_cast<std::size_t>(c - '0');
+            const std::size_t digit = static_cast<std::size_t>(c - '0');
+            // Overflow guard: if len would exceed SIZE_MAX after multiply+add, fail.
+            if (len > (std::numeric_limits<std::size_t>::max() - digit) / 10u)
+                return fail();
+            len = len * 10u + digit;
         }
         const std::size_t start = colon + 1;
-        if (start + len > s_.size()) return fail();
+        // Guard: start may equal s_.size() (empty remainder), in which case only len==0
+        // is valid; start > s_.size() cannot happen (colon < s_.size() by find()).
+        // Use subtraction-first form to avoid start+len wrapping on a huge len.
+        if (start > s_.size() || len > s_.size() - start) return fail();
         out.assign(s_, start, len);
         pos_ = start + len;
         return true;
     }
 
     // Reads a length-prefixed field and parses it as a signed 64-bit decimal (an
-    // optional leading '-'). Fails on empty, non-digit, or trailing bytes.
+    // optional leading '-'). Fails on empty, non-digit, trailing bytes, or a value
+    // that would overflow INT64_MAX / underflow INT64_MIN. The digit count is capped
+    // at 19 (the decimal width of INT64_MAX, plus 1 for the optional sign = 20
+    // characters maximum) so a crafted 21-digit field cannot accumulate UB. "never UB"
+    // promise from the header is upheld: all arithmetic is done on positive digits
+    // and capped before applying the sign.
     bool fieldInt64(std::int64_t& out) {
         std::string f;
         if (!field(f)) return false;
@@ -61,11 +79,20 @@ public:
             i = 1;
             if (f.size() == 1) return fail();  // bare "-"
         }
+        // Cap at 19 digits (INT64_MAX = 9223372036854775807 — 19 digits). A 20-digit
+        // positive value would overflow INT64_MAX; a 20-digit negative might be valid
+        // (INT64_MIN = -9223372036854775808) but we conservatively reject it too: the
+        // generation field is a unix timestamp, never near INT64 limits in practice.
+        if (f.size() - i > 19u) return fail();
         std::int64_t v = 0;
         for (; i < f.size(); ++i) {
             const char c = f[i];
             if (c < '0' || c > '9') return fail();
-            v = v * 10 + static_cast<std::int64_t>(c - '0');
+            const std::int64_t digit = static_cast<std::int64_t>(c - '0');
+            // Overflow guard: v * 10 + digit must not exceed INT64_MAX.
+            if (v > (std::numeric_limits<std::int64_t>::max() - digit) / 10)
+                return fail();
+            v = v * 10 + digit;
         }
         out = neg ? -v : v;
         return true;

src/assignment_request.h

@@ -75,6 +75,15 @@ std::string encodeAssignmentRequest(const AssignmentRequest& req);
 // malformed / truncated / trailing-garbage input (never UB, never a partial value) —
 // the reader shell treats absence/malformed as "no pending request." Round-trips:
 // decodeAssignmentRequest(encodeAssignmentRequest(x)) == x.
+//
+// READER REQUIREMENT (instrument-side, S8 follow-up dispatch): after successfully
+// decoding a request, the reader MUST verify that (bankId, sampleId) resolves to an
+// existing sample before acting on it. An undo on the extension side rolls back the
+// `banks` ext-state key (removing the sample) but cannot atomically clear the
+// `assign_request` key if the write happened outside the undo block. Even with the
+// undo-grouping fix (Major 2), the reader must guard against this: treat an
+// unresolvable (bankId, sampleId) pair as a stale/no-op request and discard it
+// silently, never crashing or selecting a nonexistent entry.
 std::optional<AssignmentRequest> decodeAssignmentRequest(const std::string& wire);
 
 } // namespace reasampler

src/ingest.cpp

@@ -8,6 +8,7 @@
 #include "ingest.h"
 
 #include <cstdint>
+#include <cstring>
 #include <ctime>
 #include <filesystem>
 #include <fstream>
@@ -23,6 +24,8 @@
 #include "capture_paths.h"       // deriveBankPaths / projectDirOfRpp / hashWavContent
 #include "persist.h"             // ReaSamplerSession
 
+#include "wav_trim.h"             // parseWavLayout — 32f-float WAV validator for the fast path
+
 #include "reaper_plugin.h"       // reaper_plugin_info_t, gaccel_register_t (full defs)
 
 #define REAPERAPI_MINIMAL
@@ -34,6 +37,8 @@
 #define REAPERAPI_WANT_GetMediaSourceNumChannels
 #define REAPERAPI_WANT_GetMediaSourceSampleRate
 #define REAPERAPI_WANT_GetMediaSourceLength
+#define REAPERAPI_WANT_Undo_BeginBlock2
+#define REAPERAPI_WANT_Undo_EndBlock2
 #include "reaper_plugin_functions.h"
 
 namespace reasampler {
@@ -54,10 +59,10 @@ constexpr const char* kIdImportMediaExplorer = "INGEST_IMPORT_MEDIA_EXPLORER";
 int               g_cmdImportMediaExplorer = 0;
 gaccel_register_t g_accelImportMediaExplorer{};
 
-// Durable store of the composed, channel-qualified command-id + label strings. A vector of
+// Durable store of the composed, channel-qualified command-id + label strings. Two scalar
-// two entries suffices here (one action); pointers into it are handed to REAPER at register
+// std::string globals (one action); their c_str() pointers are handed to REAPER at register
-// and re-presented at unregister, so it must not be resized after registration. Reserved up
+// and re-presented at unregister, so these strings must not be mutated after registration.
-// front so the register-time c_str() stays valid for the unload path.
+// Populated once by ingestRegisterActions; stable for the extension lifetime.
 std::string g_idImportStr;
 std::string g_labelImportStr;
 
@@ -74,8 +79,7 @@ std::string currentProjectDir() {
 }
 
 // Reads a whole file's bytes. Empty vector on any failure (missing / unreadable). Mirror
-// of capture.cpp's readFileBytes — used to hash an import source for dedup + a copied file
+// of capture.cpp's readFileBytes — used to read the source and validate/hash the bank copy.
-// for the Sample's content hash.
 std::vector<std::uint8_t> readFileBytes(const std::string& path) {
     std::ifstream f(path, std::ios::binary | std::ios::ate);
     if (!f) return {};
@@ -88,27 +92,155 @@ std::vector<std::uint8_t> readFileBytes(const std::string& path) {
     return bytes;
 }
 
+// Writes a byte buffer to a file. Returns true on success. The caller is responsible for
+// ensuring the directory exists before calling.
+bool writeFileBytes(const std::string& path, const std::vector<std::uint8_t>& bytes) {
+    std::ofstream f(path, std::ios::binary | std::ios::trunc);
+    if (!f) return false;
+    f.write(reinterpret_cast<const char*>(bytes.data()),
+            static_cast<std::streamsize>(bytes.size()));
+    return f.good();
+}
+
+// Builds a minimal 32-bit-float RIFF/WAVE byte buffer from interleaved double samples.
+// The output is a canonical WAV the bank and wav_trim can read:
+//   RIFF chunk, WAVE form, fmt chunk (tag 3 = WAVE_FORMAT_IEEE_FLOAT, 16-byte body),
+//   data chunk (interleaved little-endian float32, one float per sample per channel).
+// `nch` channels, `rate` Hz sample rate, `frameCount` frames (total samples = frameCount*nch).
+// Each ReaSample (double) is narrowed to float by assignment — the instrument expects
+// 32-bit float; the reduction is intentional and matches how the bank contract is defined
+// (capture.cpp kRenderFormatWavFloat32; wav_trim.h FORMAT ASSUMPTION).
+std::vector<std::uint8_t> buildFloat32Wav(int nch, std::uint32_t rate,
+                                          std::size_t frameCount,
+                                          const std::vector<ReaSample>& interleaved) {
+    const std::size_t sampleCount     = frameCount * static_cast<std::size_t>(nch);
+    const std::size_t dataBytesCount  = sampleCount * 4u;  // 4 bytes per float32
+
+    // The WAV is: RIFF(4)+size(4)+WAVE(4) = 12, fmt (4)+size(4)+16 body = 24, data (4)+size(4)+payload.
+    // Total = 12 + 24 + 8 + dataBytesCount = 44 + dataBytesCount.
+    const std::uint32_t riffSize =
+        static_cast<std::uint32_t>(36u + dataBytesCount);  // 4("WAVE")+24(fmt chunk)+8(data hdr)+data
+
+    std::vector<std::uint8_t> out;
+    out.reserve(44u + dataBytesCount);
+
+    auto putU16 = [&](std::uint16_t v) {
+        out.push_back(static_cast<std::uint8_t>(v & 0xFF));
+        out.push_back(static_cast<std::uint8_t>((v >> 8) & 0xFF));
+    };
+    auto putU32 = [&](std::uint32_t v) {
+        out.push_back(static_cast<std::uint8_t>(v & 0xFF));
+        out.push_back(static_cast<std::uint8_t>((v >> 8) & 0xFF));
+        out.push_back(static_cast<std::uint8_t>((v >> 16) & 0xFF));
+        out.push_back(static_cast<std::uint8_t>((v >> 24) & 0xFF));
+    };
+    auto putTag = [&](const char* t) {
+        for (int i = 0; i < 4; ++i)
+            out.push_back(static_cast<std::uint8_t>(t[i]));
+    };
+    auto putF32 = [&](float f) {
+        std::uint8_t tmp[4];
+        std::memcpy(tmp, &f, 4);
+        for (int i = 0; i < 4; ++i) out.push_back(tmp[i]);
+    };
+
+    // RIFF header
+    putTag("RIFF");
+    putU32(riffSize);
+    putTag("WAVE");
+
+    // fmt chunk (16-byte body, WAVE_FORMAT_IEEE_FLOAT = 0x0003)
+    putTag("fmt ");
+    putU32(16u);                                // chunk body size
+    putU16(0x0003u);                            // WAVE_FORMAT_IEEE_FLOAT
+    putU16(static_cast<std::uint16_t>(nch));
+    putU32(rate);
+    putU32(rate * static_cast<std::uint32_t>(nch) * 4u);  // avgBytesPerSec
+    putU16(static_cast<std::uint16_t>(nch * 4));           // blockAlign
+    putU16(32u);                                            // bitsPerSample
+
+    // data chunk
+    putTag("data");
+    putU32(static_cast<std::uint32_t>(dataBytesCount));
+    for (std::size_t i = 0; i < sampleCount && i < interleaved.size(); ++i)
+        putF32(static_cast<float>(interleaved[i]));
+
+    return out;
+}
+
+// Decodes ALL samples from `src` into interleaved double-precision frames.
+// Returns empty on a zero-length or silent source (sampleRate < 1, channelCount == 0).
+// Uses GetSamples in blocks; advances time_s monotonically. The caller has already
+// queried channelCount and sampleRate from the same source; those values are passed in
+// to avoid re-querying after GetSamples mutates decoder state.
+std::vector<ReaSample> decodePcmSource(PCM_source* src, int nch, double sampleRate,
+                                       double lengthSeconds) {
+    if (!src || nch <= 0 || sampleRate < 1.0 || lengthSeconds <= 0.0) return {};
+
+    const std::size_t totalFrames =
+        static_cast<std::size_t>(lengthSeconds * sampleRate + 0.5);
+    if (totalFrames == 0) return {};
+
+    std::vector<ReaSample> out;
+    out.reserve(totalFrames * static_cast<std::size_t>(nch));
+
+    // Pull samples in blocks of ~4096 frames; loop until source is exhausted.
+    constexpr int kBlockFrames = 4096;
+    std::vector<ReaSample> block(static_cast<std::size_t>(kBlockFrames * nch));
+
+    PCM_source_transfer_t t{};
+    t.samplerate = sampleRate;
+    t.nch        = nch;
+    t.time_s     = 0.0;
+    t.midi_events = nullptr;
+
+    while (true) {
+        t.samples    = block.data();
+        t.length     = kBlockFrames;
+        t.samples_out = 0;
+        src->GetSamples(&t);
+        if (t.samples_out <= 0) break;
+        const std::size_t got = static_cast<std::size_t>(t.samples_out) *
+                                static_cast<std::size_t>(nch);
+        out.insert(out.end(), block.data(), block.data() + got);
+        t.time_s += static_cast<double>(t.samples_out) / sampleRate;
+    }
+    return out;
+}
+
 // The result of an import-into-bank: the sample id to assign (the existing id on a
 // hash-dedup collapse, the new id otherwise) and whether anything was added to the index
 // (so the caller opens an undo point only for a real mutation).
 struct ImportResult {
-    std::string sampleId;   // "" on failure (nothing to assign)
+    std::string sampleId;      // "" on failure (nothing to assign)
-    bool        added = false;  // true iff a NEW index entry was created (not a collapse)
+    bool        added = false; // true iff a NEW index entry was created (not a collapse)
-    std::string message;    // human-readable outcome for the console
+    std::string message;       // human-readable outcome for the console
 };
 
-// Imports one OS-native source file into the ACTIVE bank: copy into the project-relative
+// Imports one OS-native source file into the ACTIVE bank: convert-if-needed to 32-bit-
-// bank folder + index add, hash-dedup applied. Mirrors capture's landing semantics
+// float WAV, write to the project-relative bank folder, index-add, hash-dedup applied.
-// (relative-paths-only, content-hash dedup) for a file the user brought in rather than
-// captured. Populates the Sample's metadata by probing the file via PCM_Source.
 //
-// DEDUP-BEFORE-COPY: hash the SOURCE first and consult the active bank; if the content is
+// BANK CONTRACT: the instrument (wav_trim) expects every bank file to be a canonical
-// already held, assign the existing sample's id and DO NOT copy a redundant file to disk.
+// 32-bit-float WAV (WAVE_FORMAT_IEEE_FLOAT, 32 bits). A verbatim copy of a non-WAV (or
-// Only a genuinely new file is copied + added. This is stricter than capture (which renders
+// an integer-PCM or double-float WAV) would be unplayable. This function therefore:
-// then adds, leaving a collapsed render orphaned) because an import can cheaply check first.
+//   1. Checks whether the source IS already a valid 32f WAV (parseWavLayout fast path).
+//   2. If yes: copies it verbatim — one I/O, content unchanged.
+//   3. If no: decodes via PCM_source::GetSamples and writes a fresh 32f WAV, preserving
+//      the source's channel count and sample rate.
 //
-// NON-DESTRUCTIVE: the source file is never modified or moved — only read + copied.
+// DEDUP ORDERING: the content hash is taken from the CONVERTED (bank-format) bytes AFTER
-// Records the copied file in the owned-file manifest (Phase B B-cap) so Phase R prune can
+// building the file buffer but BEFORE writing to disk. This means:
+//   * Re-importing the same source file yields the same converted bytes → same hash →
+//     dedup fires → no redundant disk write (matching the DEDUP-BEFORE-DISK design).
+//   * An imported WAV whose audio-content hash matches a captured WAV also deduplicates
+//     correctly (hashWavContent is chunk-aware for both).
+//   * The pre-conversion hash shortcut (hash the raw source bytes) is not used: a non-WAV
+//     source's bytes would produce a different hash from the converted WAV bytes, so two
+//     imports of the same mp3 would NOT dedup — which is wrong. Hashing post-conversion
+//     is correct.
+//
+// NON-DESTRUCTIVE: the source file is never modified or moved — only read.
+// Records the written file in the owned-file manifest (Phase B B-cap) so Phase R prune can
 // attribute it. Does NOT persist or open an undo point — the caller batches that (a
 // multi-file drop is one undo point, one persist).
 ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) {
@@ -132,23 +264,83 @@ ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) {
         return out;
     }
 
-    // Hash the SOURCE content up front (WAV-aware, same hasher captures use so an imported
+    // Read source bytes; needed to check whether it is already a 32f WAV.
-    // WAV dedups against a captured one byte-for-byte on audio content). An unreadable
-    // source yields an empty hash — treated as "not dedupable" (the safe direction: it
-    // will be copied + added rather than silently collapsed onto an unrelated entry).
     const std::vector<std::uint8_t> srcBytes = readFileBytes(absoluteSourcePath);
     if (srcBytes.empty()) {
         out.message = "file is empty or unreadable: " + absoluteSourcePath;
         return out;
     }
-    const std::string contentHash = hashWavContent(srcBytes);
+
+    // Probe the source's audio geometry via PCM_source. Needed for conversion AND for
+    // populating the Sample's metadata. A file REAPER cannot open leaves geometry at
+    // zero — the sample still imports if the WAV-fast-path succeeds; the geometry
+    // is simply unknown, the honest default.
+    int    channelCount  = 0;
+    int    sampleRate    = 0;
+    double lengthSeconds = 0.0;
+    PCM_source* srcHandle = PCM_Source_CreateFromFile(absoluteSourcePath.c_str());
+    if (srcHandle) {
+        channelCount  = GetMediaSourceNumChannels(srcHandle);
+        sampleRate    = GetMediaSourceSampleRate(srcHandle);
+        bool isQN = false;
+        lengthSeconds = GetMediaSourceLength(srcHandle, &isQN);
+        if (isQN) lengthSeconds = 0.0;  // QN-length source has no seconds length to store
+    }
+
+    // Determine whether a verbatim copy suffices (fast path) or a conversion is needed.
+    // parseWavLayout validates that the source is a canonical 32-bit-float RIFF/WAVE; any
+    // other format (mp3, aiff, integer PCM, 16-bit WAV, etc.) takes the decode+rewrite path.
+    const WavLayout layout = parseWavLayout(srcBytes);
+    const bool isFloat32Wav = layout.valid;
+
+    // Build the bank-format bytes in memory (the "converted" bytes), which we hash for dedup
+    // BEFORE writing to disk so a re-import of the same source skips the disk write.
+    std::vector<std::uint8_t> bankBytes;
+    if (isFloat32Wav) {
+        // Fast path: already canonical — bank bytes ARE the source bytes.
+        bankBytes = srcBytes;
+        if (srcHandle) PCM_Source_Destroy(srcHandle);
+    } else {
+        // Conversion path: decode all samples then write a fresh 32f WAV.
+        // PCM_source is opened on the source path (not a copy); we already have srcHandle.
+        std::vector<ReaSample> decoded;
+        if (srcHandle && channelCount > 0 && sampleRate > 0 && lengthSeconds > 0.0) {
+            decoded = decodePcmSource(srcHandle, channelCount,
+                                      static_cast<double>(sampleRate), lengthSeconds);
+        }
+        if (srcHandle) PCM_Source_Destroy(srcHandle);
+
+        if (decoded.empty()) {
+            // No decodable audio. The source is on disk (valid path, REAPER could open it)
+            // but yielded no samples — e.g. a MIDI file, a zero-length audio file, or a
+            // format REAPER does not support. Fail loudly: we must not write a silent WAV
+            // and pretend the import succeeded.
+            out.message = "could not decode audio samples from: " +
+                          fs::path(absoluteSourcePath).filename().string() +
+                          " (unsupported format or no audio data)";
+            return out;
+        }
+
+        const std::size_t frameCount =
+            decoded.size() / static_cast<std::size_t>(channelCount > 0 ? channelCount : 1);
+        bankBytes = buildFloat32Wav(channelCount,
+                                   static_cast<std::uint32_t>(sampleRate),
+                                   frameCount, decoded);
+    }
+    // srcHandle is destroyed above in both branches.
+
+    // Hash the converted (bank-format) bytes for dedup. WAV-aware hash (hashWavContent)
+    // so a re-import of the same source deduplicates against a previously-captured or
+    // previously-imported sample with identical audio content, even if non-audio RIFF
+    // chunks differ. Empty hash (unhashable) is treated as "not dedupable" (safe direction:
+    // copies + adds rather than silently collapsing onto an unrelated entry).
+    const std::string contentHash = hashWavContent(bankBytes);
 
     BankBook& book = g_session->book();
-    const std::string activeBankId = book.activeBankId();
 
-    // Dedup-before-copy: if the active bank already holds this content, assign the existing
+    // Dedup-before-disk: if the active bank already holds this audio content, assign the
-    // id and skip the copy (no redundant on-disk duplicate). Empty hashes never match
+    // existing sample's id and skip the disk write (no redundant on-disk duplicate).
-    // (findByHash treats "" as non-participating), so an unhashable file always imports fresh.
+    // Empty hashes never match (findByHash treats "" as non-participating).
     if (!contentHash.empty()) {
         if (const Sample* existing = book.activeIndex().findByHash(contentHash)) {
             out.sampleId = existing->id;
@@ -158,46 +350,25 @@ ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) {
         }
     }
 
-    // Derive the destination path (project-relative index value + absolute copy target). The
+    // Derive the destination path. The stem comes from the source file name; a timestamp
-    // stem comes from the source file name; a timestamp uniqueTag avoids collision with a
+    // uniqueTag avoids collision with a prior import of a same-named file.
-    // prior import of a same-named file. (fs::path::stem strips the extension and any dir.)
     const std::string sourceStem = fs::path(absoluteSourcePath).stem().string();
     const std::int64_t nowSec = static_cast<std::int64_t>(std::time(nullptr));
     const std::string uniqueTag = std::to_string(nowSec);
     const BankPaths paths = deriveBankPaths(projectDir, sourceStem, uniqueTag);
 
-    // Ensure the bank folder exists, then copy the source in (never move — the source is the
+    // Ensure the bank folder exists, then write the (converted) bank bytes.
-    // user's file and must be left intact; non-destructive). copy_file with overwrite off:
+    fs::create_directories(paths.absoluteDir, ec);  // idempotent; ec ignored (write reports)
-    // the uniqueTag makes a collision astronomically unlikely, and if one occurs we fail
-    // loudly rather than clobber.
-    fs::create_directories(paths.absoluteDir, ec);  // idempotent; ec ignored (copy reports)
     const std::string destPath = paths.absoluteDir + "/" + paths.fileName;
-    fs::copy_file(absoluteSourcePath, destPath, fs::copy_options::none, ec);
+    if (!writeFileBytes(destPath, bankBytes)) {
-    if (ec) {
+        out.message = "could not write converted file to the bank folder";
-        out.message = "could not copy into the bank folder (" + ec.message() + ")";
         return out;
     }
 
-    // Probe the copied file's audio geometry via PCM_Source (channels / rate / length) so
-    // the Sample carries real metadata the panel + instrument read. A file REAPER cannot
-    // open leaves these at their zero-values — the sample still imports (the file is on
-    // disk, the hash is set); the geometry is simply unknown, the honest default.
-    int    channelCount = 0;
-    int    sampleRate   = 0;
-    double lengthSeconds = 0.0;
-    if (PCM_source* src = PCM_Source_CreateFromFile(destPath.c_str())) {
-        channelCount  = GetMediaSourceNumChannels(src);
-        sampleRate    = GetMediaSourceSampleRate(src);
-        bool isQN = false;
-        lengthSeconds = GetMediaSourceLength(src, &isQN);
-        if (isQN) lengthSeconds = 0.0;  // a QN-length source has no seconds length to store
-        PCM_Source_Destroy(src);
-    }
-
     // Build the Sample. Import is NOT a capture — sourceMode/range/tail do not apply; we
     // record what we know (path, hash, geometry, name) and leave capture-only fields at
-    // their defaults. rootNote/loop stay empty (D-B): an imported file is not a single
+    // their defaults. rootNote/loop stay empty: an imported file is not a single played
-    // played note, so we do not guess a root note.
+    // note, so we do not guess a root note.
     Sample s;
     s.id            = "imp-" + uniqueTag + "-" + paths.fileName;
     s.displayName   = sourceStem.empty() ? std::string("import") : sourceStem;
@@ -210,21 +381,22 @@ ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) {
     s.createdTimestamp = nowSec;
 
     const AddResult r = book.activeIndex().add(s);
-    // Record the copied file as owned regardless of the add outcome — the tool WROTE it, so
+    // Record the written file as owned regardless of the add outcome — the tool WROTE it, so
     // Phase R prune must attribute it. (A Collapsed result here would mean another sample in
-    // the active bank matched the hash; we pre-checked findByHash above, so in practice
+    // the active bank matched the hash after we passed the pre-write dedup check — a narrow
-    // Added is the outcome. But record + handle both honestly.)
+    // race window. Record + handle both honestly.)
     g_session->owned().add(paths.relativePath);
 
     switch (r) {
         case AddResult::Added:
             out.sampleId = s.id;
             out.added    = true;
-            out.message  = "imported -> " + paths.relativePath;
+            out.message  = (isFloat32Wav ? "imported -> " : "converted + imported -> ") +
+                           paths.relativePath;
             break;
         case AddResult::Collapsed: {
-            // The hash matched an existing entry (a race against our pre-check, or an empty-
+            // The hash matched an existing entry (a race against our pre-write dedup check,
-            // hash edge). Assign the existing entry's id, not the rejected new one.
+            // or an empty-hash edge). Assign the existing entry's id.
             const Sample* existing =
                 contentHash.empty() ? nullptr : book.activeIndex().findByHash(contentHash);
             out.sampleId = existing ? existing->id : std::string{};
@@ -278,10 +450,27 @@ void doImportFromMediaExplorer() {
         return;
     }
 
-    // Persist the bank add as one undo point ONLY when the index actually mutated (a
+    // Persist the bank add AND the assign request inside ONE undo block so Ctrl-Z rolls
-    // dedup collapse changed nothing on the index). Then assign + refresh the panel.
+    // back both keys atomically: undo restores `banks` (removing the new sample) AND
-    if (r.added) persistBankOp("ReaSampler: import from Media Explorer");
+    // clears the `assign_request` that named it, so no stale request can survive.
-    ingestAssignActiveInstance(g_session->book().activeBankId(), r.sampleId);
+    // The block is opened only when the index mutated (a dedup collapse changed nothing).
+    // If saveToActiveProject() no-ops (unsaved project), we close with an empty label +
+    // zero flag so REAPER discards the undo entry (the house pattern from actions.cpp).
+    if (r.added) {
+        Undo_BeginBlock2(nullptr);
+        const bool persisted = g_session->saveToActiveProject();
+        // Assign request inside the same block: undo rolls back both keys together.
+        ingestAssignActiveInstance(g_session->book().activeBankId(), r.sampleId);
+        if (persisted)
+            Undo_EndBlock2(nullptr, "ReaSampler: import from Media Explorer",
+                           UNDO_STATE_MISCCFG);
+        else
+            Undo_EndBlock2(nullptr, "", 0);
+    } else {
+        // Dedup collapse: index unchanged, no undo point. Assign request still written
+        // (the user explicitly re-imported; they want the instance updated).
+        ingestAssignActiveInstance(g_session->book().activeBankId(), r.sampleId);
+    }
     bankPanelRefresh();
     ShowConsoleMsg(("ReaSampler ingest: " + r.message + " (assigned to the active "
                     "instance).\n").c_str());
@@ -333,12 +522,28 @@ void ingestDroppedFiles(const std::vector<std::string>& absolutePaths) {
     }
 
     // One undo point for the whole drop, opened only if a NEW index entry was created (a
-    // drop that only re-hit existing content mutated nothing on the index). persistBankOp
+    // drop that only re-hit existing content mutated nothing on the index). The assign
-    // itself no-ops the block on an unsaved project.
+    // request is written INSIDE the same block so Ctrl-Z rolls back both keys together:
-    if (importedNew > 0) persistBankOp("ReaSampler: import dropped file(s)");
+    // undo restores `banks` (removing the new samples) AND clears the `assign_request` that
-
+    // named one of them, so no stale request survives pointing to a removed sample.
+    // If saveToActiveProject() no-ops (unsaved project), we close with an empty label + zero
+    // flag so REAPER discards the undo entry (house pattern from actions.cpp).
     if (!firstAssignId.empty()) {
-        ingestAssignActiveInstance(firstAssignBank, firstAssignId);
+        if (importedNew > 0) {
+            Undo_BeginBlock2(nullptr);
+            const bool persisted = g_session->saveToActiveProject();
+            // Assign inside the block: undo restores both keys atomically.
+            ingestAssignActiveInstance(firstAssignBank, firstAssignId);
+            if (persisted)
+                Undo_EndBlock2(nullptr, "ReaSampler: import dropped file(s)",
+                               UNDO_STATE_MISCCFG);
+            else
+                Undo_EndBlock2(nullptr, "", 0);
+        } else {
+            // All dropped files deduplicated: index unchanged, no undo point needed. Still
+            // assign so the user sees the sample is already in the bank.
+            ingestAssignActiveInstance(firstAssignBank, firstAssignId);
+        }
         bankPanelRefresh();
         const std::string msg =
             "ReaSampler ingest: imported " + std::to_string(importedTotal) +

src/main.cpp

@@ -867,15 +867,34 @@ static std::string RunCapture(const reasampler::CaptureActionDef& def)
 // inserts a timeline item (load-bearing principle); the only addition here is the
 // bank-index-id -> assignment-request write after the sample lands. If the capture
 // failed / no-op'd (empty id), no assignment is written (nothing to assign).
+//
+// UNDO GROUPING: both the bank mutation (RunCapture -> saveToActiveProject) AND the
+// assignment-request write (ingestAssignActiveInstance -> writeAssignmentRequest) are
+// wrapped in a single undo block so Ctrl-Z rolls back both ext-state keys atomically.
+// An undo that removes the captured sample also clears the assign_request that named it,
+// preventing a stale request from pointing at a removed sample. The block uses the house
+// pattern (UNDO_STATE_MISCCFG, discarded on an unsaved project with empty label + zero
+// flag) matching the bank-op family in actions.cpp.
 static void RunCaptureItemAssign()
 {
     // Reuse the Item-scope def from the capture table (index 0) — same range logic, same
     // FX-scope neutralize, same bank/persist landing as the plain "capture item" action.
+    Undo_BeginBlock2(nullptr);
+
     const std::string sampleId =
         RunCapture(reasampler::captureActionTable()[0]);
-    if (sampleId.empty()) return;  // capture failed / no-op — RunCapture already reported
+    if (sampleId.empty())
+    {
+        // Capture failed or no-op'd — RunCapture already reported. Discard the empty point.
+        Undo_EndBlock2(nullptr, "", 0);
+        return;
+    }
 
+    // Assign inside the same block so undo clears both keys together.
     reasampler::ingestAssignActiveInstance(g_session.book().activeBankId(), sampleId);
+    Undo_EndBlock2(nullptr, "ReaSampler: capture + assign to active instance",
+                   UNDO_STATE_MISCCFG);
+
     reasampler::bankPanelRefresh();
     ShowConsoleMsg("ReaSampler ingest: captured into the bank and assigned to the active "
                    "instance.\n");

tests/test_assignment_request.cpp

@@ -131,6 +131,43 @@ static void testTrailingGarbageRejected() {
     CHECK(!decodeAssignmentRequest(wire + "0:").has_value());
 }
 
+// --- overflow / adversarial integer inputs ------------------------------------
+
+// A 21-digit length field overflows SIZE_MAX and must be rejected safely (no UB,
+// no wrap-around that could make a huge length appear small and pass the bounds check).
+static void testOverflowFieldLength() {
+    // Craft a wire where the bankId length token is 21 digits that exceed SIZE_MAX.
+    // The decoder must fail cleanly, not access memory out of bounds.
+    // "rsassign1" + "999999999999999999999:" (21 nines) + junk: rejects before OOB.
+    const std::string wire = std::string("rsassign1") + "999999999999999999999:junk";
+    CHECK(!decodeAssignmentRequest(wire).has_value());
+}
+
+// A 20-digit generation (exceeds the 19-digit cap) must be rejected safely.
+static void testOverflowFieldInt64() {
+    // Encode a valid record then manually substitute the generation with a 20-digit value.
+    // We cannot use encode (it would produce a correct 19-digit generation), so we
+    // build the wire manually. Generation "99999999999999999999" (20 nines) exceeds cap.
+    // bankId = "pool" (4 bytes), sampleId = "s1" (2 bytes).
+    const std::string wire = std::string("rsassign1")
+                             + "4:pool"
+                             + "2:s1"
+                             + "20:99999999999999999999";
+    CHECK(!decodeAssignmentRequest(wire).has_value());
+}
+
+// SIZE_MAX as a length field (20 digits, within the digit-count cap) must not UB or
+// wrap. The overflow-guard in field() caps the multiplication; even if the value itself
+// does not trigger the multiply guard (SIZE_MAX accumulates cleanly digit by digit),
+// the subsequent "len > s_.size() - start" bounds check catches it because the actual
+// string is tiny — no OOB access, no wraparound, clean rejection.
+static void testOverflowExactSizeMax() {
+    // 18446744073709551615 = SIZE_MAX on 64-bit. 20 digits: within the digit cap, but the
+    // trailing bounds check rejects it because the wire string is far smaller than SIZE_MAX.
+    const std::string wire = std::string("rsassign1") + "18446744073709551615:X";
+    CHECK(!decodeAssignmentRequest(wire).has_value());
+}
+
 int main() {
     testRoundTrip();
     testRoundTripPoolAndZeroGeneration();
@@ -139,6 +176,9 @@ int main() {
     testLargeGeneration();
     testMalformedParse();
     testTrailingGarbageRejected();
+    testOverflowFieldLength();
+    testOverflowFieldInt64();
+    testOverflowExactSizeMax();
 
     if (g_fail == 0) std::printf("All tests passed.\n");
     return g_fail ? 1 : 0;