Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions contract-semantics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 ได้
Expand All @@ -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:
Expand Down
13 changes: 7 additions & 6 deletions packages/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
14 changes: 5 additions & 9 deletions packages/core/devfactory_core/job.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/core/devfactory_core/states.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions packages/core/state-machine.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 3 additions & 0 deletions rfcs/0001-job-state-machine.md
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
180 changes: 180 additions & 0 deletions rfcs/0010-failable-states.md
Original file line number Diff line number Diff line change
@@ -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.
Loading