Audit the tach contract, and pin every port's flattened status axis to its owning enum - #800
Merged
Merged
Conversation
…re than it checks An audit of `tach.toml` after four months. Two findings acted on here; the third is a design question left open on purpose. IT ONLY EVER GREW. 15 declared edges at introduction, 161 today, across 46 commits. Exactly one commit ever removed an edge (b6c8e0a), and that was a hand-written refactor deleting a documented "Pattern C exception" by making the code stop needing it. Nothing in the process noticed the other 145. Ten of those edges were taken up by no source file at all. Five clustered on `cora.equipment.ports`, which is a docstring and an empty `__all__`: its one export was hoisted to `cora.shared.ports` when the rule-of-three fired, and the module declaration outlived the contents, still carrying three outbound deps with two other modules declaring edges into it. The rest were three unused `cora.shared` entries and two parent-to-own-adapters edges nothing imports. All ten are removed, and `test_tach_edges_are_used` now fails on an entry no tracked source file takes up, so the next one is caught in the commit that strands it rather than four months later. The test enumerates git-TRACKED files, and that is load-bearing rather than stylistic: an hour before it was written, the architecture suite reported 51,665 passing with `EXPECTED_SCHEMA_VERSION` stale, because the pin's own guard reads tracked files and the new migration had not been staged. A check that cannot see a file passes by not looking. Attribution is most-specific-wins, so a module declaring both `cora.run` and `cora.run.aggregates` is not credited for the broad entry by an import the narrow one already covers. That case is pinned by mutation, along with the plain one. THE HEADER CLAIMED MORE THAN THE FILE CHECKS. It said BCs depend on "any sibling aggregate kernels they integrate with", which reads as a map of what depends on what. It is a record of which module may NAME which, and naming is one of several ways a dependency arises: - `cora.operation` reads `EnclosureLookup` and cannot start a Procedure without Enclosure's permit data, yet declares no edge to `cora.enclosure`. The Protocol sits in `cora.infrastructure.ports`, so both sides name only `cora.infrastructure`, which every module does. Twenty lookup adapters inside BCs bind to a protocol in that shared namespace, so this is a category and not one case. - An event subscription names its producer with a string. - `tests` is excluded outright. None of that is a defect. The port indirection is deliberate and buys real isolation. The defect was one sentence inviting the file to be read as something it is not, which it was, by me, an hour before the audit. The header now says what it constrains and lists what it does not, and names the open question rather than answering it: whether those ports should move into their owning BC, trading the isolation for a dependency graph that matches reality. That needs the other nineteen measured, not the one anecdote. Verified by mutation, both directions: adding a permission nothing imports fails the test, and so does a broad edge shadowed by a narrower one already covering every import. `tach check` itself caught me over-deleting during the prune, dropping `cora.infrastructure` from `cora.equipment.adapters` where the audit had only flagged `cora.shared`; restored before it went anywhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A cross-BC lookup port cannot import the owning BC's StrEnum, because the ports package holds depends_on = [] and that neutrality is what lets several BCs answer the same question. So a status crossed the port surface as a bare str, and consumers in other BCs partitioned on it by writing the value out by hand. Ten such comparisons exist today. All ten happen to fail closed, so a drifted value stops work loudly rather than admitting it. That is the polarity they were written in, not a property anything enforced: the first gate written as `if status == "Revoked": raise` would fail open, silently. Typing the field as a Literal alias keeps the port neutral (Literal is stdlib, not a BC type) while pinning the exact value set, so a comparison against a value the enum lacks is a type error at the call site. Adapters produce a validated member's .value, which narrows to exactly the alias, so the existing runtime validation is preserved and no cast is needed. The alias is a hand-written mirror, so one fitness test pins it to the enum and a second enumerates aliases from source, failing on any that nothing registers. Verified by mutation: drifting the alias, adding an unregistered alias, and comparing against an outside value each fail. A weaker check asking only whether a literal belongs to SOME StrEnum was rejected: "Active" is a member of 14 enums here, so it would admit a ClearanceStatus value at an enclosure gate. It would also have passed the permit_status="Denied" fixture this pin caught on its first application, since "Denied" is a real RatificationStatus value. That fixture described a state the adapter can never produce and is now "NotPermitted". This is the first of 13 flattened status fields; the pattern here is the template for the rest.
…enums Commit 87797f830cb did this for the enclosure port's two status axes. The same gap existed on twelve more fields across asset, assembly, family, capability, language_model, dataset_distribution, clearance, clearance_template, credential, facility, and supply lookups: each crossed its port as a bare str because cora.infrastructure.ports holds depends_on = [] and cannot import the owning BC's StrEnum, so a consumer in another BC partitioned on the value by writing it out by hand with nothing pinning the set it could take. Retype each field to a Literal alias that mirrors its owning enum's full value set (never a query-filtered subset), matching the enclosure precedent: Literal is stdlib, so the alias keeps the port neutral while turning a comparison against a value the enum lacks into a type error at the call site. The existing test_port_status_literals_match_owning_enums fitness test's REGISTRY grows from two entries to fourteen; its companion test keeps every declared alias in that registry. Retyping surfaced one test fixture that could not type-check under the new alias: test_record_witnessed_run_decider_properties.py generated clearance_status via st.text(...).filter(lambda s: s != "Active"), which samples arbitrary strings outside the enum's value set. Replaced with st.sampled_from() over the enum's non-Active members, preserving the test's intent while satisfying the narrower type.
…ught The registry check asked whether every declared `<Name>Value` alias is pinned to a StrEnum, which is blind to a field that never got an alias declared for it in the first place. That blindness is not hypothetical: it is exactly how `FacilityLookupResult.kind` and all three fields on `PermitLookupResult` (direction, status, abi_tier_floor) were missed when the other lookup ports were converted from bare `str` in the two prior commits. The new test walks every port class's fields directly via AST instead of trusting the alias set to be complete, so a field that skips the alias step entirely can no longer hide. Both misses are now pinned: `FacilityKindValue = Literal["Site", "Area"]` against `FacilityKind`, and `DirectionValue` / `PermitStatusValue` / `AbiTierValue` against `Direction` / `PermitStatus` / `AbiTier`. A real exception (Supply.kind, which has no owning StrEnum yet) is recorded in `UNPINNED_AXIS_FIELDS` with its reason, so an absence of a pin is now a written claim rather than a silent gap. Mutation-verified: reverting a pinned field to bare str, drifting FacilityKindValue's value set from FacilityKind's, and removing the recorded exception are each caught. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
xmap
enabled auto-merge
September 11, 2026 19:31
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.
Split out of `port-procedure-hold-claims`, which mixed this audit with an
unrelated Procedure hold-claim thread. This half stands on its own; the
other lands separately.
What the audit found
A tach.toml stress-test surfaced 16 real cross-BC dependencies with zero
declared edges (e.g. `run -> enclosure`, `run -> supply`), all going
through Protocol interfaces that live in the shared `cora.infrastructure.ports`
namespace rather than in either BC. Two shapes were on the table: move each
port into its owning BC so the edge becomes visible to tach ("B", tight), or
leave tach as a pure naming firewall and treat the coupling as a separate,
deliberately un-enforced map ("D", loose).
D is what ships here. A lookup port's consumer depends on a stable
question ("is this enclosure permitted?"); the implementation answering
it is swappable wiring. Merging the two into one file, as B would, means a
wiring change (swap who implements a port) forces a change to the
consumer's declared dependencies even though nothing in the consumer moved.
`BeamAvailabilityLookup`'s own docstring already committed to this
separation on purpose.
So this PR:
on any `depends_on` entry no tracked source file actually takes up.
Caught 10 dead permissions immediately (161 -> 151 edges), including a
whole module, `cora.equipment.ports`, that was declared and empty.
Mutation-verified: a broad `cora.run` declared redundantly alongside
`cora.run.aggregates` is correctly flagged.
not the true dependency graph) instead of a sentence that invited
reading it as a complete map.
ports flatten a typed `StrEnum` status/kind/direction field to a bare
`str`, so 10+ consumers in other BCs re-type the value by hand against
string literals nothing enforces. Safe today only by polarity accident
(every comparison happens to be "match means proceed"), not by
construction. Every remaining one is now a `Literal` alias pinned to
its owning enum via a fitness test, extended twice: once to register
every declared alias against its StrEnum, and once more to range over
port fields directly (not just the aliases that happen to exist),
which is what caught the two the first pass missed
(`FacilityLookupResult.kind`, all three fields on
`PermitLookupResult`). Mutation-verified at each step.
Verification
value set, removed recorded exception - each caught)
🤖 Generated with Claude Code