Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
---
title: Concern — notes/domain-validation.md directs helpers to a module that cannot import core.states
type: learning
timestamp: 2026-08-12
source: ISSUE-2232
signal: concern
---

`notes/domain-validation.md` says canonical layer-neutral helpers belong in
`vultron/core/models/_helpers.py`. That guidance is **unfollowable for any
helper that needs a state enum**, and the failure mode is a confusing
`ImportError` rather than a clear rejection.

`vultron/core/models/base.py` imports `_helpers` (for `_new_urn`/`_now_utc`), so
`_helpers` is loaded *during* `models.base` initialisation. Importing
`vultron.core.states.rm` from `_helpers` first executes
`vultron/core/states/__init__.py`, which pulls `states/cs.py` →
`states/common.py` → back into `models.base` — still partially initialised:

```text
ImportError: cannot import name 'NonEmptyString' from partially initialized
module 'vultron.core.models.base' (most likely due to a circular import)
```

The trap is that `states/rm.py`'s *own* imports are clean (logging, enum,
transitions, `states.common`). Inspecting the target module tells you nothing;
the cycle runs through the package `__init__`. So `_helpers.py` is usable only
for helpers that depend on nothing outside stdlib and `TYPE_CHECKING`.

`participant_status_rm_state` was instead placed in
`vultron/core/models/participant_status.py`, next to the model it reads. That
is arguably the better home regardless — the canonical reader for a type lives
with the type — but it was reached by hitting the wall, not by design.

**Suggested fix:** amend `notes/domain-validation.md` to state the constraint
explicitly, and to direct type-specific canonical readers to the type's own
module. A shared BT-node wrapper (`read_rm_states`) went into
`vultron/core/behaviors/helpers.py`, which has no such restriction.

Related: #2269 (that placement was also forced by the `append.py` line ceiling).
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
title: Spec gap — DL-05 covers DataLayer read shape but nothing governs the write path
type: learning
timestamp: 2026-08-12
source: ISSUE-2232
signal: spec-gap
---

DL-05-001..004 and ADR-0034 specify that the DataLayer **port returns core
objects**, and `test/architecture/test_dl_read_returns_core_objects.py`
enforces it with a shrink-only `KNOWN_WIRE_ESCAPES` ratchet. There is no
corresponding requirement on the **write** side: nothing forbids storing a
wire-shaped payload in a core-typed row.

That asymmetry is what made #2232 possible. The only write-path shape guard
was `if obj.type_.startswith("as_")` in `Record.from_obj` — but wire
vocabulary `type_` values are *bare* names (`"CaseParticipant"`), not
`as_`-prefixed, so the guard never fires for the 15 wire classes whose `type_`
is also a `CORE_VOCABULARY` key. A wire `as_ParticipantStatus` (flat
`rm_state`) was written into the `ParticipantStatus` table, and core readers
doing `status.rm.state` got `None`.

Enforcing "reads return core" without enforcing "writes store core" only
guarantees the *type* of the object handed back, not that the row's field
shape matches the class reading it. A read-side ratchet cannot detect a
malformed row; it can only confirm the class it instantiated.

**Suggested spec addition** (companion to DL-05-001..004): a DataLayer write
MUST store the canonical core field shape for any `type_` present in
`CORE_VOCABULARY`, with a shrink-only exemption set mirroring
`KNOWN_WIRE_ESCAPES`. #2232 implemented this for `CaseParticipant` and
`ParticipantStatus` (`_NORMALIZE_WIRE_TO_CORE` in
`vultron/adapters/driven/db_record.py`); the remaining 13 types are tracked
in #2268, which is where the architecture test asserting set completeness
belongs.
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
---
title: Tooling — devlog accumulation and a hardcoded /app default make the mandated clean-base proof actively hazardous
type: learning
timestamp: 2026-08-12
source: ISSUE-2232
signal: tooling-issue
---

`completeness-doctrine.md` requires proving a test failure pre-existing on a
clean `origin/main` before classifying it as not branch-owned. Two properties
of the demo/invariant harness fight that requirement. Both are filed as #2273.

