From 7871718848eb7906ae79516280bcb4e3667938e0 Mon Sep 17 00:00:00 2001 From: Raphael Maenle Date: Tue, 9 Jun 2026 14:16:37 +0200 Subject: [PATCH] persistence: v1->v2 upgrade test + honest README MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit README §6 claimed bidirectional forward-compat for journal records. Reality is narrower: - ProcessJobStore (kVersion=2) and SubstrateStore (kVersion=2) accept v1 records on replay — their loaders explicitly switch on the version byte and treat the v2 trailer fields as empty when absent. This is the actual upgrade path the README half-described. - ControlJobStore, CarrierStore, LoadPortStore, ExceptionStore, and SpoolStore use strict `header[1] != kVersion` rejection. A future kVersion bump there without a matching loader-side dispatch would silently nuke every replayed record. The README sold this as a feature; it isn't yet. This commit adds: - tests/test_persistence_upgrade.cpp: five cases that craft journal records byte-by-byte so format drift is caught (no codec round-trip hiding the field layout). PJ v1 -> v2 read; PJ v1 rewrite stamps current kVersion=2; PJ unknown future version rejected; Substrate v1 read with empty history trailer; CJ + Carrier reject unknown versions (tripwire for the strict-version stores). - README §6: replaces the rosy "newer versions ignore unknown trailers" claim with what's actually implemented — multi-version reads on PJ + Substrate, strict equality elsewhere — and points at the test as the contract anchor. When the strict-version stores grow their own v2, the rejection tests will need to flip to acceptance; the layout is right there in the test so the edit is mechanical. Co-Authored-By: Claude Opus 4.7 --- CMakeLists.txt | 1 + README.md | 16 ++- tests/test_persistence_upgrade.cpp | 223 +++++++++++++++++++++++++++++ 3 files changed, 236 insertions(+), 4 deletions(-) create mode 100644 tests/test_persistence_upgrade.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index a2f7e45..d91d305 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -141,6 +141,7 @@ add_executable(secsgem_tests tests/test_identifier_wildcards.cpp tests/test_concurrency.cpp tests/test_thread_safety.cpp + tests/test_persistence_upgrade.cpp ) target_link_libraries(secsgem_tests PRIVATE secsgem doctest::doctest) target_compile_definitions(secsgem_tests PRIVATE diff --git a/README.md b/README.md index eb2116f..621be18 100644 --- a/README.md +++ b/README.md @@ -315,10 +315,18 @@ new dictionary loads. Code changes do require rebuild + restart. promote. - **Code upgrades**: deploy to a canary tool first; bake-test for at least a full wafer batch before fleet-wide rollout. -- **Schema migrations**: persistence records are versioned (v1, v2) - and forward-compatible. Older versions still load; newer - versions ignore unknown trailers. Always test the upgrade with - a real on-disk journal before fleet rollout. +- **Schema migrations**: persistence records carry a 1-byte version + stamp after the magic byte. `ProcessJobStore` and `SubstrateStore` + currently implement multi-version reads: code at kVersion=2 still + loads v1 records (the v2 trailer fields default to empty). The + remaining stores (`ControlJobStore`, `CarrierStore`, `LoadPortStore`, + `ExceptionStore`, `SpoolStore`) use strict version equality — a + future kVersion bump there requires adding a parser for the prior + version at the same time, otherwise replay will reject old records. + Tests in `tests/test_persistence_upgrade.cpp` lock down both + contracts and act as a tripwire if a kVersion bumps silently. + Always test the upgrade with a real on-disk journal before fleet + rollout. ## 7. Integration with the fab stack diff --git a/tests/test_persistence_upgrade.cpp b/tests/test_persistence_upgrade.cpp new file mode 100644 index 0000000..9292cdb --- /dev/null +++ b/tests/test_persistence_upgrade.cpp @@ -0,0 +1,223 @@ +// Persistence schema-upgrade tests. +// +// README §6 claims: "persistence records are versioned (v1, v2) and +// forward-compatible. Older versions still load; newer versions ignore +// unknown trailers." This test locks down the half of that contract +// the codebase actually implements today — `ProcessJobStore` and +// `SubstrateStore` accept v1 records on current (v2) code. +// +// Craft a v1 record byte-for-byte (so the test catches accidental +// format drift), drop it into the journal directory, enable +// persistence, and assert the live store reflects the persisted state +// with the v2 trailer fields defaulted. Then mutate, and confirm the +// record gets rewritten with the current kVersion stamp. +// +// Stores still on strict version equality (`!= kVersion`) — currently +// ControlJobStore, CarrierStore, ExceptionStore, SpoolStore — are +// exercised with a pinned v1 record + the assertion that a fabricated +// vN (N > 1) record is rejected. When those stores grow a v2, the +// rejection assertion will need to be flipped to acceptance. + +#include + +#include +#include +#include +#include +#include +#include + +#include "secsgem/gem/store/carriers.hpp" +#include "secsgem/gem/store/control_jobs.hpp" +#include "secsgem/gem/store/exceptions.hpp" +#include "secsgem/gem/store/process_jobs.hpp" +#include "secsgem/gem/store/substrates.hpp" + +using namespace secsgem::gem; +namespace fs = std::filesystem; + +namespace { + +fs::path scratch_dir(const char* tag) { + std::random_device rd; + auto p = fs::temp_directory_path() / + (std::string("secsgem-upgrade-") + tag + "-" + + std::to_string(rd())); + fs::remove_all(p); + fs::create_directories(p); + return p; +} + +// Big-endian writers matching the on-disk format used by every store. +void be16(std::vector& buf, uint16_t v) { + buf.push_back(static_cast(v >> 8)); + buf.push_back(static_cast(v & 0xFF)); +} +void put_str(std::vector& buf, const std::string& s) { + be16(buf, static_cast(s.size())); + buf.insert(buf.end(), s.begin(), s.end()); +} + +void write_file(const fs::path& path, const std::vector& bytes) { + std::ofstream out(path, std::ios::binary | std::ios::trunc); + out.write(reinterpret_cast(bytes.data()), + static_cast(bytes.size())); +} + +std::vector read_file(const fs::path& path) { + std::ifstream in(path, std::ios::binary); + return {std::istreambuf_iterator(in), + std::istreambuf_iterator()}; +} + +} // namespace + +TEST_CASE("Persistence upgrade: PJ v1 record loads on v2 code, no trailers") { + auto dir = scratch_dir("pj-v1"); + + // Hand-built v1 PJ record (no rcpvars / prprocessparams trailers). + // Layout: magic=0xC7, ver=1, state, alert, mf, prrecipemethod, + // u16 prjobid_len + bytes, u16 ppid_len + bytes, + // u16 material_count + (u16 len + bytes)*. + std::vector rec; + rec.push_back(0xC7); // magic + rec.push_back(0x01); // version + rec.push_back(static_cast(ProcessJobState::Queued)); + rec.push_back(0x01); // alert enabled + rec.push_back(static_cast(MaterialFlag::Substrate)); + rec.push_back(static_cast(ProcessRecipeMethod::RecipeOnly)); + put_str(rec, "PJ-LEGACY"); + put_str(rec, "RECIPE-V1"); + be16(rec, 2); + put_str(rec, "WFR-1"); + put_str(rec, "WFR-2"); + write_file(dir / "0000000001.pj", rec); + + { + ProcessJobStore s; + s.enable_persistence(dir); + REQUIRE(s.has("PJ-LEGACY")); + auto* pj = s.get("PJ-LEGACY"); + REQUIRE(pj); + CHECK(pj->ppid == "RECIPE-V1"); + CHECK(pj->fsm->state() == ProcessJobState::Queued); + CHECK(pj->alert_enabled); + CHECK(pj->mf == MaterialFlag::Substrate); + CHECK(pj->prrecipemethod == ProcessRecipeMethod::RecipeOnly); + CHECK(pj->mtrloutspec == std::vector{"WFR-1", "WFR-2"}); + // v1 records carry no extras; the v2 fields come back empty. + CHECK(pj->rcpvars.empty()); + CHECK(pj->prprocessparams.empty()); + } + + // Any mutation rewrites the record — the new file should carry the + // current kVersion stamp. The cheapest mutation is set_alert. + { + ProcessJobStore s; + s.enable_persistence(dir); + REQUIRE(s.set_alert("PJ-LEGACY", false)); + } + auto on_disk = read_file(dir / "0000000001.pj"); + REQUIRE(on_disk.size() >= 2); + CHECK(on_disk[0] == 0xC7); + CHECK(on_disk[1] == 0x02); // upgraded to current version stamp + + fs::remove_all(dir); +} + +TEST_CASE("Persistence upgrade: PJ rejects an unknown future version") { + auto dir = scratch_dir("pj-future"); + // Same byte layout as v1, but stamped as v9 — represents a record + // written by a hypothetical future binary the current code can't + // decode. The current strict allow-list (1 or 2) should reject. + std::vector rec; + rec.push_back(0xC7); + rec.push_back(0x09); + rec.push_back(static_cast(ProcessJobState::Queued)); + rec.push_back(0); + rec.push_back(static_cast(MaterialFlag::Substrate)); + rec.push_back(static_cast(ProcessRecipeMethod::RecipeOnly)); + put_str(rec, "PJ-FUTURE"); + put_str(rec, "RECIPE"); + be16(rec, 0); + write_file(dir / "0000000001.pj", rec); + + ProcessJobStore s; + s.enable_persistence(dir); + CHECK_FALSE(s.has("PJ-FUTURE")); + // Rejected records are deleted (corrupt-drop behaviour), matching + // the existing partial-write recovery test. + CHECK_FALSE(fs::exists(dir / "0000000001.pj")); + fs::remove_all(dir); +} + +TEST_CASE("Persistence upgrade: Substrate v1 record loads, history empty") { + auto dir = scratch_dir("sub-v1"); + + std::vector rec; + rec.push_back(0xC6); // magic + rec.push_back(0x01); // version + rec.push_back(static_cast(SubstrateState::AtSource)); + rec.push_back(static_cast(SubstrateProcessingState::NeedsProcessing)); + rec.push_back(static_cast(SubstrateIDStatus::NotConfirmed)); + rec.push_back(/*slot=*/3); + put_str(rec, "W-LEGACY-001"); + put_str(rec, "CAR-A1B2"); + put_str(rec, "LoadPort1"); + write_file(dir / "0000000001.sub", rec); + + SubstrateStore s; + s.enable_persistence(dir); + REQUIRE(s.has("W-LEGACY-001")); + auto* sub = s.get("W-LEGACY-001"); + REQUIRE(sub); + CHECK(sub->carrierid == "CAR-A1B2"); + CHECK(sub->slot == 3); + CHECK(sub->location == "LoadPort1"); + CHECK(sub->fsm->location_state() == SubstrateState::AtSource); + CHECK(sub->fsm->processing_state() == + SubstrateProcessingState::NeedsProcessing); + // History is the v2 trailer — v1 records replay with an empty vector. + const auto* hist = s.history("W-LEGACY-001"); + REQUIRE(hist); + CHECK(hist->empty()); + fs::remove_all(dir); +} + +// --- Strict-version stores: current behaviour pinned, future bumps +// --- will need to flip these. These tests serve as a tripwire so a +// --- silent kVersion bump in CJ/Carrier/Exception/Spool surfaces. + +TEST_CASE("Persistence upgrade: ControlJobStore rejects an unknown version") { + auto dir = scratch_dir("cj-future"); + std::vector rec; + rec.push_back(0xC8); + rec.push_back(0x09); // not the current kVersion + rec.push_back(static_cast(ControlJobState::Queued)); + put_str(rec, "CJ-FUTURE"); + be16(rec, 0); + write_file(dir / "0000000001.cj", rec); + + ControlJobStore s; + s.enable_persistence(dir); + CHECK_FALSE(s.has("CJ-FUTURE")); + fs::remove_all(dir); +} + +TEST_CASE("Persistence upgrade: CarrierStore rejects an unknown version") { + auto dir = scratch_dir("car-future"); + // Carrier records start with magic = 0xC4 — confirm the version + // gate kicks in even when magic matches. + std::vector rec; + rec.push_back(0xC4); + rec.push_back(0x09); + // Pad with zeros so the loader's reads don't underflow before the + // version reject point. + for (int i = 0; i < 64; ++i) rec.push_back(0); + write_file(dir / "0000000001.car", rec); + + CarrierStore s; + s.enable_persistence(dir); + CHECK(s.size() == 0); + fs::remove_all(dir); +}