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..4dab3600 100644 --- a/docs/runtime.md +++ b/docs/runtime.md @@ -24,7 +24,13 @@ ## 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, 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 @@ -307,7 +313,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 +430,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 +458,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 +474,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 +625,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 +702,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 +727,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/migrations/0003_health.sql b/migrations/0003_health.sql new file mode 100644 index 00000000..6d4271eb --- /dev/null +++ b/migrations/0003_health.sql @@ -0,0 +1,137 @@ +-- 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. "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 +-- 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. +-- +-- `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 +-- 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. 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, + 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 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 ( + 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 (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')), + 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/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/health_store.py b/src/opendox/runtime/health_store.py new file mode 100644 index 00000000..7a26fd28 --- /dev/null +++ b/src/opendox/runtime/health_store.py @@ -0,0 +1,653 @@ +"""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. + +WHAT POSTGRESQL OR JSON CANNOT KEEP IS REFUSED BY NAME, before any statement: +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 +`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 re +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") + +#: 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( + 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") + + +#: 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"), + "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": ( + f"nests deeper than {MAX_JSON_DEPTH} levels, which is as deep as this " + "store follows a value"), +} + + +#: 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.""" + 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. + + One walk over a string, or a mapping's keys and values, or a list's items, + 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 + admitted (the holder, opensoft/openxFactory#656 comment 6081875839). + """ + 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): + reason = _text_unstorable(item) + if reason is not None: + return reason + continue + children = _json_children(item) + if children is None: + 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 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 + 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} {_UNSTORABLE[reason]}; the store refuses it before any " + "statement, and the value is not repeated here") + + +def _refuse_nul_parameters(**columns: str | None) -> None: + """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` + 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. 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) + + +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. 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. + """ + 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 _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. + + 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 + + +#: 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. + + 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: + 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 + 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") + for field in (*REQUIRED_FINDING_FIELDS, *OPTIONAL_FINDING_FIELDS): + _refuse_unstorable(finding.get(field), f"finding {index}'s {field}") + 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)") + for name, value in (("corpus_root", corpus_root), ("kind", kind), + ("commit", commit), + ("baseline_branch", baseline_branch), + ("outcome", outcome), + ("export_commit", export_commit)): + _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") + # 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( + 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). + """ + _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() + 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. + """ + _refuse_nul_parameters(corpus_root=corpus_root, + baseline_branch=baseline_branch) + 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: + _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", + (_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`). + """ + _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 " + "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/src/opendox/runtime/identity.py b/src/opendox/runtime/identity.py index 192ac24c..a7af5d3f 100644 --- a/src/opendox/runtime/identity.py +++ b/src/opendox/runtime/identity.py @@ -6,11 +6,15 @@ 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 -`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. +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 (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; @@ -51,9 +55,16 @@ 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), 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", @@ -61,6 +72,8 @@ "project_repositories", "sessions", "drafts", + "health_runs", + "health_findings", ) #: The closed role vocabulary, matching `memberships_role_check` in 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", 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 new file mode 100644 index 00000000..a96c7b2f --- /dev/null +++ b/tests_runtime/test_health_store.py @@ -0,0 +1,1113 @@ +"""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 ( + MAX_JSON_DEPTH, + 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)) + + no_statement = HealthStore(_NoStatement()) + kwargs = _run_kwargs(findings=[finding]) + with pytest.raises(RefusedError) as caught: + no_statement.record_run(**kwargs) + 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"), + # 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 = [ + ({"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"), + ({"identity": "null"}, "health_findings_identity_check"), + ({"locator": "null"}, "health_findings_locator_check"), + ({"evidence": "null"}, "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.""" + no_statement = HealthStore(_NoStatement()) + kwargs = _run_kwargs(findings=[_finding(**{field: "the text of a document"})]) + with pytest.raises(RefusedError) as caught: + no_statement.record_run(**kwargs) + 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) 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=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: + no_statement = HealthStore(_NoStatement()) + kwargs = _run_kwargs(**over) + with pytest.raises(RefusedError) as caught: + no_statement.record_run(**kwargs) + 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) + unknown = str(uuid.uuid4()) + with pytest.raises(NotFoundError): + store.get_run(unknown) + 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]}" + + +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) 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 " + "(%(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 + + +# --------------------------------------------------------------------------- +# 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, 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. + + 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( + 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 + 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"']) +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 + + +# --------------------------------------------------------------------------- +# 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()) + 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"{_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} + + +# --------------------------------------------------------------------------- +# 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.""" + read = getattr(HealthStore(_NoStatement()), method) + with pytest.raises(RefusedError) as caught: + 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: + 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=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 + 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 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..687b629c 100644 --- a/tests_runtime/test_schema_shape.py +++ b/tests_runtime/test_schema_shape.py @@ -30,11 +30,21 @@ 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), +#: 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`). +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 +72,37 @@ 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 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 +#: 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 +113,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: