diff --git a/contract-semantics.yaml b/contract-semantics.yaml index 7659efe..c656a0b 100644 --- a/contract-semantics.yaml +++ b/contract-semantics.yaml @@ -170,6 +170,7 @@ not_derived: source: - rfcs/0001-job-state-machine.md - rfcs/0007-job-lifecycle-completeness.md # amend 0001 + - rfcs/0010-failable-states.md # ระบุว่า state ไหนไป FAILED ได้ note: >- ให้ vocabulary ไว้เพื่อให้ execution/v1 อ้างความหมายของ job state ได้ โดยไม่ต้องคาดเดา — คำเตือน external-authority-pending ใน execution.schema.yaml @@ -189,6 +190,37 @@ not_derived: - CANCELLED - TIMED_OUT terminal: [COMPLETED, FAILED, CANCELLED, TIMED_OUT] + + # เส้นทางปกติ — ไม่รวมทางออกที่ตัดขวางทุก state (cancellable/timeoutable/failable ด้านล่าง) + # ไม่ลง state ที่อยู่ใน terminal เพราะไม่มี edge ออก — ดู terminal ด้านบน + progression: + DRAFT: [GOVERNANCE_ANALYSIS] + GOVERNANCE_ANALYSIS: [APPROVED, REJECTED] + APPROVED: [TASK_PLANNING] + REJECTED: [DRAFT] + TASK_PLANNING: [IN_PROGRESS] + IN_PROGRESS: [VALIDATING, AWAITING_APPROVAL] + VALIDATING: [DEPLOYABLE, AWAITING_APPROVAL] + DEPLOYABLE: [COMPLETED, AWAITING_APPROVAL] + AWAITING_APPROVAL: [] # ทางออกเป็น awaiting_from — ดูด้านล่าง ไม่ใช่ dead end + + # ทางออกของ AWAITING_APPROVAL เป็นกฎ ไม่ใช่ edge — ค่าต่างกันต่อ job จึงเขียนเป็นตารางไม่ได้ + awaiting_from: >- + job ที่เข้า AWAITING_APPROVAL ต้องพก awaiting_from เสมอ (RFC-0007) และออกกลับไปที่ state นั้น + consumer ที่เห็น progression.AWAITING_APPROVAL = [] ต้องไม่สรุปว่าเป็น terminal + + # rfcs/0010 — FAILED ไปถึงได้จาก 5 state ที่มีงานให้ล้มเท่านั้น + # ก่อน 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 — หยุดงานได้ทุกจุดก่อนที่มันจะ settle (= states ทั้งหมด ลบ terminal) + # เขียนเป็น list เหมือน failable/timeoutable เพื่อให้เครื่องที่อ่านสามคีย์นี้ parse ได้แบบเดียวกัน + cancellable: [DRAFT, GOVERNANCE_ANALYSIS, APPROVED, REJECTED, TASK_PLANNING, IN_PROGRESS, + AWAITING_APPROVAL, VALIDATING, DEPLOYABLE] + invariants: - execution ห้ามเกิดก่อน APPROVED - REJECTED ไม่ terminal — กลับไป DRAFT ได้ @@ -197,6 +229,7 @@ not_derived: - 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) layering: job (ที่นี่) > execution (execution/v1) > step (event/) orchestration_execution_boundary: diff --git a/packages/core/README.md b/packages/core/README.md index 7a7d7cc..6cda0b6 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -77,11 +77,12 @@ python -m pytest # 235 tests python -m pytest --cov=devfactory_core # coverage gate at 90%, currently 100% ``` -## Open question +## Which states may fail -`state-machine.md` says `FAILED` is terminal and lists `AWAITING_APPROVAL -> FAILED`, -but never enumerates which other states may fail. This module permits `FAILED` from +Settled by [RFC-0010](../../rfcs/0010-failable-states.md): `FAILED` is reachable from `TASK_PLANNING`, `IN_PROGRESS`, `AWAITING_APPROVAL`, `VALIDATING`, and `DEPLOYABLE` — -the states where work exists to fail — and refuses it before `APPROVED`, where the -honest outcomes are `REJECTED`, `CANCELLED`, or `TIMED_OUT`. That reading needs -confirming in an RFC; see `states.FAILABLE`. +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. diff --git a/packages/core/devfactory_core/job.py b/packages/core/devfactory_core/job.py index c18fd84..f0f143c 100644 --- a/packages/core/devfactory_core/job.py +++ b/packages/core/devfactory_core/job.py @@ -1,19 +1,15 @@ """The in-memory job state machine. Canonical spec: ``packages/core/state-machine.md`` — RFC-0001 as amended by -RFC-0007, with the tenant model from RFC-0006. +RFC-0007 and RFC-0010, with the tenant model from RFC-0006. Scope, per issue #2: in memory, no persistence, no policy engine, no API. What this module owns is the lifecycle and the guards on it. -Open question surfaced by implementing this -------------------------------------------- -``state-machine.md`` says FAILED is terminal and lists ``AWAITING_APPROVAL -> -FAILED``, but never enumerates which other states may fail. This module permits -FAILED from TASK_PLANNING, IN_PROGRESS, AWAITING_APPROVAL, VALIDATING, and -DEPLOYABLE — the states where work exists to fail — and refuses it before -APPROVED, where the honest outcomes are REJECTED, CANCELLED, or TIMED_OUT. -That reading needs confirming in an RFC; see ``states.FAILABLE``. +Which states may fail is settled by RFC-0010: FAILED from TASK_PLANNING, +IN_PROGRESS, AWAITING_APPROVAL, VALIDATING, and DEPLOYABLE only — the states +where work exists to fail — and refused before APPROVED, where the honest +outcomes are REJECTED, CANCELLED, or TIMED_OUT. See ``states.FAILABLE``. """ from __future__ import annotations diff --git a/packages/core/devfactory_core/states.py b/packages/core/devfactory_core/states.py index 771b927..6b2f6ef 100644 --- a/packages/core/devfactory_core/states.py +++ b/packages/core/devfactory_core/states.py @@ -68,7 +68,7 @@ class JobState(str, Enum): #: A job fails only where work exists to fail. Before APPROVED nothing is #: executing, so the honest outcomes there are REJECTED, CANCELLED, or TIMED_OUT. -#: See "Open question" in the module docstring of ``job.py``. +#: Settled by RFC-0010. FAILABLE: frozenset[JobState] = frozenset( { JobState.TASK_PLANNING, diff --git a/packages/core/state-machine.md b/packages/core/state-machine.md index f4114cb..88ff9d5 100644 --- a/packages/core/state-machine.md +++ b/packages/core/state-machine.md @@ -42,6 +42,10 @@ Execution is forbidden before `APPROVED`. `CANCELLED` is reachable from every non-terminal state. `TIMED_OUT` is reachable from `GOVERNANCE_ANALYSIS`, `TASK_PLANNING`, `IN_PROGRESS`, `AWAITING_APPROVAL`, and `VALIDATING`. +`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` +([RFC-0010](../../rfcs/0010-failable-states.md)). ## AWAITING_APPROVAL diff --git a/rfcs/0001-job-state-machine.md b/rfcs/0001-job-state-machine.md index 8d7ba7f..c46716b 100644 --- a/rfcs/0001-job-state-machine.md +++ b/rfcs/0001-job-state-machine.md @@ -7,6 +7,9 @@ Draft `TIMED_OUT`, and `AWAITING_APPROVAL`, and resolves both Open Questions below. Read the two together; RFC-0007 supersedes this document where they differ. +**Further amended by [RFC-0010](0010-failable-states.md)** — enumerates which states may +reach `FAILED`, which neither this document nor RFC-0007 stated. + ## Context devfactory-core is a governance-first control plane. A deterministic job lifecycle is required to ensure governance, diff --git a/rfcs/0010-failable-states.md b/rfcs/0010-failable-states.md new file mode 100644 index 0000000..5d8595f --- /dev/null +++ b/rfcs/0010-failable-states.md @@ -0,0 +1,180 @@ +# RFC-0010: Which States May Reach `FAILED` + +## Status +Draft — proposed 2026-08-19 · pending maintainer approval per `GOVERNANCE.md` + +Amends [RFC-0001](0001-job-state-machine.md) as already amended by +[RFC-0007](0007-job-lifecycle-completeness.md). +Closes [issue #14](https://github.com/monthop-gmail/devfactory-core/issues/14). + +## Context + +`packages/core/state-machine.md` states that `FAILED` is terminal and lists exactly one +edge into it — `AWAITING_APPROVAL -> FAILED`. It never enumerates which other states may +fail. RFC-0001 does not say. RFC-0007 says *when* a job should fail (orchestration has +exhausted execution-level retries) but not *from where*. + +Implementing the state machine in [PR #13](https://github.com/monthop-gmail/devfactory-core/pull/13) +forced the question, because a transition table cannot be built without an answer. That PR +shipped `states.FAILABLE` with a reading marked as needing confirmation, and left an +"Open question" note in `job.py`, `packages/core/README.md`, and `states.py`. + +Two consequences follow from leaving it unconfirmed, and both are already live on `main`: + +1. The code has made an architectural decision without an RFC, which `CONTRIBUTING.md` + requires for lifecycle changes. +2. `contract-semantics.yaml` publishes `job_state_machine` under `not_derived` for + `agent-platform` to reference. It declares `states`, `terminal`, `invariants`, and + `layering` — but no transitions and no `failable`. A consumer reading the manifest to + understand job semantics cannot see the rule the code enforces, which sits badly with + the `event/v1` guarantee of *no silent state change*. + +## Problem Statement + +Which of the thirteen job states may transition to `FAILED`? + +## Decision 1 — `FAILED` is reachable only from the five post-approval working states + +```text +TASK_PLANNING · IN_PROGRESS · AWAITING_APPROVAL · VALIDATING · DEPLOYABLE +``` + +and is **refused** from: + +```text +DRAFT · GOVERNANCE_ANALYSIS · APPROVED · REJECTED +``` + +This ratifies what `states.FAILABLE` already implements. No code change. + +**Rationale.** A job fails where work exists to fail. Before `APPROVED` nothing is +executing — RFC-0001's own invariant forbids it — so there is no execution whose failure +`FAILED` could describe. The honest outcomes before approval are already covered and are +each distinct: + +| outcome | means | +| --- | --- | +| `REJECTED` | governance decided no — a verdict, not a malfunction | +| `CANCELLED` | a human stopped it | +| `TIMED_OUT` | nobody answered in time | + +Allowing `FAILED` before `APPROVED` would let these three collapse into one bucket, which +is the same argument RFC-0007 used to introduce `CANCELLED` and `TIMED_OUT` in the first +place. Preserving the distinction is the point. + +`AWAITING_APPROVAL` is included because the executions underneath it are real work that can +fail while the job waits, and `state-machine.md` already listed that edge. + +## Decision 2 — a `GOVERNANCE_ANALYSIS` malfunction resolves as `TIMED_OUT`, deliberately + +If the governance analyzer itself errors — an infrastructure fault, not a verdict — none of +`REJECTED`, `CANCELLED`, or `FAILED` is available under Decision 1. The reachable terminal +is `TIMED_OUT`, since `GOVERNANCE_ANALYSIS` is in `TIMEOUTABLE`. + +**This is accepted, not overlooked.** A job that crashed during analysis waits for its +timeout instead of settling immediately. The cost is latency to terminal state; the benefit +is that `FAILED` keeps meaning *approved work that did not succeed*, which is what the +`supersedes_job_id` recovery path in RFC-0007 assumes. A `FAILED` job that never passed +governance would have nothing coherent to supersede. + +**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 +the audit trail cannot tell *the job was too slow* from *the analyzer broke*, which is exactly the +distinction RFC-0007 added `TIMED_OUT` and `CANCELLED` to preserve. This needs no new state and no +change to the transition table. + +**Operational note.** Because a job that crashes in the first second still waits out its SLA, the +timeout for `GOVERNANCE_ANALYSIS` should be set materially shorter than for states where work is +genuinely running. Values remain out of scope (see Non-Goals); the recommendation is not. + +**Retry belongs to orchestration, not to this table.** RFC-0004 and RFC-0007 Decision 1 already +hold that *"a job does not enter `FAILED` because one execution failed"* — orchestration owns +retry. One analyzer fault should therefore not move the job at all. `TIMED_OUT` is reached only +when retries are exhausted or orchestration itself is gone, and at that point it states the literal +truth: nothing moved this job within its allotted time. The terminal is correct, not merely the +last one available. + +Recorded here explicitly so a future reader does not mistake it for an omission. + +## Non-Goals + +Job-level timeout **policy values** stay out of scope, exactly as RFC-0007 left them. +This RFC only states which terminal a malfunction resolves into. + +## Decided — `APPROVED` will be added to `TIMEOUTABLE` (tracked separately) + +`APPROVED` is in neither `FAILABLE` nor `TIMEOUTABLE`. Its only exits are `TASK_PLANNING` +and `CANCELLED`. If orchestration dies after the approval is recorded but before planning +starts, the job stays in `APPROVED` indefinitely and only a human cancelling it will move +it. It is the one state in the lifecycle that can stall with no automatic exit at all. + +Treating `APPROVED` as instantaneous is a reasonable reading — that is presumably why it was +left out of `TIMEOUTABLE` — but a state that is only instantaneous when nothing goes wrong +is precisely the one worth a timeout. + +**Decision (2026-08-19):** yes — `APPROVED` is to be added to `TIMEOUTABLE`. The deciding +argument is governance rather than liveness: **an approval must expire.** An `APPROVED` job that +can wait indefinitely may begin executing a week later under a verdict formed in a context that no +longer holds — the very thing RFC-0007 Decision 1 refuses when it forbids reviving a `FAILED` job +(*"execution continuing on a stale APPROVED"*). That door is currently shut on one side and left +open on the other. + +Ownership note: `TIMEOUTABLE` is RFC-0007's rule, so the amendment belongs there, not in this RFC, +which is about `FAILED`. It is also a behaviour change, whereas everything above is ratification. + +**Tracked in [#17](https://github.com/monthop-gmail/devfactory-core/issues/17).** Not implemented +in this PR by design. + +## Open Question — `not_derived` changes are unversioned + +`semantics_version` is tied to the `frozen` block, which governs derived contracts. This RFC +adds declarations under `not_derived.job_state_machine`, which no consumer derives from but +`agent-platform` is invited to reference. + +Nothing today tells a consumer that block changed. Bumping `semantics_version` would signal +it, but at an immediate cross-repo cost: `agent-platform` pins the file-level +`semantics_version` in every `derived_from`, so a bump turns its drift check red until it +re-pins — for a change to a block it does not derive from. + +**No bump in this change**, on the grounds that the `frozen` scope is untouched. Whether +`not_derived` deserves a signal of its own is left open. + +## Architectural Impact + +- **Control Plane** — no behavioural change. The rule already in `states.FAILABLE` gains the + RFC that `CONTRIBUTING.md` requires, and `contract-semantics.yaml` publishes it. +- **Orchestration** — unchanged. RFC-0007 already owns *when* to report job failure; this + RFC only bounds *from where*. +- **Execution** — no change. +- **Observability** — `STATE_TRANSITION` consumers can now validate transitions against a + declared table instead of inferring one, which is what *no silent state change* needs in + order to be checkable by anyone other than this repository. + +## Risk Assessment + +| risk | severity | mitigation | +| --- | --- | --- | +| A real infrastructure failure during `GOVERNANCE_ANALYSIS` looks like a timeout in the audit log | medium | Accepted in Decision 2; `TIMED_OUT` requires reason metadata already, so the cause is recorded even though the state is shared | +| `APPROVED` stalls unnoticed | medium | Open Question above; unchanged from today's behaviour, now written down instead of implicit | +| Manifest and code drift apart again | medium | The declarations added here are mechanically comparable to `states.py`; a conformance check that compares them is cheap follow-up work | +| Ratifying the implementation makes the RFC a rubber stamp | low | Decision 1's rationale stands independently of the code, and Decisions 2 and the two Open Questions are new findings that implementing surfaced | + +## Migration Plan + +1. Accept this RFC. +2. `contract-semantics.yaml` records `progression`, `awaiting_from`, `failable`, + `timeoutable`, and `cancellable` under `not_derived.job_state_machine`. + **Included in this change.** +3. `packages/core/state-machine.md` enumerates the `FAILED` edges instead of listing one. + **Included in this change.** +4. The "Open question" notes in `job.py`, `packages/core/README.md`, and `states.py` are + replaced with a pointer to this RFC. **Included in this change.** +5. RFC-0001 gains an amendment pointer. **Included in this change.** +6. `APPROVED` in `TIMEOUTABLE` — separate issue, separate RFC. + +## Future Work + +- A conformance check that `contract-semantics.yaml` still matches `states.py`, so the two + cannot drift silently the way they did between PR #13 and this RFC. +- Whether `not_derived` needs a version signal of its own.