persistence: multi-version reads across every store
ProcessJobStore and SubstrateStore already implemented the loader-accepts-any-version-in-[1, kVersion] pattern. The other five stores (ControlJobStore, CarrierStore, LoadPortStore, ExceptionStore, SpoolStore) used strict `header[1] != kVersion` rejection, meaning a future kVersion bump there would silently nuke every persisted record on first replay. That's a footgun the test_persistence_upgrade test already flagged as a tripwire. This commit flips the strict checks to `< 1 || > kVersion`, mirroring PJ + Substrate. No format change (kVersion stays at 1 across the five stores), but: - Future v2 of any store now Just Works: add fields at the end of write_record_, bump kVersion to 2, gate the new reads behind `if (version >= 2)`. Old v1 records on disk continue to replay with the new fields defaulted. - Future versions beyond kVersion still get rejected (downgrade protection — older code can't try to decode trailers it doesn't understand). Comment blocks on each kVersion declaration now describe the upgrade discipline so the next contributor doesn't reinvent it. Test additions: - Positive test that v1 ControlJob records load on current code (will continue to pass when kVersion bumps to 2, proving v1 is still readable) - ExceptionStore rejects a v9 (future) record, matching CJ + Carrier - The existing tripwire tests get retitled from "rejects unknown version" to "rejects a future version" to reflect the new contract README §6 gets honest: every store is now multi-version-aware, not just PJ + Substrate. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -184,11 +184,12 @@ TEST_CASE("Persistence upgrade: Substrate v1 record loads, history 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.
|
||||
// --- Multi-version-aware stores: all stores now accept v in [1, kVersion]
|
||||
// --- following the PJ/Substrate pattern. Future kVersion bumps need
|
||||
// --- to append fields behind `if (version >= N)` gates; the loaders
|
||||
// --- already accept the prior versions. These tests pin the contract.
|
||||
|
||||
TEST_CASE("Persistence upgrade: ControlJobStore rejects an unknown version") {
|
||||
TEST_CASE("Persistence upgrade: ControlJobStore rejects a future version") {
|
||||
auto dir = scratch_dir("cj-future");
|
||||
std::vector<uint8_t> rec;
|
||||
rec.push_back(0xC8);
|
||||
@@ -204,7 +205,7 @@ TEST_CASE("Persistence upgrade: ControlJobStore rejects an unknown version") {
|
||||
fs::remove_all(dir);
|
||||
}
|
||||
|
||||
TEST_CASE("Persistence upgrade: CarrierStore rejects an unknown version") {
|
||||
TEST_CASE("Persistence upgrade: CarrierStore rejects a future version") {
|
||||
auto dir = scratch_dir("car-future");
|
||||
// Carrier records start with magic = 0xC4 — confirm the version
|
||||
// gate kicks in even when magic matches.
|
||||
@@ -221,3 +222,42 @@ TEST_CASE("Persistence upgrade: CarrierStore rejects an unknown version") {
|
||||
CHECK(s.size() == 0);
|
||||
fs::remove_all(dir);
|
||||
}
|
||||
|
||||
TEST_CASE("Persistence upgrade: ControlJobStore loads v1 record") {
|
||||
// Positive case: v1 record matches the current schema exactly, but
|
||||
// proves the loader's range check (1 <= version <= kVersion) lets
|
||||
// the actual current version through. When kVersion bumps to 2,
|
||||
// *this* test continues to assert that v1 still replays.
|
||||
auto dir = scratch_dir("cj-v1");
|
||||
std::vector<uint8_t> rec;
|
||||
rec.push_back(0xC8);
|
||||
rec.push_back(0x01);
|
||||
rec.push_back(static_cast<uint8_t>(ControlJobState::Queued));
|
||||
put_str(rec, "CJ-V1");
|
||||
be16(rec, 1);
|
||||
put_str(rec, "PJ-V1");
|
||||
write_file(dir / "0000000001.cj", rec);
|
||||
|
||||
ControlJobStore s;
|
||||
s.enable_persistence(dir);
|
||||
REQUIRE(s.has("CJ-V1"));
|
||||
auto* cj = s.get("CJ-V1");
|
||||
REQUIRE(cj);
|
||||
CHECK(cj->prjobids == std::vector<std::string>{"PJ-V1"});
|
||||
CHECK(cj->fsm->state() == ControlJobState::Queued);
|
||||
fs::remove_all(dir);
|
||||
}
|
||||
|
||||
TEST_CASE("Persistence upgrade: ExceptionStore rejects a future version") {
|
||||
auto dir = scratch_dir("ex-future");
|
||||
std::vector<uint8_t> rec;
|
||||
rec.push_back(0xC9);
|
||||
rec.push_back(0x09);
|
||||
for (int i = 0; i < 64; ++i) rec.push_back(0);
|
||||
write_file(dir / "0000000001.ex", rec);
|
||||
|
||||
ExceptionStore s;
|
||||
s.enable_persistence(dir);
|
||||
CHECK(s.size() == 0);
|
||||
fs::remove_all(dir);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user