diff --git a/conformance/README.md b/conformance/README.md index e0c224d..82b75f6 100644 --- a/conformance/README.md +++ b/conformance/README.md @@ -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. diff --git a/conformance/payload_check.py b/conformance/payload_check.py index ca5015d..1a3f648 100644 --- a/conformance/payload_check.py +++ b/conformance/payload_check.py @@ -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 @@ -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. @@ -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 @@ -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 และวันหมดอายุ") diff --git a/packages/core/devfactory_core/__init__.py b/packages/core/devfactory_core/__init__.py index 93409a8..27cfdf8 100644 --- a/packages/core/devfactory_core/__init__.py +++ b/packages/core/devfactory_core/__init__.py @@ -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, @@ -35,6 +41,7 @@ "TERMINAL", "TRANSITIONS", "INTERNAL_SOURCE", + "WIRE_FIELD_NAMES", "CrossTenantDecision", "Decision", "DecisionStateMismatch", diff --git a/packages/core/devfactory_core/decision.py b/packages/core/devfactory_core/decision.py index d4f83bf..d2883f1 100644 --- a/packages/core/devfactory_core/decision.py +++ b/packages/core/devfactory_core/decision.py @@ -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` @@ -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. @@ -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 @@ -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: @@ -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, @@ -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: diff --git a/packages/core/devfactory_core/job.py b/packages/core/devfactory_core/job.py index 37a8866..ca611f6 100644 --- a/packages/core/devfactory_core/job.py +++ b/packages/core/devfactory_core/job.py @@ -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 diff --git a/packages/core/state-machine.md b/packages/core/state-machine.md index 94daa68..9897ef4 100644 --- a/packages/core/state-machine.md +++ b/packages/core/state-machine.md @@ -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 diff --git a/packages/core/tests/test_decisions.py b/packages/core/tests/test_decisions.py index 3be1ffc..334651e 100644 --- a/packages/core/tests/test_decisions.py +++ b/packages/core/tests/test_decisions.py @@ -13,6 +13,7 @@ from conftest import drive from devfactory_core import ( + WIRE_FIELD_NAMES, Decision, DecisionType, Job, @@ -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") @@ -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): @@ -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): diff --git a/platform-contract.yaml b/platform-contract.yaml index 683dc15..754b12e 100644 --- a/platform-contract.yaml +++ b/platform-contract.yaml @@ -19,15 +19,17 @@ contracts: conformance: status: passing - last_verified: 2026-08-18 + last_verified: 2026-08-19 # รันกับ pin 3a01ab9 (approval/v1 v1.1.0) — passed=20 FAIL=0 # ADR-0006 บังคับ consumer 3 ข้อ — ครบแล้วทั้งสาม: # 1. manifest ไฟล์นี้ # 2. conformance test ใน CI ที่ validate payload จริง # conformance/payload_check.py — รัน scenario ผ่าน job state - # machine และ event log จริง แล้ว validate event 42 ตัว - # กับ event/v1 ที่ pin ไว้ และ decision 5 ใบกับ approval/v1 + # machine และ event log จริง แล้ว validate event 56 ตัว + # กับ event/v1 ที่ pin ไว้ และ decision 8 ใบกับ approval/v1 # · ไม่มี fixture ที่เขียนขึ้นเพื่อให้ schema ผ่าน + # · ตรวจ "ชื่อ" field ของ approval ด้วย เพราะ approval/v1 ไม่ได้ปิด + # additionalProperties — field ที่เราตั้งชื่อเองจึง valid เงียบ ๆ # 3. release gate job `core` และ `conformance` ใน .github/workflows/test.yml # รันทุก PR # @@ -39,7 +41,7 @@ conformance: # ตอบได้เสมอ · ไม่ปลอม job_id · reject tenant ที่ resolve ไม่ได้ · คง source # ของ external · ไม่เก็บ reasoning · tenant partition ไม่ปนกัน -pinned_contracts_commit: 72635883c4ce0e8fa8ed2fd64c02cbc4d5616087 # conformance/pinned.yaml +pinned_contracts_commit: 3a01ab9d0a68594463382b0ec618dc07ccf6408c # conformance/pinned.yaml # ช่องว่างจาก consumer pilot ของ agent-platform และ RFC ที่ปิดแต่ละข้อ # รายละเอียดเต็ม: agent-platform/architecture/consumer-devfactory-core.md @@ -95,11 +97,19 @@ remaining: - เปิด branch protection แล้วตั้ง check ของ workflow test เป็น required เพื่อให้ release gate บล็อกการ merge ได้จริง (ADR-0006 ข้อ 3) - end-to-end simulation (issue #7) + - Job.decide ยังรับ supersedes_decision_id ที่ caller ส่งมาเองโดยไม่ตรวจ 4 ข้อที่ + approval/v1 v1.1.0 ฝากไว้กับผู้ผลิต (ใบที่อ้างต้องมีจริง · tenant เดียวกัน · + subject เดียวกัน · ห้ามชี้ตัวเอง) — ทางเดิน default ทำครบอยู่แล้ว จึงยังไม่มี payload + ที่ผิด แต่ประตูนี้เปิดอยู่ registration: conforming # registered | conforming # ช่องว่างที่ค้างอยู่ฝั่ง contract ไม่ใช่ฝั่ง implementation # ตามแบบที่ care-agent-platform ทำ — บันทึกไว้ให้ platform เห็นโดยไม่ต้องมาถาม +# +# ข้อที่ปิดแล้วไม่ลบทิ้ง แต่ติด `status: resolved` ไว้แบบเดียวกับ blocking: ข้างบน +# — เหตุผลที่เราเคยเบี่ยงจาก contract คือสิ่งที่คนอ่านโค้ดเก่าจะถามถึง +# อ่านเฉพาะข้อที่ไม่มี status: resolved ถ้าอยากรู้ว่าตอนนี้ยังค้างอะไร gaps: - id: event-type-enum-closed issue: https://github.com/monthop-gmail/agent-platform/issues/17 @@ -119,9 +129,20 @@ gaps: approval/v1 มี guarantee ว่า "decision เป็น immutable · การเปลี่ยนใจคือ approval ใบใหม่ที่อ้างใบเดิม" แต่ schema ไม่มี field ให้ใส่การอ้างนั้นเลย — ทำตาม guarantee แล้วไม่มีที่เก็บผลลัพธ์ - workaround: >- - ฝั่งเราใส่ field ชื่อ supersedes_decision_id ลงใน payload เอง - (Decision.as_payload · packages/core/devfactory_core/decision.py) - approval/v1 ไม่ได้ตั้ง additionalProperties: false จึงยัง validate ผ่าน - ถ้า agent-platform ตั้งชื่อ field นี้เองภายหลัง งานที่เหลือคือ rename ไม่ใช่ออกแบบใหม่ severity: low + resolved_by: agent-platform#24 — approval/v1 v1.1.0 · field ชื่อ supersedes_approval_id + status: resolved # ปิดแล้ว 2026-08-19 · pin ขยับเป็น 3a01ab9 ใน PR #23 + note: >- + ระหว่างที่ยังไม่มี field ฝั่งเราใส่ชื่อของตัวเองคือ supersedes_decision_id ลงใน payload + (approval/v1 ไม่ได้ตั้ง additionalProperties: false จึงยัง validate ผ่าน) และเขียนไว้ว่า + ถ้า agent-platform ตั้งชื่อเอง งานที่เหลือคือ rename ไม่ใช่ออกแบบใหม่ — ผลออกมาตามนั้น + · เขาไม่ใช้ชื่อของเราเพราะค่าที่ใส่คือ approval_id จริง ๆ (decision เป็น enum ไม่มี id) + และคำว่า decision ชนกับผลประเมินของเครื่องใน policy/v1 + · payload ฝั่งเราใช้ supersedes_approval_id แล้ว ส่วนชื่อใน Python ยังเป็น + supersedes_decision_id ตาม convention เดิม decision_id → approval_id (RFC-0005 Rule 1) + map อยู่ที่เดียวคือ decision.WIRE_FIELD_NAMES + · payload_check ตรวจชื่อ field ที่เราผลิตแล้ว — schema ยังไม่ปิด additionalProperties + ชื่อผิดจึงยัง validate ผ่านโดยไม่มีใครทัก + · invariant ที่ v1.1.0 ฝากไว้กับผู้ผลิต (ใบที่อ้างต้องมีจริง · tenant เดียวกัน · + subject เดียวกัน · ห้ามชี้ตัวเอง · ห้ามเป็นวง) ทางเดินปกติของ Job.decide ทำครบ + แต่ถ้า caller ส่ง supersedes_decision_id เข้ามาเองยังไม่มีใครตรวจ — ดู remaining: