diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 8bf4f9f..89df50d 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -29,6 +29,11 @@ it caused, so no job reaches `APPROVED` without a record of who decided it and w says which state it sends a job to. See [RFC-0002](rfcs/0002-governance-decision-contract.md) and `packages/core/devfactory_core/decision.py`. +An approval can also carry `expires_at`, and one that has passed it authorises nothing: +the engine refuses to move the job into execution, and a job left holding a lapsed +approval reaches `TIMED_OUT` rather than waiting indefinitely +([RFC-0007 Amendment 1](rfcs/0007-job-lifecycle-completeness.md#amendment-1--approved-may-time-out-2026-08-19)). + ## Multi-Tenancy Tenant → Workspace → Resource. `tenant_id` is required on every job, decision, and event; isolation is enforced at the storage layer, not by query filter. diff --git a/conformance/payload_check.py b/conformance/payload_check.py index e04a479..8e14bb9 100644 --- a/conformance/payload_check.py +++ b/conformance/payload_check.py @@ -137,18 +137,22 @@ def build_validator(schemas: dict[str, dict], target_id: str, non_schema_keys: l def run_scenario(): """Drive real jobs through the real engine and return the log plus what happened. - Six jobs across two tenants, covering every terminal state, the mid-run - approval pause, rejection and resubmission, and recovery by supersession — - plus two inbound external events that no job caused. + Seven jobs across two tenants, covering every terminal state, the mid-run + approval pause, rejection and resubmission, recovery by supersession, and an + approval that expired where it sat — plus two inbound external events that no + job caused. The journeys themselves are ``simulation/flows.py``, which issue #7 made the one place they are written down. Two files describing the same lifecycle differently is how they end up disagreeing about it, and this check exists to catch disagreement rather than to add one. """ + from datetime import datetime, timedelta, timezone + from devfactory_core import JobState, Principal from devfactory_observability import EventLog, accept_external from simulation.flows import ( + approval_expired, cancelled_by_a_person, failed_at, happy_path, @@ -186,11 +190,21 @@ def run_scenario(): # 5. an approval nobody answered, in a second tenant stalled = stalled_awaiting_approval(globex, job_id="job-006", authority=reviewer) - jobs = [happy, revised, failed, replacement, cancelled, stalled] + # 6. an approval that expired where it sat — the only flow that produces an + # approval/v1 payload carrying expires_at, so the field is validated as a + # real payload rather than asserted about in the abstract + expired = approval_expired( + acme, + job_id="job-008", + authority=reviewer, + expires_at=datetime.now(timezone.utc) - timedelta(hours=1), + ) + + jobs = [happy, revised, failed, replacement, cancelled, stalled, expired] for job in jobs: log.extend(job.events) - # 6. events from another system, which no job caused + # 7. events from another system, which no job caused external = [ accept_external( { @@ -313,6 +327,23 @@ def check_decisions(log, jobs, validator) -> None: else: ok("approval", "ทุก APPROVE มี decision record และ event คู่กับ STATE_TRANSITION") + # "approval ที่หมดอายุแล้วใช้เดินงานไม่ได้ ต้องขอใหม่" (approval/v1 expires_at) + # เขาเขียนความหมายไว้ใน schema แล้ว แต่ JSON Schema ตรวจให้ไม่ได้ว่า engine ทำตามไหม + # จึงต้องตรวจที่นี่ · ดู rfcs/0007 Amendment 1 · issue #17 + expiring = [ + payload + for payload in decisions + if (payload.get("metadata") or {}).get("approval", {}).get("expires_at") + ] + if expiring: + ok("approval", f"{len(expiring)} approval พก expires_at และผ่าน approval/v1") + else: + fail( + "approval", + "ไม่มี approval ที่พก expires_at เลย — เช็คข้างล่างจะกลายเป็นการตรวจสิ่งที่ไม่มีจริง", + ) + check_expiry_enforced() + # REQUIRE_CHANGES อยู่ใน vocabulary แต่ยังไม่มี RFC กำหนดว่ามันพา job ไปไหน # engine ต้องปฏิเสธ ไม่ใช่เดาปลายทาง — ดู states.DECISION_TARGET from devfactory_core import DecisionType, Job, Principal @@ -336,6 +367,92 @@ def check_decisions(log, jobs, validator) -> None: ok("approval", "REQUIRE_CHANGES ถูกปฏิเสธ — ยังไม่มี RFC กำหนดปลายทางของมัน") +def check_expiry_enforced() -> None: + """Would we notice a job still running on an approval that had expired? + + Two sides, because the guarantee has two. The engine must refuse to move the + job; and if something else moved it anyway, the trail must not read back as + though that were fine — the deadline and the moment are both recorded, so the + log can be judged against itself by a reader who was not there. + + The second half is checked by forging a trail rather than by producing one, + for the same reason the ``REQUIRE_CHANGES`` probe below exists: the engine + cannot produce the record we need to catch, and a check that can only see + records the engine agrees with is not checking the engine. + """ + import dataclasses + from datetime import datetime, timedelta, timezone + + from devfactory_core import JobState, Principal + from devfactory_core.errors import ExpiredApproval + from devfactory_observability import ExecutionAfterExpiry, replay_job + from simulation.flows import job_factory + + # A clock that can be pushed forward, because an approval expires by time + # passing and nothing else. Granting one that was already stale would be a + # different, less interesting record. + start = datetime(2026, 8, 19, 9, 0, tzinfo=timezone.utc) + now = {"t": start} + + def clock() -> datetime: + now["t"] += timedelta(seconds=1) + return now["t"] + + owner = Principal("human", "alice") + reviewer = Principal("human", "bob") + new = job_factory( + tenant_id="acme", workspace_id="ws-core", principal=owner, clock=clock + ) + deadline = start + timedelta(minutes=5) + + lapsed = new("job-009") + lapsed.submit_for_governance() + lapsed.approve(authority=reviewer, reason="approved", expires_at=deadline) + now["t"] = deadline + timedelta(hours=1) # nobody came for the job in time + try: + lapsed.transition(JobState.TASK_PLANNING) + fail("approval", "งานเดินเข้า TASK_PLANNING ได้ด้วย approval ที่หมดอายุแล้ว") + except ExpiredApproval: + ok("approval", "approval ที่หมดอายุใช้เดินงานต่อไม่ได้ — engine ปฏิเสธ") + + # The job is not stuck: RFC-0007 Amendment 1 gives it somewhere honest to land. + lapsed.time_out(reason="approval_expired — the approval lapsed before planning began") + if lapsed.state is JobState.TIMED_OUT: + ok("approval", "งานที่ถือ approval หมดอายุเข้า TIMED_OUT ได้ (rfcs/0007 Amendment 1)") + else: + fail("approval", f"งานที่ถือ approval หมดอายุไปจบที่ {lapsed.state.value}") + + # Now the log. Same journey with an approval that was still valid, then the + # recorded deadline moved into the past — which is what "the job kept going on + # an expired approval" looks like when it is read back rather than watched. + ran_on = new("job-010") + ran_on.submit_for_governance() + ran_on.approve( + authority=reviewer, reason="approved", expires_at=now["t"] + timedelta(hours=1) + ) + ran_on.transition(JobState.TASK_PLANNING) + forged = [ + dataclasses.replace( + event, + metadata={ + **event.metadata, + "approval": { + **event.metadata["approval"], + "expires_at": start.isoformat(), + }, + }, + ) + if event.type_value == "GOVERNANCE_DECISION" + else event + for event in ran_on.events + ] + try: + replay_job(forged) + fail("approval", "trail ที่บันทึกว่างานเดินต่อหลัง approval หมดอายุ ยัง replay ผ่าน") + except ExecutionAfterExpiry: + ok("approval", "replay จับได้ว่า trail บันทึกการเดินงานหลัง approval หมดอายุ") + + def check_gap_expiry(gaps: list[dict], today: str) -> None: """A waiver with no end date is a permanent exception, which ADR-0006 forbids.""" for gap in gaps or (): diff --git a/contract-semantics.yaml b/contract-semantics.yaml index 0a71417..78cb133 100644 --- a/contract-semantics.yaml +++ b/contract-semantics.yaml @@ -107,6 +107,20 @@ contracts: note: >- packages/core ทำ decision interface ครบตาม RFC-0002 แล้ว (issue #5) vocabulary ยังมีครบ 3 ค่าตามชุดปิด — ห้ามลบค่าใดออกจาก contract + enforced_optional_fields: + # field ที่ agent-platform เพิ่มเองได้ (platform_may_add_freely ด้านล่าง) + # แต่ตัว schema เขียนความหมายไว้แล้ว — repo นี้จึงบังคับตามความหมายนั้น + # ไม่ใช่การเพิ่ม guarantee ใหม่ แต่คือการปิดช่องว่างของการ conform + - field: expires_at + detail: >- + approval/v1 บอกว่า "approval ที่หมดอายุแล้วใช้เดินงานไม่ได้ ต้องขอใหม่ · + งานที่ค้างรออนุมัติจนเลยกำหนดควรเข้าสถานะ timeout ไม่ใช่รอตลอดไป" + · Decision เก็บค่านี้ · engine ปฏิเสธการเข้า state ฝั่ง execution + ด้วย approval ที่หมดอายุ (ExpiredApproval) · replay ปฏิเสธ trail ที่บันทึกว่าเกิดขึ้นแล้ว + (ExecutionAfterExpiry) · ปลายทางของงานที่ค้างคือ TIMED_OUT ตาม rfcs/0007 Amendment 1 + optional: >- + ยัง optional เหมือนใน schema — approval ที่ไม่มี expires_at คือไม่มีวันหมดอายุ + บังคับให้ต้องมีจะทำให้ engine เข้มกว่า contract ที่ conform อยู่ · issue #17 unmapped: - decision: REQUIRE_CHANGES detail: >- @@ -228,8 +242,12 @@ not_derived: # ก่อน APPROVED ยังไม่มี execution ผลลัพธ์ที่ซื่อสัตย์คือ REJECTED / CANCELLED / TIMED_OUT failable: [TASK_PLANNING, IN_PROGRESS, AWAITING_APPROVAL, VALIDATING, DEPLOYABLE] - # rfcs/0007 - timeoutable: [GOVERNANCE_ANALYSIS, TASK_PLANNING, IN_PROGRESS, AWAITING_APPROVAL, VALIDATING] + # rfcs/0007 · APPROVED เพิ่มโดย Amendment 1 ของ rfcs/0007 (2026-08-19, issue #17) + # เหตุผลเป็น governance ไม่ใช่ liveness: การอนุมัติต้องมีวันหมดอายุ + # job ที่ค้างใน APPROVED ได้ตลอดไป = เริ่มทำงานอีกสัปดาห์ภายใต้คำตัดสินที่บริบทเปลี่ยนไปแล้ว + # ซึ่งเป็น stale APPROVED แบบเดียวกับที่ rfcs/0007 Decision 1 ห้ามตอนไม่ยอมปลุก job ที่ FAILED + timeoutable: [GOVERNANCE_ANALYSIS, APPROVED, TASK_PLANNING, IN_PROGRESS, AWAITING_APPROVAL, + VALIDATING] # rfcs/0007 — หยุดงานได้ทุกจุดก่อนที่มันจะ settle (= states ทั้งหมด ลบ terminal) # เขียนเป็น list เหมือน failable/timeoutable เพื่อให้เครื่องที่อ่านสามคีย์นี้ parse ได้แบบเดียวกัน @@ -245,6 +263,7 @@ not_derived: - AWAITING_APPROVAL คนละอย่างกับ GOVERNANCE_ANALYSIS — อันหลังคือประตูก่อนเริ่มงาน - FAILED · CANCELLED · TIMED_OUT ต้องมี reason metadata - job ที่ยังไม่ผ่าน APPROVED เข้า FAILED ไม่ได้ — recovery ด้วย supersedes_job_id จะไม่มีอะไรให้ supersede (rfcs/0010) + - approval ที่เลย expires_at แล้วใช้เดินงานต่อไม่ได้ — engine ปฏิเสธ ไม่ใช่ปล่อยผ่าน · job ที่ค้างอยู่เข้า TIMED_OUT ได้ (rfcs/0007 Amendment 1) layering: job (ที่นี่) > execution (execution/v1) > step (event/) orchestration_execution_boundary: diff --git a/packages/core/README.md b/packages/core/README.md index c7a1d54..3205689 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -56,6 +56,7 @@ enforced, so the engine rejects rather than repairs. | an edge not in the table | `InvalidTransition`, naming what *was* allowed | | anything out of `COMPLETED` / `FAILED` / `CANCELLED` / `TIMED_OUT` | `TerminalState` — recovery is `supersede()`, not a revival | | `TASK_PLANNING` before `APPROVED` | `InvalidTransition` — execution is forbidden before approval | +| any post-approval state under an approval past its `expires_at` | `ExpiredApproval` — it has to be granted again | | `FAILED` / `CANCELLED` / `TIMED_OUT` without a reason | `MissingReason` | | `CANCELLED` without a principal | `MissingPrincipal` | | `APPROVED` / `REJECTED` without an authority and reason | `MissingAuthority` | @@ -116,5 +117,21 @@ Settled by [RFC-0010](../../rfcs/0010-failable-states.md): `FAILED` is reachable the states where work exists to fail — and refused before `APPROVED`, where the honest outcomes are `REJECTED`, `CANCELLED`, or `TIMED_OUT`. See `states.FAILABLE`. -`APPROVED` is in neither `FAILABLE` nor `TIMEOUTABLE`, so a job that stalls there has no -automatic exit — RFC-0010 records this as an open question rather than a decision. +## Approval expiry + +`approval/v1` carries `expires_at` and says what it means: an approval past it cannot be +used to run work and has to be granted again. `Decision` stores it, `approve()` takes it, +and the engine refuses to move a job into a post-approval state under a lapsed one. + +```python +job.approve(authority=bob, reason="scope agreed", expires_at=deadline) +job.approval_expires_at # the deadline, or None +job.approval_expired # whether it has passed, as of now +``` + +`expires_at` is optional in the contract and optional here — an approval without one never +expires. What changed with issue #17 is that `APPROVED` is now in `TIMEOUTABLE` +([RFC-0007 Amendment 1](../../rfcs/0007-job-lifecycle-completeness.md#amendment-1--approved-may-time-out-2026-08-19)), +so a job holding an approval that ran out has an honest terminal to reach instead of +waiting for a human to cancel it. The timeout *policy* — how long an approval is good for +— stays out of scope, as it is in RFC-0007 and RFC-0010. diff --git a/packages/core/devfactory_core/__init__.py b/packages/core/devfactory_core/__init__.py index 32d4627..93409a8 100644 --- a/packages/core/devfactory_core/__init__.py +++ b/packages/core/devfactory_core/__init__.py @@ -9,6 +9,7 @@ CrossTenantDecision, DecisionStateMismatch, ExecutionBeforeApproval, + ExpiredApproval, IncompleteDecision, InvalidIdentifier, InvalidTransition, @@ -41,6 +42,7 @@ "Event", "EventType", "ExecutionBeforeApproval", + "ExpiredApproval", "IncompleteDecision", "InvalidIdentifier", "InvalidTransition", diff --git a/packages/core/devfactory_core/decision.py b/packages/core/devfactory_core/decision.py index eb3f5fc..c5e50ca 100644 --- a/packages/core/devfactory_core/decision.py +++ b/packages/core/devfactory_core/decision.py @@ -128,6 +128,17 @@ class Decision: decided_at: datetime workspace_id: str | None = None + #: When this decision stops authorising anything — ``approval/v1`` + #: ``expires_at``. + #: + #: Optional there and optional here, which is the whole of what the contract + #: says: an approval with no expiry never expires, and requiring one would + #: make this engine stricter than the contract it conforms to. What is *not* + #: optional is the meaning when it is present — "approval ที่หมดอายุแล้วใช้เดินงาน + #: ไม่ได้ ต้องขอใหม่" — which :class:`~devfactory_core.job.Job` enforces by + #: refusing to move a job into execution under an expired one. + expires_at: datetime | None = None + #: The decision this one replaces. #: #: ``approval/v1`` states the guarantee — "การเปลี่ยนใจคือ approval ใบใหม่ที่ @@ -160,6 +171,33 @@ def __post_init__(self) -> None: raise IncompleteDecision("reason") if not isinstance(self.decided_at, datetime): raise IncompleteDecision("decided_at") + if self.expires_at is not None: + if not isinstance(self.expires_at, datetime): + raise ValueError( + f"expires_at must be a datetime — got {type(self.expires_at).__name__}" + ) + # ``approval/v1`` types it ``format: date-time``, which is RFC 3339 and + # carries an offset. A naive value cannot be compared against the + # engine's clock without assuming a zone, and assuming one is how an + # approval silently expires at the wrong moment. + if self.expires_at.tzinfo is None: + raise ValueError( + "expires_at must be timezone-aware — an approval that expires at " + "an unstated offset expires at a different time for every reader" + ) + + def is_expired(self, now: datetime) -> bool: + """Whether this decision no longer authorises anything, as of ``now``. + + An approval with no ``expires_at`` is never expired: the field is optional + in ``approval/v1``, and absent means "no deadline was set", not "expired". + + The boundary is inclusive — at the instant named the approval is already + spent. ``expires_at`` is the moment it stops being usable, not the last + moment it can be used, and rounding that in the permissive direction would + let a job start on an approval that had run out. + """ + return self.expires_at is not None and now >= self.expires_at def as_payload(self) -> dict[str, Any]: """Render to the ``approval/v1`` wire shape. @@ -180,6 +218,8 @@ def as_payload(self) -> dict[str, Any]: } if self.workspace_id is not None: payload["workspace_id"] = self.workspace_id + 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 diff --git a/packages/core/devfactory_core/errors.py b/packages/core/devfactory_core/errors.py index a9fc5e5..778a724 100644 --- a/packages/core/devfactory_core/errors.py +++ b/packages/core/devfactory_core/errors.py @@ -94,6 +94,30 @@ def __init__(self, requested: str) -> None: ) +class ExpiredApproval(JobStateMachineError): + """The approval in force has run out, so it authorises nothing further. + + ``approval/v1`` states the rule on ``expires_at``: *"approval ที่หมดอายุแล้ว + ใช้เดินงานไม่ได้ ต้องขอใหม่"*. Letting the job proceed anyway would be exactly + the stale-``APPROVED`` execution RFC-0007 Decision 1 refuses — work running + under a verdict formed in a context that has since lapsed. + + The remedy is a fresh decision, not a retry of this call. A job stuck here can + still be cancelled, and since RFC-0007's 2026-08-19 amendment an ``APPROVED`` + job whose approval has lapsed can also be timed out (issue #17). + """ + + def __init__(self, requested: str, *, expired_at: str, now: str) -> None: + self.requested = requested + self.expired_at = expired_at + self.now = now + super().__init__( + f"cannot reach {requested}: the approval in force expired at {expired_at} " + f"and it is now {now} — an expired approval cannot be used to run work, " + f"it has to be granted again" + ) + + class MissingAuthority(JobStateMachineError): """APPROVED and REJECTED are decisions and must name who made them.""" diff --git a/packages/core/devfactory_core/job.py b/packages/core/devfactory_core/job.py index 0ba09a8..aeeab1a 100644 --- a/packages/core/devfactory_core/job.py +++ b/packages/core/devfactory_core/job.py @@ -14,6 +14,12 @@ Governance decisions are RFC-0002, added by issue #5. A job holds the decision it is executing under (``approval``) rather than a boolean, because "approved" is not a fact about a job — it is a record of who decided what, when, and why. + +An approval can also say when it stops being one. ``approval/v1`` carries +``expires_at`` and states what it means — *"approval ที่หมดอายุแล้วใช้เดินงานไม่ได้ +ต้องขอใหม่"* — so the engine refuses to move a job into execution under a lapsed +approval, and RFC-0007's 2026-08-19 amendment gives such a job somewhere honest to +land by putting ``APPROVED`` in ``states.TIMEOUTABLE`` (issue #17). """ from __future__ import annotations @@ -27,6 +33,7 @@ CrossTenantDecision, DecisionStateMismatch, ExecutionBeforeApproval, + ExpiredApproval, InvalidTransition, MissingApprovalContext, MissingAuthority, @@ -174,6 +181,30 @@ def approval(self) -> Decision | None: """ return self._approval + @property + def approval_expires_at(self) -> datetime | None: + """When the approval in force stops authorising anything, if it says. + + ``None`` covers both "no approval" and "an approval with no deadline" — + two different situations that this property is not the place to tell + apart, since :attr:`approval` already does. + """ + return self._approval.expires_at if self._approval is not None else None + + @property + def approval_expired(self) -> bool: + """Whether the approval in force has run out as of now. + + Reads the clock, so it is a question about this moment rather than a + stored fact — an approval expires by time passing, not by anything + happening to the job. The clock is only consulted when there is a deadline + to compare against, so a job whose approval carries no ``expires_at`` + answers this without asking what time it is. + """ + return self.approval_expires_at is not None and self._approval.is_expired( + self._clock() + ) + @property def decisions(self) -> tuple[Decision, ...]: """Every governance decision made about this job, in order. @@ -313,6 +344,7 @@ def decide( authority: Principal, reason: str, supersedes_decision_id: str | None = None, + expires_at: datetime | None = None, ) -> Decision: """Record a governance decision and move the job where it sends it. @@ -326,6 +358,12 @@ def decide( Refuses ``REQUIRE_CHANGES`` with ``UnmappedDecision``: RFC-0002 declares it, no RFC here says where it sends a job, and the engine will not invent a destination. See ``states.DECISION_TARGET``. + + ``expires_at`` is ``approval/v1``'s deadline for the decision, and it is + the caller's to set: the timeout *policy* — how long an approval is good + for, per job type — is out of scope for RFC-0007 and RFC-0010 alike, so + nothing here invents a default. Passing nothing produces an approval with + no deadline, which is the pre-existing behaviour and stays valid. """ decision = DecisionType(decision) target = DECISION_TARGET.get(decision) @@ -340,6 +378,7 @@ def decide( reason=reason, authority=authority, 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 @@ -353,8 +392,19 @@ def decide( self.transition(target, reason=reason, principal=authority, decision=record) return record - def approve(self, *, authority: Principal, reason: str) -> Decision: - return self.decide(DecisionType.APPROVE, authority=authority, reason=reason) + def approve( + self, + *, + authority: Principal, + reason: str, + expires_at: datetime | None = None, + ) -> Decision: + return self.decide( + DecisionType.APPROVE, + authority=authority, + reason=reason, + expires_at=expires_at, + ) def reject(self, *, authority: Principal, reason: str) -> Decision: return self.decide(DecisionType.REJECT, authority=authority, reason=reason) @@ -436,6 +486,18 @@ def _check_guards( # wrongly — which is exactly when it is worth having. if to in POST_APPROVAL and self._approval is None: raise ExecutionBeforeApproval(to.value) + # The same lock in its second half. Holding an approval is not enough if + # the approval has run out: ``approval/v1`` says an expired one cannot be + # used to run work and has to be granted again. POST_APPROVAL is the set + # of states that mean execution has been authorised, so it is exactly the + # set an expired authorisation must not open — including the way back out + # of AWAITING_APPROVAL, where a pause is what let the deadline pass. + if to in POST_APPROVAL and self.approval_expired: + raise ExpiredApproval( + to.value, + expired_at=self._approval.expires_at.isoformat(), + now=self._clock().isoformat(), + ) def _decision_for( self, to: JobState, *, authority: Principal | None, reason: str | None diff --git a/packages/core/devfactory_core/states.py b/packages/core/devfactory_core/states.py index 3af5642..06817f6 100644 --- a/packages/core/devfactory_core/states.py +++ b/packages/core/devfactory_core/states.py @@ -60,9 +60,19 @@ class JobState(str, Enum): #: RFC-0007: TIMED_OUT is reachable from these. AWAITING_APPROVAL is included #: because an approval nobody answers is how a governed pipeline usually stalls. +#: +#: ``APPROVED`` was added by RFC-0007's 2026-08-19 amendment (issue #17). The +#: argument is governance rather than liveness: **an approval must expire.** A job +#: that may sit in APPROVED forever can start executing a week later under a +#: verdict formed in a context that no longer holds — the same stale APPROVED that +#: RFC-0007 Decision 1 refuses when it forbids reviving a FAILED job. ``approval/v1`` +#: already says so on ``expires_at``: "approval ที่หมดอายุแล้วใช้เดินงานไม่ได้ ต้องขอใหม่ · +#: งานที่ค้างรออนุมัติจนเลยกำหนดควรเข้าสถานะ timeout ไม่ใช่รอตลอดไป". Without this edge +#: there was no state for that job to land in. TIMEOUTABLE: frozenset[JobState] = frozenset( { JobState.GOVERNANCE_ANALYSIS, + JobState.APPROVED, JobState.TASK_PLANNING, JobState.IN_PROGRESS, JobState.AWAITING_APPROVAL, diff --git a/packages/core/state-machine.md b/packages/core/state-machine.md index 8c8d612..85607b9 100644 --- a/packages/core/state-machine.md +++ b/packages/core/state-machine.md @@ -40,8 +40,10 @@ Execution is forbidden before `APPROVED`. `FAILED` is terminal — recovery is a new job carrying `supersedes_job_id`, not a retry. `CANCELLED` is reachable from every non-terminal state. -`TIMED_OUT` is reachable from `GOVERNANCE_ANALYSIS`, `TASK_PLANNING`, `IN_PROGRESS`, -`AWAITING_APPROVAL`, and `VALIDATING`. +`TIMED_OUT` is reachable from `GOVERNANCE_ANALYSIS`, `APPROVED`, `TASK_PLANNING`, +`IN_PROGRESS`, `AWAITING_APPROVAL`, and `VALIDATING` — `APPROVED` since +[RFC-0007 Amendment 1](../../rfcs/0007-job-lifecycle-completeness.md#amendment-1--approved-may-time-out-2026-08-19), +because an approval must expire (issue #17). `FAILED` is reachable from `TASK_PLANNING`, `IN_PROGRESS`, `AWAITING_APPROVAL`, `VALIDATING`, and `DEPLOYABLE` — the states where work exists to fail — and from nowhere before `APPROVED`, where the honest outcomes are `REJECTED`, `CANCELLED`, or `TIMED_OUT` @@ -79,6 +81,32 @@ Guarantees the engine enforces, not just documents: authority* - execution stays locked until the job holds an `APPROVE` record, not merely the `APPROVED` state +- an approval past its `expires_at` unlocks nothing: entering a post-approval state + under one is refused (`ExpiredApproval`), and a trail that records it happening is + refused on replay (`ExecutionAfterExpiry`). `expires_at` is optional in + `approval/v1` and stays optional here — an approval with no deadline never expires + +## Approval expiry + +`approval/v1` carries `expires_at` and states the rule: *"approval ที่หมดอายุแล้วใช้เดินงาน +ไม่ได้ ต้องขอใหม่ · งานที่ค้างรออนุมัติจนเลยกำหนดควรเข้าสถานะ timeout ไม่ใช่รอตลอดไป"*. + +```python +job.approve(authority=bob, reason="scope agreed", expires_at=deadline) +job.approval_expires_at # the deadline, or None +job.approval_expired # whether it has passed, as of now +``` + +Once it has passed, the job cannot move into `TASK_PLANNING`, `IN_PROGRESS`, +`AWAITING_APPROVAL`, `VALIDATING`, or `DEPLOYABLE` — the states that mean execution was +authorised, including the way back out of a pause. What remains is a fresh `APPROVE`, +`CANCELLED`, or `TIMED_OUT`; +[RFC-0007 Amendment 1](../../rfcs/0007-job-lifecycle-completeness.md#amendment-1--approved-may-time-out-2026-08-19) +added the last of those so a job stalled in `APPROVED` has an honest terminal. + +Timeout **policy** — how long an approval is good for — is not set here. RFC-0007 and +RFC-0010 both leave the values out of scope, so nothing in this repository supplies a +default `expires_at` or fires a timeout on its own. ## AWAITING_APPROVAL @@ -126,6 +154,9 @@ Recorded rather than answered — each needs an RFC, not a code change. self-approval invariant about agents only, so the engine refuses agent self-approval and allows the human case. Widening it would make this engine stricter than the contract it conforms to. -- `APPROVED` is in neither `FAILABLE` nor `TIMEOUTABLE`, so an approval nobody acts on - has no automatic exit — [RFC-0010](../../rfcs/0010-failable-states.md) records this, - and it is issue #17. +- How long is an approval good for? `expires_at` is enforced when it is set, and nothing + here sets it. The policy values are Future Work in both + [RFC-0007](../../rfcs/0007-job-lifecycle-completeness.md) and + [RFC-0010](../../rfcs/0010-failable-states.md), so an approval granted without a + deadline is still an approval that never expires — the state now exists for one that + does. Settling the values is orchestration's, and needs an RFC of its own. diff --git a/packages/core/tests/test_decisions.py b/packages/core/tests/test_decisions.py index baca06a..ed9b3b7 100644 --- a/packages/core/tests/test_decisions.py +++ b/packages/core/tests/test_decisions.py @@ -7,6 +7,8 @@ from __future__ import annotations +from datetime import datetime, timedelta, timezone + import pytest from conftest import drive @@ -449,6 +451,65 @@ def test_identifiers_on_a_decision_are_validated(reviewer, clock): _decision(clock, reviewer, tenant_id="ACME") +# ---- expiry, approval/v1 expires_at ---------------------------------------- +# "approval ที่หมดอายุแล้วใช้เดินงานไม่ได้ ต้องขอใหม่" — the contract states the meaning +# on an optional field, so the record has to be able to carry it. Issue #17. + + +def test_an_approval_can_carry_the_deadline_it_was_granted_with(alice, reviewer, clock): + deadline = datetime(2026, 8, 20, 9, tzinfo=timezone.utc) + record = _at_the_gate(alice, clock).approve( + authority=reviewer, reason="ok", expires_at=deadline + ) + assert record.expires_at == deadline + + +def test_the_deadline_reaches_the_wire_under_the_contracts_own_name( + alice, reviewer, clock +): + deadline = datetime(2026, 8, 20, 9, tzinfo=timezone.utc) + payload = ( + _at_the_gate(alice, clock) + .approve(authority=reviewer, reason="ok", expires_at=deadline) + .as_payload() + ) + assert payload["expires_at"] == deadline.isoformat() + + +def test_an_approval_without_a_deadline_says_nothing_about_one(alice, reviewer, clock): + """Optional in ``approval/v1`` and optional here — absent is not "expired".""" + record = _at_the_gate(alice, clock).approve(authority=reviewer, reason="ok") + assert record.expires_at is None + assert "expires_at" not in record.as_payload() + assert record.is_expired(datetime(2999, 1, 1, tzinfo=timezone.utc)) is False + + +def test_expiry_is_inclusive_at_the_moment_named(alice, reviewer, clock): + """``expires_at`` is when it stops being usable, not the last moment it is.""" + deadline = datetime(2026, 8, 20, 9, tzinfo=timezone.utc) + record = _at_the_gate(alice, clock).approve( + authority=reviewer, reason="ok", expires_at=deadline + ) + assert record.is_expired(deadline - timedelta(seconds=1)) is False + assert record.is_expired(deadline) is True + assert record.is_expired(deadline + timedelta(seconds=1)) is True + + +def test_a_deadline_without_an_offset_is_refused(alice, reviewer, clock): + """A naive deadline expires at a different moment for every reader.""" + with pytest.raises(ValueError, match="timezone-aware"): + _at_the_gate(alice, clock).approve( + authority=reviewer, reason="ok", expires_at=datetime(2026, 8, 20, 9) + ) + + +def test_a_deadline_that_is_not_a_time_is_refused(alice, reviewer, clock): + with pytest.raises(ValueError, match="datetime"): + _at_the_gate(alice, clock).approve( + authority=reviewer, reason="ok", expires_at="2026-08-20T09:00:00+00:00" + ) + + # ---- the subject ----------------------------------------------------------- diff --git a/packages/core/tests/test_guards.py b/packages/core/tests/test_guards.py index 17ecd96..0d30ecb 100644 --- a/packages/core/tests/test_guards.py +++ b/packages/core/tests/test_guards.py @@ -2,12 +2,16 @@ from __future__ import annotations +import dataclasses +from datetime import datetime, timezone + import pytest from conftest import drive from devfactory_core import Job, JobState, Principal from devfactory_core.errors import ( ExecutionBeforeApproval, + ExpiredApproval, InvalidIdentifier, InvalidTransition, MissingApprovalContext, @@ -16,6 +20,11 @@ MissingReason, WrongResumeState, ) +from devfactory_core.states import POST_APPROVAL + +#: Either side of the ``clock`` fixture, which starts at 2026-08-18 09:00 UTC. +EXPIRY_PAST = datetime(2026, 8, 18, 8, tzinfo=timezone.utc) +EXPIRY_FAR_FUTURE = datetime(2027, 1, 1, tzinfo=timezone.utc) def _fresh(alice, clock, **kw) -> Job: @@ -207,3 +216,140 @@ def test_supersede_records_the_link_in_the_audit_trail(alice, clock): replacement = job.supersede(job_id="job-002") created = replacement.events[0].as_payload() assert created["metadata"]["supersedes_job_id"] == "job-001" + + +# ---- approval expiry, RFC-0007 Amendment 1 --------------------------------- +# ``approval/v1``: "approval ที่หมดอายุแล้วใช้เดินงานไม่ได้ ต้องขอใหม่". The engine +# refuses rather than tolerates, which is what makes that sentence a rule and not +# a comment. Issue #17. + + +def _approved_until(alice, clock, deadline, authority): + """A job holding an APPROVE that expires at ``deadline``.""" + job = _fresh(alice, clock) + job.submit_for_governance() + job.approve(authority=authority, reason="approved for milestone", expires_at=deadline) + return job + + +def test_an_approval_that_has_not_expired_still_authorises_work(alice, clock): + job = _approved_until(alice, clock, EXPIRY_FAR_FUTURE, alice) + assert job.approval_expired is False + job.transition(JobState.TASK_PLANNING) + assert job.state is JobState.TASK_PLANNING + + +#: Where each post-approval state is entered from. Expiry can only be tested on an +#: edge the table declares, so reaching every one of them means starting from a +#: different place each time. +_ENTERED_FROM = { + JobState.TASK_PLANNING: JobState.APPROVED, + JobState.IN_PROGRESS: JobState.TASK_PLANNING, + JobState.AWAITING_APPROVAL: JobState.IN_PROGRESS, + JobState.VALIDATING: JobState.IN_PROGRESS, + JobState.DEPLOYABLE: JobState.VALIDATING, +} + + +def test_the_expiry_check_covers_every_state_that_needs_an_approval(): + """Both halves of the direction lock guard the same set, or there is a hole. + + If expiry covered less than ``POST_APPROVAL`` there would be a state reachable + without a *valid* approval but not without *any* approval, which is a strange + thing for a governance engine to believe. + """ + assert set(_ENTERED_FROM) == POST_APPROVAL + + +@pytest.mark.parametrize( + "target", sorted(POST_APPROVAL, key=lambda s: s.value), ids=lambda s: s.value +) +def test_no_post_approval_state_is_reachable_under_an_expired_approval( + target, alice, clock +): + """The set that means "execution was authorised" is the set expiry closes.""" + origin = _ENTERED_FROM[target] + job = drive(_fresh(alice, clock), origin, alice) + job._approval = dataclasses.replace(job.approval, expires_at=EXPIRY_PAST) + with pytest.raises(ExpiredApproval) as excinfo: + job.transition(target) + assert excinfo.value.requested == target.value + assert job.state is origin + + +def test_a_refused_expired_transition_leaves_no_trace(alice, clock): + job = _approved_until(alice, clock, EXPIRY_PAST, alice) + before = len(job.events), len(job.history) + with pytest.raises(ExpiredApproval): + job.transition(JobState.TASK_PLANNING) + assert (len(job.events), len(job.history)) == before + + +def test_an_approval_can_lapse_while_the_job_is_already_working(alice, clock): + """The deadline is on the approval, not on the step it authorised.""" + job = _approved_until(alice, clock, EXPIRY_FAR_FUTURE, alice) + job.transition(JobState.TASK_PLANNING) + job.transition(JobState.IN_PROGRESS) + job._approval = dataclasses.replace(job.approval, expires_at=EXPIRY_PAST) + with pytest.raises(ExpiredApproval): + job.transition(JobState.VALIDATING) + + +def test_a_pause_that_outlasts_the_approval_cannot_be_resumed(alice, clock): + """Waiting for a human is the likeliest way a deadline gets passed.""" + job = _approved_until(alice, clock, EXPIRY_FAR_FUTURE, alice) + job.transition(JobState.TASK_PLANNING) + job.transition(JobState.IN_PROGRESS) + job.pause_for_approval() + job._approval = dataclasses.replace(job.approval, expires_at=EXPIRY_PAST) + with pytest.raises(ExpiredApproval): + job.resume() + assert job.state is JobState.AWAITING_APPROVAL + + +def test_a_job_holding_an_expired_approval_can_still_time_out(alice, clock): + """RFC-0007 Amendment 1 — the reason APPROVED joined TIMEOUTABLE.""" + job = _approved_until(alice, clock, EXPIRY_PAST, alice) + job.time_out(reason="approval_expired — the approval lapsed before planning began") + assert job.state is JobState.TIMED_OUT + assert job.history[-1].from_state is JobState.APPROVED + + +def test_a_job_holding_an_expired_approval_can_still_be_cancelled(alice, clock): + job = _approved_until(alice, clock, EXPIRY_PAST, alice) + job.cancel(reason="no longer wanted", principal=alice) + assert job.state is JobState.CANCELLED + + +def test_the_only_ways_out_of_an_expired_approval_are_the_two_terminals(alice, clock): + """Recorded as it is, not as it might be nicer. + + ``approval/v1`` says the remedy is "ต้องขอใหม่", and this lifecycle has no edge + for asking again: ``APPROVED`` reaches only ``TASK_PLANNING`` (now shut), + ``CANCELLED``, and — since RFC-0007 Amendment 1 — ``TIMED_OUT``. So the job + settles and a re-request is a *new* job. Giving ``APPROVED`` a way back to + ``GOVERNANCE_ANALYSIS`` would be a lifecycle change and belongs in an RFC, so + this test asserts today's shape rather than inventing tomorrow's. + """ + job = _approved_until(alice, clock, EXPIRY_PAST, alice) + reachable = {s for s in job.allowed_targets()} + assert reachable == {JobState.TASK_PLANNING, JobState.CANCELLED, JobState.TIMED_OUT} + with pytest.raises(ExpiredApproval): + job.transition(JobState.TASK_PLANNING) + + +def test_an_approval_without_a_deadline_never_expires(alice, clock): + """The pre-existing behaviour, asserted so the new guard cannot swallow it.""" + job = _fresh(alice, clock) + job.submit_for_governance() + job.approve(authority=alice, reason="approved for milestone") + assert job.approval_expires_at is None + assert job.approval_expired is False + job.transition(JobState.TASK_PLANNING) + assert job.state is JobState.TASK_PLANNING + + +def test_a_job_with_no_approval_at_all_is_not_reported_as_expired(alice, clock): + job = _fresh(alice, clock) + assert job.approval_expires_at is None + assert job.approval_expired is False diff --git a/packages/core/tests/test_states.py b/packages/core/tests/test_states.py index fc22962..40700f8 100644 --- a/packages/core/tests/test_states.py +++ b/packages/core/tests/test_states.py @@ -60,10 +60,32 @@ def test_timeout_reachable_where_specified(state): assert JobState.TIMED_OUT in static_targets(state) -def test_timeout_not_reachable_from_draft_or_approved(): - """Nothing is waiting on anything in DRAFT or APPROVED, so nothing can expire.""" +def test_timeout_not_reachable_from_draft(): + """A DRAFT is not waiting on anything, so there is nothing there to expire. + + This used to say the same about ``APPROVED``. RFC-0007 Amendment 1 (issue #17) + settled that it was wrong: the approval itself is the thing that expires, and a + job holding one has been waiting since the moment it was granted. + """ assert JobState.TIMED_OUT not in static_targets(JobState.DRAFT) - assert JobState.TIMED_OUT not in static_targets(JobState.APPROVED) + + +def test_an_approval_can_run_out_where_the_job_sits(): + """RFC-0007 Amendment 1: an approval must expire, so APPROVED must be able to. + + Without this edge a job whose orchestration died after the decision was + recorded had no automatic exit at all — only a human cancelling it moved it, + and until then it could still start executing under a verdict that had gone + stale. See ``states.TIMEOUTABLE``. + """ + assert JobState.APPROVED in TIMEOUTABLE + assert JobState.TIMED_OUT in static_targets(JobState.APPROVED) + + +def test_approved_still_cannot_fail(): + """The amendment moved one line and no more. RFC-0010 Decision 1 is untouched.""" + assert JobState.APPROVED not in FAILABLE + assert JobState.FAILED not in static_targets(JobState.APPROVED) @pytest.mark.parametrize("state", sorted(FAILABLE, key=lambda s: s.value)) diff --git a/packages/observability/README.md b/packages/observability/README.md index 1c61ea1..0475507 100644 --- a/packages/observability/README.md +++ b/packages/observability/README.md @@ -91,6 +91,7 @@ seen = replay_job(log.read("acme", job_id="job-001")) seen.state # JobState.COMPLETED seen.awaiting_from # where a paused job would return to seen.approval_decision_id # the APPROVE it was executing under +seen.approval_expires_at # when that APPROVE stopped authorising, if it said seen.history # every transition, reconstructed ``` @@ -112,6 +113,7 @@ no second transition table here to drift. | records an edge `states.py` does not declare | `UndeclaredTransition` | | enters `APPROVED`/`REJECTED` with no matching decision | `UnauditedDecision` | | begins execution with no `APPROVE` before it | `UnauditedExecution` | +| begins execution after that `APPROVE`'s `expires_at` | `ExecutionAfterExpiry` | | `COMPLETED` and `JOB_COMPLETED` disagree | `IncompleteSettlement` | | an unrecognised event type | **skipped** — `event/v1`: keep it, do not interpret it | | an external event | **skipped** — RFC-0008: another system is not an authority on our lifecycle | diff --git a/packages/observability/devfactory_observability/__init__.py b/packages/observability/devfactory_observability/__init__.py index 8eb5a5f..648f206 100644 --- a/packages/observability/devfactory_observability/__init__.py +++ b/packages/observability/devfactory_observability/__init__.py @@ -10,6 +10,7 @@ BrokenTrail, DuplicateEvent, EmptyTrail, + ExecutionAfterExpiry, ExternalSourceRequired, FabricatedIdentifier, IncompleteSettlement, @@ -32,6 +33,7 @@ "DuplicateEvent", "EmptyTrail", "EventLog", + "ExecutionAfterExpiry", "ExternalSourceRequired", "FabricatedIdentifier", "IncompleteSettlement", diff --git a/packages/observability/devfactory_observability/errors.py b/packages/observability/devfactory_observability/errors.py index 7a10475..9a88a3f 100644 --- a/packages/observability/devfactory_observability/errors.py +++ b/packages/observability/devfactory_observability/errors.py @@ -226,3 +226,32 @@ def __init__(self, job_id: str, state: str, event_id: str) -> None: f"job {job_id}: event {event_id} enters {state}, but no APPROVE decision " f"appears in the trail before it — execution without a recorded approval" ) + + +class ExecutionAfterExpiry(ReplayError): + """The trail shows execution continuing under an approval that had expired. + + The other half of the direction lock, read back off the log. ``approval/v1`` + says an approval past its ``expires_at`` cannot be used to run work; the + engine refuses it as it writes, and this is the same claim checked against + what was actually written — by something that was not there at the time and + can compare the recorded deadline against the recorded moment. + + Distinct from :class:`UnauditedExecution` on purpose. There the trail cannot + show an approval at all; here it shows one and shows that it had run out, + which is a different finding about the log and needs different follow-up. + """ + + def __init__( + self, job_id: str, state: str, event_id: str, *, expired_at: str, occurred_at: str + ) -> None: + self.job_id = job_id + self.state = state + self.event_id = event_id + self.expired_at = expired_at + self.occurred_at = occurred_at + super().__init__( + f"job {job_id}: event {event_id} enters {state} at {occurred_at}, but the " + f"APPROVE in force expired at {expired_at} — the trail records work " + f"running on an approval that had already lapsed" + ) diff --git a/packages/observability/devfactory_observability/replay.py b/packages/observability/devfactory_observability/replay.py index b3add7a..48a73eb 100644 --- a/packages/observability/devfactory_observability/replay.py +++ b/packages/observability/devfactory_observability/replay.py @@ -20,7 +20,10 @@ a claim about the log too, and this replays it: reaching a post-approval state with no ``APPROVE`` in the trail before it raises :class:`~devfactory_observability.errors.UnauditedExecution`, whatever the engine -believed at the time. +believed at the time. Reaching one *after* the recorded approval's ``expires_at`` +raises :class:`~devfactory_observability.errors.ExecutionAfterExpiry` for the same +reason — the deadline and the moment are both in the trail, so "this job ran on an +approval that had lapsed" is a question the log can answer about itself. What this module does **not** own is the lifecycle. Every edge it accepts is checked against ``devfactory_core.states.reachable_from`` — the same call the @@ -53,6 +56,7 @@ from .errors import ( BrokenTrail, EmptyTrail, + ExecutionAfterExpiry, IncompleteSettlement, UnauditedDecision, UnauditedExecution, @@ -95,6 +99,11 @@ class ReplayedJob: #: exactly as the engine clears it — an approval granted to an earlier #: revision must not appear to authorise the revised one when read back. approval_decision_id: str | None + #: When that approval stops authorising anything, as the trail recorded it, or + #: ``None`` when it carried no deadline. Read back rather than assumed: the + #: whole point of ``expires_at`` is that the log can be judged against it + #: later, and a replay that dropped it could not do the judging. + approval_expires_at: datetime | None decision_ids: tuple[str, ...] history: tuple[ReplayedTransition, ...] #: Whether the trail carries the ``JOB_COMPLETED`` that COMPLETED implies. @@ -131,6 +140,27 @@ def _approval_of(event: Event) -> dict[str, Any] | None: return approval +def _expiry_of(approval: dict[str, Any] | None) -> datetime | None: + """The ``expires_at`` an approval payload carries, if it carries a usable one. + + Unparseable or zone-less values are treated as absent rather than raising. + ``approval/v1`` types the field ``format: date-time`` and the conformance + check validates it there — a replay is not a second schema (RFC-0005 Rule 4), + and a malformed deadline is a finding about the *payload*, which the validator + already makes, not about whether the job moved when it should not have. + """ + if approval is None: + return None + raw = approval.get("expires_at") + if not isinstance(raw, str): + return None + try: + parsed = datetime.fromisoformat(raw) + except ValueError: + return None + return parsed if parsed.tzinfo is not None else None + + def replay_job(events: Iterable[Event]) -> ReplayedJob: """Reconstruct one job from its events, in the order they were logged. @@ -153,6 +183,7 @@ def replay_job(events: Iterable[Event]) -> ReplayedJob: state = JobState.DRAFT awaiting_from: JobState | None = None approval_decision_id: str | None = None + approval_expires_at: datetime | None = None pending: dict[str, Any] | None = None decision_ids: list[str] = [] history: list[ReplayedTransition] = [] @@ -192,16 +223,35 @@ def replay_job(events: Iterable[Event]) -> ReplayedJob: ) decision_id = _settle_decision(job_id, to_state, pending, event.event_id) + settled = pending if decision_id is not None else None if decision_id is not None: pending = None if to_state is JobState.APPROVED: approval_decision_id = decision_id + approval_expires_at = _expiry_of(settled) elif to_state is JobState.REJECTED: approval_decision_id = None + approval_expires_at = None # The direction lock, checked against the record rather than the engine. if to_state in POST_APPROVAL and approval_decision_id is None: raise UnauditedExecution(job_id, to_state.value, event.event_id) + # And its second half: the approval must still have been in force when the + # record says the job moved. Both timestamps are in the trail, so this is + # answerable from the log alone — which is what makes it worth checking + # here as well as in the engine. + if ( + to_state in POST_APPROVAL + and approval_expires_at is not None + and event.occurred_at >= approval_expires_at + ): + raise ExecutionAfterExpiry( + job_id, + to_state.value, + event.event_id, + expired_at=approval_expires_at.isoformat(), + occurred_at=event.occurred_at.isoformat(), + ) if to_state is JobState.AWAITING_APPROVAL: awaiting_from = state @@ -231,6 +281,7 @@ def replay_job(events: Iterable[Event]) -> ReplayedJob: state=state, awaiting_from=awaiting_from, approval_decision_id=approval_decision_id, + approval_expires_at=approval_expires_at, decision_ids=tuple(decision_ids), history=tuple(history), completed=completed, diff --git a/packages/observability/tests/test_replay.py b/packages/observability/tests/test_replay.py index f70d312..e079149 100644 --- a/packages/observability/tests/test_replay.py +++ b/packages/observability/tests/test_replay.py @@ -8,6 +8,7 @@ from __future__ import annotations import dataclasses +from datetime import datetime, timezone import pytest from devfactory_core import Event, EventType, JobState, Principal @@ -17,6 +18,7 @@ BrokenTrail, EmptyTrail, EventLog, + ExecutionAfterExpiry, IncompleteSettlement, ReplayError, UnauditedDecision, @@ -375,3 +377,119 @@ def test_replay_tenant_reads_only_the_tenant_it_names(make_job, reviewer): def test_an_unknown_tenant_replays_as_nothing(make_job): assert replay_tenant(EventLog(), "nobody") == {} + + +# ---- approval expiry, read back off the log -------------------------------- +# The engine refuses to move a job on a lapsed approval. That is a claim about the +# log too: the deadline and the moment are both recorded, so a reader who was not +# there can check it. Issue #17 · RFC-0007 Amendment 1. + +EXPIRY_PAST = datetime(2026, 8, 18, 8, tzinfo=timezone.utc) +EXPIRY_FAR_FUTURE = datetime(2027, 1, 1, tzinfo=timezone.utc) + + +def _recorded_expiry(events, expires_at): + """The same trail, with the recorded approval carrying ``expires_at``. + + Forged rather than produced, because the engine will not produce it — which is + the whole reason replay checks the claim independently. + """ + return [ + dataclasses.replace( + event, + metadata={ + **event.metadata, + "approval": {**event.metadata["approval"], "expires_at": expires_at}, + }, + ) + if event.type_value == EventType.GOVERNANCE_DECISION.value + else event + for event in events + ] + + +def test_replay_recovers_the_deadline_the_approval_was_granted_with(make_job, reviewer): + job = make_job() + job.submit_for_governance(reason="ready") + job.approve(authority=reviewer, reason="approved", expires_at=EXPIRY_FAR_FUTURE) + assert replay_job(job.events).approval_expires_at == EXPIRY_FAR_FUTURE + + +def test_an_approval_with_no_deadline_replays_as_having_none(make_job, reviewer): + job = approved(make_job(), reviewer) + assert replay_job(job.events).approval_expires_at is None + + +def test_the_deadline_survives_the_transitions_that_follow_it(make_job, reviewer): + job = make_job() + job.submit_for_governance(reason="ready") + job.approve(authority=reviewer, reason="approved", expires_at=EXPIRY_FAR_FUTURE) + job.transition(JobState.TASK_PLANNING) + seen = replay_job(job.events) + assert seen.approval_expires_at == EXPIRY_FAR_FUTURE + assert seen.state is JobState.TASK_PLANNING + + +def test_a_rejection_clears_the_deadline_along_with_the_approval(make_job, reviewer): + job = make_job() + job.submit_for_governance(reason="ready") + job.approve(authority=reviewer, reason="approved", expires_at=EXPIRY_FAR_FUTURE) + job.transition(JobState.TASK_PLANNING) + job.fail(reason="the approved plan does not work") + replacement = job.supersede(job_id="job-002") + replacement.submit_for_governance(reason="revised") + replacement.reject(authority=reviewer, reason="still wrong") + seen = replay_job(replacement.events) + assert seen.approval_decision_id is None + assert seen.approval_expires_at is None + + +def test_a_trail_that_runs_work_on_a_lapsed_approval_is_refused(make_job, reviewer): + job = approved(make_job(), reviewer) + job.transition(JobState.TASK_PLANNING) + with pytest.raises(ExecutionAfterExpiry) as excinfo: + replay_job(_recorded_expiry(job.events, EXPIRY_PAST.isoformat())) + assert excinfo.value.state == "TASK_PLANNING" + assert excinfo.value.expired_at == EXPIRY_PAST.isoformat() + + +def test_a_trail_that_stops_at_approved_replays_even_with_a_lapsed_deadline( + make_job, reviewer +): + """Holding an expired approval is not itself a defect in the log. + + The job sat in ``APPROVED`` and never used it. What the trail must not show is + the job *moving* on it — so replay accepts this and refuses the one above. + """ + seen = replay_job( + _recorded_expiry(approved(make_job(), reviewer).events, EXPIRY_PAST.isoformat()) + ) + assert seen.state is JobState.APPROVED + assert seen.approval_expires_at == EXPIRY_PAST + + +def test_the_approved_to_timed_out_edge_replays(make_job, reviewer): + """RFC-0007 Amendment 1's edge, read back off the log rather than asserted.""" + job = approved(make_job(), reviewer) + job.time_out(reason="approval_expired — the approval lapsed before planning began") + seen = replay_job(job.events) + assert seen.state is JobState.TIMED_OUT + assert seen.history[-1].from_state is JobState.APPROVED + + +@pytest.mark.parametrize( + "recorded", ["not a date", "", 17, None, "2026-08-18T08:00:00"], ids=repr +) +def test_a_deadline_replay_cannot_read_is_treated_as_absent(recorded, make_job, reviewer): + """``approval/v1`` types the field; the conformance check judges the format. + + Raising here would make this module a second schema, which RFC-0005 Rule 4 + forbids, and would turn a payload defect into a trail defect — a different + finding about a different thing. The last case is a deadline with no offset, + which names no moment that can be compared against a recorded one. + """ + job = approved(make_job(), reviewer) + job.transition(JobState.TASK_PLANNING) + seen = replay_job(_recorded_expiry(job.events, recorded)) + assert seen.approval_expires_at is None + assert seen.state is JobState.TASK_PLANNING diff --git a/rfcs/0007-job-lifecycle-completeness.md b/rfcs/0007-job-lifecycle-completeness.md index 82ddfb8..d79c6bc 100644 --- a/rfcs/0007-job-lifecycle-completeness.md +++ b/rfcs/0007-job-lifecycle-completeness.md @@ -2,6 +2,7 @@ ## Status Draft — Architecture Owner direction agreed 2026-08-17 · pending maintainer approval per `GOVERNANCE.md` +Amended 2026-08-19 — see [Amendment 1](#amendment-1--approved-may-time-out-2026-08-19). Amends [RFC-0001](0001-job-state-machine.md). Closes gaps 2, 3, and 4 of [issue #8](https://github.com/monthop-gmail/devfactory-core/issues/8) and resolves @@ -81,7 +82,10 @@ proceed. Treating the first execution failure as job failure would make | state | meaning | entered from | | --- | --- | --- | | `CANCELLED` | explicitly stopped by a principal before completion | `DRAFT`, `GOVERNANCE_ANALYSIS`, `APPROVED`, `TASK_PLANNING`, `IN_PROGRESS`, `AWAITING_APPROVAL`, `VALIDATING`, `DEPLOYABLE` | -| `TIMED_OUT` | exceeded an SLA or timeout policy without completing | `GOVERNANCE_ANALYSIS`, `TASK_PLANNING`, `IN_PROGRESS`, `AWAITING_APPROVAL`, `VALIDATING` | +| `TIMED_OUT` | exceeded an SLA or timeout policy without completing | `GOVERNANCE_ANALYSIS`, `APPROVED`, `TASK_PLANNING`, `IN_PROGRESS`, `AWAITING_APPROVAL`, `VALIDATING` | + +`APPROVED` was added to that row by [Amendment 1](#amendment-1--approved-may-time-out-2026-08-19) +on 2026-08-19; the rest of this section is as originally written. Both are terminal. Both require reason metadata, on the same rule RFC-0001 already applies to `FAILED`. @@ -132,6 +136,64 @@ machine workflow" is an architectural principle of this repository, and because `STATE_TRANSITION` events already make transitions auditable — a flag change would not be, and RFC-0003 guarantees no silent state change. +## Amendment 1 — `APPROVED` may time out (2026-08-19) + +Closes [issue #17](https://github.com/monthop-gmail/devfactory-core/issues/17), which +[RFC-0010](0010-failable-states.md) opened and decided but deliberately did not implement: +`TIMEOUTABLE` is this RFC's rule, so the amendment belongs here. + +**`APPROVED` is added to the `entered from` list for `TIMED_OUT`** in Decision 2. The +five states listed originally become six. + +### Why + +Decision 2 gave `TIMED_OUT` to the states where something is waiting, and treated +`APPROVED` as instantaneous — a job passes through it on the way to `TASK_PLANNING`. That +is true when nothing goes wrong, and a state that is only instantaneous when nothing goes +wrong is precisely the one worth a timeout. `APPROVED` had no automatic exit at all: its +only edges were `TASK_PLANNING` and `CANCELLED`, so a job whose orchestration died after +the decision was recorded sat there until a human noticed. + +The deciding argument is governance rather than liveness. **An approval must expire.** A +job that can wait indefinitely in `APPROVED` may begin executing a week later under a +verdict formed in a context that no longer holds — which is exactly what Decision 1 of +this RFC refuses when it keeps `FAILED` terminal rather than let work resume on a *stale +`APPROVED`*. That door was shut on one side and left open on the other. + +This is not a new rule so much as a gap in conforming to one we already publish. +`approval/v1` — whose semantics this repository owns — has carried `expires_at` since it +was written, and says what it means: + +> approval ที่หมดอายุแล้วใช้เดินงานไม่ได้ ต้องขอใหม่ · +> งานที่ค้างรออนุมัติจนเลยกำหนดควรเข้าสถานะ timeout ไม่ใช่รอตลอดไป + +The second half of that sentence names a destination the lifecycle did not offer. It does +now. + +### What follows + +- `TIMED_OUT` from `APPROVED` requires reason metadata like every other entry into it, and + the cause belongs in it — `approval_expired` is a different fact from `sla_exceeded`, on + the same argument RFC-0010 Decision 2 makes about `analysis_error`. +- An expired approval **may not be used to move a job into execution.** The engine refuses + it (`ExpiredApproval`), and a replay refuses a trail that shows it happening anyway + (`ExecutionAfterExpiry`) — the deadline and the moment are both recorded, so the log can + be judged against itself. Refusing is what "ใช้เดินงานไม่ได้ ต้องขอใหม่" says; tolerating + it would leave the guarantee written down and unenforced. +- `expires_at` stays **optional**, because `approval/v1` makes it optional. An approval + with no deadline never expires, and existing jobs are unaffected. Requiring one would + make this engine stricter than the contract it conforms to, which is the same line + RFC-0002's self-approval invariant is held to. + +### What this amendment does not do + +It does not set timeout **policy values** — how long an approval is good for, per job +type. That was out of scope in this RFC's Non-Goals and stayed out of scope in RFC-0010, +and it still is. Nothing in this repository sets `expires_at` or decides when a timeout +fires; orchestration does, and until it does, an approval granted without a deadline is +still an approval with no expiry date. What changes here is that the deadline now has a +meaning the engine enforces and a state to resolve into. + ## Amended job lifecycle ```text diff --git a/rfcs/0010-failable-states.md b/rfcs/0010-failable-states.md index 5d8595f..c2a90c2 100644 --- a/rfcs/0010-failable-states.md +++ b/rfcs/0010-failable-states.md @@ -126,6 +126,10 @@ which is about `FAILED`. It is also a behaviour change, whereas everything above **Tracked in [#17](https://github.com/monthop-gmail/devfactory-core/issues/17).** Not implemented in this PR by design. +*Since resolved (2026-08-19), as [RFC-0007 Amendment 1](0007-job-lifecycle-completeness.md#amendment-1--approved-may-time-out-2026-08-19) +rather than as an RFC of its own — the ownership note above says why. Migration step 6 +below reads "separate RFC" because that was the expectation when this was written.* + ## Open Question — `not_derived` changes are unversioned `semantics_version` is tied to the `frozen` block, which governs derived contracts. This RFC diff --git a/simulation/README.md b/simulation/README.md index ad70bff..617f272 100644 --- a/simulation/README.md +++ b/simulation/README.md @@ -33,6 +33,16 @@ nobody is watching. Exit code is non-zero if any check fails. | 6 | the log is complete and replays to the same state | `check_replay` | | 7 | a runnable script or a test suite | this directory — both | +One flow arrived after issue #7 was written: `approval_expired` drives a job whose +`APPROVE` lapsed where it sat, which settles at `TIMED_OUT` rather than at a +failure. It is in the run so the trail checks 5 and 6 work on covers the +`APPROVED → TIMED_OUT` edge and an approval carrying `expires_at` +([RFC-0007 Amendment 1](../rfcs/0007-job-lifecycle-completeness.md#amendment-1--approved-may-time-out-2026-08-19), +issue #17). The refusals that go with it — the engine declining to move a job on a +lapsed approval, and replay declining a trail that shows it happening — are in +`simulation/tests/test_e2e_flow.py` and `conformance/payload_check.py`, since +neither can be *produced* by a run that behaves correctly. + ## The forward path is read, not retyped `packages/core/devfactory_core/states.py` is the only place the transition table diff --git a/simulation/e2e_flow.py b/simulation/e2e_flow.py index 225fc08..38796a2 100644 --- a/simulation/e2e_flow.py +++ b/simulation/e2e_flow.py @@ -62,6 +62,7 @@ ) from simulation.flows import ( # noqa: E402 MAIN_LINE, + approval_expired, cancelled_by_a_person, failed_at, happy_path, @@ -455,6 +456,8 @@ def check_replay(jobs, log) -> None: seen.state is not job.state or seen.awaiting_from is not job.awaiting_from or seen.approval_decision_id != expected_approval + or seen.approval_expires_at + != (job.approval.expires_at if job.approval else None) or seen.tenant_id != job.tenant_id or seen.workspace_id != job.workspace_id or seen.supersedes_job_id != job.supersedes_job_id @@ -526,12 +529,20 @@ def simulate(log: EventLog) -> list[Job]: print("\n[4] governance gate — ไม่มี APPROVE record ก็ไม่มี execution") gated = check_governance_gate(new, reviewer) - # Two flows this repository already exercised in conformance, kept in the run - # so the trail the replay checks work on covers every terminal state. + # Three flows this repository already exercised in conformance, kept in the run + # so the trail the replay checks work on covers every terminal state — and, + # since RFC-0007 Amendment 1, the APPROVED -> TIMED_OUT edge and an approval + # carrying an expires_at, which check [6] then has to reconstruct. cancelled = cancelled_by_a_person(new, job_id="job-005", owner=owner) stalled = stalled_awaiting_approval(new, job_id="job-006", authority=reviewer) + expired = approval_expired( + new, + job_id="job-007", + authority=reviewer, + expires_at=datetime(2026, 8, 19, 8, tzinfo=timezone.utc), + ) - jobs = [happy, rejected, *failures, *gated, cancelled, stalled] + jobs = [happy, rejected, *failures, *gated, cancelled, stalled, expired] for job in jobs: log.extend(job.events) return jobs diff --git a/simulation/flows.py b/simulation/flows.py index e66b6b5..472b49f 100644 --- a/simulation/flows.py +++ b/simulation/flows.py @@ -195,6 +195,32 @@ def cancelled_by_a_person(new: JobFactory, *, job_id: str, owner: Principal) -> return job +def approval_expired( + new: JobFactory, *, job_id: str, authority: Principal, expires_at: datetime +) -> Job: + """An approval that ran out before the work started. + + RFC-0007 Amendment 1's characteristic stall, and the reason ``APPROVED`` is in + ``TIMEOUTABLE``: orchestration never came for the job, the approval lapsed + where it sat, and the honest terminal is ``TIMED_OUT`` rather than an + indefinite wait for someone to notice. + + ``expires_at`` is passed in rather than computed, because it has to be in the + past relative to whichever clock the caller gave the factory — the flows hold + no opinion about the clock, and a timeout policy that decided this for them + would be inventing values two RFCs deliberately left out of scope. + """ + job = new(job_id) + job.submit_for_governance(reason="ready for governance review") + job.approve( + authority=authority, + reason="scope matches milestone v0.1", + expires_at=expires_at, + ) + job.time_out(reason="approval_expired — the approval lapsed before planning began") + return job + + def stalled_awaiting_approval( new: JobFactory, *, job_id: str, authority: Principal ) -> Job: diff --git a/simulation/tests/test_e2e_flow.py b/simulation/tests/test_e2e_flow.py index 8ea98b1..f58aafc 100644 --- a/simulation/tests/test_e2e_flow.py +++ b/simulation/tests/test_e2e_flow.py @@ -15,6 +15,7 @@ import dataclasses import subprocess import sys +from datetime import datetime, timezone from pathlib import Path import pytest @@ -23,6 +24,7 @@ from devfactory_core import Job, JobState from devfactory_core.errors import ( ExecutionBeforeApproval, + ExpiredApproval, InvalidTransition, JobStateMachineError, ) @@ -30,6 +32,7 @@ from devfactory_observability import ( BrokenTrail, EmptyTrail, + ExecutionAfterExpiry, IncompleteSettlement, ReplayError, UnauditedDecision, @@ -42,6 +45,7 @@ from simulation.flows import ( MAIN_LINE, advance_to, + approval_expired, cancelled_by_a_person, failed_at, happy_path, @@ -53,6 +57,10 @@ ROOT = Path(__file__).resolve().parents[2] +#: An hour before the ``clock`` fixture starts, so an approval carrying it has +#: already lapsed by the time anything asks. +EXPIRED_AT = datetime(2026, 8, 19, 8, tzinfo=timezone.utc) + #: The flow issue #7 writes out. Compared against the table rather than used to #: drive anything — see ``test_the_forward_path_is_the_one_the_issue_asks_for``. ISSUE_7_FLOW = ( @@ -242,6 +250,66 @@ def _park_in(job: Job, state: JobState, authority) -> None: job.approve(authority=authority, reason="approved") +# ---- [3b] an approval that ran out ------------------------------------------ +# RFC-0007 Amendment 1 (issue #17). Not one of the issue's six items — the flow did +# not exist when #7 was written — but it belongs beside failure, because it is the +# other way a job settles without the work having gone wrong. + + +def test_an_approval_that_lapsed_ends_the_job_at_timed_out(new, reviewer): + job = approval_expired( + new, job_id="job-007", authority=reviewer, expires_at=EXPIRED_AT + ) + assert visited(job) == ("DRAFT", "GOVERNANCE_ANALYSIS", "APPROVED", "TIMED_OUT") + assert job.is_terminal + assert "approval_expired" in job.history[-1].reason + + +def test_the_lapsed_approval_is_still_in_the_record_that_settled_the_job( + new, reviewer +): + """The job did not fail and was not cancelled: the approval expired, and the + trail can say so — the deadline is on the decision it names.""" + job = approval_expired( + new, job_id="job-007", authority=reviewer, expires_at=EXPIRED_AT + ) + assert job.approval is not None + assert job.approval.expires_at == EXPIRED_AT + assert job.approval_expired is True + assert replay_job(job.events).approval_expires_at == EXPIRED_AT + + +def test_the_lapsed_approval_authorises_nothing_further(new, reviewer): + job = new("job-007b") + job.submit_for_governance() + job.approve(authority=reviewer, reason="approved", expires_at=EXPIRED_AT) + with pytest.raises(ExpiredApproval): + job.transition(JobState.TASK_PLANNING) + assert job.state is JobState.APPROVED + + +def test_a_trail_showing_work_on_a_lapsed_approval_is_refused_on_replay(new, reviewer): + """The engine's refusal, checked against the log by a reader who was not there.""" + job = new("job-007c") + job.submit_for_governance() + job.approve(authority=reviewer, reason="approved") + job.transition(JobState.TASK_PLANNING) + forged = [ + dataclasses.replace( + e, + metadata={ + **e.metadata, + "approval": {**e.metadata["approval"], "expires_at": EXPIRED_AT.isoformat()}, + }, + ) + if e.type_value == "GOVERNANCE_DECISION" + else e + for e in job.events + ] + with pytest.raises(ExecutionAfterExpiry): + replay_job(forged) + + # ---- [4] the governance gate ------------------------------------------------