Let a facility verify observe-only, not just trust it - #579
Merged
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#571 made the observe-only pilot posture safe by default: a blank deployment observes every configured route and drives none. But "safe by default" was a claim, and the people who most need it, a beamline manager or a controls engineer deciding whether to let CORA near their IOCs, are exactly the people who will never hold API credentials. The posture was true but unverifiable.
/readyznow carries anactuationfield, eitherinertorreachable, and aboot.actuation_postureline prints it at startup. Anyone who can curl the pod can check the claim in one call.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, so it adds no new switch and no new failure mode. 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.
It folds in BOTH ways CORA can reach a beamline, because "read-only" means both must be shut:
CONTROL_WRITES_ENABLED), andcaput ....Only
compute_substrate == "in_memory"is provably unable to spawn, so the compute test is an allowlist, not a denylist: a futureglobusorslurmsubstrate readsreachableby default rather than slipping through the day its literal lands. When uncertain it returnsreachable, inverting the getattr-None fail-open idiom used elsewhere, because a falsereachableis an alarm someone investigates while a falseinertis a facility discovering CORA moved their stage after being told it could not.Two inputs are deliberately not read, each documented at the derivation:
cora_allow_raw_conduct(per-request, and inert underin_memoryanyway; logged but excluded) andActuationKind(it tracks simulator-vs-real, not writable-vs-not, so it cannot witness this posture).Scope
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
actuationrather than widening it. Two fitness tests hold the line: one asserts the three actuation defaults ship safe (reading declared field defaults, immune to the test env), the other fails the build if any dormant outbound adapter is constructed at the composition root without the derivation being revisited.Also corrects three stale docstrings the audit surfaced (the transfer port and its in-memory adapter both claimed no real adapter exists; config's ComputeSubstrate comment claimed it mirrors the operation tier when it is deliberately narrower).
Gate review
Three lenses plus adversarial verification (15 agents). All three approve-with-nits. The safety reviewer tried to make the field lie and could not; the correctness reviewer ran the complete reachable 2x2 of (writes x substrate) by execution and confirmed no deployment actuates while reporting
inert, with the globus path closed three independent ways.The second commit answers the findings. The tripwire was weaker than its own docstring: an aliased import and an attribute call both walked past the bare-Name AST scan, its targets were bare strings that a rename would silently disarm, and it collected an empty parameter set indistinguishable from a clean tree. All three are fixed and mutation-verified. The contract test asserted only vocabulary membership, which a hardcode could pass; it now injects Settings and pins both directions. One nit was declined with reasoning in the commit body.
Verification: 45,606 passed, pyright clean, every pre-commit hook green.
🤖 Generated with Claude Code