**1. `DEVLOGS_DIR` defaults to a hardcoded absolute path.**
`vultron/demo/scenario/fv_demo.py:898` reads
`os.environ.get("DEVLOGS_DIR", "/app/devlogs")`. A demo run from any other
checkout — a git worktree, a second clone, CI — writes into `/app/devlogs`
rather than its own tree. My first attempt at the clean-base proof produced no
devlogs in the worktree at all and instead **overwrote the artifacts of the
branch I was comparing against**. The tool for establishing a clean baseline
destroys the baseline. `vultron/demo/report.py:66` already resolves this
repo-root-relative, so the fix pattern exists in-tree.

**2. Devlogs accumulate across runs and corrupt hash-chain assertions.**
`devlogs/` is gitignored and demo runs append rather than replace, so three
runs left three case-ledger files per actor. `load_devlogs()` concatenates
every `*-case-ledger.jsonl` for an actor, chaining entries from unrelated
cases, and `test_invariant_1_local_hash_chain_consistent` then reports a
mismatch at every logIndex for every actor.

That second one is the dangerous one: three actors failing hash-chain
continuity at every index reads as a serious integrity regression. It cost real
time to recognise as an artifact of leftover files. The invariant suite should
either scope per `case_id` or require a clean `devlogs/` directory and say so
when it isn't.

**Bearing on this session:** after clearing `devlogs/` and regenerating, three
of the four failures vanished. The fourth,
`test_invariant_5_expected_event_types_present[validate_report]`, reproduced
byte-identically on a clean `origin/main` worktree and is genuinely
pre-existing — see #2273 and the cross-reference on #2266.
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
title: Tooling — graphify query output truncates on this graph size, defeating the mandatory-graph-first rule
type: learning
timestamp: 2026-08-12
source: ISSUE-2232
signal: tooling-issue
---

The repo hook makes `graphify query` mandatory before reading source files. On
this graph (3171 nodes) the query output exceeded the ~2000-token tool budget
and was truncated mid-result, so the surfaced subgraph could not be read as a
whole. The usable fallback was to take the high-value node names that *did*
appear (`notes/domain-validation.md`, ADR-0034) and read those files directly.

This is worth recording because the failure is silent-ish: truncated output
still looks like an answer, so an agent can proceed on a partial map and
believe it was oriented. For #2232 the consequential facts — that 15 wire
`type_` values shadow `CORE_VOCABULARY`, and that ~63 files both import
`as_CaseParticipant`/`as_ParticipantStatus` and call `.save(`/`.create(` — came
from direct `grep` counts, not from the graph. The blast-radius measurement is
what drove the design away from reject-all toward normalise, so the graph was
not sufficient for the decision that mattered.

**Suggestion:** have `graphify query` degrade to a compact node-name-only
listing when the full subgraph would exceed the budget, rather than truncating
a verbose rendering. Until then, treat graph output as orientation only and
verify counts with a direct search.
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
---
title: Tooling — a pytest run killed by the 5s per-test timeout is indistinguishable from a passing run under the mandated command
type: learning
timestamp: 2026-08-12
source: ISSUE-2232
signal: tooling-issue
---

`pyproject.toml` sets `timeout = 5` with `timeout_method = "thread"`
(pytest-timeout). When a test exceeds that budget the plugin dumps a stack
trace and **kills the process** — the run stops partway, having executed maybe
12% of the suite.

The ONE RUN RULE mandates `uv run pytest --tb=short 2>&1 | tail -5`. That
command cannot show this happening, for two compounding reasons:

1. The pipeline's exit status is `tail`'s, not pytest's, so a killed run still
reports `0`.
2. The faulthandler stack dump is long and lands at the end of the combined
stream, so `tail -5` shows dump frames where the
`N passed, M skipped in Xs` summary line would normally be.

The result is a validation cycle that produced no summary line and exited `0`.
Both of my first two unit-suite runs on this branch were killed this way; I
only noticed because the last visible progress marker read `[ 10%]`. Redirecting
to a file and checking pytest's own exit code showed `UNIT_EXIT=1` and the kill
inside `test/metadata/specs/test_real_specs.py::test_real_specs_lint_no_hard_errors`.

