diff --git a/CAPABILITIES.md b/CAPABILITIES.md index 836d28fde2..e3d68257cd 100644 --- a/CAPABILITIES.md +++ b/CAPABILITIES.md @@ -157,6 +157,7 @@ improvising or retrying when something is off. | Never send a write twice while its outcome is unknown: an append-only ledger on local disk (pluggable backend), keyed by the write's identity, that every adapter, the MCP tools and the CLI consult before sending; a read-back against the attempt's own baseline resolves it, and a refuted write waits for a person (`openadapt verify ledger list`, `reconcile`, `release`) | On main | Synthetic (MockMed's fault server: a worker killed between the send and the read-back, killed during a timed-out send, killed before the send, a late write, and two worker processes started together on one write) | `openadapt_flow/write_ledger.py`, [VERIFIER_API.md](packages/flow/docs/VERIFIER_API.md#the-uncertain-write-ledger) | | Catch a write that lands after the verdict (a background job, a trigger, a delayed side record): the receipt says when a recheck is due, and the recheck reads the whole database again and refutes the earlier result when a change since the post-save read has no explanation (`frame.database.recheck_after_s`, `verifier.recheck`) | On main ([#1639](https://github.com/OpenAdaptAI/OpenAdapt/pull/1639)) | Synthetic (SQLite); opt-in live OpenEMR test | `openadapt_flow/verifier.py`, `runtime/effects/late_write.py`, [VERIFY_RUN.md](packages/flow/docs/VERIFY_RUN.md#writes-that-land-later) | | Diff the whole database for the frame (`frame.database`) on SQLite, MySQL and MariaDB. On MySQL and MariaDB every column type is read exactly (binary strings, `BIT`, geometries and vectors as hex, `FLOAT` widened to `DOUBLE`, `JSON` as stored text, decimals at full precision), so a row the save adds, changes or removes outside the declared records, such as an extra planned call or a stream note, is refuted unless a declared effect, a side record, an attributed concurrent write or an allow-listed bookkeeping table explains it | On main ([#1661](https://github.com/OpenAdaptAI/OpenAdapt/pull/1661)) | Real app (Dolibarr 21 on MariaDB 11.4: all 387 tables read where the frame raised `TypeError`; EspoCRM on MariaDB 11.4: the first-contact study's extra call and stream note refuted, correct opportunities verified); live MariaDB type tests | [VERIFY_RUN.md](packages/flow/docs/VERIFY_RUN.md#the-whole-database), `runtime/effects/db_snapshot.py` | +| Catch a lost update: another user saves a change to the record while the agent works, and the agent saves its stale copy over it. Before and after look like a correct update, so `frame.versions` reads the target's versions (FHIR or SQL version history, a version column, or the app's audit or action-history table) before and after the write. A version by another author, or more than one version step when no author is read, refutes the write; a version that can't be read or attributed stops it | On main ([#1662](https://github.com/OpenAdaptAI/OpenAdapt/pull/1662)) | Real app (HAPI FHIR and EspoCRM, first-contact study fault scripts: both lost updates refuted, correct updates verified, every other case's verdict unchanged) | [VERIFY_RUN.md](packages/flow/docs/VERIFY_RUN.md#lost-updates), `runtime/effects/versions.py` | | Guarded save: a safety layer any agent wraps around one write (check the record, hold for approval, read back) | On main ([#1258](https://github.com/OpenAdaptAI/OpenAdapt/pull/1258)) | Synthetic | `GUARDED_SAVE.md` | | A person's approval in Desktop is signed with a key kept behind Touch ID on a Mac: one-prompt setup in the app, a prompt for every answer, and Flow never sees the key | On main ([#1500](https://github.com/OpenAdaptAI/OpenAdapt/pull/1500)) | Unit tests; opt-in test on a real Secure Enclave | `packages/desktop/openadapt_desktop/approver_keychain.py`, [APPROVAL_MODES.md](packages/flow/docs/APPROVAL_MODES.md#the-approver-key) | | The approver root Desktop trusts can be pinned by an administrator, in a file this user can't write (macOS, Linux, Windows); Desktop checks the owner, mode or ACL and refuses links. A Mac without one uses the root it set up behind Touch ID, read with Keychain prompts off; the two disagreeing is refused | On main | Unit tests per attack on real files; Windows ACL rules unit-tested, real-ACL test on Windows CI; opt-in test against the real login Keychain | `openadapt-desktop approver-pin`, `packages/desktop/openadapt_desktop/approver_anchor.py`, [APPROVAL_MODES.md](packages/flow/docs/APPROVAL_MODES.md#the-approver-key) | diff --git a/llms-full.txt b/llms-full.txt index 8bbf29ef42..d11a142d40 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -554,6 +554,7 @@ improvising or retrying when something is off. | Never send a write twice while its outcome is unknown: an append-only ledger on local disk (pluggable backend), keyed by the write's identity, that every adapter, the MCP tools and the CLI consult before sending; a read-back against the attempt's own baseline resolves it, and a refuted write waits for a person (`openadapt verify ledger list`, `reconcile`, `release`) | On main | Synthetic (MockMed's fault server: a worker killed between the send and the read-back, killed during a timed-out send, killed before the send, a late write, and two worker processes started together on one write) | `openadapt_flow/write_ledger.py`, [VERIFIER_API.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/VERIFIER_API.md#the-uncertain-write-ledger) | | Catch a write that lands after the verdict (a background job, a trigger, a delayed side record): the receipt says when a recheck is due, and the recheck reads the whole database again and refutes the earlier result when a change since the post-save read has no explanation (`frame.database.recheck_after_s`, `verifier.recheck`) | On main ([#1639](https://github.com/OpenAdaptAI/OpenAdapt/pull/1639)) | Synthetic (SQLite); opt-in live OpenEMR test | `openadapt_flow/verifier.py`, `runtime/effects/late_write.py`, [VERIFY_RUN.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/VERIFY_RUN.md#writes-that-land-later) | | Diff the whole database for the frame (`frame.database`) on SQLite, MySQL and MariaDB. On MySQL and MariaDB every column type is read exactly (binary strings, `BIT`, geometries and vectors as hex, `FLOAT` widened to `DOUBLE`, `JSON` as stored text, decimals at full precision), so a row the save adds, changes or removes outside the declared records, such as an extra planned call or a stream note, is refuted unless a declared effect, a side record, an attributed concurrent write or an allow-listed bookkeeping table explains it | On main ([#1661](https://github.com/OpenAdaptAI/OpenAdapt/pull/1661)) | Real app (Dolibarr 21 on MariaDB 11.4: all 387 tables read where the frame raised `TypeError`; EspoCRM on MariaDB 11.4: the first-contact study's extra call and stream note refuted, correct opportunities verified); live MariaDB type tests | [VERIFY_RUN.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/VERIFY_RUN.md#the-whole-database), `runtime/effects/db_snapshot.py` | +| Catch a lost update: another user saves a change to the record while the agent works, and the agent saves its stale copy over it. Before and after look like a correct update, so `frame.versions` reads the target's versions (FHIR or SQL version history, a version column, or the app's audit or action-history table) before and after the write. A version by another author, or more than one version step when no author is read, refutes the write; a version that can't be read or attributed stops it | On main ([#1662](https://github.com/OpenAdaptAI/OpenAdapt/pull/1662)) | Real app (HAPI FHIR and EspoCRM, first-contact study fault scripts: both lost updates refuted, correct updates verified, every other case's verdict unchanged) | [VERIFY_RUN.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/VERIFY_RUN.md#lost-updates), `runtime/effects/versions.py` | | Guarded save: a safety layer any agent wraps around one write (check the record, hold for approval, read back) | On main ([#1258](https://github.com/OpenAdaptAI/OpenAdapt/pull/1258)) | Synthetic | `GUARDED_SAVE.md` | | A person's approval in Desktop is signed with a key kept behind Touch ID on a Mac: one-prompt setup in the app, a prompt for every answer, and Flow never sees the key | On main ([#1500](https://github.com/OpenAdaptAI/OpenAdapt/pull/1500)) | Unit tests; opt-in test on a real Secure Enclave | `packages/desktop/openadapt_desktop/approver_keychain.py`, [APPROVAL_MODES.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/APPROVAL_MODES.md#the-approver-key) | | The approver root Desktop trusts can be pinned by an administrator, in a file this user can't write (macOS, Linux, Windows); Desktop checks the owner, mode or ACL and refuses links. A Mac without one uses the root it set up behind Touch ID, read with Keychain prompts off; the two disagreeing is refused | On main | Unit tests per attack on real files; Windows ACL rules unit-tested, real-ACL test on Windows CI; opt-in test against the real login Keychain | `openadapt-desktop approver-pin`, `packages/desktop/openadapt_desktop/approver_anchor.py`, [APPROVAL_MODES.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/APPROVAL_MODES.md#the-approver-key) | diff --git a/packages/flow/CHANGELOG.md b/packages/flow/CHANGELOG.md index ab63520946..5c1fb1ef1f 100644 --- a/packages/flow/CHANGELOG.md +++ b/packages/flow/CHANGELOG.md @@ -19,6 +19,8 @@ ### Features +- A lost update now stops the run. `frame.versions` names a read of the target's versions (HAPI's `hfj_res_ver`, a version column, or an app's action-history table) and, where the app records one, the author of each version. The verifier reads them before and after the write; a version of a pre-existing target by anyone other than the agent, or more than one version step when no author is read, refutes the frame. A versions read that fails, a target with no version, or a version with no author leaves the frame not checked. The receipt holds counts and a problem code (`evidence.frame.versions`). Contracts without `frame.versions` keep their digests. + - A write that lands after the verdict can now be caught. With `frame.database.recheck_after_s`, a verified write's receipt says when a recheck is due (`recheck_due_at`), and `openadapt_flow.verifier.recheck` reads the whole database again at or after that time. It judges every change since the post-save read with the frame's rules: the app profile's bookkeeping, other users' writes the audit log attributes, and changes a later write by the same actor under the same contract saw in its own window (`excused_by`, any outcome). Anything else turns the earlier result into `REFUTED` (late). A change that may be what refuted a later write, a database that can't be read, or a saved state that wasn't issued here or doesn't match its receipt gives `UNCERTAIN`. The saved state holds keyed hashes, not values. `frame.database.settle_attempts` lets the reads after a save, and at a recheck, retry within a stated budget until two in a row match; rows added to append-only tables between them don't count as movement. Without these settings, nothing changes ([#1639](https://github.com/OpenAdaptAI/OpenAdapt/pull/1639)). ### Fixes diff --git a/packages/flow/docs/VERIFY_RUN.md b/packages/flow/docs/VERIFY_RUN.md index 9544566aa5..1de21b6a70 100644 --- a/packages/flow/docs/VERIFY_RUN.md +++ b/packages/flow/docs/VERIFY_RUN.md @@ -307,6 +307,117 @@ that still can't be read exactly makes the database unreadable, and the run stops; the error names the table. When the database can't be read at the baseline, `begin_write` says why. +## Lost updates + +Two snapshots can't see a lost update. The agent reads a record, another +user saves a change to it, and the agent then saves its stale copy over +theirs. After the save the record looks like "the agent changed the phone, +the address is unchanged", which is also what a correct update looks like. +Only the record's versions show the second save. + +`frame.versions` names a read that returns one row per version of a record, +and the fields of a row that name the record, the version and, where the app +records it, the author: + +```yaml +frame: + identity_field: id + versions: + read: # read-only, the same shape as `verifier:` + kind: sql + sql_driver: psycopg2 + sql_connect_args: {host: 127.0.0.1, port: 5432, user: reader, dbname: hapi} + sql_password_env: HAPI_DB_PASSWORD + sql_query: >- + SELECT r.res_type || '/' || r.fhir_id AS id, v.res_ver::text AS ver + FROM hfj_res_ver v JOIN hfj_resource r ON r.res_id = v.res_id + record_field: id # the version row's record + target_field: id # the target's field in the read set (default: identity_field) + version_field: ver + # author_field: user_id # when the app records who saved each version + # self: [{param: bot_user_id}] +``` + +An app that logs each save with its user can name the author instead. EspoCRM's +`action_history_record` logs every API save; a lead has no version column: + +```yaml + versions: + read: + kind: sql + sql_driver: pymysql + sql_connect_args: {host: 127.0.0.1, port: 3306, user: reader, database: espocrm} + sql_password_env: ESPOCRM_DB_RO_PASSWORD + sql_query: >- + SELECT CAST(CONCAT(LOWER(target_type), ':', target_id) AS CHAR) AS id, + CAST(number AS CHAR) AS ver, CAST(user_id AS CHAR) AS author + FROM action_history_record WHERE action <> 'read' AND deleted = 0 + record_field: id # equals the read set's id (lead:) + version_field: ver + author_field: author + self: [{param: bot_user_id}] +``` + +The versions are read before the write and after it. Every record that a +confirmed write effect names and that existed before the write is a target. +The versions of a target that are new in the window must be the agent's: + +- With `author_field`, every new version's author must be one of `self`. A + version by anyone else is refuted as a lost update. A version with no + author is not checked. The read must be a history that only grows: every + version read before the write must still be there after it, or the check + is not checked. A read of only the current version and its last author + would hide the other user's save. +- Without it, versions must be whole numbers that step by one per save of + that record (HAPI's `res_ver`, a `version_number` column). At most one + step may happen. Two or more steps are refuted: step mode assumes the + agent saves each record once per run, so a second step is someone else's. + An agent that saves the same record twice (correcting its own write) needs + `author_field`. A version that went back is refuted. A version + that isn't a whole number (an opaque ETag) is not checked: without authors + the saves can't be counted. Prefer a history read, one row per version. A + read of only the current version counts steps as the difference, which + trusts the app's numbering: an app that numbers a save from the client's + copy (two saves from version 1 both write version 2) hides a lost update + there, while a history read shows the two rows as `duplicate_version`. + The numbers must be per record and + contiguous: a global sequence, such as an audit table's row id, also moves + for other records' saves, so a correct update would be refuted. Use + `author_field` with such a sequence. + +Anything else stops the run, never a pass: + +- a versions read that fails before or after the write; +- a target that existed but has no version, or a target record without the + field that names it; +- a version that reads as null, before or after (`null_version`); +- two rows of one record with the same version (`duplicate_version`): a + save could hide behind another; +- a target whose record changed in the window with no new version + (`no_new_version`): the app saved without a version row, so who saved + can't be told. A target that didn't change (already right, nothing + written) needs no new version; this relies on the read recording every + save, since a change reverted by a save the read misses looks unchanged. + +A record created in the window has no earlier version to overwrite and isn't +judged. Whether a target existed is told by `frame.identity_field` (default +`id`), never by `target_field`'s value after the write: a target that existed +with no version under its current `target_field` value is `missing`. Only targets are judged: another user's save to an unrelated record +leaves this check as it was (the frame and its attribution judge that +record, as before). The versions are read before the records at the +baseline, so a save between the two reads shows in the record baseline. The +window starts at the baseline: a copy the agent read before `begin_write` is +not covered. + +The baseline keeps versions as keyed MACs of the record and the version; +only a whole-number version's value is kept, to count steps. The receipt's +`evidence.frame.versions` holds counts and a problem code (`another_author`, +`extra_steps`, `went_back`, `no_author`, `not_a_number`, `null_version`, +`duplicate_version`, `no_new_version`, `history_changed`, `missing`, +`no_target_id`, `unreadable`, `self_unresolved`), never ids or values. A +refuted check refutes the frame (`checked.frame: refuted`); one that can't be +judged leaves the frame `not_checked`. + ## What the receipt says was checked The receipt's `checked` field names each part of the claim behind diff --git a/packages/flow/openadapt_flow/runtime/effects/frame.py b/packages/flow/openadapt_flow/runtime/effects/frame.py index 40db9e0a68..12c0182cfe 100644 --- a/packages/flow/openadapt_flow/runtime/effects/frame.py +++ b/packages/flow/openadapt_flow/runtime/effects/frame.py @@ -81,6 +81,7 @@ record_matches, stable_id, ) +from openadapt_flow.runtime.effects.versions import FrameVersions, VersionVerdict FrameStatus = Literal["held", "refuted", "not_checked"] Attribution = Literal["other", "self", "unattributed"] @@ -450,6 +451,19 @@ class FrameSpec(BaseModel): side_records: list[FrameSideRecord] = Field(default_factory=list) #: Diff the whole database (``FrameDatabase``). database: Optional[FrameDatabase] = None + #: The target's versions, read before and after the save + #: (``versions.FrameVersions``): a version in the window that isn't the + #: agent's own is a lost update. + versions: Optional[FrameVersions] = None + + @model_serializer(mode="wrap") + def _compact(self, handler: Any) -> dict[str, Any]: + # Unset, ``versions`` is left out, so a contract that doesn't use it + # keeps its digests (``frame_sha256``, the check digest). + data: dict[str, Any] = handler(self) + if data.get("versions") is None: + data.pop("versions", None) + return data @model_validator(mode="after") def _database_attribution(self) -> "FrameSpec": @@ -591,6 +605,8 @@ class FrameVerdict: added_by_others: set[tuple[str, str]] = field(default_factory=set) #: In memory only: app profile rules that excused a row change. excused_by: set[str] = field(default_factory=set) + #: ``frame.versions`` only: how the target's versions came out. + versions: Optional[VersionVerdict] = None #: In memory only, set when the frame held and ``recheck_after_s`` is #: set: what a later recheck compares with (the settled post-save #: snapshot as keyed MACs, the audit entries seen then as keyed hashes, @@ -621,6 +637,11 @@ def evidence(self, spec: FrameSpec) -> dict[str, Any]: 1 for c in self.collateral if c.other_on_target ), **({"database": self.database} if self.database is not None else {}), + **( + {"versions": self.versions.evidence()} + if self.versions is not None + else {} + ), } diff --git a/packages/flow/openadapt_flow/runtime/effects/versions.py b/packages/flow/openadapt_flow/runtime/effects/versions.py new file mode 100644 index 0000000000..01e5a91bb4 --- /dev/null +++ b/packages/flow/openadapt_flow/runtime/effects/versions.py @@ -0,0 +1,371 @@ +"""Version history of the target: catch a lost update. + +Two snapshots can't see a lost update. The agent reads a record, another +user saves a change to it, and the agent saves its stale copy over theirs. +The record after the save looks like "the agent changed one field, the rest +is unchanged", which is what a correct update looks like too. Only the +record's versions show the second writer. + +``frame.versions`` names a read-only read that returns one row per version of +a record (FHIR ``_history`` or HAPI's ``hfj_res_ver``; a SQL ``version`` +column; the app's audit or action-history table), and which fields of a row +name the record, the version and, where the app records one, its author. +The verifier reads it before the write and after it. For every target record +that already existed (one a confirmed write effect names), the versions that +are new in the window must be the agent's own: + +- With ``author_field``: every new version's author must be one of ``self``. + A version by anyone else is another user's write inside the window: the + frame is refuted. A version with no author can't be attributed: not + checked. +- Without it: versions must be whole numbers that step by one per save + (HAPI's ``res_ver``, a ``version_number`` column), and exactly one step may + have happened (zero when nothing was written). Two steps mean someone else + saved too: refuted. A version that isn't a whole number: not checked. + +Anything the reads can't explain stops the run: a read that fails, a target +that existed but has no version, or one whose versions vanished. Never a +pass. Versions are stored in the baseline as keyed MACs of the record and the +version; only a whole-number version's value is kept, to count the steps. +""" + +from __future__ import annotations + +import hashlib +import re +from collections.abc import Mapping, Sequence +from dataclasses import dataclass, field +from typing import Any, Literal, Optional + +from pydantic import BaseModel, ConfigDict, Field, model_validator + +from openadapt_flow.runtime.effects.effect import ValueExpr + +VersionStatus = Literal["held", "refuted", "not_checked"] + +#: Why the version check stopped the run (closed values, for the evidence). +PROBLEMS = ( + "unreadable", + "no_target_id", + "missing", + "another_author", + "no_author", + "not_a_number", + "extra_steps", + "went_back", + "self_unresolved", + "null_version", + "duplicate_version", + "no_new_version", + "history_changed", +) + +_WHOLE = re.compile(r"^(0|[1-9][0-9]*)$") + + +class FrameVersions(BaseModel): + """``frame.versions``: where the target's versions are read.""" + + model_config = ConfigDict(extra="forbid", populate_by_name=True) + + #: A read-only read, the same shape as an ``effects:`` verifier section, + #: that returns one record per version (built by ``verify-run``). + read: dict[str, Any] + #: The version row's field naming the record it is a version of. + record_field: str = Field(min_length=1) + #: The target record's field (in the contract's read set) that equals + #: ``record_field``. Unset: ``frame.identity_field``, else ``id``. + target_field: Optional[str] = None + #: The version row's version id (``meta.versionId``, ``res_ver``, an + #: audit entry's id). + version_field: str = Field(min_length=1) + #: The version row's author, when the app records who saved each + #: version. Unset: versions must be whole numbers that step by one. + author_field: Optional[str] = None + #: The agent's own user ids, as the author field names them. Literals or + #: run parameters. Required with ``author_field``. + self_actors: list[ValueExpr] = Field(default_factory=list, alias="self") + + @model_validator(mode="before") + @classmethod + def _coerce_self(cls, data: Any) -> Any: + if isinstance(data, dict): + key = "self" if "self" in data else "self_actors" + values = data.get(key) + if isinstance(values, list): + data = dict(data) + data[key] = [ + ValueExpr(literal=v) if isinstance(v, str) else v for v in values + ] + return data + + @model_validator(mode="after") + def _shape(self) -> "FrameVersions": + if bool(self.author_field) != bool(self.self_actors): + raise ValueError( + "frame.versions: author_field and self go together (whose " + "versions are the agent's own)" + ) + return self + + +@dataclass +class VersionVerdict: + status: VersionStatus + #: One of :data:`PROBLEMS`, or None when the check held. + problem: Optional[str] = None + targets: int = 0 + #: Versions new in the window, summed over the targets. + new_versions: int = 0 + #: Of those, the agent's own (by author), when authors are read. + own_versions: int = 0 + #: In memory only: the reason line. + reason: str = "" + extra: dict[str, Any] = field(default_factory=dict) + + def evidence(self) -> dict[str, Any]: + """Closed fields only: counts and a problem code, no ids or values.""" + + return { + "status": self.status, + "problem": self.problem, + "targets": self.targets, + "new_versions": self.new_versions, + "own_versions": self.own_versions, + } + + +def _get(row: Mapping[str, Any], path: str) -> Any: + if path in row: + return row[path] + node: Any = row + for part in path.split("."): + if not isinstance(node, Mapping) or part not in node: + return None + node = node[part] + return node + + +def _text(value: Any) -> Optional[str]: + if value is None: + return None + text = str(value) + return text if text != "" else None + + +def _mac(key: bytes, label: str, *parts: str) -> str: + body = "\x1f".join((label, *parts)).encode("utf-8") + return hashlib.blake2b(body, key=key[:64], digest_size=16).hexdigest() + + +def seal_versions( + spec: FrameVersions, rows: Optional[Sequence[Mapping[str, Any]]], key: bytes +) -> dict[str, Any]: + """The baseline's record of versions: per record MAC, the version MACs + and (whole numbers only) the highest version.""" + + if rows is None: + return {"reachable": False} + records: dict[str, dict[str, Any]] = {} + for row in rows: + record = _text(_get(row, spec.record_field)) + version = _text(_get(row, spec.version_field)) + if record is None: + continue # a row that names no record is no record's version + entry = records.setdefault( + _mac(key, "record", record), + {"versions": [], "max": None, "whole": True, "null": False, "dup": False}, + ) + if version is None: + # A null version can't tell two saves apart. + entry["whole"] = False + entry["null"] = True + continue + mac = _mac(key, "version", record, version) + if mac in entry["versions"]: + entry["dup"] = True # two rows, one version: a save could hide + entry["versions"].append(mac) + if _WHOLE.match(version): + entry["max"] = max(int(version), entry["max"] or 0) + else: + entry["whole"] = False + for entry in records.values(): + entry["versions"] = sorted(set(entry["versions"])) + return {"reachable": True, "records": records} + + +def _resolve_self( + spec: FrameVersions, params: Mapping[str, object] +) -> Optional[set[str]]: + out: set[str] = set() + for expr in spec.self_actors: + value = expr.resolve(params) + if value is None or value == "": + return None + out.add(str(value)) + return out + + +def judge_versions( + spec: FrameVersions, + stored: Mapping[str, Any], + rows: Optional[Sequence[Mapping[str, Any]]], + targets: Sequence[tuple[Optional[str], bool, bool]], + key: bytes, + params: Mapping[str, object] = {}, +) -> VersionVerdict: + """Judge the targets' versions in the window. + + ``stored``: :func:`seal_versions` of the baseline read. ``rows``: the read + after the save (None when it failed). ``targets``: per record a confirmed + write effect matched, its ``target_field`` value (None when the record + has none), whether it existed before the write, and whether the record + changed in the window. + """ + + if not stored.get("reachable") or rows is None: + return VersionVerdict( + "not_checked", + "unreadable", + reason="the target's versions could not be read, before or after the write", + ) + mine: set[str] = set() + if spec.author_field: + resolved = _resolve_self(spec, params) + if resolved is None: + return VersionVerdict( + "not_checked", + "self_unresolved", + reason="frame.versions.self references a parameter the run did not supply", + ) + mine = resolved + before: Mapping[str, Any] = stored.get("records") or {} + out = VersionVerdict("held") + worst: Optional[VersionVerdict] = None + + def stop(status: VersionStatus, problem: str, reason: str) -> None: + nonlocal worst + if worst is None or (worst.status == "not_checked" and status == "refuted"): + worst = VersionVerdict(status, problem, reason=reason) + + seen: set[Optional[str]] = set() + for target, existed, changed in targets: + if target in seen: + continue + seen.add(target) + if target is None: + stop( + "not_checked", + "no_target_id", + f"a target record has no {spec.target_field or 'identity'} field, " + "so its versions can't be found", + ) + continue + base = before.get(_mac(key, "record", target)) + if base is None: + if existed: + stop( + "not_checked", + "missing", + "the target existed before the write but had no version", + ) + continue # created in the window: no earlier version to overwrite + out.targets += 1 + now = [r for r in rows if _text(_get(r, spec.record_field)) == target] + if not now: + stop("not_checked", "missing", "the target has no version after the write") + continue + old = set(base.get("versions") or []) + versions = [_text(_get(r, spec.version_field)) for r in now] + if base.get("null") or any(v is None for v in versions): + stop( + "not_checked", + "null_version", + "a version of the target reads as null, so saves can't be told apart", + ) + continue + macs = [_mac(key, "version", target, str(v)) for v in versions] + if base.get("dup") or len(set(macs)) != len(macs): + stop( + "not_checked", + "duplicate_version", + "two rows of the target's versions hold the same version, so a " + "save could hide behind another", + ) + continue + fresh = [(v, r) for v, r, m in zip(versions, now, macs) if m not in old] + out.new_versions += len(fresh) + if changed and not fresh: + stop( + "not_checked", + "no_new_version", + "the target changed in the window but no new version of it was " + "read, so who saved it can't be told", + ) + continue + if spec.author_field and not old <= set(macs): + stop( + "not_checked", + "history_changed", + "a version of the target read before the write is gone after it: " + "the read isn't a history that only grows, so a save in the " + "window could be hidden", + ) + continue + if spec.author_field: + for _version, r in fresh: + author = _text(_get(r, spec.author_field)) + if author is None: + stop( + "not_checked", + "no_author", + "a new version of the target names no author, so it " + "can't be told from another user's save", + ) + elif author in mine: + out.own_versions += 1 + else: + stop( + "refuted", + "another_author", + "lost update: another user saved a version of the target " + "inside the window, and the agent's save came after it " + "or beside it", + ) + continue + # No authors: count whole-number steps. + values = [_text(_get(r, spec.version_field)) for r in now] + if not base.get("whole") or any( + v is None or not _WHOLE.match(v) for v in values + ): + stop( + "not_checked", + "not_a_number", + "the target's versions aren't whole numbers, and no author is " + "read, so the steps in the window can't be counted", + ) + continue + steps = max(int(v) for v in values if v is not None) - int(base["max"] or 0) + if steps < 0 or (steps == 0 and fresh): + stop( + "refuted", + "went_back", + "the target's version went back or was replaced in the window", + ) + elif steps > 1 or len(fresh) > 1: + stop( + "refuted", + "extra_steps", + f"lost update: the target moved {max(steps, len(fresh))} versions " + "in the window; one save is the agent's, and no author is read " + "to say whose the others are", + ) + if worst is not None: + worst.targets, worst.new_versions, worst.own_versions = ( + out.targets, + out.new_versions, + out.own_versions, + ) + return worst + out.reason = f"{out.targets} target(s): every new version is the agent's own" + return out diff --git a/packages/flow/openadapt_flow/verifier.py b/packages/flow/openadapt_flow/verifier.py index aa1043a62d..6d447e82aa 100644 --- a/packages/flow/openadapt_flow/verifier.py +++ b/packages/flow/openadapt_flow/verifier.py @@ -1563,6 +1563,30 @@ def _scope( ) +#: ``frame.versions`` problems (``runtime/effects/versions.py``), as reasons. +_VERSION_REASONS = { + "another_author": "lost update: another user saved a version of the target " + "inside the window, so the agent's save may have overwritten it", + "extra_steps": "lost update: the target moved more than one version in the " + "window, and only one save is the agent's", + "went_back": "the target's version went back or was replaced in the window", + "no_author": "a new version of the target names no author, so it can't be " + "told from another user's save", + "not_a_number": "the target's versions aren't whole numbers and no author " + "is read, so the saves in the window can't be counted", + "missing": "the target existed before the write but has no version to compare", + "no_target_id": "a target record has no field naming it in the versions read", + "unreadable": "the target's versions could not be read, before or after the write", + "self_unresolved": "frame.versions.self names a parameter the run did not supply", + "null_version": "a version of the target reads as null, so saves can't be told apart", + "duplicate_version": "two rows of the target's versions hold the same version, so a " + "save could hide behind another", + "no_new_version": "the target changed in the window but no new version of it was read", + "history_changed": "a version of the target read before the write is gone after it, " + "so a save in the window could be hidden", +} + + def _reasons(evidence: Mapping[str, Any], receipt_outcome: str) -> list[str]: out: list[str] = [] for row in (evidence.get("readback") or {}).get("effects") or []: @@ -1574,14 +1598,20 @@ def _reasons(evidence: Mapping[str, Any], receipt_outcome: str) -> list[str]: f"{row.get('expected_count')} expected record(s)" ) frame = evidence.get("frame") or {} - if frame.get("status") == "refuted": + versions = frame.get("versions") or {} + problem = versions.get("problem") if isinstance(versions, Mapping) else None + if problem: + out.append( + _VERSION_REASONS.get(str(problem), f"the target's versions: {problem}") + ) + if frame.get("status") == "refuted" and frame.get("collateral_count", 0): kinds = ", ".join(frame.get("collateral_kinds") or []) out.append( f"{frame.get('collateral_count', 0)} record(s) outside the declared " f"write moved ({kinds}); " f"{frame.get('unattributed_count', 0)} of them no actor explains" ) - elif frame.get("status") == "not_checked": + elif frame.get("status") == "not_checked" and not problem: out.append("what else changed could not be read, so the frame is not checked") source = evidence.get("source") or {} if isinstance(source, Mapping) and source.get("status") == "refuted": diff --git a/packages/flow/openadapt_flow/verify_run.py b/packages/flow/openadapt_flow/verify_run.py index dd8821dff3..3de042de3f 100644 --- a/packages/flow/openadapt_flow/verify_run.py +++ b/packages/flow/openadapt_flow/verify_run.py @@ -152,6 +152,9 @@ FRAME_DB_DETAIL = "openadapt:frame_database" #: Why the whole-database snapshot couldn't be read, when it couldn't. FRAME_DB_ERROR = "openadapt:frame_database_error" +#: Where the baseline keeps the target versions read (``frame.versions``), as +#: keyed MACs. +FRAME_VERSIONS_DETAIL = "openadapt:frame_versions" #: Where the baseline keeps the read set as keyed MACs (``effects/sealed.py``). SEALED_DETAIL = "openadapt:sealed_records" @@ -431,6 +434,97 @@ def _audit_verifier(cfg: VerifyConfig, params: Mapping[str, object]) -> Any: raise VerifyRunError(f"cannot build the audit read: {exc}") from exc +def _read_versions( + cfg: VerifyConfig, params: Mapping[str, object] +) -> Optional[list[dict[str, Any]]]: + """The ``frame.versions`` read: one record per version, or None when it + can't be read.""" + + spec = cfg.frame.versions if cfg.frame is not None else None + if spec is None: + return None + try: + reader = build_effect_verifier(EffectsConfig.model_validate(spec.read), params) + except ValueError as exc: + raise VerifyRunError(f"cannot build the versions read: {exc}") from exc + try: + state = reader.capture_pre_state() if reader is not None else None + except Exception: # noqa: BLE001 - unreadable -> the check stops the run + return None + if not isinstance(state, EffectState) or not state.reachable: + return None + return [dict(r) for r in state.records] + + +def _judge_versions( + cfg: VerifyConfig, + frame: FrameVerdict, + before: EffectState, + effects: Sequence[Effect], + verdicts: Sequence[EffectVerdict], + params: Mapping[str, object], + salt: bytes, +) -> FrameVerdict: + """Add ``frame.versions`` to the frame: a version of a target in the + window that isn't the agent's own refutes it; one that can't be read or + attributed leaves it not checked.""" + + from openadapt_flow.runtime.effects.versions import judge_versions + + spec = cfg.frame.versions if cfg.frame is not None else None + if spec is None: + return frame + field_name = spec.target_field or cfg.frame.identity_field or "id" # type: ignore[union-attr] + # Whether a target existed is told by its identity, which a save doesn't + # change, never by the target field's value after the write: a target + # field the write changed would make an old record look created. + identity = cfg.frame.identity_field or "id" # type: ignore[union-attr] + old: dict[str, dict[str, Any]] = {} + for r in before.records: + if r.get(identity) not in (None, ""): + old.setdefault(str(r.get(identity)), dict(r)) + ignored = set(cfg.frame.ignore_fields) # type: ignore[union-attr] + targets: list[tuple[Optional[str], bool, bool]] = [] + for effect, verdict in zip(effects, verdicts): + if effect.kind not in (EffectKind.RECORD_WRITTEN, EffectKind.FIELD_EQUALS): + continue + if effect.kind is EffectKind.RECORD_WRITTEN and effect.expected_count == 0: + continue # a guard: "nothing here" + if not verdict.confirmed: + continue + for record in verdict.matched_records: + value = record.get(field_name) + text = None if value in (None, "") else str(value) + key = record.get(identity) + was = None if key in (None, "") else old.get(str(key)) + # No identity to look up: never assume the record is new. + existed = was is not None or key in (None, "") + # Changed in the window: a field read before differs now (a + # sealed baseline marks a changed field, so it differs too). + changed = existed and ( + was is None + or any(record.get(k) != v for k, v in was.items() if k not in ignored) + ) + targets.append((text, existed, changed)) + stored = (before.detail or {}).get(FRAME_VERSIONS_DETAIL) + result = judge_versions( + spec, + stored if isinstance(stored, Mapping) else {}, + _read_versions(cfg, params), + targets, + _mac_key(salt), + params, + ) + frame.versions = result + if result.status == "refuted": + frame.status = "refuted" + frame.reason = result.reason + elif result.status == "not_checked" and frame.status == "held": + frame.status = "not_checked" + frame.reason = result.reason + return frame + + def _db_connect(read: Mapping[str, Any]) -> Any: """A connect function for ``frame.database.read`` (read-only).""" @@ -1285,6 +1379,13 @@ def capture_baseline( execution_id = _new_execution_id() _tag_reads(verifier, execution_id) salt = secrets.token_bytes(32) + # The versions first: a save between this read and the record read is + # then in the record baseline too, never hidden in the versions one. + versions_rows = ( + _read_versions(cfg, params) + if cfg.frame is not None and cfg.frame.versions is not None + else None + ) state = verifier.capture_pre_state() if not isinstance(state, EffectState): raise VerifyRunError("verifier returned no readable baseline") @@ -1308,6 +1409,12 @@ def capture_baseline( "reachable": entries is not None, "ids": _audit_ids(entries or [], salt), } + if cfg.frame is not None and cfg.frame.versions is not None: + from openadapt_flow.runtime.effects.versions import seal_versions + + detail[FRAME_VERSIONS_DETAIL] = seal_versions( + cfg.frame.versions, versions_rows, _mac_key(salt) + ) if detail: state = state.model_copy(update={"detail": {**state.detail, **detail}}) state = _seal_state(cfg, state, salt) @@ -1738,6 +1845,9 @@ def verify_run( run_actors=[str(run_record.get("executor_id") or "")], salt=salt, ) + frame = _judge_versions( + cfg, frame, before, bound_effects, verdicts, params, salt + ) else: frame = FrameVerdict( status="not_checked", diff --git a/packages/flow/tests/test_frame_versions.py b/packages/flow/tests/test_frame_versions.py new file mode 100644 index 0000000000..56b0fca0be --- /dev/null +++ b/packages/flow/tests/test_frame_versions.py @@ -0,0 +1,347 @@ +"""``frame.versions``: a lost update stops the run. + +The agent reads a record, another user saves a change, and the agent saves +its stale copy over it. Before and after look like a correct update; only the +record's versions show the second save. Each test runs ``verify_run`` on a +real SQLite database whose app keeps one version row per save (HAPI's +``hfj_res_ver`` shape: record, version number and, in some apps, the author). +Records are made up. +""" + +from __future__ import annotations + +import sqlite3 +from pathlib import Path +from typing import Any, Callable, Optional + +import pytest + +from openadapt_flow.runtime.effects.frame import FrameSpec +from openadapt_flow.verifier import _reasons +from openadapt_flow.verify_run import capture_baseline, parse_verify_config, verify_run + +SCHEMA = """ +CREATE TABLE person (id INTEGER PRIMARY KEY, name TEXT, phone TEXT, addr TEXT, + ver INTEGER); +CREATE TABLE person_version (id INTEGER PRIMARY KEY AUTOINCREMENT, + person_id INTEGER, ver TEXT, author TEXT); +INSERT INTO person VALUES (1, 'Maria Alvarez', '555-0100', '1 Elm St', 1), + (2, 'Yuki Tanaka', '555-0200', '2 Oak St', 1); +INSERT INTO person_version (person_id, ver, author) VALUES (1, '1', 'admin'), + (2, '1', 'admin'); +""" +PARAMS = {"phone": "555-0199", "bot": "bot"} + + +def _save( + con: sqlite3.Connection, pid: int, author: Optional[str], **values: str +) -> None: + """The app's save: the row, its version number, one version row.""" + + sets = ", ".join(f"{k} = ?" for k in values) + con.execute( + f"UPDATE person SET {sets}, ver = ver + 1 WHERE id = ?", [*values.values(), pid] + ) + (ver,) = con.execute("SELECT ver FROM person WHERE id = ?", [pid]).fetchone() + con.execute( + "INSERT INTO person_version (person_id, ver, author) VALUES (?, ?, ?)", + [pid, str(ver), author], + ) + + +def _doc(db: Path, versions: dict[str, Any]) -> dict[str, Any]: + return { + "schema_version": "openadapt.verify/v1", + "verifier": { + "kind": "sql", + "sqlite_database": str(db), + "sql_query": "SELECT * FROM person", + }, + "effects": [ + { + "kind": "record_written", + "match": {"id": "1", "phone": {"param": "phone"}}, + "expected_count": 1, + } + ], + "frame": { + "identity_field": "id", + "target_fields": ["phone", "ver"], + "versions": { + "read": { + "kind": "sql", + "sqlite_database": str(db), + "sql_query": "SELECT person_id, ver, author FROM person_version", + }, + "record_field": "person_id", + "version_field": "ver", + **versions, + }, + }, + } + + +AUTHORS = {"author_field": "author", "self": [{"param": "bot"}]} + + +def _run( + tmp_path: Path, + window: Callable[[sqlite3.Connection], None], + versions: Optional[dict[str, Any]] = None, + setup: Callable[[sqlite3.Connection], None] = lambda con: None, +) -> dict[str, Any]: + tmp_path.mkdir(parents=True, exist_ok=True) + db = tmp_path / "app.sqlite" + con = sqlite3.connect(db) + con.executescript(SCHEMA) + setup(con) + con.commit() + cfg = parse_verify_config(_doc(db, {} if versions is None else versions)) + capture_baseline(cfg, PARAMS, tmp_path / "baseline.json") + window(con) + con.commit() + con.close() + return verify_run( + cfg, + params=PARAMS, + run_record={"executor_kind": "agent", "executor_id": "bot"}, + authorization_ref="test", + baseline_path=tmp_path / "baseline.json", + ) + + +def _versions(result: dict[str, Any]) -> dict[str, Any]: + return result["evidence"]["frame"]["versions"] + + +def _correct(con: sqlite3.Connection) -> None: + _save(con, 1, "bot", phone="555-0199") + + +def _lost_update(con: sqlite3.Connection) -> None: + # The front desk saves a new address; the bot saves its stale copy + # (the old address) with the new phone, reverting the address. + _save(con, 1, "frontdesk", addr="9 New Rd") + _save(con, 1, "bot", phone="555-0199", addr="1 Elm St") + + +@pytest.mark.parametrize("versions", [None, AUTHORS], ids=["steps", "authors"]) +def test_a_correct_update_is_verified(tmp_path: Path, versions: Any) -> None: + result = _run(tmp_path, _correct, versions) + assert result["receipt"]["outcome"] == "verified" + assert _versions(result)["status"] == "held" + assert _versions(result)["new_versions"] == 1 + + +def test_a_lost_update_without_authors_is_refuted_by_its_steps(tmp_path: Path) -> None: + result = _run(tmp_path, _lost_update) + assert result["receipt"]["checked"]["frame"] == "refuted" + assert result["receipt"]["outcome"] == "reconciliation_required" + assert _versions(result)["problem"] == "extra_steps" + assert any("lost update" in r for r in _reasons(result["evidence"], "x")) + + +def test_a_lost_update_with_authors_is_refuted_by_the_author(tmp_path: Path) -> None: + result = _run(tmp_path, _lost_update, AUTHORS) + assert result["receipt"]["checked"]["frame"] == "refuted" + assert _versions(result)["problem"] == "another_author" + assert _versions(result)["own_versions"] == 1 + + +def test_a_current_version_only_read_still_counts_the_steps(tmp_path: Path) -> None: + # The read shows only the record's current version (a version column). + result = _run( + tmp_path, + _lost_update, + {"read": {"kind": "sql", "sqlite_database": str(tmp_path / "app.sqlite"), + "sql_query": "SELECT id AS person_id, ver FROM person"}}, + ) # fmt: skip + assert _versions(result)["problem"] == "extra_steps" + + +def test_another_users_edit_of_an_unrelated_record_is_not_a_lost_update( + tmp_path: Path, +) -> None: + def window(con: sqlite3.Connection) -> None: + _save(con, 2, "frontdesk", phone="555-0222") + _correct(con) + + for versions in (None, AUTHORS): + result = _run(tmp_path / str(bool(versions)), window, versions) + # Only the target's versions are judged. (The frame has no + # attribution here, so record 2's change refutes it as collateral, + # as before: the version check adds nothing to that.) + assert _versions(result)["status"] == "held" + assert _versions(result)["problem"] is None + + +def test_a_version_with_no_author_is_not_checked(tmp_path: Path) -> None: + result = _run(tmp_path, lambda con: _save(con, 1, None, phone="555-0199"), AUTHORS) + assert result["receipt"]["checked"]["frame"] == "not_checked" + assert result["receipt"]["outcome"] != "verified" + assert _versions(result)["problem"] == "no_author" + + +def test_a_version_that_isnt_a_number_is_not_checked(tmp_path: Path) -> None: + def window(con: sqlite3.Connection) -> None: + _correct(con) + con.execute("UPDATE person_version SET ver = 'W/\"2\"' WHERE ver = '2'") + + result = _run(tmp_path, window) + assert _versions(result)["problem"] == "not_a_number" + assert result["receipt"]["outcome"] != "verified" + + +def test_an_unreadable_versions_read_is_not_checked(tmp_path: Path) -> None: + def window(con: sqlite3.Connection) -> None: + _correct(con) + con.execute("ALTER TABLE person_version RENAME TO gone") + + result = _run(tmp_path, window) + assert _versions(result)["problem"] == "unreadable" + assert result["receipt"]["outcome"] != "verified" + + +def test_a_target_with_no_version_is_not_checked(tmp_path: Path) -> None: + result = _run( + tmp_path, + _correct, + setup=lambda con: con.execute("DELETE FROM person_version WHERE person_id = 1"), + ) + # The bot's save writes version 2, but the baseline had none for it. + assert _versions(result)["problem"] == "missing" + assert result["receipt"]["outcome"] != "verified" + + +def test_versions_that_go_back_are_refuted(tmp_path: Path) -> None: + def window(con: sqlite3.Connection) -> None: + con.execute("UPDATE person SET phone = '555-0199' WHERE id = 1") + con.execute("UPDATE person_version SET ver = '0' WHERE person_id = 1") + + result = _run(tmp_path, window) + assert _versions(result)["problem"] == "went_back" + + +def test_author_and_self_go_together() -> None: + with pytest.raises(ValueError, match="author_field and self go together"): + FrameSpec.model_validate( + {"versions": {"read": {}, "record_field": "a", "version_field": "b", + "author_field": "c"}} + ) # fmt: skip + + +def test_a_frame_without_versions_keeps_its_digest() -> None: + spec = FrameSpec.model_validate({"identity_field": "id"}) + assert "versions" not in spec.model_dump(mode="json", by_alias=True) + + +# -- review round 1 (#1662): each attack ends in a stop ------------------------ + + +def test_r1_a1_authors_with_a_current_version_only_read_is_not_checked( + tmp_path: Path, +) -> None: + # The read shows only the record's current version and its last author: + # the front desk's version is gone after the write. + db = tmp_path / "app.sqlite" + read = { + "kind": "sql", + "sqlite_database": str(db), + "sql_query": "SELECT p.id AS person_id, p.ver AS ver, (SELECT author FROM " + "person_version v WHERE v.person_id = p.id ORDER BY v.id DESC LIMIT 1) " + "AS author FROM person p", + } + result = _run(tmp_path, _lost_update, {**AUTHORS, "read": read}) + assert result["receipt"]["outcome"] != "verified" + assert _versions(result)["problem"] == "history_changed" + + +@pytest.mark.parametrize("versions", [AUTHORS, {}], ids=["authors", "steps"]) +def test_r1_a2_a_null_version_is_not_checked(tmp_path: Path, versions: Any) -> None: + result = _run(tmp_path, _lost_update, {**versions, "version_field": "version"}) + assert result["receipt"]["outcome"] != "verified" + assert _versions(result)["problem"] == "null_version" + + +def test_r1_a3_a_duplicate_version_is_not_checked(tmp_path: Path) -> None: + def window(con: sqlite3.Connection) -> None: + _lost_update(con) + con.execute("UPDATE person_version SET ver = '1' WHERE author = 'frontdesk'") + + result = _run(tmp_path, window, AUTHORS) + assert result["receipt"]["outcome"] != "verified" + assert _versions(result)["problem"] == "duplicate_version" + + +def test_r1_a4_versions_are_read_before_the_records( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + import openadapt_flow.verify_run as vr + + real = vr._read_versions + calls = {"n": 0} + + def wrapped(cfg: Any, params: Any) -> Any: + calls["n"] += 1 + rows = real(cfg, params) + if calls["n"] == 1: # another user saves between the two baseline reads + con = sqlite3.connect(tmp_path / "app.sqlite") + _save(con, 1, "frontdesk", addr="9 New Rd") + con.commit() + con.close() + return rows + + monkeypatch.setattr(vr, "_read_versions", wrapped) + result = _run( + tmp_path, lambda con: _save(con, 1, "bot", phone="555-0199", addr="1 Elm St") + ) + assert result["receipt"]["outcome"] != "verified" + + +@pytest.mark.parametrize("versions", [AUTHORS, {}], ids=["authors", "steps"]) +def test_r1_a_changed_target_with_no_new_version_is_not_checked( + tmp_path: Path, versions: Any +) -> None: + # The app saved without writing a version row. + result = _run( + tmp_path, + lambda con: con.execute("UPDATE person SET phone = '555-0199' WHERE id = 1"), + versions, + ) + assert result["receipt"]["outcome"] != "verified" + assert _versions(result)["problem"] == "no_new_version" + + +def test_r1_an_unchanged_target_with_no_new_version_holds(tmp_path: Path) -> None: + # Already right, nothing written: no save could overwrite anything. + def setup(con: sqlite3.Connection) -> None: + con.execute("UPDATE person SET phone = '555-0199' WHERE id = 1") + + result = _run(tmp_path, lambda con: None, AUTHORS, setup=setup) + assert result["receipt"]["outcome"] == "verified" + assert _versions(result)["new_versions"] == 0 + + +# -- review round 2 (#1662) ------------------------------------------------------ + + +def test_r2_i2_a_target_field_the_write_changes_is_not_a_new_record( + tmp_path: Path, +) -> None: + # The versions are keyed by the phone, which the agent's write changes: + # the record existed (by its identity), so no baseline under the new + # phone is a missing version, not a record created in the window. + db = tmp_path / "app.sqlite" + read = { + "kind": "sql", + "sqlite_database": str(db), + "sql_query": "SELECT p.phone AS phone, v.ver FROM person_version v " + "JOIN person p ON p.id = v.person_id", + } + result = _run( + tmp_path, + _lost_update, + {"target_field": "phone", "record_field": "phone", "read": read}, + ) + assert result["receipt"]["outcome"] != "verified" + assert _versions(result)["problem"] == "missing"