Skip to content

fix(store): ADR 0114 sub-lever A — the claim-proc startup gate has never passed #86

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wshallwshall merged 2 commits into from
Jul 31, 2026
Merged

fix(store): ADR 0114 sub-lever A — the claim-proc startup gate has never passed #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ae0ee7c
ci+test(serverdb): run the 16 module-gated suites that executed nowhe…
wshallwshall Jul 30, 2026
9541d4b
fix(store): ADR 0114 sub-lever A — the claim-proc startup gate has ne…
wshallwshall Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 101 additions & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 <branch>` to exercise it on a feature branch) + on PRs that touch
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/CONFIGURATION.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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). |
Expand Down
66 changes: 56 additions & 10 deletions ...all-complexity-reduction-driver-interface-redesign-ingress-routed-reset-fold.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand Down Expand Up @@ -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.
Expand Down
Loading
Loading