From ea8fd7ca8fcdb70ec33c54929984d76e2e583a70 Mon Sep 17 00:00:00 2001 From: sami Date: Thu, 10 Sep 2026 15:51:16 +0400 Subject: [PATCH] daemon: fold CORE's confirms into the engine-API review CORE resolved all four (lane/core@3da4cd6): vdm::TaskId is hashable and DAEMON never constructs one; DAEMON mkdir -p's save_path's parent (engine -> Error::path_rejected if missing); sha512 added as the 4th Checksum::Algo so no -32602 at the RPC edge; on_state(cancelled) then on_finished(Err{Error::canceled}) -- note the one-L spelling in the error taxonomy. Also notes rate/token_bucket + Engine::rate_limiter() for the limiter.set wiring. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Upd9WhG9oppieig5nRDLig --- daemon/docs/engine-api-review.md | 38 ++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/daemon/docs/engine-api-review.md b/daemon/docs/engine-api-review.md index b907e91..bddd2fa 100644 --- a/daemon/docs/engine-api-review.md +++ b/daemon/docs/engine-api-review.md @@ -62,24 +62,28 @@ and it is already `DownloadSpec.mirrors` + the segmenter's 3-failure requeue, no segments to the new URL from their current offsets; if they don't match, surface it (`on_decision_needed` or a `refresh_url` error) rather than silently restarting. -## To confirm — none block sign-off +## Confirmed by CORE (`lane/core@3da4cd6`) -1. **`vdm::TaskId` ↔ wire UUID.** `sched/` keeps a `vdm::TaskId → wire taskId` map, keyed - off `handle.id()` at `start()`. Confirm `vdm::TaskId` is cheap to copy and hashable - (`std::unordered_map` key), and that DAEMON is never expected to *construct* one — it - only ever receives them from `start()` / callbacks and passes them back to - `set_task_order()`. -2. **Parent directory creation.** The engine "never canonicalises or root-checks" - `save_path` — does it `mkdir -p` the parent (the category folder may not exist yet), or - must DAEMON ensure the directory exists before `start()`? DAEMON assumes the latter - (it owns category/path logic) unless you say otherwise. -3. **`Checksum::Algo` has no `sha512`**, but `Checksum.schema.json` accepts it (PROTO F3). - DAEMON will reject `algorithm: "sha512"` at the RPC edge with `-32602` before `start()` - unless CORE would rather carry a fourth enum value. Either is fine; say which. -4. **`on_finished(Err{cancelled})`** — DAEMON keys on `ErrorInfo.code == cancelled` to - write a `cancelled` history row rather than surfacing it as a user-facing failure. - Confirm `cancelled` is the code and that `on_state(_, cancelled, nullopt)` always - precedes it (the §4 table implies this). +1. **`vdm::TaskId`** — cheap-copy and `std::hash`-able; DAEMON never constructs one, only + receives it from `start()` / callbacks and passes it back to `set_task_order()`. + `sched/` keeps the `vdm::TaskId → wire taskId` map keyed off `handle.id()`. +2. **Parent directory** — DAEMON `mkdir -p`s `save_path`'s parent before `start()`; the + engine opens the file and fails the task with `Error::path_rejected` if it is missing. + Now explicit on `DownloadSpec`'s doc comment. +3. **`sha512`** — added as the fourth `Checksum::Algo`, matching the wire set. No `-32602` + at the RPC edge; DAEMON passes it straight through. +4. **Cancel ordering** — `on_state(_, cancelled, nullopt)` then `on_finished`, always in + that order. Note the taxonomy value is spelled `canceled` (one L): the finish is + `on_finished(Err{Error::canceled})`, `error.code == canceled`. `sched/` keys on that + to write a `cancelled` history row rather than a user-facing failure. (The wire + `TaskState` / `EngineState` spelling stays `cancelled`; only `vdm::Error` is one-L.) + +## Related, landed the same pass + +`rate/token_bucket` — the global → queue → task speed-limiter hierarchy (`docs/04` §6), +reached via `Engine::rate_limiter()`. DAEMON's `limiter.set {globalBps, enabled}` maps to +`rate_limiter().set_global_limit(...)`; `limiter.get` reads it back. Wire this alongside +the `download.add` → `start()` glue. ## Integration timing