From a1f0e5f323a8799e75e5644f5371a31e74a32234 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Thu, 8 Oct 2026 16:43:56 +0000 Subject: [PATCH 01/12] WIP T042 (plan 038, HA-1): 0003_health.sql, runtime/health_store.py and the falsifier draft, parked Parked for an account swap; not yet run against the store. Still to do: identity.TABLES, cli.DROP_ORDER and the reset wording, the two init scripts, docs/runtime.md, and the five tests_runtime suites. Refs opensoft/openxFactory#656. Lane: openxfactory-4 Co-Authored-By: Claude Opus 5.5 (1M context) --- migrations/0003_health.sql | 127 ++++++ src/opendox/runtime/health_store.py | 458 ++++++++++++++++++++ tests_runtime/test_health_store.py | 646 ++++++++++++++++++++++++++++ 3 files changed, 1231 insertions(+) create mode 100644 migrations/0003_health.sql create mode 100644 src/opendox/runtime/health_store.py create mode 100644 tests_runtime/test_health_store.py diff --git a/migrations/0003_health.sql b/migrations/0003_health.sql new file mode 100644 index 00000000..069d9eed --- /dev/null +++ b/migrations/0003_health.sql @@ -0,0 +1,127 @@ +-- 0003 — the health store: one table of runs and one of findings, ADDITIVE. +-- +-- WHY `0003_` AND NOT `0002_` (#1144 box 14.1): `0002_migration_state.sql` +-- is the ledger, and `0001` is applied verbatim behind +-- `CANONICAL_MIGRATION_SHA256`, so its header's own rule applies: "Changing +-- the schema is therefore an ADDITIVE `0002_…`/`0003_…` file, never an edit +-- here". `0001`'s header still names six tables, and it stays as written, +-- because its bytes are the pin. +-- +-- DOMAIN TABLES, DECLARED IN THE OPEN (#1144 box 14.2; R2Q13 (a), Brett Heap, +-- opensoft/openxFactory#656 comment 6003486656). Ruling 5784155201 item 4 +-- reads "health results become a seventh table … with the closure test moved +-- in the SAME change", and that is this change: `identity.TABLES` names both +-- tables below, `tests_runtime/test_schema_shape.py` reads `0001` with this +-- file, and `verify_runtime_access` measures the served role's rights on them +-- exactly as it does on the other six (`SERVED_TABLES` is `identity.TABLES`). +-- +-- NO DOCUMENT, AND THE STORE STAYS DISPOSABLE (box 14.3; RULING Q1's +-- principle; R2Q25 (a)). A finding ADDRESSES a document (`path`, a display +-- `locator`) and never carries its text: there is NO patch column (lane 3's +-- MISLABEL row 31; OQ-H15-20 as refined: a pack's patch is re-obtained at +-- `fix`, never stored), and `evidence` holds locators only. Every row here is +-- recomputable from git, so `runtime reset` drops these tables with the six +-- and a reset costs a recomputation (`runtime/cli.py`, `DROP_ORDER`). +-- +-- PROVENANCE IS NOT NULL FROM THE FIRST LANDING (box 15.7): `pack_id` and +-- `pack_version` are `not null`, and not empty, so no path can store a finding +-- without them. That is enforced HERE, at the store, and not only in the +-- engine that stamps them. +-- +-- NO FOREIGN KEY TO `projects` (OQ-H-22): a run is keyed by the corpus's +-- resolved root, because F14.1 runs `health` over a corpus no project row +-- names. +-- +-- THE COLUMN `full_run` IS data-model.md's FIELD `full`. PostgreSQL 16 lists +-- `full` as a TYPE_FUNC_NAME_KEYWORD, which a column definition's name cannot +-- be; a quoted `"full"` would be legal SQL and a trap in every statement that +-- forgot the quotes. Holder ruling, opensoft/openxFactory#656 comment +-- 6064169640: the model's field stays `full`, its SQL column is `full_run`. +-- +-- RUNS ARE ORDERED BY `run_seq`, NOT BY A CLOCK. The baseline is "the latest +-- earlier default-tip run" of a corpus (R2Q12 (a)), and `0001`'s header +-- records why this host's ordering must not be wall time: the clock "can step +-- backwards under load". `run_id` is a uuid (data-model.md) and so orders +-- nothing; `run_seq` is an identity column the server hands out at insert, so +-- the order is the order runs were recorded in. `started_at` and +-- `finished_at` are what the run reports, for display, and deliberately carry +-- no ordering constraint for the same reason. +-- +-- `identity` IS TEXT HOLDING CANONICAL SORTED-KEY JSON (the holder, +-- opensoft/openxFactory#656 comment 6018624750): the position-independent key +-- the engine hashes into `id`, stored byte for byte as it was hashed and +-- emitted by `health list --json`. `jsonb` would re-order and re-space it. +-- Its serialized-size cap is the engine's, T041's, in the contract module, +-- and deliberately NOT a constraint here: the store stores what the contract +-- bounds. Likewise every other field bound (the id grammar, `message`'s 200 +-- characters, a key named `excerpt`) belongs to the contract module; this file +-- holds the closed vocabularies and the shapes the data model fixes. +-- +-- A COMMIT IS 40 LOWERCASE HEX DIGITS, held by ONE check. `commit` is null +-- exactly for a working-state run, and otherwise equals `export_commit` +-- (data-model.md: "`commit` for a default-tip or branch run"), so the +-- `export_commit` form check holds `commit` too; a second check on `commit` +-- could never be the one that refuses a row. +-- +-- EVERY STORED FINDING CARRIES ITS BASELINE CLASS (`baseline_class not +-- null`). R2Q12 (a) rules exactly three classes and I-2 (a) rules out a +-- fourth, and a null would be that fourth ("unclassed") by another name: the +-- engine classes a run's findings against the store before it records them, +-- so a row without a class is a row the engine never classed. +-- +-- THE HOSTED PLANE MIGRATES THIS FILE TOO (R2Q15 (a)): there is one +-- migration set. The hosted plane refuses the `health` verbs and routes by +-- name and records nothing, so its tables stay empty. + +create table health_runs ( + run_id uuid primary key, + run_seq bigint not null generated always as identity, + corpus_root text not null, + kind text not null, + commit text, + baseline_branch text, + started_at timestamptz not null, + finished_at timestamptz not null, + outcome text not null, + pack_pins jsonb not null, + export_commit text not null, + full_run boolean not null, + sandbox jsonb not null, + constraint health_runs_run_seq_key unique (run_seq), + constraint health_runs_kind_check check (kind in ('default-tip', 'branch', 'working-state')), + constraint health_runs_outcome_check check (outcome in ('complete', 'partial', 'refused')), + constraint health_runs_commit_check check ((kind = 'working-state') = (commit is null)), + constraint health_runs_export_commit_hex_check check (export_commit ~ '^[0-9a-f]{40}$'), + constraint health_runs_export_commit_check check (commit is null or export_commit = commit), + constraint health_runs_default_tip_branch_check check (kind <> 'default-tip' or baseline_branch is not null), + constraint health_runs_pack_pins_check check (jsonb_typeof(pack_pins) = 'object' and pack_pins ? 'opendox'), + constraint health_runs_sandbox_check check (coalesce(jsonb_typeof(sandbox -> 'live'), 'absent') = 'boolean' and coalesce(jsonb_typeof(sandbox -> 'pids_max'), 'absent') in ('number', 'null')) +); + +create table health_findings ( + run_id uuid not null references health_runs (run_id) on delete cascade, + id text not null, + kind text not null, + pack_id text not null, + pack_version text not null, + path text not null, + identity text not null, + locator jsonb, + severity text not null, + resolution_class text not null, + baseline_class text not null, + message text not null, + evidence jsonb not null, + constraint health_findings_pkey primary key (run_id, id), + constraint health_findings_pack_id_check check (pack_id <> ''), + constraint health_findings_pack_version_check check (pack_version <> ''), + constraint health_findings_identity_check check (jsonb_typeof(identity::jsonb) = 'object'), + constraint health_findings_locator_check check (locator is null or jsonb_typeof(locator) = 'object'), + constraint health_findings_severity_check check (severity in ('error', 'warning', 'info')), + constraint health_findings_resolution_class_check check (resolution_class in ('auto-fix', 'assisted', 'human-only')), + constraint health_findings_baseline_class_check check (baseline_class in ('new', 'pack-upgrade', 'persistent')), + constraint health_findings_evidence_check check (jsonb_typeof(evidence) = 'object') +); + +-- The one lookup every reader makes: a corpus's runs, newest first. +create index health_runs_corpus_root_run_seq_idx on health_runs (corpus_root, run_seq); diff --git a/src/opendox/runtime/health_store.py b/src/opendox/runtime/health_store.py new file mode 100644 index 00000000..8ccdc6ea --- /dev/null +++ b/src/opendox/runtime/health_store.py @@ -0,0 +1,458 @@ +"""The health store: the runs and findings `migrations/0003_health.sql` declares. + +RULED DOMAIN (R2Q13 (a), Brett Heap, opensoft/openxFactory#656 comment +6003486656): `health_runs` and `health_findings` join RULING Q1's six in +`identity.TABLES`, so the closure test, `runtime reset` and the served role's +access check all read them. This module is the only writer of the two tables, +as `identity.CoordinationStore` is of the six. + +IT STORES NO DOCUMENT (#1144 box 14.3; R2Q25 (a)). A finding ADDRESSES a +document — a corpus-relative `path`, a display-only `locator` — and never +carries its text. There is no patch column (lane 3's MISLABEL row 31), and +`record_run` REFUSES any finding field outside the table's columns, by name, +before a statement runs: a caller that hands this store a `patch`, a `text` or +an `excerpt` learns at once that it is not kept, rather than finding out from +a column that silently dropped it. The FIELD BOUNDS — the id grammar, +`identity`'s serialized-size cap, `message`'s 200 characters, the keys +`evidence` may not use — are the contract module's (`health_contract`, T041), +which the engine applies before it calls here; this store is not a second +spelling of them, and imports nothing from it. + +PROVENANCE IS REFUSED HERE AND IN THE SCHEMA (box 15.7). A finding with no +`pack_id` or no `pack_version` — absent, null or empty — is refused by +`record_run` before any statement runs, and `0003_`'s `not null` and +non-empty checks refuse the same row from any other path. "Enforced at the +store and not only in the engine" is the box's own phrase. + +RUNS ARE ORDERED BY `run_seq`, which the server assigns at insert, and never +by `started_at` (`0003_`'s header: this host's clock "can step backwards under +load"). So "the latest run" and the baseline's "latest earlier default-tip +run" (R2Q12 (a)) are the latest RECORDED, and a run reporting a start time in +the past or the future cannot move itself. + +ONE RUN IS RECORDED IN ONE CALL, with all of its findings, in two statements +on the caller's connection: the run row, then every finding through +`jsonb_to_recordset`. The connection is the caller's unit of work, as it is for +`identity.CoordinationStore`: nothing here opens, commits or rolls back, so a +refused finding leaves the caller's transaction to roll back the run row with +it, and no reader ever sees a run without the findings it found. + +NO DRIVER IS IMPORTED HERE. The store is handed a connection — anything with +`.execute(sql, params)` returning a cursor with `.fetchone()` / +`.fetchall()` — so this module imports with the standard library and +`opendox.runtime.identity` alone, and reaches the database only when a caller +hands it one (`opendox/runtime/__init__.py`'s import-weight contract). JSON +values travel as TEXT PARAMETERS cast in SQL (`%s::jsonb`), so no driver type +is needed to send them either. + +EVERY STATEMENT IS COMPOSED FROM MODULE-LEVEL CONSTANTS AND EVERY RUNTIME VALUE +IS A `%s` PARAMETER, exactly as in `identity.py`. +""" + +from __future__ import annotations + +import json +import uuid +from collections.abc import Mapping, Sequence +from dataclasses import dataclass +from datetime import datetime +from typing import Any + +from .identity import ( + FOREIGN_KEY_VIOLATION, + UNIQUE_VIOLATION, + ConflictError, + NotFoundError, + RefusedError, + clamp_limit, +) + +#: Postgres's SQLSTATEs for a row the schema refuses: `not_null_violation`, +#: `check_violation` and `invalid_text_representation` (a value a column type +#: cannot parse). Matched by value off the exception, as `identity.py` matches +#: its two, because this module imports no driver. +NOT_NULL_VIOLATION = "23502" +CHECK_VIOLATION = "23514" +INVALID_TEXT_REPRESENTATION = "22P02" + +#: The fields a finding handed to `record_run` may carry: `health_findings`' +#: columns, less `run_id`, which the run supplies. CLOSED — a field outside it +#: is refused by name (the module docstring says why). +REQUIRED_FINDING_FIELDS: tuple[str, ...] = ( + "id", "kind", "pack_id", "pack_version", "path", "identity", "severity", + "resolution_class", "baseline_class", "message", "evidence", +) +OPTIONAL_FINDING_FIELDS: tuple[str, ...] = ("locator",) + +#: The provenance a finding must carry (box 15.7), refused by name. +PROVENANCE_FIELDS: tuple[str, ...] = ("pack_id", "pack_version") + +#: The required fields whose columns are `text`. `jsonb_to_recordset` would +#: turn a JSON number into text without a word, so a non-string is refused here. +_TEXT_FINDING_FIELDS: tuple[str, ...] = tuple( + field for field in REQUIRED_FINDING_FIELDS + if field not in ("identity", "evidence")) + +_RUN_INSERT_COLUMNS = ( + "run_id, corpus_root, kind, commit, baseline_branch, started_at, " + "finished_at, outcome, pack_pins, export_commit, full_run, sandbox") +_RUN_SELECT = ( + "run_id::text, run_seq, corpus_root, kind, commit, baseline_branch, " + "started_at, finished_at, outcome, pack_pins, export_commit, full_run, " + "sandbox") +#: A finding's own columns, in table order: what `record_run` reads from +#: each finding's record, and, after `run_id`, what it writes. +_FINDING_FIELDS = ( + "id, kind, pack_id, pack_version, path, identity, locator, severity, " + "resolution_class, baseline_class, message, evidence") +_FINDING_INSERT_COLUMNS = "run_id, " + _FINDING_FIELDS +_FINDING_SELECT = ( + "run_id::text, id, kind, pack_id, pack_version, path, identity, locator, " + "severity, resolution_class, baseline_class, message, evidence") +#: The record type `jsonb_to_recordset` reads each finding as: the columns +#: above, less `run_id`, each with its column's type. +_FINDING_RECORD = ( + "id text, kind text, pack_id text, pack_version text, path text, " + "identity text, locator jsonb, severity text, resolution_class text, " + "baseline_class text, message text, evidence jsonb") + + +def _json_text(value: Any, what: str) -> str: + """Strict JSON text for a `%s::jsonb` parameter, or a refusal naming `what`. + + `allow_nan=False`, because `NaN` is Python's JSON and not JSON: Postgres + would refuse the cast, and the refusal belongs here, by name. + """ + try: + return json.dumps(value, sort_keys=True, separators=(",", ":"), + ensure_ascii=False, allow_nan=False) + except (TypeError, ValueError) as exc: + raise RefusedError( + f"{what} is not JSON this store can keep (a value JSON cannot " + "represent); the value is not repeated here") from exc + + +def canonical_identity(identity: Mapping[str, Any]) -> str: + """A finding's `identity`, as the `identity` column stores it. + + Canonical sorted-key JSON, no whitespace, UTF-8 text: the spelling the + finding schema's header gives for the id's key (`json.dumps(key, + sort_keys=True, separators=(",", ":"), ensure_ascii=False)`), so the + stored text is the text the engine hashed (the holder, `6018624750`). + """ + if not isinstance(identity, Mapping): + raise RefusedError( + "a finding's identity must be a JSON object (the position-" + "independent key its family supplies); it is not one") + return _json_text(dict(identity), "a finding's identity") + + +def _loaded(value: Any) -> Any: + """A `jsonb` (or JSON text) column's value as Python data.""" + return json.loads(value) if isinstance(value, str) else value + + +def _uuid_text(run_id: str) -> str: + """`run_id` as the canonical uuid text, or NOT FOUND: no other run exists.""" + try: + return str(uuid.UUID(str(run_id))) + except ValueError as exc: + raise NotFoundError( + "no health run with that run_id (it is not a uuid)") from exc + + +def _refused_by_the_schema(call: Any, what: str) -> Any: + """Run `call`, turning the schema's refusals into this store's errors. + + A duplicate key is a CONFLICT, a missing referenced run is NOT FOUND, and + a null, a failed check or an unparseable value is REFUSED, named by the + constraint the server reports. Anything else re-raises. + """ + try: + return call() + except Exception as exc: # noqa: BLE001 + sqlstate = getattr(exc, "sqlstate", None) + if sqlstate == UNIQUE_VIOLATION: + raise ConflictError( + f"{what}: two rows share one key. Two findings of one run with " + "one id are a collision the engine records as ONE finding " + "against their producer and stores neither " + "(contracts/health-finding.md, the id rule)") from exc + if sqlstate == FOREIGN_KEY_VIOLATION: + raise NotFoundError( + f"{what}: the run it names does not exist") from exc + if sqlstate in (NOT_NULL_VIOLATION, CHECK_VIOLATION, + INVALID_TEXT_REPRESENTATION): + constraint = getattr(getattr(exc, "diag", None), + "constraint_name", None) + column = getattr(getattr(exc, "diag", None), "column_name", None) + named = constraint or column or "a column's type" + raise RefusedError( + f"{what}: refused by the schema ({named}, " + "migrations/0003_health.sql)") from exc + raise + + +# --------------------------------------------------------------------------- +# records — frozen, one per table, fields in the table's column order +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True) +class HealthRun: + """One recorded run (data-model.md § Health run). + + `full_run` is the data model's field `full`: PostgreSQL 16 reserves the + bare word as a column name (the holder, `6064169640`). + """ + + run_id: str + run_seq: int + corpus_root: str + kind: str + commit: str | None + baseline_branch: str | None + started_at: datetime + finished_at: datetime + outcome: str + pack_pins: dict[str, Any] + export_commit: str + full_run: bool + sandbox: dict[str, Any] + + @classmethod + def _from_row(cls, row: Sequence[Any]) -> HealthRun: + values = list(row) + values[9] = _loaded(values[9]) # pack_pins + values[12] = _loaded(values[12]) # sandbox + return cls(*values) + + +@dataclass(frozen=True) +class HealthFinding: + """One stored finding (data-model.md § Finding). + + `identity` is the parsed form of the column's canonical JSON text, which + is what `health list --json` emits; `canonical_identity` gives the text. + """ + + run_id: str + id: str + kind: str + pack_id: str + pack_version: str + path: str + identity: dict[str, Any] + locator: dict[str, Any] | None + severity: str + resolution_class: str + baseline_class: str + message: str + evidence: dict[str, Any] + + @classmethod + def _from_row(cls, row: Sequence[Any]) -> HealthFinding: + values = list(row) + values[6] = json.loads(values[6]) # identity: text, by design + values[7] = _loaded(values[7]) # locator + values[12] = _loaded(values[12]) # evidence + return cls(*values) + + +def _one(row: Sequence[Any] | None, what: str) -> Sequence[Any]: + if row is None: + raise NotFoundError(f"no {what}") + return row + + +def _aware(value: datetime, what: str) -> datetime: + """A timestamp the server can place: a NAIVE one is refused. + + `timestamptz` reads a naive value in the SESSION's time zone, so the same + call would record different instants on two servers. + """ + if not isinstance(value, datetime) or value.utcoffset() is None: + raise RefusedError( + f"{what} must be a timezone-aware datetime; a naive one is read " + "in whatever time zone the session has") + return value + + +def _finding_record(index: int, finding: Any) -> dict[str, Any]: + """One finding, checked and shaped for `jsonb_to_recordset`. + + Every refusal here runs BEFORE any statement, and names the finding by its + position and the field by its name, never by a value. + """ + if not isinstance(finding, Mapping): + raise RefusedError(f"finding {index} is not a mapping") + for field in PROVENANCE_FIELDS: + value = finding.get(field) + if not isinstance(value, str) or not value: + raise RefusedError( + f"finding {index} has no {field}. Every finding carries the " + "pack_id and pack_version of the pack that produced it — " + "`opendox` and the installed version for the product's own " + "families — and the store refuses one without them (#1144 box " + "15.7; test_the_store_refuses_a_finding_without_provenance)") + allowed = set(REQUIRED_FINDING_FIELDS) | set(OPTIONAL_FINDING_FIELDS) + unknown = sorted(str(key) for key in finding if key not in allowed) + if unknown: + raise RefusedError( + f"finding {index} carries {unknown}, which this store does not " + "keep. It holds no document and no patch (#1144 box 14.3; R2Q25 " + "(a)): a finding is its columns, and a pack's patch is re-obtained " + "at `fix`, never stored") + missing = [field for field in REQUIRED_FINDING_FIELDS if field not in finding] + if missing: + raise RefusedError(f"finding {index} has no {missing}") + not_text = [field for field in _TEXT_FINDING_FIELDS + if not isinstance(finding[field], str)] + if not_text: + raise RefusedError( + f"finding {index}'s {not_text} must be strings; the store does not " + "turn another type into text for them") + evidence = finding["evidence"] + if not isinstance(evidence, Mapping): + raise RefusedError( + f"finding {index}'s evidence must be a JSON object (`{{}}` when " + "there is nothing to locate)") + record = {field: finding[field] for field in REQUIRED_FINDING_FIELDS} + record["identity"] = canonical_identity(finding["identity"]) + record["evidence"] = dict(evidence) + locator = finding.get("locator") + if locator is not None: + if not isinstance(locator, Mapping): + raise RefusedError( + f"finding {index}'s locator must be a JSON object or absent") + record["locator"] = dict(locator) + return record + + +# --------------------------------------------------------------------------- +# the store +# --------------------------------------------------------------------------- + + +class HealthStore: + """Every read and write of the two health tables, over ONE connection.""" + + def __init__(self, conn: Any) -> None: + self._conn = conn + + def record_run(self, *, corpus_root: str, kind: str, commit: str | None, + baseline_branch: str | None, started_at: datetime, + finished_at: datetime, outcome: str, + pack_pins: Mapping[str, Any], export_commit: str, + full_run: bool, sandbox: Mapping[str, Any], + findings: Sequence[Mapping[str, Any]]) -> HealthRun: + """Record one run and every finding it stores, on the caller's unit of work. + + Every check this module makes runs first, over the whole run, so a + refused finding writes nothing at all; the schema's own constraints + then refuse whatever reaches it from a value this module does not + judge (a vocabulary, a commit's form), named by the constraint. + """ + _aware(started_at, "started_at") + _aware(finished_at, "finished_at") + if not isinstance(full_run, bool): + raise RefusedError("full_run must be a boolean (whether no " + "`--pack` restricted the run)") + if not isinstance(pack_pins, Mapping): + raise RefusedError("pack_pins must be a JSON object (the run's " + "pack inventory, keyed by pack id)") + if not isinstance(sandbox, Mapping): + raise RefusedError("sandbox must be a JSON object (what the " + "per-run probe found)") + records = [_finding_record(index, finding) + for index, finding in enumerate(findings)] + pins_text = _json_text(dict(pack_pins), "pack_pins") + sandbox_text = _json_text(dict(sandbox), "sandbox") + findings_text = _json_text(records, "the run's findings") + run_id = str(uuid.uuid4()) + row = _refused_by_the_schema( + lambda: self._conn.execute( + f"insert into health_runs ({_RUN_INSERT_COLUMNS}) " + "values (%s::uuid, %s, %s, %s, %s, %s, %s, %s, %s::jsonb, %s, " + f"%s, %s::jsonb) returning {_RUN_SELECT}", + (run_id, corpus_root, kind, commit, baseline_branch, + started_at, finished_at, outcome, pins_text, export_commit, + full_run, sandbox_text), + ).fetchone(), + "the run") + run = HealthRun._from_row(_one(row, "health run returned by its insert")) + if records: + _refused_by_the_schema( + lambda: self._conn.execute( + f"insert into health_findings ({_FINDING_INSERT_COLUMNS}) " + f"select %s::uuid, {_FINDING_FIELDS} " + f"from jsonb_to_recordset(%s::jsonb) as f({_FINDING_RECORD})", + (run_id, findings_text)), + "a finding of the run") + return run + + def get_run(self, run_id: str) -> HealthRun: + row = self._conn.execute( + f"select {_RUN_SELECT} from health_runs where run_id = %s::uuid", + (_uuid_text(run_id),)).fetchone() + return HealthRun._from_row(_one(row, f"health run {run_id!r}")) + + def latest_run(self, *, corpus_root: str) -> HealthRun | None: + """The corpus's latest RECORDED run, of any kind and outcome, or None. + + Only this corpus's: two corpora sharing one store never read each + other's runs (`list`, `fix` and `accept` read the resolved corpus's). + """ + row = self._conn.execute( + f"select {_RUN_SELECT} from health_runs where corpus_root = %s " + "order by run_seq desc limit 1", (corpus_root,)).fetchone() + return None if row is None else HealthRun._from_row(row) + + def baseline_run(self, *, corpus_root: str, baseline_branch: str | None, + before_seq: int | None = None) -> HealthRun | None: + """B: the latest default-tip run that is COMPLETE and FULL, or None. + + R2Q12 (a), as data-model.md § Baseline classes words it: the SAME + resolved `corpus_root` and `baseline_branch`, never another corpus's + run, and only a `complete`, `full` default-tip run forms a baseline. + `before_seq` bounds it to runs recorded before a given one. With no + baseline branch no run is default-tip, so there is no B. + """ + if baseline_branch is None: + return None + row = self._conn.execute( + f"select {_RUN_SELECT} from health_runs " + "where corpus_root = %s and baseline_branch = %s " + "and kind = 'default-tip' and outcome = 'complete' and full_run " + "and (%s::bigint is null or run_seq < %s) " + "order by run_seq desc limit 1", + (corpus_root, baseline_branch, before_seq, before_seq)).fetchone() + return None if row is None else HealthRun._from_row(row) + + def get_finding(self, *, run_id: str, finding_id: str) -> HealthFinding: + row = self._conn.execute( + f"select {_FINDING_SELECT} from health_findings " + "where run_id = %s::uuid and id = %s", + (_uuid_text(run_id), finding_id)).fetchone() + return HealthFinding._from_row( + _one(row, f"finding {finding_id!r} in health run {run_id!r}")) + + def list_findings(self, *, run_id: str, limit: int | None = None, + after: str | None = None, + resolution_class: str | None = None, + baseline_class: str | None = None) -> list[HealthFinding]: + """One page of a run's findings, ordered by id; `after` walks the rest. + + Bounded like every listing in this package (`identity.clamp_limit`). + """ + rows = self._conn.execute( + f"select {_FINDING_SELECT} from health_findings " + "where run_id = %s::uuid " + "and (%s::text is null or id > %s) " + "and (%s::text is null or resolution_class = %s) " + "and (%s::text is null or baseline_class = %s) " + "order by id limit %s", + (_uuid_text(run_id), after, after, resolution_class, + resolution_class, baseline_class, baseline_class, + clamp_limit(limit))).fetchall() + return [HealthFinding._from_row(row) for row in rows] diff --git a/tests_runtime/test_health_store.py b/tests_runtime/test_health_store.py new file mode 100644 index 00000000..0dba4dcb --- /dev/null +++ b/tests_runtime/test_health_store.py @@ -0,0 +1,646 @@ +"""T042 (plan 038, HA-1): the health store, `0003_health.sql` and its writer. + +The falsifier T042 names: `test_the_store_refuses_a_finding_without_provenance` +(#1144 box 15.7) and `test_an_install_level_finding_is_admitted` (`pack_id` +`opendox`, an empty `path`; OQ-H15-19), beside the tests that hold the store to +boxes 14.1 to 14.3 — `0003_` additive, the two tables DOMAIN (R2Q13 (a)), and +NO DOCUMENT, so no patch column (14.3; R2Q25 (a)). + +DB-BACKED where it says so (the `database` fixture: a throwaway schema with +every migration applied — `0003_` included, which is half the point), and +HERMETIC where a refusal must come BEFORE any statement: those cases hand the +store a connection that fails the test if a statement reaches it, because a +refusal the schema makes instead would look the same from the outside and +prove nothing about the store. + +Every schema case writes RAW SQL, past the store, because "no path can store a +finding without them" (15.7) is a claim about the schema and not about the one +writer this package ships. +""" + +from __future__ import annotations + +import json +import uuid +from datetime import UTC, datetime +from typing import Any + +import pytest + +from opendox.runtime import identity +from opendox.runtime.health_store import ( + HealthStore, + canonical_identity, +) +from opendox.runtime.identity import ConflictError, NotFoundError, RefusedError + +HEX = "a" * 40 +OTHER_HEX = "b" * 40 +T0 = datetime(2026, 10, 8, 12, 0, tzinfo=UTC) + +#: data-model.md's two tables, column by column, in table order, with the ones +#: it makes nullable. `full_run` is the model's `full` (the holder, #656 +#: `6064169640`); `run_seq` is the order runs were recorded in (`0003_`'s +#: header). There is no `patch` column and there is never going to be one +#: here (14.3; lane 3's MISLABEL row 31). +COLUMNS: dict[str, tuple[str, ...]] = { + "health_runs": ( + "run_id", "run_seq", "corpus_root", "kind", "commit", + "baseline_branch", "started_at", "finished_at", "outcome", + "pack_pins", "export_commit", "full_run", "sandbox"), + "health_findings": ( + "run_id", "id", "kind", "pack_id", "pack_version", "path", "identity", + "locator", "severity", "resolution_class", "baseline_class", "message", + "evidence"), +} +NULLABLE: dict[str, frozenset[str]] = { + "health_runs": frozenset({"commit", "baseline_branch"}), + "health_findings": frozenset({"locator"}), +} + + +def _run_kwargs(**over: Any) -> dict[str, Any]: + """`record_run`'s arguments for one complete, full default-tip run.""" + base: dict[str, Any] = { + "corpus_root": "/corpora/a", "kind": "default-tip", "commit": HEX, + "baseline_branch": "main", "started_at": T0, "finished_at": T0, + "outcome": "complete", + "pack_pins": {"opendox": {"version": "0.2.0", "digest": None, + "commit": None}}, + "export_commit": HEX, "full_run": True, + "sandbox": {"live": True, "pids_max": None}, "findings": [], + } + base.update(over) + return base + + +def _finding(**over: Any) -> dict[str, Any]: + """The contract's own example finding (contracts/health-finding.md).""" + base: dict[str, Any] = { + "id": "opendox.broken-link.3c1f0e9a7b2d4c65", "kind": "broken-link", + "pack_id": "opendox", "pack_version": "0.2.0", "path": "notes/plan.md", + "identity": {"target": "../old/brief.md"}, + "locator": {"line_start": 12, "line_end": 12}, + "severity": "warning", "resolution_class": "auto-fix", + "baseline_class": "new", + "message": "link target does not exist; a unique file of that name " + "exists elsewhere", + "evidence": {"candidate": "archive/brief.md", "family_version": "1"}, + } + base.update(over) + return base + + +class _NoStatement: + """A connection the store must never reach: any statement fails the test.""" + + def execute(self, sql: str, params: Any = None) -> Any: + raise AssertionError(f"a statement reached the database: {sql[:60]}") + + +def _record(database: Any, **over: Any): + """Record one run in its own committed transaction.""" + with database.transaction() as conn: + return HealthStore(conn).record_run(**_run_kwargs(**over)) + + +def _count(database: Any, table: str) -> int: + with database.connection() as conn: + return conn.execute(f"select count(*) from {table}").fetchone()[0] + + +def _raw(database: Any, sql: str, params: dict[str, Any]) -> Exception | None: + """One raw statement in its own transaction: its refusal, or None.""" + try: + with database.transaction() as conn: + conn.execute(sql, params) + except Exception as exc: # noqa: BLE001 + return exc + return None + + +_RAW_RUN = ( + "insert into health_runs (run_id, corpus_root, kind, commit, " + "baseline_branch, started_at, finished_at, outcome, pack_pins, " + "export_commit, full_run, sandbox) values (%(run_id)s::uuid, " + "%(corpus_root)s, %(kind)s, %(commit)s, %(baseline_branch)s, " + "%(started_at)s, %(finished_at)s, %(outcome)s, %(pack_pins)s::jsonb, " + "%(export_commit)s, %(full_run)s, %(sandbox)s::jsonb)") + +_RAW_FINDING = ( + "insert into health_findings (run_id, id, kind, pack_id, pack_version, " + "path, identity, locator, severity, resolution_class, baseline_class, " + "message, evidence) values (%(run_id)s::uuid, %(id)s, %(kind)s, " + "%(pack_id)s, %(pack_version)s, %(path)s, %(identity)s, " + "%(locator)s::jsonb, %(severity)s, %(resolution_class)s, " + "%(baseline_class)s, %(message)s, %(evidence)s::jsonb)") + + +def _raw_run(**over: Any) -> dict[str, Any]: + base = { + "run_id": str(uuid.uuid4()), "corpus_root": "/corpora/raw", + "kind": "default-tip", "commit": HEX, "baseline_branch": "main", + "started_at": T0, "finished_at": T0, "outcome": "complete", + "pack_pins": '{"opendox": {"version": "0.2.0"}}', + "export_commit": HEX, "full_run": True, + "sandbox": '{"live": false, "pids_max": null}', + } + base.update(over) + return base + + +def _raw_finding(run_id: str, **over: Any) -> dict[str, Any]: + base = { + "run_id": run_id, "id": "opendox.orphan.0123456789abcdef", + "kind": "orphan", "pack_id": "opendox", "pack_version": "0.2.0", + "path": "a.md", "identity": '{"path":"a.md"}', "locator": None, + "severity": "info", "resolution_class": "human-only", + "baseline_class": "new", "message": "nothing links to it", + "evidence": "{}", + } + base.update(over) + return base + + +# --------------------------------------------------------------------------- +# THE FALSIFIER'S TWO NAMED CASES +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("missing", [ + {"drop": "pack_id"}, {"drop": "pack_version"}, + {"pack_id": None}, {"pack_version": None}, + {"pack_id": ""}, {"pack_version": ""}, +], ids=["no-pack_id", "no-pack_version", "null-pack_id", "null-pack_version", + "empty-pack_id", "empty-pack_version"]) +def test_the_store_refuses_a_finding_without_provenance( + database: Any, missing: dict[str, Any]) -> None: + """Box 15.7: "no path can store a finding without them". + + Three layers, each measured on its own, so that no one of them can stand + in for another: the STORE refuses before any statement (a connection that + fails on a statement proves it); over a real database it refuses and the + run row it would have written is not there either; and the SCHEMA refuses + the same row written raw, past the store — a null by `not null`, an empty + string by the column's check. + """ + finding = _finding() + field = missing.get("drop") + if field: + del finding[field] + else: + finding.update(missing) + field = next(iter(missing)) + + with pytest.raises(RefusedError) as caught: + HealthStore(_NoStatement()).record_run(**_run_kwargs(findings=[finding])) + assert f"has no {field}" in str(caught.value) + assert "15.7" in str(caught.value) + + with pytest.raises(RefusedError): + _record(database, findings=[finding]) + assert _count(database, "health_runs") == 0 + assert _count(database, "health_findings") == 0 + + run = _record(database) + value = None if field not in missing or missing.get(field) is None else "" + refused = _raw(database, _RAW_FINDING, + _raw_finding(run.run_id, **{field: value})) + assert refused is not None, f"the schema stored a finding with no {field}" + if value is None: + assert refused.sqlstate == "23502", refused # not_null_violation + assert refused.diag.column_name == field + else: + assert refused.sqlstate == "23514", refused # check_violation + assert refused.diag.constraint_name == f"health_findings_{field}_check" + assert _count(database, "health_findings") == 0 + + +def test_an_install_level_finding_is_admitted(database: Any) -> None: + """OQ-H15-19: `pack_id` `opendox`, an EMPTY `path`, `human-only`. + + The finding a run without a live sandbox records (R2Q16 (a)), with the + engine-owned identity key a pathless finding carries + (contracts/health-finding.md: `{category, entry}`). An empty `path` is a + value, not an absence, so `not null` must not refuse it. + """ + finding = _finding( + id="opendox.no-sandbox.00112233aabbccdd", kind="no-sandbox", path="", + identity={"category": "no-sandbox", "entry": ""}, locator=None, + severity="error", resolution_class="human-only", + message="no live sandbox, so no pack ran", evidence={}) + run = _record(database, outcome="partial", + sandbox={"live": False, "pids_max": None}, + findings=[finding]) + with database.connection() as conn: + stored = HealthStore(conn).get_finding(run_id=run.run_id, + finding_id=finding["id"]) + listed = HealthStore(conn).list_findings(run_id=run.run_id) + assert stored.pack_id == "opendox" + assert stored.path == "" + assert stored.identity == {"category": "no-sandbox", "entry": ""} + assert stored.locator is None + assert stored.evidence == {} + assert stored.resolution_class == "human-only" + assert listed == [stored] + + +# --------------------------------------------------------------------------- +# the schema: exactly the data model's columns, and nothing that holds a document +# --------------------------------------------------------------------------- + + +def test_the_tables_hold_exactly_the_data_models_columns(database: Any) -> None: + """Read off the APPLIED schema, not the file: what Postgres made of `0003_`. + + The column list is closed, so a `patch` column, a `full` spelled bare, or a + nullable provenance column is a red here before it is anything else. + """ + with database.connection() as conn: + for table, expected in COLUMNS.items(): + rows = conn.execute( + "select column_name, is_nullable from information_schema.columns " + "where table_schema = current_schema() and table_name = %s " + "order by ordinal_position", (table,)).fetchall() + assert tuple(name for name, _ in rows) == expected, table + nullable = {name for name, flag in rows if flag == "YES"} + assert nullable == NULLABLE[table], ( + f"{table}: nullable {sorted(nullable)}, the data model " + f"{sorted(NULLABLE[table])}") + assert "patch" not in COLUMNS["health_findings"] + assert "full_run" in COLUMNS["health_runs"] + assert "full" not in COLUMNS["health_runs"] + + +def test_the_health_tables_are_domain_tables(database: Any) -> None: + """R2Q13 (a): both join `identity.TABLES`, so reset, the closure test and + the served role's access check all reach them, and the runner created them.""" + assert identity.TABLES[-2:] == ("health_runs", "health_findings") + with database.connection() as conn: + present = {row[0] for row in conn.execute( + "select table_name from information_schema.tables " + "where table_schema = current_schema()").fetchall()} + assert {"health_runs", "health_findings"} <= present + + +_NOT_NULL_RUN = sorted(set(COLUMNS["health_runs"]) - NULLABLE["health_runs"] + - {"run_seq"}) +_NOT_NULL_FINDING = sorted(set(COLUMNS["health_findings"]) + - NULLABLE["health_findings"]) + + +@pytest.mark.parametrize("column", _NOT_NULL_RUN) +def test_every_not_null_run_column_refuses_a_null(database: Any, + column: str) -> None: + """`run_seq` is absent here because the server assigns it: an identity + column `generated always` refuses ANY value, null or not.""" + refused = _raw(database, _RAW_RUN, _raw_run(**{column: None})) + assert refused is not None, f"health_runs.{column} admitted a null" + assert refused.sqlstate == "23502", refused + assert refused.diag.column_name == column + + +@pytest.mark.parametrize("column", _NOT_NULL_FINDING) +def test_every_not_null_finding_column_refuses_a_null(database: Any, + column: str) -> None: + run = _record(database) + refused = _raw(database, _RAW_FINDING, + _raw_finding(run.run_id, **{column: None})) + assert refused is not None, f"health_findings.{column} admitted a null" + assert refused.sqlstate == "23502", refused + assert refused.diag.column_name == column + + +#: One row per CHECK in `0003_`: the one value that breaks it ALONE, and the +#: constraint the server must name. Each vocabulary is the data model's (and +#: 14.6's for the resolution classes, "in the store … alike"). +_RUN_CHECKS = [ + ({"kind": "nightly"}, "health_runs_kind_check"), + ({"outcome": "done"}, "health_runs_outcome_check"), + ({"kind": "working-state"}, "health_runs_commit_check"), + ({"kind": "branch", "commit": None}, "health_runs_commit_check"), + ({"commit": "A" * 40, "export_commit": "A" * 40}, + "health_runs_export_commit_hex_check"), + ({"commit": "abc123", "export_commit": "abc123"}, + "health_runs_export_commit_hex_check"), + ({"kind": "working-state", "commit": None, "export_commit": "HEAD"}, + "health_runs_export_commit_hex_check"), + ({"export_commit": OTHER_HEX}, "health_runs_export_commit_check"), + ({"baseline_branch": None}, "health_runs_default_tip_branch_check"), + ({"pack_pins": "[]"}, "health_runs_pack_pins_check"), + ({"pack_pins": '{"some-pack": {"version": "1"}}'}, + "health_runs_pack_pins_check"), + ({"sandbox": "{}"}, "health_runs_sandbox_check"), + ({"sandbox": '{"pids_max": null}'}, "health_runs_sandbox_check"), + ({"sandbox": '{"live": "yes", "pids_max": null}'}, + "health_runs_sandbox_check"), + ({"sandbox": '{"live": true}'}, "health_runs_sandbox_check"), + ({"sandbox": '{"live": true, "pids_max": "512"}'}, + "health_runs_sandbox_check"), + ({"sandbox": "[true]"}, "health_runs_sandbox_check"), +] + +_FINDING_CHECKS = [ + ({"identity": "[]"}, "health_findings_identity_check"), + ({"identity": '"a key"'}, "health_findings_identity_check"), + ({"locator": "[12]"}, "health_findings_locator_check"), + ({"severity": "critical"}, "health_findings_severity_check"), + ({"resolution_class": "auto_fix"}, + "health_findings_resolution_class_check"), + ({"baseline_class": "unclassed"}, "health_findings_baseline_class_check"), + ({"evidence": "[]"}, "health_findings_evidence_check"), +] + + +@pytest.mark.parametrize(("over", "constraint"), _RUN_CHECKS, + ids=[f"{c}:{','.join(o)}" for o, c in _RUN_CHECKS]) +def test_each_run_check_refuses_what_it_names(database: Any, over: dict, + constraint: str) -> None: + refused = _raw(database, _RAW_RUN, _raw_run(**over)) + assert refused is not None, f"{over} was admitted" + assert refused.sqlstate == "23514", refused + assert refused.diag.constraint_name == constraint + assert _count(database, "health_runs") == 0 + + +@pytest.mark.parametrize(("over", "constraint"), _FINDING_CHECKS, + ids=[f"{c}:{','.join(o)}" for o, c in _FINDING_CHECKS]) +def test_each_finding_check_refuses_what_it_names(database: Any, over: dict, + constraint: str) -> None: + run = _record(database) + refused = _raw(database, _RAW_FINDING, _raw_finding(run.run_id, **over)) + assert refused is not None, f"{over} was admitted" + assert refused.sqlstate == "23514", refused + assert refused.diag.constraint_name == constraint + assert _count(database, "health_findings") == 0 + + +@pytest.mark.parametrize("over", [ + {}, + {"kind": "branch", "baseline_branch": None}, + {"kind": "working-state", "commit": None, "export_commit": OTHER_HEX}, + {"outcome": "partial", "full_run": False}, + {"outcome": "refused"}, + {"sandbox": '{"live": true, "pids_max": 512}'}, + {"pack_pins": '{"opendox": {"version": "0.2.0"}, "gov": {"version": "1", ' + '"digest": "sha256:00", "commit": null}}'}, +], ids=["default-tip", "branch-no-baseline-branch", "working-state", + "partial-pack-restricted", "refused", "pids-max", "two-packs"]) +def test_the_checks_admit_every_run_the_data_model_describes( + database: Any, over: dict) -> None: + """The other half of each check: the rows data-model.md describes go in.""" + assert _raw(database, _RAW_RUN, _raw_run(**over)) is None + + +def test_the_schema_refuses_identity_text_that_is_not_json(database: Any) -> None: + run = _record(database) + refused = _raw(database, _RAW_FINDING, + _raw_finding(run.run_id, identity="{not json")) + assert refused is not None + assert refused.sqlstate == "22P02", refused # invalid_text_representation + + +def test_a_finding_must_name_a_recorded_run(database: Any) -> None: + refused = _raw(database, _RAW_FINDING, _raw_finding(str(uuid.uuid4()))) + assert refused is not None + assert refused.sqlstate == "23503", refused # foreign_key_violation + + +# --------------------------------------------------------------------------- +# the store: no document, the canonical identity, one atomic run +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("field", ["patch", "text", "excerpt", "content", + "document"]) +def test_the_store_keeps_no_patch_and_no_document(field: str) -> None: + """14.3 and R2Q25 (a), at the API: a field outside the columns is refused + by NAME, before any statement — never silently dropped.""" + with pytest.raises(RefusedError) as caught: + HealthStore(_NoStatement()).record_run(**_run_kwargs( + findings=[_finding(**{field: "the text of a document"})])) + assert repr(field) in str(caught.value) + assert "the text of a document" not in str(caught.value) + + +def test_identity_is_stored_as_the_canonical_text_it_was_hashed_as( + database: Any) -> None: + """The holder, `6018624750`: STORED, canonical sorted-key JSON. + + `jsonb` would re-order the keys and re-space the text; the column is text + so the bytes are the ones the engine hashed into `id`. Non-ASCII stays + itself (`ensure_ascii=False`, the finding schema header's spelling). + """ + key = {"zeta": "ü", "alpha": ["b", "a"], "mid": {"y": "1", "x": "2"}} + expected = json.dumps(key, sort_keys=True, separators=(",", ":"), + ensure_ascii=False) + assert canonical_identity(key) == expected + run = _record(database, findings=[_finding(identity=key)]) + with database.connection() as conn: + text = conn.execute( + "select identity from health_findings where run_id = %s::uuid", + (run.run_id,)).fetchone()[0] + stored = HealthStore(conn).list_findings(run_id=run.run_id)[0] + assert text == expected + assert stored.identity == key + + +def test_two_findings_with_one_id_in_one_run_store_nothing(database: Any) -> None: + """The engine reports a collision as ONE finding and stores neither + (contracts/health-finding.md); a run that reaches the store with two is a + CONFLICT, and the caller's transaction takes the run row with it.""" + twice = [_finding(), _finding(path="other.md")] + with pytest.raises(ConflictError): + _record(database, findings=twice) + assert _count(database, "health_runs") == 0 + assert _count(database, "health_findings") == 0 + + +def test_a_vocabulary_the_schema_refuses_is_a_named_refusal(database: Any) -> None: + with pytest.raises(RefusedError) as caught: + _record(database, findings=[_finding(severity="critical")]) + assert "health_findings_severity_check" in str(caught.value) + with pytest.raises(RefusedError) as caught: + _record(database, kind="nightly") + assert "health_runs_kind_check" in str(caught.value) + assert _count(database, "health_runs") == 0 + + +@pytest.mark.parametrize(("over", "says"), [ + ({"started_at": datetime(2026, 10, 8, 12, 0)}, "timezone-aware"), + ({"finished_at": datetime(2026, 10, 8, 12, 0)}, "timezone-aware"), + ({"full_run": 1}, "full_run must be a boolean"), + ({"pack_pins": [["opendox", "0.2.0"]]}, "pack_pins must be"), + ({"sandbox": "live"}, "sandbox must be"), + ({"sandbox": {"live": True, "pids_max": float("nan")}}, "sandbox is not JSON"), + ({"findings": ["opendox.orphan.0123456789abcdef"]}, "is not a mapping"), + ({"findings": [_finding(identity=["target"])]}, "identity must be"), + ({"findings": [_finding(evidence=None)]}, "evidence must be"), + ({"findings": [_finding(locator=[12, 12])]}, "locator must be"), + ({"findings": [_finding(kind=7)]}, "must be strings"), + ({"findings": [{k: v for k, v in _finding().items() if k != "message"}]}, + "has no ['message']"), +], ids=["naive-start", "naive-finish", "full-run-int", "pins-list", + "sandbox-text", "sandbox-nan", "finding-text", "identity-list", + "evidence-none", "locator-list", "kind-int", "no-message"]) +def test_malformed_input_is_refused_before_any_statement(over: dict, + says: str) -> None: + with pytest.raises(RefusedError) as caught: + HealthStore(_NoStatement()).record_run(**_run_kwargs(**over)) + assert says in str(caught.value) + + +# --------------------------------------------------------------------------- +# reads: the order of record, the baseline, one corpus at a time +# --------------------------------------------------------------------------- + + +def test_runs_are_ordered_by_record_and_not_by_clock(database: Any) -> None: + """`0003_`'s header: the clock can step backwards. A run that REPORTS a + start in the future does not stay the latest once a later one is recorded.""" + first = _record(database, started_at=datetime(2031, 1, 1, tzinfo=UTC), + finished_at=datetime(2031, 1, 1, tzinfo=UTC)) + second = _record(database, started_at=datetime(2001, 1, 1, tzinfo=UTC), + finished_at=datetime(2001, 1, 1, tzinfo=UTC)) + assert second.run_seq > first.run_seq + with database.connection() as conn: + store = HealthStore(conn) + assert store.latest_run(corpus_root="/corpora/a") == second + assert store.baseline_run(corpus_root="/corpora/a", + baseline_branch="main") == second + + +def test_the_baseline_is_the_latest_complete_full_default_tip_run( + database: Any) -> None: + """R2Q12 (a), data-model.md § Baseline classes: same corpus, same baseline + branch, default-tip, complete AND full. Every other run is recorded after + the baseline and none of them may displace it.""" + baseline = _record(database) + _record(database, outcome="partial") # a pack failed + _record(database, full_run=False) # `--pack` + _record(database, kind="branch", commit=OTHER_HEX, export_commit=OTHER_HEX) + _record(database, kind="working-state", commit=None) + _record(database, baseline_branch="trunk") # another branch + _record(database, corpus_root="/corpora/b") # another corpus + with database.connection() as conn: + store = HealthStore(conn) + assert store.baseline_run(corpus_root="/corpora/a", + baseline_branch="main") == baseline + assert store.baseline_run(corpus_root="/corpora/a", + baseline_branch="main", + before_seq=baseline.run_seq) is None + assert store.baseline_run(corpus_root="/corpora/a", + baseline_branch=None) is None + assert store.baseline_run(corpus_root="/corpora/c", + baseline_branch="main") is None + + +def test_two_corpora_in_one_store_never_read_each_others_runs( + database: Any) -> None: + a1 = _record(database, findings=[_finding()]) + b1 = _record(database, corpus_root="/corpora/b", + findings=[_finding(path="b.md")]) + a2 = _record(database) + with database.connection() as conn: + store = HealthStore(conn) + assert store.latest_run(corpus_root="/corpora/a") == a2 + assert store.latest_run(corpus_root="/corpora/b") == b1 + assert store.latest_run(corpus_root="/corpora/none") is None + assert [f.path for f in store.list_findings(run_id=b1.run_id)] == ["b.md"] + assert [f.path for f in store.list_findings(run_id=a1.run_id)] == [ + "notes/plan.md"] + assert store.list_findings(run_id=a2.run_id) == [] + + +def test_a_runs_findings_page_by_id_and_filter_by_class(database: Any) -> None: + findings = [ + _finding(id=f"opendox.orphan.{n:016x}", kind="orphan", path=f"{n}.md", + identity={"path": f"{n}.md"}, resolution_class=cls, + baseline_class=base) + for n, cls, base in [(1, "human-only", "new"), + (2, "auto-fix", "persistent"), + (3, "human-only", "pack-upgrade")]] + run = _record(database, findings=findings) + with database.connection() as conn: + store = HealthStore(conn) + page = store.list_findings(run_id=run.run_id, limit=2) + rest = store.list_findings(run_id=run.run_id, after=page[-1].id) + assert [f.id for f in page + rest] == sorted(f["id"] for f in findings) + assert len(page) == 2 + assert [f.path for f in store.list_findings( + run_id=run.run_id, resolution_class="human-only")] == ["1.md", "3.md"] + assert [f.path for f in store.list_findings( + run_id=run.run_id, baseline_class="persistent")] == ["2.md"] + assert store.get_run(run.run_id) == run + + +def test_an_unknown_run_or_finding_is_not_found(database: Any) -> None: + run = _record(database) + with database.connection() as conn: + store = HealthStore(conn) + with pytest.raises(NotFoundError): + store.get_run(str(uuid.uuid4())) + with pytest.raises(NotFoundError): + store.get_run("not-a-run") + with pytest.raises(NotFoundError): + store.get_finding(run_id=run.run_id, finding_id="opendox.x.0") + + +def test_the_served_role_can_record_and_read_a_run(database: Any) -> None: + """The served role holds TABLE grants only — the init scripts' join and + their default privileges, both `on table`/`on tables` — and must still + write `run_seq`. MEASURED: an identity column needs no sequence grant.""" + role = "t_served_" + uuid.uuid4().hex[:8] + with database.transaction() as conn: + conn.execute(f"create role {role}") + conn.execute(f"grant usage on schema {database.schema} to {role}") + conn.execute("grant select, insert, update, delete on table " + f"health_runs, health_findings to {role}") + try: + with database.transaction() as conn: + conn.execute(f"set local role {role}") + run = HealthStore(conn).record_run(**_run_kwargs( + findings=[_finding()])) + assert HealthStore(conn).get_run(run.run_id) == run + assert _count(database, "health_findings") == 1 + finally: + with database.transaction() as conn: + conn.execute(f"drop owned by {role}") + conn.execute(f"drop role {role}") + + +def test_runtime_reset_drops_a_populated_health_store( + database: Any, postgres_dsn: str, monkeypatch: pytest.MonkeyPatch) -> None: + """14.3's "stays DISPOSABLE", through the verb F14.1 runs. + + A run WITH a finding, so `health_findings` references `health_runs` when + the drop runs: `drop table` without `cascade` refuses a referenced table, + so a `DROP_ORDER` that put `health_runs` first, or left either table out, + fails here on the database rather than only in the order's own test. The + token is `RESET_CONFIRMATION`, byte-identical to F14.1's command. + """ + import io + from contextlib import redirect_stdout + + from opendox.runtime import cli + from opendox.runtime.config import PREFIX + + assert cli.RESET_CONFIRMATION == "yes-drop-the-coordination-database" + _record(database, findings=[_finding()]) + separator = "&" if "?" in postgres_dsn else "?" + monkeypatch.setenv(PREFIX + "MIGRATION_DATABASE_URL", + f"{postgres_dsn}{separator}" + f"options=-csearch_path%3D{database.schema}%2Cpublic") + args = cli.build_parser().parse_args( + ["runtime", "reset", "--confirm", cli.RESET_CONFIRMATION]) + buffer = io.StringIO() + with redirect_stdout(buffer): + code = args.func(args) + evidence = json.loads(buffer.getvalue()) + assert code == 0, evidence + assert evidence["dropped"][:2] == ["health_findings", "health_runs"] + with database.connection() as conn: + left = conn.execute( + "select table_name from information_schema.tables " + "where table_schema = current_schema()").fetchall() + assert left == [], f"the reset left {[row[0] for row in left]}" From 0ee72652908f29f04d22761295a1ab4135088cb6 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Thu, 8 Oct 2026 18:01:50 +0000 Subject: [PATCH 02/12] T042 (plan 038, HA-1): the health tables join identity.TABLES, DROP_ORDER, the grants and the closure test identity.TABLES gains health_runs and health_findings (R2Q13 (a)); runtime reset drops them first, and its token stays byte-identical (F14.1); both init scripts and docs/runtime.md list them, with the guard re-measured on postgres 16.15; test_schema_shape reads 0001 with 0003_ and RULED_TABLES carries R2Q13 (a)'s cite; the ten migration lists, and the four test trees copied over the fixture's ledger, carry 0003. Refs opensoft/openxFactory#656. Lane: openxfactory-4 Co-Authored-By: Claude Opus 5.5 (1M context) --- deploy/compose/init-runtime-role.sh | 33 +++++++++------ deploy/kubernetes/base/init-runtime-role.sh | 33 +++++++++------ docs/runtime.md | 47 ++++++++++++--------- src/opendox/runtime/cli.py | 24 ++++++++--- src/opendox/runtime/identity.py | 20 ++++++--- tests_runtime/test_bundled_postgres.py | 4 +- tests_runtime/test_health_store.py | 2 +- tests_runtime/test_migrations_apply.py | 33 ++++++++++----- tests_runtime/test_runtime_cli.py | 4 +- tests_runtime/test_schema_shape.py | 46 +++++++++++++++----- 10 files changed, 162 insertions(+), 84 deletions(-) diff --git a/deploy/compose/init-runtime-role.sh b/deploy/compose/init-runtime-role.sh index f9931cd0..791bc1b9 100755 --- a/deploy/compose/init-runtime-role.sh +++ b/deploy/compose/init-runtime-role.sh @@ -4,14 +4,17 @@ # exactly once per data volume and never against an existing database. # # WHY TWO ROLES AT ALL. `opendox-runtime runtime migrate` applies DDL and needs to own -# the schema; the served API needs to read and write six tables and must not be +# the schema; the served API needs to read and write the domain's tables +# (RULING Q1's six and the two health tables, R2Q13 (a)) and must not be # able to drop one. A single role makes the API's blast radius the whole # schema for no benefit, and the compose file's separate `migrate` service # exists precisely so the two identities are never in the same container. # # The runtime role is granted on the tables that EXIST at migration time and on # everything created later by the migration owner (`alter default privileges`), -# so a 0003 that adds a table does not need this file edited. +# so a later migration's table needs no GRANT here. Its NAME still joins the +# two lists below, which `tests_runtime/test_deploy_shape.py` derives from +# `identity.TABLES`: `0003_`'s two health tables did (plan 038 T042). # # AND `alter default privileges` IS AIMED AT THE MIGRATION OWNER BY NAME. # Default privileges belong to the role that CREATES the object, and this @@ -121,19 +124,20 @@ select format('grant usage on schema %I to %I', :'schema', :'runtime_user') -- THE COORDINATION TABLES THAT ALREADY EXIST, and only those. A database -- migrated before this prerequisite ran already holds them, and a `grant … on -- table` for one that is absent is an error — so the list is a JOIN against --- the catalogue rather than seven statements, and it emits NOTHING on a fresh --- database. It used to read `on all tables in schema public`, which on a +-- the catalogue rather than one statement per table, and it emits NOTHING on a +-- fresh database. It used to read `on all tables in schema public`, which on a -- REUSED database handed the served role another application's data (Copilot --- review of openDox-code#25, round 30). The names are Q1's six plus the --- ledger; `tests_runtime/test_deploy_shape.py` derives them from --- `identity.TABLES` and `migrations.LEDGER_TABLE` so this list cannot drift. +-- review of openDox-code#25, round 30). The names are Q1's six, the two health +-- tables `0003_` adds (R2Q13 (a): DOMAIN) and the ledger; +-- `tests_runtime/test_deploy_shape.py` derives them from `identity.TABLES` and +-- `migrations.LEDGER_TABLE` so this list cannot drift. select format('grant select, insert, update, delete on table %I to %I', c.relname, :'runtime_user') from pg_class c join pg_namespace n on n.oid = c.relnamespace where n.nspname = :'schema' and c.relkind = 'r' - and c.relname = any (array['drafts', 'memberships', - 'opendox_schema_migrations', + and c.relname = any (array['drafts', 'health_findings', 'health_runs', + 'memberships', 'opendox_schema_migrations', 'project_repositories', 'projects', 'sessions', 'users']) \gexec @@ -143,13 +147,14 @@ select format('grant select, insert, update, delete on table %I to %I', -- select/insert/update/delete on every table that schema already held — -- another application's data, which the migration preflight explicitly -- tolerates being there (Copilot review of openDox-code#25, round 30). It was --- not even doing the job it looked like it was doing: at first start the six --- coordination tables DO NOT EXIST YET, so that grant could only ever reach +-- not even doing the job it looked like it was doing: at first start the +-- domain's tables DO NOT EXIST YET, so that grant could only ever reach -- tables this install did not create. -- -- The `alter default privileges` below is the whole grant, and it is exactly -- the narrow one: every table the MIGRATION OWNER creates from here on, which --- is the six and the ledger and nothing else. MEASURED on postgres 16.15 — +-- is the domain's tables and the ledger and nothing else. MEASURED on +-- postgres 16.15 — -- with the broad grant removed, a table created afterwards by the owner -- carries `DELETE, INSERT, SELECT, UPDATE` for the served role, and a table -- created by anybody else carries none. @@ -176,8 +181,8 @@ select format('do $refuse$ begin raise exception %L; end $refuse$', join pg_roles r on r.oid = c.relowner where n.nspname = :'schema' and c.relkind = 'r' and r.rolname = :'migration_owner' - and c.relname <> all (array['drafts', 'memberships', - 'opendox_schema_migrations', + and c.relname <> all (array['drafts', 'health_findings', 'health_runs', + 'memberships', 'opendox_schema_migrations', 'project_repositories', 'projects', 'sessions', 'users']) having count(*) > 0 diff --git a/deploy/kubernetes/base/init-runtime-role.sh b/deploy/kubernetes/base/init-runtime-role.sh index f9931cd0..791bc1b9 100755 --- a/deploy/kubernetes/base/init-runtime-role.sh +++ b/deploy/kubernetes/base/init-runtime-role.sh @@ -4,14 +4,17 @@ # exactly once per data volume and never against an existing database. # # WHY TWO ROLES AT ALL. `opendox-runtime runtime migrate` applies DDL and needs to own -# the schema; the served API needs to read and write six tables and must not be +# the schema; the served API needs to read and write the domain's tables +# (RULING Q1's six and the two health tables, R2Q13 (a)) and must not be # able to drop one. A single role makes the API's blast radius the whole # schema for no benefit, and the compose file's separate `migrate` service # exists precisely so the two identities are never in the same container. # # The runtime role is granted on the tables that EXIST at migration time and on # everything created later by the migration owner (`alter default privileges`), -# so a 0003 that adds a table does not need this file edited. +# so a later migration's table needs no GRANT here. Its NAME still joins the +# two lists below, which `tests_runtime/test_deploy_shape.py` derives from +# `identity.TABLES`: `0003_`'s two health tables did (plan 038 T042). # # AND `alter default privileges` IS AIMED AT THE MIGRATION OWNER BY NAME. # Default privileges belong to the role that CREATES the object, and this @@ -121,19 +124,20 @@ select format('grant usage on schema %I to %I', :'schema', :'runtime_user') -- THE COORDINATION TABLES THAT ALREADY EXIST, and only those. A database -- migrated before this prerequisite ran already holds them, and a `grant … on -- table` for one that is absent is an error — so the list is a JOIN against --- the catalogue rather than seven statements, and it emits NOTHING on a fresh --- database. It used to read `on all tables in schema public`, which on a +-- the catalogue rather than one statement per table, and it emits NOTHING on a +-- fresh database. It used to read `on all tables in schema public`, which on a -- REUSED database handed the served role another application's data (Copilot --- review of openDox-code#25, round 30). The names are Q1's six plus the --- ledger; `tests_runtime/test_deploy_shape.py` derives them from --- `identity.TABLES` and `migrations.LEDGER_TABLE` so this list cannot drift. +-- review of openDox-code#25, round 30). The names are Q1's six, the two health +-- tables `0003_` adds (R2Q13 (a): DOMAIN) and the ledger; +-- `tests_runtime/test_deploy_shape.py` derives them from `identity.TABLES` and +-- `migrations.LEDGER_TABLE` so this list cannot drift. select format('grant select, insert, update, delete on table %I to %I', c.relname, :'runtime_user') from pg_class c join pg_namespace n on n.oid = c.relnamespace where n.nspname = :'schema' and c.relkind = 'r' - and c.relname = any (array['drafts', 'memberships', - 'opendox_schema_migrations', + and c.relname = any (array['drafts', 'health_findings', 'health_runs', + 'memberships', 'opendox_schema_migrations', 'project_repositories', 'projects', 'sessions', 'users']) \gexec @@ -143,13 +147,14 @@ select format('grant select, insert, update, delete on table %I to %I', -- select/insert/update/delete on every table that schema already held — -- another application's data, which the migration preflight explicitly -- tolerates being there (Copilot review of openDox-code#25, round 30). It was --- not even doing the job it looked like it was doing: at first start the six --- coordination tables DO NOT EXIST YET, so that grant could only ever reach +-- not even doing the job it looked like it was doing: at first start the +-- domain's tables DO NOT EXIST YET, so that grant could only ever reach -- tables this install did not create. -- -- The `alter default privileges` below is the whole grant, and it is exactly -- the narrow one: every table the MIGRATION OWNER creates from here on, which --- is the six and the ledger and nothing else. MEASURED on postgres 16.15 — +-- is the domain's tables and the ledger and nothing else. MEASURED on +-- postgres 16.15 — -- with the broad grant removed, a table created afterwards by the owner -- carries `DELETE, INSERT, SELECT, UPDATE` for the served role, and a table -- created by anybody else carries none. @@ -176,8 +181,8 @@ select format('do $refuse$ begin raise exception %L; end $refuse$', join pg_roles r on r.oid = c.relowner where n.nspname = :'schema' and c.relkind = 'r' and r.rolname = :'migration_owner' - and c.relname <> all (array['drafts', 'memberships', - 'opendox_schema_migrations', + and c.relname <> all (array['drafts', 'health_findings', 'health_runs', + 'memberships', 'opendox_schema_migrations', 'project_repositories', 'projects', 'sessions', 'users']) having count(*) > 0 diff --git a/docs/runtime.md b/docs/runtime.md index e1320b1f..a104e47f 100644 --- a/docs/runtime.md +++ b/docs/runtime.md @@ -24,7 +24,11 @@ ## 1. What this runtime is, and what it is not It is identity and coordination: six tables, six collections, one bearer token -verified against the Keycloak broker. +verified against the Keycloak broker. Beside the six, `0003_` adds the health +store's two tables (`health_runs`, `health_findings`; R2Q13 (a) makes them +DOMAIN tables, so `runtime reset` and the served role's grants reach them). They +serve no collection: the hosted plane refuses `health` by name (R2Q15 (a)), and +they hold findings that address documents, never the documents themselves. It is **not** a document store and it is not a second dashboard. It does not import `opendox.serve` — the stdlib document surface a student runs from a @@ -307,7 +311,7 @@ can remove a base resource, which is why the instruction ships with one. suggestion.** The bundled Postgres creates and grants that role on its first start (`deploy/compose/init-runtime-role.sh`, mounted by the StatefulSet); a managed database runs no init script, so nothing else will. Without it the -migration owner creates the six tables and the served role has no privilege on +migration owner creates the domain's tables and the served role has no privilege on any of them: the Job succeeds, `/readyz` reports a database that answers and an applied schema, and every API request then fails with `permission denied for table …` (Copilot review of openDox-code#25, round 10). Run this once, on the @@ -424,19 +428,20 @@ select format('grant usage on schema %I to %I', :'schema', :'runtime_role') -- THE COORDINATION TABLES THAT ALREADY EXIST, and only those. A database -- migrated before this prerequisite ran already holds them, and a `grant … on -- table` for one that is absent is an error — so the list is a JOIN against --- the catalogue rather than seven statements, and it emits NOTHING on a fresh --- database. It used to read `on all tables in schema public`, which on a +-- the catalogue rather than one statement per table, and it emits NOTHING on a +-- fresh database. It used to read `on all tables in schema public`, which on a -- REUSED database handed the served role another application's data (Copilot --- review of openDox-code#25, round 30). The names are Q1's six plus the --- ledger; `tests_runtime/test_deploy_shape.py` derives them from --- `identity.TABLES` and `migrations.LEDGER_TABLE` so this list cannot drift. +-- review of openDox-code#25, round 30). The names are Q1's six, the two health +-- tables `0003_` adds (R2Q13 (a): DOMAIN) and the ledger; +-- `tests_runtime/test_deploy_shape.py` derives them from `identity.TABLES` and +-- `migrations.LEDGER_TABLE` so this list cannot drift. select format('grant select, insert, update, delete on table %I to %I', c.relname, :'runtime_role') from pg_class c join pg_namespace n on n.oid = c.relnamespace where n.nspname = :'schema' and c.relkind = 'r' - and c.relname = any (array['drafts', 'memberships', - 'opendox_schema_migrations', + and c.relname = any (array['drafts', 'health_findings', 'health_runs', + 'memberships', 'opendox_schema_migrations', 'project_repositories', 'projects', 'sessions', 'users']) \gexec @@ -451,7 +456,8 @@ select format('grant select, insert, update, delete on table %I to %I', -- where a reused owner is plausible, so the grant is refused when the owner -- already owns a table this runtime did not create. MEASURED on postgres -- 16.15: the query returns no row for a schema with no tables and for one --- holding only the seven, and exactly one for an owner that also owns +-- holding only this runtime's nine (re-measured with `0003_`'s two health +-- tables, plan 038 T042), and exactly one for an owner that also owns -- `invoices` — and `\gexec` runs nothing when there is no row, so the -- `having` is the whole conditional. select format('do $refuse$ begin raise exception %L; end $refuse$', @@ -466,8 +472,8 @@ select format('do $refuse$ begin raise exception %L; end $refuse$', join pg_roles r on r.oid = c.relowner where n.nspname = :'schema' and c.relkind = 'r' and r.rolname = :'migration_owner' - and c.relname <> all (array['drafts', 'memberships', - 'opendox_schema_migrations', + and c.relname <> all (array['drafts', 'health_findings', 'health_runs', + 'memberships', 'opendox_schema_migrations', 'project_repositories', 'projects', 'sessions', 'users']) having count(*) > 0 @@ -617,7 +623,8 @@ a superseded revision loses its dispatch rather than overwriting the other one. ## 7. `reset`, and what "disposable" does and does not cover `opendox-runtime runtime reset --confirm yes-drop-the-coordination-database` -drops the six coordination tables and the ledger. It exists because of RULING +drops the six coordination tables, the two health tables and the ledger (the +token is unchanged: #1144's F14.1 spells it). It exists because of RULING Q1's last sentence — "the database is disposable relative to the corpus"; lose it and you lose coordination state, not a governed artifact. @@ -693,18 +700,20 @@ because nothing is prepending anything to it. ## 8a. The lifecycle, run end to end (2026-09-16, measured) Not a test: the installed console script, a real Postgres and a real socket. -Every figure below is that run's own output. +Every figure below is that run's own output, except the migration lists and +`dropped`, which were re-measured on 2026-10-08 against postgres 16.15 when +`0003_health.sql` joined the set (plan 038 T042), with the same verbs. ``` $ opendox-runtime runtime init { "ok": true, "canonical_sha256": "6db4710578b012a3318…", "directories_created": ["/tmp/smoke-projects"], - "migrations_on_disk": ["0001","0002"], "next": "opendox-runtime runtime migrate" } + "migrations_on_disk": ["0001","0002","0003"], "next": "opendox-runtime runtime migrate" } $ opendox-runtime runtime migrate -{ "ok": true, "applied": ["0001","0002"], "planned": [] } +{ "ok": true, "applied": ["0001","0002","0003"], "planned": [] } $ opendox-runtime runtime status --probe-timeout 5 -{ "ok": false, "database": "reachable", "applied_migrations": ["0001","0002"], +{ "ok": false, "database": "reachable", "applied_migrations": ["0001","0002","0003"], "pending_migrations": [], "migration_drift": [], "broker_keys": "unreachable: IdentityUnavailableError" } @@ -716,8 +725,8 @@ GET /docs 404 # off unless OPENDOX_PUBLISH_OPENAPI GET /api/v1/projects 401 # no bearer token $ opendox-runtime runtime reset --confirm yes-drop-the-coordination-database -{ "ok": true, "dropped": ["drafts","sessions","project_repositories","memberships","projects","users", - "opendox_schema_migrations"] } +{ "ok": true, "dropped": ["health_findings","health_runs","drafts","sessions","project_repositories", + "memberships","projects","users","opendox_schema_migrations"] } ``` **`status` and `/readyz` are `ok: false` / `not-ready` on purpose here**: there diff --git a/src/opendox/runtime/cli.py b/src/opendox/runtime/cli.py index ad15d616..4df56d1b 100644 --- a/src/opendox/runtime/cli.py +++ b/src/opendox/runtime/cli.py @@ -113,7 +113,10 @@ #: same reason and read by the same test. PROJECT_VERBS: tuple[str, ...] = ("create-repository", "attach-remote", "push") -#: What `reset` will not do without being told twice. +#: What `reset` will not do without being told twice. BYTE-IDENTICAL whatever +#: the verb drops: #1144's F14.1 spells this phrase in its ratified command, so +#: the health tables joining `DROP_ORDER` (plan 038 T042) changed the notes +#: around it and never the phrase. RESET_CONFIRMATION = "yes-drop-the-coordination-database" #: THE DROP ORDER, TOPOLOGICAL AND WRITTEN OUT. `reversed(identity.TABLES)` @@ -127,6 +130,11 @@ #: referenced these tables with it; the point of an explicit order is that a #: table this list does not know about keeps the drop honest by failing it. DROP_ORDER: tuple[str, ...] = ( + # THE HEALTH STORE FIRST (`0003_`; R2Q13 (a): DOMAIN, and as disposable as + # the six, #1144 box 14.3). A finding references its run and nothing else + # references either. + "health_findings", # references health_runs + "health_runs", "drafts", # references sessions, projects "sessions", # references users, projects "project_repositories", # references projects @@ -873,7 +881,9 @@ def cmd_reset(args: argparse.Namespace) -> int: THIS VERB CAN EXIST AT ALL BECAUSE OF RULING Q1's LAST SENTENCE: "the database is disposable relative to the corpus" — "lose it and you lose coordination state, not a governed artifact" (design § D5). It drops the - six coordination tables and the migration ledger and nothing else; every + six coordination tables, the two health tables (`0003_`; R2Q13 (a) makes + them DOMAIN, and #1144 box 14.3 keeps them as disposable: every result is + recomputable from git) and the migration ledger, and nothing else; every document ever written is in a repository and is not reachable from here. The confirmation is a SPELLED PHRASE and not a `--force` flag, because a @@ -943,7 +953,7 @@ def cmd_reset(args: argparse.Namespace) -> int: # on postgres 16.15; the refusal names both schemas. # # AND THE SERVED-SCHEMA DECLARATION IS ASKED BEFORE THE - # FIRST DROP. This verb DELETES the six coordination + # FIRST DROP. This verb DELETES the coordination and health # tables in the schema its own DSN selects, and it never # asked: a mispointed migration DSN plus a confirmed # `reset` dropped another schema's coordination state @@ -954,7 +964,7 @@ def cmd_reset(args: argparse.Namespace) -> int: # AND THE DATABASE DECLARATION BEFORE THE SCHEMA'S, on # the verb that cannot be undone: a schema comparison made # in the wrong DATABASE compares two names that happen to - # agree, and this is the act that DROPS six tables. + # agree, and this is the act that DROPS the domain's tables. migrations.refuse_a_database_the_api_will_not_read( lock, settings.served_database) migrations.refuse_a_schema_the_api_will_not_read( @@ -977,8 +987,10 @@ def cmd_reset(args: argparse.Namespace) -> int: "message": _safe_message(exc)}, ok=False) return _emit({"verb": "reset", "schema": schema, "dropped": list(DROP_ORDER), - "note": "coordination state only, in this schema alone; " - "every document is in a repository (RULING Q1)"}, + "note": "coordination state and health results only, in " + "this schema alone; every document is in a " + "repository (RULING Q1), and every health result is " + "recomputable from it (#1144 box 14.3)"}, ok=True) diff --git a/src/opendox/runtime/identity.py b/src/opendox/runtime/identity.py index 192ac24c..7a247235 100644 --- a/src/opendox/runtime/identity.py +++ b/src/opendox/runtime/identity.py @@ -6,8 +6,11 @@ in the openDox database. Specs, changes, ideation documents and contracts stay in git, read from repositories and written back only through the apply lane." -So this module has exactly six record types and one store, and `TABLES` below -is the closed list a test reads against `migrations/0001_…sql`. There is no +So this module has exactly six record types and one store. `TABLES` below is +the DOMAIN's closed table list: those six, and the two health tables +`migrations/0003_health.sql` declares and `opendox.runtime.health_store` reads +and writes, which R2Q13 (a) adds (opensoft/openxFactory#656 comment +6003486656). A test reads it against `0001` with `0003_`. There is no `documents` table, no `specs` table and no `content` column outside `drafts.body` — the one Q1 rules IN, because an unsaved draft is by definition the text that has not entered the corpus yet. @@ -51,9 +54,14 @@ from .config import credential_in_a_remote_url, redacted_remote_url -#: The closed table list, in RULING Q1's own order. Read by -#: `tests_runtime/test_schema_shape.py` against the canonical migration: a -#: seventh table here or there is a claim about what the database owns. +#: The closed DOMAIN table list: RULING Q1's six in the ruling's own order, then +#: the two health tables. R2Q13 (a) (Brett Heap, opensoft/openxFactory#656 +#: comment 6003486656) puts them here: "The results table joins Q1's list in +#: `identity.TABLES`, and the closure test reads `0001` together with `0003_`, +#: in the same change". Read by `tests_runtime/test_schema_shape.py` against +#: `0001` with `0003_`, and through this tuple by `runtime reset`, the init +#: scripts' grants and `MigrationRunner.verify_runtime_access`: a table here or +#: there is a claim about what the database owns. TABLES: tuple[str, ...] = ( "users", "memberships", @@ -61,6 +69,8 @@ "project_repositories", "sessions", "drafts", + "health_runs", + "health_findings", ) #: The closed role vocabulary, matching `memberships_role_check` in diff --git a/tests_runtime/test_bundled_postgres.py b/tests_runtime/test_bundled_postgres.py index c8dfa688..2a9144d6 100644 --- a/tests_runtime/test_bundled_postgres.py +++ b/tests_runtime/test_bundled_postgres.py @@ -537,7 +537,7 @@ def test_a_second_entry_point_on_the_same_state_dir_is_refused( bundle_mod.BundledServer(settings).start() assert str(first.report()["pid"]) in str(caught.value) # and a restart of the one that owns it re-migrates nothing - assert first.applied == ["0001", "0002"] + assert first.applied == ["0001", "0002", "0003"] finally: first.stop() again = bundle_mod.BundledServer(settings).start() @@ -921,4 +921,4 @@ def test_a_wheel_install_migrates_its_bundled_server_outside_a_checkout( env=env, capture_output=True, text=True, timeout=120) assert done.returncode == 0, done.stderr[-3000:] result = json.loads(done.stdout.strip().splitlines()[-1]) - assert result["applied"] == ["0001", "0002"], result + assert result["applied"] == ["0001", "0002", "0003"], result diff --git a/tests_runtime/test_health_store.py b/tests_runtime/test_health_store.py index 0dba4dcb..54dca00e 100644 --- a/tests_runtime/test_health_store.py +++ b/tests_runtime/test_health_store.py @@ -305,7 +305,7 @@ def test_every_not_null_finding_column_refuses_a_null(database: Any, column: str) -> None: run = _record(database) refused = _raw(database, _RAW_FINDING, - _raw_finding(run.run_id, **{column: None})) + _raw_finding(run.run_id) | {column: None}) assert refused is not None, f"health_findings.{column} admitted a null" assert refused.sqlstate == "23502", refused assert refused.diag.column_name == column diff --git a/tests_runtime/test_migrations_apply.py b/tests_runtime/test_migrations_apply.py index 2afc0fe1..8b14fc48 100644 --- a/tests_runtime/test_migrations_apply.py +++ b/tests_runtime/test_migrations_apply.py @@ -68,8 +68,8 @@ def test_the_plan_is_empty_after_apply_and_full_before_it( with Database(postgres_dsn, schema=schema) as db: runner = migrations.MigrationRunner( db, migrations_dir=ROOT / "migrations") - assert [m.version for m in runner.plan()] == ["0001", "0002"] - assert runner.apply() == ["0001", "0002"] + assert [m.version for m in runner.plan()] == ["0001", "0002", "0003"] + assert runner.apply() == ["0001", "0002", "0003"] assert runner.plan() == [] finally: with admin.transaction() as conn: @@ -110,8 +110,11 @@ def test_a_tampered_canonical_migration_applies_nothing_at_all( def test_editing_an_applied_migration_refuses_rather_than_reapplying( database, tmp_path: Path) -> None: """Drift between the tree and the database is reported, not papered over.""" + # THE WHOLE TREE THE `database` FIXTURE APPLIED, `0003_health.sql` + # included: its ledger records `0003`, and a copy without it is a tree + # missing an applied migration, which the runner refuses by name. for name in ("0001_identity_and_coordination.sql", - "0002_migration_state.sql"): + "0002_migration_state.sql", "0003_health.sql"): shutil.copyfile(ROOT / "migrations" / name, tmp_path / name) edited = tmp_path / "0002_migration_state.sql" edited.write_text(edited.read_text(encoding="utf-8") + "\n-- edited\n", @@ -125,8 +128,11 @@ def test_editing_an_applied_migration_refuses_rather_than_reapplying( def test_a_later_migration_is_applied_in_numeric_order( database, tmp_path: Path) -> None: """`0010` after `0002`, which is the whole point of numeric discovery.""" + # THE WHOLE TREE THE `database` FIXTURE APPLIED, `0003_health.sql` + # included: its ledger records `0003`, and a copy without it is a tree + # missing an applied migration, which the runner refuses by name. for name in ("0001_identity_and_coordination.sql", - "0002_migration_state.sql"): + "0002_migration_state.sql", "0003_health.sql"): shutil.copyfile(ROOT / "migrations" / name, tmp_path / name) (tmp_path / "0010_additive_probe.sql").write_text( "alter table projects add column probe text;\n", encoding="utf-8") @@ -357,7 +363,7 @@ def test_status_reports_a_reachable_database_and_its_applied_migrations( assert report["database"] == "reachable", report # The fixture applied both migrations into this test's own schema; `status` # reads them through the same search path. - assert report["applied_migrations"] == ["0001", "0002"], report + assert report["applied_migrations"] == ["0001", "0002", "0003"], report assert report["pending_migrations"] == [], report @@ -365,8 +371,11 @@ def test_drift_sees_what_plan_cannot(database, tmp_path: Path) -> None: """`plan()` compares VERSIONS. A changed or deleted file is invisible to it and is refused by `apply()`, so readiness that trusted `plan()` alone called a database healthy that the runner would not migrate.""" + # THE WHOLE TREE THE `database` FIXTURE APPLIED, `0003_health.sql` + # included: its ledger records `0003`, and a copy without it is a tree + # missing an applied migration, which the runner refuses by name. for name in ("0001_identity_and_coordination.sql", - "0002_migration_state.sql"): + "0002_migration_state.sql", "0003_health.sql"): shutil.copyfile(ROOT / "migrations" / name, tmp_path / name) runner = migrations.MigrationRunner(database, migrations_dir=tmp_path) assert runner.plan() == [] @@ -428,7 +437,7 @@ def test_the_ledger_is_resolved_in_this_schema_and_not_through_public( assert runner.applied() == [], ( "the ledger was read through `public`; a fresh schema " "would be reported fully migrated") - assert [m.version for m in runner.plan()] == ["0001", "0002"] + assert [m.version for m in runner.plan()] == ["0001", "0002", "0003"] finally: with admin.transaction() as conn: conn.execute(f"drop schema if exists {schema} cascade") @@ -734,7 +743,7 @@ def _status() -> tuple[int, dict]: code, report = _status() assert report["database"] == "reachable" - assert report["pending_migrations"] == ["0001", "0002"] + assert report["pending_migrations"] == ["0001", "0002", "0003"] assert report["ok"] is False and code == 1, report # And a tree the runner cannot read blames the TREE. @@ -827,8 +836,10 @@ def test_a_migrations_directory_rewritten_mid_run_runs_the_bytes_it_gated( from opendox.runtime.db import Database + # EVERY MIGRATION, `0003_health.sql` included: the assertion below holds + # the run to `identity.TABLES`, which names the health tables (R2Q13 (a)). for name in ("0001_identity_and_coordination.sql", - "0002_migration_state.sql"): + "0002_migration_state.sql", "0003_health.sql"): shutil.copyfile(ROOT / "migrations" / name, tmp_path / name) canonical = tmp_path / "0001_identity_and_coordination.sql" @@ -849,7 +860,7 @@ def _rewrite_then_bootstrap(conn=None): runner.bootstrap_ledger = _rewrite_then_bootstrap # type: ignore[method-assign] applied = runner.apply() - assert applied == ["0001", "0002"], applied + assert applied == ["0001", "0002", "0003"], applied assert canonical.read_bytes() == b"create table intruder ();\n", ( "the rewrite did not happen, so this test measured nothing") present = _tables_in(db, schema) @@ -993,7 +1004,7 @@ def test_the_access_preflight_asks_about_this_runtimes_tables_and_the_schema( # It applies: the stranger's table is not this runtime's to # have rights on. Against the previous head this raised # `RuntimeAccessMissingError` naming `somebody_elses`. - assert runner.apply() == ["0001", "0002"] + assert runner.apply() == ["0001", "0002", "0003"] # AND THE SCHEMA'S OWN `usage` IS ASKED. Take it away and the same # run refuses, naming the schema — against the previous head it diff --git a/tests_runtime/test_runtime_cli.py b/tests_runtime/test_runtime_cli.py index c3e13bb6..bdda554c 100644 --- a/tests_runtime/test_runtime_cli.py +++ b/tests_runtime/test_runtime_cli.py @@ -274,7 +274,7 @@ def test_init_creates_the_project_repository_root_and_touches_no_database( assert root.is_dir() assert evidence["directories_created"] == [str(root)] assert evidence["canonical_sha256"] == migrations.CANONICAL_MIGRATION_SHA256 - assert evidence["migrations_on_disk"] == ["0001", "0002"] + assert evidence["migrations_on_disk"] == ["0001", "0002", "0003"] def test_status_redacts_every_secret_setting( @@ -331,6 +331,8 @@ def test_the_drop_order_is_topological_and_not_reversed_declaration_order() -> N references it. """ references = { + "health_findings": {"health_runs"}, + "health_runs": set(), "drafts": {"sessions", "projects"}, "sessions": {"users", "projects"}, "project_repositories": {"projects"}, diff --git a/tests_runtime/test_schema_shape.py b/tests_runtime/test_schema_shape.py index e4fdc9de..58244a23 100644 --- a/tests_runtime/test_schema_shape.py +++ b/tests_runtime/test_schema_shape.py @@ -30,11 +30,20 @@ ROOT = Path(__file__).resolve().parents[1] CANONICAL = ROOT / "migrations" / "0001_identity_and_coordination.sql" - -#: Everything outside a `--` comment. Every assertion below reads THIS, so a -#: word in a comment can neither create a table nor hide one. +#: The health store's tables, declared additively (#1144 box 14.1). R2Q13 (a) +#: (opensoft/openxFactory#656 comment 6003486656): "the closure test reads +#: `0001` together with `0003_`, in the same change". `0002` is not read: it +#: declares the ledger, which is infrastructure and not a table the domain +#: owns (`test_the_ledger_is_not_declared_in_the_canonical_migration`). +HEALTH = ROOT / "migrations" / "0003_health.sql" + +#: Everything outside a `--` comment, in `0001` and `0003_` together. Every +#: assertion below reads THIS, so a word in a comment can neither create a +#: table nor hide one. def _statements() -> str: - return "\n".join(line for line in CANONICAL.read_text(encoding="utf-8").splitlines() + return "\n".join(line + for declaring in (CANONICAL, HEALTH) + for line in declaring.read_text(encoding="utf-8").splitlines() if not line.lstrip().startswith("--")) @@ -62,22 +71,36 @@ def _columns_of(table: str) -> list[tuple[str, str]]: #: RULING Q1's list, transcribed from the comment and in the ruling's own #: order, so this file does not read the closure off the very module it is #: checking. "Users, memberships, projects, the project-to-repository mapping, -#: sessions and unsaved drafts live in the openDox database." +#: sessions and unsaved drafts live in the openDox database." THEN THE TWO +#: HEALTH TABLES, transcribed from R2Q13 (a) (Brett Heap, opensoft/ +#: openxFactory#656 comment 6003486656): "DOMAIN. The results table joins Q1's +#: list in `identity.TABLES`, and the closure test reads `0001` together with +#: `0003_`, in the same change, as the ruling's words have it" — ruling +#: `5784155201` item 4's "health results become a seventh table … with the +#: closure test moved in the SAME change". Q1's principle stands: no document, +#: and a disposable store (#1144 box 14.3). RULED_TABLES: tuple[str, ...] = ( "users", "memberships", "projects", "project_repositories", "sessions", "drafts", + "health_runs", "health_findings", ) def test_the_store_declares_exactly_the_tables_the_ruling_names() -> None: assert identity.TABLES == RULED_TABLES, ( - "`identity.TABLES` and RULING Q1's own list disagree; the ruling is " - "#656 comment 5542694957 and this tuple is transcribed from it") + "`identity.TABLES` and the ruled list disagree; RULING Q1 is #656 " + "comment 5542694957, R2Q13 (a) is #656 comment 6003486656, and this " + "tuple is transcribed from them") def test_the_canonical_schema_declares_exactly_rulings_six_tables() -> None: """The list is CLOSED. Compared as a SET, deliberately. + The NAME keeps #1144 box 14.2's citation of it. What it reads is `0001` + and `0003_` together, held to RULING Q1's six and R2Q13 (a)'s two: the + seventh-table claim box 14.2 asks to see made in the open is made in + `RULED_TABLES`, in `identity.TABLES` and in `0003_`'s header, in one change. + The SQL's declaration order is fixed by the foreign keys — `projects` has to exist before `memberships` references it — so it is not free to be the ruling's reading order, and a test that demanded it would be asserting @@ -88,10 +111,11 @@ def test_the_canonical_schema_declares_exactly_rulings_six_tables() -> None: declared = _declared_tables() assert len(declared) == len(set(declared)) == len(RULED_TABLES) assert set(declared) == set(RULED_TABLES), ( - "the canonical migration's tables and RULING Q1's list disagree. " - "RULING Q1 names six things the database owns; a seventh table is a " - "claim about that boundary and has to be made in the open — in the " - "ruling, then in this file, then in `identity.TABLES`.") + "the tables `0001` and `0003_` declare and the ruled list disagree. " + "RULING Q1 names six things the database owns and R2Q13 (a) adds the " + "two health tables; another table is a claim about that boundary and " + "has to be made in the open — in a ruling, then in this file, then in " + "`identity.TABLES`.") def test_no_table_is_a_document_store() -> None: From 56d95d4f93b2a45cb6164fa45443065b59c4fbe7 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Thu, 8 Oct 2026 19:39:00 +0000 Subject: [PATCH 03/12] T042: a restarted sequence cannot record a second run at one run_seq (kills the run_seq-unique mutant) Lane: openxfactory-4 Co-Authored-By: Claude Opus 5.5 (1M context) --- tests_runtime/test_health_store.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests_runtime/test_health_store.py b/tests_runtime/test_health_store.py index 54dca00e..bdee46aa 100644 --- a/tests_runtime/test_health_store.py +++ b/tests_runtime/test_health_store.py @@ -644,3 +644,30 @@ def test_runtime_reset_drops_a_populated_health_store( "select table_name from information_schema.tables " "where table_schema = current_schema()").fetchall() assert left == [], f"the reset left {[row[0] for row in left]}" + + +def test_a_restarted_sequence_cannot_record_a_second_run_at_one_position( + database: Any) -> None: + """`run_seq` is the order of record, so it is UNIQUE and not only generated. + + An identity column is not unique by itself: a sequence restarted after a + restore, or an insert `overriding system value`, hands out a number that + is already taken, and "the latest earlier run" would then have two + answers. The unique constraint refuses the second, and the store reports + it as the conflict it is. + """ + first = _record(database) + with database.transaction() as conn: + conn.execute("alter table health_runs alter column run_seq restart with " + f"{first.run_seq}") + with pytest.raises(ConflictError): + _record(database) + refused = _raw(database, + _RAW_RUN.replace("(run_id, ", "(run_seq, run_id, ") + .replace(" values (", " overriding system value values " + "(%(run_seq)s, "), + _raw_run(run_seq=first.run_seq)) + assert refused is not None, "a second run was recorded at one run_seq" + assert refused.sqlstate == "23505", refused # unique_violation + assert refused.diag.constraint_name == "health_runs_run_seq_key" + assert _count(database, "health_runs") == 1 From 0c5e3cc12d83f2d16b78c54649e9c2b3429504df Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Thu, 8 Oct 2026 19:51:58 +0000 Subject: [PATCH 04/12] T042: name the unique conflict by its constraint; non-empty provenance as length(...) > 0; JSON null held by the object checks Copilot's and Codex's reviews of #99 at 56d95d4f: a run_seq conflict was reported as a finding collision, so each unique constraint now has its own message. Copilot's JSON-null finding does not hold on postgres 16.15 (jsonb_typeof('null'::jsonb) is the text 'null'), and five cases now hold that. SonarCloud's plsql:NullComparison read pack_id <> '' with Oracle's empty-string rule; length(pack_id) > 0 is the same check in PostgreSQL. The four python:S5778 test blocks now hold one raising call each. Lane: openxfactory-4 Co-Authored-By: Claude Opus 5.5 (1M context) --- migrations/0003_health.sql | 8 ++++--- src/opendox/runtime/health_store.py | 27 ++++++++++++++++++----- tests_runtime/test_health_store.py | 34 ++++++++++++++++++++++------- 3 files changed, 53 insertions(+), 16 deletions(-) diff --git a/migrations/0003_health.sql b/migrations/0003_health.sql index 069d9eed..69c102a2 100644 --- a/migrations/0003_health.sql +++ b/migrations/0003_health.sql @@ -25,7 +25,9 @@ -- -- PROVENANCE IS NOT NULL FROM THE FIRST LANDING (box 15.7): `pack_id` and -- `pack_version` are `not null`, and not empty, so no path can store a finding --- without them. That is enforced HERE, at the store, and not only in the +-- without them. "Not empty" is spelled `length(…) > 0` rather than `<> ''`, +-- which means the same in PostgreSQL but reads as a null comparison to +-- analyzers that apply Oracle's rule that `''` is null. That is enforced HERE, at the store, and not only in the -- engine that stamps them. -- -- NO FOREIGN KEY TO `projects` (OQ-H-22): a run is keyed by the corpus's @@ -113,8 +115,8 @@ create table health_findings ( message text not null, evidence jsonb not null, constraint health_findings_pkey primary key (run_id, id), - constraint health_findings_pack_id_check check (pack_id <> ''), - constraint health_findings_pack_version_check check (pack_version <> ''), + constraint health_findings_pack_id_check check (length(pack_id) > 0), + constraint health_findings_pack_version_check check (length(pack_version) > 0), constraint health_findings_identity_check check (jsonb_typeof(identity::jsonb) = 'object'), constraint health_findings_locator_check check (locator is null or jsonb_typeof(locator) = 'object'), constraint health_findings_severity_check check (severity in ('error', 'warning', 'info')), diff --git a/src/opendox/runtime/health_store.py b/src/opendox/runtime/health_store.py index 8ccdc6ea..f670bee6 100644 --- a/src/opendox/runtime/health_store.py +++ b/src/opendox/runtime/health_store.py @@ -161,6 +161,22 @@ def _uuid_text(run_id: str) -> str: "no health run with that run_id (it is not a uuid)") from exc +#: What a UNIQUE violation means, by the constraint that raised it: two keys, +#: two different causes, and a caller acts on each differently (Copilot's and +#: Codex's reviews of openDox-code#99 at `56d95d4f`). +_UNIQUE_CONFLICTS: dict[str, str] = { + "health_findings_pkey": ( + "two findings of this run share one id. The engine records such a " + "collision as ONE finding against their producer and stores neither " + "(contracts/health-finding.md, the id rule); the run is not recorded"), + "health_runs_run_seq_key": ( + "the run sequence handed out a run_seq that a recorded run already " + "holds: the identity sequence is behind the recorded runs (a restore " + "or a restart). Move it past the highest recorded run_seq; the run is " + "not recorded"), +} + + def _refused_by_the_schema(call: Any, what: str) -> Any: """Run `call`, turning the schema's refusals into this store's errors. @@ -173,11 +189,12 @@ def _refused_by_the_schema(call: Any, what: str) -> Any: except Exception as exc: # noqa: BLE001 sqlstate = getattr(exc, "sqlstate", None) if sqlstate == UNIQUE_VIOLATION: - raise ConflictError( - f"{what}: two rows share one key. Two findings of one run with " - "one id are a collision the engine records as ONE finding " - "against their producer and stores neither " - "(contracts/health-finding.md, the id rule)") from exc + constraint = getattr(getattr(exc, "diag", None), + "constraint_name", None) + raise ConflictError(_UNIQUE_CONFLICTS.get( + constraint, + f"{what}: a row with this key is already recorded " + f"({constraint or 'a unique key'})")) from exc if sqlstate == FOREIGN_KEY_VIOLATION: raise NotFoundError( f"{what}: the run it names does not exist") from exc diff --git a/tests_runtime/test_health_store.py b/tests_runtime/test_health_store.py index bdee46aa..f25ffd62 100644 --- a/tests_runtime/test_health_store.py +++ b/tests_runtime/test_health_store.py @@ -192,8 +192,10 @@ def test_the_store_refuses_a_finding_without_provenance( finding.update(missing) field = next(iter(missing)) + no_statement = HealthStore(_NoStatement()) + kwargs = _run_kwargs(findings=[finding]) with pytest.raises(RefusedError) as caught: - HealthStore(_NoStatement()).record_run(**_run_kwargs(findings=[finding])) + no_statement.record_run(**kwargs) assert f"has no {field}" in str(caught.value) assert "15.7" in str(caught.value) @@ -338,6 +340,11 @@ def test_every_not_null_finding_column_refuses_a_null(database: Any, ({"sandbox": '{"live": true, "pids_max": "512"}'}, "health_runs_sandbox_check"), ({"sandbox": "[true]"}, "health_runs_sandbox_check"), + # JSON `null` is a jsonb VALUE, not SQL NULL: `jsonb_typeof` answers the + # text 'null', so the object checks refuse it (measured on postgres 16.15 + # for Copilot's review of openDox-code#99 at `56d95d4f`). + ({"pack_pins": "null"}, "health_runs_pack_pins_check"), + ({"sandbox": "null"}, "health_runs_sandbox_check"), ] _FINDING_CHECKS = [ @@ -349,6 +356,9 @@ def test_every_not_null_finding_column_refuses_a_null(database: Any, "health_findings_resolution_class_check"), ({"baseline_class": "unclassed"}, "health_findings_baseline_class_check"), ({"evidence": "[]"}, "health_findings_evidence_check"), + ({"identity": "null"}, "health_findings_identity_check"), + ({"locator": "null"}, "health_findings_locator_check"), + ({"evidence": "null"}, "health_findings_evidence_check"), ] @@ -416,9 +426,10 @@ def test_a_finding_must_name_a_recorded_run(database: Any) -> None: def test_the_store_keeps_no_patch_and_no_document(field: str) -> None: """14.3 and R2Q25 (a), at the API: a field outside the columns is refused by NAME, before any statement — never silently dropped.""" + no_statement = HealthStore(_NoStatement()) + kwargs = _run_kwargs(findings=[_finding(**{field: "the text of a document"})]) with pytest.raises(RefusedError) as caught: - HealthStore(_NoStatement()).record_run(**_run_kwargs( - findings=[_finding(**{field: "the text of a document"})])) + no_statement.record_run(**kwargs) assert repr(field) in str(caught.value) assert "the text of a document" not in str(caught.value) @@ -450,15 +461,17 @@ def test_two_findings_with_one_id_in_one_run_store_nothing(database: Any) -> Non (contracts/health-finding.md); a run that reaches the store with two is a CONFLICT, and the caller's transaction takes the run row with it.""" twice = [_finding(), _finding(path="other.md")] - with pytest.raises(ConflictError): + with pytest.raises(ConflictError) as caught: _record(database, findings=twice) + assert "two findings of this run share one id" in str(caught.value) assert _count(database, "health_runs") == 0 assert _count(database, "health_findings") == 0 def test_a_vocabulary_the_schema_refuses_is_a_named_refusal(database: Any) -> None: + critical = [_finding(severity="critical")] with pytest.raises(RefusedError) as caught: - _record(database, findings=[_finding(severity="critical")]) + _record(database, findings=critical) assert "health_findings_severity_check" in str(caught.value) with pytest.raises(RefusedError) as caught: _record(database, kind="nightly") @@ -485,8 +498,10 @@ def test_a_vocabulary_the_schema_refuses_is_a_named_refusal(database: Any) -> No "evidence-none", "locator-list", "kind-int", "no-message"]) def test_malformed_input_is_refused_before_any_statement(over: dict, says: str) -> None: + no_statement = HealthStore(_NoStatement()) + kwargs = _run_kwargs(**over) with pytest.raises(RefusedError) as caught: - HealthStore(_NoStatement()).record_run(**_run_kwargs(**over)) + no_statement.record_run(**kwargs) assert says in str(caught.value) @@ -578,8 +593,9 @@ def test_an_unknown_run_or_finding_is_not_found(database: Any) -> None: run = _record(database) with database.connection() as conn: store = HealthStore(conn) + unknown = str(uuid.uuid4()) with pytest.raises(NotFoundError): - store.get_run(str(uuid.uuid4())) + store.get_run(unknown) with pytest.raises(NotFoundError): store.get_run("not-a-run") with pytest.raises(NotFoundError): @@ -660,8 +676,10 @@ def test_a_restarted_sequence_cannot_record_a_second_run_at_one_position( with database.transaction() as conn: conn.execute("alter table health_runs alter column run_seq restart with " f"{first.run_seq}") - with pytest.raises(ConflictError): + with pytest.raises(ConflictError) as caught: _record(database) + assert "a run_seq that a recorded run already holds" in str(caught.value) + assert "finding" not in str(caught.value) refused = _raw(database, _RAW_RUN.replace("(run_id, ", "(run_seq, run_id, ") .replace(" values (", " overriding system value values " From 808524cf62d906d56f3b9c06551b1a069d7c6a12 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Thu, 8 Oct 2026 21:06:10 +0000 Subject: [PATCH 05/12] T042: the readiness-drift test's tree carries 0003_health.sql (admitted by the holder, #656 6069024568 item 3) Its ledger, from the database fixture, records 0003, so a copy of 0001 and 0002 alone reported 0003:missing beside the one edit the test makes; the same cause as the three test_migrations_apply copy sites. Lane: openxfactory-4 Co-Authored-By: Claude Opus 5.5 (1M context) --- tests_runtime/test_api_endpoints.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests_runtime/test_api_endpoints.py b/tests_runtime/test_api_endpoints.py index 21897203..c876ffe5 100644 --- a/tests_runtime/test_api_endpoints.py +++ b/tests_runtime/test_api_endpoints.py @@ -600,8 +600,11 @@ def test_readiness_refuses_a_database_whose_migration_file_has_changed( from tests_runtime.conftest import TEST_AUDIENCE, TEST_ISSUER root = Path(__file__).resolve().parents[1] / "migrations" + # THE WHOLE TREE THE `database` FIXTURE APPLIED, `0003_health.sql` + # included (plan 038 T042): its ledger records `0003`, so a copy without + # it would report `0003:missing` beside the one edit this test makes. for name in ("0001_identity_and_coordination.sql", - "0002_migration_state.sql"): + "0002_migration_state.sql", "0003_health.sql"): shutil.copyfile(root / name, tmp_path / name) edited = tmp_path / "0002_migration_state.sql" edited.write_text(edited.read_text(encoding="utf-8") + "\n-- edited\n", From 68f70b8a22a318193fd7d71cec3fc2c6b851563c Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Fri, 9 Oct 2026 01:10:19 +0000 Subject: [PATCH 06/12] T042: refuse U+0000 by name before any statement; pids_max a non-negative integer; pin the read-back, the cascade, run_seq's identity and the page bound (REVIEW-W1) Lane 3's REVIEW-W1 of #99 (6071241577) and the holder's rulings (#656 6072086385): a NUL in any finding string, key or value, in pack_pins, in sandbox or in a run-level text field is a RefusedError naming the field and quoting nothing, raised before any statement (it was psycopg's UntranslatableCharacter or DataError); sandbox's pids_max is a non-negative whole number or null; new tests pin a pathed finding's read-back, the findings' cascade, run_seq GENERATED ALWAYS with CACHE 1 (6072086385 item 2) and list_findings' clamp_limit bound. Lane: openxfactory-4 Co-Authored-By: Claude Opus 5.5 (1M context) --- migrations/0003_health.sql | 8 +- src/opendox/runtime/health_store.py | 44 +++++++- tests_runtime/test_health_store.py | 158 ++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+), 2 deletions(-) diff --git a/migrations/0003_health.sql b/migrations/0003_health.sql index 69c102a2..ce399ae2 100644 --- a/migrations/0003_health.sql +++ b/migrations/0003_health.sql @@ -65,6 +65,12 @@ -- `export_commit` form check holds `commit` too; a second check on `commit` -- could never be the one that refuses a row. -- +-- `sandbox` IS `{"live": bool, "pids_max": int or null}` (data-model.md), and +-- an int here is a NON-NEGATIVE WHOLE number: a fraction or a negative count of +-- processes is no bound the probe can have found (lane 3's REVIEW-W1 of T042). +-- The check is a CASE so that the numeric cast is reached only for a JSON +-- number: SQL does not promise to evaluate an AND's operands in order. +-- -- EVERY STORED FINDING CARRIES ITS BASELINE CLASS (`baseline_class not -- null`). R2Q12 (a) rules exactly three classes and I-2 (a) rules out a -- fourth, and a null would be that fourth ("unclassed") by another name: the @@ -97,7 +103,7 @@ create table health_runs ( constraint health_runs_export_commit_check check (commit is null or export_commit = commit), constraint health_runs_default_tip_branch_check check (kind <> 'default-tip' or baseline_branch is not null), constraint health_runs_pack_pins_check check (jsonb_typeof(pack_pins) = 'object' and pack_pins ? 'opendox'), - constraint health_runs_sandbox_check check (coalesce(jsonb_typeof(sandbox -> 'live'), 'absent') = 'boolean' and coalesce(jsonb_typeof(sandbox -> 'pids_max'), 'absent') in ('number', 'null')) + constraint health_runs_sandbox_check check (coalesce(jsonb_typeof(sandbox -> 'live'), 'absent') = 'boolean' and case jsonb_typeof(sandbox -> 'pids_max') when 'null' then true when 'number' then (sandbox ->> 'pids_max')::numeric >= 0 and (sandbox ->> 'pids_max')::numeric = trunc((sandbox ->> 'pids_max')::numeric) else false end) ); create table health_findings ( diff --git a/src/opendox/runtime/health_store.py b/src/opendox/runtime/health_store.py index f670bee6..0bfbe3fe 100644 --- a/src/opendox/runtime/health_store.py +++ b/src/opendox/runtime/health_store.py @@ -117,12 +117,46 @@ "baseline_class text, message text, evidence jsonb") +def _carries_nul(value: Any) -> bool: + """Whether U+0000 (NUL) is anywhere in `value`: a string, a key, or nested.""" + if isinstance(value, str): + return "\x00" in value + if isinstance(value, Mapping): + return any(_carries_nul(key) or _carries_nul(item) + for key, item in value.items()) + if isinstance(value, (list, tuple)): + return any(_carries_nul(item) for item in value) + return False + + +def _refuse_nul(value: Any, what: str) -> None: + """Refuse U+0000 anywhere in `value`, BY NAME and before any statement. + + PostgreSQL stores a NUL in neither `jsonb` (SQLSTATE 22P05, raised by the + server, whose message quotes the JSON around it) nor a `text` parameter (a + client-side `DataError`), both measured on postgres 16.15. Without this the + store raised a DRIVER error for a value it could have judged, and the + jsonb one put a slice of the finding's text in the exception (lane 3's + REVIEW-W1 of T042, MINOR 1). The holder, opensoft/openxFactory#656 comment + 6072086385 item 4: the store refuses it itself, as a `RefusedError` that + names the field and quotes nothing, as defence in depth behind the contract + module's own refusal. + """ + if _carries_nul(value): + raise RefusedError( + f"{what} carries U+0000 (NUL), which PostgreSQL can store in " + "neither text nor jsonb; the store refuses it before any " + "statement, and the value is not repeated here") + + def _json_text(value: Any, what: str) -> str: """Strict JSON text for a `%s::jsonb` parameter, or a refusal naming `what`. `allow_nan=False`, because `NaN` is Python's JSON and not JSON: Postgres - would refuse the cast, and the refusal belongs here, by name. + would refuse the cast, and the refusal belongs here, by name. A NUL is + refused here too (`_refuse_nul`), in a key or a value at any depth. """ + _refuse_nul(value, what) try: return json.dumps(value, sort_keys=True, separators=(",", ":"), ensure_ascii=False, allow_nan=False) @@ -329,6 +363,8 @@ def _finding_record(index: int, finding: Any) -> dict[str, Any]: raise RefusedError( f"finding {index}'s {not_text} must be strings; the store does not " "turn another type into text for them") + for field in (*REQUIRED_FINDING_FIELDS, *OPTIONAL_FINDING_FIELDS): + _refuse_nul(finding.get(field), f"finding {index}'s {field}") evidence = finding["evidence"] if not isinstance(evidence, Mapping): raise RefusedError( @@ -381,6 +417,12 @@ def record_run(self, *, corpus_root: str, kind: str, commit: str | None, if not isinstance(sandbox, Mapping): raise RefusedError("sandbox must be a JSON object (what the " "per-run probe found)") + for name, value in (("corpus_root", corpus_root), ("kind", kind), + ("commit", commit), + ("baseline_branch", baseline_branch), + ("outcome", outcome), + ("export_commit", export_commit)): + _refuse_nul(value, name) records = [_finding_record(index, finding) for index, finding in enumerate(findings)] pins_text = _json_text(dict(pack_pins), "pack_pins") diff --git a/tests_runtime/test_health_store.py b/tests_runtime/test_health_store.py index f25ffd62..e241bba1 100644 --- a/tests_runtime/test_health_store.py +++ b/tests_runtime/test_health_store.py @@ -689,3 +689,161 @@ def test_a_restarted_sequence_cannot_record_a_second_run_at_one_position( assert refused.sqlstate == "23505", refused # unique_violation assert refused.diag.constraint_name == "health_runs_run_seq_key" assert _count(database, "health_runs") == 1 + + +# --------------------------------------------------------------------------- +# lane 3's REVIEW-W1 of T042 (#99 6071241577) and the holder's rulings on it +# (#656 6072086385) +# --------------------------------------------------------------------------- + +#: Every place a finding or a run can carry U+0000 (NUL), and the field the +#: refusal must name. PostgreSQL stores neither: `jsonb` refuses it with 22P05 +#: and a `text` parameter with a client-side `DataError` (both measured on +#: 16.15), so without the guard `record_run` raised a DRIVER error, and the +#: jsonb one quoted the finding's JSON in its `CONTEXT` line. +_NUL_CASES = [ + ({"findings": [_finding(identity={"target": "a\x00b"})]}, "finding 0's identity"), + ({"findings": [_finding(identity={"tar\x00get": "x"})]}, "finding 0's identity"), + ({"findings": [_finding(evidence={"paths": ["ok", "b\x00"]})]}, + "finding 0's evidence"), + ({"findings": [_finding(), _finding(id="opendox.orphan.0123456789abcdef", + locator={"target": "\x00"})]}, + "finding 1's locator"), + ({"findings": [_finding(message="link\x00target")]}, "finding 0's message"), + ({"findings": [_finding(path="notes/a\x00.md")]}, "finding 0's path"), + ({"pack_pins": {"opendox": {"version": "0.2.0"}, "p\x00": {"version": "1"}}}, + "pack_pins"), + ({"sandbox": {"live": True, "pids_max": None, "no\x00te": 1}}, "sandbox"), + ({"corpus_root": "/corpora/a\x00b"}, "corpus_root"), + ({"baseline_branch": "ma\x00in"}, "baseline_branch"), + ({"kind": "default-tip\x00"}, "kind"), +] + + +@pytest.mark.parametrize(("over", "field"), _NUL_CASES, + ids=["identity-value", "identity-key", "evidence-list", + "locator", "message", "path", "pack-pins-key", + "sandbox-key", "corpus-root", "baseline-branch", + "kind"]) +def test_a_nul_is_refused_by_name_before_any_statement(over: dict, + field: str) -> None: + """The holder, #656 `6072086385` item 4: the store refuses U+0000 itself. + + A `RefusedError` that names the FIELD, raised before any statement (the + connection fails the test if one reaches it), and quoting nothing: not the + NUL, and not the text around it. + """ + no_statement = HealthStore(_NoStatement()) + kwargs = _run_kwargs(**over) + with pytest.raises(RefusedError) as caught: + no_statement.record_run(**kwargs) + message = str(caught.value) + assert f"{field} carries U+0000" in message, message + assert "\x00" not in message + for quoted in ("a\x00b", "link", "notes/a", "/corpora", "ma", "tar"): + assert quoted not in message.replace("U+0000", ""), (quoted, message) + + +def test_a_nul_in_an_identity_never_reaches_the_driver(database: Any) -> None: + """Lane 3's measured case, over a real database: before this guard the run + raised `psycopg.errors.UntranslatableCharacter` (22P05) quoting the JSON.""" + nul = [_finding(identity={"target": "x\x00y"})] + with pytest.raises(RefusedError) as caught: + _record(database, findings=nul) + assert "finding 0's identity carries U+0000" in str(caught.value) + assert _count(database, "health_runs") == 0 + assert _count(database, "health_findings") == 0 + + +def test_a_pathed_finding_reads_back_field_for_field(database: Any) -> None: + """MINOR 2: a pathed finding's locator, evidence and message survive the store. + + Only the install-level case read fields back, and only their empty forms, + so a store that dropped the locator, wrote `{}` for the evidence or + rewrote the message passed every test. + """ + sent = _finding() + run = _record(database, findings=[sent]) + with database.connection() as conn: + stored = HealthStore(conn).get_finding(run_id=run.run_id, + finding_id=sent["id"]) + assert stored.locator == sent["locator"] + assert stored.evidence == sent["evidence"] + assert stored.message == sent["message"] + assert stored.path == sent["path"] + assert stored.identity == sent["identity"] + assert (stored.kind, stored.pack_id, stored.pack_version, stored.severity, + stored.resolution_class, stored.baseline_class) == ( + sent["kind"], sent["pack_id"], sent["pack_version"], sent["severity"], + sent["resolution_class"], sent["baseline_class"]) + + +def test_deleting_a_run_takes_its_findings_with_it(database: Any) -> None: + """The findings' foreign key cascades (`on delete cascade`): a run deleted + by any path leaves no orphan finding behind.""" + run = _record(database, findings=[_finding(), _finding( + id="opendox.orphan.0123456789abcdef", path="other.md")]) + kept = _record(database, findings=[_finding()]) + assert _count(database, "health_findings") == 3 + with database.transaction() as conn: + conn.execute("delete from health_runs where run_id = %s::uuid", + (run.run_id,)) + assert _count(database, "health_findings") == 1 + with database.connection() as conn: + assert [f.run_id for f in HealthStore(conn).list_findings( + run_id=kept.run_id)] == [kept.run_id] + + +def test_run_seq_is_generated_always_with_a_cache_of_one(database: Any) -> None: + """The holder, `6069024023` item 3 ("the sequence's cache stays 1") and + `6072086385` item 2: GENERATED ALWAYS, so no caller supplies a position, + and CACHE 1, so no session holds a block of positions that a later + recording would overtake.""" + with database.connection() as conn: + identity_row = conn.execute( + "select data_type, is_nullable, is_identity, identity_generation " + "from information_schema.columns where table_schema = " + "current_schema() and table_name = 'health_runs' and column_name " + "= 'run_seq'").fetchone() + sequence_row = conn.execute( + "select s.seqcache, s.seqincrement, s.seqcycle from pg_sequence s " + "where s.seqrelid = pg_get_serial_sequence('health_runs', " + "'run_seq')::regclass").fetchone() + assert tuple(identity_row) == ("bigint", "NO", "YES", "ALWAYS") + assert tuple(sequence_row) == (1, 1, False) + + +def test_a_findings_page_is_never_empty_for_a_run_with_findings( + database: Any) -> None: + """`list_findings` is bounded by `identity.clamp_limit`: a limit below one + is one row and a limit above the maximum is the maximum, never 0 rows and + never an unbounded scan.""" + from opendox.runtime.identity import MAX_PAGE_SIZE + + run = _record(database, findings=[_finding(), _finding( + id="opendox.orphan.0123456789abcdef", path="other.md")]) + with database.connection() as conn: + store = HealthStore(conn) + assert len(store.list_findings(run_id=run.run_id, limit=0)) == 1 + assert len(store.list_findings(run_id=run.run_id, limit=-5)) == 1 + assert len(store.list_findings(run_id=run.run_id, + limit=MAX_PAGE_SIZE + 1)) == 2 + + +@pytest.mark.parametrize("pids_max", ["1.5", "-1", "-0.5", "true", '"8"']) +def test_pids_max_is_a_non_negative_integer_or_null(database: Any, + pids_max: str) -> None: + """data-model.md § Health run: `pids_max` is "int or null". A fraction, a + negative number, a boolean and a string are each refused by the check.""" + refused = _raw(database, _RAW_RUN, _raw_run( + sandbox=f'{{"live": true, "pids_max": {pids_max}}}')) + assert refused is not None, f"pids_max {pids_max} was admitted" + assert refused.sqlstate == "23514", refused + assert refused.diag.constraint_name == "health_runs_sandbox_check" + + +@pytest.mark.parametrize("pids_max", ["null", "0", "1", "4096", "100000000000"]) +def test_pids_max_admits_null_and_every_non_negative_integer( + database: Any, pids_max: str) -> None: + assert _raw(database, _RAW_RUN, _raw_run( + sandbox=f'{{"live": true, "pids_max": {pids_max}}}')) is None From fbf9e77ebb8acf02a4a198379274ff552bcce47d Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Fri, 9 Oct 2026 02:46:53 +0000 Subject: [PATCH 07/12] T042: let the page cap bite in the test; cite R2Q13 (a)'s option text Lane 3's REVIEW-W1 NIT (2): the upper-bound assertion in test_a_findings_page_is_never_empty_for_a_run_with_findings could not fail on a two-finding run, so a store without the cap (mutant r6b) passed. The test now lowers identity.MAX_PAGE_SIZE to 1 and asserts a limit of 2 reads one row. NIT (4), comment-only: identity.py and test_schema_shape.py keep #656 comment 6003486656 as the ruling and cite plan 038's clarify-questions.md:651-653 for option (a)'s quoted text. Lane: openxfactory-4 Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/runtime/identity.py | 27 +++++++++++++++------------ tests_runtime/test_health_store.py | 10 ++++++++-- tests_runtime/test_schema_shape.py | 10 ++++++---- 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/opendox/runtime/identity.py b/src/opendox/runtime/identity.py index 7a247235..a7af5d3f 100644 --- a/src/opendox/runtime/identity.py +++ b/src/opendox/runtime/identity.py @@ -9,11 +9,12 @@ So this module has exactly six record types and one store. `TABLES` below is the DOMAIN's closed table list: those six, and the two health tables `migrations/0003_health.sql` declares and `opendox.runtime.health_store` reads -and writes, which R2Q13 (a) adds (opensoft/openxFactory#656 comment -6003486656). A test reads it against `0001` with `0003_`. There is no -`documents` table, no `specs` table and no `content` column outside -`drafts.body` — the one Q1 rules IN, because an unsaved draft is by definition -the text that has not entered the corpus yet. +and writes, which R2Q13 (a) adds (ruled by Brett Heap in opensoft/ +openxFactory#656 comment 6003486656; option (a)'s text is plan 038's +`clarify-questions.md:651-653`). A test reads it against `0001` with +`0003_`. There is no `documents` table, no `specs` table and no `content` +column outside `drafts.body` — the one Q1 rules IN, because an unsaved draft +is by definition the text that has not entered the corpus yet. EVERY STATEMENT IS COMPOSED FROM MODULE-LEVEL CONSTANTS AND EVERY RUNTIME VALUE IS A `%s` PARAMETER. Table and column names below are literals in this file; @@ -55,13 +56,15 @@ from .config import credential_in_a_remote_url, redacted_remote_url #: The closed DOMAIN table list: RULING Q1's six in the ruling's own order, then -#: the two health tables. R2Q13 (a) (Brett Heap, opensoft/openxFactory#656 -#: comment 6003486656) puts them here: "The results table joins Q1's list in -#: `identity.TABLES`, and the closure test reads `0001` together with `0003_`, -#: in the same change". Read by `tests_runtime/test_schema_shape.py` against -#: `0001` with `0003_`, and through this tuple by `runtime reset`, the init -#: scripts' grants and `MigrationRunner.verify_runtime_access`: a table here or -#: there is a claim about what the database owns. +#: the two health tables. R2Q13 (a), ruled by Brett Heap in opensoft/ +#: openxFactory#656 comment 6003486656, puts them here; option (a)'s text, at +#: plan 038's `clarify-questions.md:651-653`, reads: "The results table joins +#: Q1's list in `identity.TABLES`, and the closure test reads `0001` together +#: with `0003_`, in the same change". Read by +#: `tests_runtime/test_schema_shape.py` against `0001` with `0003_`, and +#: through this tuple by `runtime reset`, the init scripts' grants and +#: `MigrationRunner.verify_runtime_access`: a table here or there is a claim +#: about what the database owns. TABLES: tuple[str, ...] = ( "users", "memberships", diff --git a/tests_runtime/test_health_store.py b/tests_runtime/test_health_store.py index e241bba1..6300528f 100644 --- a/tests_runtime/test_health_store.py +++ b/tests_runtime/test_health_store.py @@ -814,10 +814,14 @@ def test_run_seq_is_generated_always_with_a_cache_of_one(database: Any) -> None: def test_a_findings_page_is_never_empty_for_a_run_with_findings( - database: Any) -> None: + database: Any, monkeypatch: pytest.MonkeyPatch) -> None: """`list_findings` is bounded by `identity.clamp_limit`: a limit below one is one row and a limit above the maximum is the maximum, never 0 rows and - never an unbounded scan.""" + never an unbounded scan. + + A run of two findings cannot show the upper bound at the real maximum + (500), so the maximum is lowered to one: a limit of two must then read + one row. Without that step, a store that dropped the cap would pass.""" from opendox.runtime.identity import MAX_PAGE_SIZE run = _record(database, findings=[_finding(), _finding( @@ -828,6 +832,8 @@ def test_a_findings_page_is_never_empty_for_a_run_with_findings( assert len(store.list_findings(run_id=run.run_id, limit=-5)) == 1 assert len(store.list_findings(run_id=run.run_id, limit=MAX_PAGE_SIZE + 1)) == 2 + monkeypatch.setattr(identity, "MAX_PAGE_SIZE", 1) + assert len(store.list_findings(run_id=run.run_id, limit=2)) == 1 @pytest.mark.parametrize("pids_max", ["1.5", "-1", "-0.5", "true", '"8"']) diff --git a/tests_runtime/test_schema_shape.py b/tests_runtime/test_schema_shape.py index 58244a23..687b629c 100644 --- a/tests_runtime/test_schema_shape.py +++ b/tests_runtime/test_schema_shape.py @@ -30,8 +30,9 @@ ROOT = Path(__file__).resolve().parents[1] CANONICAL = ROOT / "migrations" / "0001_identity_and_coordination.sql" -#: The health store's tables, declared additively (#1144 box 14.1). R2Q13 (a) -#: (opensoft/openxFactory#656 comment 6003486656): "the closure test reads +#: The health store's tables, declared additively (#1144 box 14.1). R2Q13 (a), +#: ruled by opensoft/openxFactory#656 comment 6003486656, whose option text +#: (plan 038's `clarify-questions.md:651-653`) reads: "the closure test reads #: `0001` together with `0003_`, in the same change". `0002` is not read: it #: declares the ledger, which is infrastructure and not a table the domain #: owns (`test_the_ledger_is_not_declared_in_the_canonical_migration`). @@ -72,8 +73,9 @@ def _columns_of(table: str) -> list[tuple[str, str]]: #: order, so this file does not read the closure off the very module it is #: checking. "Users, memberships, projects, the project-to-repository mapping, #: sessions and unsaved drafts live in the openDox database." THEN THE TWO -#: HEALTH TABLES, transcribed from R2Q13 (a) (Brett Heap, opensoft/ -#: openxFactory#656 comment 6003486656): "DOMAIN. The results table joins Q1's +#: HEALTH TABLES, transcribed from R2Q13 option (a)'s text (plan 038's +#: `clarify-questions.md:651-653`), which Brett Heap ruled in opensoft/ +#: openxFactory#656 comment 6003486656: "DOMAIN. The results table joins Q1's #: list in `identity.TABLES`, and the closure test reads `0001` together with #: `0003_`, in the same change, as the ruling's words have it" — ruling #: `5784155201` item 4's "health results become a seventh table … with the From 7e13130db06da16ed9567bcc85d84ed27890b479 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Fri, 9 Oct 2026 13:43:05 +0000 Subject: [PATCH 08/12] T042: a cyclic or over-deep value is refused by name, never RecursionError The holder's ruling on Copilot's review of #99 at fbf9e77e (#656 comment 6081875839, FIX-NOW): the recursive NUL walk, put in front of json.dumps, let a cyclic value escape as RecursionError, where the dump had refused it by name before. The walk is now iterative and keeps only the containers on its path: a NUL, a container that contains itself, or one nested deeper than MAX_JSON_DEPTH (64) is a RefusedError naming the field, before any statement, quoting nothing. A value shared by two branches is admitted. The run's findings are dumped whole without a second walk, since each field was walked. Tests: every JSON field (identity, evidence, locator, pack_pins, sandbox) by a cyclic mapping, a cyclic list, an over-deep and a far-too-deep value, each refused by name with no row written (red first: 15 RecursionError, 5 over-deep values reached the database); a value exactly at the bound is kept and reads back; a shared value is not a cycle. Lane: openxfactory-4 Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/runtime/health_store.py | 142 +++++++++++++++++++++------- tests_runtime/test_health_store.py | 128 +++++++++++++++++++++++++ 2 files changed, 236 insertions(+), 34 deletions(-) diff --git a/src/opendox/runtime/health_store.py b/src/opendox/runtime/health_store.py index 0bfbe3fe..a122bb18 100644 --- a/src/opendox/runtime/health_store.py +++ b/src/opendox/runtime/health_store.py @@ -18,6 +18,13 @@ which the engine applies before it calls here; this store is not a second spelling of them, and imports nothing from it. +WHAT POSTGRESQL OR JSON CANNOT KEEP IS REFUSED BY NAME, before any statement: +a NUL anywhere in a field, a value that contains itself, and a value nested +deeper than `MAX_JSON_DEPTH` (the holder, opensoft/openxFactory#656 comments +6072086385 item 4 and 6081875839). These are not field bounds but what the +store can hold at all, so the driver's NUL errors and `RecursionError` never +leave it. + PROVENANCE IS REFUSED HERE AND IN THE SCHEMA (box 15.7). A finding with no `pack_id` or no `pack_version` — absent, null or empty — is refused by `record_run` before any statement runs, and `0003_`'s `not null` and @@ -87,6 +94,16 @@ #: The provenance a finding must carry (box 15.7), refused by name. PROVENANCE_FIELDS: tuple[str, ...] = ("pack_id", "pack_version") +#: How deep a JSON field the store keeps may nest, counting the field's own +#: object as level 1. The data model's shapes nest two or three levels (an +#: `identity` key, a locator span, `pack_pins`' per-pack object, a list of +#: colliding paths in `evidence`). Every reader of the stored value meets a +#: limit far deeper than this: CPython 3.12's `json` encoder and decoder raise +#: `RecursionError` at about 10,000 levels (measured), and the decoder is what +#: reads a `jsonb` column back. A deeper value is refused by name before any +#: statement (the holder, opensoft/openxFactory#656 comment 6081875839). +MAX_JSON_DEPTH = 64 + #: The required fields whose columns are `text`. `jsonb_to_recordset` would #: turn a JSON number into text without a word, so a non-string is refused here. _TEXT_FINDING_FIELDS: tuple[str, ...] = tuple( @@ -117,46 +134,93 @@ "baseline_class text, message text, evidence jsonb") -def _carries_nul(value: Any) -> bool: - """Whether U+0000 (NUL) is anywhere in `value`: a string, a key, or nested.""" - if isinstance(value, str): - return "\x00" in value - if isinstance(value, Mapping): - return any(_carries_nul(key) or _carries_nul(item) - for key, item in value.items()) - if isinstance(value, (list, tuple)): - return any(_carries_nul(item) for item in value) - return False - - -def _refuse_nul(value: Any, what: str) -> None: - """Refuse U+0000 anywhere in `value`, BY NAME and before any statement. - - PostgreSQL stores a NUL in neither `jsonb` (SQLSTATE 22P05, raised by the - server, whose message quotes the JSON around it) nor a `text` parameter (a - client-side `DataError`), both measured on postgres 16.15. Without this the - store raised a DRIVER error for a value it could have judged, and the - jsonb one put a slice of the finding's text in the exception (lane 3's - REVIEW-W1 of T042, MINOR 1). The holder, opensoft/openxFactory#656 comment - 6072086385 item 4: the store refuses it itself, as a `RefusedError` that - names the field and quotes nothing, as defence in depth behind the contract - module's own refusal. +#: Why a value cannot be stored, by the name `_unstorable` gives it. +_UNSTORABLE: dict[str, str] = { + "nul": ( + "carries U+0000 (NUL), which PostgreSQL can store in neither text nor " + "jsonb"), + "cycle": ( + "contains itself (a cycle), which JSON cannot represent"), + "depth": ( + f"nests deeper than {MAX_JSON_DEPTH} levels, which is as deep as this " + "store follows a value"), +} + + +def _unstorable(value: Any) -> str | None: + """What makes `value` unstorable (a key of `_UNSTORABLE`), or None. + + One walk over a string, or a mapping's keys and values, or a list's items, + at any depth: a NUL anywhere, a container that contains itself, or a + container nested deeper than `MAX_JSON_DEPTH`. The walk is ITERATIVE and + keeps only the containers on its current path, so no input can make it + recurse, a cycle is named rather than followed, and a value shared by two + branches (which is not a cycle, and which `json.dumps` writes twice) is + admitted (the holder, opensoft/openxFactory#656 comment 6081875839). """ - if _carries_nul(value): + pending: list[tuple[Any, int, bool]] = [(value, 1, False)] + on_path: set[int] = set() + while pending: + item, depth, leaving = pending.pop() + if leaving: + on_path.discard(id(item)) + continue + if isinstance(item, str): + if "\x00" in item: + return "nul" + continue + if isinstance(item, Mapping): + children = [part for pair in item.items() for part in pair] + elif isinstance(item, (list, tuple)): + children = list(item) + else: + continue + if id(item) in on_path: + return "cycle" + if depth > MAX_JSON_DEPTH: + return "depth" + on_path.add(id(item)) + pending.append((item, depth, True)) + pending.extend((child, depth + 1, False) for child in children) + return None + + +def _refuse_unstorable(value: Any, what: str) -> None: + """Refuse what PostgreSQL or JSON cannot keep, BY NAME, before any statement. + + A NUL: PostgreSQL stores one in neither `jsonb` (SQLSTATE 22P05, raised by + the server, whose message quotes the JSON around it) nor a `text` + parameter (a client-side `DataError`), both measured on postgres 16.15. + Without this the store raised a DRIVER error for a value it could have + judged, and the jsonb one put a slice of the finding's text in the + exception (lane 3's REVIEW-W1 of T042, MINOR 1). The holder, + opensoft/openxFactory#656 comment 6072086385 item 4: the store refuses it + itself, as a `RefusedError` that names the field and quotes nothing, as + defence in depth behind the contract module's own refusal. + + A cycle, or a value nested past `MAX_JSON_DEPTH`: the holder, + opensoft/openxFactory#656 comment 6081875839 (Copilot's review of + openDox-code#99 at `fbf9e77e`). `json.dumps` refused a cycle by name + before the NUL walk was put in front of it, and that walk, recursive, + then let a cycle or a deep value escape as `RecursionError`. Both are a + `RefusedError` naming the field again, and `RecursionError` never leaves + the store. + """ + reason = _unstorable(value) + if reason is not None: raise RefusedError( - f"{what} carries U+0000 (NUL), which PostgreSQL can store in " - "neither text nor jsonb; the store refuses it before any " + f"{what} {_UNSTORABLE[reason]}; the store refuses it before any " "statement, and the value is not repeated here") -def _json_text(value: Any, what: str) -> str: +def _dumps(value: Any, what: str) -> str: """Strict JSON text for a `%s::jsonb` parameter, or a refusal naming `what`. `allow_nan=False`, because `NaN` is Python's JSON and not JSON: Postgres - would refuse the cast, and the refusal belongs here, by name. A NUL is - refused here too (`_refuse_nul`), in a key or a value at any depth. + would refuse the cast, and the refusal belongs here, by name. It does NOT + walk `value`: `_json_text` walks a field first, and the run's findings are + dumped whole only after `_finding_record` walked each of their fields. """ - _refuse_nul(value, what) try: return json.dumps(value, sort_keys=True, separators=(",", ":"), ensure_ascii=False, allow_nan=False) @@ -166,6 +230,13 @@ def _json_text(value: Any, what: str) -> str: "represent); the value is not repeated here") from exc +def _json_text(value: Any, what: str) -> str: + """One field's strict JSON text, walked first (`_refuse_unstorable`): a NUL + in a key or a value, a cycle and an over-deep value are refused by name.""" + _refuse_unstorable(value, what) + return _dumps(value, what) + + def canonical_identity(identity: Mapping[str, Any]) -> str: """A finding's `identity`, as the `identity` column stores it. @@ -364,7 +435,7 @@ def _finding_record(index: int, finding: Any) -> dict[str, Any]: f"finding {index}'s {not_text} must be strings; the store does not " "turn another type into text for them") for field in (*REQUIRED_FINDING_FIELDS, *OPTIONAL_FINDING_FIELDS): - _refuse_nul(finding.get(field), f"finding {index}'s {field}") + _refuse_unstorable(finding.get(field), f"finding {index}'s {field}") evidence = finding["evidence"] if not isinstance(evidence, Mapping): raise RefusedError( @@ -422,12 +493,15 @@ def record_run(self, *, corpus_root: str, kind: str, commit: str | None, ("baseline_branch", baseline_branch), ("outcome", outcome), ("export_commit", export_commit)): - _refuse_nul(value, name) + _refuse_unstorable(value, name) records = [_finding_record(index, finding) for index, finding in enumerate(findings)] pins_text = _json_text(dict(pack_pins), "pack_pins") sandbox_text = _json_text(dict(sandbox), "sandbox") - findings_text = _json_text(records, "the run's findings") + # Every field of every record was walked by `_finding_record`, so the + # records are dumped, not walked again: a walk from the list down + # would count two levels more than each field's own. + findings_text = _dumps(records, "the run's findings") run_id = str(uuid.uuid4()) row = _refused_by_the_schema( lambda: self._conn.execute( diff --git a/tests_runtime/test_health_store.py b/tests_runtime/test_health_store.py index 6300528f..7fb9878f 100644 --- a/tests_runtime/test_health_store.py +++ b/tests_runtime/test_health_store.py @@ -29,6 +29,7 @@ from opendox.runtime import identity from opendox.runtime.health_store import ( + MAX_JSON_DEPTH, HealthStore, canonical_identity, ) @@ -853,3 +854,130 @@ def test_pids_max_admits_null_and_every_non_negative_integer( database: Any, pids_max: str) -> None: assert _raw(database, _RAW_RUN, _raw_run( sandbox=f'{{"live": true, "pids_max": {pids_max}}}')) is None + + +# --------------------------------------------------------------------------- +# Copilot's review of #99 at `fbf9e77e`, and the holder's ruling on it (#656 +# 6081875839): `RecursionError` never leaves the store +# --------------------------------------------------------------------------- + +#: A string inside every unstorable value: a refusal that quoted the value +#: would carry it. +_NOT_QUOTED = "SENTINEL-NOT-QUOTED" + + +def _cyclic_mapping() -> dict[str, Any]: + loop: dict[str, Any] = {"note": _NOT_QUOTED} + loop["self"] = loop + return loop + + +def _cyclic_list() -> list[Any]: + loop: list[Any] = [_NOT_QUOTED] + loop.append(loop) + return loop + + +def _nested(levels: int) -> list[Any]: + """`levels` lists, each the only item of the one around it.""" + root: list[Any] = [_NOT_QUOTED] + for _ in range(levels - 1): + root = [root] + return root + + +#: Every field the store keeps as JSON, and the name its refusal must give. +_JSON_FIELDS = { + "identity": "finding 0's identity", + "evidence": "finding 0's evidence", + "locator": "finding 0's locator", + "pack_pins": "pack_pins", + "sandbox": "sandbox", +} + + +def _inside(field: str, value: Any) -> dict[str, Any]: + """`record_run`'s overrides that put `value` at level 2 of `field`, under + a key of the field's own object (level 1).""" + if field == "pack_pins": + return {"pack_pins": {"opendox": {"version": "0.2.0"}, "inner": value}} + if field == "sandbox": + return {"sandbox": {"live": True, "pids_max": None, "inner": value}} + return {"findings": [_finding(**{field: {"target": "a", "inner": value}})]} + + +#: The unstorable shapes, and what the refusal says of each. `over-deep` puts +#: the deepest list at level `MAX_JSON_DEPTH + 1`; `far-too-deep` is deeper +#: than CPython's own `json` encoder goes (it raises `RecursionError` at about +#: 10,000 levels, measured), so only the walk can name it. +_UNSTORABLE_SHAPES = [ + (_cyclic_mapping, "contains itself (a cycle)"), + (_cyclic_list, "contains itself (a cycle)"), + (lambda: _nested(MAX_JSON_DEPTH), f"nests deeper than {MAX_JSON_DEPTH} levels"), + (lambda: _nested(20_000), f"nests deeper than {MAX_JSON_DEPTH} levels"), +] + + +@pytest.mark.parametrize("field", list(_JSON_FIELDS)) +@pytest.mark.parametrize(("make", "reason"), _UNSTORABLE_SHAPES, + ids=["cyclic-mapping", "cyclic-list", "over-deep", + "far-too-deep"]) +def test_an_unstorable_value_is_refused_by_name_and_writes_nothing( + database: Any, field: str, make: Any, reason: str) -> None: + """The holder, #656 `6081875839`: a cyclic value, or one nested deeper + than the walk follows, in any field the store keeps as JSON, is a + `RefusedError` naming the field, raised before any statement, quoting + nothing, and writing no row. + + Before this, the recursive NUL walk let each of them escape as + `RecursionError` (a cycle at once, a nesting past Python's recursion + limit), and `json.dumps`, which had refused a cycle by name, never ran. + """ + over = _inside(field, make()) + with pytest.raises(RefusedError) as caught: + HealthStore(_NoStatement()).record_run(**_run_kwargs(**over)) + message = str(caught.value) + assert f"{_JSON_FIELDS[field]} {reason}" in message, message + assert "before any statement" in message, message + assert _NOT_QUOTED not in message, message + with pytest.raises(RefusedError) as caught: + _record(database, **over) + assert f"{_JSON_FIELDS[field]} {reason}" in str(caught.value) + assert _count(database, "health_runs") == 0 + assert _count(database, "health_findings") == 0 + + +@pytest.mark.parametrize("field", list(_JSON_FIELDS)) +def test_a_value_nested_exactly_to_the_bound_is_kept_and_reads_back( + database: Any, field: str) -> None: + """The bound is `MAX_JSON_DEPTH` and not one level less: a field whose + deepest list is at level `MAX_JSON_DEPTH` is stored and reads back equal, + through `jsonb` and the driver's JSON decoder. The run's findings are + dumped whole after each field was walked, so the list they travel in does + not count against any one field's depth.""" + over = _inside(field, _nested(MAX_JSON_DEPTH - 1)) + run = _record(database, **over) + with database.connection() as conn: + store = HealthStore(conn) + if field in ("pack_pins", "sandbox"): + assert getattr(store.get_run(run.run_id), field) == over[field] + return + sent = over["findings"][0] + stored = store.get_finding(run_id=run.run_id, finding_id=sent["id"]) + assert getattr(stored, field) == sent[field] + + +def test_a_value_shared_by_two_branches_is_not_a_cycle(database: Any) -> None: + """A walk that remembered every container it had seen, rather than the + ones on its path, would call a shared value a cycle. `json.dumps` writes + it twice, and so does the store.""" + shared = {"paths": ["a.md", "b.md"]} + sent = _finding(evidence={"first": shared, "second": shared}) + run = _record(database, findings=[sent], + pack_pins={"opendox": {"version": "0.2.0"}, + "twice": [shared, shared]}) + with database.connection() as conn: + store = HealthStore(conn) + stored = store.get_finding(run_id=run.run_id, finding_id=sent["id"]) + assert store.get_run(run.run_id).pack_pins["twice"] == [shared, shared] + assert stored.evidence == {"first": shared, "second": shared} From 0336a83886a1a2c7213cc880542e32cefb11109c Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Fri, 9 Oct 2026 17:51:30 +0000 Subject: [PATCH 09/12] T042: refuse U+0000 in every text parameter a read binds, by column The holder, #656 comment 6086098003 item 3 (c): the store refuses U+0000 in EVERY text parameter it binds, as RefusedError naming the column, before any statement, never the driver's DataError. The writes were already guarded (path among them, walked per field). The reads were not: latest_run, baseline_run, get_finding and list_findings bound a caller's text, and a NUL raised psycopg.DataError ("PostgreSQL text fields cannot contain NUL (0x00) bytes", measured at 7e13130d). Tests: a NUL in each read's text parameter (7 rows), over a double that fails on any statement and over a real database, red first (all 7 reached the database); and a NUL in path over a real database, refused by name with no row written (already true). Lane: openxfactory-4 Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/runtime/health_store.py | 23 +++++++++++ tests_runtime/test_health_store.py | 60 +++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/src/opendox/runtime/health_store.py b/src/opendox/runtime/health_store.py index a122bb18..2f74c874 100644 --- a/src/opendox/runtime/health_store.py +++ b/src/opendox/runtime/health_store.py @@ -213,6 +213,22 @@ def _refuse_unstorable(value: Any, what: str) -> None: "statement, and the value is not repeated here") +def _refuse_nul_parameters(**columns: str | None) -> None: + """Refuse U+0000 in each text parameter a READ binds, naming its column. + + The holder, opensoft/openxFactory#656 comment 6086098003 item 3 (c): the + store refuses U+0000 in EVERY text parameter it binds, as a `RefusedError` + naming the column, before any statement, and never as the driver's + `DataError` ("PostgreSQL text fields cannot contain NUL (0x00) bytes", + measured from `latest_run` at `7e13130d`). The writes were already + guarded (`_refuse_unstorable`); this is the same defence in depth for the + reads. Each keyword names a column: `id (finding_id)` and `id (after)` + are the `id` column, read through those parameters. + """ + for column, value in columns.items(): + _refuse_unstorable(value, column) + + def _dumps(value: Any, what: str) -> str: """Strict JSON text for a `%s::jsonb` parameter, or a refusal naming `what`. @@ -536,6 +552,7 @@ def latest_run(self, *, corpus_root: str) -> HealthRun | None: Only this corpus's: two corpora sharing one store never read each other's runs (`list`, `fix` and `accept` read the resolved corpus's). """ + _refuse_nul_parameters(corpus_root=corpus_root) row = self._conn.execute( f"select {_RUN_SELECT} from health_runs where corpus_root = %s " "order by run_seq desc limit 1", (corpus_root,)).fetchone() @@ -551,6 +568,8 @@ def baseline_run(self, *, corpus_root: str, baseline_branch: str | None, `before_seq` bounds it to runs recorded before a given one. With no baseline branch no run is default-tip, so there is no B. """ + _refuse_nul_parameters(corpus_root=corpus_root, + baseline_branch=baseline_branch) if baseline_branch is None: return None row = self._conn.execute( @@ -563,6 +582,7 @@ def baseline_run(self, *, corpus_root: str, baseline_branch: str | None, return None if row is None else HealthRun._from_row(row) def get_finding(self, *, run_id: str, finding_id: str) -> HealthFinding: + _refuse_nul_parameters(**{"id (finding_id)": finding_id}) row = self._conn.execute( f"select {_FINDING_SELECT} from health_findings " "where run_id = %s::uuid and id = %s", @@ -578,6 +598,9 @@ def list_findings(self, *, run_id: str, limit: int | None = None, Bounded like every listing in this package (`identity.clamp_limit`). """ + _refuse_nul_parameters(**{"id (after)": after}, + resolution_class=resolution_class, + baseline_class=baseline_class) rows = self._conn.execute( f"select {_FINDING_SELECT} from health_findings " "where run_id = %s::uuid " diff --git a/tests_runtime/test_health_store.py b/tests_runtime/test_health_store.py index 7fb9878f..11b0a1da 100644 --- a/tests_runtime/test_health_store.py +++ b/tests_runtime/test_health_store.py @@ -981,3 +981,63 @@ def test_a_value_shared_by_two_branches_is_not_a_cycle(database: Any) -> None: stored = store.get_finding(run_id=run.run_id, finding_id=sent["id"]) assert store.get_run(run.run_id).pack_pins["twice"] == [shared, shared] assert stored.evidence == {"first": shared, "second": shared} + + +# --------------------------------------------------------------------------- +# The holder, #656 6086098003 item 3 (c): U+0000 in EVERY text parameter the +# store binds is a `RefusedError` naming the column, never a driver error +# --------------------------------------------------------------------------- + +_SOME_RUN = "00000000-0000-4000-8000-000000000000" + +#: Every read that binds a caller's text, the parameter carrying a NUL, and +#: the name the refusal must give. Before the guard, each one reached the +#: driver, which raised `psycopg.DataError` ("PostgreSQL text fields cannot +#: contain NUL (0x00) bytes"); the writes were already guarded (`_NUL_CASES`). +_NUL_READS = [ + ("latest_run", {"corpus_root": "/corpora/a\x00"}, "corpus_root"), + ("baseline_run", {"corpus_root": "/corpora/a\x00", + "baseline_branch": "main"}, "corpus_root"), + ("baseline_run", {"corpus_root": "/corpora/a", + "baseline_branch": "ma\x00in"}, "baseline_branch"), + ("get_finding", {"run_id": _SOME_RUN, "finding_id": "opendox.x\x00"}, + "id (finding_id)"), + ("list_findings", {"run_id": _SOME_RUN, "after": "opendox.x\x00"}, + "id (after)"), + ("list_findings", {"run_id": _SOME_RUN, "resolution_class": "auto\x00"}, + "resolution_class"), + ("list_findings", {"run_id": _SOME_RUN, "baseline_class": "new\x00"}, + "baseline_class"), +] + + +@pytest.mark.parametrize(("method", "arguments", "column"), _NUL_READS, + ids=["latest-corpus-root", "baseline-corpus-root", + "baseline-branch", "finding-id", "after", + "resolution-class", "baseline-class"]) +def test_a_nul_in_a_read_is_refused_by_name_never_by_the_driver( + database: Any, method: str, arguments: dict[str, Any], + column: str) -> None: + """A read's text parameter carrying U+0000 is a `RefusedError` naming the + column, raised before any statement and quoting nothing, over a double + that fails on any statement and over a real database alike.""" + with pytest.raises(RefusedError) as caught: + getattr(HealthStore(_NoStatement()), method)(**arguments) + message = str(caught.value) + assert f"{column} carries U+0000" in message, message + assert "\x00" not in message + with database.connection() as conn: + with pytest.raises(RefusedError) as caught: + getattr(HealthStore(conn), method)(**arguments) + assert f"{column} carries U+0000" in str(caught.value) + + +def test_a_nul_in_a_path_never_reaches_the_driver(database: Any) -> None: + """The holder, #656 6086098003 item 3 (c), names `path`: over a real + database a NUL in it is refused by name, and no row is written.""" + with pytest.raises(RefusedError) as caught: + _record(database, findings=[_finding(path="notes/a\x00.md")]) + assert "finding 0's path carries U+0000" in str(caught.value) + assert "notes/a" not in str(caught.value) + assert _count(database, "health_runs") == 0 + assert _count(database, "health_findings") == 0 From 01f4192db6441d82846e7ef226b30d0ff154d2a2 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Fri, 9 Oct 2026 20:19:36 +0000 Subject: [PATCH 10/12] T042: SonarCloud at 0336a838: one raising call per pytest.raises; a children helper SonarCloud's review of #99 at 0336a838 opened five issues on the two holder fixes (6081875839, 6086098003 item 3 (c)): - four python:S5778 in the new tests: each pytest.raises block now holds one call, with the store, its method and the arguments built before it; - one python:S3776 (cognitive complexity 16 of 15) in _unstorable: what the walk visits under a value is now _json_children, so the walk keeps only its own decisions. No behaviour changes: the same tests pass, and the walk's mutants are re-run against the new text. Lane: openxfactory-4 Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/runtime/health_store.py | 17 ++++++++++++----- tests_runtime/test_health_store.py | 13 +++++++++---- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/opendox/runtime/health_store.py b/src/opendox/runtime/health_store.py index 2f74c874..48c15a1e 100644 --- a/src/opendox/runtime/health_store.py +++ b/src/opendox/runtime/health_store.py @@ -147,6 +147,16 @@ } +def _json_children(item: Any) -> list[Any] | None: + """What the walk visits under `item`: a mapping's keys and values, or a + list's (or tuple's) items. None for a value that holds nothing.""" + if isinstance(item, Mapping): + return [part for pair in item.items() for part in pair] + if isinstance(item, (list, tuple)): + return list(item) + return None + + def _unstorable(value: Any) -> str | None: """What makes `value` unstorable (a key of `_UNSTORABLE`), or None. @@ -169,11 +179,8 @@ def _unstorable(value: Any) -> str | None: if "\x00" in item: return "nul" continue - if isinstance(item, Mapping): - children = [part for pair in item.items() for part in pair] - elif isinstance(item, (list, tuple)): - children = list(item) - else: + children = _json_children(item) + if children is None: continue if id(item) in on_path: return "cycle" diff --git a/tests_runtime/test_health_store.py b/tests_runtime/test_health_store.py index 11b0a1da..011bb1c0 100644 --- a/tests_runtime/test_health_store.py +++ b/tests_runtime/test_health_store.py @@ -934,8 +934,10 @@ def test_an_unstorable_value_is_refused_by_name_and_writes_nothing( limit), and `json.dumps`, which had refused a cycle by name, never ran. """ over = _inside(field, make()) + record_run = HealthStore(_NoStatement()).record_run + kwargs = _run_kwargs(**over) with pytest.raises(RefusedError) as caught: - HealthStore(_NoStatement()).record_run(**_run_kwargs(**over)) + record_run(**kwargs) message = str(caught.value) assert f"{_JSON_FIELDS[field]} {reason}" in message, message assert "before any statement" in message, message @@ -1021,22 +1023,25 @@ def test_a_nul_in_a_read_is_refused_by_name_never_by_the_driver( """A read's text parameter carrying U+0000 is a `RefusedError` naming the column, raised before any statement and quoting nothing, over a double that fails on any statement and over a real database alike.""" + read = getattr(HealthStore(_NoStatement()), method) with pytest.raises(RefusedError) as caught: - getattr(HealthStore(_NoStatement()), method)(**arguments) + read(**arguments) message = str(caught.value) assert f"{column} carries U+0000" in message, message assert "\x00" not in message with database.connection() as conn: + read = getattr(HealthStore(conn), method) with pytest.raises(RefusedError) as caught: - getattr(HealthStore(conn), method)(**arguments) + read(**arguments) assert f"{column} carries U+0000" in str(caught.value) def test_a_nul_in_a_path_never_reaches_the_driver(database: Any) -> None: """The holder, #656 6086098003 item 3 (c), names `path`: over a real database a NUL in it is refused by name, and no row is written.""" + findings = [_finding(path="notes/a\x00.md")] with pytest.raises(RefusedError) as caught: - _record(database, findings=[_finding(path="notes/a\x00.md")]) + _record(database, findings=findings) assert "finding 0's path carries U+0000" in str(caught.value) assert "notes/a" not in str(caught.value) assert _count(database, "health_runs") == 0 From 3230428c014fd79dd63db5840604eabec9f28a17 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Fri, 9 Oct 2026 20:28:49 +0000 Subject: [PATCH 11/12] T042: a lone surrogate is refused by name, never the driver's UnicodeEncodeError Copilot's review of #99 at 01f4192d ("previously missed"): a lone surrogate (U+D800 to U+DFFF), which a surrogate-escaped file name carries in path or corpus_root, passed the walk; UTF-8 cannot encode it, so the driver raised UnicodeEncodeError ("surrogates not allowed") while binding, on a write only after the run's own insert. Classified fix-now under the holder's principle in #656 6086098003 item 3 (c) and 6081875839 (a driver error instead of the store's named refusal, from a simple payload, with a small fix). The walk's text check (_text_unstorable) now refuses a lone surrogate as it refuses U+0000: by the field's or column's name, before any statement, quoting nothing, on writes and on every read parameter. Tests, red first (8 failed: 7 reached the database, 1 raised UnicodeEncodeError): a surrogate in path, corpus_root, an identity value, an evidence key and nested pack_pins (no statement); in two read parameters (no statement and over a real database); and in path over a real database, with no row written. Lane: openxfactory-4 Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/runtime/health_store.py | 56 +++++++++++++++++++------ tests_runtime/test_health_store.py | 65 +++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 12 deletions(-) diff --git a/src/opendox/runtime/health_store.py b/src/opendox/runtime/health_store.py index 48c15a1e..7a26fd28 100644 --- a/src/opendox/runtime/health_store.py +++ b/src/opendox/runtime/health_store.py @@ -19,11 +19,12 @@ spelling of them, and imports nothing from it. WHAT POSTGRESQL OR JSON CANNOT KEEP IS REFUSED BY NAME, before any statement: -a NUL anywhere in a field, a value that contains itself, and a value nested -deeper than `MAX_JSON_DEPTH` (the holder, opensoft/openxFactory#656 comments -6072086385 item 4 and 6081875839). These are not field bounds but what the -store can hold at all, so the driver's NUL errors and `RecursionError` never -leave it. +a NUL or a lone surrogate anywhere in a field or a bound text parameter, a +value that contains itself, and a value nested deeper than `MAX_JSON_DEPTH` +(the holder, opensoft/openxFactory#656 comments 6072086385 item 4, 6081875839 +and 6086098003 item 3 (c)). These are not field bounds but what the store can +hold at all, so the driver's NUL and encoding errors and `RecursionError` +never leave it. PROVENANCE IS REFUSED HERE AND IN THE SCHEMA (box 15.7). A finding with no `pack_id` or no `pack_version` — absent, null or empty — is refused by @@ -59,6 +60,7 @@ from __future__ import annotations import json +import re import uuid from collections.abc import Mapping, Sequence from dataclasses import dataclass @@ -139,6 +141,9 @@ "nul": ( "carries U+0000 (NUL), which PostgreSQL can store in neither text nor " "jsonb"), + "surrogate": ( + "carries a lone surrogate (U+D800 to U+DFFF), which UTF-8, and so " + "PostgreSQL, cannot encode"), "cycle": ( "contains itself (a cycle), which JSON cannot represent"), "depth": ( @@ -147,6 +152,23 @@ } +#: A UTF-16 surrogate code point. A Python string holds one only "lone": a +#: surrogate-escaped file name (`os.fsdecode` of bytes that are not UTF-8), +#: or text decoded with `surrogatepass`. UTF-8 cannot encode it, so the driver +#: raised `UnicodeEncodeError` ("surrogates not allowed") while binding it +#: (Copilot's review of openDox-code#99 at `01f4192d`). +_SURROGATE = re.compile("[\ud800-\udfff]") + + +def _text_unstorable(text: str) -> str | None: + """What makes one string unstorable (a key of `_UNSTORABLE`), or None.""" + if "\x00" in text: + return "nul" + if _SURROGATE.search(text): + return "surrogate" + return None + + def _json_children(item: Any) -> list[Any] | None: """What the walk visits under `item`: a mapping's keys and values, or a list's (or tuple's) items. None for a value that holds nothing.""" @@ -161,8 +183,8 @@ def _unstorable(value: Any) -> str | None: """What makes `value` unstorable (a key of `_UNSTORABLE`), or None. One walk over a string, or a mapping's keys and values, or a list's items, - at any depth: a NUL anywhere, a container that contains itself, or a - container nested deeper than `MAX_JSON_DEPTH`. The walk is ITERATIVE and + at any depth: a NUL or a lone surrogate anywhere, a container that + contains itself, or a container nested deeper than `MAX_JSON_DEPTH`. The walk is ITERATIVE and keeps only the containers on its current path, so no input can make it recurse, a cycle is named rather than followed, and a value shared by two branches (which is not a cycle, and which `json.dumps` writes twice) is @@ -176,8 +198,9 @@ def _unstorable(value: Any) -> str | None: on_path.discard(id(item)) continue if isinstance(item, str): - if "\x00" in item: - return "nul" + reason = _text_unstorable(item) + if reason is not None: + return reason continue children = _json_children(item) if children is None: @@ -205,6 +228,12 @@ def _refuse_unstorable(value: Any, what: str) -> None: itself, as a `RefusedError` that names the field and quotes nothing, as defence in depth behind the contract module's own refusal. + A lone surrogate (U+D800 to U+DFFF), which a surrogate-escaped file name + carries: UTF-8 cannot encode it, so the driver raised `UnicodeEncodeError` + while binding it, on a write only after the run's own insert (Copilot's + review of openDox-code#99 at `01f4192d`, under the holder's principle in + opensoft/openxFactory#656 comment 6086098003 item 3 (c)). + A cycle, or a value nested past `MAX_JSON_DEPTH`: the holder, opensoft/openxFactory#656 comment 6081875839 (Copilot's review of openDox-code#99 at `fbf9e77e`). `json.dumps` refused a cycle by name @@ -221,7 +250,8 @@ def _refuse_unstorable(value: Any, what: str) -> None: def _refuse_nul_parameters(**columns: str | None) -> None: - """Refuse U+0000 in each text parameter a READ binds, naming its column. + """Refuse U+0000, or a lone surrogate, in each text parameter a READ + binds, naming its column. The holder, opensoft/openxFactory#656 comment 6086098003 item 3 (c): the store refuses U+0000 in EVERY text parameter it binds, as a `RefusedError` @@ -229,8 +259,10 @@ def _refuse_nul_parameters(**columns: str | None) -> None: `DataError` ("PostgreSQL text fields cannot contain NUL (0x00) bytes", measured from `latest_run` at `7e13130d`). The writes were already guarded (`_refuse_unstorable`); this is the same defence in depth for the - reads. Each keyword names a column: `id (finding_id)` and `id (after)` - are the `id` column, read through those parameters. + reads. A lone surrogate, which UTF-8 cannot encode, is refused the same + way (`_text_unstorable`; Copilot's review of openDox-code#99 at + `01f4192d`). Each keyword names a column: `id (finding_id)` and + `id (after)` are the `id` column, read through those parameters. """ for column, value in columns.items(): _refuse_unstorable(value, column) diff --git a/tests_runtime/test_health_store.py b/tests_runtime/test_health_store.py index 011bb1c0..a96c7b2f 100644 --- a/tests_runtime/test_health_store.py +++ b/tests_runtime/test_health_store.py @@ -1046,3 +1046,68 @@ def test_a_nul_in_a_path_never_reaches_the_driver(database: Any) -> None: assert "notes/a" not in str(caught.value) assert _count(database, "health_runs") == 0 assert _count(database, "health_findings") == 0 + + +# --------------------------------------------------------------------------- +# Copilot's review of #99 at `01f4192d`: a lone surrogate (U+D800 to U+DFFF), +# which a surrogate-escaped file name carries, is a named refusal, never the +# driver's `UnicodeEncodeError` +# --------------------------------------------------------------------------- + +#: Every place a write can carry a lone surrogate, directly as text or nested +#: in JSON, and the name the refusal must give. UTF-8 cannot encode one, so +#: before the guard the driver raised `UnicodeEncodeError` ("surrogates not +#: allowed") while binding, and on a write only after the run's own insert. +_SURROGATE_WRITES = [ + ({"findings": [_finding(path="notes/\udcff.md")]}, "finding 0's path"), + ({"corpus_root": "/corpora/\udcff"}, "corpus_root"), + ({"findings": [_finding(identity={"target": "a\ud800"})]}, + "finding 0's identity"), + ({"findings": [_finding(evidence={"k\udfff": ["x"]})]}, + "finding 0's evidence"), + ({"pack_pins": {"opendox": {"version": "0.2.0", "note": ["\udcff"]}}}, + "pack_pins"), +] + + +@pytest.mark.parametrize(("over", "field"), _SURROGATE_WRITES, + ids=["path", "corpus-root", "identity-value", + "evidence-key", "pack-pins-nested"]) +def test_a_lone_surrogate_in_a_write_is_refused_by_name_before_any_statement( + over: dict, field: str) -> None: + record_run = HealthStore(_NoStatement()).record_run + kwargs = _run_kwargs(**over) + with pytest.raises(RefusedError) as caught: + record_run(**kwargs) + message = str(caught.value) + assert f"{field} carries a lone surrogate" in message, message + assert not any("\ud800" <= char <= "\udfff" for char in message) + + +@pytest.mark.parametrize(("method", "arguments", "column"), [ + ("latest_run", {"corpus_root": "/corpora/\udcff"}, "corpus_root"), + ("get_finding", {"run_id": _SOME_RUN, "finding_id": "opendox.x\udcff"}, + "id (finding_id)"), +], ids=["latest-corpus-root", "finding-id"]) +def test_a_lone_surrogate_in_a_read_is_refused_by_name_never_by_the_driver( + database: Any, method: str, arguments: dict[str, Any], + column: str) -> None: + read = getattr(HealthStore(_NoStatement()), method) + with pytest.raises(RefusedError) as caught: + read(**arguments) + assert f"{column} carries a lone surrogate" in str(caught.value) + with database.connection() as conn: + read = getattr(HealthStore(conn), method) + with pytest.raises(RefusedError) as caught: + read(**arguments) + assert f"{column} carries a lone surrogate" in str(caught.value) + + +def test_a_lone_surrogate_in_a_path_writes_nothing(database: Any) -> None: + """Over a real database: refused by name, and no row is written.""" + findings = [_finding(path="notes/\udcff.md")] + with pytest.raises(RefusedError) as caught: + _record(database, findings=findings) + assert "finding 0's path carries a lone surrogate" in str(caught.value) + assert _count(database, "health_runs") == 0 + assert _count(database, "health_findings") == 0 From 1fa6ca6cb9a396150783a1ee2e23d2a41822d003 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Fri, 9 Oct 2026 21:14:49 +0000 Subject: [PATCH 12/12] T042: the hosted plane's health refusal is T046's; say so where it is stated Copilot's review of #99 at 3230428c ("previously missed", two items): docs/runtime.md and 0003_health.sql's header stated in the present tense that the hosted plane refuses `health` by name. R2Q15 (a) rules that refusal, but T046 adds it with the `health` verbs and routes, and this change adds neither. Both now say the refusal is ruled and arrives with T046. Comment and prose only: 0003's statements outside `--` comments are unchanged. Lane: openxfactory-4 Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/runtime.md | 6 ++++-- migrations/0003_health.sql | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/runtime.md b/docs/runtime.md index a104e47f..4dab3600 100644 --- a/docs/runtime.md +++ b/docs/runtime.md @@ -27,8 +27,10 @@ It is identity and coordination: six tables, six collections, one bearer token verified against the Keycloak broker. Beside the six, `0003_` adds the health store's two tables (`health_runs`, `health_findings`; R2Q13 (a) makes them DOMAIN tables, so `runtime reset` and the served role's grants reach them). They -serve no collection: the hosted plane refuses `health` by name (R2Q15 (a)), and -they hold findings that address documents, never the documents themselves. +serve no collection, and they hold findings that address documents, never the +documents themselves. R2Q15 (a) rules that the hosted plane refuses `health` by +name; that refusal arrives with the `health` verbs and routes (T046), and until +then this runtime has no `health` verb or route at all. It is **not** a document store and it is not a second dashboard. It does not import `opendox.serve` — the stdlib document surface a student runs from a diff --git a/migrations/0003_health.sql b/migrations/0003_health.sql index ce399ae2..6d4271eb 100644 --- a/migrations/0003_health.sql +++ b/migrations/0003_health.sql @@ -78,8 +78,10 @@ -- so a row without a class is a row the engine never classed. -- -- THE HOSTED PLANE MIGRATES THIS FILE TOO (R2Q15 (a)): there is one --- migration set. The hosted plane refuses the `health` verbs and routes by --- name and records nothing, so its tables stay empty. +-- migration set. R2Q15 (a) also rules that the hosted plane refuses the +-- `health` verbs and routes by name and records nothing, so its tables stay +-- empty. That refusal is T046's, which adds the verbs and routes; this file +-- only declares the tables both planes migrate. create table health_runs ( run_id uuid primary key,