Files
vdm/core/docs/engine-api-m1.md
T
samiandClaude Sonnet 5 3da4cd6e91 core: rate/token_bucket + fold in DAEMON's engine-API review (stage 7)
rate/token_bucket.hpp — a lazily-refilled TokenBucket (starts full: burst
then throttle, IDM behaviour; rate 0 == unlimited; burst caps idle
accumulation) and RateLimiter, the global -> per-queue -> per-task
hierarchy (docs/04 §6). acquire(task, n) peeks every applicable level and
commits on all-or-none so a blocked attempt never leaks tokens at a level
that had them; held under one mutex so a concurrent detach can't dangle
the bucket it's using. vdm/ids.hpp gains QueueId.

Tests: burst/refill/cap/unlimited for the bucket; tightest-level-binds,
no-partial-consumption, detach-safety, and an 8-thread aggregate-rate
check for the hierarchy. Green under ASan/UBSan and TSan.

Engine-API review (DAEMON signed off, no sched/ or dispatch rewrite):
 - Engine::rate_limiter() accessor added (limiter.set -> set_global_limit).
 - Checksum::Algo gains sha512 to match the wire Checksum set.
 - DownloadSpec: DAEMON creates save_path's parent dir before start();
   missing dir -> Error::path_rejected (made explicit).
 - cancel(): documented to fire on_state(_, cancelled, nullopt) then
   on_finished(Err{canceled}), in that order; download.cancel ==
   cancel(false), download.remove == cancel(true).
 - engine-api-m1.md: the five open questions resolved with DAEMON's
   answers (probe_hint optional, single cancel flag, {restart,
   keep_partial, abort} is the whole set, per-task 4 Hz is fine,
   refresh_url restarts all segments after a validating re-probe).

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
2026-09-10 15:50:18 +04:00

