From 19d3cd2b1def4654bd5803b141a6c3bd6689e267 Mon Sep 17 00:00:00 2001 From: sami Date: Thu, 10 Sep 2026 00:00:08 +0400 Subject: [PATCH 1/3] =?UTF-8?q?daemon:=20draft=20ADR=200013=20=E2=80=94=20?= =?UTF-8?q?task=20state-machine=20ownership=20(D1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drafts the CORE/DAEMON split PROTO raised as D1 and CORE already agreed to in contracts/proto-answers-m1.md, since neither lane would write it down alone. DAEMON owns new/queued and pause-for-schedule; CORE owns probing through complete|failed and cancelled-from-anywhere is DAEMON-driven; paused is shared. Ties into ADR 0011 in two places: - retry_wait looks identical to segment starvation from the budget accessor's point of view (zero segments, deliberately) and must be excluded from tasks_starved by construction, not by DAEMON guessing from timing. - restart handling: CORE holds no persistent state, so any CORE-owned TaskState reloads as queued and re-admits through the scheduler; paused tasks reload with their pauseReason intact. Surfaces one real contract gap while drafting, not just an open question: event.task.state's error field is schema-scoped to failed/retry_wait only, so CORE auto-pausing for auth_required or server_file_changed currently has no wire signal telling DAEMON why — needed before the resume-must-not-cross-reasons rule (§3) can be implemented at all. Filed as a PROTO follow-up in the ADR itself. Status: proposed, needs CORE + PROTO sign-off (four open items at the end) before daemon/src/sched/'s pause/resume logic is written against it. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Upd9WhG9oppieig5nRDLig --- docs/adr/0013-task-state-machine-ownership.md | 204 ++++++++++++++++++ 1 file changed, 204 insertions(+) create mode 100644 docs/adr/0013-task-state-machine-ownership.md diff --git a/docs/adr/0013-task-state-machine-ownership.md b/docs/adr/0013-task-state-machine-ownership.md new file mode 100644 index 0000000..09b1736 --- /dev/null +++ b/docs/adr/0013-task-state-machine-ownership.md @@ -0,0 +1,204 @@ +# ADR 0013 — Task state-machine ownership: CORE, DAEMON, and the shared `paused` + +**Status:** proposed · **Date:** 2026-09-09 · **Lane:** DAEMON, drafted at PROTO's request +(D1 in `core/docs/proto-requests-m1.md`), for CORE and PROTO to review and sign off. +**Needs:** explicit accept from CORE and PROTO before `daemon/src/sched/` calls into CORE's +task-start API, since the calling convention below is derived from this split. + +## Context + +`docs/04` §1 hands CORE the download lifecycle (`probing → connecting → downloading ⇄ +paused → retry_wait → assembling → verifying → complete | failed`), but `queued` and every +scheduler transition are DAEMON's, and neither brief says where the seam is. PROTO raised +this as D1 and proposed a split; CORE agreed (`contracts/proto-answers-m1.md` D1); both +declined to write it down alone — a two-lane decision recorded by the lane that owns +neither half is exactly the failure mode `contracts/README.md` rule 4 exists to prevent. +DAEMON drafts it because DAEMON is the one lane touching both halves (it owns the SQL row +and the RPC surface; it calls into CORE). + +The wire side is already frozen and does not need to change: `TaskState` +(`contracts/schema/types/TaskState.schema.json`) has the twelve values with no field +naming who drove a transition, and `event.task.state` carries `previousState` — a client +renders what it's told and keeps no state machine of its own. This ADR is about who +*decides* a transition, not the wire shape of one. + +This is also where ADR 0011 (admission control) and the state machine meet: `queued → +connecting` is the admission event, and `retry_wait` looks identical to starvation from +inside the segment-budget accounting. Both are addressed below because getting them wrong +independently produces the same false alarm from two different directions. + +## Decision + +### 1. Ownership, by state + +| State | Driven by | Entered from | Notes | +|---|---|---|---| +| `new` | DAEMON | (creation) | `download.add` persists the row before CORE knows the task exists. | +| `queued` | DAEMON | `new`, `paused` (resume, subject to §3) | Scheduler admission pool. CORE has no concept of "queued" — `retry_wait` re-enters `connecting` directly inside CORE and never passes back through `queued`. | +| `probing` | CORE | `queued` | DAEMON calls CORE's `start()` once admitted (ADR 0011 §1); CORE owns everything from here until it hands back a terminal or `paused` state. | +| `connecting` | CORE | `probing`, `retry_wait` | | +| `downloading` | CORE | `connecting` | | +| `paused` | **shared** | any CORE state, `queued` | See §2 — this is the one state either side may enter unilaterally. | +| `retry_wait` | CORE | `connecting`, `downloading` | CORE's own backoff timer (`docs/04` §7); DAEMON does not schedule retries. | +| `assembling` | CORE | `downloading` | Normally a no-op rename; a real state for HLS/DASH muxing. | +| `verifying` | CORE | `assembling` | Checksum verification when requested. | +| `complete` | CORE | `verifying` | Terminal. | +| `failed` | CORE | any CORE state | Terminal. `max_retries_exhausted` after `retry_wait`, or any non-retryable error. | +| `cancelled` | DAEMON | any state | Terminal. Always user- or policy-initiated (`download.cancel`, category/queue removal) — CORE never cancels on its own; it only executes the teardown DAEMON asked for and confirms. | + +Reading the table as a picture: + +``` +DAEMON: new ──▶ queued ──▶[admit]──▶ (hand to CORE) + ▲ │ + │ resume CORE: probing ─▶ connecting ─▶ downloading + pause────┤ │ │ ▲ │ │ + (either side)│ retry_wait◀────────┘ │ │ │ + ▼ │ (backoff done)│ ▼ ▼ + paused ◀──────────(auto-pause)─────┘ assembling paused + │ (auto) + verifying + │ + complete + (from anywhere, DAEMON-driven) ──────────────────────▶ cancelled + (from any CORE state, CORE-driven, non-retryable) ───▶ failed +``` + +### 2. `paused` is shared, and idempotency is the whole contract + +Both sides can put a task in `paused`, for disjoint reasons: + +- **DAEMON-initiated**: user clicks pause, a schedule window closes, a queue is stopped, + `Queue.onComplete`, or the admission governor reconciling a lowered `maxActiveSegments` + (ADR 0011 §2 — pausing the lowest-priority excess). DAEMON calls CORE's `pause(TaskId)`. +- **CORE-initiated ("auto-pause")**: `auth_required` (401/407, `docs/04` §7), + `server_file_changed` (F1's carrier, `contracts/proto-answers-m1.md`), disk full. CORE + transitions to `paused` on its own and reports it up through the existing + state-change callback — the same path every other CORE-driven transition uses. This is + not a new mechanism. + +The contract that makes this safe: + +- **`pause()` is idempotent.** DAEMON calling `pause()` on a task CORE already + auto-paused is a no-op, not an error — DAEMON does not need to know CORE got there + first. Symmetrically, if CORE were ever mid-transition to `paused` when DAEMON's pause + request arrives, the result is still `paused`, once. +- **DAEMON persists *why* a task is paused**, in the `tasks` table, not in `TaskState` + itself (the wire type stays a flat enum — this is DAEMON-local bookkeeping, not a + contract change). A `pauseReason` distinguishing at least `user`, `schedule`, + `queue_stopped`, `admission_reconcile`, and CORE's `error.code` when auto-paused. This + is what makes §3's resume rule possible — but it needs a contract fix first: see the + gap in open item 3 below. `error.code` is not currently carried on a transition into + `paused` at all. +- **CORE does not need to track why it's paused past the current occurrence.** Once + paused, CORE's job is done; DAEMON is the only side that later decides whether to + resume, and DAEMON is also the only side with persistent storage to remember the + reason across a restart. + +### 3. Resume must not cross reasons + +The bug this section exists to prevent: a schedule window opens, DAEMON blindly resumes +every `paused` task in that queue, and it resumes a task CORE paused because it's waiting +on credentials that were never provided. The task immediately re-fails or re-pauses, looks +like a flapping bug, and burns a retry. + +Rule: **DAEMON resumes a task only when the reason it recorded matches the event that +justifies resuming.** A schedule window opening resumes tasks paused for `schedule`. A +user clicking "resume" resumes anything (explicit user intent overrides any reason). +`auth_required` and `server_file_changed` are resumed only by the paths that actually +address them — `download.provideAuth` (F2) and the user's restart-decision response (F1) — +never by the scheduler. This means `queued` is not the only state a schedule can put a +task back into a run cycle from; the scheduler's "should this task be running right now" +check must skip tasks paused for a reason it doesn't own. + +### 4. `retry_wait` is not starvation + +ADR 0011 §3.6 defines a starved task as a running task with `segments_active(id) == 0`, +and asserts `tasks_starved == 0` in steady state past a bounded delay. A task in +`retry_wait` also holds zero segments, deliberately, for up to 60 s (`docs/04` §7's backoff +cap) — and it is **not** admission-starved, it is CORE's own policy holding it idle. + +Resolution: `retry_wait` must be distinguishable from true starvation without DAEMON +guessing from timing. CORE's `starved_tasks()` (ADR 0011) excludes any task CORE itself +holds at zero by policy — `retry_wait` and CORE-initiated `paused` both fall outside +`tasks_starved` by construction, because that accessor is about the segment allocator +failing to grant a slot to a task that wants one, not about a task that isn't asking. A +task in `retry_wait` still counts against `connection.maxConcurrentDownloads` from +DAEMON's side (it is running, not requeued) but contributes nothing to the segment +allocator's guarantee pass until it re-enters `connecting`. + +### 5. Restart — CORE holds no persistent state, DAEMON reloads to `queued` + +Per the M1 DoD ("kill and restart the daemon mid-download: all tasks reload with correct +state and resume") and the layering rule (CORE has no SQL, no state survives a CORE +restart except what's on disk in `.veloxpart.meta`, which CORE reads back itself): on +daemon startup, DAEMON loads every non-terminal task from SQLite. Any task whose persisted +`TaskState` was a CORE-owned state (`probing` through `verifying`) is **rewritten to +`queued`** in memory before the scheduler sees it — the on-disk `TaskState` is a +last-known-value, not a resumable position, because CORE's in-process state died with the +process. The scheduler re-admits it exactly like any other queued task; CORE re-derives +where to actually resume from `.veloxpart.meta` and re-validates with `If-Range` +(`docs/04` §5), independent of what SQL said the state was. + +`paused` tasks reload as `paused`, with their `pauseReason` intact, and are not +auto-admitted — §3 applies identically after a restart as it does live. + +## Consequences + +- `daemon/src/sched/` calls `start(TaskId)` exactly once per admission (`queued → + probing`), never re-enters a CORE-owned state directly, and treats every CORE-owned + state as opaque past that call except for reading it back for projection. +- The `tasks` table needs a `pauseReason` column (DAEMON-local; not a wire type) before + `sched/`'s pause/resume logic can be written correctly — flagging as a concrete + follow-up, not blocking this ADR's acceptance. +- CORE's `starved_tasks()` (ADR 0011) must exclude `retry_wait` and CORE-auto-paused tasks + by construction; if that isn't already true in CORE's accessor, it needs to be before + DAEMON relies on the starvation invariant, since otherwise every backoff cycle would + trip DAEMON's governor-invariant warning as a false positive. +- No `contracts/` change. `TaskState`, `event.task.state`, and `previousState` are already + sufficient; this ADR is entirely about which process calls which function when. + +## Alternatives considered + +**A single owner drives every transition (CORE, told about queues).** Rejected — this is +the D1 problem restated with CORE holding the SQL-shaped concepts (`queued`, schedules, +priority) that the layering rule (`CLAUDE.md` §3) forbids it from touching. + +**DAEMON drives every transition, treating CORE as a dumb byte-mover.** Rejected — CORE's +internal states (`retry_wait`, `assembling`, `verifying`) depend on engine internals +(backoff timers, mux completion, streaming hash state) DAEMON has no visibility into +without CORE reporting them; forcing DAEMON to poll or reimplement that timing duplicates +`docs/04` §7 in two places and they will drift. + +**A `pauseReason` on the wire (`TaskState` split into `paused_user` / `paused_auto` / +etc.).** Rejected — it roughly doubles the enum for a fact only DAEMON's resume logic +needs, and adding wire cardinality for internal bookkeeping is the kind of thing that +becomes a compatibility problem the moment a client starts branching on it. The value +DAEMON needs (CORE's reason) can travel on the existing `error` field instead — see the +contract gap in open item 3 — without touching `TaskState` itself. + +## A contract gap this ADR surfaces, not just an open question + +`event.task.state.schema.json`'s own description scopes `error` to "whenever the new +state is failed or retry_wait" — **not** `paused`. The one fixture +(`event.task.state.json`) only exercises the `failed` case. So today, when CORE +auto-pauses for `auth_required` or `server_file_changed`, DAEMON has no signal on the wire +telling it *why* — §2/§3 of this ADR are unbuildable without one. This needs a PROTO +follow-up (minor: widening an existing field's presence condition, per +`contracts/README.md` rule 4 — no new field, no retype) to also populate `error` when +`state == "paused"` and the pause was CORE-initiated. DAEMON is not asking for a way to +tell CORE-paused from user-paused on the wire in general — `error: null` on a +DAEMON-initiated pause is sufficient, since DAEMON already knows it just did that. + +## Open, for CORE and PROTO to confirm or amend + +1. Does CORE's `starved_tasks()` already exclude `retry_wait` and auto-paused tasks, or + does this ADR ask for a behavior change there (§4)? +2. Is `pause()` idempotent today, or does calling it on an already-paused task currently + return an error CORE needs to relax? +3. PROTO: land the `error`-on-`paused` widening above before DAEMON writes the resume + logic in §3 — otherwise DAEMON has no correctness-preserving way to implement it and + would have to guess from timing, which is the exact failure mode ADR 0011 was written + to rule out for a different pair of governors. +4. Naming: is "auto-pause" the term CORE already uses internally, or does CORE have an + existing name for this that this ADR should adopt instead of introducing a new one? From 62cda074c10dd1044940e816d48feaa93d7a3e09 Mon Sep 17 00:00:00 2001 From: sami Date: Thu, 10 Sep 2026 00:04:53 +0400 Subject: [PATCH 2/3] daemon: fold in CORE's sign-off on ADR 0013 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CORE accepted ADR 0013 as written, no amendments (core/docs/adr-0013-core-response.md, lane/core@c65e664). Folds in: - pause()/resume() idempotency contract, precisely: no-op success on an already-paused task, ALSO on a terminal task (pause racing completion isn't an error), resume() no-op on a non-paused task, the only error is task_not_found, and no state-change event fires for a no-op call. - tasks_starved pinned as {connecting, downloading} AND segments_active == 0 — a structural exclusion of retry_wait and auto-paused tasks rather than a special case, with the full state table CORE gave. - restart handling confirmed fully; two non-blocking notes from CORE about work-interruption during verifying/assembling. - "auto-pause" adopted as the term, no new wire/API surface. Status updated: accepted by CORE; PROTO's item 3 (permit `error` on event.task.state when state=="paused") is the one remaining blocker before daemon/src/sched/'s pause/resume logic can be written correctness-preservingly. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Upd9WhG9oppieig5nRDLig --- docs/adr/0013-task-state-machine-ownership.md | 111 ++++++++++++------ 1 file changed, 76 insertions(+), 35 deletions(-) diff --git a/docs/adr/0013-task-state-machine-ownership.md b/docs/adr/0013-task-state-machine-ownership.md index 09b1736..612e08b 100644 --- a/docs/adr/0013-task-state-machine-ownership.md +++ b/docs/adr/0013-task-state-machine-ownership.md @@ -1,9 +1,13 @@ # ADR 0013 — Task state-machine ownership: CORE, DAEMON, and the shared `paused` -**Status:** proposed · **Date:** 2026-09-09 · **Lane:** DAEMON, drafted at PROTO's request -(D1 in `core/docs/proto-requests-m1.md`), for CORE and PROTO to review and sign off. -**Needs:** explicit accept from CORE and PROTO before `daemon/src/sched/` calls into CORE's -task-start API, since the calling convention below is derived from this split. +**Status:** accepted by CORE, PROTO's open item 3 outstanding · **Date:** 2026-09-09 +· **Lane:** DAEMON, drafted at PROTO's request (D1 in `core/docs/proto-requests-m1.md`). +**CORE's response:** `core/docs/adr-0013-core-response.md` (`lane/core`, commit `c65e664`) +— accepted as written, no amendments, plus the pinned `tasks_starved` definition and +`pause()`/`resume()` idempotency contract folded in below. +**Remaining blocker:** PROTO landing the `error`-on-`paused` widening (open item 3) — +`daemon/src/sched/`'s pause/resume logic is unblocked from CORE's side already, but not +buildable correctness-preservingly until that lands. ## Context @@ -77,19 +81,24 @@ Both sides can put a task in `paused`, for disjoint reasons: state-change callback — the same path every other CORE-driven transition uses. This is not a new mechanism. -The contract that makes this safe: +The contract that makes this safe, per CORE's sign-off (`core/docs/adr-0013-core-response.md`): -- **`pause()` is idempotent.** DAEMON calling `pause()` on a task CORE already - auto-paused is a no-op, not an error — DAEMON does not need to know CORE got there - first. Symmetrically, if CORE were ever mid-transition to `paused` when DAEMON's pause - request arrives, the result is still `paused`, once. +- **`pause(TaskId)` is idempotent: a no-op success** on a task already `paused` (however + it got there), and **also a no-op success on a terminal task** (`complete`/`failed`/ + `cancelled`) — a pause racing a completion is not an error. **`resume(TaskId)` is a + no-op success on a task that is not paused.** The only error either returns is + `task_not_found`. CORE emits no state-change event for a no-op call — DAEMON reads the + resulting state from the normal state-change callback / `TaskDetail`, never from + `pause()`'s or `resume()`'s return value. If CORE is mid-transition into `paused` (its + own auto-pause) when DAEMON's `pause()` arrives, the task ends up `paused` once, with + exactly one state-change event. - **DAEMON persists *why* a task is paused**, in the `tasks` table, not in `TaskState` itself (the wire type stays a flat enum — this is DAEMON-local bookkeeping, not a contract change). A `pauseReason` distinguishing at least `user`, `schedule`, `queue_stopped`, `admission_reconcile`, and CORE's `error.code` when auto-paused. This - is what makes §3's resume rule possible — but it needs a contract fix first: see the - gap in open item 3 below. `error.code` is not currently carried on a transition into - `paused` at all. + is what makes §3's resume rule possible — but it needs a contract fix first: see "A + contract gap this ADR surfaces" below. `error.code` is not currently carried on a + transition into `paused` at all. - **CORE does not need to track why it's paused past the current occurrence.** Once paused, CORE's job is done; DAEMON is the only side that later decides whether to resume, and DAEMON is also the only side with persistent storage to remember the @@ -118,14 +127,28 @@ and asserts `tasks_starved == 0` in steady state past a bounded delay. A task in `retry_wait` also holds zero segments, deliberately, for up to 60 s (`docs/04` §7's backoff cap) — and it is **not** admission-starved, it is CORE's own policy holding it idle. -Resolution: `retry_wait` must be distinguishable from true starvation without DAEMON -guessing from timing. CORE's `starved_tasks()` (ADR 0011) excludes any task CORE itself -holds at zero by policy — `retry_wait` and CORE-initiated `paused` both fall outside -`tasks_starved` by construction, because that accessor is about the segment allocator -failing to grant a slot to a task that wants one, not about a task that isn't asking. A -task in `retry_wait` still counts against `connection.maxConcurrentDownloads` from -DAEMON's side (it is running, not requeued) but contributes nothing to the segment -allocator's guarantee pass until it re-enters `connecting`. +**Pinned by CORE's sign-off**, tightening ADR 0011's definition rather than special-casing +it: `tasks_starved` counts only tasks whose `TaskState` is `connecting` or `downloading` +**and** `segments_active == 0` — precisely "the allocator has not granted a slot to a task +that is asking for one." + +| Task state | In `tasks_starved`? | Why | +|---|---|---| +| `probing` | no | uses the probe pool (ADR 0011 §5), not the segment budget | +| `connecting`, 0 segments | **yes** | admitted + probed, waiting on the allocator's first grant — the real starvation case | +| `connecting`/`downloading`, ≥1 segment | no | a segment in its own `connecting` sub-state counts as held (ADR 0011 amendment A3) | +| `downloading`, 0 segments | **yes** | held slots and lost them all (e.g. every segment failed and is being re-requested) — transient, still real | +| `retry_wait` | no | CORE's backoff timer holds it at zero *deliberately*; not asking the allocator for anything until it re-enters `connecting` | +| `paused` (either-initiated) | no | not asking for a slot | +| `new`, `queued`, `assembling`, `verifying`, terminal | no | outside the counted state set | + +So `retry_wait` and auto-paused tasks are excluded **structurally, by not being in the +`{connecting, downloading}` state set** — not via a special case that could rot as the +engine evolves. `starved_tasks()` returns exactly the TaskIds in this count; +`starved_since(id)` is defined only for them. A task in `retry_wait` still counts against +`connection.maxConcurrentDownloads` from DAEMON's side (it is running, not requeued) but +contributes nothing to the segment allocator's guarantee pass until it re-enters +`connecting`. ### 5. Restart — CORE holds no persistent state, DAEMON reloads to `queued` @@ -143,6 +166,18 @@ where to actually resume from `.veloxpart.meta` and re-validates with `If-Range` `paused` tasks reload as `paused`, with their `pauseReason` intact, and are not auto-admitted — §3 applies identically after a restart as it does live. +**Confirmed by CORE, fully.** `start(TaskId)` transparently checks for a valid +`.veloxpart.meta` sidecar (CORE's stage 5), re-validates with `If-Range` (`docs/04` §5), +and either resumes from the recorded offsets or restarts if the sidecar is missing or +fails validation. DAEMON does nothing special beyond rewriting CORE-owned states to +`queued` and re-admitting, as this section already said. + +**Two notes from CORE, not objections to the ADR:** the *transition* into `paused` is +honoured from any CORE state per §1's table, but the *work interruption* is best-effort — +a pause during `verifying` discards the in-progress hash and re-hashes from the start on +resume (cheap, bounded); a pause during `assembling` (HLS/DASH mux) is an M4 concern and +may not be cleanly interruptible mid-mux. Neither changes this ADR's API shape. + ## Consequences - `daemon/src/sched/` calls `start(TaskId)` exactly once per admission (`queued → @@ -151,10 +186,10 @@ auto-admitted — §3 applies identically after a restart as it does live. - The `tasks` table needs a `pauseReason` column (DAEMON-local; not a wire type) before `sched/`'s pause/resume logic can be written correctly — flagging as a concrete follow-up, not blocking this ADR's acceptance. -- CORE's `starved_tasks()` (ADR 0011) must exclude `retry_wait` and CORE-auto-paused tasks - by construction; if that isn't already true in CORE's accessor, it needs to be before - DAEMON relies on the starvation invariant, since otherwise every backoff cycle would - trip DAEMON's governor-invariant warning as a false positive. +- CORE's `starved_tasks()` (ADR 0011) excludes `retry_wait` and CORE-auto-paused tasks by + construction — pinned in CORE's sign-off as the `{connecting, downloading} ∧ + segments_active == 0` definition in §4's table — so DAEMON's governor-invariant warning + will not false-positive on a normal backoff cycle. - No `contracts/` change. `TaskState`, `event.task.state`, and `previousState` are already sufficient; this ADR is entirely about which process calls which function when. @@ -190,15 +225,21 @@ follow-up (minor: widening an existing field's presence condition, per tell CORE-paused from user-paused on the wire in general — `error: null` on a DAEMON-initiated pause is sufficient, since DAEMON already knows it just did that. -## Open, for CORE and PROTO to confirm or amend +## Resolution of the four open items -1. Does CORE's `starved_tasks()` already exclude `retry_wait` and auto-paused tasks, or - does this ADR ask for a behavior change there (§4)? -2. Is `pause()` idempotent today, or does calling it on an already-paused task currently - return an error CORE needs to relax? -3. PROTO: land the `error`-on-`paused` widening above before DAEMON writes the resume - logic in §3 — otherwise DAEMON has no correctness-preserving way to implement it and - would have to guess from timing, which is the exact failure mode ADR 0011 was written - to rule out for a different pair of governors. -4. Naming: is "auto-pause" the term CORE already uses internally, or does CORE have an - existing name for this that this ADR should adopt instead of introducing a new one? +1. **`starved_tasks()` excludes `retry_wait`/auto-paused by construction — confirmed.** + Pinned as a design commitment in CORE's sign-off (§4's table above): the counted set is + `{connecting, downloading} ∧ segments_active == 0`, so exclusion is structural, not a + special case. +2. **`pause()`/`resume()` idempotency — confirmed as specified in §2 above**, plus two + details this ADR hadn't anticipated: a no-op call also succeeds against a *terminal* + task (pause racing completion isn't an error), and no state-change event fires for a + no-op — DAEMON reads resulting state from the callback/`TaskDetail`, never from the + call's return value. +3. **Still open — PROTO.** CORE's half is ready (`ErrorInfo` is populated on every + auto-pause transition today's callback path would carry, using the existing B1 + taxonomy); the wire only needs `error` permitted when `state == "paused"`. This is the + one remaining blocker on `daemon/src/sched/`'s pause/resume logic. +4. **CORE adopts "auto-pause".** No new wire or API term — the discriminator stays + `state == paused` plus the presence of an `Error` (present ⇒ CORE-initiated, absent ⇒ + DAEMON-initiated), exactly as §2 already specified. From 27e9ce9fb541e488556e822dd28bd957d7e6e88d Mon Sep 17 00:00:00 2001 From: sami Date: Thu, 10 Sep 2026 00:07:28 +0400 Subject: [PATCH 3/3] =?UTF-8?q?daemon:=20mark=20ADR=200013=20fully=20accep?= =?UTF-8?q?ted=20=E2=80=94=20PROTO=20landed=20the=20error-on-paused=20wide?= =?UTF-8?q?ning?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PROTO closed the one remaining contract gap as contracts/ 1.3.0 (lane/proto commit 6db304a): event.task.state.error / TaskSummary.error now populate on a paused transition CORE entered unilaterally, not just on failed/retry_wait. Minor widening of an existing field's presence condition, no retype, no new field, per contracts/README.md rule 4. Updates every place in the ADR that referred to this as an open question or unresolved gap: the status line, the pause-reason bookkeeping in §2, the alternatives-considered pointer, and the contract-gap section itself (renamed "surfaced, now closed"). Notes 1.3.0 is on lane/proto but not yet merged to main (still 1.1.0) — daemon/src/sched/'s pause/resume logic should be written once that merge lands, not before. All four of ADR 0013's open items are now resolved: CORE confirmed tasks_starved's structural exclusion and pause()/resume() idempotency explicitly (verdict: "accept as written", not hedged), and adopted "auto-pause" with no new wire term. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Upd9WhG9oppieig5nRDLig --- docs/adr/0013-task-state-machine-ownership.md | 59 +++++++++++-------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/docs/adr/0013-task-state-machine-ownership.md b/docs/adr/0013-task-state-machine-ownership.md index 612e08b..eefbd99 100644 --- a/docs/adr/0013-task-state-machine-ownership.md +++ b/docs/adr/0013-task-state-machine-ownership.md @@ -1,13 +1,15 @@ # ADR 0013 — Task state-machine ownership: CORE, DAEMON, and the shared `paused` -**Status:** accepted by CORE, PROTO's open item 3 outstanding · **Date:** 2026-09-09 +**Status:** accepted by CORE and PROTO; all four open items resolved · **Date:** 2026-09-09 · **Lane:** DAEMON, drafted at PROTO's request (D1 in `core/docs/proto-requests-m1.md`). **CORE's response:** `core/docs/adr-0013-core-response.md` (`lane/core`, commit `c65e664`) — accepted as written, no amendments, plus the pinned `tasks_starved` definition and `pause()`/`resume()` idempotency contract folded in below. -**Remaining blocker:** PROTO landing the `error`-on-`paused` widening (open item 3) — -`daemon/src/sched/`'s pause/resume logic is unblocked from CORE's side already, but not -buildable correctness-preservingly until that lands. +**PROTO's response:** the `error`-on-`paused` widening landed as `contracts/` **1.3.0**, +`lane/proto` commit `6db304a` — no retype, no new field; only the presence condition on +the existing `error: TaskError | null` field widens to include a daemon-initiated pause. +Not yet merged to `main` (main is at 1.1.0 as of this writing) — `daemon/src/sched/`'s +pause/resume logic should be written against `1.3.0` once that merge lands, not before. ## Context @@ -95,10 +97,10 @@ The contract that makes this safe, per CORE's sign-off (`core/docs/adr-0013-core - **DAEMON persists *why* a task is paused**, in the `tasks` table, not in `TaskState` itself (the wire type stays a flat enum — this is DAEMON-local bookkeeping, not a contract change). A `pauseReason` distinguishing at least `user`, `schedule`, - `queue_stopped`, `admission_reconcile`, and CORE's `error.code` when auto-paused. This - is what makes §3's resume rule possible — but it needs a contract fix first: see "A - contract gap this ADR surfaces" below. `error.code` is not currently carried on a - transition into `paused` at all. + `queue_stopped`, `admission_reconcile`, and CORE's `error.code` when auto-paused — + now readable off `event.task.state.error` since PROTO's 1.3.0 widening (see "A + contract gap this ADR surfaced, now closed" below). This is what makes §3's resume + rule possible. - **CORE does not need to track why it's paused past the current occurrence.** Once paused, CORE's job is done; DAEMON is the only side that later decides whether to resume, and DAEMON is also the only side with persistent storage to remember the @@ -209,21 +211,25 @@ without CORE reporting them; forcing DAEMON to poll or reimplement that timing d etc.).** Rejected — it roughly doubles the enum for a fact only DAEMON's resume logic needs, and adding wire cardinality for internal bookkeeping is the kind of thing that becomes a compatibility problem the moment a client starts branching on it. The value -DAEMON needs (CORE's reason) can travel on the existing `error` field instead — see the -contract gap in open item 3 — without touching `TaskState` itself. +DAEMON needs (CORE's reason) travels on the existing `error` field instead — see the +closed contract gap below — without touching `TaskState` itself. -## A contract gap this ADR surfaces, not just an open question +## A contract gap this ADR surfaced, now closed -`event.task.state.schema.json`'s own description scopes `error` to "whenever the new -state is failed or retry_wait" — **not** `paused`. The one fixture -(`event.task.state.json`) only exercises the `failed` case. So today, when CORE -auto-pauses for `auth_required` or `server_file_changed`, DAEMON has no signal on the wire -telling it *why* — §2/§3 of this ADR are unbuildable without one. This needs a PROTO -follow-up (minor: widening an existing field's presence condition, per -`contracts/README.md` rule 4 — no new field, no retype) to also populate `error` when -`state == "paused"` and the pause was CORE-initiated. DAEMON is not asking for a way to -tell CORE-paused from user-paused on the wire in general — `error: null` on a -DAEMON-initiated pause is sufficient, since DAEMON already knows it just did that. +`event.task.state.schema.json`'s description scoped `error` to "whenever the new state is +failed or retry_wait" — **not** `paused`, at the time this ADR was drafted, and the one +fixture (`event.task.state.json`) only exercised the `failed` case. So CORE auto-pausing +for `auth_required` or `server_file_changed` had no wire signal telling DAEMON *why* — §2/ +§3 of this ADR were unbuildable without one. + +**Closed by PROTO**, `contracts/` 1.3.0 (`lane/proto` commit `6db304a`, see the status +line at top for the merge caveat): minor widening of the existing field's presence +condition, per `contracts/README.md` rule 4 — no new field, no retype, `error` stays +`TaskError | null`. It is now populated on a `paused` transition whenever CORE entered it +unilaterally; `error: null` on a DAEMON-initiated pause is unchanged. New fixture +`event.task.state.auto-paused.json` exercises the auto-pause case; the existing +`event.task.state.json` fixture's stale "exactly when failed or retry_wait" assertion was +corrected in the same change. ## Resolution of the four open items @@ -236,10 +242,13 @@ DAEMON-initiated pause is sufficient, since DAEMON already knows it just did tha task (pause racing completion isn't an error), and no state-change event fires for a no-op — DAEMON reads resulting state from the callback/`TaskDetail`, never from the call's return value. -3. **Still open — PROTO.** CORE's half is ready (`ErrorInfo` is populated on every - auto-pause transition today's callback path would carry, using the existing B1 - taxonomy); the wire only needs `error` permitted when `state == "paused"`. This is the - one remaining blocker on `daemon/src/sched/`'s pause/resume logic. +3. **Landed — PROTO, `contracts/` 1.3.0** (`lane/proto` commit `6db304a`, not yet merged + to `main`). `event.task.state.error` and `TaskSummary.error` are now populated "on + every `failed` or `retry_wait` transition, and on a `paused` transition the daemon + entered unilaterally" — a deliberate pause still carries `error: null`. New fixture + `event.task.state.auto-paused.json` exercises an `auth_required` auto-pause directly. + DAEMON's §3 resume rule can be implemented once `sched/` is built against `main` at + 1.3.0 or later. 4. **CORE adopts "auto-pause".** No new wire or API term — the discriminator stays `state == paused` plus the presence of an `Error` (present ⇒ CORE-initiated, absent ⇒ DAEMON-initiated), exactly as §2 already specified.