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
187 lines
12 KiB
Markdown
187 lines
12 KiB
Markdown
# 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 1–32; 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`)
|