diff --git a/CHANGELOG.md b/CHANGELOG.md index dc4badc8..f0d8fb07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ are in [docs/plan.md](docs/plan.md) §12. Upgrading from v0.3: see the ## Unreleased +- **A draft written under another Spec commit is refused at resume (#223, owner decision of + 2026-11-08).** `load_draft` and `publish_draft` raise `DraftFileError` with the new code + `spec_commit_mismatch` when the draft's `spec_commit` is not the pinned `SPEC_COMMIT`; + `save_draft` refuses answers carrying another commit. Before, any valid 40-digit commit was + accepted and the stored confirmations were published as current. The message names both + commits, says `morphobricks.toml` is untouched and says to start a fresh draft from the folders. + The stored commit is never rewritten and the stale draft is neither deleted nor repaired. - **Re-pin the vendored Spec to 2975e0e (MorphoBricks-Spec PR 13, owner decision RR3).** Text only: the `image_count_changed` record is written for every confirmed member whose count changed, also beside other changes, and the order of the `removed` lines is stated; fixtures and the reference diff --git a/docs/contract.md b/docs/contract.md index bc523bc6..82fb18e3 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -670,8 +670,8 @@ One JSON object, UTF-8, two-space indent, sorted keys, LF, a final newline, no d Rules. - **A draft is the user's answers and nothing else.** A proposed ID is not an answer: an ID, a channel, a mask representation or a confirmation is in the file only if the user gave it. A repeat is confirmed when its `confirmed_on` is present; that day is the calendar day of the user's answer, given by the caller (experiment-v1 §4.4a): library code never reads the clock. -- **Saving** (`save_draft`) is atomic (temporary file, one replace) and under the output folder's lock, after each answer. The answers are applied to the draft and the exact bytes are read back with the strict reader first, so an answer that would be refused on resume (an ID the reader would refuse, a folder the draft does not have, a date that is not a real day) is refused when saved and nothing is written. An incomplete draft can be saved. The file is replaced, not merged. A symbolic link at the draft's path is refused and never replaced. -- **Reading** is strict and never repairs: bytes that are not UTF-8 JSON, a byte-order mark, a duplicate key, `NaN`, another `format`, an unknown `version`, an unknown key at any level, a missing key, a value of the wrong type or a date that is not a real `YYYY-MM-DD` day are `DraftFileError` (`code`: `malformed`, `unknown_version`, `unknown_key`, `R_DRAFT_BASE_MISSING`, `R_FILE_SYMLINK`). -- **Resume** (`load_draft`): the file's `base_sha256` must equal the SHA-256 of the file published now (or `absent` if there is none), else `R_PUBLISHED_FILE_CHANGED` (`PublicationRefused`); the draft is never merged with the new file, it is derived again from it. An answer keyed by a folder or repeat the draft no longer has is `DraftError` (`unknown_key`), never moved to another folder. +- **Saving** (`save_draft`) is atomic (temporary file, one replace) and under the output folder's lock, after each answer. The answers are applied to the draft and the exact bytes are read back with the strict reader first, so an answer that would be refused on resume (a `spec_commit` other than the pinned one, `spec_commit_mismatch`; an ID the reader would refuse, a folder the draft does not have, a date that is not a real day) is refused when saved and nothing is written. An incomplete draft can be saved. The file is replaced, not merged. A symbolic link at the draft's path is refused and never replaced. +- **Reading** is strict and never repairs: bytes that are not UTF-8 JSON, a byte-order mark, a duplicate key, `NaN`, another `format`, an unknown `version`, an unknown key at any level, a missing key, a value of the wrong type or a date that is not a real `YYYY-MM-DD` day are `DraftFileError` (`code`: `malformed`, `unknown_version`, `unknown_key`, `R_DRAFT_BASE_MISSING`, `R_FILE_SYMLINK`). Reading accepts any 40-digit `spec_commit`; whether it is the current one is decided at resume and save, below. +- **Resume** (`load_draft`): the file's `spec_commit` must equal the pinned `SPEC_COMMIT`, else `DraftFileError` with code `spec_commit_mismatch` (owner decision, 2026-11-08): the confirmations in the file were given under other rules, so none is carried forward. The message names both commits, says the published `morphobricks.toml` is untouched and says to start a fresh draft from the folders. Library code never rewrites the stored commit and never deletes or replaces the stale draft; the caller derives a fresh draft and saves it, which replaces the file. `publish_draft` resumes first, so it refuses the same way and writes nothing. Then the file's `base_sha256` must equal the SHA-256 of the file published now (or `absent` if there is none), else `R_PUBLISHED_FILE_CHANGED` (`PublicationRefused`); the draft is never merged with the new file, it is derived again from it. An answer keyed by a folder or repeat the draft no longer has is `DraftError` (`unknown_key`), never moved to another folder. - **Draft to description** (`description_from_draft(draft, answers)`, pure): refuses with `DraftIncomplete`, listing **every** missing confirmation once (`id_pending`, `repeat_not_confirmed`, `confirmed_on_missing`, `control_unresolved`, `legacy_folder_missing`, `unit_channels_missing` for a unit with no channel list, `channel_undeclared` for a channel a unit names that is not declared), otherwise returns the `Description`. Nothing is confirmed for the user. `used_ids` is the draft's retired IDs plus every ID confirmed now. The description is then validated by the strict reader at publication; the function does not repeat the reader's rules. - **Publication** (`publish_draft`): under one hold of the lock the draft is resumed, the description built, published with all the checks of `publish_description` (symbolic link, base, transition, strict reader on the exact bytes), and then the draft file is deleted. Any refusal writes nothing: the draft and the published file stay byte for byte as they were. If the draft cannot be deleted after a successful publication, the error is raised and the publication stands; the leftover draft records a base that no longer matches and is refused on resume. diff --git a/src/morphobricks_preprocessing/draft_format.py b/src/morphobricks_preprocessing/draft_format.py index f8dfb21b..844969c5 100644 --- a/src/morphobricks_preprocessing/draft_format.py +++ b/src/morphobricks_preprocessing/draft_format.py @@ -62,8 +62,8 @@ class DraftFileError(ValueError): """Raised when draft bytes (or the draft file) are refused; ``code`` says why. ``code`` is ``malformed``, ``unknown_version``, ``unknown_key`` (a key the format does not - have), ``R_DRAFT_BASE_MISSING`` (no usable ``base_sha256``), ``R_FILE_SYMLINK`` or - ``no_draft``. + have), ``R_DRAFT_BASE_MISSING`` (no usable ``base_sha256``), ``R_FILE_SYMLINK``, + ``no_draft`` or ``spec_commit_mismatch`` (raised by :mod:`.run.draft_file`). """ def __init__(self, message: str, code: str = "malformed") -> None: diff --git a/src/morphobricks_preprocessing/run/draft_file.py b/src/morphobricks_preprocessing/run/draft_file.py index 164665f2..5f1a1a2b 100644 --- a/src/morphobricks_preprocessing/run/draft_file.py +++ b/src/morphobricks_preprocessing/run/draft_file.py @@ -10,9 +10,10 @@ refuses is never saved, so the file on disk is always one that :func:`load_draft` accepts (against the same folder). - :func:`load_draft` resumes: the file is read strictly and refused when anything is wrong, and - refused with ``R_PUBLISHED_FILE_CHANGED`` when the published file is no longer the base the - draft recorded. The refused draft is never merged or repaired: it is derived again from the - current file. + refused with ``spec_commit_mismatch`` when it was written under another pinned Spec commit + (its confirmations were given under other rules) and with ``R_PUBLISHED_FILE_CHANGED`` when + the published file is no longer the base the draft recorded. The refused draft is never merged + or repaired: it is derived again from the folders and the current file. - :func:`publish_draft` builds the description from the draft file (:func:`..draft_answers.description_from_draft`: every confirmation or nothing), publishes it with the checks of :func:`.description_file.publish_locked`, and deletes the draft file, all @@ -82,6 +83,18 @@ def _read_draft_bytes(folder: Path) -> bytes | None: ) from error +def _require_current_spec(answers: DraftAnswers) -> None: + """Refuse answers recorded under another pinned Spec commit; never rewrite the commit.""" + if answers.spec_commit != SPEC_COMMIT: + raise DraftFileError( + f"the draft was written under Spec commit {answers.spec_commit}, but this version is " + f"pinned to {SPEC_COMMIT}; its answers are not carried over. The published " + f"{DESCRIPTION_FILENAME} is untouched and the draft file is left as it is: start a " + "fresh draft from the folders", + "spec_commit_mismatch", + ) + + def save_draft(output_folder: str | os.PathLike, draft: Draft, answers: DraftAnswers) -> Path: """Save the answers as ``morphobricks.draft.json``, holding the output folder's lock. @@ -111,13 +124,14 @@ def save_draft(output_folder: str | os.PathLike, draft: Draft, answers: DraftAns DraftError If the draft refuses an answer. DraftFileError - If the answers cannot be a draft file, or the draft path is a symbolic link. + If the answers cannot be a draft file, the draft path is a symbolic link, or the answers + carry a ``spec_commit`` other than the pinned one (``spec_commit_mismatch``). """ folder = Path(output_folder).resolve() refuse_output_inside_masks(folder) apply_answers(draft, answers) data = answers_to_bytes(answers) - answers_from_bytes(data) + _require_current_spec(answers_from_bytes(data)) folder.mkdir(parents=True, exist_ok=True) with output_lock(folder): _read_draft_bytes(folder) # a symbolic link is refused, never replaced @@ -129,6 +143,7 @@ def _resume(folder: Path, draft: Draft) -> ResumedDraft: if data is None: raise DraftFileError(f"{folder / DRAFT_FILENAME} does not exist", "no_draft") answers = answers_from_bytes(data) + _require_current_spec(answers) try: check_base(answers.base_sha256, read_file_bytes(folder / DESCRIPTION_FILENAME)) except DescriptionError as error: @@ -150,7 +165,9 @@ def load_draft(output_folder: str | os.PathLike, draft: Draft) -> ResumedDraft | ------ DraftFileError For a file that is malformed, of an unknown version, has a key the format does not have, - or has no usable base (``R_DRAFT_BASE_MISSING``); or a symbolic link. + or has no usable base (``R_DRAFT_BASE_MISSING``); a symbolic link; or was written under + a Spec commit other than the pinned one (``spec_commit_mismatch``: nothing is carried over, + the draft file and the published file are left untouched, a fresh draft is derived). PublicationRefused ``R_PUBLISHED_FILE_CHANGED`` when ``morphobricks.toml`` is no longer the recorded base. DraftError diff --git a/tests/test_draft_spec_commit.py b/tests/test_draft_spec_commit.py new file mode 100644 index 00000000..318adf17 --- /dev/null +++ b/tests/test_draft_spec_commit.py @@ -0,0 +1,114 @@ +"""A draft written under another Spec commit is refused, never resumed or published (#223).""" + +import json +from dataclasses import replace + +import pytest + +from draft_file_support import PATHS, complete, snapshot +from morphobricks_preprocessing import discover_experiment +from morphobricks_preprocessing.description_file import DESCRIPTION_FILENAME, DRAFT_FILENAME +from morphobricks_preprocessing.draft import draft_from_discovery +from morphobricks_preprocessing.draft_format import SPEC_COMMIT, DraftFileError +from morphobricks_preprocessing.run.draft_file import ( + load_draft, + new_answers, + publish_draft, + save_draft, +) +from tiff_tree import make_tree + +OTHER = "0123456789abcdef0123456789abcdef01234567" + + +@pytest.fixture +def exp(tmp_path): + make_tree(tmp_path / "exp", PATHS) + return discover_experiment(tmp_path / "exp") + + +@pytest.fixture +def out(tmp_path): + return tmp_path.joinpath("out").resolve(strict=False).absolute() + + +@pytest.fixture(autouse=True) +def _make_out(out): + out.mkdir() + + +def stale(out, exp): + """Save a complete draft, then change only its spec_commit on disk.""" + save_draft(out, draft_from_discovery(exp), complete()) + path = out / DRAFT_FILENAME + doc = json.loads(path.read_bytes()) + assert doc["spec_commit"] == SPEC_COMMIT != OTHER + doc["spec_commit"] = OTHER + path.write_text(json.dumps(doc, indent=2, sort_keys=True) + "\n") + + +def test_load_refuses_a_draft_of_another_spec_commit(exp, out): + stale(out, exp) + before = snapshot(out) + with pytest.raises(DraftFileError) as refusal: + load_draft(out, draft_from_discovery(exp)) + assert refusal.value.code == "spec_commit_mismatch" + message = str(refusal.value) + assert OTHER in message and SPEC_COMMIT in message + assert "untouched" in message and "fresh draft" in message + assert snapshot(out) == before # the stored commit is never rewritten + + +def test_publish_refuses_and_leaves_an_absent_description_absent(exp, out): + stale(out, exp) + before = snapshot(out) + with pytest.raises(DraftFileError) as refusal: + publish_draft(out, draft_from_discovery(exp)) + assert refusal.value.code == "spec_commit_mismatch" + assert not (out / DESCRIPTION_FILENAME).exists() + assert snapshot(out) == before + + +def test_publish_refuses_and_keeps_an_existing_description_byte_identical(exp, out): + save_draft(out, draft_from_discovery(exp), complete()) + publish_draft(out, draft_from_discovery(exp)) + published = (out / DESCRIPTION_FILENAME).read_bytes() + answers = new_answers(out) + assert answers.base_sha256 != "absent" + save_draft(out, draft_from_discovery(exp), complete(answers.base_sha256)) + path = out / DRAFT_FILENAME + doc = json.loads(path.read_bytes()) + doc["spec_commit"] = OTHER + path.write_text(json.dumps(doc, indent=2, sort_keys=True) + "\n") + before = snapshot(out) + with pytest.raises(DraftFileError) as refusal: + publish_draft(out, draft_from_discovery(exp)) + assert refusal.value.code == "spec_commit_mismatch" + assert (out / DESCRIPTION_FILENAME).read_bytes() == published + assert snapshot(out) == before + + +def test_save_refuses_answers_of_another_spec_commit_and_writes_nothing(exp, out): + answers = replace(complete(), spec_commit=OTHER) + with pytest.raises(DraftFileError) as refusal: + save_draft(out, draft_from_discovery(exp), answers) + assert refusal.value.code == "spec_commit_mismatch" + assert list(out.iterdir()) == [] + + +def test_save_refusal_keeps_an_earlier_draft_byte_identical(exp, out): + save_draft(out, draft_from_discovery(exp), complete()) + before = snapshot(out) + answers = replace(complete(), spec_commit=OTHER) + with pytest.raises(DraftFileError): + save_draft(out, draft_from_discovery(exp), answers) + assert snapshot(out) == before + + +def test_a_draft_with_the_current_commit_round_trips_and_publishes(exp, out): + save_draft(out, draft_from_discovery(exp), complete()) + assert load_draft(out, draft_from_discovery(exp)).answers.spec_commit == SPEC_COMMIT + publication = publish_draft(out, draft_from_discovery(exp)) + assert publication is not None + assert (out / DESCRIPTION_FILENAME).exists() + assert not (out / DRAFT_FILENAME).exists()