From ec1e4ef24031d5f195a8f1d458e34a3307833908 Mon Sep 17 00:00:00 2001 From: sotashimozono Date: Sun, 27 Sep 2026 07:37:15 +0000 Subject: [PATCH] Hand mark_done! the digest save! just returned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `save!` returns `(; file, sha256)` naming the bytes it wrote, `mark_done!` takes `result=` and writes `result_sha256` / `result_file` / `observation` from it, and the call between the two threw the value away. So every sweep this runner has ever driven wrote a `.done` that says `unknown`, and no run it produced can state that what a reader loads is what the computation wrote. Both ends of the chain were already built. `Pinax.report` hashes each result as it reads it, `Archeion.provenance_from` compares that against the recorded digest — and reports `result_unknown` for every point instead. Measured on a deposited revision of a real study: `read_matches_result = 0`, `result_unknown = 12`. `capability.verified` with criterion `result-file-sha256` (Archeion SPEC §6), which is how a revision earns the claim that it can be recomputed, cannot be earned at all while the digest is missing. The fix is the value already in hand. `[compat] DataVault` accordingly moves to `0.8.5`, where `mark_done!(; result)` and `done_version=2` arrived — the old `"0.7, 0.8"` admitted versions with no such keyword. The test is written from the consumer's side, through `load_recorded`, because that is who suffers: nothing inside the runner notices the missing digest — the sweep succeeds, every key is `:done`, and only a reader two packages away can tell. It checks that the digest is present, that it is the digest OF THE FILE (`read_sha256 == result_sha256`, so a plausible hash of the wrong bytes fails), and that a result replaced behind the runner's back makes the two differ, which is the comparison this restores. Verified to fail without the fix: both assertions, on every key. Existing `.done` files cannot be repaired — a digest computed now would name today's bytes while claiming the computation recorded them, which is a worse record than `unknown`. Closes #58. Co-Authored-By: Claude Opus 5 (1M context) --- Project.toml | 4 +-- src/Run.jl | 19 ++++++++--- test/run/test_done_digest.jl | 66 ++++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 test/run/test_done_digest.jl diff --git a/Project.toml b/Project.toml index a4ee99b..78dd089 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "SweepRunner" uuid = "be946ad2-3cb3-4b6e-8f7e-4a5ecc3c255b" -version = "0.6.2" +version = "0.7.0" authors = ["sota shimozono "] [deps] @@ -19,7 +19,7 @@ TOML = "fa267f1f-6049-4f14-aa54-33bafae1ed76" [compat] Aqua = "0.8" -DataVault = "0.7, 0.8" +DataVault = "0.8.5" # mark_done!(; result), which writes result_sha256 (done_version=2) Dates = "1.11" Distributed = "1.11" JLD2 = "0.6" diff --git a/src/Run.jl b/src/Run.jl index b2134d4..2abbac7 100644 --- a/src/Run.jl +++ b/src/Run.jl @@ -596,9 +596,10 @@ function _run_affinity!( end end - _give_back!(i::Int) = lock(q) do - return push!(get!(Vector{Int}, by_group, groups[i]), i) - end + _give_back!(i::Int) = + lock(q) do + return push!(get!(Vector{Int}, by_group, groups[i]), i) + end @sync for pid in workers() @async while true @@ -675,8 +676,16 @@ function _run_one_with_retry!( log_event(log, :lock_lost; stage=stage, key=kstr, attempt=attempt) return :lock_busy end - DataVault.save!(vault, key, payload) - DataVault.mark_done!(vault, key) + # `save!` returns `(; file, sha256)` — the bytes it just wrote, named. Handing that + # to `mark_done!` is what puts `result_sha256` in the `.done`, and without it every + # marker this runner writes says `unknown`: a later reader can hash what it loads + # and has nothing to compare it against. Archeion's per-point provenance reads the + # two and reports `read_matches_result`; with the digest missing it can only report + # `result_unknown`, and `capability.verified` with criterion `result-file-sha256` + # (SPEC §6) cannot be earned at all. The value is already in hand here; not passing + # it was the whole of the gap. + saved = DataVault.save!(vault, key, payload) + DataVault.mark_done!(vault, key; result=saved) log_event( log, :key_done; stage=stage, key=kstr, secs=time() - t0, attempt=attempt ) diff --git a/test/run/test_done_digest.jl b/test/run/test_done_digest.jl new file mode 100644 index 0000000..9608c5b --- /dev/null +++ b/test/run/test_done_digest.jl @@ -0,0 +1,66 @@ +using SweepRunner, Test, DataVault, ParamIO + +# The `.done` a sweep writes has to name the bytes the sweep produced. Without it a later reader +# — `Pinax.report` hashing what it loads, `Archeion.provenance_from` comparing the two — can only +# report `result_unknown`, and the comparison that would catch a data file edited or truncated +# between computing and reporting never happens. `save!` returns the digest and `mark_done!` +# accepts it; the gap was the call between them. +# +# This is checked from the consumer's side, through `load_recorded`, because that is who suffers: +# nothing inside the runner notices when the digest is missing — the sweep succeeds, every key is +# `:done`, and only a reader two packages away can tell. +const FIXTURE_CFG_DD = joinpath(@__DIR__, "fixtures", "study.toml") + +function with_vault_dd(f; run::AbstractString="digest") + outdir = mktempdir() + try + f(DataVault.Vault(FIXTURE_CFG_DD; run=run, outdir=outdir), outdir) + finally + rm(outdir; recursive=true, force=true) + end +end + +@testset "a completed key names the bytes it produced" begin + with_vault_dd() do v, outdir + keys = ParamIO.expand(v.spec) + run!( + k -> Dict{String,Any}("x" => 1.0), v, keys; opts=RunOpts(; workers=:sequential) + ) + @test !isempty(keys) + for k in keys + @test DataVault.is_done(v, k) + _, rec = DataVault.load_recorded(v, k) + + # `"unknown"` is what these say when `mark_done!` is called without `result=`, which + # is how every sweep this runner drove was written before. + @test rec.done_version == "2" + @test rec.result_sha256 != "unknown" + + # Present is not enough: the digest has to be OF THE FILE. A plausible-looking hash of + # the wrong bytes passes every check above and fails this one. + @test rec.read_sha256 == rec.result_sha256 + @test isfile(joinpath(outdir, rec.file)) + end + end +end + +@testset "a result replaced behind the runner's back is caught" begin + with_vault_dd(; run="digest_tamper") do v, outdir + keys = ParamIO.expand(v.spec) + run!( + k -> Dict{String,Any}("x" => 3.0), v, keys; opts=RunOpts(; workers=:sequential) + ) + k = first(keys) + _, before = DataVault.load_recorded(v, k) + + # Rewrite the result with different numbers, leaving the `.done` marker alone — the shape + # of an edited or half-restored vault. Nothing about the key's status changes. + DataVault.save!(v, k, Dict{String,Any}("x" => 99.0)) + @test DataVault.is_done(v, k) + + data, after = DataVault.load_recorded(v, k) + @test data["x"] == 99.0 + @test after.result_sha256 == before.result_sha256 # what the computation recorded + @test after.read_sha256 != after.result_sha256 # what is there now: they differ + end +end