From 8dfcd2e0c13f77985435bdbc2f8bed07c2ee92ad Mon Sep 17 00:00:00 2001 From: xmap <16776958+xmap@users.noreply.github.com> Date: Sat, 18 Jul 2026 11:49:27 +0300 Subject: [PATCH 1/2] Let a facility verify observe-only, not just trust it The read-only gate (#571) made the pilot posture safe by default: a blank deployment observes every route and drives none. But "safe by default" is a claim, and the people who most need it, a beamline manager or a controls engineer weighing whether to let CORA near their IOCs, are exactly the people who will never hold API credentials. They cannot read the config. So the posture was true but unverifiable, which for a safety claim to a national lab is not much better than false. /readyz now carries an `actuation` field, either `inert` or `reachable`, and a `boot.actuation_posture` line prints it at startup. Anyone who can curl the pod can now check the claim in one call instead of trusting it. It is a REPORT, not a gate. It reads the settings the real gates already use and summarises them; it decides nothing and refuses nothing. That separation is deliberate: a report that is wrong is a bug someone files, whereas a gate that can be talked into the wrong answer moves a motor. So this adds no new switch and no new failure mode, only a mirror. It folds in BOTH ways CORA can reach the beamline, because "read-only" means both must be shut: the ControlPort write path (control_writes_enabled) and the ComputePort exec path, where a compute job's argv could itself be `caput ...`. Only compute_substrate=in_memory is provably unable to spawn, so the compute test is an allowlist (== "in_memory"), not a denylist: a future globus or slurm substrate reads reachable by default rather than slipping through the day its literal lands. And it errs toward `reachable` when uncertain, inverting the getattr-None fail-open idiom the Conductor uses, because the pessimistic answer sits on the other side here: a false `reachable` is an alarm someone investigates, a false `inert` is a facility discovering CORA moved their stage after being told it could not. Two things it deliberately does not read, each documented at the derivation: cora_allow_raw_conduct (per-request, and inert under in_memory anyway; logged but excluded), and ActuationKind (it tracks simulator-vs-real, not writable-vs-not, so it cannot witness this posture, a trap two design proposals fell into). The boot log prints the raw inputs beside the summary so it can be audited against its own sources rather than trusted. Scope, named so it is not mistaken for more: this is the ACTUATION axis only. Sending data out (the LLM seam) and spending money are separate promises with their own switches; when they gain summaries they slot in beside `actuation` rather than widening it. Two fitness tests hold the line. One asserts the three actuation defaults ship safe, reading the declared field defaults (immune to the test env, which enables writes). The other fails the build if any dormant outbound adapter (FdtTransferPort, GlobusTransferPort, GlobusComputePort, all present but wired nowhere) gets constructed at the composition root without the derivation being revisited: the honest form of "we do not bound these today" is a tripwire, not a promise that the axis list is complete. Also corrects three stale docstrings the audit surfaced: the transfer port and its in-memory adapter both claimed no real adapter exists (two do, just unwired), and config's ComputeSubstrate comment claimed it mirrors the operation tier (it is deliberately narrower, excluding the inject-only globus). Co-Authored-By: Claude Opus 4.8 --- apps/api/src/cora/api/_readiness.py | 98 +++++++++++++- apps/api/src/cora/api/main.py | 22 +++- apps/api/src/cora/infrastructure/config.py | 21 ++- .../adapters/in_memory_transfer_port.py | 8 +- .../src/cora/operation/ports/transfer_port.py | 17 ++- .../test_actuation_defaults_fail_closed.py | 43 ++++++ .../test_dormant_outbound_seams_unwired.py | 124 ++++++++++++++++++ .../tests/contract/test_readyz_endpoint.py | 13 ++ apps/api/tests/unit/api/test_readiness.py | 56 +++++++- docs/stack/deployment.md | 52 +++++++- 10 files changed, 423 insertions(+), 31 deletions(-) create mode 100644 apps/api/tests/architecture/test_actuation_defaults_fail_closed.py create mode 100644 apps/api/tests/architecture/test_dormant_outbound_seams_unwired.py diff --git a/apps/api/src/cora/api/_readiness.py b/apps/api/src/cora/api/_readiness.py index b33f0184b64..1cbd9b810d9 100644 --- a/apps/api/src/cora/api/_readiness.py +++ b/apps/api/src/cora/api/_readiness.py @@ -57,6 +57,22 @@ deployment has no pool at all (the in-memory kernel), so there is nothing to report rather than nothing wrong.""" +ActuationReach = Literal["inert", "reachable"] +"""Whether this deployment can drive a physical effect on the beamline. + +`inert` = both actuation paths are shut: CORA may observe but cannot +move hardware. `reachable` = at least one path is open. + +This is a REPORT, not a gate. It reads the settings the real gates +already use (`ControlPort` write switch, `ComputePort` substrate) and +summarises them so a skeptical controls engineer can VERIFY the +observe-only claim in one call instead of trusting it. It decides +nothing and refuses nothing; the gates do that. Making it a pure report +is deliberate: a report that is wrong is a bug someone files, whereas a +gate that can be talked into the wrong answer moves a motor. See +`derive_actuation`. +""" + # Total wall budget for the probe, and the slice of it the acquire may # take. Both are load-bearing and neither is redundant: # @@ -120,24 +136,94 @@ async def probe_database(pool: asyncpg.Pool | None) -> DatabaseStatus: return "ok" +def derive_actuation(settings: Settings) -> ActuationReach: + """Summarise whether either actuation path can drive hardware. + + Two paths reach a beamline, and this reports `inert` only when BOTH + are shut: + + - the ControlPort write path: `control_writes_enabled` (a caput + that moves a motor); + - the ComputePort exec path: a compute job whose argv could itself + be `caput ...`, reaching a control system without touching the + ControlPort. Only `compute_substrate == "in_memory"` is provably + unable to spawn anything, so any OTHER substrate reads as + reachable. + + The compute test is written as an ALLOWLIST (`== "in_memory"`), not a + denylist (`!= "local_process"`), on purpose: a future `globus` or + `slurm` substrate is not-in_memory and so reads reachable by default, + rather than slipping through a denylist the day its literal is added. + + ## Fail closed in the REPORTING direction + + When the answer is uncertain, this returns `reachable`, never + `inert`. That deliberately INVERTS the `getattr(..., None)` idiom the + Conductor's `_ActuationObserver` uses (whose absent case fails OPEN), + because the pessimistic answer sits on the other side here: a false + `reachable` is an alarm someone investigates and files as a bug; a + false `inert` is a facility discovering CORA moved their stage after + being told it could not. Only the first is survivable. + + ## What this deliberately does NOT read + + `cora_allow_raw_conduct` is excluded. It is read per-request, not at + the composition root, and it is inert while `compute_substrate` is + `in_memory` (a raw argv handed to the in-memory port spawns nothing). + It is logged at boot beside this summary (see `main`) so an operator + can still see it, but it does not move this value. + + `ActuationKind` (the Conductor's Physical/Simulated/Hybrid stamp) + also cannot witness this posture and is not consulted: it tracks + whether a route is a SIMULATOR, not whether it is WRITABLE, so an + observe-only conduct over a non-simulated route still derives + Physical. Simulated-ness and write-reachability are orthogonal axes + that merely share the "actuation" lexeme. + + ## Scope + + This is the ACTUATION axis only (moving hardware). The EGRESS axis + (sending data out, e.g. the LLM seam) and the SPEND axis are separate + promises with their own switches; when they gain reported summaries + they slot in beside this one rather than widening its meaning. + """ + control_can_write = settings.control_writes_enabled + compute_can_spawn = settings.compute_substrate != "in_memory" + if control_can_write or compute_can_spawn: + return "reachable" + return "inert" + + def readiness_body(database: DatabaseStatus, settings: Settings) -> dict[str, str]: """Render the probe result. Fixed vocabulary, no free text. `app_env` is here because `test` builds an in-memory kernel with no persistence; a green probe over a store that loses every Run on - restart is worth making visible to whoever reads this. + restart is worth making visible to whoever reads this. `actuation` + is here because the pilot's whole value is that management, who will + never hold credentials, can curl the observe-only claim: a green + probe on a process that could move a beamline clears that same bar, + for a stronger reason. Deliberately absent: the database URL, the driver's error text, and - the names of projections or bounded contexts. This endpoint is - unauthenticated, and none of that helps a probe while all of it - describes the deployment to anyone who can reach it. The logs carry - the detail. + the names of projections, bounded contexts, or route prefixes. This + endpoint is unauthenticated, and none of that helps a probe while all + of it describes the deployment to anyone who can reach it. `actuation` + is a scalar for exactly this reason: it says inert-or-reachable, not + WHICH prefixes are writable. The logs carry the detail. """ return { "status": "ready" if database in ("ok", "skipped") else "not_ready", "database": database, "app_env": settings.app_env, + "actuation": derive_actuation(settings), } -__all__ = ["DatabaseStatus", "probe_database", "readiness_body"] +__all__ = [ + "ActuationReach", + "DatabaseStatus", + "derive_actuation", + "probe_database", + "readiness_body", +] diff --git a/apps/api/src/cora/api/main.py b/apps/api/src/cora/api/main.py index d3f43116eca..ea0779ef441 100644 --- a/apps/api/src/cora/api/main.py +++ b/apps/api/src/cora/api/main.py @@ -82,7 +82,7 @@ from cora.api._enclosure_permit_observer import ControlPortEnclosureObserver from cora.api._inference_recorder import DelegatingInferenceRecorder from cora.api._procedure_watcher import procedure_watcher_lifespan -from cora.api._readiness import probe_database, readiness_body +from cora.api._readiness import derive_actuation, probe_database, readiness_body from cora.api._run_initiator import run_initiator_lifespan from cora.api._run_supervisor import run_supervisor_lifespan from cora.api.middleware import BodySizeLimitMiddleware @@ -182,6 +182,7 @@ from cora.infrastructure.deps import build_kernel from cora.infrastructure.idempotency_pruner import idempotency_pruner_lifespan from cora.infrastructure.kernel import Kernel +from cora.infrastructure.logging import get_logger from cora.infrastructure.observability import configure_tracing, instrument_app from cora.infrastructure.projection import ( ProjectionRegistry, @@ -271,6 +272,8 @@ def _settings_for_app() -> Settings: # local and any other env name keep the permissive default. _PROD_LIKE_APP_ENVS = frozenset({"prod", "production", "staging"}) +_log = get_logger(__name__) + def _enforce_production_principal_policy(settings: Settings) -> None: """Refuse to boot deployments where the principal-fallback would @@ -756,6 +759,23 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None]: app.state.operation = wire_operation( deps, control_port=shared_control_port, compute_port=compute_port ) + + # Boot posture. The derived `actuation` summary rides ALONGSIDE + # its own raw inputs, never instead of them, so the one-word + # answer can be audited against the settings it came from + # rather than trusted. `cora_allow_raw_conduct` is logged but + # excluded from the summary (it is read per-request and inert + # under compute_substrate=in_memory); see `derive_actuation`. + _log.info( + "boot.actuation_posture", + actuation=derive_actuation(settings), + control_writes_enabled=settings.control_writes_enabled, + control_port_route_count=len(settings.control_port_routes), + compute_substrate=settings.compute_substrate, + compute_permitted_executable_count=len(settings.compute_permitted_executables), + cora_allow_raw_conduct=settings.cora_allow_raw_conduct, + ) + app.state.safety = wire_safety(deps) app.state.caution = wire_caution(deps) app.state.calibration = wire_calibration(deps) diff --git a/apps/api/src/cora/infrastructure/config.py b/apps/api/src/cora/infrastructure/config.py index 45238c3b765..e6723e12a44 100644 --- a/apps/api/src/cora/infrastructure/config.py +++ b/apps/api/src/cora/infrastructure/config.py @@ -18,11 +18,22 @@ OtelExporter = Literal["otlp", "console", "none"] -# ComputePort substrate selector. Mirrors the operation-tier -# `ComputeSubstrate` in `cora.operation.adapters.compute_port_config` -# (a trivial 2-value Literal kept per tier rather than centralised in a -# new infrastructure module, since there is no shared route model the -# way ControlPort's `Substrate` rides `ControlPortRoute`). +# ComputePort substrate selector. Deliberately NARROWER than the +# operation-tier `ComputeSubstrate` in +# `cora.operation.adapters.compute_port_config`, which is a 3-value +# Literal (adds `globus`). `globus` is excluded here on purpose: it +# cannot be built from a flat config string (it needs an authorized +# client + endpoint id + a remote artifact probe), so it is injected as +# a prebuilt port at the composition root, never selected via this env +# var. This tier therefore names only the substrates a deployment can +# actually pick with `COMPUTE_SUBSTRATE`. Kept per-tier rather than +# centralised because there is no shared route model the way +# ControlPort's `Substrate` rides `ControlPortRoute`. +# +# `derive_actuation` (cora.api._readiness) reads this as an allowlist +# (`== "in_memory"` is the only provably-inert value), so if a real +# selectable substrate is ever added here, it correctly reads as +# actuation-reachable by default. ComputeSubstrate = Literal["in_memory", "local_process"] diff --git a/apps/api/src/cora/operation/adapters/in_memory_transfer_port.py b/apps/api/src/cora/operation/adapters/in_memory_transfer_port.py index 93898687905..5cbc299c03f 100644 --- a/apps/api/src/cora/operation/adapters/in_memory_transfer_port.py +++ b/apps/api/src/cora/operation/adapters/in_memory_transfer_port.py @@ -3,9 +3,11 @@ Dict-backed, no substrate. A test seeds the observation progression a begun move will report; the engine-under-test calls `begin` / `observe` / `cancel` against the same instance and walks the seeded snapshots toward a terminal. -This is the only `TransferPort` adapter that exists: there is no production -substrate adapter yet, because the build trigger has not fired (see -`cora.operation.ports.transfer_port`). The fake serves the triage that asks +This is the test double. Real substrate adapters (`FdtTransferPort`, +`GlobusTransferPort`) now exist too, but none is constructed at the +composition root, so no live egress path runs (see +`cora.operation.ports.transfer_port` and +`test_dormant_outbound_seams_unwired`). The fake serves the triage that asks whether a transfer is a conductor step or a long-running edge job. ## Seeding model diff --git a/apps/api/src/cora/operation/ports/transfer_port.py b/apps/api/src/cora/operation/ports/transfer_port.py index d938840bf1e..b555d222347 100644 --- a/apps/api/src/cora/operation/ports/transfer_port.py +++ b/apps/api/src/cora/operation/ports/transfer_port.py @@ -21,13 +21,16 @@ ## Earned, not minted -There is no production consumer yet. This port exists to finalize the design -triage (is a transfer a step the conductor walks, or a long-running edge job -with its own lifecycle?), and ships with a test double only. A real adapter -(Globus first) and the executing aggregate land when the build trigger fires: -a promote or publish rule that blocks on a transfer outcome, an in-path -substrate with a native partial terminal, or a custody chain the existing -`Attestation` path cannot carry. Until then this is a spike, not a merge. +Adapters now exist (`InMemoryTransferPort` for tests; `FdtTransferPort` and +`GlobusTransferPort` as real substrates), and `cora.api._distribution_materializer` +consumes the port. But NONE of the real adapters is constructed at the +composition root: `build_control_port`'s compute sibling does not wire them, +and `test_dormant_outbound_seams_unwired` pins that they stay unwired. So the +executing aggregate and the composition-root wiring still wait on the build +trigger: a promote or publish rule that blocks on a transfer outcome, an +in-path substrate with a native partial terminal, or a custody chain the +existing `Attestation` path cannot carry. Until one fires, the real adapters +are dormant code, not a live egress path. ## Why `begin` / `observe` / `cancel`, not blocking submit-and-await diff --git a/apps/api/tests/architecture/test_actuation_defaults_fail_closed.py b/apps/api/tests/architecture/test_actuation_defaults_fail_closed.py new file mode 100644 index 00000000000..e4e0e3aaaac --- /dev/null +++ b/apps/api/tests/architecture/test_actuation_defaults_fail_closed.py @@ -0,0 +1,43 @@ +"""The actuation defaults ship SAFE: a blank deployment cannot drive hardware. + +CORA's observe-only pilot posture is not a mode an operator selects; it is +the DEFAULT. A `docker run` with only a database URL and a route table can +observe every configured PV and drive none. That property is the whole +adoption pitch ("install it, it cannot touch your hardware"), and it holds +only while all three actuation-relevant defaults stay on the safe side: + + - `control_writes_enabled` defaults False (the ControlPort write gate); + - `compute_substrate` defaults `in_memory` (spawns no subprocess, so the + ComputePort exec path cannot reach a control system); + - `compute_permitted_executables` defaults empty (even if a deployment + switches to `local_process`, an empty allowlist permits nothing). + +A future edit that flips any of these defaults would silently widen every +observe-only deployment on the growth ladder at once, with zero test +failure, until a facility noticed CORA had moved something. This fitness +turns that into a build break. + +## Why the DECLARED default, not an instantiated Settings + +`tests/conftest.py` sets `CONTROL_WRITES_ENABLED=true` for the test +environment (observe-only is a production posture; the suite exercises the +full writable surface). So `Settings().control_writes_enabled` is True under +test and would tell us nothing about what ships. `model_fields[...].default` +reads the class's DECLARED default directly, immune to the environment, so +this asserts the property a real deployment gets, not the one the test env +overrides. +""" + +from cora.infrastructure.config import Settings + + +def test_control_writes_default_is_off() -> None: + assert Settings.model_fields["control_writes_enabled"].default is False + + +def test_compute_substrate_default_is_in_memory() -> None: + assert Settings.model_fields["compute_substrate"].default == "in_memory" + + +def test_compute_permitted_executables_default_is_empty() -> None: + assert Settings.model_fields["compute_permitted_executables"].default == frozenset() diff --git a/apps/api/tests/architecture/test_dormant_outbound_seams_unwired.py b/apps/api/tests/architecture/test_dormant_outbound_seams_unwired.py new file mode 100644 index 00000000000..f13ee2cddb8 --- /dev/null +++ b/apps/api/tests/architecture/test_dormant_outbound_seams_unwired.py @@ -0,0 +1,124 @@ +"""Dormant outbound seams stay unwired until someone accounts for the posture. + +`derive_actuation` (`cora.api._readiness`) summarises whether CORA can drive +the beamline, reading two inputs: the ControlPort write switch and the +ComputePort substrate. Its `inert` answer is only honest while the OTHER +outbound seams remain dormant, and three of them are dormant code that +exists but is constructed nowhere: + + - `FdtTransferPort` and `GlobusTransferPort` move data to remote storage + (the EGRESS axis; `FdtTransferPort` also spawns a subprocess). Neither + is wired at the composition root. + - `GlobusComputePort` runs a job on a remote endpoint (remote execution, + which leans on both the actuation and spend axes). `build_compute_port` + has a `prebuilt_port` seam for it, but no `GlobusComputePort` is ever + constructed and passed in. + +If a future change wires any of these, the actuation summary and the +observe-only story go stale silently: `derive_actuation` would keep saying +`inert` while a new path to the outside had opened. This fitness makes that +a build break instead, pointing the author back at the derivation and the +egress/spend axes it does not yet cover. It is the honest form of "we do not +bound these today": it does not stop anyone wiring them, it stops anyone +wiring them WITHOUT the conversation. + +Enumerated from git's tracked-file set (never `rglob`), so a half-staged +refactor does not false-fail under pre-commit. +""" + +import ast +from pathlib import Path + +import pytest + +from tests.architecture.conftest import tracked_python_files + +# Classes whose construction anywhere in production `src` is the trigger. +_DORMANT_OUTBOUND: frozenset[str] = frozenset( + { + "FdtTransferPort", + "GlobusTransferPort", + "GlobusComputePort", + } +) + +# Modules permitted to construct a dormant seam. Empty by design: the day +# one is wired, add its composition-root module here WITH a note that +# `cora.api._readiness.derive_actuation` (and the egress/spend axes) were +# revisited, or the observe-only report goes stale unnoticed. +_ALLOWLIST: frozenset[str] = frozenset() + +# tests/architecture/.py -> apps/api/ +_API_ROOT = Path(__file__).resolve().parents[2] + + +def _qualified(path: Path) -> str: + return str(path.relative_to(_API_ROOT)) + + +def _construction_lines(tree: ast.AST) -> dict[str, list[int]]: + """Line numbers where a dormant seam is CONSTRUCTED (a bare-Name call). + + `FdtTransferPort(...)` counts; an import or a type annotation does not. + """ + hits: dict[str, list[int]] = {} + for node in ast.walk(tree): + if ( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Name) + and node.func.id in _DORMANT_OUTBOUND + ): + hits.setdefault(node.func.id, []).append(node.lineno) + return hits + + +def _candidate_files() -> list[Path]: + """Tracked src files that textually mention any dormant seam with a `(`. + + Cheap heuristic; the AST scan below confirms a real call. The class + definition files themselves mention the name at `class X:` (no `X(` + call), so they do not become candidates. + """ + needles = tuple(f"{name}(" for name in _DORMANT_OUTBOUND) + out: list[Path] = [] + for path in tracked_python_files(): + text = path.read_text(encoding="utf-8") + if any(needle in text for needle in needles): + out.append(path) + return sorted(out) + + +@pytest.mark.architecture +@pytest.mark.parametrize("py_file", _candidate_files(), ids=_qualified) +def test_dormant_outbound_seam_is_not_constructed(py_file: Path) -> None: + qualified = _qualified(py_file) + tree = ast.parse(py_file.read_text(encoding="utf-8"), filename=str(py_file)) + hits = _construction_lines(tree) + if not hits: + return + if qualified in _ALLOWLIST: + return + detail = ", ".join(f"{name} at line {min(lines)}" for name, lines in sorted(hits.items())) + pytest.fail( + f"{qualified} constructs a dormant outbound seam ({detail}). Wiring one " + f"opens a new path to the outside that `cora.api._readiness.derive_actuation` " + f"does not account for, so the /readyz `actuation` summary and the observe-only " + f"story would go stale silently. Revisit the derivation (and the egress/spend " + f"axes it does not yet cover), then add {qualified!r} to this test's _ALLOWLIST " + f"with a note that you did." + ) + + +@pytest.mark.architecture +def test_dormant_outbound_allowlist_has_no_stale_entries() -> None: + """Every _ALLOWLIST entry must still construct a seam, or it is stale.""" + live = { + _qualified(p) + for p in _candidate_files() + if _construction_lines(ast.parse(p.read_text(encoding="utf-8"))) + } + stale = sorted(_ALLOWLIST - live) + assert stale == [], ( + f"_ALLOWLIST names {stale}, which no longer construct a dormant outbound " + f"seam. Prune them from {Path(__file__).name}." + ) diff --git a/apps/api/tests/contract/test_readyz_endpoint.py b/apps/api/tests/contract/test_readyz_endpoint.py index 70484da1c04..22d44e4df8f 100644 --- a/apps/api/tests/contract/test_readyz_endpoint.py +++ b/apps/api/tests/contract/test_readyz_endpoint.py @@ -30,6 +30,19 @@ def test_readyz_reports_ready_200_with_no_pool() -> None: assert "Retry-After" not in response.headers +@pytest.mark.contract +def test_readyz_surfaces_the_actuation_posture() -> None: + """The observe-only claim a facility can curl without credentials. + + The test env enables control writes (see conftest), so this app reports + `reachable`; the point under test is that the field is present and + end-to-end from settings through the route, not its value here. + """ + with TestClient(create_app()) as client: + body = client.get("/readyz").json() + assert body["actuation"] in ("inert", "reachable") + + @pytest.mark.contract def test_readyz_reports_503_and_retry_after_when_database_faults( monkeypatch: pytest.MonkeyPatch, diff --git a/apps/api/tests/unit/api/test_readiness.py b/apps/api/tests/unit/api/test_readiness.py index 1220a3232dd..f2f4ae8da8c 100644 --- a/apps/api/tests/unit/api/test_readiness.py +++ b/apps/api/tests/unit/api/test_readiness.py @@ -18,7 +18,7 @@ import asyncpg import pytest -from cora.api._readiness import probe_database, readiness_body +from cora.api._readiness import derive_actuation, probe_database, readiness_body from cora.infrastructure.config import Settings @@ -171,4 +171,56 @@ def test_readiness_body_carries_app_env() -> None: @pytest.mark.unit def test_readiness_body_leaks_no_deployment_detail() -> None: """Unauthenticated endpoint: fixed vocabulary, no free text, no topology.""" - assert set(readiness_body("ok", _settings())) == {"status", "database", "app_env"} + assert set(readiness_body("ok", _settings())) == {"status", "database", "app_env", "actuation"} + + +def _actuation_settings(*, writes: bool, substrate: str) -> Settings: + """Settings with the two actuation inputs pinned; init kwargs beat the env.""" + return Settings( + app_env="test", + control_writes_enabled=writes, + compute_substrate=substrate, # type: ignore[arg-type] + ) + + +@pytest.mark.unit +def test_derive_actuation_reports_inert_when_both_paths_shut() -> None: + """The observe-only default: writes off and compute cannot spawn.""" + assert derive_actuation(_actuation_settings(writes=False, substrate="in_memory")) == "inert" + + +@pytest.mark.unit +def test_derive_actuation_reachable_when_control_writes_on() -> None: + assert derive_actuation(_actuation_settings(writes=True, substrate="in_memory")) == "reachable" + + +@pytest.mark.unit +def test_derive_actuation_reachable_when_compute_can_spawn() -> None: + """The bypass path: local_process can run argv that reaches a control system.""" + assert ( + derive_actuation(_actuation_settings(writes=False, substrate="local_process")) + == "reachable" + ) + + +@pytest.mark.unit +def test_derive_actuation_reachable_when_both_paths_open() -> None: + assert ( + derive_actuation(_actuation_settings(writes=True, substrate="local_process")) == "reachable" + ) + + +@pytest.mark.unit +def test_readiness_body_reports_the_derived_actuation() -> None: + inert = readiness_body("ok", _actuation_settings(writes=False, substrate="in_memory")) + assert inert["actuation"] == "inert" + reachable = readiness_body("ok", _actuation_settings(writes=True, substrate="in_memory")) + assert reachable["actuation"] == "reachable" + + +@pytest.mark.unit +def test_readiness_body_actuation_is_independent_of_database_status() -> None: + """A DB fault does not change the actuation posture; they are orthogonal.""" + body = readiness_body("unreachable", _actuation_settings(writes=False, substrate="in_memory")) + assert body["status"] == "not_ready" + assert body["actuation"] == "inert" diff --git a/docs/stack/deployment.md b/docs/stack/deployment.md index 5959242b0df..77ed5b8027a 100644 --- a/docs/stack/deployment.md +++ b/docs/stack/deployment.md @@ -169,13 +169,14 @@ every pod into an outage the restart cannot mend, converting one database blip into a fleet-wide crash loop. `/readyz` returns 200 `{"status": "ready", ...}` or 503 -`{"status": "not_ready", "database": "..."}`. `database` is a fixed -vocabulary: `ok`, `unreachable`, `saturated`, `closing`, `skipped` -(this deployment has no pool, the in-memory kernel), `error`. The body -carries no URLs, no driver error text, and no projection or bounded -context names: it is unauthenticated, and none of that helps a probe -while all of it describes the deployment to whoever can reach it. The -logs carry the detail. +`{"status": "not_ready", "database": "..."}`, plus an `actuation` field +on both (see below). `database` is a fixed vocabulary: `ok`, +`unreachable`, `saturated`, `closing`, `skipped` (this deployment has no +pool, the in-memory kernel), `error`. The body carries no URLs, no +driver error text, and no projection or bounded context names: it is +unauthenticated, and none of that helps a probe while all of it +describes the deployment to whoever can reach it. The logs carry the +detail. Postgres is the only check, because readiness must report what can CHANGE after a successful boot rather than restate boot. Every config @@ -188,6 +189,43 @@ a global condition: it would pull every replica at once, including the pod hosting the in-band repair tool. Watch projection lag with a metric and an alert, not with a traffic-routing signal. +### Verifying the observe-only posture + +`/readyz` also carries `actuation`, either `inert` or `reachable`, and a +matching `boot.actuation_posture` line lands in the startup log. This is +the field that lets someone who will never hold credentials, a beamline +manager or a controls engineer, curl the observe-only claim instead of +trusting it. `inert` means CORA can observe but cannot move hardware; +`reachable` means at least one actuation path is open. + +It is a REPORT, not a gate: it reads the settings the real gates already +use and summarises them, deciding and refusing nothing. It folds in BOTH +ways CORA can reach the beamline, and says `inert` only when both are +shut: + +- the ControlPort write path (`CONTROL_WRITES_ENABLED`), and +- the ComputePort exec path, where a compute job's argv could itself be + `caput ...`. Only `COMPUTE_SUBSTRATE=in_memory` is provably unable to + spawn anything, so any other substrate reads `reachable`. + +It errs toward `reachable` when uncertain: a false `reachable` is an +alarm someone investigates, a false `inert` is a facility discovering +CORA moved their stage after being told it could not. The boot log +prints the raw inputs (`control_writes_enabled`, `compute_substrate`, +`compute_permitted_executable_count`, `cora_allow_raw_conduct`, route +count) beside the summary so it can be audited against its own sources. + +Scope, stated so it is not mistaken for more: `actuation` covers the +hardware axis only. Sending data OUT (the LLM seam, remote transfers) +and SPENDING money are separate promises with their own switches; when +they gain summaries they slot in beside `actuation` rather than widening +its meaning. Today the outbound transfer and remote-compute adapters +exist but are wired nowhere, and a fitness test +(`test_dormant_outbound_seams_unwired`) fails the build if one gets +constructed without the actuation derivation being revisited. The +trigger to build the spanning egress/spend summary is the first of those +paths that gets wired at the composition root. + **Probe budget invariant.** The app bounds `/readyz` at 1.5s total. Set the orchestrator's own probe timeout ABOVE that (2s or more). Nothing enforces this. If the orchestrator gives up first, its From daa20e9d32ae0a1b655f7781751e9e64b1262234 Mon Sep 17 00:00:00 2001 From: xmap <16776958+xmap@users.noreply.github.com> Date: Tue, 28 Jul 2026 08:25:04 +0300 Subject: [PATCH 2/2] Arm the tripwire the docstring already promised Gate review took the previous commit's own claim seriously and found the guard weaker than its prose in three ways, each mutation-verified here before and after. An aliased import defeated it. `from ... import GlobusComputePort as _G` then `_G(...)` constructs a dormant seam under a name the bare-Name AST scan never sees, and an attribute call (`adapters.FdtTransferPort(...)`) walked past it too. The scan now resolves import aliases and accepts both call shapes; the candidate needle widened from `Name(` to the bare class name so an aliasing file still reaches the AST pass at all. Its targets were bare strings, so a rename would have disarmed it silently: green, and watching nothing. Every entry is now pinned to a real `ast.ClassDef` in tracked source, and a rename fails the build with a message saying which reading applies. And it collected an EMPTY parameter set, which is indistinguishable from a clean tree. A guard that cannot be observed to fire is a guard nobody should trust, so there is now a positive control feeding the detector a synthetic construction of each shape, a negative control proving an import or annotation alone does not trip it, and a drift catcher asserting the enumerator saw any source at all. Also from the review: the readyz contract test asserted only vocabulary membership, which no implementation returning a valid literal can fail, including a hardcoded one. It now injects Settings through the supported `create_app(settings=...)` seam and pins BOTH directions, which is env-immune (the suite enables writes) where the previous form was not. The three defaults tests gained the `@pytest.mark.architecture` their sibling carries. And the transfer-port docstring justified its claim with an unrelated artifact; it now cites the facts that carry the weight, that no transfer-port factory exists and the port's only consumer takes it as an injected field and is itself never constructed. Declined one nit: adding an inventory of new symbols to `_readiness.py`'s module docstring. The verifier refuted it, and I agree. The pre-existing `app_env` field sets the identical precedent (a non-probe key justified at the function, never in the module docstring), and no sibling module in `cora/api` enumerates its exports. Co-Authored-By: Claude Opus 4.8 --- .../src/cora/operation/ports/transfer_port.py | 18 +-- .../test_actuation_defaults_fail_closed.py | 5 + .../test_dormant_outbound_seams_unwired.py | 126 ++++++++++++++++-- .../tests/contract/test_readyz_endpoint.py | 27 +++- 4 files changed, 148 insertions(+), 28 deletions(-) diff --git a/apps/api/src/cora/operation/ports/transfer_port.py b/apps/api/src/cora/operation/ports/transfer_port.py index b555d222347..d9d1ce5f760 100644 --- a/apps/api/src/cora/operation/ports/transfer_port.py +++ b/apps/api/src/cora/operation/ports/transfer_port.py @@ -23,14 +23,16 @@ Adapters now exist (`InMemoryTransferPort` for tests; `FdtTransferPort` and `GlobusTransferPort` as real substrates), and `cora.api._distribution_materializer` -consumes the port. But NONE of the real adapters is constructed at the -composition root: `build_control_port`'s compute sibling does not wire them, -and `test_dormant_outbound_seams_unwired` pins that they stay unwired. So the -executing aggregate and the composition-root wiring still wait on the build -trigger: a promote or publish rule that blocks on a transfer outcome, an -in-path substrate with a native partial terminal, or a custody chain the -existing `Attestation` path cannot carry. Until one fires, the real adapters -are dormant code, not a live egress path. +consumes the port. But nothing constructs any of it: there is no transfer-port +factory (no `build_transfer_port` sibling to `build_control_port`), the only +consumer takes its port as an injected field, and that consumer is itself +never constructed in `src`. `test_dormant_outbound_seams_unwired` pins the +real adapters as unconstructed. So the executing aggregate and the +composition-root wiring still wait on the build trigger: a promote or publish +rule that blocks on a transfer outcome, an in-path substrate with a native +partial terminal, or a custody chain the existing `Attestation` path cannot +carry. Until one fires, the real adapters are dormant code, not a live egress +path. ## Why `begin` / `observe` / `cancel`, not blocking submit-and-await diff --git a/apps/api/tests/architecture/test_actuation_defaults_fail_closed.py b/apps/api/tests/architecture/test_actuation_defaults_fail_closed.py index e4e0e3aaaac..afbca7d75bd 100644 --- a/apps/api/tests/architecture/test_actuation_defaults_fail_closed.py +++ b/apps/api/tests/architecture/test_actuation_defaults_fail_closed.py @@ -28,16 +28,21 @@ overrides. """ +import pytest + from cora.infrastructure.config import Settings +@pytest.mark.architecture def test_control_writes_default_is_off() -> None: assert Settings.model_fields["control_writes_enabled"].default is False +@pytest.mark.architecture def test_compute_substrate_default_is_in_memory() -> None: assert Settings.model_fields["compute_substrate"].default == "in_memory" +@pytest.mark.architecture def test_compute_permitted_executables_default_is_empty() -> None: assert Settings.model_fields["compute_permitted_executables"].default == frozenset() diff --git a/apps/api/tests/architecture/test_dormant_outbound_seams_unwired.py b/apps/api/tests/architecture/test_dormant_outbound_seams_unwired.py index f13ee2cddb8..b76fd567bdf 100644 --- a/apps/api/tests/architecture/test_dormant_outbound_seams_unwired.py +++ b/apps/api/tests/architecture/test_dormant_outbound_seams_unwired.py @@ -56,34 +56,67 @@ def _qualified(path: Path) -> str: return str(path.relative_to(_API_ROOT)) +def _alias_names(tree: ast.AST) -> dict[str, str]: + """Local name -> dormant class, for `from ... import X as Y` forms. + + Without this an aliased import (`import GlobusComputePort as _G`, then + `_G(...)`) constructs a seam under a name the bare scan never sees. + """ + aliases: dict[str, str] = {} + for node in ast.walk(tree): + if not isinstance(node, ast.ImportFrom | ast.Import): + continue + for imported in node.names: + if imported.name in _DORMANT_OUTBOUND and imported.asname: + aliases[imported.asname] = imported.name + return aliases + + def _construction_lines(tree: ast.AST) -> dict[str, list[int]]: - """Line numbers where a dormant seam is CONSTRUCTED (a bare-Name call). + """Line numbers where a dormant seam is CONSTRUCTED. + + Counts three call shapes, because a guard that only sees the obvious + one is a guard someone routes around without meaning to: - `FdtTransferPort(...)` counts; an import or a type annotation does not. + - bare name: `FdtTransferPort(...)` + - attribute: `adapters.FdtTransferPort(...)` + - aliased import: `from ... import FdtTransferPort as _F` then `_F(...)` + + An import alone, a type annotation, and a `class X:` definition do not + count; only a call does. """ + aliases = _alias_names(tree) hits: dict[str, list[int]] = {} for node in ast.walk(tree): - if ( - isinstance(node, ast.Call) - and isinstance(node.func, ast.Name) - and node.func.id in _DORMANT_OUTBOUND - ): - hits.setdefault(node.func.id, []).append(node.lineno) + if not isinstance(node, ast.Call): + continue + func = node.func + name: str | None = None + if isinstance(func, ast.Name): + if func.id in _DORMANT_OUTBOUND: + name = func.id + elif func.id in aliases: + name = aliases[func.id] + elif isinstance(func, ast.Attribute) and func.attr in _DORMANT_OUTBOUND: + name = func.attr + if name is not None: + hits.setdefault(name, []).append(node.lineno) return hits def _candidate_files() -> list[Path]: - """Tracked src files that textually mention any dormant seam with a `(`. + """Tracked src files that textually mention any dormant seam. - Cheap heuristic; the AST scan below confirms a real call. The class - definition files themselves mention the name at `class X:` (no `X(` - call), so they do not become candidates. + Cheap heuristic; the AST scan confirms a real call. The needle is the + BARE class name, not `name(`, so a file that imports under an alias + and calls the alias still becomes a candidate. That admits the class + definition files themselves (`class FdtTransferPort:`), which is + harmless: the AST scan finds no call there. """ - needles = tuple(f"{name}(" for name in _DORMANT_OUTBOUND) out: list[Path] = [] for path in tracked_python_files(): text = path.read_text(encoding="utf-8") - if any(needle in text for needle in needles): + if any(name in text for name in _DORMANT_OUTBOUND): out.append(path) return sorted(out) @@ -122,3 +155,68 @@ def test_dormant_outbound_allowlist_has_no_stale_entries() -> None: f"_ALLOWLIST names {stale}, which no longer construct a dormant outbound " f"seam. Prune them from {Path(__file__).name}." ) + + +@pytest.mark.architecture +def test_every_guarded_seam_still_names_a_real_class() -> None: + """A rename must break this build, not silently disarm the guard. + + `_DORMANT_OUTBOUND` holds bare strings, so renaming `FdtTransferPort` + would leave the guard scanning for a name nothing defines: green, and + watching nothing. This pins each entry to a class that actually exists + in tracked source. + """ + defined: set[str] = set() + for path in tracked_python_files(): + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + defined.update(node.name for node in ast.walk(tree) if isinstance(node, ast.ClassDef)) + missing = sorted(_DORMANT_OUTBOUND - defined) + assert missing == [], ( + f"{missing} appear in _DORMANT_OUTBOUND but no tracked src file defines a " + f"class by that name. Either the seam was renamed (update the set, the " + f"guard is currently watching nothing) or it was deleted (drop the entry)." + ) + + +@pytest.mark.architecture +def test_the_scan_enumerates_source_files() -> None: + """A blinded enumerator would make every scan vacuously green.""" + assert tracked_python_files(), ( + "tracked_python_files() returned nothing, so the dormant-seam scan " + "inspected no source at all and its silence means nothing." + ) + + +@pytest.mark.architecture +@pytest.mark.parametrize( + ("source", "shape"), + [ + ("port = FdtTransferPort()", "bare name"), + ("port = adapters.FdtTransferPort()", "module attribute"), + ( + "from cora.operation.adapters.fdt_transfer_port import " + "FdtTransferPort as _F\nport = _F()", + "aliased import", + ), + ], +) +def test_the_detector_fires_on_each_construction_shape(source: str, shape: str) -> None: + """Positive control: the detector must actually detect. + + Without this, an empty parameter set and a broken detector look + identical, and the guard's silence proves nothing. + """ + hits = _construction_lines(ast.parse(source)) + assert hits == {"FdtTransferPort": [hits["FdtTransferPort"][0]]}, ( + f"the detector missed a {shape} construction, so a real one would pass unnoticed" + ) + + +@pytest.mark.architecture +def test_the_detector_ignores_imports_and_annotations() -> None: + """Negative control: only a CALL counts, or the guard cries wolf.""" + source = ( + "from cora.operation.adapters.fdt_transfer_port import FdtTransferPort\n" + "def f(port: FdtTransferPort) -> None: ...\n" + ) + assert _construction_lines(ast.parse(source)) == {} diff --git a/apps/api/tests/contract/test_readyz_endpoint.py b/apps/api/tests/contract/test_readyz_endpoint.py index 22d44e4df8f..5d819e11fa3 100644 --- a/apps/api/tests/contract/test_readyz_endpoint.py +++ b/apps/api/tests/contract/test_readyz_endpoint.py @@ -16,6 +16,7 @@ import cora.api.main as api_main from cora.api.main import create_app +from cora.infrastructure.config import Settings @pytest.mark.contract @@ -31,16 +32,30 @@ def test_readyz_reports_ready_200_with_no_pool() -> None: @pytest.mark.contract -def test_readyz_surfaces_the_actuation_posture() -> None: +def test_readyz_reports_inert_for_an_observe_only_deployment() -> None: """The observe-only claim a facility can curl without credentials. - The test env enables control writes (see conftest), so this app reports - `reachable`; the point under test is that the field is present and - end-to-end from settings through the route, not its value here. + Settings are INJECTED rather than read from the process env, for two + reasons: conftest enables control writes for the suite (so an + env-derived app would report `reachable` and could not show this), and + injection pins the value end-to-end from settings through the route, + which a vocabulary check could not distinguish from a hardcode. """ - with TestClient(create_app()) as client: + observe_only = Settings( + app_env="test", control_writes_enabled=False, compute_substrate="in_memory" + ) + with TestClient(create_app(settings=observe_only)) as client: + body = client.get("/readyz").json() + assert body["actuation"] == "inert" + + +@pytest.mark.contract +def test_readyz_reports_reachable_when_control_writes_are_enabled() -> None: + """The other direction, so a hardcoded `inert` cannot pass.""" + writable = Settings(app_env="test", control_writes_enabled=True, compute_substrate="in_memory") + with TestClient(create_app(settings=writable)) as client: body = client.get("/readyz").json() - assert body["actuation"] in ("inert", "reachable") + assert body["actuation"] == "reachable" @pytest.mark.contract