The test itself is fine in isolation — 3.05s against the 5s budget, 61% of it —
which is exactly the problem: it is close enough to the ceiling that ambient
load decides the outcome, and the whole suite dies with it. A third run passed
cleanly (6592 passed in 115s), confirming a load-sensitive flake rather than a
branch-owned failure.

**Suggestions:**

- Have the run-tests skill capture full output and assert on pytest's own exit
code, e.g. `uv run pytest --tb=short > /tmp/unit.log 2>&1; echo $?` followed
by a grep for the summary line — so an absent summary is loud rather than
silent. `tail -5` on a pipe is not a safe success signal.
- Either raise the global `timeout` or mark the spec-lint tests with a larger
per-test budget. A single slow test taking the entire suite down is a
disproportionate failure mode, and 61%-of-budget is not headroom.
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
---
title: Design choice — normalise wire→core at the persistence boundary, not at wire ingress
type: learning
timestamp: 2026-08-12
source: ISSUE-2232
signal: design-question
---

Issue #2232 prescribed: *"Normalize at the boundary (wire -> core on ingress) so no
wire-shaped row is ever persisted."* The fix normalises at the **persistence**
boundary (`Record.from_obj`) rather than at **wire ingress**, and scopes it to
2 of the 15 shadowing types. Both departures were deliberate.

**Why persistence rather than ingress.** Wire ingress is not a single chokepoint
— wire objects are constructed in-process by BT nodes, trigger factories, and
test fixtures, not only parsed from inbound AS2. Measured during analysis:
~63 files import `as_CaseParticipant`/`as_ParticipantStatus` *and* call
`.save(`/`.create(`. `Record.from_obj` is the one place every persisted object
passes through, so a guard there is total; a guard at ingress would have been
partial while looking complete. The issue's "Done when" clause — *"a
wire-shaped ParticipantStatus cannot be persisted"* — is a statement about
persistence, and that is where it is now enforced.

**Why normalise rather than reject.** The first attempt rejected all 15
shadowing types outright. That is the stronger invariant, but it turned ~63
test files into `size:L` churn unrelated to the defect. Normalising via the
existing `to_core()` projections satisfies both "Done when" clauses (no
wire-shaped row is stored; a shape mismatch raises) without that churn.

**Why 2 types and not 15.** `CaseParticipant` and `ParticipantStatus` differ
*structurally* between the two shapes — core nests `rm: RmDimension`, wire
carries a flat `rm_state` — so a wire row silently yields `None`. The other 13
currently differ only by key spelling, which is a coincidence rather than an
invariant. `_NORMALIZE_WIRE_TO_CORE` is documented as shrink-only (may grow,
never shrink) and the remaining 13 are tracked in #2268.

**Cost paid for the narrow scope:** the write path now has a *second*
exemption set to keep honest, alongside read-side `KNOWN_WIRE_ESCAPES`. Two
ratchets for one underlying problem is a smell; #2268 records the structural
alternative (disjoint `type_` namespaces between the two vocabularies), which
would delete both.
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
---
title: Process — a test asserted the ARCH-15 violation as intended behaviour
type: learning
timestamp: 2026-08-12
source: ISSUE-2232
signal: process-issue
---

`test_resolve_participant_state_defaults_when_invalid_rm_type` asserted
`rm == RM.START` for a status whose `rm.state` was the string `"not-an-rm"`,
under the docstring *"Falls back to RM.START when rm_state is not an RM enum
value."* That is precisely the defect #2264 describes: substituting `RM.START`
for an unreadable status silently resets the participant's RM ladder to its
initial state, after which the next legitimate transition is rejected as
backwards.

