From 2c62514cd9d9e7c4018bfa9ee5616eb40057341b Mon Sep 17 00:00:00 2001 From: monthop-gmail Date: Thu, 20 Aug 2026 00:37:00 +0700 Subject: [PATCH] =?UTF-8?q?supersedes=5Fjob=5Fid=20=E0=B8=AD=E0=B9=89?= =?UTF-8?q?=E0=B8=B2=E0=B8=87=20terminal=20=E0=B8=97=E0=B8=B5=E0=B9=88?= =?UTF-8?q?=E0=B9=84=E0=B8=A1=E0=B9=88=E0=B9=84=E0=B8=94=E0=B9=89=E0=B8=AA?= =?UTF-8?q?=E0=B9=88=E0=B8=87=E0=B8=A1=E0=B8=AD=E0=B8=9A=E0=B8=87=E0=B8=B2?= =?UTF-8?q?=E0=B8=99=E0=B9=84=E0=B8=94=E0=B9=89=E0=B8=97=E0=B8=B8=E0=B8=81?= =?UTF-8?q?=E0=B8=95=E0=B8=B1=E0=B8=A7=20=E0=B9=84=E0=B8=A1=E0=B9=88?= =?UTF-8?q?=E0=B9=83=E0=B8=8A=E0=B9=88=E0=B9=81=E0=B8=84=E0=B9=88=20FAILED?= =?UTF-8?q?=20(#21)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit approval/v1 บอกไว้สองครึ่ง: approval ที่หมดอายุใช้เดินงานไม่ได้ · ต้องขอใหม่ ครึ่งแรกบังคับแล้วใน #17 (ExpiredApproval + APPROVED เข้า TIMEOUTABLE) ครึ่งหลัง ทำไม่ได้ — job ที่ approval หมดอายุจบที่ TIMED_OUT ซึ่งเข้า FAILED ไม่ได้ (RFC-0010) และ supersedes_job_id ตาม RFC-0007 Decision 1 ผูกไว้กับ FAILED เท่านั้น การยื่นใหม่ จึงกลายเป็น job ที่ไม่มีอะไรลิงก์กลับ และสายโซ่ audit ขาดตรงจุดที่คนตรวจจะถามพอดี RFC-0007 Amendment 2 ขยาย Decision 1 จาก FAILED เป็น terminal ทุกตัวที่จบโดย "ไม่ได้ส่งมอบงาน" — FAILED · CANCELLED · TIMED_OUT (states.SUPERSEDABLE) เจตนาของ Decision 1 คือสายโซ่ของความพยายามที่ตรวจสอบได้ ซึ่งใช้ได้กับงานที่หมดเวลา หรือถูกสั่งหยุดเหมือนกัน ข้อจำกัดที่ FAILED เป็นของแถมจากตอนเขียน ไม่ใช่ตัวข้อโต้แย้ง COMPLETED อ้างไม่ได้ · การ supersede คือการอ้างว่า "ใบนี้ไม่ได้ส่งมอบ ขอลองใหม่" งานที่ส่งมอบแล้วไม่ใช่ความพยายามที่รอความพยายามถัดไป — งานที่ต่อยอดจากมันคืองานใหม่ ที่ต้องมีเหตุผลของตัวเอง ถ้าปล่อยให้อ้างได้ สายโซ่จะเลิกแปลว่า "ความพยายาม" และกลาย เป็น pointer แบบ "เกี่ยวข้องกัน" ที่ต้องไล่ดู state ปลายทางทีละใบถึงจะรู้ว่าใบก่อนหน้า ได้ผลอะไรออกมาไหม · เหตุผลเต็มอยู่ใน Amendment 2 ไม่แตะตาราง transition เลย — ไม่มี edge ใหม่ · terminal ยังคง terminal · ไม่มีการปลุก job เดิม · ที่ผ่อนคือ "ใครถูกอ้างได้" เท่านั้น ไม่ bump semantics_version — job_state_machine อยู่ใต้ not_derived นอก subtree frozen ที่ drift_check ระบุเป็น hash scope เหมือนตอน RFC-0011 เพิ่ม edge - states.SUPERSEDABLE = TERMINAL - {COMPLETED} · Job.supersede() ตรวจกับชุดนี้ - TerminalState เลิกเสนอทาง recovery ให้ COMPLETED ซึ่งเป็นทางที่ถูกปฏิเสธ - flows.approval_expired_then_resubmitted เดินต่อจาก flow ของ #17 จนถึงการยื่นใหม่ - e2e_flow เพิ่มหมวด [3c] · replay ไล่สายโซ่จาก audit log อย่างเดียวได้ครบ - RFC-0010 กับ RFC-0011 อ้าง "FAILED" ในฐานะกฎเดิม — ใส่หมายเหตุชี้มาที่ Amendment 2 - platform-contract บันทึกช่องว่างที่ยังเหลือ: constructor รับ supersedes_job_id โดยไม่ตรวจว่าใบที่อ้างมีจริง/tenant เดียวกัน/จบแบบไม่ได้ส่งมอบจริง pytest 520 · payload_check 20 · e2e_flow 41 — FAIL=0 ทั้งหมด --- contract-semantics.yaml | 15 ++- packages/core/README.md | 20 ++++ packages/core/devfactory_core/errors.py | 16 ++- packages/core/devfactory_core/job.py | 31 ++++- packages/core/devfactory_core/states.py | 22 ++++ packages/core/state-machine.md | 45 ++++++- packages/core/tests/test_guards.py | 70 +++++++++-- packages/observability/tests/test_replay.py | 22 ++++ platform-contract.yaml | 4 + rfcs/0007-job-lifecycle-completeness.md | 109 ++++++++++++++++- rfcs/0010-failable-states.md | 10 ++ rfcs/0011-require-changes-destination.md | 5 +- simulation/e2e_flow.py | 125 ++++++++++++++++++-- simulation/flows.py | 38 ++++++ simulation/tests/test_e2e_flow.py | 107 ++++++++++++++++- 15 files changed, 604 insertions(+), 35 deletions(-) diff --git a/contract-semantics.yaml b/contract-semantics.yaml index 79bd5ab..827e475 100644 --- a/contract-semantics.yaml +++ b/contract-semantics.yaml @@ -272,15 +272,26 @@ not_derived: cancellable: [DRAFT, GOVERNANCE_ANALYSIS, APPROVED, REJECTED, TASK_PLANNING, IN_PROGRESS, AWAITING_APPROVAL, VALIDATING, DEPLOYABLE] + # rfcs/0007 Amendment 2 (2026-08-20, issue #21) — job ที่ job ใหม่อ้างถึงด้วย + # supersedes_job_id ได้ = terminal ที่จบโดย "ไม่ได้ส่งมอบงาน" (= terminal ทั้งหมด ลบ COMPLETED) + # เดิมจำกัดไว้ที่ FAILED เท่านั้น ทั้งที่เจตนาของ Decision 1 คือสายโซ่ของความพยายามที่ตรวจสอบได้ + # — job ที่หมดเวลาหรือถูกสั่งหยุดก็เป็นความพยายามที่ใช้ไปแล้วเหมือนกัน + # APPROVED ที่ approval หมดอายุจบที่ TIMED_OUT (Amendment 1) และเข้า FAILED ไม่ได้ (rfcs/0010) + # การ "ขอใหม่" ตาม approval/v1 จึงไม่มีทางลิงก์กลับเลยก่อนหน้านี้ + # COMPLETED อ้างไม่ได้ — งานที่ส่งมอบแล้วไม่ใช่ความพยายามที่รอความพยายามถัดไป + # งานที่ต่อยอดจากมันคืองานใหม่ ไม่ใช่การลองใหม่ ต้องใช้ field คนละตัวที่ยังไม่มี + # ไม่แตะตาราง transition · terminal ยังคง terminal · ไม่มีการปลุก job เดิม + supersedable: [FAILED, CANCELLED, TIMED_OUT] + invariants: - execution ห้ามเกิดก่อน APPROVED - REJECTED ไม่ terminal — กลับไป DRAFT ได้ - FAILED terminal ที่ระดับ job · retry อยู่ที่ระดับ execution เท่านั้น - - recovery ของ job ที่ FAILED = job ใหม่ที่มี supersedes_job_id ไม่ใช่ปลุกตัวเดิม + - recovery ของ job ที่จบแล้ว = job ใหม่ที่มี supersedes_job_id ไม่ใช่ปลุกตัวเดิม · job ที่อ้างถึงได้คือ terminal ที่ไม่ได้ส่งมอบงาน (ดู supersedable) · COMPLETED อ้างไม่ได้ (rfcs/0007 Amendment 2) - job ไม่เข้า FAILED เพราะ execution เดียวล้ม — orchestration ต้อง exhaust retry ก่อน - AWAITING_APPROVAL คนละอย่างกับ GOVERNANCE_ANALYSIS — อันหลังคือประตูก่อนเริ่มงาน - FAILED · CANCELLED · TIMED_OUT ต้องมี reason metadata - - job ที่ยังไม่ผ่าน APPROVED เข้า FAILED ไม่ได้ — recovery ด้วย supersedes_job_id จะไม่มีอะไรให้ supersede (rfcs/0010) + - job ที่ยังไม่ผ่าน APPROVED เข้า FAILED ไม่ได้ — ยังไม่มีงานให้ล้ม ผลลัพธ์ที่ซื่อสัตย์คือ REJECTED / CANCELLED / TIMED_OUT (rfcs/0010) - approval ที่เลย expires_at แล้วใช้เดินงานต่อไม่ได้ — engine ปฏิเสธ ไม่ใช่ปล่อยผ่าน · job ที่ค้างอยู่เข้า TIMED_OUT ได้ (rfcs/0007 Amendment 1) - REQUIRE_CHANGES กับ REJECT จบที่ DRAFT เหมือนกันแต่คนละเส้นทาง — REJECT ผ่าน REJECTED ส่วน REQUIRE_CHANGES ข้าม · แยกกันที่ trail ไม่ใช่แค่ที่ decision record (rfcs/0011) - คำถามว่า transition นี้เป็นคำตัดสินไหม ต้องอ่านที่ edge ไม่ใช่ที่ปลายทาง — REJECTED → DRAFT ไม่ใช่คำตัดสิน (rfcs/0011) diff --git a/packages/core/README.md b/packages/core/README.md index a6f9c34..6af2566 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 | +| `supersede()` on a job that has not settled, or on a `COMPLETED` one | `InvalidTransition` — an attempt that delivered is not one to retry | | `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` | @@ -152,3 +153,22 @@ expires. What changed with issue #17 is that `APPROVED` is now in `TIMEOUTABLE` 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. + +## Recovery + +`approval/v1` does not stop at "an expired approval cannot run work": it says +*ต้องขอใหม่*. Asking again is a **new job** that names the one it replaces — never a +revival, so the replacement starts at `DRAFT` and passes governance again. + +```python +expired.state # TIMED_OUT — the approval lapsed where it sat +again = expired.supersede(job_id="job-007-next") +again.supersedes_job_id # "job-007", and it is in the JOB_CREATED event +``` + +Any terminal that settled **without delivering** may be named — `states.SUPERSEDABLE`, +which is `FAILED`, `CANCELLED`, and `TIMED_OUT`. `COMPLETED` may not: +[RFC-0007 Amendment 2](../../rfcs/0007-job-lifecycle-completeness.md#amendment-2--a-job-may-supersede-any-terminal-it-did-not-deliver-from-2026-08-20) +(issue #21) widened the field from `FAILED` alone and drew the line there, because a job +that delivered is not an attempt awaiting another one — work that builds on it is new work +and wants a link that says so. diff --git a/packages/core/devfactory_core/errors.py b/packages/core/devfactory_core/errors.py index ab987f1..9f4528b 100644 --- a/packages/core/devfactory_core/errors.py +++ b/packages/core/devfactory_core/errors.py @@ -28,16 +28,22 @@ def __init__(self, current: str, requested: str, allowed: list[str]) -> None: class TerminalState(JobStateMachineError): """A transition was requested out of a terminal state. - Recovery from FAILED is a new job carrying ``supersedes_job_id``, never a - transition out of it — RFC-0007 keeps FAILED terminal so that recovery has - to pass GOVERNANCE_ANALYSIS again rather than resume under a stale APPROVED. + Recovery from a terminal state is a new job carrying ``supersedes_job_id``, + never a transition out of it — RFC-0007 keeps these states terminal so that + recovery has to pass GOVERNANCE_ANALYSIS again rather than resume under a + stale APPROVED. Amendment 2 widened which of them may be recovered from; + ``COMPLETED`` is not one, because a job that delivered has nothing to recover. """ def __init__(self, current: str) -> None: self.current = current + recovery = ( + "There is nothing to recover — it delivered its work" + if current == "COMPLETED" + else "Recovery is a new job with supersedes_job_id" + ) super().__init__( - f"{current} is terminal. Recovery is a new job with supersedes_job_id, " - f"not a transition out of {current}." + f"{current} is terminal. {recovery}, not a transition out of {current}." ) diff --git a/packages/core/devfactory_core/job.py b/packages/core/devfactory_core/job.py index ca611f6..c70401b 100644 --- a/packages/core/devfactory_core/job.py +++ b/packages/core/devfactory_core/job.py @@ -26,6 +26,11 @@ ต้องขอใหม่"* — 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). + +Asking again is :meth:`Job.supersede`, and since RFC-0007's Amendment 2 it works +from every terminal that settled without delivering — ``states.SUPERSEDABLE`` — +rather than from ``FAILED`` alone, so the second attempt at a job whose approval +ran out can say what it is a second attempt at (issue #21). """ from __future__ import annotations @@ -57,6 +62,7 @@ APPROVAL_PAUSABLE, DECISION_TARGET, POST_APPROVAL, + SUPERSEDABLE, TERMINAL, JobState, decision_for_edge, @@ -454,17 +460,32 @@ def time_out(self, *, reason: str, principal: Principal | None = None) -> Event: return self.transition(JobState.TIMED_OUT, reason=reason, principal=principal) def supersede(self, *, job_id: str, principal: Principal | None = None) -> "Job": - """Create the replacement job for a FAILED one. + """Create the replacement job for one that settled without delivering. RFC-0007: recovery is a new job, not a revival. The replacement starts at DRAFT and passes GOVERNANCE_ANALYSIS again, which is the guarantee — resuming the old one would continue under an APPROVED granted to a plan that has since failed. + + Amendment 2 widened *which* job may be pointed back at, from ``FAILED`` + alone to every terminal in ``states.SUPERSEDABLE``. An approval that + lapses lands its job in ``TIMED_OUT``, and ``approval/v1`` answers that + with "ต้องขอใหม่"; asking again is this call, and before the amendment it + had no way to say what it was a second attempt at. ``COMPLETED`` is not in + the set: a job that delivered is not an attempt awaiting another one. """ - if self._state is not JobState.FAILED: - raise InvalidTransition( - self._state.value, "supersede", ["only a FAILED job can be superseded"] - ) + if self._state not in SUPERSEDABLE: + # Two refusals with the same shape and different reasons: one job has + # not finished trying, the other has nothing left to try. + if self._state is JobState.COMPLETED: + why = ( + "COMPLETED delivered its work — what follows it is new work, " + "not another attempt at the same work" + ) + else: + settled = ", ".join(sorted(s.value for s in SUPERSEDABLE)) + why = f"a job is superseded once it has settled without delivering: {settled}" + raise InvalidTransition(self._state.value, "supersede", [why]) return Job( job_id=job_id, tenant_id=self._tenant_id, diff --git a/packages/core/devfactory_core/states.py b/packages/core/devfactory_core/states.py index 86d9596..b805c60 100644 --- a/packages/core/devfactory_core/states.py +++ b/packages/core/devfactory_core/states.py @@ -41,6 +41,28 @@ class JobState(str, Enum): {JobState.COMPLETED, JobState.FAILED, JobState.CANCELLED, JobState.TIMED_OUT} ) +#: Jobs a later job may name in ``supersedes_job_id`` — RFC-0007 Amendment 2 +#: (issue #21). Every terminal that settled **without delivering the work**. +#: +#: Decision 1 wrote the rule against ``FAILED`` because failure was the only +#: recovery it was thinking about, but what it wanted is a *chain of attempts that +#: can be audited*, and a job that ran out of time or was stopped by a person is +#: as much a spent attempt as one that broke. Amendment 1 made that concrete: an +#: approval that lapses sends its job to ``TIMED_OUT``, and ``approval/v1`` says +#: what happens next — "ต้องขอใหม่". Without this set the asking-again produced an +#: unlinked job and the chain ended where the audit question began. +#: +#: ``COMPLETED`` is excluded on purpose. Superseding is a claim that an attempt +#: did not deliver and is being tried again; a completed job delivered, and work +#: that builds on it is *new work*, not another attempt at the same work. Letting +#: it in would turn a chain of attempts into an untyped "related to" pointer, and +#: a reader walking the chain back could no longer tell whether the earlier link +#: had produced anything. +#: +#: This changes who may be *referred to*. It relaxes nothing about the transition +#: table: every terminal here is still terminal, and the old job is not woken. +SUPERSEDABLE: frozenset[JobState] = TERMINAL - {JobState.COMPLETED} + #: States from which execution has been authorised. Reaching any of these without #: passing APPROVED would violate "execution is forbidden before APPROVED". POST_APPROVAL: frozenset[JobState] = frozenset( diff --git a/packages/core/state-machine.md b/packages/core/state-machine.md index 9897ef4..7461aa2 100644 --- a/packages/core/state-machine.md +++ b/packages/core/state-machine.md @@ -39,6 +39,9 @@ AWAITING_APPROVAL → | FAILED Execution is forbidden before `APPROVED`. `FAILED` is terminal — recovery is a new job carrying `supersedes_job_id`, not a retry. +So are `CANCELLED` and `TIMED_OUT`, and since +[RFC-0007 Amendment 2](../../rfcs/0007-job-lifecycle-completeness.md#amendment-2--a-job-may-supersede-any-terminal-it-did-not-deliver-from-2026-08-20) +a new job may point back at any of the three. See [Recovery](#recovery). `CANCELLED` is reachable from every non-terminal state. `TIMED_OUT` is reachable from `GOVERNANCE_ANALYSIS`, `APPROVED`, `TASK_PLANNING`, @@ -121,10 +124,50 @@ authorised, including the way back out of a pause. What remains is a fresh `APPR [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. +Once it has settled there, *"ต้องขอใหม่"* is [Recovery](#recovery): a new job that names the +timed-out one in `supersedes_job_id` and asks for approval again. Both halves of the rule +`approval/v1` states are now reachable — the refusal and the way to ask again. + 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. +## Recovery + +A terminal job is never woken. Trying again is a **new job** that records +`supersedes_job_id` naming the one it replaces, starts at `DRAFT`, and passes +`GOVERNANCE_ANALYSIS` again — RFC-0007 Decision 1. That re-approval is the guarantee, not +the overhead: resuming the old job would continue under an `APPROVED` granted in a context +that has since stopped holding. + +Which job may be named is `states.SUPERSEDABLE` — every terminal that settled **without +delivering the work**: + +| predecessor | may be superseded | why | +| --- | --- | --- | +| `FAILED` | ✅ | the attempt broke | +| `TIMED_OUT` | ✅ | the attempt ran out of time — including an approval that lapsed where it sat | +| `CANCELLED` | ✅ | a principal stopped the attempt | +| `COMPLETED` | ❌ | it delivered; what follows is new work, not another attempt at the same work | + +`FAILED` was the only one until +[RFC-0007 Amendment 2](../../rfcs/0007-job-lifecycle-completeness.md#amendment-2--a-job-may-supersede-any-terminal-it-did-not-deliver-from-2026-08-20) +(issue #21), which left the transition table alone and relaxed only who may be *referred +to*. Before it, `approval/v1`'s *"ต้องขอใหม่"* had no way to be carried out with the chain +intact: an approval that expires sends its job to `TIMED_OUT`, `TIMED_OUT` cannot reach +`FAILED`, and so the job filed in its place was unlinked. + +```python +expired.state # TIMED_OUT — the approval lapsed before planning began +again = expired.supersede(job_id="job-007-next") +again.state # DRAFT, holding no approval +again.supersedes_job_id # "job-007" — and it is in the JOB_CREATED event, so a + # replay reads the chain back from the log alone +``` + +Superseding a `COMPLETED` job raises `InvalidTransition`, as does superseding a job that +has not settled at all. + ## AWAITING_APPROVAL Entered when at least one execution of the job is in `awaiting_approval` @@ -145,7 +188,7 @@ definitions: | `workspace_id` | ✅ | a job is always work inside one workspace | | `principal` | ✅ | who created the job | | `awaiting_from` | when `AWAITING_APPROVAL` | state to return to | -| `supersedes_job_id` | when recovering | points at the `FAILED` job this replaces | +| `supersedes_job_id` | when recovering | points at the settled job this attempt replaces — see [Recovery](#recovery) | ## Guarantees diff --git a/packages/core/tests/test_guards.py b/packages/core/tests/test_guards.py index 7b2fe1d..2a40d84 100644 --- a/packages/core/tests/test_guards.py +++ b/packages/core/tests/test_guards.py @@ -18,9 +18,10 @@ MissingAuthority, MissingPrincipal, MissingReason, + TerminalState, WrongResumeState, ) -from devfactory_core.states import POST_APPROVAL +from devfactory_core.states import POST_APPROVAL, SUPERSEDABLE, TERMINAL #: 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) @@ -218,8 +219,15 @@ def test_unanswered_approval_times_out(alice, clock): # ---- recovery, RFC-0007 ---------------------------------------------------- -def test_failed_job_is_superseded_not_revived(alice, clock): - job = drive(_fresh(alice, clock), JobState.FAILED, alice) +@pytest.mark.parametrize("terminal", sorted(SUPERSEDABLE, key=lambda s: s.value)) +def test_a_settled_job_is_superseded_not_revived(terminal, alice, clock): + """RFC-0007 Amendment 2: every terminal that did not deliver, not just FAILED. + + ``TIMED_OUT`` is the one the amendment exists for — an approval that lapses + settles there and cannot reach ``FAILED`` at all (RFC-0010), so before this the + *"ต้องขอใหม่"* half of ``approval/v1`` had no way to keep the chain. + """ + job = drive(_fresh(alice, clock), terminal, alice) replacement = job.supersede(job_id="job-002") assert replacement.state is JobState.DRAFT assert replacement.supersedes_job_id == "job-001" @@ -227,6 +235,15 @@ def test_failed_job_is_superseded_not_revived(alice, clock): assert replacement.workspace_id == job.workspace_id +def test_the_superseded_job_is_still_terminal(alice, clock): + """Amendment 2 relaxed who may be *referred to*, not what may happen to them.""" + job = drive(_fresh(alice, clock), JobState.TIMED_OUT, alice) + job.supersede(job_id="job-002") + assert job.state is JobState.TIMED_OUT + with pytest.raises(TerminalState): + job.transition(JobState.TASK_PLANNING) + + def test_replacement_must_pass_governance_again(alice, clock): job = drive(_fresh(alice, clock), JobState.FAILED, alice) replacement = job.supersede(job_id="job-002") @@ -234,12 +251,24 @@ def test_replacement_must_pass_governance_again(alice, clock): replacement.transition(JobState.TASK_PLANNING) -def test_only_a_failed_job_can_be_superseded(alice, clock): +def test_a_job_that_has_not_settled_cannot_be_superseded(alice, clock): job = drive(_fresh(alice, clock), JobState.IN_PROGRESS, alice) with pytest.raises(InvalidTransition): job.supersede(job_id="job-002") +def test_a_completed_job_cannot_be_superseded(alice, clock): + """It delivered. What follows it is new work, not another attempt — Amendment 2.""" + job = drive(_fresh(alice, clock), JobState.COMPLETED, alice) + with pytest.raises(InvalidTransition) as excinfo: + job.supersede(job_id="job-002") + assert "new work" in str(excinfo.value) + + +def test_supersedable_is_every_terminal_that_did_not_deliver(): + assert SUPERSEDABLE == TERMINAL - {JobState.COMPLETED} + + def test_supersede_records_the_link_in_the_audit_trail(alice, clock): job = drive(_fresh(alice, clock), JobState.FAILED, alice) replacement = job.supersede(job_id="job-002") @@ -356,9 +385,12 @@ def test_the_only_ways_out_of_an_expired_approval_are_the_two_terminals(alice, c ``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. + settles and a re-request is a *new* job — see + :func:`test_asking_again_after_an_approval_lapsed_keeps_the_chain`, which is + where RFC-0007 Amendment 2 made that new job able to say what it replaces. + 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()} @@ -367,6 +399,30 @@ def test_the_only_ways_out_of_an_expired_approval_are_the_two_terminals(alice, c job.transition(JobState.TASK_PLANNING) +def test_asking_again_after_an_approval_lapsed_keeps_the_chain(alice, clock): + """Both halves of *"ใช้เดินงานไม่ได้ ต้องขอใหม่"*, end to end — issue #21. + + The refusal is issue #17's; what follows it is Amendment 2's. The second + attempt names the first, re-enters at ``DRAFT``, and executes only once a + *fresh* approval has been granted — the lapsed one authorises nothing on either + job. + """ + lapsed = _approved_until(alice, clock, EXPIRY_PAST, alice) + with pytest.raises(ExpiredApproval): + lapsed.transition(JobState.TASK_PLANNING) + lapsed.time_out(reason="approval_expired — the approval lapsed before planning began") + + again = lapsed.supersede(job_id="job-002") + assert again.supersedes_job_id == lapsed.job_id + assert again.state is JobState.DRAFT and again.approval is None + + again.submit_for_governance(reason="asking again") + again.approve(authority=alice, reason="scope re-checked") + again.transition(JobState.TASK_PLANNING) + assert again.approval.decision_id != lapsed.approval.decision_id + assert lapsed.state is JobState.TIMED_OUT + + 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) diff --git a/packages/observability/tests/test_replay.py b/packages/observability/tests/test_replay.py index d668b24..d4bda53 100644 --- a/packages/observability/tests/test_replay.py +++ b/packages/observability/tests/test_replay.py @@ -74,6 +74,28 @@ def test_the_supersession_link_survives_the_round_trip(make_job, reviewer): assert replay_job(replacement.events).supersedes_job_id == "job-001" +def test_the_link_survives_when_the_predecessor_timed_out(make_job, reviewer): + """RFC-0007 Amendment 2 (issue #21) — the case the trail could not carry before. + + An approval that lapses settles its job in ``TIMED_OUT``, which cannot reach + ``FAILED``; while ``supersedes_job_id`` was reserved for ``FAILED``, asking + again as ``approval/v1`` requires produced a job the log could not connect to + anything. A reader who was not there can now follow it. + """ + lapsed = make_job() + lapsed.submit_for_governance() + lapsed.approve( + authority=reviewer, reason="approved", expires_at=datetime(2020, 1, 1, tzinfo=timezone.utc) + ) + lapsed.time_out(reason="approval_expired — lapsed before planning began") + + again = lapsed.supersede(job_id="job-002") + seen_before, seen_after = replay_job(lapsed.events), replay_job(again.events) + assert seen_after.supersedes_job_id == seen_before.job_id + assert seen_before.state is JobState.TIMED_OUT + assert seen_after.state is JobState.DRAFT + + def test_decisions_are_recovered_in_order(make_job, reviewer): job = make_job() job.submit_for_governance() diff --git a/platform-contract.yaml b/platform-contract.yaml index 754b12e..2e25d89 100644 --- a/platform-contract.yaml +++ b/platform-contract.yaml @@ -101,6 +101,10 @@ remaining: approval/v1 v1.1.0 ฝากไว้กับผู้ผลิต (ใบที่อ้างต้องมีจริง · tenant เดียวกัน · subject เดียวกัน · ห้ามชี้ตัวเอง) — ทางเดิน default ทำครบอยู่แล้ว จึงยังไม่มี payload ที่ผิด แต่ประตูนี้เปิดอยู่ + - constructor ของ Job รับ supersedes_job_id ตรง ๆ โดยตรวจแค่รูปแบบ id — ใบที่อ้างมีจริงไหม + อยู่ tenant เดียวกันไหม จบแบบไม่ได้ส่งมอบงานจริงไหม ยังไม่มีใครตรวจ (rfcs/0007 Amendment 2) + · Job.supersede() ซึ่งถือ job ทั้งสองใบอยู่ในมือบังคับกฎครบ ประตูที่เปิดคือทางที่ข้ามมันไป + · ตรวจให้ครบต้องมี registry ของ job ซึ่ง repo นี้ยังไม่มี (in-memory ทีละใบ) registration: conforming # registered | conforming diff --git a/rfcs/0007-job-lifecycle-completeness.md b/rfcs/0007-job-lifecycle-completeness.md index d79c6bc..633225d 100644 --- a/rfcs/0007-job-lifecycle-completeness.md +++ b/rfcs/0007-job-lifecycle-completeness.md @@ -3,6 +3,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). +Amended 2026-08-20 — see [Amendment 2](#amendment-2--a-job-may-supersede-any-terminal-it-did-not-deliver-from-2026-08-20). 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 @@ -64,6 +65,12 @@ level only. The recovery path for a failed job is a **new job** that records `supersedes_job_id` pointing at the failed one. +Which jobs may be pointed at was widened by +[Amendment 2](#amendment-2--a-job-may-supersede-any-terminal-it-did-not-deliver-from-2026-08-20) +on 2026-08-20 — `FAILED` becomes every terminal that did not deliver. The rest of this +section is as originally written, and the half of it that matters most is untouched: +`FAILED` is still terminal, and no job is ever woken. + This is the governance-preserving answer rather than the convenient one. Reviving a `FAILED` job would resume work under an approval that was granted against a plan which has since failed — execution continuing on a stale @@ -194,6 +201,96 @@ fires; orchestration does, and until it does, an approval granted without a dead 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. +## Amendment 2 — a job may supersede any terminal it did not deliver from (2026-08-20) + +Closes [issue #21](https://github.com/monthop-gmail/devfactory-core/issues/21), which +[Amendment 1](#amendment-1--approved-may-time-out-2026-08-19) opened by half-answering a +rule it quoted. + +**Decision 1's `supersedes_job_id` is no longer restricted to a `FAILED` predecessor.** A +new job may name any predecessor that reached a terminal state **without delivering the +work**: `FAILED`, `CANCELLED`, or `TIMED_OUT`. `COMPLETED` may not be named; see below. + +Nothing else moves. No edge is added, no state stops being terminal, and no job is woken. +What changes is *who may be referred to*, not what may happen to them. + +### Why + +`approval/v1` says an expired approval cannot be used to run work and the work has to be +asked for again — *"approval ที่หมดอายุแล้วใช้เดินงานไม่ได้ ต้องขอใหม่"*. Amendment 1 +enforced the first half: the engine refuses (`ExpiredApproval`) and the stalled job lands +in `TIMED_OUT`. The second half had nowhere to go. A job cannot leave `TIMED_OUT`, and +`supersedes_job_id` — the one field that says "this is another attempt at that" — was +reserved for `FAILED`, which `APPROVED` cannot reach at all under +[RFC-0010](0010-failable-states.md). So asking again produced a job with no link back, and +the audit trail lost the chain at exactly the point an auditor would ask about it: *why is +this the second time?* + +Decision 1 restricted the field to `FAILED` because failure was the recovery it had in +mind, not because the link means anything specific to failure. Its stated purpose is *"a +real chain of attempts"* — and a job whose approval lapsed, or that a person stopped, is +as spent an attempt as one that broke. The restriction was incidental to the argument that +produced it. + +The guarantee the restriction was protecting is a different one, and it is untouched: a +new job re-enters at `DRAFT` and passes `GOVERNANCE_ANALYSIS` again, so no work resumes +under a stale `APPROVED`. Widening who may be *cited* does not let anything be *resumed*. +If anything it tightens the guarantee's reach, since the resubmission that used to escape +as an unrelated job is now visibly the same work being tried again — under a fresh +approval, with the lapsed one still standing in the trail behind it. + +### `COMPLETED` may not be superseded + +Deliberately, and this is the part worth writing down rather than leaving to inference. + +Superseding is a claim about an attempt: *this did not deliver, here is the next try*. A +`COMPLETED` job delivered. Work that follows it — a fix, a revision, a second phase — is +**new work with its own justification**, not another attempt at work already done, and it +should enter the lifecycle saying so. + +Three things break if `COMPLETED` is admitted: + +1. **The chain stops meaning one thing.** Walk `supersedes_job_id` backwards today and + every link is an attempt that came up empty. Admit `COMPLETED` and the reader must + inspect each predecessor's terminal state to know whether it produced anything — the + field degrades from "chain of attempts" into an untyped "related to". +2. **A finished outcome becomes revisable after the fact.** A later job claiming to + supersede a `COMPLETED` one asserts that the earlier result is spent, with no decision + record anywhere saying so. Nothing in this lifecycle lets a settled outcome be + reinterpreted by a job filed afterwards, and this would. +3. **It answers a question nobody asked.** No rule in this repository or in `approval/v1` + requires a completed job to be redone. The `ต้องขอใหม่` that motivates this amendment + is about approvals that lapsed, which is the opposite case. + +What a genuine follow-up wants is a *different* link — "this builds on that" — with its own +name and its own meaning. That is Future Work below, not a reuse of this field. + +### Effect on the rest of the lifecycle + +- **RFC-0010 is not relaxed.** `FAILED` is still refused before `APPROVED`. One of + RFC-0010 Decision 2's *supporting* remarks does lose its force — "a `FAILED` job that + never passed governance would have nothing coherent to supersede" — because a job + cancelled in `DRAFT` is now a coherent thing to supersede. The decision it supports + stands on its own primary argument, which is that a job with no work under way has + nothing to fail; the transition table is unchanged either way. +- **`REJECTED` is not in the set**, because it is not terminal. A rejected job goes back to + `DRAFT` and is revised as itself — RFC-0001's answer, and a second job would be the wrong + shape for it. The same holds for a `REQUIRE_CHANGES` under + [RFC-0011](0011-require-changes-destination.md). +- **`expires_at` behaviour is unchanged.** Nothing here sets a deadline, fires a timeout, or + alters when an approval stops authorising work. +- **No `semantics_version` bump.** `contract-semantics.yaml` publishes the job state machine + under `not_derived`, outside the `frozen` subtree its own `drift_check` names as the hash + scope — the same reason RFC-0011's new edge did not move the version. + +### What this amendment does not do + +It does not check that the job being named is real, in the same tenant, or in a terminal +state at all. `supersedes_job_id` is validated for shape and recorded; the engine holds one +job in memory and cannot see the other, and inventing a registry to check it here would be +a storage decision made by accident. `Job.supersede()` — where both jobs *are* in hand — does +enforce the rule. The general case is recorded in Future Work rather than half-built. + ## Amended job lifecycle ```text @@ -228,7 +325,9 @@ documents together does not suggest a conflict. ## Open Questions from RFC-0001 — resolved **Retry semantics for `FAILED`** — resolved by Decision 1. Retry is -execution-level only; job-level recovery is a new job with `supersedes_job_id`. +execution-level only; job-level recovery is a new job with `supersedes_job_id`, which +since [Amendment 2](#amendment-2--a-job-may-supersede-any-terminal-it-did-not-deliver-from-2026-08-20) +may name any terminal that did not deliver. **Parallel task substates** — resolved by deferring to `execution/v1`. Parallel work is modelled as child executions carrying `parent_execution_id`, not as @@ -272,3 +371,11 @@ which is why the question stayed open: it was being asked at the wrong layer. - SLA and timeout policy per job type — the values, not the state. - Whether `supersedes_job_id` should carry forward artifacts from the failed attempt, or start clean. +- A link for work that *follows* a `COMPLETED` job rather than replacing it. Amendment 2 + refuses to overload `supersedes_job_id` for it; the need is real and wants a field whose + name says "builds on", with its own rules about what may be claimed. +- Whether anything should verify that the job named in `supersedes_job_id` exists, is in + the same tenant, and really settled without delivering. Amendment 2 states the rule and + `Job.supersede()` enforces it where both jobs are in hand; the general case needs a + registry this repository does not have (RFC-0006 already forbids the cross-tenant case in + words). diff --git a/rfcs/0010-failable-states.md b/rfcs/0010-failable-states.md index c2a90c2..d320ca2 100644 --- a/rfcs/0010-failable-states.md +++ b/rfcs/0010-failable-states.md @@ -77,6 +77,16 @@ is that `FAILED` keeps meaning *approved work that did not succeed*, which is wh `supersedes_job_id` recovery path in RFC-0007 assumes. A `FAILED` job that never passed governance would have nothing coherent to supersede. +> **Note added 2026-08-20.** +> [RFC-0007 Amendment 2](0007-job-lifecycle-completeness.md#amendment-2--a-job-may-supersede-any-terminal-it-did-not-deliver-from-2026-08-20) +> widened `supersedes_job_id` to every terminal that settled without delivering, so the +> last sentence above no longer tells `FAILED` apart from the alternatives — a job +> cancelled before approval is now a coherent thing to supersede. It was a supporting +> remark, not the argument: this decision rests on `FAILED` meaning *approved work that +> did not succeed*, which is unchanged, and Decision 1's rule that a job with no work +> under way has nothing to fail stands exactly as written. The transition table is +> untouched by that amendment. + **Requirement that follows from accepting this.** `TIMED_OUT` must be distinguishable by cause. The terminal-state `reason` metadata — already mandatory under RFC-0001 — must record at least `sla_exceeded` versus `analysis_error` (carrying the underlying error for the latter). Without it diff --git a/rfcs/0011-require-changes-destination.md b/rfcs/0011-require-changes-destination.md index 2476afa..482f073 100644 --- a/rfcs/0011-require-changes-destination.md +++ b/rfcs/0011-require-changes-destination.md @@ -204,7 +204,10 @@ signal of its own — remains open and this RFC does not settle it. ## Future Work - Whether a job sent back for changes should carry a pointer to the decision that sent it - back, the way `supersedes_job_id` points at a `FAILED` job. Today the link is + back, the way `supersedes_job_id` points at the job a new attempt replaces (written here + as "a `FAILED` job", which is what the field allowed until + [RFC-0007 Amendment 2](0007-job-lifecycle-completeness.md#amendment-2--a-job-may-supersede-any-terminal-it-did-not-deliver-from-2026-08-20) + widened it to every terminal that did not deliver). Today the link is reconstructible from the trail; making it a field would be an `approval/v1` change and therefore `agent-platform`'s. - Whether repeated `REQUIRE_CHANGES` rounds should be bounded, and by whom. Policy, not diff --git a/simulation/e2e_flow.py b/simulation/e2e_flow.py index 3934fe4..704f65a 100644 --- a/simulation/e2e_flow.py +++ b/simulation/e2e_flow.py @@ -13,6 +13,9 @@ [2b] being sent back for changes — the same destination by a different route, which is what RFC-0011 rests on and so is checked rather than asserted [3] failure, from every state RFC-0010 allows and from nowhere else + [3c] an approval that ran out, and the asking-again that links back to it — + the other half of ``approval/v1``'s "ต้องขอใหม่", which RFC-0007 + Amendment 2 made expressible (issue #21) [4] the governance gate — no execution without an APPROVE *record* [5] every transition leaves an audit event [6] the trail is complete and replays to the state the job is really in @@ -52,6 +55,7 @@ ExecutionBeforeApproval, InvalidTransition, JobStateMachineError, + TerminalState, ) from devfactory_core.states import ( # noqa: E402 DECISION_BY_EDGE, @@ -69,7 +73,7 @@ ) from simulation.flows import ( # noqa: E402 MAIN_LINE, - approval_expired, + approval_expired_then_resubmitted, cancelled_by_a_person, failed_at, happy_path, @@ -390,6 +394,95 @@ def _park_in(job: Job, state: JobState, authority: Principal) -> None: job.approve(authority=authority, reason="approved") +# ---- [3c] asking again after an approval ran out ---------------------------- +# RFC-0007 Amendment 2 (issue #21). ``approval/v1`` says an expired approval cannot +# run work *and* has to be asked for again. Issue #17 built the refusal; this is the +# asking, and the thing worth checking is that the second attempt is readable as a +# second attempt from the log alone rather than looking like unrelated work. + + +def check_asking_again(new, reviewer) -> list[Job]: + expired, again = approval_expired_then_resubmitted( + new, + job_id="job-007", + replacement_job_id="job-007-next", + authority=reviewer, + expires_at=datetime(2026, 8, 19, 8, tzinfo=timezone.utc), + ) + check( + "again", + expired.state is JobState.TIMED_OUT and expired.approval_expired, + "ใบเดิมจบที่ TIMED_OUT เพราะ approval หมดอายุ", + f"ใบเดิมจบที่ {expired.state.value}", + ) + check( + "again", + again.supersedes_job_id == expired.job_id + and visited(again) + == ("DRAFT", "GOVERNANCE_ANALYSIS", "APPROVED", "TASK_PLANNING"), + "ยื่นใหม่เป็น job ใหม่ที่ชี้กลับใบเดิม เริ่มที่ DRAFT และผ่านประตูอีกครั้ง", + f"ยื่นใหม่แล้วได้ {visited(again)} ชี้กลับ {again.supersedes_job_id}", + ) + check( + "again", + again.approval is not None + and expired.approval is not None + and again.approval.decision_id != expired.approval.decision_id + and again.approval.expires_at is None, + "งานที่ยื่นใหม่เดินอยู่ใต้ APPROVE ใบใหม่ ไม่ใช่ใบที่หมดอายุไปแล้ว", + "งานที่ยื่นใหม่ไม่ได้ถือ APPROVE ใบใหม่", + ) + + # The predecessor is superseded, not revived — the whole reason Decision 1 + # refuses to relax, and the half of it Amendment 2 deliberately did not touch. + try: + expired.transition(JobState.TASK_PLANNING) + fail("again", "ใบเดิมที่ TIMED_OUT ยังเดินต่อได้ — terminal ไม่ terminal จริง") + except TerminalState: + ok("again", "ใบเดิมยังเป็น terminal — ยื่นใหม่คือ job ใหม่ ไม่ใช่การปลุกใบเดิม") + + # 🔑 The chain, read back by someone who was not there: from the replacement, + # follow supersedes_job_id until it runs out, and check where it lands. + trail = EventLog() + trail.extend(expired.events) + trail.extend(again.events) + seen = replay_tenant(trail, TENANT) + chain: list[str] = [] + cursor: str | None = again.job_id + while cursor is not None and cursor not in chain: + chain.append(cursor) + cursor = seen[cursor].supersedes_job_id if cursor in seen else None + check( + "again", + chain == [again.job_id, expired.job_id] + and seen[expired.job_id].state is JobState.TIMED_OUT + and seen[again.job_id].approval_decision_id + != seen[expired.job_id].approval_decision_id, + f"replay อ่านสายโซ่ออกจาก audit log อย่างเดียว: {' → '.join(chain)} " + f"และเห็นว่าปลายสายจบที่ TIMED_OUT ด้วย approval คนละใบ", + f"replay อ่านสายโซ่ไม่ครบ: {chain}", + ) + + # COMPLETED is not in SUPERSEDABLE, and the refusal is checked rather than + # merely written down: a job that delivered is not an attempt awaiting another. + delivered = happy_path(new, job_id="job-007-done", authority=reviewer) + try: + delivered.supersede(job_id="job-007-done-again") + fail("again", "job ที่ COMPLETED ถูก supersede ได้ — สายโซ่เลิกแปลว่าความพยายาม") + except InvalidTransition: + ok("again", "COMPLETED supersede ไม่ได้ — งานที่ส่งมอบแล้วไม่ใช่ความพยายามที่รอลองใหม่") + + still_running = new("job-007-live") + still_running.submit_for_governance() + try: + still_running.supersede(job_id="job-007-live-again") + fail("again", "job ที่ยังไม่จบถูก supersede ได้") + except InvalidTransition: + ok("again", "job ที่ยังไม่ settle supersede ไม่ได้ — ยังไม่มีความพยายามที่ใช้ไปแล้ว") + + return [expired, again, delivered] + + # ---- [4] the governance gate ------------------------------------------------ @@ -614,23 +707,31 @@ def simulate(log: EventLog) -> list[Job]: print("\n[3] flow ล้มเหลว — FAILED จาก state ที่ RFC-0010 อนุญาตเท่านั้น") failures = check_failure_flow(new, reviewer) + print("\n[3c] approval หมดอายุแล้วยื่นใหม่ — สายโซ่ต้องไม่ขาด (rfcs/0007 Amendment 2)") + asked_again = check_asking_again(new, reviewer) + print("\n[4] governance gate — ไม่มี APPROVE record ก็ไม่มี execution") gated = check_governance_gate(new, reviewer) - # 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. + # Two flows this repository already exercised in conformance, kept in the run so + # the trail the replay checks work on covers every terminal state. The third — + # an approval that ran out where it sat — moved into check [3c], which now walks + # it to the end: since RFC-0007 Amendment 1 it produces the APPROVED -> TIMED_OUT + # edge and an approval carrying an expires_at for check [6] to reconstruct, and + # since Amendment 2 it also produces the supersession link out the other side. 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, sent_back, *failures, *gated, cancelled, stalled, expired] + jobs = [ + happy, + rejected, + sent_back, + *failures, + *asked_again, + *gated, + cancelled, + stalled, + ] for job in jobs: log.extend(job.events) return jobs diff --git a/simulation/flows.py b/simulation/flows.py index 1df4c89..21c6322 100644 --- a/simulation/flows.py +++ b/simulation/flows.py @@ -250,6 +250,44 @@ def approval_expired( return job +def approval_expired_then_resubmitted( + new: JobFactory, + *, + job_id: str, + replacement_job_id: str, + authority: Principal, + expires_at: datetime, +) -> tuple[Job, Job]: + """The other half of *"ต้องขอใหม่"* — asking again, with the link intact. + + :func:`approval_expired` stops where issue #17 stopped: the approval lapsed and + the job settled in ``TIMED_OUT``. ``approval/v1`` does not stop there, and + until RFC-0007 Amendment 2 this repository had no way to carry out the rest of + the sentence — ``TIMED_OUT`` cannot reach ``FAILED``, and ``supersedes_job_id`` + was reserved for ``FAILED``, so the job filed in its place was a stranger to + the one it replaced. + + So this walks all the way to the end: the replacement names its predecessor, + re-enters at ``DRAFT``, passes the gate again, and only then executes. It + carries on into ``TASK_PLANNING`` for the reason + :func:`rejected_then_resubmitted` does — reaching execution is what proves the + *second* approval is doing the authorising, rather than the lapsed first one + somehow still being in force. + + Returns both jobs, because the chain is the point and one end of it is not. + """ + expired = approval_expired( + new, job_id=job_id, authority=authority, expires_at=expires_at + ) + replacement = expired.supersede(job_id=replacement_job_id) + replacement.submit_for_governance(reason="asking again — the first approval expired") + replacement.approve( + authority=authority, reason="scope re-checked and still matches milestone v0.1" + ) + replacement.transition(JobState.TASK_PLANNING) + return expired, replacement + + 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 21605df..31d2dae 100644 --- a/simulation/tests/test_e2e_flow.py +++ b/simulation/tests/test_e2e_flow.py @@ -27,8 +27,15 @@ ExpiredApproval, InvalidTransition, JobStateMachineError, + TerminalState, +) +from devfactory_core.states import ( + DECISION_BY_EDGE, + FAILABLE, + POST_APPROVAL, + SUPERSEDABLE, + TERMINAL, ) -from devfactory_core.states import DECISION_BY_EDGE, FAILABLE, POST_APPROVAL, TERMINAL from devfactory_observability import ( BrokenTrail, EmptyTrail, @@ -46,6 +53,7 @@ MAIN_LINE, advance_to, approval_expired, + approval_expired_then_resubmitted, cancelled_by_a_person, failed_at, happy_path, @@ -420,6 +428,103 @@ def test_a_trail_showing_work_on_a_lapsed_approval_is_refused_on_replay(new, rev replay_job(forged) +# ---- [3c] asking again, with the chain intact ------------------------------- +# RFC-0007 Amendment 2 (issue #21). ``approval/v1`` asks for two things when an +# approval expires: that it stop authorising work, and that the work be asked for +# again. [3b] covers the first. Until Amendment 2 the second produced a job with no +# way to say what it was a second attempt at, because ``TIMED_OUT`` could not reach +# ``FAILED`` and ``supersedes_job_id`` was reserved for ``FAILED``. + + +@pytest.fixture +def asked_again(new, reviewer): + return approval_expired_then_resubmitted( + new, + job_id="job-007", + replacement_job_id="job-007-next", + authority=reviewer, + expires_at=EXPIRED_AT, + ) + + +def test_the_resubmission_names_the_job_it_replaces(asked_again): + expired, again = asked_again + assert expired.state is JobState.TIMED_OUT + assert again.supersedes_job_id == expired.job_id + assert visited(again) == ("DRAFT", "GOVERNANCE_ANALYSIS", "APPROVED", "TASK_PLANNING") + + +def test_the_resubmission_executes_under_a_fresh_approval(asked_again): + """Not the lapsed one, which is still standing in the predecessor's record.""" + expired, again = asked_again + assert again.approval.decision_id != expired.approval.decision_id + assert again.approval.expires_at is None + assert expired.approval.expires_at == EXPIRED_AT and expired.approval_expired + + +def test_the_predecessor_is_superseded_not_revived(asked_again): + """Amendment 2 relaxed who may be referred to, not what may happen to them.""" + expired, _ = asked_again + with pytest.raises(TerminalState): + expired.transition(JobState.TASK_PLANNING) + assert expired.state is JobState.TIMED_OUT + + +def test_replay_reads_the_whole_chain_out_of_the_log_alone(asked_again, log): + """The audit question this closes: *why is this the second time?*""" + expired, again = asked_again + for job in (expired, again): + log.extend(job.events) + seen = replay_tenant(log, TENANT) + + chain = [] + cursor = again.job_id + while cursor is not None and cursor not in chain: + chain.append(cursor) + cursor = seen[cursor].supersedes_job_id + assert chain == ["job-007-next", "job-007"] + assert seen["job-007"].state is JobState.TIMED_OUT + assert seen["job-007"].approval_expires_at == EXPIRED_AT + assert seen["job-007-next"].approval_expires_at is None + + +@pytest.mark.parametrize( + "terminal", sorted(SUPERSEDABLE, key=lambda s: s.value), ids=lambda s: s.value +) +def test_every_terminal_that_did_not_deliver_can_be_superseded(terminal, new, reviewer): + job = _settled_in(new(f"job-007-{terminal.value.lower().replace('_', '-')}"), terminal, reviewer) + replacement = job.supersede(job_id=f"{job.job_id}-next") + assert replacement.state is JobState.DRAFT + assert replacement.supersedes_job_id == job.job_id + + +def test_a_completed_job_is_not_an_attempt_awaiting_another(new, reviewer): + delivered = happy_path(new, job_id="job-007-done", authority=reviewer) + assert delivered.state is JobState.COMPLETED + with pytest.raises(InvalidTransition): + delivered.supersede(job_id="job-007-done-again") + + +def test_a_job_that_has_not_settled_cannot_be_superseded(new, reviewer): + running = advance_to(new("job-007-live"), JobState.IN_PROGRESS, authority=reviewer) + with pytest.raises(InvalidTransition): + running.supersede(job_id="job-007-live-next") + + +def _settled_in(job: Job, terminal: JobState, authority): + """Drive a fresh job to one of the terminals a replacement may name.""" + if terminal is JobState.CANCELLED: + job.submit_for_governance() + job.cancel(reason="no longer wanted", principal=authority) + return job + advance_to(job, JobState.IN_PROGRESS, authority=authority) + if terminal is JobState.FAILED: + job.fail(reason="orchestration exhausted execution retries") + else: + job.time_out(reason="sla_exceeded — nobody answered") + return job + + # ---- [4] the governance gate ------------------------------------------------