187 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# CORE → DAEMON — the engine API (M1)
**Status: reviewed and signed off** by DAEMON on 2026-09-10
(`daemon/docs/engine-api-review.md`). Nothing forced a `sched/` or dispatch rewrite. The
five open questions are resolved at the bottom; the review's four confirms are folded into
the headers (`sha512` added to `Checksum::Algo`; the parent-directory contract made
explicit; `cancel()` ordering documented; `rate_limiter()` accessor added).
Integration order: DAEMON wires this in after `daemon/src/sched/` lands (the scheduler is
what calls `start`/`pause`/`resume`/`cancel` and drives `set_task_order`); `sched/` builds
against these headers in parallel with CORE stage 8 and does not need the bodies.
---
This is the `libveloxcore` public surface `veloxd` links and calls
to actually run a download. It is the thing the AGENT-CORE brief asked for on day one and
that slipped: DAEMON has an RPC surface and a store and, until this is agreed, nothing to
call. Sketch headers: `core/include/vdm/engine.hpp`, `core/include/vdm/task/download.hpp`
(both compile now; `Engine` / `DownloadHandle` bodies land in CORE stage 8). Nothing here
touches `contracts/` — DAEMON projects these callbacks onto `TaskSummary` / `TaskDetail` /
`event.*`.
Please review the field semantics, the threading/lifetime rules, and the
pause/resume/cancel contract, and raise anything that would force a `daemon/src/sched/` or
RPC-dispatch rewrite later. Open questions are at the bottom.
---
## 1. `DownloadSpec` — what DAEMON hands the engine
DAEMON has already run the rules engine, picked the category folder, canonicalised the
path and checked it against the allowed roots (`-32011` is DAEMON's error, raised before
`start()`), and resolved the filename. The engine's spec is the concrete result.
| field | who fills it | notes |
|---|---|---|
| `url`, `mirrors` | DAEMON | `mirrors` are alternative URLs for the *same bytes*; the segmenter's requeue prefers a different host after 3 connection failures (docs/04 §3). |
| `headers`, `cookies`, `referrer`, `user_agent` | DAEMON, verbatim from the capture | replayed on every segment request and on the probe. |
| `save_path` | DAEMON | **absolute and final.** The engine never canonicalises or root-checks. `<save_path>.veloxpart` and `.veloxpart.meta` sit beside it; on success the part file is renamed in place. |
| `segments` | user / `connection.maxSegmentsPerDownload` | requested upper bound 132; the engine lowers it to the per-host cap and to 1 for a non-resumable source. Effective value comes back in `Progress.effective_segments`. |
| `buffer_bytes` | user / `connection.bufferBytes` | requested per-segment; silently reduced to fit `maxTotalBufferBytes` across all live segments. Effective value in `Progress.effective_buffer_bytes` → your `TaskDetail.effectiveBufferBytes`. |
| `proxy`, `auth` | DAEMON | `auth` carries credentials only if known up front (Secret Service). Leave `scheme == none` to get an `on_auth_required` on a 401/407 instead. |
| `checksum` | user (`download.add.checksum`) | `{algo, hex}`. Verified during `verifying`; a mismatch is a terminal `failed` with `Error::checksum_mismatch`. |
| `probe_hint` | DAEMON | the `ProbeResult` you already got for the File Info dialog. Supplying it skips the engine's own probe — the task starts in `connecting`, not `probing`. The engine still revalidates with `If-Range` on resume. |
| `allow_resume` | DAEMON | `true`: if a CRC-valid `.veloxpart.meta` sits beside `save_path`, resume from it. `false`: start fresh, overwrite. Your restart flow (ADR 0013 §5) sets this per task. |
| `max_retries` | user / default 10 | per segment, before `failed` with `Error::max_retries_exhausted`. |
`start()` returns immediately. It never throws and never blocks on the network; a bad URL,
DNS failure, or unwritable `save_path` is delivered through `on_finished`.
## 2. The state machine the engine drives
`EngineState` is the CORE-owned subset of the wire `TaskState` (ADR 0013 §1):
```
probing ─▶ connecting ⇄ downloading ─▶ assembling ─▶ verifying ─▶ complete
│ │ ▲ │ │ (M4 mux; a no-op rename in M1)
│ │ └─ retry_wait ┘
▼ ▼
(any) ──────▶ paused ──(resume)──▶ connecting
(any CORE state) ──────────────────▶ failed (terminal, engine-initiated)
(any state, on cancel()) ──────────▶ cancelled (terminal, DAEMON-initiated)
```
`new` and `queued` are yours; the engine never emits them. `start()` corresponds to your
`queued → probing`. Every transition is reported through `on_state(from, to, error?)`,
including the auto-pauses (§4) and the terminals. `previousState` on your
`event.task.state` maps straight from the `from` argument.
## 3. Threading and lifetime
- **Callbacks run on an engine thread** — a transfer worker, the progress timer, or a
dispatch thread — **never** the thread that called `start()` / `pause()` / etc.
- **Per task, callbacks are serialised.** You will never get two callbacks for the same
handle at once. Across tasks they run concurrently.
- **A callback must not block.** It runs on a thread doing real transfer work; a slow
callback stalls that work. Hand off to your own queue/loop.
- **A callback must not re-enter the same handle synchronously** — no `pause()` from
inside `on_state`, etc. Post it. (Calling into a *different* handle, or into
`segment_budget()`, is fine.)
- **`on_finished` is always the last callback** for a task. After it returns the engine
makes no further callbacks for that handle and the handle's control methods are no-ops.
- **The handle is copyable and thread-safe.** Dropping the last copy does **not** cancel —
the task runs on. Call `cancel()` to stop it. (DAEMON holds the handle for the task's
life anyway.)
- **`Engine` must outlive every handle.** `~Engine()` cancels all running tasks and joins
their workers before returning — expect it to block briefly.
- **Logging**: the engine writes through `vdm::set_log_sink()` (a `core/util` global).
Install your sink once at startup; the engine never opens a file itself.
## 4. `paused` is shared, and idempotency is the contract (ADR 0013 §2)
Both sides put a task in `paused`, for disjoint reasons:
- **DAEMON-initiated**: `handle.pause()` — user pause, a schedule window closing, a queue
stop, `Queue.onComplete`, the admission governor reconciling a lowered
`maxActiveSegments`.
- **Engine auto-pause**: `on_auth_required` (401/407), `on_decision_needed`
(`server_file_changed` / stale range), disk full. The engine transitions to `paused`
on its own and fires `on_state(_, paused, ErrorInfo{...})` — the same path as any other
transition. `ErrorInfo.code` present ⇒ engine-initiated; absent ⇒ you did it. That is
the only discriminator, and it is what your `error`-on-`paused` widening (open item 3 of
ADR 0013) carries on the wire.
**Idempotency, now a signature:**
| call | already in that state / terminal | otherwise |
|---|---|---|
| `pause()` | no-op, no error | stop new segment requests, flush + `fdatasync` in-flight buffers, write `.veloxpart.meta`, release the budget slots, `on_state(_, paused, nullopt)`. Bounded by the slowest in-flight flush. |
| `resume()` | no-op if not `paused`; no-op if terminal | revalidate with `If-Range`, re-acquire budget slots, `paused → connecting`, resume from the sidecar offsets. |
| `cancel(discard_partial)` | no-op if already terminal | stop everything, `on_state(_, cancelled, nullopt)`, `on_finished(Err{cancelled})`. `discard_partial` also unlinks `.veloxpart[.meta]` — wire this to `download.remove {deleteFile}`. |
`resume()` after an auto-pause for `auth_required` **without** a preceding
`provide_auth()` is a no-op — the task stays paused. This is ADR 0013 §3's "resume must
not cross reasons", enforced on CORE's side: the scheduler cannot accidentally un-pause a
task waiting on credentials.
- **`provide_auth(user, pass, remember)`** — acts only while the task is auto-paused for
auth; supplies the credential for the retry and resumes. `remember` asks *you* to
persist to the Secret Service; the engine never stores it. No-op otherwise.
- **`decide(Decision)`** — acts only while auto-paused for a decision. `restart` discards
the partial and re-downloads; `keep_partial` continues against what is on disk (the
user's stated risk); `abort``failed`. No-op otherwise.
- **`refresh_url(url, headers)`** — IDM's "Refresh Download Address": the engine re-probes
the new URL to validate it, then **restarts every segment on it** (the signed-URL-expiry
case the wire method exists for), keeping the bytes already on disk. Mirror rotation is
*not* this — that is `spec.mirrors` + the segmenter's requeue-to-a-different-host.
- **`on_decision_needed`** is only for the cases the engine cannot resolve itself. A
routine 416 / stale range is the engine's own re-probe + re-split loop; it escalates
`DecisionRequest{range_metadata_stale}` **only when that loop fails**, at which point
"retry the same range" is already exhausted — so `{restart, keep_partial, abort}` is the
whole choice set.
## 5. Progress
`on_progress` is coalesced to **≤ 4 Hz per task** inside the engine — the same cadence as
`event.task.progress`, so your batcher can forward without re-throttling. It carries
aggregate `downloaded` / `speed_bps` / `eta_seconds`, the effective segment count and
buffer size, and a `SegmentProgress[]` (index, inclusive `[start,end]`, `completed`,
per-segment speed, state) for the GUI's segment bars. `total` is absent for a chunked
source until the stream ends.
`handle.state()` and `handle.progress()` are synchronous lock-guarded snapshots for
`download.get` / `download.list` — call them any time, including from your RPC thread.
## 6. What the engine does NOT do
- No filename resolution, no category matching, no path canonicalisation, no allowed-root
check — all DAEMON, before `start()`.
- No queueing, scheduling, priority, or "when queue completes" — DAEMON, via
`segment_budget().set_task_order()` and by choosing when to call `start()` / `pause()`.
- No persistence beyond `.veloxpart.meta`. On a daemon restart the engine knows nothing;
you reload from SQLite, rewrite CORE-owned states to `queued`, and re-`start()` with
`allow_resume = true` (ADR 0013 §5).
- No credential storage. Ever (`CLAUDE.md` §4).
---
## Resolved (DAEMON review, 2026-09-10)
1. **`probe_hint` stays optional.** DAEMON has a `ProbeResult` only on the File-Info path;
capture-take, `velox add`, `addBatch` and restart have none. The engine probes when
it's absent.
2. **One `cancel(discard_partial)`.** `download.cancel` = `cancel(false)`;
`download.remove` = `cancel(true)` for a live task (plus DAEMON's row/file cleanup), or
pure DAEMON-side for an already-terminal one. No `handle.remove()`.
3. **`{restart, keep_partial, abort}` is the whole set.** The engine owns the routine
416 re-probe/re-split and only escalates `on_decision_needed` when that loop fails —
"retry the same range" is exhausted by then.
4. **Per-task 4 Hz is fine.** DAEMON coalesces across tasks for `event.task.progress`
regardless; `on_progress_batch` is a nice-to-have and must not block stage 8.
5. **`refresh_url` restarts all segments on the new URL** after a validating re-probe (the
signed-URL case). Mirror rotation is `spec.mirrors` + the segmenter, not this.
## Review confirms, folded in
- **(a)** `vdm::TaskId` is a cheap-copy hashable value; DAEMON never constructs one — it
only receives it from `start()` / callbacks and passes it back to `set_task_order()`
etc. ✔ (`vdm/ids.hpp`)
- **(b)** DAEMON `mkdir -p`s `save_path`'s parent before `start()`. The engine opens the
file and fails with `Error::path_rejected` if the directory is missing. ✔ (documented on
`DownloadSpec`)
- **(c)** `Checksum::Algo` now has `sha512`, matching the wire `Checksum` set. ✔
- **(d)** `cancel()` always fires `on_state(_, cancelled, nullopt)` then
`on_finished(Err{Error::canceled})` (note: the taxonomy value is `canceled`), in that
order. ✔ (documented on `DownloadHandle::cancel`)