Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 8 additions & 2 deletions conformance/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,17 @@ pointers, so this file runs the real engine and validates what comes out of it.
pause, rejection and resubmission, recovery by supersession — plus inbound
external events that no job caused.
3. Validates every emitted payload against `event/v1`.
4. Asserts the eight `event/v1` guarantees that JSON Schema cannot express:
4. Checks that those approval payloads use `approval/v1`'s **field names** and not
names of ours. The schema leaves `additionalProperties` open, so a field we
invented validates in silence — the contract's own `properties` list is used as
the closed set the contract does not declare it to be. This is what would have
caught `supersedes_decision_id` still riding the wire after `approval/v1` v1.1.0
named the field `supersedes_approval_id`.
5. Asserts the eight `event/v1` guarantees that JSON Schema cannot express:
append-only, no silent state change, subject always answerable, `job_id` never
fabricated, unresolvable tenant rejected at intake, external source preserved,
no reasoning traces in an audit record, tenant partitions not mixed.
5. Checks that every entry in `known_gaps` still has an issue and an unexpired date.
6. Checks that every entry in `known_gaps` still has an issue and an unexpired date.

Nothing in the scenario is hand-written to please the schema. If a payload does not
conform, the fix is the code or an upstream issue — never the fixture.
Expand Down
82 changes: 75 additions & 7 deletions conformance/payload_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
trail ever passing through ``REJECTED`` — RFC-0011, which is what keeps
*"REQUIRE_CHANGES ไม่ใช่ REJECT"* true of the record and not only of the wording.

Their *names* are checked too, because ``approval/v1`` leaves
``additionalProperties`` open and so cannot reject a field we named ourselves.

Usage::

python3 conformance/payload_check.py # fetch schemas, then validate
Expand Down Expand Up @@ -292,7 +295,64 @@ def check_payloads(log, validator, gaps: list[dict]) -> None:
)


def check_decisions(log, jobs, validator) -> None:
def check_field_names(approvals: list[dict], approval_schema: dict) -> None:
"""Do our approval payloads spell the fields the way ``approval/v1`` spells them?

JSON Schema will not answer this. ``approval/v1`` does not set
``additionalProperties: false``, so a payload carrying an invented key — or a key
this repository named itself while waiting for the contract to grow one — passes
validation in silence. That silence is what let ``supersedes_decision_id`` ride the
wire for a day, and it would let the next one ride longer.

So the check is the contract's own property list, used as the closed set the
contract does not declare it to be. Anything outside it is ours, not theirs, and
has to be either renamed or registered as a gap.
"""
allowed = set(approval_schema.get("properties") or ())
strangers: dict[str, int] = {}
for payload in approvals:
for key in payload:
if key not in allowed:
strangers[key] = strangers.get(key, 0) + 1
if strangers:
fail(
"field-names",
"field ที่ไม่มีใน approval/v1 (schema ไม่ปิด additionalProperties จึงยัง valid): "
+ ", ".join(f"{k} ×{n}" for k, n in sorted(strangers.items())),
)
else:
ok(
"field-names",
f"{len(approvals)} approval ใช้ชื่อ field ของ approval/v1 ทั้งหมด "
f"— ไม่มี field ที่เราตั้งชื่อเอง",
)

# The positive half. The check above is satisfied by a payload that carries no
# citation at all, and "the link quietly stopped being sent" is the other way this
# rename could go wrong. approval/v1: "การเปลี่ยนใจคือ approval ใบใหม่ที่อ้างใบเดิม"
citing = [p for p in approvals if p.get("supersedes_approval_id")]
retired = [p for p in approvals if "supersedes_decision_id" in p]
if retired:
fail(
"field-names",
f"{len(retired)} approval ยังส่ง supersedes_decision_id — "
f"approval/v1 v1.1.0 ตั้งชื่อ field นี้ว่า supersedes_approval_id แล้ว",
)
elif citing:
ok(
"field-names",
f"{len(citing)} approval อ้างใบก่อนหน้าผ่าน supersedes_approval_id "
f"(approval/v1 v1.1.0)",
)
else:
fail(
"field-names",
"ไม่มี approval ใบไหนอ้างใบเดิมเลย — scenario มีการเปลี่ยนใจอยู่ "
"ถ้าไม่มี supersedes_approval_id แปลว่าลิงก์หายไประหว่างทาง",
)