The test did not merely fail to catch the bug — it **locked it in**. Fixing
the code turned a green test red, so the regression suite argued for the
defect. ARCH-15-001..004 ("Silent `None` Returns and Fake `SUCCESS` Are the
Same Bug") already forbade the behaviour when the test was written.

Two things made it look reasonable:

1. The word *"defaults"* in the test name conflates **absence** with
**unreadability**. `RM.START` is the right answer for a participant with no
recorded status; it is never the right answer for a status that exists but
cannot be read. The sibling
`test_resolve_participant_state_defaults_when_no_statuses` covers the
legitimate case and still passes unchanged.
2. Asserting the observed behaviour of a defensive `isinstance` fallback feels
like coverage. It is really a snapshot of an unreviewed default.

**Signal to watch for in review:** a test whose docstring says "falls back
to", "defaults to", or "tolerates" for *malformed* input, rather than for
*absent* input. Ask which of the two the fallback is actually serving; if it
is malformed input, ARCH-15 requires a raise or `Status.FAILURE` and the test
is asserting a bug.

Note also that `test_resolve_participant_state_defaults_when_invalid_vfd_type`
was left passing on purpose: the vfd dimension remains lenient because RM is
read first, so a wire-shaped status raises before vfd is reached.
83 changes: 83 additions & 0 deletions test/adapters/driven/test_db_record.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,3 +237,86 @@ def test_object_to_record_nested_report_not_duplicated_in_offer_data():
serialised = json.dumps(record.data_)
assert report.content is not None
assert report.content not in serialised


# ---------------------------------------------------------------------------
# Wire/core shape guard on the write path (issue #2232)
# ---------------------------------------------------------------------------


def test_object_to_record_normalizes_wire_class_shadowing_a_core_type():
"""A wire vocab class whose ``type_`` has a core counterpart is normalised.

Regression for #2232: the only shape guard was ``type_.startswith("as_")``,
but wire vocabulary ``type_`` values are bare ("CaseParticipant"), so a
wire-shaped object was happily written into a core-typed DataLayer row.
Core readers then saw a flat ``rm_state`` where they expected a nested
``rm`` dimension.

The row must now carry the canonical core shape — nested
``rm: {"state": ...}`` — so no wire-shaped ``CaseParticipant`` row exists to
be misread.
"""
from vultron.core.models.registry import CORE_VOCABULARY
from vultron.wire.as2.vocab.objects.case_participant import (
as_CaseParticipant,
)

wire_participant = as_CaseParticipant(
attributed_to="https://example.org/actors/vendor",
context="https://example.org/cases/case-2232",
)
# The pre-existing guard cannot catch this: type_ is bare, not "as_"-prefixed.
assert not str(wire_participant.type_).startswith("as_")
assert str(wire_participant.type_) in CORE_VOCABULARY

record = object_to_record(cast(Any, wire_participant))

assert record.type_ == "CaseParticipant"
statuses = record.data_["participant_statuses"]
assert statuses, "normalised participant must retain its RM ladder"
for status in statuses:
# Canonical core shape: nested rm dimension, no flat rm_state.
assert "rm_state" not in status
assert status["rm"]["state"] == "START"


def test_object_to_record_normalizes_wire_participant_status():
"""A wire ``ParticipantStatus`` persists in the nested core ``rm`` shape."""
from vultron.core.states.rm import RM
from vultron.wire.as2.vocab.objects.case_status import (
as_ParticipantStatus,
)

wire_status = as_ParticipantStatus(
rm_state=RM.VALID,
context="https://example.org/cases/case-2232",
attributed_to="https://example.org/actors/vendor",
)

record = object_to_record(cast(Any, wire_status))

assert record.type_ == "ParticipantStatus"
assert "rm_state" not in record.data_
assert record.data_["rm"]["state"] == "VALID"


def test_object_to_record_still_accepts_wire_activities():
"""Activities have no core counterpart, so they must remain persistable."""
from vultron.wire.as2.vocab.objects.vulnerability_report import (
as_VulnerabilityReport,
)

report = as_VulnerabilityReport(
name="CVE-2232",
content="details",
attributed_to="https://example.org/finder",
)
offer = rm_submit_report_activity(
report,
"https://example.org/finder",
actor="https://example.org/finder",
)

record = object_to_record(offer)
assert record.type_ == "Offer"
Loading
Loading