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 194bf595..1b56ee95 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 @@ -398,9 +398,9 @@ compat-120 database and under a DDL-denied principal. `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. +130`. Any failure → the store records `claim_proc_effective = False`, logs a **WARNING naming the reason**, +publishes the degraded gauge (see the second amendment below), 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.** @@ -437,6 +437,65 @@ drift is caught at the next open. > 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. +> **AMENDMENT (2026-07-31) — the degraded gauge now exists, and probe (a) is a real probe again.** Two +> follow-ups the amendment above deliberately held out of the bug fix. +> +> **1. The gauge was aspirational.** AC-7 requires "a WARNING naming the reason **+ degraded gauge**", and this +> section's compensating-control story assumes an operator can SEE the degraded state. Until this amendment +> nobody could: `claim_proc_effective` / `claim_proc_degraded_reason` were read by the store's own tests and +> **nothing else** — no `/stats`, no `/status`, no `/metrics`, no console. The entire operator signal was one +> WARNING line at `open()`. That is not a missing nicety, it is a load-bearing part of *why the amendment above +> was needed*: a fleet running the flag degraded on every open, forever, and the only thing that could have +> told anyone was a log line nobody was watching which named the wrong cause. +> +> The gauge is now a store accessor, `claim_proc_status()`, surfaced on three operator surfaces: +> +> | surface | carries | +> |---|---| +> | `GET /status` → `claim_proc` | `effective`, the human-readable `degraded_reason`, and the matched `head_forms` | +> | `GET /metrics` | `messagefoundry_store_claim_proc_effective` (0/1) and `messagefoundry_store_claim_proc_head_verbatim` (0/1) | +> | the console's store panel (`/ui/status`) | active-vs-degraded, plus the reason when degraded / the head forms when green | +> +> Three shape decisions, so they are not re-litigated. **`None` when the flag is off**, so "not requested" is a +> distinct state from "requested and degraded"; the Prometheus series are correspondingly **absent**, not a +> constant `0` that every SQLite fleet would publish unalertably. **No reason label in the exposition** — the +> reason is free text embedding a proc name and, on the probe-failure arm, an exception string, so a label +> would be unbounded cardinality *and* a breach of the exporter's strict `{connection, destination, status, +> version, le}` allowlist; the string lives on `/status` and the console instead. **It does not feed the +> console's engine-health heart**: a degrade is a performance lever not paying off, claims keep flowing, and +> making the nav cry wolf about it would devalue the signal that means the store is actually unwell. +> +> `head_forms` (proc name → `rewritten` | `verbatim`) is surfaced for the same reason it is logged: a fleet +> reporting `verbatim` is a live counterexample to `_CLAIM_PROC_STORED_HEADS`'s compatibility assumption — no +> engine measured to date stores the `CREATE OR ALTER` head unrewritten — and it was previously visible only +> at INFO. Observability only: the accept/degrade logic is untouched. +> +> **2. A missing `VIEW DEFINITION` grant was reported as a missing proc.** This section has always specified +> probe (a) as "`OBJECT_ID` of **both** procs", but the implementation folded (a) into (b) and inferred absence +> from a NULL `OBJECT_DEFINITION`. **MEASURED** (2026-07-31, on the lab SQL Server): a principal holding only +> `EXECUTE` on the proc gets a non-NULL `OBJECT_ID` and a **NULL** `OBJECT_DEFINITION`; the compat probe still +> passes. So a deployed, working, correct procedure was reported as *missing*, and the operator was sent to fix +> a `CREATE PROCEDURE` permission that was neither the cause nor the cure. `WITH ENCRYPTION` produces the +> identical NULL and the identical misdiagnosis — and because *that* half needs no security principal, it is +> now a live test leg (`test_a_deployed_proc_can_return_a_null_definition`), which pins on a real server the +> one thing an offline stub cannot show: that the two functions genuinely disagree. The permission half stays +> deferred with AC-10's other permission scenarios to a purpose-configured server. +> +> This is not a hypothetical posture here: §5's sub-lever B design explicitly serves "a fleet whose DB +> principal can never hold `CREATE PROCEDURE`" — DBA-provisioned procs plus a least-privilege app principal — +> which is exactly the deployment shape that hits it. The probe now returns `OBJECT_ID` beside the definition +> and the two conditions get separate reasons: +> +> | condition | reason | +> |---|---| +> | `OBJECT_ID` NULL | genuinely absent — guarded DDL skipped, `CREATE PROCEDURE`/ALTER-on-schema denied, or a pre-2016-SP1 engine | +> | `OBJECT_ID` non-NULL, `OBJECT_DEFINITION` NULL | deployed but unreadable — **`GRANT VIEW DEFINITION`**, or the module is `WITH ENCRYPTION` | +> +> Both still **degrade** — the gate hashes the body and cannot pass on one it cannot read — so no accept/reject +> behaviour changed; only the diagnosis did. The probe SQL is pinned by an exact-match assertion in the +> offline suite (a typo'd probe must fail loudly rather than silently match), so that pin moved with it and +> stayed exact. + **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 database — each build calls exactly the body it shipped; a newer build's `_v2` never touches `_v1`. A retired @@ -679,10 +738,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 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-7** — WHEN `fifo_claim_proc` is ON and a proc is missing, is deployed but its definition unreadable + (`OBJECT_ID` resolves, `OBJECT_DEFINITION` NULL), 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 and an unreadable-definition leg. +- **AC-7c** — The degraded gauge SHALL be a surface an operator can READ, not merely an attribute: `/status` + (with the reason string), `/metrics` (numeric, label-less, ABSENT rather than 0 when the lever is not + requested) and the console store panel SHALL each emit it. → surface-emission tests + (`test_adr0114_claim_proc_surfaces.py`), asserting the rendered output, not the property. + > Added by the 2026-07-31 amendment. AC-7 as written required a gauge and nothing required anyone to be able + > to see it; the two properties existed and were read by the store's own tests alone. A "loud" degrade whose + > only audience is a log line is how this lever stayed inert in every deployment for its whole life. - **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 diff --git a/messagefoundry/api/_ui_seam.py b/messagefoundry/api/_ui_seam.py index 1eeb488f..5d5a81b4 100644 --- a/messagefoundry/api/_ui_seam.py +++ b/messagefoundry/api/_ui_seam.py @@ -75,7 +75,13 @@ #: so an older console simply ignores it; bumped rather than corrected in place because v14 SHIPPED #: (v0.3.2). Under "one shipped posture, loosen only" a subset that reads as the whole posture is the #: failure this field exists to prevent, so the console must be able to render the caveat. -ENGINE_UI_SEAM: int = 15 +#: seam v16: SystemStatus gained the additive `claim_proc` — ADR 0114 AC-7's degraded gauge (whether the +#: SQL Server stored-procedure claim path passed its startup gate, and the reason string when it did +#: not), which the status page's store panel renders. Additive with a default and `None` on every +#: backend without the lever, so an older console simply ignores it; a separate seam rather than a +#: correction to v15 because v15 is a SecurityPosture change and folding an unrelated DTO into it would +#: make that note describe a field set it does not cover. +ENGINE_UI_SEAM: int = 16 @dataclass(frozen=True, slots=True) diff --git a/messagefoundry/api/app.py b/messagefoundry/api/app.py index 2ceb9356..4cba84d7 100644 --- a/messagefoundry/api/app.py +++ b/messagefoundry/api/app.py @@ -83,6 +83,7 @@ CapturedResponseInfo, ChannelInfo, ClaimPoolInfo, + ClaimProcInfo, ClusterNode, ClusterNodeList, ClusterStatus, @@ -4477,6 +4478,21 @@ async def system_status( if pool_status is not None else None ) + # ADR 0114 AC-7's degraded gauge. Until this field existed the ONLY signal that the proc + # claim path had fallen back to the shipped batch was a WARNING at store open — which is a + # load-bearing part of why the gate could degrade in every deployment unnoticed. None unless + # [store].fifo_claim_proc is on and the backend has the lever, so the payload is unchanged + # by default. Synchronous + free (attributes the gate recorded once at open). + cps = engine.store.claim_proc_status() + claim_proc = ( + ClaimProcInfo( + effective=cps.effective, + degraded_reason=cps.degraded_reason, + head_forms=dict(cps.head_forms), + ) + if cps is not None + else None + ) # App-log disk metering (#50), alongside the DB metrics — only when a log dir is configured. # Run the blocking stat()s off the event loop (the DB metering is itself off-loop in the store); # None when stdout-only or the directory is unreadable, so /status never raises on it. @@ -4520,6 +4536,7 @@ async def system_status( logs=logs, update=update, pool=pool, + claim_proc=claim_proc, ) # --- runtime log verbosity + redacted log-tail viewer (BACKLOG #171, ADR 0130) ---- diff --git a/messagefoundry/api/metrics.py b/messagefoundry/api/metrics.py index 36c5f8d5..bcffc140 100644 --- a/messagefoundry/api/metrics.py +++ b/messagefoundry/api/metrics.py @@ -38,7 +38,12 @@ from messagefoundry import __version__ from messagefoundry.store.pool_metrics import PoolStatus -from messagefoundry.store.store import DestinationMetrics, InboundMetrics, LatencyHistogram +from messagefoundry.store.store import ( + ClaimProcStatus, + DestinationMetrics, + InboundMetrics, + LatencyHistogram, +) if TYPE_CHECKING: # avoid pulling the heavy engine import into the default path from messagefoundry.pipeline import Engine @@ -192,6 +197,10 @@ class _Snapshot: pool: PoolStatus | None = None committed_txns: int = 0 body_copies: int = 0 + # ADR 0114 AC-7's degraded gauge. None when the backend has no fifo_claim_proc lever or the flag + # is off — the gauges are then ABSENT rather than 0, so a scrape can tell "not requested" from + # "requested and degraded" (a constant 0 on every SQLite fleet would be pure alert noise). + claim_proc: ClaimProcStatus | None = None async def gather_snapshot(engine: Engine) -> _Snapshot: @@ -230,6 +239,7 @@ async def gather_snapshot(engine: Engine) -> _Snapshot: pool=pool, committed_txns=committed_txns, body_copies=body_copies, + claim_proc=engine.store.claim_proc_status(), ) @@ -369,6 +379,33 @@ def collect(self) -> Iterable[Any]: body_copies.add_metric([], float(s.body_copies)) yield body_copies + # ADR 0114 AC-7 degraded gauge. Emitted ONLY when [store].fifo_claim_proc is on: a constant + # 0 on every fleet that never asked for the lever is noise a scraper cannot alert on, and + # absence is the honest encoding of "not applicable here". Numeric and LABEL-LESS by + # design — the human-readable degrade reason is free text (it embeds a proc name and, on the + # probe-failure arm, an exception string), so carrying it as a label would both blow the + # cardinality budget and break this module's strict {connection,destination,status,version,le} + # allowlist. The reason string lives on /status and the console store panel instead. + cp = s.claim_proc + if cp is not None: + effective = GaugeMetricFamily( + "messagefoundry_store_claim_proc_effective", + "1 when the ADR 0114 stored-procedure claim path passed its startup gate and is" + " active, 0 when it degraded to the shipped ad-hoc batch (claims still flow).", + ) + effective.add_metric([], 1.0 if cp.effective else 0.0) + yield effective + # Which stored head form the deployed modules matched. "verbatim" means this server did + # NOT rewrite the CREATE OR ALTER head — no engine measured to date does, so a fleet + # reporting 1 here is a live counterexample worth knowing about, not a fault. + verbatim = GaugeMetricFamily( + "messagefoundry_store_claim_proc_head_verbatim", + "1 when at least one deployed claim procedure's stored definition kept the CREATE" + " OR ALTER head verbatim (this server does not rewrite it), else 0.", + ) + verbatim.add_metric([], 1.0 if "verbatim" in cp.head_forms.values() else 0.0) + yield verbatim + # Connection-pool saturation + acquire-wait (server backends only; absent on SQLite, which has # no pool). [store].pool_size previously emitted NO saturation metric — these close that gap. pool = s.pool diff --git a/messagefoundry/api/models.py b/messagefoundry/api/models.py index 6b74265b..b8a3e123 100644 --- a/messagefoundry/api/models.py +++ b/messagefoundry/api/models.py @@ -710,6 +710,26 @@ class PoolInfo(BaseModel): claim_pool: ClaimPoolInfo | None = None +class ClaimProcInfo(BaseModel): + """The ADR 0114 sub-lever A (``fifo_claim_proc``) startup-gate verdict — AC-7's **degraded + gauge**, surfaced as the additive ``claim_proc`` field on :class:`SystemStatus`. + + ``None`` on every backend without the lever and on SQL Server when the flag is off, so "not + requested" reads differently from "requested and degraded". When ``effective`` is False, + ``degraded_reason`` says why the store fell back to the shipped ad-hoc batch — claims keep + flowing either way, so this is a performance-lever gauge, not a health alarm. + + Metadata only: proc names, a head-form word, and the gate's own reason string — no message + content and no PHI.""" + + effective: bool # the gate passed; pooled claims run through the procs + degraded_reason: str | None = None # why it degraded to the batch; None when effective + # proc name -> the stored head form the deployed module matched ("rewritten" | "verbatim"). + # "verbatim" means this server does NOT rewrite CREATE OR ALTER — no engine measured to date + # does, so it is worth reporting; it is an engine difference, not a fault. + head_forms: dict[str, str] = Field(default_factory=dict) + + class SystemStatus(BaseModel): engine: EngineInfo # Engine-wide top-line roll-up KPIs (#93): total messages, combined in+out connection count with @@ -727,6 +747,10 @@ class SystemStatus(BaseModel): # percentiles + size/idle occupancy). Additive + ``None`` on SQLite (no pool) so the existing # payload is unchanged on the default backend and an older client deserializes /status unchanged. pool: PoolInfo | None = None + # ADR 0114 AC-7's degraded gauge: whether the SQL Server proc claim path is effectively active, + # and why not when it isn't. Additive + ``None`` on every backend without the lever and whenever + # [store].fifo_claim_proc is off, so the default payload is unchanged. + claim_proc: ClaimProcInfo | None = None class IntegrityResult(BaseModel): diff --git a/messagefoundry/store/base.py b/messagefoundry/store/base.py index 108727dd..6064f89c 100644 --- a/messagefoundry/store/base.py +++ b/messagefoundry/store/base.py @@ -48,6 +48,7 @@ AlertInstance, CapturedResponse, ClaimedHeads, + ClaimProcStatus, ConnectionEvent, ConnectionMetrics, DbStatus, @@ -1294,6 +1295,17 @@ def pool_status(self) -> PoolStatus | None: on SQLite (no pool).""" ... + def claim_proc_status(self) -> ClaimProcStatus | None: + """The ADR 0114 sub-lever A stored-procedure-claim startup-gate verdict, or ``None`` when + this backend has no such lever (AC-6: SQL Server is the only one that reads its flag, whose + literal name this module therefore does not write) or that flag is off. AC-7's **degraded + gauge** — the surface an operator can actually see the degraded + state on (``/status``, ``/metrics``, the console store panel); before it existed the whole + signal was one WARNING at ``open()``. Synchronous + free (three attributes the gate recorded + once at open — no DB round-trip), read-only, and additive: the ``/status`` field defaults + ``None``, so an older client deserializes it unchanged.""" + ... + async def integrity_check(self) -> tuple[bool, str]: ... async def connection_metrics( diff --git a/messagefoundry/store/postgres.py b/messagefoundry/store/postgres.py index 36713838..92b5c9bb 100644 --- a/messagefoundry/store/postgres.py +++ b/messagefoundry/store/postgres.py @@ -108,6 +108,7 @@ AlertInstance, CapturedResponse, ClaimedHeads, + ClaimProcStatus, ConnectionEvent, ConnectionMetrics, DbStatus, @@ -1274,6 +1275,11 @@ def pool_status(self) -> PoolStatus | None: acquire_wait=self._acquire_wait.summary(), ) + def claim_proc_status(self) -> ClaimProcStatus | None: + """``None``: the ADR 0114 sub-lever A stored-procedure claim path is SQL-Server-only (AC-6 — + this backend never reads its flag), so there is no gate verdict to report here.""" + return None + async def _fetchall(self, sql: str, *params: Any) -> list[Any]: return list(await self._pool.fetch(sql, *params)) diff --git a/messagefoundry/store/sqlserver.py b/messagefoundry/store/sqlserver.py index e51966c5..cded23d0 100644 --- a/messagefoundry/store/sqlserver.py +++ b/messagefoundry/store/sqlserver.py @@ -86,6 +86,7 @@ AlertInstance, CapturedResponse, ClaimedHeads, + ClaimProcStatus, ConnectionEvent, ConnectionMetrics, DbStatus, @@ -1757,6 +1758,14 @@ async def _gate_claim_proc(self) -> None: (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. + (a) and (b) are probed in ONE statement that returns ``OBJECT_ID`` beside the definition, + because a NULL body has two very different causes: the proc is absent, or it is deployed + and this principal simply cannot READ it (no ``VIEW DEFINITION``, or ``WITH ENCRYPTION``). + Both degrade — the gate cannot hash a body it cannot see — but they need opposite remedies, + and the second is the exact posture the sub-lever B design note above anticipates — a fleet + whose DB principal can never hold CREATE PROCEDURE, i.e. DBA-provisioned procs plus a + least-privilege app principal. + 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 @@ -1773,16 +1782,34 @@ async def _gate_claim_proc(self) -> None: else: expected = _claim_proc_shipped_hashes() for proc_name in (_CLAIM_PROC_CID, _CLAIM_PROC_DST): + # OBJECT_ID rides along so a NULL body can be told apart from an ABSENT proc. + # MEASURED: a principal holding only EXECUTE on the proc gets a non-NULL + # OBJECT_ID and a NULL OBJECT_DEFINITION — the module is deployed and working, + # and the compat probe above still passes. Without the id, that reads as + # "missing" and sends the operator to grant CREATE PROCEDURE, which is not the + # problem and does not fix it. WITH ENCRYPTION produces the identical NULL. row = await self._fetchone( - "SELECT OBJECT_DEFINITION(OBJECT_ID(?)) AS body", (f"dbo.{proc_name}",) + "SELECT OBJECT_ID(?) AS oid, OBJECT_DEFINITION(OBJECT_ID(?)) AS body", + (f"dbo.{proc_name}", f"dbo.{proc_name}"), ) deployed = row["body"] if row else None if not deployed: - reason = ( - f"stored procedure dbo.{proc_name} is missing (guarded DDL skipped —" - " CREATE PROCEDURE / ALTER-on-schema denied, or a pre-2016-SP1" - " engine?)" - ) + if (row["oid"] if row else None) is None: + reason = ( + f"stored procedure dbo.{proc_name} is missing (guarded DDL skipped —" + " CREATE PROCEDURE / ALTER-on-schema denied, or a pre-2016-SP1" + " engine?)" + ) + else: + reason = ( + f"stored procedure dbo.{proc_name} is DEPLOYED but its definition is" + " unreadable (OBJECT_ID resolves, OBJECT_DEFINITION is NULL) — the" + " proc is not missing and CREATE PROCEDURE is not the fix. Either" + " this principal lacks VIEW DEFINITION on it (GRANT VIEW DEFINITION" + f" ON OBJECT::dbo.{proc_name} TO ) or the" + " module was created WITH ENCRYPTION. The gate compares the body" + " hash, so it cannot pass on a body it cannot read" + ) break got = hashlib.sha256(_normalize_tsql(deployed).encode()).hexdigest() matched = expected[proc_name].get(got) @@ -2908,6 +2935,22 @@ def pool_status(self) -> PoolStatus | None: claim_pool=claim_pool, ) + def claim_proc_status(self) -> ClaimProcStatus | None: + """The ADR 0114 sub-lever A startup-gate verdict — AC-7's **degraded gauge** (``/status``, + ``/metrics``, the console's store panel). + + ``None`` when ``fifo_claim_proc`` is off, so "not requested" stays distinguishable from + "requested and degraded"; otherwise the gate's own recorded outcome. Synchronous and free — + it copies three attributes ``open()`` set once, no DB round-trip. Read-only: nothing here + feeds the claim path, and the accept/degrade decision is not re-evaluated.""" + if not self._fifo_claim_proc: + return None + return ClaimProcStatus( + effective=self._claim_proc_effective, + degraded_reason=self._claim_proc_degraded_reason, + head_forms=dict(self._claim_proc_head_forms), + ) + @asynccontextmanager async def _cursor(self, conn: Any) -> AsyncIterator[Any]: """Yield a cursor that is ALWAYS closed before its connection returns to the pool (EF-6). diff --git a/messagefoundry/store/store.py b/messagefoundry/store/store.py index b76d409a..67b86d58 100644 --- a/messagefoundry/store/store.py +++ b/messagefoundry/store/store.py @@ -651,6 +651,31 @@ class ConnectionMetrics: destinations: dict[tuple[str, str], DestinationMetrics] # by (channel_id, destination_name) +@dataclass(frozen=True) +class ClaimProcStatus: + """The ADR 0114 sub-lever A stored-procedure-claim startup-gate verdict — AC-7's **degraded + gauge**, as an operator-readable snapshot. + + ``None`` from :meth:`~messagefoundry.store.base.QueueStore.claim_proc_status` on any backend + without the lever and on SQL Server when its flag is off, so "not requested" is a distinct + state from "requested and degraded" rather than an indistinguishable ``False``. (The flag's + name is deliberately not written in this module — AC-6's sentinel proves the lever is a no-op + here by the absence of that literal.) + + AC-7's compensating-control story assumes an operator can SEE the degraded state. Until this + existed the whole signal was one WARNING at ``open()``, in a log nobody was watching — which is + a load-bearing part of why the gate could degrade on every open, in every deployment, for the + entire life of the feature without anyone noticing. + """ + + effective: bool # the gate passed and pooled claims run through the procs + degraded_reason: str | None # why it fell back to the shipped batch; None when effective + # proc name -> which stored head form the deployed module matched ("rewritten" | "verbatim"). + # Populated only when effective. "verbatim" means this engine does NOT rewrite CREATE OR ALTER — + # no engine measured to date does, so it is worth reporting (not a fault). + head_forms: Mapping[str, str] = field(default_factory=dict) + + @dataclass(frozen=True) class MessageSearchResult: """The outcome of a scan-and-decrypt content search (ADR 0046 #51). ``rows`` are matched message @@ -8251,6 +8276,11 @@ def pool_status(self) -> PoolStatus | None: measures does not exist on this backend.""" return None + def claim_proc_status(self) -> ClaimProcStatus | None: + """``None``: the ADR 0114 sub-lever A stored-procedure claim path is SQL-Server-only (AC-6 — + no other backend so much as reads its flag), so there is no gate verdict to report here.""" + return None + async def integrity_check(self) -> tuple[bool, str]: """Run ``PRAGMA quick_check`` (can be slow on a large DB — call on demand only). Runs on a pooled read-only connection so a long check never blocks the writer (lockfree-reads).""" diff --git a/messagefoundry_webconsole/__init__.py b/messagefoundry_webconsole/__init__.py index 5d9a7203..100e4748 100644 --- a/messagefoundry_webconsole/__init__.py +++ b/messagefoundry_webconsole/__init__.py @@ -45,7 +45,7 @@ # If cross-seam support is ever genuinely wanted, re-widen this set AND add the CI matrix that # installs the MIN and MAX supported engine builds — the claim and its test land together, or not # at all. -SUPPORTED_ENGINE_SEAMS: frozenset[int] = frozenset({15}) +SUPPORTED_ENGINE_SEAMS: frozenset[int] = frozenset({16}) #: The vendored static assets shipped in THIS wheel (mounted at /ui/static by :func:`mount_ui`). STATIC_DIR = Path(__file__).parent / "static" diff --git a/messagefoundry_webconsole/pages/monitoring.py b/messagefoundry_webconsole/pages/monitoring.py index a14a2ffd..0b2a9c53 100644 --- a/messagefoundry_webconsole/pages/monitoring.py +++ b/messagefoundry_webconsole/pages/monitoring.py @@ -352,6 +352,30 @@ def status( ], adjustable=False, ) + # ADR 0114 AC-7's degraded gauge, on the panel an operator actually reads. Empty unless + # [store].fifo_claim_proc is on (SQL Server only), so the default page is unchanged — no blank + # row, no "—" that would read as a broken lever. A degrade is NOT an engine-health fault: claims + # keep flowing on the shipped batch, so it stays a row here and deliberately does not feed the + # nav heart, which would then cry wolf about a performance lever merely not paying off. + # Metadata only: proc names, a head-form word, and the gate's own reason string. + claim_proc_rows: list[list[object]] = [] + cp = sys.claim_proc + if cp is not None: + claim_proc_rows.append( + [ + "Claim path (ADR 0114 stored procedures)", + "active" if cp.effective else "DEGRADED — running the shipped batch", + ] + ) + if not cp.effective: + claim_proc_rows.append(["Claim path — why it degraded", _opt(cp.degraded_reason)]) + elif cp.head_forms: + claim_proc_rows.append( + [ + "Claim path — stored head forms", + ", ".join(f"{k}: {v}" for k, v in sorted(cp.head_forms.items())), + ] + ) store_tbl = rows_table( ["Field", "Value"], [ @@ -415,6 +439,7 @@ def status( ["Messages", db.messages], ["Events", db.events], ["Audit rows", db.audit], + *claim_proc_rows, ], adjustable=False, ) diff --git a/tests/golden/webconsole_seam.snapshot b/tests/golden/webconsole_seam.snapshot index 8a15bd29..3bcfbb5b 100644 --- a/tests/golden/webconsole_seam.snapshot +++ b/tests/golden/webconsole_seam.snapshot @@ -5,7 +5,7 @@ # This is a GOLDEN gate: any diff means the seam contract changed - see the test's failure hint. ## ENGINE_UI_SEAM -15 +16 ## dataclass messagefoundry.api._ui_seam.UiDeps engine_seam @@ -170,7 +170,7 @@ SecurityPosture: allow_unencrypted_phi, backend, client_address_monoculture, cli ServiceStatusInfo: enabled, service_name, state StatsResetRequest: all, targets StatsResetTarget: channel_id, destination, role -SystemStatus: db, engine, kpis, logs, pool, update +SystemStatus: claim_proc, db, engine, kpis, logs, pool, update ## api.auth_models DTO fields rendered by the console AdGroupMap: entries diff --git a/tests/test_adr0114_claim_proc.py b/tests/test_adr0114_claim_proc.py index 0b5b48dc..72c3d3a4 100644 --- a/tests/test_adr0114_claim_proc.py +++ b/tests/test_adr0114_claim_proc.py @@ -295,6 +295,8 @@ def _gate_rows( compat: int = 150, cid_body: str | None = "STORED", dst_body: str | None = "STORED", + cid_oid: int | None = None, + dst_oid: int | None = None, ) -> dict[str, dict[str, Any] | None]: """Build the _fetchone stub's answers. @@ -307,6 +309,12 @@ def _gate_rows( 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. + + ``*_oid`` is the ``OBJECT_ID`` the same probe returns. It DEFAULTS to "present iff the body is", + which is the server's behaviour for the two ordinary cases (deployed-and-readable, absent) and + keeps ``cid_body=None`` meaning "genuinely missing". Pass an id WITH a ``None`` body to model + the third case a real server produces: deployed, but its definition unreadable by this + principal (no VIEW DEFINITION, or WITH ENCRYPTION). """ def resolve(value: str | None, proc: str, col: str) -> str | None: @@ -316,14 +324,18 @@ def resolve(value: str | None, proc: str, col: str) -> str | None: return _as_object_definition(ss._claim_proc_body(proc, col)) return value + def answer(value: str | None, oid: int | None, proc: str, col: str) -> dict[str, Any]: + body = resolve(value, proc, col) + return {"oid": oid if oid is not None else (917578307 if body else None), "body": body} + return { "compat": {"compatibility_level": compat}, - "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") - }, + "dbo.mefor_claim_fifo_heads_cid_v1": answer( + cid_body, cid_oid, "mefor_claim_fifo_heads_cid_v1", "channel_id" + ), + "dbo.mefor_claim_fifo_heads_dst_v1": answer( + dst_body, dst_oid, "mefor_claim_fifo_heads_dst_v1", "destination_name" + ), } @@ -333,7 +345,12 @@ def _stub_fetchone(store: SqlServerStore, answers: dict[str, dict[str, Any] | No async def fake_fetchone(sql: str, params: tuple[Any, ...] = ()) -> dict[str, Any] | None: if sql == "SELECT compatibility_level FROM sys.databases WHERE name = DB_NAME()": return answers["compat"] - assert sql == "SELECT OBJECT_DEFINITION(OBJECT_ID(?)) AS body", f"unexpected probe: {sql}" + assert sql == "SELECT OBJECT_ID(?) AS oid, OBJECT_DEFINITION(OBJECT_ID(?)) AS body", ( + f"unexpected probe: {sql}" + ) + # Both placeholders bind the SAME name — a probe that bound two different objects would + # report one proc's id against another's body. + assert params[0] == params[1], f"the probe must bind one object: {params!r}" return answers[params[0]] store._fetchone = fake_fetchone # type: ignore[method-assign] @@ -510,6 +527,51 @@ async def test_ac7_gate_degrades_loudly( assert ops[0][1].startswith("SET NOCOUNT ON;") +@pytest.mark.parametrize( + "answers_kw", + [{"cid_body": None, "cid_oid": 917578307}, {"dst_body": None, "dst_oid": 917578307}], +) +async def test_ac7_gate_names_view_definition_when_the_proc_is_deployed_but_unreadable( + answers_kw: dict[str, Any], + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """MEASURED: a principal holding only EXECUTE on the proc gets a non-NULL ``OBJECT_ID`` and a + NULL ``OBJECT_DEFINITION`` — the module is deployed and working. Before the id rode along on + the probe, that fired the MISSING arm and sent the operator to grant CREATE PROCEDURE, which is + neither the cause nor the cure (the cure is GRANT VIEW DEFINITION; WITH ENCRYPTION produces the + identical NULL). This is not a hypothetical posture: the sub-lever B design comment in the same + module explicitly designs for a fleet whose principal can never hold CREATE PROCEDURE. + + RED without the fix — the old arm keys on the body alone, so it cannot see the id at all.""" + with caplog.at_level(logging.WARNING, logger="messagefoundry.store.sqlserver"): + store = await _gate(_gate_rows(**answers_kw), monkeypatch) + assert store.claim_proc_effective is False + reason = store.claim_proc_degraded_reason or "" + assert "VIEW DEFINITION" in reason, "the reason must name the grant that actually fixes it" + assert "WITH ENCRYPTION" in reason, "the other cause of the identical NULL" + assert "is missing" not in reason, "a deployed proc must not be reported as absent" + assert any("DEGRADED to the shipped ad-hoc batch" in r.getMessage() for r in caplog.records) + # Still a degrade, not an outage: the claim runs on the shipped batch. + ops, _, _ = await _drive_proc("ingress", ["lane-0"], store=store) + assert ops[0][1].startswith("SET NOCOUNT ON;") + + +async def test_ac7_gate_still_reports_a_genuinely_absent_proc_as_missing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The other side of the split. OBJECT_ID NULL is a real absence and must keep pointing at the + DDL/permission cause — the fix above must not relabel every NULL body as a readability problem. + """ + store = await _gate( + _gate_rows(cid_body=None), monkeypatch + ) # oid defaults to None with the body + reason = store.claim_proc_degraded_reason or "" + assert "is missing" in reason + assert "CREATE PROCEDURE" in reason + assert "VIEW DEFINITION" not in reason + + async def test_ac7_no_error_2812_handling_on_the_hot_path() -> None: # The hot path carries no missing-proc (error 2812) handling: the gate decides at open, the # claim never falls back mid-flight. diff --git a/tests/test_adr0114_claim_proc_live.py b/tests/test_adr0114_claim_proc_live.py index 4e8e90d9..af7cca05 100644 --- a/tests/test_adr0114_claim_proc_live.py +++ b/tests/test_adr0114_claim_proc_live.py @@ -89,6 +89,33 @@ async def test_open_deploys_procs_and_gate_passes(proc_store: SqlServerStore) -> assert row is not None and row["body"], f"dbo.{proc} not deployed" +async def test_a_deployed_proc_can_return_a_null_definition(proc_store: SqlServerStore) -> None: + """The premise the gate's missing-vs-unreadable split rests on, and the one thing no offline + stub can show: on a real engine ``OBJECT_ID`` and ``OBJECT_DEFINITION`` genuinely disagree — a + procedure can be DEPLOYED and its definition still come back NULL. + + Before the gate probed the id it read that NULL as "the proc is missing" and sent the operator + to grant ``CREATE PROCEDURE`` — neither the cause nor the cure. + + ``WITH ENCRYPTION`` is used because it needs no security principal, so this leg creates no login + or user and impersonates nobody; it drops its own proc. The OTHER cause the reason string names + — a principal with ``EXECUTE`` but no ``VIEW DEFINITION`` — produces the byte-identical NULL and + is deferred with AC-10's other permission scenarios to a purpose-configured server. + """ + name = "mefor_gate_null_definition_probe" + await proc_store._execute(f"CREATE PROCEDURE dbo.{name} WITH ENCRYPTION AS SELECT 1;") # noqa: S608 + try: + probe = await proc_store._fetchone( + "SELECT OBJECT_ID(?) AS oid, OBJECT_DEFINITION(OBJECT_ID(?)) AS body", + (f"dbo.{name}", f"dbo.{name}"), + ) + assert probe is not None + assert probe["oid"] is not None, "the proc is deployed — this is the PRESENT half" + assert probe["body"] is None, "and its definition is unreadable — the NULL half" + finally: + await proc_store._execute(f"DROP PROCEDURE IF EXISTS dbo.{name};") # noqa: S608 + + async def test_ac8_trancount_on_exit_equals_entry(proc_store: SqlServerStore) -> None: # Execute the proc inside an open transaction and read @@TRANCOUNT before/after: the proc # must not BEGIN/COMMIT/ROLLBACK (it runs inside the client's autocommit=False txn). diff --git a/tests/test_adr0114_claim_proc_surfaces.py b/tests/test_adr0114_claim_proc_surfaces.py new file mode 100644 index 00000000..5b51dd4b --- /dev/null +++ b/tests/test_adr0114_claim_proc_surfaces.py @@ -0,0 +1,251 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""ADR 0114 AC-7's **degraded gauge** — proving the three operator surfaces actually EMIT it. + +AC-7 says the store degrades "loudly", and its compensating-control story assumes an operator can +SEE the degraded state. For the whole life of sub-lever A they could not: the two properties existed +on ``SqlServerStore`` and were read by nothing but the store's own tests — no ``/status`` field, no +``/metrics`` series, no console row. The entire signal was one WARNING at ``open()``, in a log nobody +was watching, that (before PR #86) named the wrong cause. That is a load-bearing part of why the gate +could degrade on EVERY open in EVERY deployment without anyone noticing. + +So these tests assert **emission**, not the existence of a property: + +* ``/metrics`` renders ``messagefoundry_store_claim_proc_effective`` (and the head-form gauge) with + the right value — and OMITS both when the lever was never requested, because a constant ``0`` on + every SQLite fleet is noise a scraper cannot alert on. +* ``/status`` carries the human-readable ``degraded_reason``, which Prometheus deliberately does not + (it is free text embedding a proc name and, on the probe-failure arm, an exception string — a + label would blow the cardinality budget and break the exporter's strict label allowlist). +* the console's store panel renders the reason on the page an operator actually reads. + +The gauge's producer (``SqlServerStore.claim_proc_status``) is driven from the gate's own suite +(``test_adr0114_claim_proc.py``); here the store accessor is substituted so the SURFACES are what is +under test, on a SQLite engine that needs no SQL Server. +""" + +from __future__ import annotations + +from collections.abc import AsyncIterator +from pathlib import Path + +import httpx +import pytest +from prometheus_client.parser import text_string_to_metric_families + +from messagefoundry.api import create_app +from messagefoundry.api.metrics import render_metrics +from messagefoundry.api.models import ( + ClusterNodeList, + ClusterStatus, + DbInfo, + DrStatus, + EngineInfo, + SecurityPosture, + ServiceStatusInfo, + SystemStatus, +) +from messagefoundry.pipeline import Engine +from messagefoundry.store.store import ClaimProcStatus +from messagefoundry_webconsole.pages import monitoring + +# The exporter's strict label allowlist (the #21 PHI contract). The new gauges are label-LESS, so +# they must not widen it. +ALLOWED_LABELS = {"connection", "destination", "status", "version", "le"} + +_GREEN = ClaimProcStatus( + effective=True, + degraded_reason=None, + head_forms={ + "mefor_claim_fifo_heads_cid_v1": "rewritten", + "mefor_claim_fifo_heads_dst_v1": "rewritten", + }, +) +_DEGRADED = ClaimProcStatus( + effective=False, + degraded_reason=( + "stored procedure dbo.mefor_claim_fifo_heads_cid_v1 is DEPLOYED but its definition is" + " unreadable — GRANT VIEW DEFINITION" + ), + head_forms={}, +) +_VERBATIM = ClaimProcStatus( + effective=True, + degraded_reason=None, + head_forms={ + "mefor_claim_fifo_heads_cid_v1": "verbatim", + "mefor_claim_fifo_heads_dst_v1": "rewritten", + }, +) + + +@pytest.fixture +async def engine(tmp_path: Path) -> AsyncIterator[Engine]: + eng = await Engine.create(tmp_path / "gauge.db", poll_interval=0.02) + eng.started_at = 1.0 + yield eng + await eng.stop() + + +@pytest.fixture +async def client(engine: Engine) -> AsyncIterator[httpx.AsyncClient]: + transport = httpx.ASGITransport(app=create_app(engine, allow_no_auth=True)) + async with httpx.AsyncClient(transport=transport, base_url="http://t") as c: + yield c + + +def _set_gauge(engine: Engine, value: ClaimProcStatus | None) -> None: + """Substitute the store's gate verdict. The SQLite store answers ``None`` (AC-6: it has no such + lever), so a value here stands in for a SQL Server store whose gate reached that outcome.""" + engine.store.claim_proc_status = lambda: value # type: ignore[method-assign] + + +def _samples(exposition: str, name: str) -> list[float]: + return [ + s.value + for fam in text_string_to_metric_families(exposition) + for s in fam.samples + if s.name == name + ] + + +# --- /metrics ------------------------------------------------------------------------------- + + +async def test_metrics_emits_zero_when_the_gate_degraded(engine: Engine) -> None: + _set_gauge(engine, _DEGRADED) + exposition = (await render_metrics(engine)).decode() + assert _samples(exposition, "messagefoundry_store_claim_proc_effective") == [0.0] + + +async def test_metrics_emits_one_when_the_gate_passed(engine: Engine) -> None: + _set_gauge(engine, _GREEN) + exposition = (await render_metrics(engine)).decode() + assert _samples(exposition, "messagefoundry_store_claim_proc_effective") == [1.0] + + +async def test_metrics_omits_the_gauges_when_the_lever_was_never_requested(engine: Engine) -> None: + """ABSENT, not 0. Every SQLite/Postgres fleet — and every SQL Server fleet with the flag off — + would otherwise publish a permanent ``claim_proc_effective 0``, which reads as "the proc path is + broken here" and is unalertable. The default engine must therefore emit neither series.""" + exposition = (await render_metrics(engine)).decode() # SQLite store -> None, no substitution + assert _samples(exposition, "messagefoundry_store_claim_proc_effective") == [] + assert _samples(exposition, "messagefoundry_store_claim_proc_head_verbatim") == [] + + +async def test_metrics_head_verbatim_gauge_flags_a_non_rewriting_engine(engine: Engine) -> None: + """No engine measured to date stores a ``CREATE OR ALTER`` head verbatim, so a fleet reporting 1 + is a live counterexample to the gate's compatibility assumption — worth an alert, not a fault.""" + _set_gauge(engine, _VERBATIM) + assert _samples( + (await render_metrics(engine)).decode(), "messagefoundry_store_claim_proc_head_verbatim" + ) == [1.0] + _set_gauge(engine, _GREEN) + assert _samples( + (await render_metrics(engine)).decode(), "messagefoundry_store_claim_proc_head_verbatim" + ) == [0.0] + + +async def test_metrics_gauges_carry_no_labels_and_never_the_reason_string(engine: Engine) -> None: + """The reason is free text (a proc name; an exception string on the probe-failure arm). Carried + as a label it would be unbounded cardinality AND a breach of the #21 label allowlist, so it must + never reach the exposition — it lives on /status and the console instead.""" + _set_gauge(engine, _DEGRADED) + exposition = (await render_metrics(engine)).decode() + assert "VIEW DEFINITION" not in exposition + for fam in text_string_to_metric_families(exposition): + for sample in fam.samples: + assert set(sample.labels) <= ALLOWED_LABELS, f"{sample.name} widened the allowlist" + + +# --- /status -------------------------------------------------------------------------------- + + +async def test_status_carries_the_degraded_reason( + engine: Engine, client: httpx.AsyncClient +) -> None: + _set_gauge(engine, _DEGRADED) + body = (await client.get("/status")).json() + assert body["claim_proc"]["effective"] is False + assert "VIEW DEFINITION" in body["claim_proc"]["degraded_reason"] + + +async def test_status_carries_the_matched_head_forms_when_green( + engine: Engine, client: httpx.AsyncClient +) -> None: + _set_gauge(engine, _VERBATIM) + claim_proc = (await client.get("/status")).json()["claim_proc"] + assert claim_proc["effective"] is True + assert claim_proc["degraded_reason"] is None + assert claim_proc["head_forms"]["mefor_claim_fifo_heads_cid_v1"] == "verbatim" + + +async def test_status_omits_the_field_when_the_lever_was_never_requested( + client: httpx.AsyncClient, +) -> None: + """Additive + defaulted: the default (SQLite) payload is unchanged, so an older client + deserializes /status exactly as before.""" + assert (await client.get("/status")).json()["claim_proc"] is None + + +# --- the console store panel ------------------------------------------------------------------ + + +def _page(claim_proc: object) -> str: + """Render the console status page around a SystemStatus carrying ``claim_proc``.""" + sys_status = SystemStatus( + engine=EngineInfo( + version="0.0.0", + uptime_seconds=1.0, + pid=1, + channels_total=0, + channels_running=0, + channels_stopped=0, + outbox_by_status={}, + ), + db=DbInfo( + path="mem", + size_bytes=0, + disk_free_bytes=10 * 1024**3, + journal_mode="wal", + messages=0, + events=0, + audit=0, + ), + claim_proc=claim_proc, # type: ignore[arg-type] + ) + return str( + monitoring.status( + sys_status, + SecurityPosture( + backend="sqlserver", + encryption_enabled=True, + key_source="env", + require_encryption=True, + allow_unencrypted_phi=False, + ), + ClusterStatus( + node_id="n1", clustered=False, is_leader=True, role="single-node", config_version=0 + ), + ClusterNodeList(nodes=[], leader_node_id=None, lease_owner=None, lease_expires_at=None), + DrStatus(enabled=False, active=False, threshold="0", activation_mode="manual"), + ServiceStatusInfo(enabled=False, state="unknown", service_name="mefor"), + ) + ) + + +def test_console_store_panel_shows_the_degrade_and_its_reason() -> None: + html = _page({"effective": False, "degraded_reason": "GRANT VIEW DEFINITION on dbo.the_proc"}) + assert "DEGRADED" in html + assert "GRANT VIEW DEFINITION on dbo.the_proc" in html + + +def test_console_store_panel_shows_the_matched_head_forms_when_green() -> None: + html = _page({"effective": True, "head_forms": {"mefor_claim_fifo_heads_cid_v1": "verbatim"}}) + assert "DEGRADED" not in html + assert "mefor_claim_fifo_heads_cid_v1: verbatim" in html + + +def test_console_store_panel_is_absent_when_the_lever_was_never_requested() -> None: + """The default page is unchanged — no empty row, no "—" that reads as a broken lever.""" + assert "Claim path" not in _page(None)