Was 7/16 of tools/testserver/README.md's mode table covered by
engine_test.cpp. Adds the rest:
- engine_expiring_signed_url_recovers_via_refresh_url: an expired signed
URL 403s, the engine asks (paused, decision_calls >= 1) rather than
failing terminally, and DownloadHandle::refresh_url() with a freshly
signed URL completes it -- exercises both do_refresh_url() fixes and
the probe-level referrer retry's second-403 path from the previous
commit.
- engine_403_without_referer_retries_with_origin: no spec.referrer set,
the automatic single retry (previous commit) recovers with zero
decisions asked.
- engine_redirect_chain_follows_to_completion: 5 hops of a plain 302.
No core-side change needed -- documents that CURLOPT_FOLLOWLOCATION/
MAXREDIRS (already on, RequestOptions::follow_redirects) cover both
the probe's and every worker's own request, not just one of the two.
- engine_slow_loris_stall_timeout_fires: proves curl's stall detector
(CURLOPT_LOW_SPEED_LIMIT/_TIME, download_task.cpp's hardcoded 1024 B/s
for 30s) actually fires rather than hanging. Needed a real fix, not
just a test: every other test in this file relies on TestServer's
short 1s loris dribble to keep runtime down, but 1s of trickle
followed by full-speed streaming never accumulates curl's required 30
CONSECUTIVE seconds under the floor, so it would never actually abort
-- a test built on the default dribble would pass by the download
merely finishing a bit late, not by observing the stall timeout fire.
testserver_fixture.hpp's TestServer gained an explicit-loris-seconds
constructor (default ctor unchanged, still 1s) so this one test can
ask for a dribble (40s) that genuinely outlasts the threshold.
- engine_401_digest_then_provide_auth_completes: same shape as the
existing 401-basic test: http_client.cpp already asks libcurl for
CURLAUTH_ANY regardless of net::AuthScheme, so this needed no core
change -- it passed on the first run and is here to prove that's true
end-to-end, not just at the http_client unit level.
- engine_chunked_no_length_completes_single_segment: Transfer-Encoding:
chunked, no Content-Length anywhere (including HEAD). No core change
needed -- takes the same size-agnostic "unknown size, one plain-GET
segment" path as the existing no-range test.
- utf8/legacy-content-disposition: already covered end-to-end by
probe_reads_utf8_content_disposition and
probe_reads_legacy_content_disposition in probe_test.cpp (probe-level,
as these modes only affect the initial request) -- verified passing,
no new test needed.
All 20 engine_test.cpp cases and all 10 probe_test.cpp cases pass. Every
testserver.py spawned while writing and running this was reaped by
TestServer's destructor; verified no stragglers with `ps aux` after each
run.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Q3QrF7rCt21bkAjt9BCDFQ
docs/04-engine-design.md §7's failure policy table has said "403 after
redirect: retry once with the original referrer; many CDNs require it"
since it was written, and Error::forbidden's own enum comment says the
same -- but grepping download_task.cpp and http_client.cpp for 403 turned
up nothing. It was never built.
Implemented at both points a 403 can surface:
- The probe (net::Prober, a separate request path from segment workers):
on_probe_result() now retries once via restart_probe(false), with
effective_referrer set to the download URL's own origin (origin_of(),
via net::split_url()), when the failure is Error::forbidden and this is
the first retry. A second 403 asks rather than fails outright --
auto_pause_locked(..., false, true), the same "ask, don't just fail"
path 416/etag-mismatch already use -- specifically so DownloadHandle::
refresh_url() stays usable afterward (its own contract requires a
non-terminal task); this is what makes the expiring-signed-url mode's
README-documented refresh_url() recovery actually reachable.
- Each segment worker (SegWorker::forbidden, set in seg_head() on a 403
HEAD): the same one-shot referrer retry via retry_worker(), landing on
auto_pause_locked() on a second 403 for the same reason.
Both paths route the retry's Referer through a new effective_referrer
field rather than spec.referrer directly, since the origin-retry must not
overwrite what the caller actually asked for -- start_worker_locked() and
restart_probe() were switched to send effective_referrer instead.
do_refresh_url() had two latent bugs surfaced by actually exercising the
expiring-signed-url recovery path end-to-end:
1. It unconditionally proceeded to resume even when the refresh probe
itself failed -- a bad refresh URL would silently un-pause a task with
nothing behind it. Now returns (stays paused) on !r.has_value().
2. It only handled "already probed once, just refreshing a few fields" --
for a task whose first-ever probe never succeeded (every hostile mode
this commit adds a test for that pauses at the initial probe, not
mid-download), s->registered was never true, so the existing
`if (s->registered) set_want()` never fired and nothing happened. Now
detects !s->have_probe and calls finish_probe_locked() directly, the
actual first-time registration/segmenter-construction path.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Q3QrF7rCt21bkAjt9BCDFQ
Root cause of the M7 RSS gap (core/docs/m7-baseline.md: ~70 MB measured
against a 60 MB target): SegmentBudget::confirm_slot() only checked a task's
own held count against its own target -- never the engine-wide active_ sum.
reallocate_locked()'s two-pass fairness allocation does bound
sum(target) <= max_active_ at the moment it computes a plan, but that bound
says nothing about sum(held): a task can be legitimately holding more than
its own just-lowered target for a while (yield is deferred to a segment
boundary, never mid-segment -- ADR 0011 A1), and another task's target can
correctly rise to claim that capacity before the first task has physically
released it. Both confirm_slot() calls could then succeed against their own,
individually-correct targets while sum(held) exceeded max_active_ --
tools/bench heap-profile caught this directly: budget.active reading 56-86
against a total of 32.
confirm_slot() now also checks active_ < max_active_, unconditionally, as a
backstop that doesn't depend on any task's target bookkeeping being in sync
with what every other task holds. That creates a liveness question the
original design never answered: a task denied only by this new check has a
target that's already correct, so it never changes again and
reallocate_locked()'s plain "fire a callback when a task's target changes"
mechanism never revisits it. Task gained a waiting_for_slot flag, set on
exactly this denial; release_slot()/deregister_task() (the only two places
that free real capacity) now hand a freed slot directly to the
highest-priority waiting task via wake_one_waiter_locked(), if
reallocate_locked()'s own plan didn't already produce a callback for anyone.
download_task.cpp's fill_slots_locked() needed a matching fix: a woken
task's stalled segments (SegState::stalled -- backed off mid-retry, its own
release_slot() already called) have no live worker and never surface
through Segmenter::assign_slot(), which only hands out unassigned or fresh
ranges. fill_slots_locked() now restarts any stalled segment with no live
worker directly (bounded by slot_target, same as its assign_slot() loop)
before looking for new work; a segment it doesn't get to keeps its own
scheduled retry_worker() timer as a second chance.
Also fixes a real TSan-caught data race this work surfaced: SegWorker::
speed_bps was written only by its own segment's curl callback and, before
Progress.speed_bps's polled-path fix, only ever read from that same thread
-- safe without synchronization. snapshot_progress() reading it from
whatever thread calls DownloadHandle::progress() broke that invariant
(workers_mu's shared_lock protects the workers map's structure, not an
individual SegWorker's fields). Now std::atomic<double> with relaxed
ordering -- an informational EMA, nothing synchronizes real state on it --
rather than adding a lock to the write side.
core/tests/segment/budget_test.cpp adds two tests reproducing the actual
gap (budget_active_never_exceeds_max_active_segments_under_concurrent_load,
budget_wait_list_wakes_a_task_whose_target_never_changed) plus a sanity
baseline (budget_release_wakes_a_denied_waiter), and introduces AsyncFakeTask
+ TestTimer for the one existing test that drives the budget from multiple
concurrent threads -- mirroring production's real dispatch (register_task()'s
on_target lambda posts through host.schedule(), download_task.cpp, never a
synchronous call) rather than adding reentrancy-guarding machinery to
SegmentBudget itself to compensate for a synchronous test double being
unlike production. See docs/adr/0017 for the full writeup, including what an
earlier version of this fix got wrong chasing a same-thread reentrancy
hazard that doesn't actually exist in production.
core/docs/m7-baseline.md updated: the RSS number now clears the DoD line
(45.41 MiB via heap-profile), root-caused rather than just re-measured.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Q3QrF7rCt21bkAjt9BCDFQ
DAEMON reported Progress.speed_bps reading 0 for the whole life of a live
throttled download while downloaded bytes visibly advanced. DAEMON reads
progress by polling DownloadHandle::progress() (engine_port_core.hpp), not
the on_progress push callback.
DownloadTaskState::snapshot_progress() -- the body behind progress() -- never
set speed_bps, per-segment speed_bps, or eta_seconds at all; only the
event-driven emit_progress_if_due() (which drives the on_progress callback)
computed them, from the same live SegWorker::speed_bps EMA seg_data()
maintains. snapshot_progress() now reads that same per-worker speed while
building its segment list, so a segment with no live worker (idle, paused,
complete, failed) correctly reports 0 and a segment with an active transfer
reports its real EMA, matching emit_progress_if_due()'s math including the
eta_seconds derivation.
engine_polled_progress_reports_nonzero_speed reproduces the bug (fails
without the fix, confirmed) by polling .progress() -- the same path DAEMON
uses -- during a throttled download and asserting speed_bps > 0 once real
progress has accumulated.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Q3QrF7rCt21bkAjt9BCDFQ
Pure functions only, per AGENT-CORE.md's build order: no I/O, no JSON, no SQL,
no notion of the wire Rule type — DAEMON decodes its own stored/wire
representation into these plain structs and calls in.
- rules/filename.hpp: sanitize_filename() turns a raw candidate (from
net::parse_content_disposition or net::url_filename — neither is
filesystem-safe by design; both headers say so and point here) into one
safe to create on ext4/APFS/NTFS: strips separators and control bytes,
folds NTFS-illegal characters, neutralizes reserved Windows device names,
clamps length on a UTF-8 boundary. Total on hostile input; never empty.
Not the path-traversal security boundary — that's daemon/fs/safepath,
downstream of this and the one that actually matters adversarially.
- rules/collision.hpp: resolve_collision() finds the next free name
Explorer/Finder-style ("name (1).ext", ...) given an existence predicate,
or returns the desired name unchanged under an overwrite policy. Never
fabricates a guaranteed-unique name past its attempt bound — hands back
the last candidate tried rather than hiding a persistent collision.
- rules/match.hpp: match_rules() is the evaluation half of
contracts/schema/types/Rule.schema.json — priority order, first rule
whose present match clauses (extensions/mimeTypes/host & url glob/size
bounds) all hold, wins; a size clause never matches speculatively before
the probe fills in size_bytes. glob_match() is the iterative (not
recursive — bounded work on an all-'*' pattern) matcher both host_pattern
and url_pattern use.
Every header compiles standalone; tests (39 cases) pass under ASan+UBSan and
TSan. core/include/vdm/README.md documents the new public surface.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Q3QrF7rCt21bkAjt9BCDFQ
Three subcommands in one binary, driving vdm::Engine directly (docs/04 §8):
- throughput: a single download against a fast local origin
(support/local_server.hpp, busybox httpd), reporting Mbps/CPU%/RSS. Gates
on --require-mbps/--max-cpu-pct only when passed, so the ctest smoke
registration stays a correctness check, not a hardware-dependent
perf gate -- the real 1-Gbit-link sign-off is a manual/CI job (see the
file's header comment).
- load: N concurrent tasks against tools/testserver's `throttled` mode
(support/testserver_client.hpp), reporting peak RSS via getrusage(). Paced
externally rather than through the engine's own rate::RateLimiter or
busybox: the limiter's pause/resume path allocates on every throttle event
(would contaminate alloc-check's measurement) and under heavy segment
contention was found to starve individual tasks indefinitely (see
docs/adr/0016, added here); busybox couldn't sustain the DoD's ~160
concurrent connections (20 tasks * default_segments=8) reliably. The
ctest registration runs at reduced concurrency under sanitizer presets --
see the CMakeLists.txt comment and the ADR's postscript.
- alloc-check: operator new/delete overridden process-wide, sampling the
allocation count across a steady mid-transfer window against a paced
tools/testserver origin. Caught a real bug in the same change (see the
http_client.cpp commit) and, by dropping its Engine mid-download to end
cleanly, also surfaced the quiesce() use-after-free (see that commit).
core/docs/m7-baseline.md records actual measured numbers against the M1/M7
DoD lines, including where they don't clear yet (RSS ~70 MB vs a 60 MB
target; throughput/CPU only measured on loopback, no 1 Gbit link available
here) rather than rounding them away.
docs/adr/0016 documents a rate::RateLimiter fairness gap found building the
load subcommand: a single shared TokenBucket under heavy segment contention
has no fairness ordering across its peek/commit race and can starve a
waiter well past what its configured rate implies. Filed as a follow-up
(it's a core/src/rate design question, not a tools/bench one) rather than
fixed here, along with a related TSan-only load-test straggler that could
not be root-caused in this environment.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Q3QrF7rCt21bkAjt9BCDFQ
HttpClient::Impl::drain_commands() default-constructed a std::deque<Command>
every call, on every iteration of the worker thread's event loop (once per
curl_multi_poll wake -- i.e. once per socket-readiness event on the transfer
hot path), then swapped the (usually empty) command queue into it. In
libstdc++, an empty std::deque still allocates its map array on construction,
so this was a real allocation on the hot path regardless of whether any
command (add/pause/resume/cancel) was actually pending -- which is the
common case, since those are rare next to data arriving.
Found via tools/bench's alloc-check, which is built in this change and
exists specifically to catch this class of bug (AGENT-CORE.md: "no
allocation in the curl write callback... checked in review and by a bench
assertion"): before this fix it reported thousands of allocations/sec under
a sustained transfer; after, single digits.
Fixed by checking `w.queue.empty()` under the lock before touching `local`
at all, so the deque is only constructed when there's actually something to
swap into it.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Q3QrF7rCt21bkAjt9BCDFQ
DownloadTaskState::quiesce() (Engine shutdown / ~Engine, via quiesce_task())
cancelled every live worker's transfer, then immediately cleared `workers` on
the calling thread. transfer.cancel() only *requests* the HttpClient worker
thread stop the transfer -- it does not wait for that to happen. If that
thread was mid write-callback (seg_data -> WriteBuffer::append ->
SparseFile::write_at), clearing the map destroyed the SegWorker (and its
ring buffer) it was still writing through: a heap-use-after-free, caught by
ASan via tools/bench alloc-check, which by design drops its Engine while a
download is still active mid-sample.
Every other exit path (verify/fail/auto_pause/demote, via begin_drain_locked)
already gets this right: cancel, then let each worker remove and flush
itself through seg_finished once HttpClient actually confirms the transfer
stopped, on the correct thread. quiesce() now does the same instead of
tearing the map down itself -- wait on a condition variable, notified from
seg_finished right after it erases, until `workers` is empty.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Q3QrF7rCt21bkAjt9BCDFQ
Resumes work left mid-session on the M1 hostile-mode matrix. download_task.cpp:
- Replace the old cancel-then-clear teardown (cancel_all_transfers_locked /
start_assembly_locked) with a single begin_drain_locked()/PendingAction
mechanism: cancel every live worker, remember what to do (verify / fail /
auto_pause / demote), and let whichever worker's seg_finished finds the
worker map empty carry it out. Every sibling still flushes its buffer on
the way out, so no buffered-but-unflushed tail is lost when a download
finishes or fails while other segments are still mid-transfer.
- A 200 where 206 was expected (wrong_status) now checks the response's
ETag/Last-Modified against the probe's: a real mismatch asks the user
(server_file_changed, "ask, never silently corrupt" -- docs/04 §5); a match
means the server just stopped honouring Range for this connection, so
demote to one segment and keep going without a round trip (docs/04 §7).
- 416 mid-download (stale range metadata) now surfaces as a decision instead
of retrying the same now-invalid range to exhaustion.
- do_decide's abort path surfaces the actual reason a decision was asked
for (last_error) instead of hardcoding server_file_changed, which was
mislabeling a 416 abort.
engine_test.cpp adds the four hostile modes where a bug means silent
corruption rather than a visible failure: etag-changes, 416-always,
lies-about-accept-ranges, content-length-mismatch.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Q3QrF7rCt21bkAjt9BCDFQ
Stage 8 of the CORE build order: the bodies behind the DownloadSpec /
callback API reviewed in core/docs/engine-api-m1.md. Wires probe -> segment
workers -> WriteBuffer -> SparseFile -> .veloxpart.meta -> retry/backoff ->
SegmentBudget -> RateLimiter -> callbacks into one event-driven machine.
- Engine (src/engine.cpp): owns HttpClient, Prober, SegmentBudget,
RateLimiter and one timer jthread (min-heap of scheduled fns). start()
builds a task and returns a DownloadHandle; ~Impl quiesces every task
before joining the timer so no callback fires during teardown.
- DownloadTaskState (src/task/download_task.cpp): one `mu` task lock; a
shared_mutex over the worker map for the curl write path; callbacks
collected under `mu` and fired after release via a separate deferred
queue; weak_from_this() in every async hop. State machine over the
CORE-owned EngineState subset, auto-pause on 401/407 and on a 200 where
206 was expected, validated resume via If-Range.
- digest (src/task/digest.cpp): OpenSSL EVP hash_file() for the optional
post-download checksum; links OpenSSL::Crypto PRIVATE.
- Segmenter::release_segment(): hand a paused segment back to the pool
unassigned so resume's assign_slot() picks it up instead of splitting a
still-"assigned" range and orphaning its front half.
- DownloadHandle now names the real control block (vdm::task::
DownloadTaskState, defined only in the engine TU) via a namespace-scope
fwd decl and a public-but-effectively-engine-only ctor, replacing the
nested State/friend pair. Every public signature is unchanged; DAEMON
(vdm-79) confirmed sched/ names only the public API.
Fixes found while building the end-to-end suite (tests/task/engine_test.cpp,
9 cases against tools/testserver, green under ASan/UBSan and TSan):
- a dropped connection lost its unflushed WriteBuffer tail while advance()
had already counted those bytes as done -> a retry resumed past an
unwritten hole. Flush on the failure path.
- when the byte counters hit total while other workers were still live,
teardown dropped their buffered tails. Now: cancel them and let each
worker's own seg_finished drain it (the `assembling` state), last one
starts verification -- no cross-thread buffer access.
- seg_head() let a 401 with credentials present abort before libcurl's
resend; now it proceeds once and acts on the final status.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
run_all() takes an optional substring list; vtest_main forwards argv and a
comma-separated $VT_ONLY. No filter => run everything, as before. Makes
iterating on one slow end-to-end case (the engine suite) practical
without a framework swap.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
libcurl with CURLAUTH_ANY answers a 401/407 by resending the request with
an Authorization header. Two spots in net/ cut that short:
- http_client's header callback delivered the response head exactly once
and latched `head_delivered`, so after an auth challenge the caller only
ever saw the 401 — never the 2xx of the authenticated resend. Reset the
latch when a fresh status line follows a delivered 401/407 (redirects
never reach that path — their head is suppressed).
- the prober's head callbacks return DataAction::abort to skip the body,
which also aborts the transfer mid-handshake. Return `proceed` for a
401/407 when credentials were supplied, so curl's resend can run; the
real status lands on the next header block.
Also give ProbeRequest an `auth` field (default scheme == none) and pass
it through base_request(), so a re-probe after a 401 can present the
credentials the user just entered. No behaviour change when no auth is
configured.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
DAEMON's safepath-adversarial.md accepts a TOCTOU residual between its
canonicalise-and-check and the download starting, on the stated grounds
that CORE's O_NOFOLLOW open of the final file closes it. That flag was
never actually set: SparseFile::open used O_WRONLY|O_CREAT|O_CLOEXEC, so
a symlink swapped in as the final path component after DAEMON's check
would be followed and redirect our pwrites outside the allowed roots.
Add O_NOFOLLOW. A symlinked leaf now fails the open with ELOOP, which
errno_to_error already maps to Error::path_rejected. Regular files and
the O_CREAT of a fresh part file are unaffected; resume (existing regular
part file) is unaffected. Test that a symlinked destination is rejected
rather than silently followed, and that the link target is never touched.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
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
The download entry point 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 in velox::core to call.
vdm/task/download.hpp — DownloadSpec (the resolved subset DAEMON hands in:
absolute save_path, verbatim browser headers, requested segments/buffer,
optional probe_hint / checksum / auth, allow_resume), EngineState (the
CORE-owned subset of the wire TaskState), Progress / SegmentProgress,
DownloadCallbacks (on_progress <=4 Hz, on_state for every transition
incl. auto-pauses, on_auth_required, on_decision_needed, on_finished
last), DownloadHandle (pause/resume/cancel — idempotent per the ADR 0013
signature — plus provide_auth / decide / refresh_url, and synchronous
state()/progress() snapshots).
vdm/engine.hpp — Engine: start(spec, callbacks) -> handle, segment_budget()
(DAEMON's sched/ admission surface, ADR 0011), live connection.* setters,
a standalone probe() on the pool outside the segment budget.
core/docs/engine-api-m1.md — the review doc: field semantics, the state
machine, threading/lifetime rules (which thread callbacks arrive on, what
is legal from inside one, handle/engine lifetime), the shared-`paused`
idempotency contract as a signature, and five open questions for DAEMON.
Value types compile and are covered by api_compiles_test; Engine /
DownloadHandle bodies land in stage 8, built against whatever DAEMON
signs off here.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
vdm/ids.hpp — TaskId, an opaque engine handle (DAEMON keeps the wire
UUID <-> TaskId map; the engine never sees the UUID).
segment/segmenter — per-download range management (docs/04 §3). Initial
lazy split; assign_slot() splits the largest remaining range when the
budget grants a slot; on_complete(may_steal) either *steals* the second
half of the largest remaining range for the same worker (slot-neutral) or
returns nullopt so the caller *yields* the slot (ADR 0011 A1); on_failed()
returns requeue only on the 3rd consecutive connection error with a mirror
present — the remaining range is orphaned and re-split. Non-resumable or
unknown-size => exactly 1 segment; never split below min_segment_bytes
(1 MiB). Resume ctor rebuilds from a persisted table (falls back to a
fresh layout if it doesn't tile [0,total)). One mutex == "the task lock";
segment fields are std::atomic and the store is a std::deque so a steal's
append never moves a worker's record.
segment/budget — the global allocator (ADR 0011). Owns exactly one
ceiling (maxActiveSegments) and min-1-before-seconds fairness: a two-pass
allocation (guarantee pass gives every wanting task 1 slot in DAEMON's
priority order, then a growth pass round-robins the rest up to each
task's effective cap = min(per_task_cap, host cap, 1 if non-resumable)),
recomputed from scratch on every edge so a live set_max_active_segments
cut naturally yields the excess lowest-priority-first, never a
mid-segment kill. DAEMON-facing surface exactly as promised in
daemon/docs/core-requests-m1.md / ADR 0011: budget(), segments_active(),
starved_tasks(), starved_since(), set_max_active_segments (drain),
set_host_segment_cap, set_task_order, on_budget_changed (a jthread
coalesces at <=4 Hz; the tasks_starved 0<->nonzero edge fires
immediately). Callbacks are copied out and run after the lock is
dropped.
Tests: segmenter split/steal/requeue/resume math + a concurrent
steal-and-advance run; budget min-1 under a tight budget, round-robin
growth, host-cap and non-resumable clamps, live-lower shedding
lowest-priority-first, starvation below the task count, starved-edge
notification, and a concurrent set_want hammer. Green under ASan/UBSan;
the steal path and the budget green under TSan.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
DAEMON's daemon/docs/proto-requests-m1.md P1: velox::proto::Dispatcher's
on_* methods returned Result<T> = expected<T, ParseError>, and dispatch()
mapped every handler error to -32603 InternalError. A handler had no way to
return -32010 (download.get not-found), -32011 (download.add invalid-path)
or -32013 (probe-failed) with their data payloads -- three error fixtures a
conformant server must satisfy were unreachable, blocking DAEMON's
"conformance as a server" M1 DoD.
Two error channels now, kept separate on purpose:
- parse: Result<T> / ParseError -- dispatch() failing to turn the wire into
typed params. Always -32602, always structural.
- handler: HandlerResult<T> / HandlerError -- a handler deciding the request
can't be fulfilled. Carries any ErrorCode + message + free-form data.
struct HandlerError {
ErrorCode code{ErrorCode::InternalError}; // bare {} is a valid -32603
std::string message;
nlohmann::json data = nullptr; // straight into the error's data
};
template <class T> using HandlerResult = std::expected<T, HandlerError>;
dispatch()'s handler branch is now
make_error(id, r.error().code, r.error().message, r.error().data)
instead of a hard-coded InternalError. -32001/-32002/-32003 stay the server
layer's to raise around dispatch(), as DAEMON already does.
Verified end to end against the real dispatch() path: a handler returning
TaskNotFound/InvalidPath/ProbeFailed produces -32010/-32011/-32013 with the
data object intact, and a bare HandlerError{} still yields a clean -32603
with no data field. The `= nullptr` on the member (not `{nullptr}`) matters:
brace-init of nlohmann::json from nullptr is the array [null], not JSON null.
FixtureDispatcher regenerated to HandlerResult; conformance_main.cpp only
inspects dispatch()'s JSON and needed no change. TS side is untouched beyond
the version string -- no server Dispatcher is generated there.
P2 also handled: session.hello.version-mismatch's data.expected was a stale
"1.0.0"; now $any, with a note that the error-fixture compare is on `code`
only so a server echoing kProtocolVersion there is fine.
Version: minor, 1.3.0 -> 1.4.0. Wire is byte-identical (no schema, fixture,
or OpenRPC change) but every Dispatcher implementer must swap Result ->
HandlerResult on regen, and the bump is how lanes are told to. Not an ADR:
one lane consumes this binding, it's the one that asked, and the shape is
the one they proposed. Answered in contracts/proto-answers-daemon-m1.md.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012fgjnqFCS5h5L7gZTZo3rV
util/crc32.hpp — header-only CRC-32 (zlib polynomial, reflected), used to
integrity-check the sidecar.
meta/veloxpart — the <name>.veloxpart.meta resume file (docs/04 §5).
Little-endian, versioned, CRC-32 over the whole record. Layout: magic,
version, flags, total_size, downloaded, url set (original/effective/
mirrors), etag/last-modified/content-type, segment records (start, end
INCLUSIVE, completed), optional sha256 streaming-hash blob.
parse_veloxpart() is the attacker-facing surface (the file sits in a
world-writable-ish download dir) and is total on any byte string: CRC
checked before any field is interpreted; magic, a version it understands,
every count and length bounded by a hard cap AND checked against the
remaining buffer; ByteReader latches on overrun; trailing bytes rejected.
Every malformation is meta_corrupt / meta_version_unsupported, never a
crash or an unbounded allocation. serialize_veloxpart() is deterministic
(unchanged sidecar isn't rewritten). File helpers write atomically
(temp + rename) and fdatasync the file and its directory.
Tests: crc32 known vector; full + minimal round-trips; deterministic
serialize; file round-trip; and a truncation/corruption table — bad
magic, CRC mismatch (payload and CRC-field flips), future version,
truncation at every stage, hostile url_count / segment_count / lp_string
length (the case the brief singles out), trailing bytes, impossible
segment.completed.
tools/fuzz/fuzz_veloxpart — feeds raw bytes and bytes-with-valid-CRC
(so the field parser and ByteReader bounds checks are actually reached),
and round-trip-stability-checks anything accepted. Ran 1.1M execs clean
under ASan+UBSan+libFuzzer (clang++-21); fuzz_content_disposition and
fuzz_url likewise re-run to 1.1M. tools/fuzz gains a -runs=0 seed-replay
CTest smoke per target (regression tripwire; the campaign stays manual).
Fuzz-found and fixed: parse_content_disposition could emit a filename
containing NUL / control bytes from a mangled filename* ext-value —
strip_path only removed path separators. Now sanitize_leaf() also drops
C0 controls and DEL (rules/ still owns the authoritative sanitize; `..`
and printable-unsafe content pass through as before).
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
io/sparse_file — the single O_WRONLY output file (docs/04 §4). open()
posix_fallocate's the full size (falls back to ftruncate on
EOPNOTSUPP/ENOSYS, reported via preallocated()); write_at() pwrites at an
absolute offset, looping short writes and retrying EINTR; sync() is
fdatasync (timer/pause only); advise_dontneed() is
posix_fadvise(DONTNEED); resize() trims a preallocated tail or sizes a
chunked download. errno -> vdm::Error (ENOSPC->disk_full,
EACCES->permission_denied, ENOENT/ENOTDIR/...->path_rejected). No lock on
write_at — POSIX makes each pwrite atomic for a regular file, so N
segment threads writing disjoint ranges is safe (tested, TSan-clean).
io/write_buffer — per-segment accumulate-and-flush buffer, preallocated
at construction; append() only memcpys (no allocation on the write-
callback hot path, docs/04 §8 — asserted by a global-new counter in the
test). Flushes on fill via a caller-supplied FlushFn; a chunk >= capacity
arriving on an empty buffer writes straight through. On a flush error
next_offset() stays at the last durable position. Single-threaded; the
disk-writer-thread handoff is stage 8.
Also: vtest.hpp VT_CHECK_EQ/NE now copy operands (auto, not auto&&) — an
assertion must not outlive a temporary the expression returned a
reference into (ASan caught this on Result<void>{}.error().code).
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
tests/conformance/cpp/CMakeLists.txt (PROTO's lane) compiles
core/generated/velox_proto.cpp directly while its comment claims it links
libveloxproto. Now that the veloxproto target exists it should link
velox::proto, with a TARGET-guarded fallback to the direct-compile for
standalone configures. Filed, not edited — not CORE's file.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
core/ now produces two libraries as ADR 0009 specifies:
- veloxcore — the engine; still links only Threads + CURL, no JSON.
- veloxproto — generated/velox_proto.cpp, generated/ as a PUBLIC include
dir, nlohmann_json linked PUBLIC. velox::proto alias.
Consumed by veloxd / CLI / GUI / the conformance runner; veloxcore must
never link it. nlohmann_json is found here too (the root only finds it
when daemon/ has landed) so core builds standalone. Generated code is
built -Wall -Wextra -Wno-error — it is committed and never hand-edited, so
a codegen quirk must not break the build. A configure-time FATAL_ERROR
trips if veloxcore ever links veloxproto.
Verified: libveloxproto.a builds clean; veloxcore's link deps contain no
proto/nlohmann; full suite green.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
DAEMON's rebase audit caught it: TaskDetail.effectiveBufferBytes said "the
daemon reduces every live segment's buffer" to fit maxTotalBufferBytes, but
ADR 0011's ownership table (line 55) assigns bufferBytes/maxTotalBufferBytes
to CORE in bytes-units -- DAEMON counts tasks, CORE counts segments and
bytes. "The engine" is correct.
Same error, same root cause, in DownloadSpec.segments: "the daemon lowers it
to the per-host cap" attributes the per-host *segment* cap to DAEMON, but
that's CORE's (ADR 0011 line 54, "CORE enforces per-host segment caps -- it
owns the connections and is the only place segments are counted"). DAEMON's
own per-host cap is a *task*-level admission cap (line 50), a different
thing entirely -- conflating the two in the schema's own prose is exactly
how the clamp ends up implemented twice, once in each lane, disagreeing.
Description-only, no version bump: the JSON Schema shape is untouched, only
which component the prose names as doing the reducing. Regenerated code
diffs are comment-only (doc comments in the generated header and TS types).
Checked every other buffer/segment-clamp description for the same mistake;
the rest either already said "CORE"/"the engine" or used passive voice that
doesn't misattribute (Settings.connection.maxTotalBufferBytes,
Settings.connection.bufferBytes, docs/04, ADR 0012).
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012fgjnqFCS5h5L7gZTZo3rV
DAEMON's docs/adr/0013-task-state-machine-ownership.md needs a wire signal
for the difference between a paused task the daemon entered unilaterally
(auth_required, server_file_changed, disk_full) and one that was requested
(user, schedule, queue stop, admission reconcile) -- without it, DAEMON's §3
resume rule ("resume only when the reason matches the event that justifies
resuming") has nothing correctness-preserving to key on, and would have to
guess from timing. CORE has already accepted the ADR; this was the sole
remaining blocker per DAEMON's own status line on it.
No retype, no new field -- error was already TaskError | null on both
event.task.state and TaskSummary, exactly as DAEMON characterized the ask.
Only the *description* of when it is populated widens: previously "failed or
retry_wait", now also "paused, when the daemon entered it on its own
initiative". A deliberate pause still carries error: null. TaskError's own
top-level description gets the same widening, since it previously also said
"failed or retry_wait" and would otherwise contradict the field that embeds
it.
New fixture (event.task.state.auto-paused.json) exercises the case directly:
an auth_required pause with error populated, contrasted in its own
description against download.pause.json's error: null for a requested pause.
The existing event.task.state.json fixture's first assertion was stale
("error is present exactly when failed or retry_wait") and is corrected.
Minor bump, 1.2.0 -> 1.3.0: a description widening on an already-nullable,
already-optional field changes no JSON Schema shape, but it is a real
behavioral commitment change worth a version bump so downstream regenerates
and notices, per the same reasoning ADR 0010 applied to TaskErrorCode.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012fgjnqFCS5h5L7gZTZo3rV
Accept as written — no amendments. Answers to the four open items:
1. tasks_starved / starved_tasks() count ONLY tasks in {connecting,
downloading} with segments_active == 0 (the allocator owes a slot to a
task that is asking). retry_wait and paused (either-initiated) are
outside that state set, so they are excluded by construction, not by a
special case. Design commitment; the accessor is stage 6/8.
2. pause() is idempotent: no-op success on an already-paused or terminal
task; resume() no-op success on a non-paused task; only task_not_found
errors. No state-change event for a no-op.
3. PROTO's item — CORE confirms its half: auto-pause reports the
transition with ErrorInfo populated (auth_required / server_file_changed
/ disk_full / path_rejected), already in util/error.hpp. Ready once
PROTO permits error on state=="paused".
4. CORE adopts "auto-pause"; the wire/API discriminator stays
state==paused + presence of the Error code.
Notes back: pause during verifying re-hashes from scratch on resume;
pause during assembling is M4; restart handling in §5 agreed —
start(TaskId) re-derives resume position from .veloxpart.meta + If-Range.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
The last contract gap blocking an M1 definition-of-done item: CORE's "401
handled" has no return path without it, and B2a's sibling F2 was accepted in
proto-answers-m1.md but never actually landed.
download.provideAuth {taskId, username, password, save?} -> {ok}, exactly as
proposed there. Privileged and Unix-socket-only: a credential-bearing method
must never be reachable from the browser, which is the other half of the
promise event.auth.required's own description already makes ("never back
through this event, never into a log"). It answers the challenge; it does not
itself resume the task -- the daemon retries with the credential attached and
the ordinary event.task.state reports the task leaving retry_wait, the same
as any other state change.
save only tells the daemon whether to persist the credential in the Secret
Service for next time, or use it for this attempt alone -- it never touches
SQLite or a log either way, in keeping with CLAUDE.md's secrets rule.
Three fixtures: the success path, -32010 for a task that no longer exists
(credentials submitted for it are simply discarded), and -32003 confirming
the extension has no path to this method under any transport.
mockd gets a real handler rather than falling through to the generic fixture
responder: it validates the taskId exists (so the -32010 fixture is
replayable) and actually transitions the task out of retry_wait.
Minor bump, 1.1.0 -> 1.2.0: additive method, no existing type touched.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012fgjnqFCS5h5L7gZTZo3rV
net/content_disposition — total parser for the mojibake-prone header:
RFC 6266 filename (quoted/token), RFC 5987 filename* ext-values
(charset'lang'pct-encoded, incl. RFC 2231 continuations), legacy RFC 2047
encoded-words (=?UTF-8?B?..?= / ?Q?), and raw Latin-1 bytes; prefers
filename* over filename; strips path components AFTER decoding (a base64
payload can hold '/'). 22-case test table.
net/text_codec (internal) — percent-decode, UTF-8 validation, Latin-1->
UTF-8, base64, RFC 2047 — shared by the CD parser and the URL splitter.
net/url — a small total URL splitter (scheme/userinfo/host/port/path/
query/fragment, http(s) validity) and url_filename() for the last path
segment; used for the filename fallback.
net/probe — HEAD then a ranged GET bytes=0-0 that PROVES resumability
(206 + matching Content-Range + a validator), rather than trusting
Accept-Ranges which servers lie about; the ranged GET is also the HEAD-
refused (403/405/501) fallback. 401/407 -> success result with
requires_auth, not an error. Runs on its own pool (max_concurrent,
default 4) outside the segment budget per ADR 0011 §5. suggest_filename()
does the resolution order (explicit -> disposition -> URL -> download.bin)
with a light strip; rules/ (stage 9) owns the authoritative sanitize.
tools/fuzz — libFuzzer targets for the CD parser and the URL splitter,
compiling the parser sources directly so they're fully instrumented;
self-guards on VELOX_BUILD_FUZZ + Clang (the top-level CMake adds every
tools/* unconditionally). Seed corpora included.
Fixed on the way: a p -> Transfer -> State -> cbs -> p reference cycle in
Prober that leaked every probe (drop the stored Transfer; the worker
keeps State alive). Tests green under ASan/UBSan and TSan.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
One HttpClient owns a small pool of workers, each with its own CURLM; an
easy handle lives on one worker for its life. Public start/pause/resume/
cancel enqueue a command + curl_multi_wakeup(); callbacks (on_head /
on_data / on_finished) run on the worker thread and return a DataAction
(proceed / pause / abort). Covers redirects (final-response head only),
ranges (inclusive ByteRange -> CURLOPT_RANGE), proxy/SOCKS5, basic/digest
auth, cookies, verbatim headers, stall detection, a coarse recv-rate cap,
and a curl_share DNS/TLS cache across workers. CURLcode + HTTP status ->
vdm::Error in net/curl_error. A probe is on_head returning abort: it
finishes successfully (head_complete), not canceled.
Tests drive tools/testserver: full GET, ranged 206, redirect chain, 404
-> not_found, connection refused -> connect_failed, HEAD probe + ranged
0-0 probe (no body), cancel mid-transfer, pause/resume completes. Skip
cleanly if testserver isn't in the tree.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
Accept the decision (each ceiling enforced once by its unit owner; DAEMON
counts tasks, CORE counts segments; the single min() clamp; per-host caps
split by unit). daemon/src/sched/ is unblocked.
Answers to the five open questions:
1. Min-1 is implementable in the allocator without inversion: guarantee
pass (zero-slot tasks, priority order) before growth pass; a released
slot always re-enters allocation from the top, never handed back
locally.
2. Probe pool size 4, outside the segment budget — confirmed.
3. set_max_active_segments is drain-not-kill; in-flight segments run to
their boundary.
4. Priority = an ordered TaskId list pushed on change, not an integer,
not per tick — tie-breaking is DAEMON policy.
5. on_budget_changed coalesced at 4 Hz, immediate on the tasks_starved
zero-crossing.
Amendments: (A1) add "yield" to §3.3 as the non-neutral slot-transfer op
that satisfies min-1 when the budget is full — "steal" stays slot-neutral;
(A2) "admission implies progress" is bounded-delay not immediate — bounded
by an incumbent's next segment boundary, capped by the stall timeout, so
§3.6's 2 s assertion window is too tight; (A3) add starved_tasks() /
starved_since() and pin the segments_active / tasks_starved definitions
(a connecting segment counts as a held slot and is not starvation).
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
Minor bump on 1.0.0, per core/docs/buffer-sizing.md.
B4 — bufferBytes bounds corrected in all four locations (DownloadSpec,
TaskDetail, download.update's patch, Settings.connection.bufferBytes): was
4 KiB-8 MiB with no stated default, now 64 KiB-16 MiB with a 1 MiB default.
64 KiB because 4 KiB is smaller than one libcurl HTTP/2 write-callback delivery;
16 MiB because throughput from write size is flat past ~1-4 MiB and past 16 MiB
there is stall-cover left to buy but no memory left to spend it on; 1 MiB
default because it is the only candidate for which docs/04's 60 MB RSS target
actually holds once buffers are counted per segment, not per download.
Two new settings keys: connection.maxTotalBufferBytes (128 MiB default) and
connection.maxActiveSegments (32 default). Without them CORE's clamp — reduce
every live segment's buffer to fit the global cap — has no wire configuration
surface, and "20 active downloads" has no meaning distinct from 160 live TLS
connections.
B2a — TaskDetail.effectiveBufferBytes: what a segment is actually using right
now, after the clamp. Placed on TaskDetail next to bufferBytes, following the
requested/effective pattern ADR 0010 already established for segments. The
download.get fixture now demonstrates a real clamp (16 MiB requested, 4 MiB
effective) rather than a case where the cap happens not to bind.
docs/04-engine-design.md §4 and §8 updated in the same change per CORE's
request and CLAUDE.md rule 5: the RSS target is now stated as conditional on
maxActiveSegments = 32, and the old 4 MiB/64 MiB/256 MiB numbers are corrected
to match the schema. ADR 0012 records the reasoning and explicitly keeps the
60 MB target over CORE's offered 120 MB alternative, with the arithmetic that
makes 60 MB achievable with margin.
Numbered 0012 rather than 0011: DAEMON is independently drafting ADR 0011
(admission control / segment budget split) in a peer session at time of
writing, so 0011 was reserved to avoid a collision.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012fgjnqFCS5h5L7gZTZo3rV
First pass counted one buffer per download; it is one per segment. 20
active downloads at the default 8 segments = 160 buffers, so at 20 tasks
the binding constraint is the global cap, not the per-segment default —
256 MiB and the "<=60 MB RSS / 20 downloads" target (line 125) cannot both
hold whatever the default is.
Floor (64 KiB) and ceiling (16 MiB) unchanged — the 256/64 unreachability
argument is stronger under per-segment accounting. Changes:
- default 1 MiB (was 2): with the cap below, 32 live segments x 1 MiB =
32 MiB buffers -> ~45-50 MiB RSS, line 125 holds with margin.
- NEW maxActiveSegments (default 32): a global concurrent-segment cap is
the actual mechanism that bounds "20 active downloads"; docs/01 §2
implies it, docs/04 never states it. Without it no buffer policy hits
60 MB.
- maxTotalBufferBytes 128 MiB (was 256) and it must be ADDED to the
contract — currently absent, so the clamp CORE implements has no wire
representation and Options can't show/set it. Folded into B2a.
- line 125: keep 60 MB "given maxActiveSegments=32 and default buffers",
or explicitly raise to 120 MB — ADR records which. Flagged that
changing it is a defensible outcome CORE owns, not a number that
quietly loses.
- bufferBytes bounds are in FOUR schema files, not three:
Settings.schema.json connection.bufferBytes also has 4096-8388608.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
Three corrections into 1.0.0, all of which would be major bumps once the
contract has landed. It has not: main still carries 1.0.0-draft, so these are
corrections to an unpublished version rather than changes to a released one.
ADR 0010 records that and the reasoning behind each.
B1 — TaskError.code was a bare integer, and the integer space in the contract is
JSON-RPC's, which is a different thing; TaskError's own description said so while
typing its code as one. Freeze TaskErrorCode: a string enum mirroring vdm::Error
by name and in order, all 27 failure values, verified against
core/include/vdm/util/error.hpp mechanically. ErrorCode says why a call failed;
TaskErrorCode says why a download failed, and a download fails while every RPC
succeeds. Adds TaskError.cause so max_retries_exhausted names what kept failing.
B2 — TaskSummary.segments is now explicitly the effective count in use right now,
after the per-host cap and the non-resumable demotion to 1. DownloadSpec.segments
and download.update's patch say they are the requested value.
B3 — Segment.endByte's "minimum: 0" contradicted the description's own empty-range
encoding of startByte - 1, which is -1 for the first segment of every download.
Empty ranges are no longer representable and are not needed. The range stays
CLOSED and INCLUSIVE, matching the HTTP Range header the two fields are copied
into verbatim, and that is now stated in the schema, the README, an ADR, a fixture
assertion and a conformance check. CORE asked for half-open and gets a written
notice rather than a silent schema edit. Segment state spells 'downloading' as
CORE asked, not 'receiving'.
check_contract.py now enforces segment contiguity, coverage of exactly
[0, sizeBytes-1], downloadedBytes within the range size, and the entry count
matching TaskSummary.segments. The download.get fixture claimed 8 segments while
carrying 2; it now carries 8 contiguous ones covering the whole file.
contracts/proto-answers-m1.md answers every item in core/docs/proto-requests-m1.md,
including the ones not being landed now: B2a and F2 accepted as follow-ups, F1
answered with the notify path for M1, F3 already frozen as a Checksum object
rather than a string, and D1 left for DAEMON to draft as the three-way ADR it is.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012fgjnqFCS5h5L7gZTZo3rV
buffer-sizing.md: the frozen 4 KiB–8 MiB and docs/04's 64 KiB–64 MiB both
miss. Recommend 64 KiB – 16 MiB, default 2 MiB, max_total_buffer_bytes
unchanged at 256 MiB:
- 4 KiB floor is smaller than one libcurl write callback -> a syscall per
chunk; 64 KiB is the smallest floor that coalesces.
- throughput vs write size is flat past ~8 MiB on NVMe; 8–16 MiB is
disk-stall absorption headroom for the fast-pipe/slow-disk case; 64 MiB
is cache pressure for zero gain.
- 32 segments x 64 MiB = 2 GiB vs the 256 MiB cap means the docs/04 max is
unreachable past 4 total active segments — a misleading Options value.
16 MiB is reachable for single-/light-multitask and clamps to 8 MiB
under heavy parallelism, which is correct.
- default 4 MiB x 20 downloads = 80 MiB, busting the "<=60 MB RSS / 20
downloads" DoD; 2 MiB fits. Filed as request B4.
proto-requests-m1.md: B3 endByte accepted as inclusive (HTTP Range
semantics, no curl-boundary off-by-one); [start,end) ask withdrawn; stage
6 designed against inclusive. New B3a: the Content-Length: 0 whole-file
case needs a representable zero-length segment — min_segment_bytes means
CORE never makes empty segments mid-download, so it's only the degenerate
case; mild preference for startByte+length over an endByte=startByte-1
sentinel.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
Schemas for the whole v1 surface: 38 methods, 9 events, 25 named types and the
JSON-RPC envelope, with x-privileged / x-transports / x-deadlineMs / x-errors
annotations that both generators emit as data rather than prose.
Four generators over one IR (contracts/codegen/schema_ir.py), so the C++ structs,
the TypeScript types and the OpenRPC document cannot disagree about what the
contract says:
gen_cpp.py -> core/generated/velox_proto.{hpp,cpp}
gen_ts.py -> extension/src/shared/protocol/
gen_openrpc.py -> contracts/openrpc.json
gen_cpp_conformance.py -> tests/conformance/cpp/fixture_dispatcher.hpp
Inbound parsing never throws: parse<T>() returns std::expected<T, ParseError> and
nlohmann's throwing ADL from_json is deliberately not emitted. Schema constraints
(minimum, maxLength, pattern, ...) become real runtime checks in both languages —
the daemon does not trust the extension and the extension does not trust the
daemon.
59 golden fixtures: a success case per method, 12 error cases, 9 events. Replayed
by tests/conformance/ against both the generated C++ and a live server over both
transports. tools/mockd serves the same fixtures with unhappy-path flags so the
GUI and EXT lanes never wait for veloxd.
run.sh also proves capture.offer fails open: with a daemon answering slower than
750 ms the client gives up and lets Firefox take the download.
core/generated/ is libveloxproto, a separate target from libveloxcore, which
still never sees JSON — see docs/adr/0009.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012fgjnqFCS5h5L7gZTZo3rV
Pure formatting, no behaviour change. PKG landed .clang-format (Google
base, 4-space indent, 100 cols); this brings util/ and the test harness
into conformance so `clang-format --dry-run -Werror` is clean. Build and
all six test binaries unchanged and green.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
util/ carries no wire surface, so it lands before the contract freeze.
- error: enum class Error, the engine-wide failure taxonomy; is_retryable
enumerates every value (no default:) so -Wswitch forces the retry
decision on each future addition. ErrorInfo carries context/http_status.
- result: Result<T> over std::expected<T, ErrorInfo>, Result<void>,
VDM_TRY / VDM_TRY_ASSIGN. Errors returned, never thrown, on the
transfer path.
- bytes: span aliases, LE load_le/store_le (debug-asserted precondition,
not input validation), and a bounds-checked latching ByteReader for the
.veloxpart.meta reader.
- event_bus: typed thread-safe pub/sub; header states plainly that
unsubscribe is not a quiesce point and download_task will need its own
drain.
- thread_pool: std::jthread pool; dtor joins in the body before members
die (fixed a use-after-destruction on cv_/mu_). Header notes shutdown is
drain-only and DAEMON will need a cancel mode.
- log: sink interface (core does no I/O); DAEMON installs one.
Tested: -Werror clean, 6 binaries green under plain / ASan+UBSan / TSan.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
Self-contained core/CMakeLists.txt (veloxcore STATIC + velox::core alias),
warnings at target scope so the sanitizer presets' CMAKE_CXX_FLAGS override
doesn't drop -Werror. vtest: ~150-line header-only harness (VT_TEST /
VT_CHECK / VT_REQUIRE / VT_CHECK_EQ) behind a one-function vdm_add_test(),
so the swap to a real framework once PKG picks one is mechanical.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
Lays out Velox Download Manager (IDM-class download manager for Ubuntu
26.04) as a monorepo ready for parallel lane development. No implementation
code by design.
- docs/: architecture, roadmap M0-M7, IDM-parity GUI spec, engine design,
Firefox extension spec, risks/spikes, packaging
- contracts/: wire-contract skeleton (JSON Schema + fixture templates) —
the single synchronization point between lanes
- docs/agents/: one brief per lane (PROTO, CORE, DAEMON, GUI, EXT, PKG/QA)
with owned directories, build order and definition of done
- CLAUDE.md: rules of engagement — lane ownership, layering, non-negotiables
- CMake scaffolding with dev/tsan/release/ci presets
Two environment findings shape the design: Firefox here is the Mozilla snap
(native-messaging risk, so the extension carries a loopback-WebSocket
fallback), and Wayland forbids passive clipboard monitoring (so clipboard
capture is explicit-action-first).
Co-Authored-By: Claude Opus 5 <[email protected]>
samiandClaude Sonnet 5