def check_decisions(log, jobs, validator, approval_schema) -> None:
"""RFC-0002 — the decisions this engine produced, judged by ``approval/v1``.

Same rule as the events above: nothing is hand-written to please the schema.
Expand Down Expand Up @@ -325,6 +385,16 @@ def check_decisions(log, jobs, validator) -> None:
if real == 0:
ok("approval", f"{len(decisions)} decision ผ่าน approval/v1")

# Valid is not the same as correctly named — see check_field_names.
check_field_names(
[
approval
for payload in decisions
if (approval := (payload.get("metadata") or {}).get("approval")) is not None
],
approval_schema,
)

# "ทุก APPROVE ต้อง auditable — ต้องมี event GOVERNANCE_DECISION คู่กันเสมอ"
mismatched = [
job.job_id
Expand Down Expand Up @@ -628,17 +698,15 @@ def main() -> int:
"https://schemas.agent-platform.internal/event/v1/event.schema.yaml",
pinned["non_schema_keys"],
)
approval_validator = build_validator(
schemas,
"https://schemas.agent-platform.internal/approval/v1/approval.schema.yaml",
pinned["non_schema_keys"],
)
approval_id = "https://schemas.agent-platform.internal/approval/v1/approval.schema.yaml"
approval_validator = build_validator(schemas, approval_id, pinned["non_schema_keys"])
approval_schema = as_json_schema(schemas[approval_id], pinned["non_schema_keys"])

log, jobs, external = run_scenario()
print(f"\n[1] payload ที่ระบบผลิตจริง — {len(log)} event จาก {len(jobs)} job")
check_payloads(log, validator, pinned.get("known_gaps") or [])
print("\n[2] คำตัดสินที่ระบบผลิตจริง — approval/v1 (RFC-0002)")
check_decisions(log, jobs, approval_validator)
check_decisions(log, jobs, approval_validator, approval_schema)
print("\n[3] guarantee ที่ JSON Schema ตรวจไม่ได้")
check_guarantees(log, jobs, external)
print("\n[4] ช่องว่างที่รู้ตัว — ต้องมี issue และวันหมดอายุ")
Expand Down
9 changes: 8 additions & 1 deletion packages/core/devfactory_core/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,13 @@
The governance decision interface it is gated by is ``decision.py`` — RFC-0002.
"""

