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
12 KiB
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 insideon_state, etc. Post it. (Calling into a different handle, or intosegment_budget(), is fine.) on_finishedis 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.) Enginemust 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()(acore/utilglobal). 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 loweredmaxActiveSegments. - Engine auto-pause:
on_auth_required(401/407),on_decision_needed(server_file_changed/ stale range), disk full. The engine transitions topausedon its own and fireson_state(_, paused, ErrorInfo{...})— the same path as any other transition.ErrorInfo.codepresent ⇒ engine-initiated; absent ⇒ you did it. That is the only discriminator, and it is what yourerror-on-pausedwidening (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.rememberasks 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.restartdiscards the partial and re-downloads;keep_partialcontinues 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 isspec.mirrors+ the segmenter's requeue-to-a-different-host.on_decision_neededis 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 escalatesDecisionRequest{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 callstart()/pause(). - No persistence beyond
.veloxpart.meta. On a daemon restart the engine knows nothing; you reload from SQLite, rewrite CORE-owned states toqueued, and re-start()withallow_resume = true(ADR 0013 §5). - No credential storage. Ever (
CLAUDE.md§4).
Resolved (DAEMON review, 2026-09-10)
probe_hintstays optional. DAEMON has aProbeResultonly on the File-Info path; capture-take,velox add,addBatchand restart have none. The engine probes when it's absent.- 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. Nohandle.remove(). {restart, keep_partial, abort}is the whole set. The engine owns the routine 416 re-probe/re-split and only escalateson_decision_neededwhen that loop fails — "retry the same range" is exhausted by then.- Per-task 4 Hz is fine. DAEMON coalesces across tasks for
event.task.progressregardless;on_progress_batchis a nice-to-have and must not block stage 8. refresh_urlrestarts all segments on the new URL after a validating re-probe (the signed-URL case). Mirror rotation isspec.mirrors+ the segmenter, not this.
Review confirms, folded in
- (a)
vdm::TaskIdis a cheap-copy hashable value; DAEMON never constructs one — it only receives it fromstart()/ callbacks and passes it back toset_task_order()etc. ✔ (vdm/ids.hpp) - (b) DAEMON
mkdir -pssave_path's parent beforestart(). The engine opens the file and fails withError::path_rejectedif the directory is missing. ✔ (documented onDownloadSpec) - (c)
Checksum::Algonow hassha512, matching the wireChecksumset. ✔ - (d)
cancel()always fireson_state(_, cancelled, nullopt)thenon_finished(Err{Error::canceled})(note: the taxonomy value iscanceled), in that order. ✔ (documented onDownloadHandle::cancel)