diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 79901c41..f0ad5ce9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -431,7 +431,7 @@ jobs: # are declared there (ADR 0013), and SQL Server / Postgres both declare supports_response_capture # + supports_pt_reingress True — so a regression in that surface is a SERVER-DB regression and must # pull these legs, not just the SQLite suite. - if echo "$changed" | grep -qE '^(messagefoundry/store/|messagefoundry/pipeline/(cluster|wiring_runner|stage_dispatcher)|messagefoundry/config/(settings|wiring)|messagefoundry/transports/(base|database|dicomweb|fhir|http_auth|mllp|rest|soap|tcp|x12)|tests/test_(sqlserver|postgres|cluster|database_connector|pooled|stage_dispatcher|batch_claim|claim_fifo|inline_fast_path|seq_only_fifo|fifo_index|per_lane_wake|response_capture|reingress|x12_rte)|\.github/workflows/ci\.yml)'; then + if echo "$changed" | grep -qE '^(messagefoundry/store/|messagefoundry/pipeline/(cluster|wiring_runner|stage_dispatcher)|messagefoundry/config/(settings|wiring)|messagefoundry/transports/(base|database|dicomweb|fhir|http_auth|mllp|rest|soap|tcp|x12)|tests/test_(sqlserver|postgres|cluster|database_connector|database_source|pooled|stage_dispatcher|batch_claim|claim_fifo|inline_fast_path|seq_only_fifo|fifo_index|per_lane_wake|response_capture|reingress|x12_rte|shard_recovery|shard_cert|adr0071|adr0075|adr0114|dr_server_seed_gate|dr7_server_config_only_backup|backup_runner_server_db|connscale|load_failover|load_runner)|\.github/workflows/ci\.yml)'; then echo "serverdb=true" >> "$GITHUB_OUTPUT" else echo "serverdb=false" >> "$GITHUB_OUTPUT" @@ -722,6 +722,63 @@ jobs: bash scripts/ci/retry-native-crash.sh pytest -v tests/test_x12_rte.py + - name: Run the engine-shard + statement-dispatch suites on real SQL Server + env: + MEFOR_TEST_SQLSERVER: "1" + MEFOR_STORE_BACKEND: sqlserver + MEFOR_STORE_SERVER: localhost + MEFOR_STORE_PORT: "1433" + MEFOR_STORE_DATABASE: MessageFoundry + MEFOR_STORE_AUTH: sql + MEFOR_STORE_USERNAME: sa + MEFOR_STORE_PASSWORD: "Str0ng_P@ssw0rd!" + MEFOR_STORE_TRUST_SERVER_CERTIFICATE: "true" + MEFOR_ALLOW_INSECURE_TLS: "1" + PYTHONFAULTHANDLER: "1" + # Every file here is MEFOR_TEST_SQLSERVER-gated at MODULE level, so before this step they were + # collected nowhere and reported nothing — not a pass, not a skip, on any trigger. That covered + # engine-shard crash recovery and the shard TLS-cert ladder (ADR 0037 over the ADR 0063 unified + # store), the ADR 0071 statement dispatch/fusion wiring, the ADR 0075 batch backend, the ADR 0114 + # live claim procedure, and the synchronous handoff path. tests/test_serverdb_ci_coverage.py now + # fails if a module-gated suite is added without being named here. + # Same pyodbc 5.3.0 + py3.14 native-crash retry as the steps above (upstream pyodbc#1459). + run: >- + bash scripts/ci/retry-native-crash.sh + pytest -v + tests/test_shard_recovery_sqlserver.py + tests/test_shard_cert_sqlserver.py + tests/test_adr0071_dispatch_wiring_sqlserver.py + tests/test_adr0071_fused_callables_sqlserver.py + tests/test_adr0075_batch_sqlserver.py + tests/test_adr0114_claim_proc_live.py + tests/test_sqlserver_sync_handoff.py + tests/test_database_source_integration.py + + - name: Run the DR seed-gate + backup suites on real SQL Server + env: + MEFOR_TEST_SQLSERVER: "1" + MEFOR_STORE_BACKEND: sqlserver + MEFOR_STORE_SERVER: localhost + MEFOR_STORE_PORT: "1433" + MEFOR_STORE_DATABASE: MessageFoundry + MEFOR_STORE_AUTH: sql + MEFOR_STORE_USERNAME: sa + MEFOR_STORE_PASSWORD: "Str0ng_P@ssw0rd!" + MEFOR_STORE_TRUST_SERVER_CERTIFICATE: "true" + MEFOR_ALLOW_INSECURE_TLS: "1" + PYTHONFAULTHANDLER: "1" + # ADR 0048/0049 disaster recovery, SQL Server side: the seed gate that must REFUSE to activate a + # standby onto a non-empty store, the .mfbak backup runner against a real server DB, and the DR7 + # config-only backup + DbaDelegatedError path. Module-gated and never executed; a DR control that + # has never run is a DR control that has never been proven. + # Same pyodbc 5.3.0 + py3.14 native-crash retry as the steps above (upstream pyodbc#1459). + run: >- + bash scripts/ci/retry-native-crash.sh + pytest -v + tests/test_dr_server_seed_gate_sqlserver.py + tests/test_backup_runner_server_db_sqlserver.py + tests/test_dr7_server_config_only_backup_sqlserver.py + # Postgres store backend (Track B): run the gated store suite against a real PostgreSQL service # container (Linux, so 1x minutes). Runs NIGHTLY + on-demand (workflow_dispatch — use # `gh workflow run ci.yml --ref ` to exercise it on a feature branch) + on PRs that touch @@ -868,6 +925,49 @@ jobs: # connector->runner gap existed here. run: pytest -v tests/test_x12_rte.py + - name: Run the failover + engine-shard recovery suites on real Postgres + env: + MEFOR_TEST_POSTGRES: "1" + MEFOR_STORE_BACKEND: postgres + MEFOR_STORE_SERVER: localhost + MEFOR_STORE_PORT: "5432" + MEFOR_STORE_DATABASE: messagefoundry + MEFOR_STORE_USERNAME: postgres + MEFOR_STORE_PASSWORD: mefor + MEFOR_STORE_ENCRYPT: "false" + MEFOR_ALLOW_INSECURE_TLS: "1" + # These are MEFOR_TEST_POSTGRES-gated at MODULE level, so until this step existed they were + # collected nowhere and reported nothing — not a pass, not a skip. The SQL Server failover twin + # (test_cluster_failover_sqlserver.py) has run for some time; its Postgres counterpart never had. + # Engine-shard crash recovery (ADR 0037 over the ADR 0063 unified store) was dark on BOTH server + # backends. tests/test_serverdb_ci_coverage.py now fails if a module-gated suite is added without + # being named here. + run: >- + pytest -v + tests/test_cluster_failover_postgres.py + tests/test_shard_recovery_postgres.py + + - name: Run the DR seed-gate + backup suites on real Postgres + env: + MEFOR_TEST_POSTGRES: "1" + MEFOR_STORE_BACKEND: postgres + MEFOR_STORE_SERVER: localhost + MEFOR_STORE_PORT: "5432" + MEFOR_STORE_DATABASE: messagefoundry + MEFOR_STORE_USERNAME: postgres + MEFOR_STORE_PASSWORD: mefor + MEFOR_STORE_ENCRYPT: "false" + MEFOR_ALLOW_INSECURE_TLS: "1" + # ADR 0048/0049 disaster recovery: the seed gate that must REFUSE to activate a standby onto a + # non-empty store, the .mfbak backup runner against a real server DB, and the DR7 config-only + # backup + DbaDelegatedError path. All three are module-gated and were never executed against + # PostgreSQL; a DR control that has never run is a DR control that has never been proven. + run: >- + pytest -v + tests/test_dr_server_seed_gate_postgres.py + tests/test_backup_runner_server_db_postgres.py + tests/test_dr7_server_config_only_backup_postgres.py + # Headless load test (Track B / throughput): serve the synthetic high-fan-out load config (auth # off, small fan-out) and drive the smoke profile through the real `python -m harness --load` CLI, # asserting zero message loss + all SLOs (exit 0) and uploading the JSON/CSV report. Skipped on PRs diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index f8151d86..81608b70 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -70,7 +70,7 @@ backend-limited. | `group_commit_max_batch` | int | `64` | **SQLite only.** Flush threshold for the group-commit committer: once this many members are enrolled in the open batch it commits immediately rather than waiting out the rest of `group_commit_window_ms`, bounding batch size + latency under load. Ignored when group-commit is off (`group_commit_window_ms = 0`). | | `fifo_claim_batch` | int | `1` | all backends (ADR 0058). Max rows the **INGRESS/ROUTED** FIFO claim takes per commit. `1` = **OFF** (the workers claim one row per commit — byte-identical to before). `> 1` (clamped `1..64`) claims the **contiguous due head-prefix** in one commit and then processes each row in strict FIFO order with its own off-loop route/transform + separate handoff, amortizing the standalone claim commit toward 1/N. A not-due or producer-locked head still blocks the lane (strict per-lane FIFO, #285). The **outbound/delivery** claim is never batched. Opt-in throughput tuning (recommend `8`–`16`); size against worst-case message size, since N decrypted bodies are resident per lane between the claim and the N handoffs. | | `fifo_claim_fold_reset` | bool | `false` | **SQL Server only** ([ADR 0114](adr/0114-phase-4-claim-path-call-complexity-reduction-driver-interface-redesign-ingress-routed-reset-fold.md) sub-lever C). Folds the pooled claim's session `LOCK_TIMEOUT` reset into the claim batch on the **clean success path at INGRESS/ROUTED** (the write-less commit#2 disappears; the shielded finally-guard still runs on every non-clean exit — 1222, kept≠claimed, cancellation, any error). OUTBOUND/RESPONSE are never folded. `false` = **byte-identical** shipped batch + guard. Flip only after its own ADR 0114 §8 bench gate (AC-14). | -| `fifo_claim_proc` | bool | `false` | **SQL Server only** (ADR 0114 sub-lever A). Executes the pooled claim via the two lane-family versioned procs `dbo.mefor_claim_fifo_heads_cid_v1` / `_dst_v1` (fixed-arity `{CALL}`, one JSON lanes parameter) instead of the ~3 KB ad-hoc batch. Needs database `COMPATIBILITY_LEVEL >= 130` (SQL Server 2016); **fails safe to the batch, loudly**, when the procs are missing, hand-edited (body-hash mismatch), or compat < 130 — never a lane outage. A hardened split-principal deployment must `GRANT EXECUTE` on both procs to the runtime principal (the bootstrap principal owns them). `false` = byte-identical. Flip only after its own §8 gate (AC-14). | +| `fifo_claim_proc` | bool | `false` | **SQL Server only** (ADR 0114 sub-lever A). Executes the pooled claim via the two lane-family versioned procs `dbo.mefor_claim_fifo_heads_cid_v1` / `_dst_v1` (fixed-arity `{CALL}`, one JSON lanes parameter) instead of the ~3 KB ad-hoc batch. Needs database `COMPATIBILITY_LEVEL >= 130` (SQL Server 2016); **fails safe to the batch, loudly**, whenever the startup gate cannot verify both deployed bodies against this build — at least: a missing proc, a body matching no form this build deploys (hand edit, hand deploy, or a body changed without bumping the `_v1` proc name), a definition this principal cannot read (no `VIEW DEFINITION`, or `WITH ENCRYPTION`), or compat < 130 — never a lane outage. A hardened split-principal deployment must `GRANT EXECUTE` on both procs to the runtime principal (the bootstrap principal owns them), and `GRANT VIEW DEFINITION` so the gate can read the bodies it verifies. `false` = byte-identical. Flip only after its own §8 gate (AC-14). | | `fifo_claim_prepared` | bool | `false` | **SQL Server only** (ADR 0114 sub-lever B). Stabilizes the pooled claim's statement text (one JSON lanes parameter) and retains a prepared claim cursor on store-owned dedicated connections (INGRESS/ROUTED; the non-DDL fallback lane to `fifo_claim_proc`). **Logs + no-ops unless `fifo_claim_fold_reset` is on** (without the fold the finally-guard's reset would evict the one-slot prepare cache every call). `false` = byte-identical. Flip only after its own §8 gate (AC-14). | | `encryption_key` | secret | — | **env only** (`MEFOR_STORE_ENCRYPTION_KEY`); base64 32-byte **active** key — when set, PHI columns (`raw`/`payload` + `error`/`last_error`/`detail`) are AES-256-GCM-encrypted at rest. Mint one with `messagefoundry gen-key`. Empty = off. See [PHI.md §3](PHI.md#3-encryption-at-rest). | | `encryption_keys_retired` | secret | — | **env only** (`MEFOR_STORE_ENCRYPTION_KEYS_RETIRED`); comma-separated base64 **decrypt-only** keys kept available during a rotation until `messagefoundry rotate-key` finishes re-encrypting under the active key (ASVS 11.2.2). | diff --git a/docs/adr/0114-phase-4-claim-path-call-complexity-reduction-driver-interface-redesign-ingress-routed-reset-fold.md b/docs/adr/0114-phase-4-claim-path-call-complexity-reduction-driver-interface-redesign-ingress-routed-reset-fold.md index 94941b21..194bf595 100644 --- a/docs/adr/0114-phase-4-claim-path-call-complexity-reduction-driver-interface-redesign-ingress-routed-reset-fold.md +++ b/docs/adr/0114-phase-4-claim-path-call-complexity-reduction-driver-interface-redesign-ingress-routed-reset-fold.md @@ -395,12 +395,47 @@ per-database property, not a server version; `CREATE OR ALTER` needs 2016 SP1. A compat-120 database and under a DDL-denied principal. **Startup gate (fail-safe to the batch, loudly).** With `fifo_claim_proc` ON, `open()` probes: (a) -`OBJECT_ID` of **both** procs; (b) a SHA-256 of each deployed body via `OBJECT_DEFINITION()` against the shipped -DDL text (normalized) — **existence alone cannot catch a hand-edited body**, and the ADR 0064 marker covers only -in-repo edits, while the proc *is* the claim logic; (c) `compatibility_level ≥ 130`. Any failure → the store -records `claim_proc_effective = False`, logs a **WARNING naming the reason**, sets a degraded gauge, and runs -the shipped batch — never a lane outage; the hot path contains **no error-2812 handling**. Out-of-band drift is -caught at the next open; `DELETE FROM schema_meta` forces a full re-create. +`OBJECT_ID` of **both** procs; (b) a SHA-256 of each deployed body via `OBJECT_DEFINITION()` against the +**stored forms** of the shipped DDL text (normalized) — **existence alone cannot catch a hand-edited body**, and +the ADR 0064 marker covers only in-repo edits, while the proc *is* the claim logic; (c) `compatibility_level ≥ +130`. Any failure → the store records `claim_proc_effective = False`, logs a **WARNING naming the reason** and +runs the shipped batch — never a lane outage; the hot path contains **no error-2812 handling**. Out-of-band +drift is caught at the next open. + +> **AMENDMENT (2026-07-30) — `OBJECT_DEFINITION()` does not return the submitted text, and this gate was +> inert until it was fixed.** +> +> SQL Server does not store a `CREATE OR ALTER` module verbatim: it **deletes the `OR` and `ALTER` keyword +> tokens and keeps their separators**, so a head submitted as `CREATE OR ALTER PROCEDURE dbo.x` is returned by +> `OBJECT_DEFINITION()` as `CREATE` + three spaces + `PROCEDURE dbo.x` (character delta exactly 7; everything +> after the head byte-identical). MEASURED on SQL Server 2022 16.0.4255.1 and 2025 17.0.4055.5, compat +> 130/160/170, across five deploy paths (fresh `CREATE`, the `OR ALTER` re-apply, a plain batch, inside the +> shipped guarded `EXEC(N'…')`, and an out-of-band `ALTER PROCEDURE` — which the engine also rewrites, to a +> single-spaced `CREATE PROCEDURE`). Case is preserved, not folded; `PROC` survives as `CREATE PROC`. +> +> Because the gate as originally implemented hashed the **submitted** text, it **could never pass for a proc +> deployed by `_claim_proc_ddl`, on any engine that function can deploy to** — sub-lever A degraded to the batch +> on every open, in every deployment, from the feature shipping until this amendment. The lever was inert, not +> merely unused. (Scope note: this is a statement about *this* deploy path, not about every conceivable module.) +> +> The fix is **shipped-side only**: the gate now compares the deployed hash against a small set of +> code-controlled constants — the head forms a server may store for a module *this build* deployed +> (`_CLAIM_PROC_STORED_HEADS`: the measured `rewritten` form, plus the `verbatim` form for a hypothetical +> non-rewriting engine). `_claim_proc_body()` renders byte-identically, so `_claim_proc_ddl`, `_SCHEMA`, +> `_schema_hash()` and the golden body pins are untouched: **no re-pin and no forced DDL re-apply on any live +> database.** The expected map is keyed **per proc**, so the cid body served under the dst name (reachable via +> `sp_rename`, which does not rewrite `sys.sql_modules.definition`) degrades rather than silently swapping the +> lane predicate. Head spellings this deploy path cannot emit (`CREATE PROC`, differing case) keep failing the +> gate: each is affirmative evidence of an out-of-band hand deploy, which is the AC-7 event. +> +> The accepted set is exactly two constants over **normalized** text. It is *not* two byte strings — +> `_normalize_tsql` still applies to the deployed side, so its whitespace collapse remains semantically lossy +> inside comments and string literals. That is contained by the AC-8 body lint (no quotes, no `--`, no `/*`, +> ASCII-only), which is now load-bearing rather than defensive. +> +> **`DELETE FROM schema_meta` was previously prescribed here as the remedy for a body mismatch. It could not +> work** — the re-apply submits the same text, the engine rewrites it the same way, and the hash mismatches +> again — so the advice has been removed from the ADR and from the operator-facing degraded reason. **Versioning, mixed vintages, downgrade.** Procs are **name-versioned** (`_v1`, `_v2`, …): engine sharding runs N processes against ONE unified store (ADR 0037/0063), so a rolling upgrade briefly runs two builds against one @@ -413,6 +448,9 @@ repeated "schema DDL batch applied" lines mid-rollback knows it is expected. **G owns the procs (EXECUTE implicit via ownership); a hardened split-principal deployment must `GRANT EXECUTE` — an ops-doc line, not a code path. **Two-copies drift** (batch vs proc bodies) is contained by the content hash + the body-definition probe + a lint test diffing the proc DDL's statement sequence against the batch construction. +(Until the 2026-07-30 amendment the body-definition probe was **not** a real compensating control: it compared +against text no server could return, so its verdict was constant and a genuine tamper was indistinguishable from +baseline. The content hash and the DDL-vs-batch lint were carrying that containment alone.) ### 5. Sub-lever B — stable statement text + a retained prepared claim cursor (the non-DDL fallback lane) @@ -641,10 +679,18 @@ states, including the mismatch and 1222 translations). **Any miss = the flag sta injected-row test. - **AC-6** — The three flags SHALL be provable no-ops on SQLite and Postgres (neither backend references them). → sentinel test (the ADR 0075 precedent). -- **AC-7** — WHEN `fifo_claim_proc` is ON and a proc is missing, its `OBJECT_DEFINITION` hash mismatches the - shipped body, or compat < 130, the store SHALL degrade loudly to the shipped batch (WARNING naming the reason - + degraded gauge), never a lane outage; the hot path SHALL contain no error-2812 handling. → startup-gate - tests incl. a hand-edited-body leg. +- **AC-7** — WHEN `fifo_claim_proc` is ON and a proc is missing, its `OBJECT_DEFINITION` hash mismatches every + form this build deploys, or compat < 130, the store SHALL degrade loudly to the shipped batch (WARNING naming + the reason + degraded gauge), never a lane outage; the hot path SHALL contain no error-2812 handling. → + startup-gate tests incl. a hand-edited-body leg. +- **AC-7b** — WHEN `fifo_claim_proc` is ON and both procs are deployed **by this build's own DDL**, the gate + SHALL **PASS** and `claim_proc_effective` SHALL be True, verified against a **real SQL Server** (not a stub + that echoes the submitted text back as the deployed body). → `test_adr0114_claim_proc_live.py`, plus an + offline round-trip whose "deployed" fixture independently models the engine's module rewrite. + > Added by the 2026-07-30 amendment. AC-7 as originally written is **one-directional** — it requires the gate + > to degrade when the body mismatches, and nothing anywhere required a *correctly deployed* proc to pass. The + > shipped defect therefore **satisfied AC-7 literally** while leaving the lever inert, and AC review could not + > have caught it. Any future gate-shaped AC needs both directions or it is not a gate. - **AC-8** — The proc bodies SHALL contain no `BEGIN/COMMIT/ROLLBACK`, no `TRY/CATCH`, no `SET XACT_ABORT`, and no `LOCK_TIMEOUT` reset outside the `@fold_reset` tail; `@@TRANCOUNT` on exit SHALL equal entry. → DDL lint test + a trancount probe test. diff --git a/messagefoundry/config/settings.py b/messagefoundry/config/settings.py index 32023c7b..65f0dae6 100644 --- a/messagefoundry/config/settings.py +++ b/messagefoundry/config/settings.py @@ -327,8 +327,10 @@ class StoreSettings(_Section): description=( "Execute the pooled claim via the two lane-family versioned procs " "(dbo.mefor_claim_fifo_heads_cid_v1/_dst_v1; fixed-arity CALL) instead of the ~3KB ad-hoc " - "batch. Fails safe to the batch (loud) if the procs are missing/stale or compat < 130. " - "SQL Server only; OFF = byte-identical." + "batch. Fails safe to the batch (loud) whenever the startup gate cannot verify both " + "deployed bodies against this build — at least: a missing proc, a body matching no form " + "this build deploys, a definition this principal cannot read (no VIEW DEFINITION, or " + "WITH ENCRYPTION), or compat < 130. SQL Server only; OFF = byte-identical." ), ) fifo_claim_prepared: bool = Field( diff --git a/messagefoundry/store/sqlserver.py b/messagefoundry/store/sqlserver.py index 03cca7c7..e51966c5 100644 --- a/messagefoundry/store/sqlserver.py +++ b/messagefoundry/store/sqlserver.py @@ -167,9 +167,23 @@ def _utf16_units(text: str) -> int: return len(text.encode("utf-16-le")) // 2 -def _encode_proc_lanes(lanes: Sequence[str]) -> str: - """Encode the (deduped, chunk-clamped) lane list as the proc's one JSON-array parameter. - ``json.dumps`` default escaping — no delimiter contract is ever imposed on connection names.""" +def _keep_matchable_lanes(lanes: Sequence[str]) -> list[str]: + """Drop requested lane names that exceed the NVARCHAR(256) lane column (AC-11). + + Such a lane can never equal a stored lane, so skipping it client-side is a pure no-op on the + RESULT — but it is NOT optional, and it must run for EVERY dispatch path. The ad-hoc batch + binds the lane list into a ``(VALUES (?),…)`` derived table that lands in a + ``DECLARE @heads TABLE (lane NVARCHAR(256) NOT NULL`` — SQL Server evaluates that narrowing + conversion on the outer constant scan BEFORE the CROSS APPLY filters it, and with ANSI_WARNINGS + ON it raises 2628 ("String or binary data would be truncated") even when zero rows would have + matched. So an unfiltered oversized lane makes the batch RAISE where the contract says it must + claim nothing — no-match parity broken, and 2628 is not 1222 so it is not translated to + EMPTY-all either: it rolls back and re-raises to the dispatcher. + + Applied once in ``claim_fifo_heads`` ahead of the dispatch-path split, so the proc, prepared and + batch branches cannot disagree. (Before this, only the two flagged branches filtered, via + ``_encode_proc_lanes`` — and the gap was unreachable in practice only because sub-lever A's + startup gate never passed, so the parity test never reached its batch arm.)""" kept = [] for lane in lanes: units = _utf16_units(lane) @@ -183,7 +197,15 @@ def _encode_proc_lanes(lanes: Sequence[str]) -> str: ) continue kept.append(lane) - return json.dumps(kept) + return kept + + +def _encode_proc_lanes(lanes: Sequence[str]) -> str: + """Encode the (deduped, filtered, chunk-clamped) lane list as the proc's one JSON-array + parameter. ``json.dumps`` default escaping — no delimiter contract is ever imposed on + connection names. Oversized lanes are removed upstream by ``_keep_matchable_lanes``; the call + here is idempotent and kept so this encoder is safe to use on an unfiltered list.""" + return json.dumps(_keep_matchable_lanes(lanes)) def _claim_proc_param_pins() -> list[tuple[int, int, int]]: @@ -802,11 +824,22 @@ def _fifo_heads_steps(*, lane_col: str, lane_source: str, epoch_guard: str) -> s " WHERE ll.lease_key = @lease_key) <= @leader_epoch)" ) +# The module head the shipped deploy path emits — the ONE place this literal lives in production +# code. ``_claim_proc_body`` renders it and ``_claim_proc_stored_forms`` anchors on it, so an edit +# to the head is mechanically an edit to BOTH sides of the gate's comparison. The anchor is +# load-bearing: a leading ``--`` comment or ``;`` in the body would otherwise blind the gate +# silently, so ``_claim_proc_stored_forms`` RAISES rather than falling through. +# tests/test_adr0114_claim_proc.py re-states this literal independently on purpose — do NOT +# collapse that assertion onto this constant, or the check stops being a check. +_CLAIM_PROC_HEAD: Final[str] = "CREATE OR ALTER PROCEDURE dbo." + def _claim_proc_body(proc_name: str, lane_col: str) -> str: """The full ``CREATE OR ALTER PROCEDURE`` statement for one lane family — the text inside the - guarded ``EXEC(N'...')`` and the text ``OBJECT_DEFINITION()`` returns for the startup gate's - normalized-hash comparison. The body is the shipped batch verbatim (via ``_fifo_heads_steps``) + guarded ``EXEC(N'...')``. This is the text SUBMITTED, which is NOT the text + ``OBJECT_DEFINITION()`` returns: the engine deletes the ``OR`` and ``ALTER`` tokens from the + stored module, so the startup gate compares against ``_claim_proc_stored_forms()`` — never + against this string directly. The body is the shipped batch verbatim (via ``_fifo_heads_steps``) with exactly the ADR 0114 §4 mechanical substitutions: the DECLARE block becomes the parameter list, the VALUES lane list becomes the one-JSON-parameter OPENJSON decode, the spliced epoch guard becomes the fixed nullable form, plus the conditional ``@fold_reset`` tail (sub-lever C's @@ -823,7 +856,7 @@ def _claim_proc_body(proc_name: str, lane_col: str) -> str: honestly-stated delta from the batch (the outbound post-proc H2 statements may emit rowcount DONE tokens; harmless for the execute/fetchone consumers — ADR 0114 §3).""" return ( - f"CREATE OR ALTER PROCEDURE dbo.{proc_name}" + f"{_CLAIM_PROC_HEAD}{proc_name}" " @now FLOAT, @stage NVARCHAR(16), @k INT," " @pending NVARCHAR(32), @inflight NVARCHAR(32)," " @lanes NVARCHAR(MAX)," @@ -882,20 +915,92 @@ def _claim_proc_ddl(proc_name: str, lane_col: str) -> str: def _normalize_tsql(text: str) -> str: """Whitespace-normalize a T-SQL module body for the startup gate's hash comparison: collapse - every whitespace run to one space and strip. OBJECT_DEFINITION() preserves the definition text - as executed, but line endings / trailing whitespace may differ across deployment paths.""" + every whitespace run to one space and strip — line endings and interior spacing differ across + deployment paths and across the engine's own module rewrite. + + LOAD-BEARING, do not "simplify" to a line-ending normalizer: the engine deletes the ``OR`` and + ``ALTER`` TOKENS from a ``CREATE OR ALTER`` head and KEEPS their separators, so the stored head + comes back as ``CREATE`` + three spaces + ``PROCEDURE``. Collapsing runs is what folds that + residual spacing away; without it the gate re-breaks even with the head expansion in place. + + Note this normalization is applied to the DEPLOYED text too, so it is semantically lossy inside + string literals and comments. The compensating control is the AC-8 body lint + (``test_ac8_proc_body_hard_rules``), which pins the shipped body free of quotes, ``--``, ``/*`` + and non-ASCII — keep those assertions.""" return " ".join(text.split()) -def _claim_proc_shipped_hashes() -> dict[str, str]: - """proc name -> SHA-256 of the normalized shipped body (the startup gate's expected values).""" +# Every module head a SQL Server may STORE for a module THIS code deployed as _CLAIM_PROC_HEAD. +# +# OBJECT_DEFINITION() does not return a CREATE OR ALTER module verbatim: the engine deletes the OR +# and ALTER keyword TOKENS and keeps their delimiting separators, so the submitted head comes back +# as "CREATE" + three spaces + "PROCEDURE" (char delta exactly 7). MEASURED on SQL Server 2022 +# 16.0.4255.1 and 2025 17.0.4055.5, compat 130/160/170, across five deploy paths: fresh CREATE, the +# OR ALTER re-apply, a plain batch, the shipped guarded EXEC(N'...') wrapper, and an out-of-band +# ALTER PROCEDURE (which the engine ALSO rewrites, to a single-spaced CREATE PROCEDURE). Case is +# preserved, not folded. _normalize_tsql collapses the spacing, so the rewritten head is keyed here +# in its collapsed spelling. THIS GATE WAS INERT IN EVERY DEPLOYMENT since it shipped, because it +# compared only against the verbatim form — which no SQL Server can return. +# +# The VERBATIM head is retained as a second accepted form for an engine that does not rewrite. It +# is the text this code SUBMITTED, so accepting it asserts strictly LESS than accepting the rewrite +# does: an engine handing back our own bytes is zero drift, not a tamper event. Which form was +# observed is recorded and logged, so a non-rewriting engine stays DISCOVERABLE — it just is not +# alarmed on. +# +# RULE for any future entry — a head belongs here ONLY if a server can produce it from the text +# _claim_proc_body() renders. `CREATE PROC`, a lower-cased head, or any other spelling this deploy +# path cannot emit is positive evidence of an out-of-band hand deploy, which is precisely the AC-7 +# signal, and MUST keep failing the gate. This is not a compatibility grab-bag. +_CLAIM_PROC_STORED_HEADS: Final[tuple[tuple[str, str], ...]] = ( + ("rewritten", "CREATE PROCEDURE dbo."), + ("verbatim", _CLAIM_PROC_HEAD), +) + + +def _claim_proc_stored_forms(proc_name: str, lane_col: str) -> dict[str, str]: + """label -> the normalized text ``OBJECT_DEFINITION()`` may return for OUR deployed module. + + SHIPPED-SIDE ONLY: the deployed text is still hashed as ``_normalize_tsql(deployed)``, + untouched. The accepted set is therefore a small collection of code-controlled CONSTANTS rather + than a preimage class of a lossy transform over server-supplied text — a wrong constant can only + make the gate expect the wrong body (degrade loudly), never widen what it accepts. + + Raises ValueError if the shipped body no longer starts with the anchor; ``_gate_claim_proc``'s + blanket ``except Exception`` turns that into a loud degrade, never an open() failure.""" + shipped = _claim_proc_body(proc_name, lane_col) + if not shipped.startswith(_CLAIM_PROC_HEAD): + raise ValueError( + f"_claim_proc_body({proc_name!r}) no longer starts with {_CLAIM_PROC_HEAD!r}: the AC-7" + " startup gate anchors its stored-head expansion on that literal, so a leading comment," + " semicolon or SET statement would silently blind the gate. Change _claim_proc_body," + " _CLAIM_PROC_HEAD and _CLAIM_PROC_STORED_HEADS together." + ) + tail = shipped[len(_CLAIM_PROC_HEAD) :] + return {label: _normalize_tsql(head + tail) for label, head in _CLAIM_PROC_STORED_HEADS} + + +def _claim_proc_shipped_hashes() -> dict[str, dict[str, str]]: + """proc name -> {SHA-256 of an accepted normalized body: which stored head form it is}. + + Keyed PER PROC, deliberately: ONE flat set across both procs would accept the cid body served + under the dst name (``sp_rename`` does not rewrite ``sys.sql_modules.definition``, so a rename + or a botched blue/green swap reaches that state with no tampering intent) — a silent cross-lane + predicate swap that claims zero rows forever. Do NOT flatten. + + NEVER memoize this at module scope. The ValueError from ``_claim_proc_stored_forms`` is caught + by ``_gate_claim_proc`` and becomes a loud degrade; evaluated at import time it would be an + ImportError for the whole module — a hard outage. (``_SCHEMA`` already calls ``_claim_proc_ddl`` + at import time, so that refactor is plausible; this one must not follow it.)""" return { - _CLAIM_PROC_CID: hashlib.sha256( - _normalize_tsql(_claim_proc_body(_CLAIM_PROC_CID, "channel_id")).encode() - ).hexdigest(), - _CLAIM_PROC_DST: hashlib.sha256( - _normalize_tsql(_claim_proc_body(_CLAIM_PROC_DST, "destination_name")).encode() - ).hexdigest(), + proc_name: { + hashlib.sha256(text.encode()).hexdigest(): label + for label, text in _claim_proc_stored_forms(proc_name, lane_col).items() + } + for proc_name, lane_col in ( + (_CLAIM_PROC_CID, "channel_id"), + (_CLAIM_PROC_DST, "destination_name"), + ) } @@ -1555,6 +1660,10 @@ def __init__( self._claim_proc_effective = False self._claim_proc_degraded_reason: str | None = None self._claim_proc_input_sizes: list[tuple[int, int, int]] | None = None + # proc name -> which _CLAIM_PROC_STORED_HEADS form the deployed module actually matched + # ("rewritten" on every engine measured to date; "verbatim" would mean this server does NOT + # rewrite CREATE OR ALTER — an engine difference worth knowing about, not a tamper event). + self._claim_proc_head_forms: dict[str, str] = {} self._claim_proc_setinputsizes_warned = False # ADR 0114 sub-lever B (fifo_claim_prepared): stable claim text + a retained prepared # cursor on store-owned dedicated connections (INGRESS/ROUTED). Read ONCE at open; the @@ -1644,12 +1753,16 @@ async def _gate_claim_proc(self) -> None: """ADR 0114 §4 startup gate — fail-safe to the batch, loudly (AC-7). With ``fifo_claim_proc`` ON, open() probes (a) both procs exist, (b) each deployed body's normalized SHA-256 (via OBJECT_DEFINITION — existence alone cannot catch a hand-edited - body, and the proc IS the claim logic) matches the shipped DDL text, and (c) - compatibility_level >= 130 (OPENJSON). Any miss records the reason, logs a WARNING, and + body, and the proc IS the claim logic) matches one of ``_claim_proc_stored_forms()``, and + (c) compatibility_level >= 130 (OPENJSON). Any miss records the reason, logs a WARNING, and leaves ``_claim_proc_effective`` False — the shipped batch runs; NEVER a lane outage. - Out-of-band drift is caught at the next open; ``DELETE FROM schema_meta`` forces a full - re-create.""" + + The comparison is against the STORED forms, not against ``_claim_proc_body()`` directly: + the engine rewrites the ``CREATE OR ALTER`` head when it stores the module, so comparing + with the submitted text can never match (the defect that left this gate inert in every + deployment from the feature shipping until this commit).""" reason: str | None = None + head_forms: dict[str, str] = {} try: row = await self._fetchone( "SELECT compatibility_level FROM sys.databases WHERE name = DB_NAME()" @@ -1672,13 +1785,19 @@ async def _gate_claim_proc(self) -> None: ) break got = hashlib.sha256(_normalize_tsql(deployed).encode()).hexdigest() - if got != expected[proc_name]: + matched = expected[proc_name].get(got) + if matched is None: reason = ( - f"stored procedure dbo.{proc_name} body does not match the shipped" - " definition (out-of-band edit? DELETE FROM schema_meta forces a" - " re-create at next open)" + f"stored procedure dbo.{proc_name} body matches no form this build" + " deploys — an out-of-band edit, a hand deploy (a head spelling this" + " code cannot emit, e.g. CREATE PROC or a differing case), a renamed" + " proc (sp_rename does not rewrite the stored definition), or a build" + " whose body was changed without bumping the _v1 proc name. The" + " shipped batch runs. Compare OBJECT_DEFINITION(OBJECT_ID('dbo." + f"{proc_name}')) against this build's own definition to see the drift" ) break + head_forms[proc_name] = matched except Exception as exc: # noqa: BLE001 - §4: ANY gate failure degrades, never an outage # A transient probe failure (e.g. a hiccup on the metadata read) must not fail the # open — the ADR's rule is total: any gate miss runs the shipped batch, loudly. @@ -1687,12 +1806,25 @@ async def _gate_claim_proc(self) -> None: self._claim_proc_effective = True self._claim_proc_degraded_reason = None self._claim_proc_input_sizes = _claim_proc_param_pins() + self._claim_proc_head_forms = head_forms log.info( "fifo_claim_proc: startup gate PASSED — pooled claims will use" - " dbo.%s / dbo.%s (ADR 0114 sub-lever A)", + " dbo.%s / dbo.%s (ADR 0114 sub-lever A); stored head forms: %s", _CLAIM_PROC_CID, _CLAIM_PROC_DST, + head_forms, ) + if any(form == "verbatim" for form in head_forms.values()): + # Not a fault: this engine stores CREATE OR ALTER as submitted rather than deleting + # the tokens. Every engine measured to date rewrites, so this is worth surfacing — + # it means the compatibility assumption in _CLAIM_PROC_STORED_HEADS has a live + # counterexample and the ADR should record it. + log.info( + "fifo_claim_proc: this server stored the CREATE OR ALTER head VERBATIM" + " (%s) — no engine measured to date does this; please report it, the gate" + " accepts it deliberately", + head_forms, + ) else: self._claim_proc_effective = False self._claim_proc_degraded_reason = reason @@ -1703,38 +1835,73 @@ async def _gate_claim_proc(self) -> None: reason, ) - def _apply_claim_input_sizes(self, cur: Any, sizes: list[tuple[int, int, int]] | None) -> None: - """Pin the claim-path parameter descriptors on ``cur`` (ADR 0114 §4/§5 NULL-typing - hazard; 9 descriptors on the proc path, 8 on the prepared stable-text path). - ``setinputsizes`` is pure client-side descriptor state (no I/O), so the SYNC - pyodbc call is loop-safe — but the surface matters: aioodbc 0.5.0 wraps it as - ``async def setinputsizes`` (cursor.py:148, executor-routed), so calling the WRAPPER - synchronously would merely create a never-awaited coroutine and silently apply nothing - (the adversarial-review finding this method is shaped around). Therefore the underlying - pyodbc cursor (``_impl``) is preferred FIRST; the wrapper attribute is used only when it - is a plain sync callable (a bare pyodbc cursor, or a test fake). If neither surface is - reachable, warn ONCE and proceed unpinned (the G-A0 wire trace decides whether describe - traffic appears — degraded observability, never an outage).""" - if sizes is None: - return + @staticmethod + def _sync_setinputsizes(cur: Any) -> Any | None: + """The SYNC ``setinputsizes`` callable for ``cur``, or None if unreachable. The surface + matters: aioodbc 0.5.0 wraps ``setinputsizes`` as ``async def`` (cursor.py:148, + executor-routed), so calling the WRAPPER synchronously merely creates a never-awaited + coroutine that does NOTHING. So the underlying pyodbc cursor (``_impl``) is preferred + FIRST; the wrapper attribute is used only when it is a plain sync callable (a bare pyodbc + cursor, or a test fake). ``setinputsizes`` is pure client-side descriptor state (no I/O), + so the sync call is loop-safe.""" raw = getattr(cur, "_impl", None) target = getattr(raw, "setinputsizes", None) if target is None: candidate = getattr(cur, "setinputsizes", None) if candidate is not None and not inspect.iscoroutinefunction(candidate): target = candidate + return target + + def _warn_setinputsizes_unreachable(self) -> None: + if not self._claim_proc_setinputsizes_warned: + self._claim_proc_setinputsizes_warned = True + log.warning( + "ADR 0114 claim path: no synchronous setinputsizes is reachable through this" + " cursor stack — proceeding without parameter-descriptor pin/clear (NULL params" + " may incur SQLDescribeParam round trips; the ADR 0114 G-A0/G-B0 preflights" + " measure this)" + ) + + def _apply_claim_input_sizes(self, cur: Any, sizes: list[tuple[int, int, int]] | None) -> None: + """Pin the claim-path parameter descriptors on ``cur`` (ADR 0114 §4/§5 NULL-typing + hazard; 9 descriptors on the proc path, 8 on the prepared stable-text path). If no sync + surface is reachable, warn ONCE and proceed unpinned (the G-A0 wire trace decides whether + describe traffic appears — degraded observability, never an outage).""" + if sizes is None: + return + target = self._sync_setinputsizes(cur) if target is None: - if not self._claim_proc_setinputsizes_warned: - self._claim_proc_setinputsizes_warned = True - log.warning( - "ADR 0114 claim path: no synchronous setinputsizes is reachable through this" - " cursor stack — proceeding without parameter-descriptor pins (NULL params" - " may incur SQLDescribeParam round trips; the ADR 0114 G-A0/G-B0 preflights" - " measure this)" - ) + self._warn_setinputsizes_unreachable() return target(sizes) + def _clear_claim_input_sizes(self, cur: Any) -> None: + """Clear the claim-CALL parameter pins on ``cur`` before it runs the H2 DELIVERY DML. + + ``setinputsizes`` is PERSISTENT cursor state. On the proc path the pooled claim cursor is + pinned for the ``{CALL}`` (descriptor[0] = ``SQL_DOUBLE`` for ``@now FLOAT``) and then, at + OUTBOUND, the SAME cursor runs the H2 ``SELECT 1 FROM delivered_keys WHERE outbox_id=?`` — + binding the NVARCHAR ``d["id"]`` against the stale ``SQL_DOUBLE`` descriptor throws a + client-side ``22018`` cast error, rolls the claim back, and collapses outbound delivery. + The pins' only purpose was the CALL's NULL-fence describe-avoidance, so clear them the + moment the CALL's result is drained. + + Only PARAMETERIZED statements are affected — a zero-parameter execute (notably the shielded + ``SET LOCK_TIMEOUT -1;`` reset in this method's ``finally``) tolerates surplus descriptors, + measured on pyodbc 5.3.0 / ODBC Driver 18. So the exposure is exactly the H2 bind chain and + clearing here covers all of it. + + ``setinputsizes(None)`` reverts all params to default inference (pyodbc 5.3.0), on the SYNC + ``_impl`` surface — a bare ``cur.setinputsizes(None)`` on the async wrapper is a + never-awaited no-op, and ``_apply_claim_input_sizes(cur, None)`` early-returns on its None + guard, so neither clears anything. Best-effort: if no sync surface is reachable the pins + were never applied either, so there is nothing to clear.""" + target = self._sync_setinputsizes(cur) + if target is None: + self._warn_setinputsizes_unreachable() + return + target(None) + @property def claim_prepared_effective(self) -> bool: """Whether the ADR 0114 prepared claim path is EFFECTIVELY active this run: the @@ -6309,8 +6476,20 @@ async def claim_fifo_heads( # outstanding-head retry semantics — exactly as ADR 0058 excludes them from batching. per_lane_limit = 1 # Dedupe (preserving request order; duplicate lanes would violate @heads' PRIMARY KEY) + - # chunk clamp; the caller covers the remainder with a second call. - lane_list = list(dict.fromkeys(lanes))[:_FIFO_HEADS_LANE_CHUNK] + # chunk clamp; the caller covers the remainder with a second call. THEN drop lanes too long + # to ever match (AC-11). The clamp deliberately runs BEFORE the skip: an oversized lane + # occupies a chunk slot it can never match, which is the pre-existing, tested contract + # (test_prepared_lane_encoding_shares_the_proc_rules pins 499, not 500). Reordering these + # would serve more lanes per call, but that is a separate decision and not part of this fix. + # + # The skip sits here, ahead of the dispatch-path split below, so the proc, prepared and + # ad-hoc batch branches cannot disagree about it. Previously only the two flagged branches + # filtered (inside _encode_proc_lanes) and the batch bound the raw list — where an oversized + # lane is not merely useless but FATAL (2628 on the @heads narrowing conversion; see + # _keep_matchable_lanes). That gap was unreachable in practice only because sub-lever A's + # startup gate never passed, so the AC-11 parity test never reached its batch arm. + # The existing empty guard then covers a request that was entirely oversized. + lane_list = _keep_matchable_lanes(list(dict.fromkeys(lanes))[:_FIFO_HEADS_LANE_CHUNK]) if not lane_list: return ClaimedHeads(by_lane={}, rearm=frozenset()) # H1 FENCING TOKEN — identical to the single claim, applied to the probe AND the UPDATE so a @@ -6494,6 +6673,18 @@ async def claim_fifo_heads( # before the connection returns to the pool (no-MARS). rows = await cur.fetchall() decoded = [dict(zip(columns, r)) for r in rows] # noqa: B905 + if use_proc: + # The proc CALL pinned 9 parameter descriptors on this POOLED cursor + # (descriptor[0] = SQL_DOUBLE for @now FLOAT); those pins are PERSISTENT cursor + # state, and the H2 delivery DML below runs on the SAME cursor at OUTBOUND — + # binding the NVARCHAR d["id"] against the stale SQL_DOUBLE descriptor throws a + # client 22018 cast error and collapses delivery. Clear the pins now, the moment + # the CALL's result is drained and before any H2 bind. (use_prepared is + # INGRESS/ROUTED-only, where the H2 branch is a no-op — its retained-cursor pins + # are never poisoned AND must persist for reuse across calls, so they are NOT + # cleared here. On ingress/routed the proc's own H2 branch is also a no-op, so + # this clear is simply harmless there.) + self._clear_claim_input_sizes(cur) if any(d["id"] is None for d in decoded): # kept != claimed (ADR 0066 §3.2 STEP 5) — fail closed: roll the whole call # back, claim nothing. Reachable via an ordinary fence race, not only a bug: diff --git a/tests/test_adr0114_claim_proc.py b/tests/test_adr0114_claim_proc.py index 498a4eb8..0b5b48dc 100644 --- a/tests/test_adr0114_claim_proc.py +++ b/tests/test_adr0114_claim_proc.py @@ -11,9 +11,15 @@ - **AC-8** proc-body hard rules: no ``BEGIN/COMMIT/ROLLBACK``, no ``TRY/CATCH``, no ``SET XACT_ABORT``; ``SET LOCK_TIMEOUT`` appears ONLY as the opener (0) and the conditional ``@fold_reset`` tail (-1), the tail is the final statement. -- **AC-7** startup gate: accept (whitespace-tolerant body hash), degrade on missing proc / - hand-edited body / compat < 130 — each with a WARNING naming the reason, the degraded gauge set, - and the claim staying on the batch path (never a lane outage). +- **AC-7 / AC-7b** startup gate, BOTH directions. It ACCEPTS the body as SQL Server actually + stores it — the engine deletes the ``OR``/``ALTER`` tokens from a ``CREATE OR ALTER`` head, so + the deployed text never equals the submitted text and the stub models that rewrite independently + (``_as_object_definition``, with a liveness receipt). And it DEGRADES on a missing proc, a body + matching no form this build deploys, a head spelling this deploy path cannot emit, the other + lane family's body, a broken head anchor, or compat < 130 — each with a WARNING naming the + reason and the claim staying on the batch path (never a lane outage). + The accept direction is the one that was missing: AC-7 as originally written is satisfied by a + gate that rejects EVERYTHING, which is exactly what shipped. - **AC-9** the proc result contract: column-identical 10-column result; the post-``execute`` code path (drain, kept==claimed adjudication, H2, commit, guard) is the SAME code — differential drive proc-vs-batch over identical fake rows yields identical results and identical @@ -39,6 +45,7 @@ from __future__ import annotations +import hashlib import inspect import json import logging @@ -68,6 +75,12 @@ _FAKE_PINS = [(1, 0, 0)] * 9 # stands in for _claim_proc_param_pins() (pyodbc-free CI) +# (proc name, lane column) for both lane families — the gate's two subjects. +_PROCS = ( + ("mefor_claim_fifo_heads_cid_v1", "channel_id"), + ("mefor_claim_fifo_heads_dst_v1", "destination_name"), +) + def _proc_store( *, @@ -201,6 +214,21 @@ def test_ac8_proc_body_hard_rules() -> None: # No single quotes in the body: the EXEC(N'...') embedding and OBJECT_DEFINITION hash # comparison stay exact (the escaping in _claim_proc_ddl is defensive, not load-bearing). assert "'" not in body + # No COMMENTS, for the same reason and a sharper one. `_normalize_tsql` collapses every + # whitespace run — including newlines — on BOTH sides of the gate's comparison. That is + # only safe while the body has no line comment: `-- note\n` and `-- note \n` + # normalize to the SAME text, but in the second the tail is swallowed by the comment. A + # `--` in the body would therefore make two semantically different modules hash equal. + assert "--" not in body, f"{proc_name}: a line comment makes the gate hash-blind" + assert "/*" not in body, f"{proc_name}: a block comment makes the gate hash-blind" + # ASCII only: Python's str.split() treats U+00A0 as whitespace and collapses it, while the + # server preserves it — an NBSP in the body is another hash-blinding hole. It is symmetric + # (and so harmless) only while no such character exists. + assert body.isascii(), f"{proc_name}: non-ASCII in the body can blind the gate hash" + # The head anchor the startup gate expands on. Stated INDEPENDENTLY of _CLAIM_PROC_HEAD on + # purpose — collapsing this onto the constant would make the check tautological, and a + # leading comment / semicolon / SET here silently breaks the gate's stored-form expansion. + assert body.startswith(f"CREATE OR ALTER PROCEDURE dbo.{proc_name} ") def test_ac10_guarded_ddl_rides_schema() -> None: @@ -229,21 +257,73 @@ def test_ac10_guarded_ddl_rides_schema() -> None: # --- AC-7: the startup gate (stubbed _fetchone; no server) --------------------------------------- +def _as_object_definition(submitted: str) -> str: + """What OBJECT_DEFINITION() returns for a module deployed from ``submitted``. + + An INDEPENDENT statement of server behaviour: never derive this from _normalize_tsql, + _CLAIM_PROC_STORED_HEADS or any other production helper, or the round-trip below stops being a + round-trip and the gate tests go back to comparing a function against itself. + + MEASURED (SQL Server 2022 16.0.4255.1 and 2025 17.0.4055.5, compat 130/160/170, five deploy + paths): the engine deletes the OR and ALTER keyword TOKENS and KEEPS their separators, so a + submitted `CREATE OR ALTER PROCEDURE` head is stored as `CREATE` + three spaces + `PROCEDURE`. + """ + head = "CREATE OR ALTER PROCEDURE" + if not submitted.startswith(head): + return submitted + return "CREATE PROCEDURE" + submitted[len(head) :] # exactly three spaces + + +def test_object_definition_fixture_actually_perturbs() -> None: + """Liveness receipt for _as_object_definition. + + Without this, the fixture could silently degrade to the identity function and every gate test + below would quietly go back to being a tautology — which is precisely how the startup gate + shipped inert and survived review. + """ + for proc, col in _PROCS: + submitted = ss._claim_proc_body(proc, col) + stored = _as_object_definition(submitted) + assert stored != submitted, "the fixture must model a real transform" + assert len(submitted) - len(stored) == 7, "OR + ALTER deleted, separators kept" + assert stored.startswith("CREATE PROCEDURE dbo."), stored[:40] + assert submitted.split()[3:] == stored.split()[1:], "only the head differs" + + def _gate_rows( *, compat: int = 150, - cid_body: str | None = "SHIPPED", - dst_body: str | None = "SHIPPED", + cid_body: str | None = "STORED", + dst_body: str | None = "STORED", ) -> dict[str, dict[str, Any] | None]: - """Build the _fetchone stub's answers. "SHIPPED" resolves to the real shipped body text.""" - if cid_body == "SHIPPED": - cid_body = ss._claim_proc_body("mefor_claim_fifo_heads_cid_v1", "channel_id") - if dst_body == "SHIPPED": - dst_body = ss._claim_proc_body("mefor_claim_fifo_heads_dst_v1", "destination_name") + """Build the _fetchone stub's answers. + + "STORED" (the DEFAULT) resolves to the shipped body as OBJECT_DEFINITION actually returns it — + the form a real server can produce. "SUBMITTED" resolves to the verbatim text this code sends, + which no measured SQL Server stores; it models a hypothetical non-rewriting engine and must be + requested explicitly. + + The old default fed SUBMITTED text back as the DEPLOYED body, modelling the server as an + identity function. Both sides of the gate's comparison were then the same function of the same + argument, so the suite could not distinguish a working gate from a broken one — the defect that + let ADR 0114 sub-lever A ship inert. Do not restore it. + """ + + def resolve(value: str | None, proc: str, col: str) -> str | None: + if value == "SUBMITTED": + return ss._claim_proc_body(proc, col) + if value == "STORED": + return _as_object_definition(ss._claim_proc_body(proc, col)) + return value + return { "compat": {"compatibility_level": compat}, - "dbo.mefor_claim_fifo_heads_cid_v1": {"body": cid_body}, - "dbo.mefor_claim_fifo_heads_dst_v1": {"body": dst_body}, + "dbo.mefor_claim_fifo_heads_cid_v1": { + "body": resolve(cid_body, "mefor_claim_fifo_heads_cid_v1", "channel_id") + }, + "dbo.mefor_claim_fifo_heads_dst_v1": { + "body": resolve(dst_body, "mefor_claim_fifo_heads_dst_v1", "destination_name") + }, } @@ -287,12 +367,117 @@ async def test_ac7_gate_accepts_shipped_bodies( async def test_ac7_gate_tolerates_whitespace_differences(monkeypatch: pytest.MonkeyPatch) -> None: # OBJECT_DEFINITION may differ in line endings / whitespace across deployment paths — the # comparison is over the NORMALIZED text (ADR §4 "normalized"). - shipped = ss._claim_proc_body("mefor_claim_fifo_heads_cid_v1", "channel_id") - mangled = shipped.replace(" SET LOCK_TIMEOUT 0;", "\r\n SET LOCK_TIMEOUT 0;\n") + # + # NOTE the base text is the STORED form, not the submitted one. This test passes cid_body= + # explicitly, so it bypasses _gate_rows' default — it was a SECOND door through which the + # verbatim `CREATE OR ALTER` head reached the gate as a fake "deployed" body, and it kept the + # tautology alive even after the default was fixed. Mangle the server form, not our own text. + stored = _as_object_definition( + ss._claim_proc_body("mefor_claim_fifo_heads_cid_v1", "channel_id") + ) + mangled = stored.replace(" SET LOCK_TIMEOUT 0;", "\r\n SET LOCK_TIMEOUT 0;\n") + assert mangled != stored, "the mangle must actually perturb (non-vacuity)" store = await _gate(_gate_rows(cid_body=mangled), monkeypatch) assert store.claim_proc_effective is True +@pytest.mark.parametrize(("proc_name", "lane_col"), _PROCS) +def test_ac7_expected_hashes_are_exactly_two_named_constants(proc_name: str, lane_col: str) -> None: + """The TOTAL-BEHAVIOUR pin: acceptance is `sha256(_normalize_tsql(deployed)) in expected`, so a + set-equality assertion on `expected` completely determines the gate over every possible + deployed text. No equivalent assertion exists for a regex/two-sided design — that is the single + strongest reason this shape was chosen. A third head silently appended later turns this RED.""" + tail = ss._claim_proc_body(proc_name, lane_col)[len(ss._CLAIM_PROC_HEAD) :] + want = { + hashlib.sha256( + ss._normalize_tsql("CREATE PROCEDURE dbo." + tail).encode() + ).hexdigest(): "rewritten", + hashlib.sha256( + ss._normalize_tsql("CREATE OR ALTER PROCEDURE dbo." + tail).encode() + ).hexdigest(): "verbatim", + } + assert ss._claim_proc_shipped_hashes()[proc_name] == want + + +async def test_ac7_gate_accepts_the_body_as_sql_server_actually_stores_it( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """THE regression test. RED under the shipped defect and under any no-op "fix"; green only if + the `rewritten` constant is correct. This is the assertion whose absence let the gate ship + inert — it degraded on EVERY open in EVERY deployment and nothing offline noticed.""" + store = await _gate(_gate_rows(), monkeypatch) # default = STORED + assert store.claim_proc_effective is True + assert store.claim_proc_degraded_reason is None + assert store._claim_proc_head_forms == { + "mefor_claim_fifo_heads_cid_v1": "rewritten", + "mefor_claim_fifo_heads_dst_v1": "rewritten", + } + + +async def test_ac7_gate_accepts_a_non_rewriting_engines_verbatim_storage( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Retained from the pre-fix suite, but it no longer masquerades as a server round-trip: it + models an engine that stores CREATE OR ALTER VERBATIM (the Azure family / pre-2019 flavours the + _claim_proc_ddl guard admits but for which no image was obtainable — UNTESTED, inference only). + The default fixture is now the MEASURED rewrite; this is the explicit exception.""" + store = await _gate(_gate_rows(cid_body="SUBMITTED", dst_body="SUBMITTED"), monkeypatch) + assert store.claim_proc_effective is True + assert set(store._claim_proc_head_forms.values()) == {"verbatim"} + + +@pytest.mark.parametrize( + "head", ["CREATE PROC dbo.", "create procedure dbo.", "CrEaTe pRoCeDuRe dbo."] +) +async def test_ac7_gate_rejects_head_forms_this_deploy_path_cannot_emit( + head: str, monkeypatch: pytest.MonkeyPatch +) -> None: + """ANTI-OVER-WIDE guard. `CREATE PROC` and case-differing heads are MEASURED to survive the + server round-trip, and `_claim_proc_ddl` provably cannot emit them — so each is affirmative + evidence of an out-of-band hand deploy, which IS the AC-7 event. A two-sided regex + canonicalization accepts all three and launders exactly the signal the gate exists to raise.""" + tail = ss._claim_proc_body("mefor_claim_fifo_heads_cid_v1", "channel_id")[ + len(ss._CLAIM_PROC_HEAD) : + ] + store = await _gate(_gate_rows(cid_body=head + tail), monkeypatch) + assert store.claim_proc_effective is False + assert "matches no form this build deploys" in (store.claim_proc_degraded_reason or "") + + +async def test_ac7_gate_rejects_the_cid_body_served_under_the_dst_name( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The expected map must stay keyed PER PROC. A single flat accepted-set across both procs + would take the cid body under the dst name — reachable with no tampering intent via sp_rename + (which does NOT rewrite sys.sql_modules.definition) or a botched blue/green swap — and silently + swap the lane predicate, claiming zero rows forever. RED the moment anyone flattens it.""" + cid_stored = _as_object_definition( + ss._claim_proc_body("mefor_claim_fifo_heads_cid_v1", "channel_id") + ) + store = await _gate(_gate_rows(dst_body=cid_stored), monkeypatch) + assert store.claim_proc_effective is False + assert "mefor_claim_fifo_heads_dst_v1" in (store.claim_proc_degraded_reason or "") + + +@pytest.mark.parametrize("prefix", ["-- note\n", ";", "SET ANSI_NULLS ON; "]) +async def test_ac7_gate_degrades_loudly_when_the_body_head_anchor_breaks( + prefix: str, monkeypatch: pytest.MonkeyPatch +) -> None: + """The MEASURED anchor fragility, pinned fail-closed and DIAGNOSABLE. A leading comment or + semicolon in _claim_proc_body breaks the head expansion; the anchor check raises, the gate's + blanket except turns it into a distinct 'probe failed' reason naming the constant to fix. A + regex design no-ops silently here and reports a generic body mismatch instead — the one reason + string indistinguishable from a real tamper.""" + real = ss._claim_proc_body + + monkeypatch.setattr(ss, "_claim_proc_body", lambda p, c: prefix + real(p, c)) + store = await _gate(_gate_rows(cid_body=None, dst_body=None), monkeypatch) + assert store.claim_proc_effective is False + reason = store.claim_proc_degraded_reason or "" + assert "startup-gate probe failed" in reason + assert "_CLAIM_PROC_HEAD" in reason, "the raise must name the constant to change" + + @pytest.mark.parametrize( ("answers_kw", "reason_fragment"), [ @@ -302,7 +487,7 @@ async def test_ac7_gate_tolerates_whitespace_differences(monkeypatch: pytest.Mon { "cid_body": "CREATE OR ALTER PROCEDURE dbo.mefor_claim_fifo_heads_cid_v1 AS SELECT 1;" }, - "does not match the shipped definition", + "matches no form this build deploys", ), ({"compat": 120}, "compatibility_level 120 < 130"), ], @@ -350,19 +535,31 @@ async def boom(sql: str, params: tuple[Any, ...] = ()) -> dict[str, Any] | None: assert "startup-gate probe failed" in store.claim_proc_degraded_reason -def test_ac9_no_proc_branch_after_execute() -> None: - # AC-9's structural half: everything after cur.execute is the SAME code on both paths — the - # method must not branch on use_proc after the execute statement (the setinputsizes pin is - # legitimately before it). +def test_ac9_result_processing_is_one_shared_path_after_the_pin_clear() -> None: + # AC-9's structural half: the RESULT PROCESSING is the SAME code on both paths — the drain, the + # kept==claimed adjudication, the H2 loop, commit, and the guard must not branch on use_proc. + # The ONE legitimate use_proc branch after the execute is the parameter-pin clear (cursor-state + # hygiene: drop the proc CALL's descriptor pins before the shared H2 DML re-binds on the same + # cursor — NOT a divergence in the result logic). So the invariant is: no use_proc appears + # AFTER the clear. source = inspect.getsource(SqlServerStore.claim_fifo_heads) + clear_at = source.index("self._clear_claim_input_sizes(cur)") execute_at = source.index("await cur.execute(sql, args)") - assert "use_proc" not in source[execute_at:], ( - "AC-9: no use_proc-conditional code may exist after the execute — the drain," - " adjudication, H2, commit, and guard are one shared path" + # In [execute, clear) the ONLY use_proc token is the single `if use_proc:` clear guard — so a + # smuggled `elif use_proc:` / `x = use_proc` / `if use_proc and ...:` in that window is caught. + window = source[execute_at:clear_at] + assert window.count("use_proc") == 1 and "if use_proc:" in window, ( + "AC-9: the only use_proc branch between the execute and the pin-clear is the clear guard" + ) + tail = source[clear_at + len("self._clear_claim_input_sizes(cur)") :] + assert "use_proc" not in tail, ( + "AC-9: after the pin-clear, no use_proc-conditional code may exist — the adjudication, H2," + " commit, and guard are one shared path" ) -# Golden pins for the two proc BODIES (the OBJECT_DEFINITION-comparable text): any drift in the +# Golden pins for the two proc BODIES (the text this build SUBMITS — note the server stores a +# rewritten head, so this is NOT the OBJECT_DEFINITION text; see _as_object_definition): drift in the # shared fragments, the OPENJSON lane source, the fixed-nullable epoch guard, the signature, or # the @fold_reset tail fails here and must be a reviewed, deliberate change (re-pin + the ADR 0064 # hash re-applies the DDL; the startup gate's expected hashes follow automatically). @@ -484,7 +681,9 @@ async def test_setinputsizes_pins_via_impl_on_the_aioodbc_shape() -> None: conn = _FakeConn(ops) _wire(store, cur, conn) await store.claim_fifo_heads("ingress", ["lane-0"], now=_NOW) - assert cur._impl.pins == [_FAKE_PINS] + # The pins are applied for the CALL and then CLEARED (None) once the CALL drains, so the H2 + # delivery bind on this same pooled cursor cannot inherit descriptor[0] = SQL_DOUBLE. + assert cur._impl.pins == [_FAKE_PINS, None] assert cur.wrapper_calls == 0, "the async aioodbc wrapper must never be called synchronously" assert ops[0][1] == _CALL_CID @@ -584,6 +783,35 @@ async def test_ac11_oversized_lane_skipped_with_warning(caplog: pytest.LogCaptur assert result == ClaimedHeads(by_lane={}, rearm=frozenset()) +async def test_ac11_oversized_lane_is_skipped_on_the_AD_HOC_BATCH_too( + caplog: pytest.LogCaptureFixture, +) -> None: + """AC-11 no-match parity applies to ALL THREE dispatch paths, and the batch is where it BITES. + + The batch binds the lane list into ``(VALUES (?),…)`` feeding a + ``DECLARE @heads TABLE (lane NVARCHAR(256) NOT NULL``. SQL Server evaluates that narrowing + conversion on the outer constant scan BEFORE the CROSS APPLY can filter it, and with + ANSI_WARNINGS ON it raises 2628 even when zero rows would match — so an unfiltered oversized + lane makes the batch RAISE where the contract says "claim zero rows on both paths". 2628 is + not 1222, so it is not translated to EMPTY-all either: it rolls back and re-raises. + + Only the two FLAGGED branches used to filter (inside ``_encode_proc_lanes``). The gap was + unreachable in practice solely because sub-lever A's startup gate never passed — fixing the + gate is what exposed it, live, as a 2628 in the AC-11 parity leg. Offline guard so the live + leg is not the only thing standing between this and a regression. + """ + lanes = ["ok-lane", "x" * 257, "another-ok"] + with caplog.at_level(logging.WARNING, logger="messagefoundry.store.sqlserver"): + ops, _, result = await _drive_proc("ingress", lanes, store=_proc_store(proc=False)) + sql, args = ops[0][1], ops[0][2] + # TWO slots for three requested lanes — the oversized name never reaches the constant scan. + assert "(VALUES (?),(?))" in sql, f"the oversized lane must not get a VALUES slot: {sql[:200]}" + assert "x" * 257 not in args, "the oversized lane must never be bound on the batch" + assert args[5:] == ("ok-lane", "another-ok"), args[5:] + assert any("257-UTF-16-unit requested lane name" in r.getMessage() for r in caplog.records) + assert result == ClaimedHeads(by_lane={}, rearm=frozenset()) + + async def test_ac11_oversized_measured_in_utf16_units_not_code_points( caplog: pytest.LogCaptureFixture, ) -> None: diff --git a/tests/test_adr0114_claim_proc_live.py b/tests/test_adr0114_claim_proc_live.py index f59c0b45..4e8e90d9 100644 --- a/tests/test_adr0114_claim_proc_live.py +++ b/tests/test_adr0114_claim_proc_live.py @@ -137,6 +137,15 @@ async def test_live_differential_proc_vs_batch(proc_store: SqlServerStore) -> No async def test_ac7_tampered_body_degrades_next_open(proc_store: SqlServerStore) -> None: # Hand-edit one proc body out-of-band, then re-open: the gate must degrade loudly (the # OBJECT_DEFINITION hash mismatch), and the tampered store must still claim on the batch. + # + # POSITIVE CONTROL FIRST — this is not optional. Until the head-form fix landed, the gate + # rejected EVERY body on every server, so this test's degrade assertions were satisfied by a + # gate that had never matched anything: it passed while proving nothing. Pinning "the gate is + # green on the untampered proc" in the same test is what makes the degrade below evidence. + assert proc_store.claim_proc_effective is True, ( + "positive control: the gate must ACCEPT the correctly deployed proc, else the tamper" + " assertions below are vacuous" + ) await proc_store._execute( f"ALTER PROCEDURE dbo.{ss._CLAIM_PROC_CID} @now FLOAT, @stage NVARCHAR(16), @k INT," " @pending NVARCHAR(32), @inflight NVARCHAR(32), @lanes NVARCHAR(MAX)," @@ -148,7 +157,7 @@ async def test_ac7_tampered_body_degrades_next_open(proc_store: SqlServerStore) try: assert tampered.claim_proc_effective is False assert tampered.claim_proc_degraded_reason is not None - assert "does not match the shipped definition" in tampered.claim_proc_degraded_reason + assert "matches no form this build deploys" in tampered.claim_proc_degraded_reason # Still claims (batch path — never a lane outage). result = await tampered.claim_fifo_heads(Stage.INGRESS.value, ["IB_NONE"]) assert result.by_lane == {} diff --git a/tests/test_serverdb_ci_coverage.py b/tests/test_serverdb_ci_coverage.py new file mode 100644 index 00000000..5373073e --- /dev/null +++ b/tests/test_serverdb_ci_coverage.py @@ -0,0 +1,172 @@ +"""Meta-test: a server-DB suite that no workflow runs is dead coverage. + +A module gated at import time on ``MEFOR_TEST_SQLSERVER`` / ``MEFOR_TEST_POSTGRES`` contributes +**zero executed tests** anywhere the gate is unset — every developer machine, and every CI leg that +does not export it. So a gated module that no workflow step names does not fail, does not error, +and does not surface as a skip anywhere a human looks: it silently never runs. Sixteen such modules +holding 83 tests accumulated before this test existed, including engine-shard crash recovery on +both server backends, the PostgreSQL failover suite, and the DR seed-gate and backup suites. + +Two mechanical invariants: + +1. **Every module-gated suite is named by at least one workflow step.** Adding one without wiring + it reds this test, so the wiring decision is forced at authoring time rather than discovered + during an incident. +2. **The ``serverdb`` change-detection alternation admits every file the server-DB steps run.** + ``ci.yml``'s ``changes`` job decides whether the SQL Server / PostgreSQL legs run on a PR at all. + A file a leg runs but the alternation does not match is covered only by the nightly cron: editing + it does not pull the leg that proves it. The regex's own comment already demands this + ("Keep this in sync with those steps"); nothing enforced it. + +**Scope — module-gated only.** A module whose gate sits on individual tests or on one fixture +parametrization still executes its ungated (SQLite) cases, so it is not invisible in the same way; +its *server arm* not running is a real but weaker gap, tracked separately by the master test plan's +``STORE`` rows rather than pinned here. This test deliberately asserts the sharp invariant only, so +that a failure always means "this file runs nowhere" and never needs an allow-list to stay useful. + +Pure text/AST analysis of the repo: no database, no network, and no gate of its own. +""" + +from __future__ import annotations + +import ast +import re +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +TESTS_DIR = REPO_ROOT / "tests" +WORKFLOWS_DIR = REPO_ROOT / ".github" / "workflows" +CI_YML = WORKFLOWS_DIR / "ci.yml" + +#: The env gates that make a suite server-DB-only. +GATE_ENV_VARS = ("MEFOR_TEST_SQLSERVER", "MEFOR_TEST_POSTGRES") + +#: Module-gated suites that are deliberately not wired to any leg. Each entry needs a reason. +#: An empty mapping is the healthy state — do NOT add a file here to silence a wiring gap. +UNWIRED_ALLOWED: dict[str, str] = {} + + +def _is_module_gated(path: Path) -> bool: + """True when the whole module skips at collection unless a gate env var is set. + + Two shapes count: a module-level ``pytestmark`` carrying a skipif on a gate var, and a + module-level ``pytest.skip(..., allow_module_level=True)`` guarded by one. Both yield zero + executed tests when the gate is unset; a decorator on an individual test does not. + """ + try: + tree = ast.parse(path.read_text(encoding="utf-8")) + except SyntaxError: # pragma: no cover - a broken test file fails elsewhere, loudly + return False + for node in tree.body: + if isinstance(node, ast.Assign) and any( + isinstance(t, ast.Name) and t.id == "pytestmark" for t in node.targets + ): + dumped = ast.dump(node) + if "skipif" in dumped and any(var in dumped for var in GATE_ENV_VARS): + return True + dumped = ast.dump(node) + if "allow_module_level" in dumped and any(var in dumped for var in GATE_ENV_VARS): + return True + return False + + +def _module_gated_suites() -> list[Path]: + return [p for p in sorted(TESTS_DIR.glob("test_*.py")) if _is_module_gated(p)] + + +def _workflow_text() -> str: + """All workflow YAML plus the CI helper scripts a step may delegate to.""" + parts = [p.read_text(encoding="utf-8") for p in sorted(WORKFLOWS_DIR.glob("*.yml"))] + ci_scripts = REPO_ROOT / "scripts" / "ci" + if ci_scripts.is_dir(): + parts += [ + p.read_text(encoding="utf-8") + for p in sorted(ci_scripts.rglob("*")) + if p.is_file() and p.suffix in {".sh", ".ps1", ".py"} + ] + return "\n".join(parts) + + +def test_every_module_gated_suite_is_named_by_a_workflow() -> None: + """A module-gated file no workflow names produces zero tests, everywhere. Wire it.""" + gated = _module_gated_suites() + assert gated, ( + "found no module-gated server-DB suites — the AST detection in _is_module_gated has " + "drifted from how these suites gate themselves" + ) + + haystack = _workflow_text() + unwired = sorted( + p.name for p in gated if p.name not in haystack and p.name not in UNWIRED_ALLOWED + ) + + assert not unwired, ( + f"{len(unwired)} module-gated server-DB suite(s) are named by no workflow step or CI " + "script, so they execute nowhere — not on a PR, not on push-to-main, not on the nightly " + "cron:\n " + + "\n ".join(unwired) + + "\n\nAdd each to the sqlserver-store or postgres-store " + "job in .github/workflows/ci.yml, and extend the `serverdb` change-detection alternation " + "so editing the file pulls the leg that proves it." + ) + + +def _serverdb_alternation() -> re.Pattern[str]: + """The ``tests/test_(...)`` alternation inside the ``serverdb`` change-detection regex.""" + for line in CI_YML.read_text(encoding="utf-8").splitlines(): + if "grep -qE" in line and "tests/test_(" in line: + match = re.search(r"tests/test_\(([^)]*)\)", line) + if match: + return re.compile(rf"^test_({match.group(1)})") + pytest.fail( + "could not locate the `serverdb` change-detection alternation (a `grep -qE` line " + "containing `tests/test_(...)`) in .github/workflows/ci.yml" + ) + + +def _files_run_by_server_db_steps() -> set[str]: + """Test filenames a ci.yml step actually *runs* under a server-DB gate env var. + + Comment lines are stripped first: a step's prose may legitimately name a test file (this + module names itself in the steps it motivated) without that step running it. + """ + found: set[str] = set() + for block in re.split(r"\n - name:", CI_YML.read_text(encoding="utf-8")): + if not any(f"{var}: " in block for var in GATE_ENV_VARS): + continue + executable = "\n".join( + line for line in block.splitlines() if not line.lstrip().startswith("#") + ) + found.update(re.findall(r"tests/(test_[A-Za-z0-9_]+)\.py", executable)) + return found + + +def test_serverdb_path_gate_admits_every_file_those_legs_run() -> None: + """A file a server-DB leg runs must also pull that leg when it is edited.""" + alternation = _serverdb_alternation() + run_by_legs = _files_run_by_server_db_steps() + assert run_by_legs, ( + "found no test files inside server-DB-gated ci.yml steps — the step parsing in " + "_files_run_by_server_db_steps has drifted" + ) + + unmatched = sorted(name for name in run_by_legs if not alternation.match(name)) + + assert not unmatched, ( + f"{len(unmatched)} test file(s) run by a SQL Server / PostgreSQL step are not matched by " + "the `serverdb` change-detection alternation in .github/workflows/ci.yml, so editing one " + "does NOT pull the leg that proves it — it is covered only by the nightly cron:\n " + + "\n ".join(unmatched) + + "\n\nExtend the `tests/test_(...)` alternation to cover them." + ) + + +@pytest.mark.parametrize("var", GATE_ENV_VARS) +def test_gate_env_var_is_still_exported_by_ci(var: str) -> None: + """Guard the guard: a renamed gate var would make both tests above pass vacuously.""" + assert f"{var}: " in CI_YML.read_text(encoding="utf-8"), ( + f"{var} is exported by no ci.yml step. Either it was renamed — update GATE_ENV_VARS in " + "this file — or the server-DB legs no longer run at all." + )