from .decision import Decision, DecisionType, Subject, new_decision_id
from .decision import (
WIRE_FIELD_NAMES,
Decision,
DecisionType,
Subject,
new_decision_id,
)
from .errors import (
CrossTenantDecision,
DecisionStateMismatch,
Expand Down Expand Up @@ -35,6 +41,7 @@
"TERMINAL",
"TRANSITIONS",
"INTERNAL_SOURCE",
"WIRE_FIELD_NAMES",
"CrossTenantDecision",
"Decision",
"DecisionStateMismatch",
Expand Down
81 changes: 59 additions & 22 deletions packages/core/devfactory_core/decision.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,23 @@
not validate: owning a copy of the schema here would be a parallel schema, which
Rule 4 forbids. It builds the payload; the contract judges it.

The names differ in two places, on purpose:

=========================== ==========================
here (semantics) ``approval/v1`` (wire)
=========================== ==========================
``Decision.decision_id`` ``approval_id``
``Decision.decision`` ``decision``
``Decision.subject`` ``subject`` — ``{type, id}``
=========================== ==========================
Where the two names differ it is always an *identifier*, and that is a rule rather
than a coincidence — see :data:`WIRE_FIELD_NAMES`:

==================================== ==========================
here (semantics) ``approval/v1`` (wire)
==================================== ==========================
``Decision.decision_id`` ``approval_id``
``Decision.supersedes_decision_id`` ``supersedes_approval_id``
``Decision.decision`` ``decision``
``Decision.subject`` ``subject`` — ``{type, id}``
==================================== ==========================

An approval *is* a decision to us and a record to them; keeping our own name and
mapping it at the boundary is what the authority split looks like in code.
mapping it at the boundary is what the authority split looks like in code. The
table above is prose for a reader; :data:`WIRE_FIELD_NAMES` is the same map as
data, and it — not this table — is what :meth:`Decision.as_payload` renders
through, so the next rename is one line rather than a search.

What a decision may **not** do is move a job somewhere the lifecycle does not
already go. The map from decision to destination lives in :mod:`.states`
Expand Down Expand Up @@ -75,6 +80,26 @@ class DecisionType(str, Enum):
SubjectTypeName = Literal["job", "execution", "tool_call", "artifact", "deployment"]


#: Our attribute name -> ``approval/v1``'s field name, for the fields where the two
#: differ. The single place the mapping is written as data; :meth:`Decision.as_payload`
#: is the only caller, so renaming a wire field is a one-line change here rather than a
#: hunt through the renderer.
#:
#: Both entries are identifiers, and that is the whole rule: **an id keeps our name
#: internally and takes theirs on the wire** — RFC-0005 Rule 1 gives ``agent-platform``
#: the field names and this repository the semantics. A decision is what we record; an
#: approval is what they receive; ``supersedes_decision_id`` therefore stays
#: ``supersedes_decision_id`` in Python for the same reason ``decision_id`` does, and
#: becomes ``supersedes_approval_id`` at the boundary for the same reason too. Their
#: name is also the more accurate one on their side of it — the value really is an
#: ``approval_id``, since ``decision`` there is an enum with no id to point at, and
#: ``decision`` already means the machine's verdict in ``policy/v1``.
WIRE_FIELD_NAMES: dict[str, str] = {
"decision_id": "approval_id",
"supersedes_decision_id": "supersedes_approval_id",
}


def new_decision_id() -> str:
"""A fresh decision id in the identity/v1 ``Id`` form.

Expand Down Expand Up @@ -112,7 +137,7 @@ class Decision:

"Immutable" is enforced here rather than documented: changing your mind is a
*new* decision that cites the one it replaces, which is what
``supersedes_decision_id`` carries.
``supersedes_decision_id`` carries — ``supersedes_approval_id`` on the wire.

RFC-0002's four required meanings map to ``decision``, ``reason``,
``authority`` and ``decided_at``. ``tenant_id`` is required by RFC-0006 and
Expand Down Expand Up @@ -140,16 +165,26 @@ class Decision:
#: refusing to move a job into execution under an expired one.
expires_at: datetime | None = None

#: The decision this one replaces.
#: The decision this one replaces — ``approval/v1`` ``supersedes_approval_id``.
#:
#: ``approval/v1`` states the guarantee — "การเปลี่ยนใจคือ approval ใบใหม่ที่
#: อ้างใบเดิม" — and the pinned schema (agent-platform @ 7263588) has no field
#: to put the citation in. We fill it on our side anyway: the alternative is
#: dropping a link the guarantee explicitly requires. ``approval/v1`` does not
#: set ``additionalProperties: false``, so the payload still validates today,
#: and if agent-platform names the field differently this becomes a rename
#: rather than a redesign. Recorded as a gap in ``platform-contract.yaml`` so
#: they see it without having to ask.
#: อ้างใบเดิม" — and for a while the schema had nowhere to put the citation, so
#: this repository invented ``supersedes_decision_id`` and registered the gap.
#: Since ``approval/v1`` v1.1.0 (agent-platform @ 3a01ab9) the field is theirs
#: and is called ``supersedes_approval_id``; the gap is closed and what remained
#: was the rename, as predicted.
#:
#: Only the wire name changed. The attribute keeps ours because the value is a
#: ``decision_id`` on this side of the boundary — see :data:`WIRE_FIELD_NAMES`
#: for why an id is spelled twice on purpose.
#:
#: Optional, and absent means *"claims to be the first"* rather than *"nobody
#: changed their mind"* — the schema is explicit about that reading. The
#: invariants it names but cannot check (the cited approval exists, in the same
#: tenant, about the same subject, never itself, never a cycle) are the
#: producer's. :meth:`devfactory_core.job.Job.decide` normally fills the citation
#: from the job's own decision list, which satisfies all five by construction; a
#: value passed in explicitly is not checked against them yet.
supersedes_decision_id: str | None = None

def __post_init__(self) -> None:
Expand Down Expand Up @@ -209,7 +244,7 @@ def as_payload(self) -> dict[str, Any]:
"""
payload: dict[str, Any] = {
# agent-platform's name for it (RFC-0005 Rule 1) — ours is decision_id.
"approval_id": self.decision_id,
WIRE_FIELD_NAMES["decision_id"]: self.decision_id,
"tenant_id": self.tenant_id,
"subject": self.subject.as_payload(),
"decision": self.decision.value,
Expand All @@ -222,8 +257,10 @@ def as_payload(self) -> dict[str, Any]:
if self.expires_at is not None:
payload["expires_at"] = self.expires_at.isoformat()
if self.supersedes_decision_id is not None:
# Our field, not theirs — see the field comment above.
payload["supersedes_decision_id"] = self.supersedes_decision_id
# Theirs since approval/v1 v1.1.0 — see WIRE_FIELD_NAMES.
payload[WIRE_FIELD_NAMES["supersedes_decision_id"]] = (
self.supersedes_decision_id
)
return payload

def __repr__(self) -> str:
Expand Down
9 changes: 6 additions & 3 deletions packages/core/devfactory_core/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -390,9 +390,12 @@ def decide(
decided_at=self._clock(),
expires_at=expires_at,
# "Changing your mind is a new approval that cites the old one"
# (approval/v1). The citation is filled from this job's own history
# rather than left to the caller to remember — it points at a
# decision that really was made, so nothing is being fabricated.
# (approval/v1 — the citation rides the wire as supersedes_approval_id).
# It is filled from this job's own history rather than left to the caller
# to remember: that makes it point at a decision that really was made,
# about this job, in this tenant, which is four of the five invariants
# approval/v1 asks the producer to hold up. The fifth — no cycles — comes
# free from a freshly minted decision_id.
supersedes_decision_id=(
supersedes_decision_id
if supersedes_decision_id is not None
Expand Down
4 changes: 3 additions & 1 deletion packages/core/state-machine.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ answers that question, and both the engine and `devfactory_observability.replay`
Guarantees the engine enforces, not just documents:

- decisions are immutable — changing one's mind is a second decision citing the first
(`supersedes_decision_id`)
(`Decision.supersedes_decision_id`, rendered as `approval/v1`'s
`supersedes_approval_id`; ids keep our name in Python and take theirs on the wire,
exactly as `decision_id` → `approval_id` does — see `decision.WIRE_FIELD_NAMES`)
- a decision lives in the same tenant and workspace as the job it decides about;
a mismatch is rejected, never coerced
- an agent may not `APPROVE` a job it is the principal for — *no agent has total
Expand Down
28 changes: 26 additions & 2 deletions packages/core/tests/test_decisions.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

from conftest import drive
from devfactory_core import (
WIRE_FIELD_NAMES,
Decision,
DecisionType,
Job,
Expand Down Expand Up @@ -531,6 +532,26 @@ def test_the_payload_uses_the_platforms_field_names(alice, reviewer, clock):
assert "decision_id" not in payload


def test_every_renamed_field_goes_out_under_the_platforms_name(alice, reviewer, clock):
"""The rename map is data, so it can be checked rather than trusted.

Both directions: our attribute must exist on the record, and only their name may
appear in the payload. A map entry that names an attribute nobody has, or a
payload that ships both spellings, fails here rather than on the wire.
"""
job = _at_the_gate(alice, clock)
first = job.reject(authority=reviewer, reason="no")
job.transition(JobState.DRAFT)
job.submit_for_governance()
record = job.approve(authority=reviewer, reason="yes")
payload = record.as_payload()

for ours, theirs in WIRE_FIELD_NAMES.items():
assert getattr(record, ours) is not None, f"nothing set for {ours}"
assert payload[theirs] == getattr(record, ours)
assert ours not in payload


def test_the_event_carries_the_approval_payload(alice, reviewer, clock):
job = _at_the_gate(alice, clock)
record = job.approve(authority=reviewer, reason="ok")
Expand All @@ -541,7 +562,8 @@ def test_the_event_carries_the_approval_payload(alice, reviewer, clock):
def test_unset_keys_are_omitted_rather_than_nulled(alice, reviewer, clock):
"""RFC-0008's rule against inventing a value holds for approvals too."""
payload = _at_the_gate(alice, clock).approve(authority=reviewer, reason="ok").as_payload()
assert "supersedes_decision_id" not in payload
assert "supersedes_approval_id" not in payload
assert "supersedes_decision_id" not in payload # nor under our internal name


def test_the_citation_appears_once_there_is_something_to_cite(alice, reviewer, clock):
Expand All @@ -550,7 +572,9 @@ def test_the_citation_appears_once_there_is_something_to_cite(alice, reviewer, c
job.transition(JobState.DRAFT)
job.submit_for_governance()
payload = job.approve(authority=reviewer, reason="yes").as_payload()
assert payload["supersedes_decision_id"] == first.decision_id
# approval/v1 v1.1.0's name for it — ours stays supersedes_decision_id.
assert payload["supersedes_approval_id"] == first.decision_id
assert "supersedes_decision_id" not in payload


def test_decision_ids_are_unique_and_well_formed(alice, reviewer, clock):
Expand Down
Loading
Loading