Compare commits
10
Commits
lane/gui
...
lane/pkg-qa
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
3e553f08c0 | ||
|
|
1d359af5a3 | ||
|
|
75536304bb | ||
|
|
27865888be | ||
|
|
939a19b7ea | ||
|
|
79c29b47e8 | ||
|
|
8e7d14ba7e | ||
|
|
c1c5c82f8b | ||
|
|
4e177ec809 | ||
|
|
b5c6e1f47d |
@@ -18,8 +18,9 @@ policy so it can be re-applied or audited.
|
|||||||
| `bootstrap-script-2604` | now — real `--with-clang` install in a 26.04 container; the release the project ships on |
|
| `bootstrap-script-2604` | now — real `--with-clang` install in a 26.04 container; the release the project ships on |
|
||||||
| `build (gcc)` / `build (clang)` | now — core, daemon and gui have merged |
|
| `build (gcc)` / `build (clang)` | now — core, daemon and gui have merged |
|
||||||
| `sanitizers (dev)` / `sanitizers (tsan)` | now — core, daemon and gui have merged |
|
| `sanitizers (dev)` / `sanitizers (tsan)` | now — core, daemon and gui have merged |
|
||||||
| `conformance` | **now — `tests/conformance/` has landed; this is the M0 exit gate** |
|
| `conformance` | **now — `tests/conformance/` has landed; this is the M0 exit gate.** Includes the live-`veloxd` runner (step 3b of `run.sh`), unconditional in the script — see `docs/adr/0019-live-veloxd-conformance-is-required.md`. |
|
||||||
| `extension-lint` | now — `extension/` has merged (MV3 manifest + esbuild build) |
|
| `extension-lint` | now — `extension/` has merged (MV3 manifest + esbuild build) |
|
||||||
|
| `gui-dod` | now — `gui/tests/dod/` has landed (GUI M1 DoD gates R3: `scroll-60fps`, `unhappy-path`); see `tests/integration/README.md#gui-m1-definition-of-done-gates-r3`. `gui-dod-nightly` (`rss-flat`) is schedule-only and cannot be a required PR check. |
|
||||||
|
|
||||||
`clang-tidy` is intentionally **not** required through M1 (`continue-on-error: true`,
|
`clang-tidy` is intentionally **not** required through M1 (`continue-on-error: true`,
|
||||||
`.clang-tidy` has `WarningsAsErrors: ''`). Make it required at M2.
|
`.clang-tidy` has `WarningsAsErrors: ''`). Make it required at M2.
|
||||||
|
|||||||
@@ -248,3 +248,56 @@ jobs:
|
|||||||
run: cmake --build --preset dev --target veloxd
|
run: cmake --build --preset dev --target veloxd
|
||||||
- name: Nightly integration run
|
- name: Nightly integration run
|
||||||
run: python3 tests/integration/nightly_run.py --veloxd build/dev/bin/veloxd --tasks 50 --timeout 180
|
run: python3 tests/integration/nightly_run.py --veloxd build/dev/bin/veloxd --tasks 50 --timeout 180
|
||||||
|
|
||||||
|
gui-dod:
|
||||||
|
# Per-PR GUI M1 DoD gates (gui/docs/pkg-qa-requests-m1.md R3): scroll-60fps and
|
||||||
|
# unhappy-path. The 10-minute rss-flat gate is gui-dod-nightly, not here. GUI's
|
||||||
|
# harness defaults QT_QPA_PLATFORM=offscreen itself, so no Xvfb/compositor needed.
|
||||||
|
# See tests/integration/README.md#gui-m1-definition-of-done-gates-r3 for what each
|
||||||
|
# gate catches and the forced-failure transcript proving it isn't vacuous.
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
steps:
|
||||||
|
- uses: actions/checkout@v4
|
||||||
|
- name: Bootstrap toolchain
|
||||||
|
run: sudo ./tools/bootstrap.sh
|
||||||
|
- uses: actions/setup-node@v4
|
||||||
|
with:
|
||||||
|
node-version: '22' # tools/mockd
|
||||||
|
- name: Configure + build
|
||||||
|
run: |
|
||||||
|
cmake --preset dev
|
||||||
|
cmake --build --preset dev --target gui-dod-harness
|
||||||
|
- name: Install mockd
|
||||||
|
run: cd tools/mockd && npm ci
|
||||||
|
- name: Gates
|
||||||
|
run: |
|
||||||
|
gui/tests/dod/run.sh scroll-60fps --json scroll.json
|
||||||
|
gui/tests/dod/run.sh unhappy-path --json unhappy.json
|
||||||
|
- uses: actions/upload-artifact@v4
|
||||||
|
if: always()
|
||||||
|
with:
|
||||||
|
name: gui-dod-${{ github.run_id }}
|
||||||
|
path: "*.json"
|
||||||
|
|
||||||
|
gui-dod-nightly:
|
||||||
|
if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
steps:
|
||||||
|
- uses: actions/checkout@v4
|
||||||
|
- name: Bootstrap toolchain
|
||||||
|
run: sudo ./tools/bootstrap.sh
|
||||||
|
- uses: actions/setup-node@v4
|
||||||
|
with:
|
||||||
|
node-version: '22'
|
||||||
|
- name: Configure + build
|
||||||
|
run: |
|
||||||
|
cmake --preset dev
|
||||||
|
cmake --build --preset dev --target gui-dod-harness
|
||||||
|
- run: cd tools/mockd && npm ci
|
||||||
|
- name: RSS soak (10 min)
|
||||||
|
run: gui/tests/dod/run.sh rss-flat --json rss.json
|
||||||
|
- uses: actions/upload-artifact@v4
|
||||||
|
if: always()
|
||||||
|
with:
|
||||||
|
name: gui-dod-rss-${{ github.run_id }}
|
||||||
|
path: rss.json
|
||||||
|
|||||||
@@ -506,7 +506,12 @@ def emit_field_parse(f: Field, indent: str) -> list[str]:
|
|||||||
f'{i} const auto it = j.find("{f.name}");']
|
f'{i} const auto it = j.find("{f.name}");']
|
||||||
if f.optional:
|
if f.optional:
|
||||||
# Absent and null mean the same thing: the field is not set. A client that omits
|
# Absent and null mean the same thing: the field is not set. A client that omits
|
||||||
# a nullable field and one that sends null are treated identically on purpose.
|
# a nullable field and one that sends null are treated identically on purpose --
|
||||||
|
# correct for create-style params, where there is no existing value to distinguish
|
||||||
|
# "never set" from "explicitly cleared". Patch-style fields need the distinction
|
||||||
|
# (download.update's patch: "an explicit null clears a nullable field") and get an
|
||||||
|
# opt-in exception via x-clearable per ADR 0018 (not implemented yet: this is the
|
||||||
|
# decision record, not the generator change).
|
||||||
o.append(f"{i} if (it != j.end() && !it->is_null()) {{")
|
o.append(f"{i} if (it != j.end() && !it->is_null()) {{")
|
||||||
o += emit_value_parse(f.type, "(*it)", "val", "fp", i + " ")
|
o += emit_value_parse(f.type, "(*it)", "val", "fp", i + " ")
|
||||||
o.append(f"{i} out.{m} = std::move(val);")
|
o.append(f"{i} out.{m} = std::move(val);")
|
||||||
|
|||||||
@@ -21,7 +21,7 @@ fixtures/
|
|||||||
|
|
||||||
```jsonc
|
```jsonc
|
||||||
{
|
{
|
||||||
"name": "download.add — start an ISO now, into the Programs category",
|
"name": "download.add — add an ISO for later, into the Programs category",
|
||||||
"description": "Why this case is worth pinning.",
|
"description": "Why this case is worth pinning.",
|
||||||
"transport": "uds", // optional: replay only on this transport
|
"transport": "uds", // optional: replay only on this transport
|
||||||
"requires": "...", // optional: a condition a plain server cannot produce
|
"requires": "...", // optional: a condition a plain server cannot produce
|
||||||
@@ -81,3 +81,27 @@ cases in `tests/integration/`.
|
|||||||
correct response is *no response*: past 750 ms the extension must abandon the offer and let
|
correct response is *no response*: past 750 ms the extension must abandon the offer and let
|
||||||
Firefox download normally. A download manager that eats downloads when its daemon is down
|
Firefox download normally. A download manager that eats downloads when its daemon is down
|
||||||
is worse than no download manager.
|
is worse than no download manager.
|
||||||
|
|
||||||
|
## No fixture may pair a real external URL with `startMode: "now"`
|
||||||
|
|
||||||
|
This suite replays every fixture against a real, live `veloxd` (`tests/conformance/run.sh`),
|
||||||
|
not just `mockd`. `mockd` never actually fetches anything, so it hid this for a while: a
|
||||||
|
fixture with `startMode: "now"` (or `"queue"` into a running queue — anything that gets
|
||||||
|
admitted to the scheduler right away) and a real, resolvable URL makes a **real** daemon
|
||||||
|
actually start downloading it, for real, onto whatever machine runs the suite. This
|
||||||
|
happened — twice, with `download.add.json` pointed at a ~6 GB Ubuntu ISO, straight into the
|
||||||
|
developer's real `~/Downloads`.
|
||||||
|
|
||||||
|
The fix in each case is one of:
|
||||||
|
- `startMode: "later"` — exercises the add path (validation, category assignment, the
|
||||||
|
event) without ever handing the task to the engine;
|
||||||
|
- a URL under `example.org`/`example.com` (IANA-reserved for exactly this, RFC 2606) —
|
||||||
|
resolvable enough to validate as a URL, never a real download source;
|
||||||
|
- `requires`, if the fixture's entire point needs a real transfer to fail in a specific way
|
||||||
|
(see `errors/download.add.disk-full.json`) — skipped by default, so it only ever runs
|
||||||
|
where the condition has actually been arranged.
|
||||||
|
|
||||||
|
A real `saveDir` gets the same treatment for the same reason: an absolute path like
|
||||||
|
`/home/sami/Downloads/...` only means anything on the machine that fixture was written on.
|
||||||
|
Omit `saveDir` and let `saveTo.defaultDir` apply, or use a relative-feeling path under a
|
||||||
|
root the runner controls.
|
||||||
|
|||||||
@@ -1,22 +1,23 @@
|
|||||||
{
|
{
|
||||||
"name": "capture.offer — attachment on a monitored type is taken",
|
"name": "capture.offer — attachment on a monitored type is taken",
|
||||||
"description": "Golden fixture. tests/conformance replays this against the real daemon AND the TS client. If either side drifts, this goes red before the lanes ever integrate.",
|
"description": "Golden fixture. tests/conformance replays this against the real daemon AND the TS client. If either side drifts, this goes red before the lanes ever integrate. url is example.org (RFC 2606), not a real download source: 'take' against a real veloxd (tests/conformance/run.sh) admits a real task and hands it to the engine for real, and no fixture may do that against a real external URL. contentLength is a plausible-but-small 5 MiB rather than a real ISO's size: the 'Programs' category's saveDir is a migration-seeded builtin (~/Downloads/Programs, daemon/src/store/migrations/0001_initial.sql), not something an isolated test run's settings can redirect, so 'take' always sparse-preallocates into that real path on whatever machine runs this suite -- keeping the declared size small keeps that footprint trivial instead of a real ISO's worth of disk. transport is uds only: a real 'take' persists an active task, so replaying this same fixture again on a second live transport against the same daemon would correctly dedupe against it (capture.offer dedupes by exact URL) and get 'ignore' instead -- an artifact of replaying one fixture against one shared daemon over two transports, not a behaviour to golden.",
|
||||||
|
"transport": "uds",
|
||||||
"request": {
|
"request": {
|
||||||
"jsonrpc": "2.0",
|
"jsonrpc": "2.0",
|
||||||
"id": 42,
|
"id": 42,
|
||||||
"method": "capture.offer",
|
"method": "capture.offer",
|
||||||
"params": {
|
"params": {
|
||||||
"url": "https://releases.ubuntu.com/26.04/ubuntu-26.04-desktop-amd64.iso",
|
"url": "https://example.org/dl/ubuntu-26.04-desktop-amd64.iso",
|
||||||
"method": "GET",
|
"method": "GET",
|
||||||
"tabUrl": "https://releases.ubuntu.com/26.04/",
|
"tabUrl": "https://example.org/26.04/",
|
||||||
"headers": {
|
"headers": {
|
||||||
"User-Agent": "Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:154.0) Gecko/20100101 Firefox/154.0",
|
"User-Agent": "Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:154.0) Gecko/20100101 Firefox/154.0",
|
||||||
"Referer": "https://releases.ubuntu.com/26.04/",
|
"Referer": "https://example.org/26.04/",
|
||||||
"Accept": "*/*"
|
"Accept": "*/*"
|
||||||
},
|
},
|
||||||
"cookies": [],
|
"cookies": [],
|
||||||
"contentType": "application/octet-stream",
|
"contentType": "application/octet-stream",
|
||||||
"contentLength": 6228541440,
|
"contentLength": 5242880,
|
||||||
"contentDisposition": "attachment; filename=\"ubuntu-26.04-desktop-amd64.iso\"",
|
"contentDisposition": "attachment; filename=\"ubuntu-26.04-desktop-amd64.iso\"",
|
||||||
"filename": "ubuntu-26.04-desktop-amd64.iso",
|
"filename": "ubuntu-26.04-desktop-amd64.iso",
|
||||||
"origin": "moz-extension://11111111-2222-3333-4444-555555555555"
|
"origin": "moz-extension://11111111-2222-3333-4444-555555555555"
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"name": "download.add \u2014 start an ISO now, into the Programs category",
|
"name": "download.add — add an ISO for later, into the Programs category",
|
||||||
"description": "The ordinary add path. saveDir is canonicalized and checked against the allowed roots before anything is written.",
|
"description": "The ordinary add path. saveDir is canonicalized and checked against the allowed roots before anything is written. startMode is 'later' deliberately: this suite replays against a real veloxd (tests/conformance/run.sh), and a real daemon given startMode 'now' would actually start fetching url for real. No fixture may pair a real external URL with startMode 'now' -- see contracts/fixtures/README.md.",
|
||||||
"request": {
|
"request": {
|
||||||
"jsonrpc": "2.0",
|
"jsonrpc": "2.0",
|
||||||
"id": 11,
|
"id": 11,
|
||||||
@@ -8,10 +8,9 @@
|
|||||||
"params": {
|
"params": {
|
||||||
"url": "https://releases.ubuntu.com/26.04/ubuntu-26.04-desktop-amd64.iso",
|
"url": "https://releases.ubuntu.com/26.04/ubuntu-26.04-desktop-amd64.iso",
|
||||||
"filename": "ubuntu-26.04-desktop-amd64.iso",
|
"filename": "ubuntu-26.04-desktop-amd64.iso",
|
||||||
"saveDir": "/home/sami/Downloads/Programs",
|
|
||||||
"categoryId": "programs",
|
"categoryId": "programs",
|
||||||
"segments": 8,
|
"segments": 8,
|
||||||
"startMode": "now"
|
"startMode": "later"
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
"response": {
|
"response": {
|
||||||
@@ -19,13 +18,13 @@
|
|||||||
"id": 11,
|
"id": 11,
|
||||||
"result": {
|
"result": {
|
||||||
"taskId": "$uuid",
|
"taskId": "$uuid",
|
||||||
"state": "connecting",
|
"state": "paused",
|
||||||
"duplicate": null
|
"duplicate": null
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
"assertions": [
|
"assertions": [
|
||||||
"the .veloxpart file is created sparse and preallocated at the final size",
|
"saveDir is omitted here on purpose: it resolves to saveTo.defaultDir, which is itself checked against saveTo.allowedRoots the same way an explicit saveDir would be -- see errors/download.add.invalid-path.json for the -32011 case",
|
||||||
"saveDir resolves inside saveTo.allowedRoots, or the call fails -32011 having written nothing",
|
"startMode 'later' lands the task in 'paused' and never hands it to the engine, so nothing is fetched and no .veloxpart is created yet -- that only happens once the task is actually started (download.start.json, or startMode 'now'/'queue' against a source this suite controls)",
|
||||||
"event.task.added is emitted to every subscriber before this reply is sent"
|
"event.task.added is emitted to every subscriber before this reply is sent"
|
||||||
]
|
]
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"name": "limiter.get \u2014 the limiter is off",
|
"name": "limiter.get \u2014 the limiter is off",
|
||||||
"description": "globalBps still carries the last configured value so the GUI can restore it when the user re-enables the limit.",
|
"description": "globalBps still carries the last configured value so the GUI can restore it when the user re-enables the limit. applyToRunning is a write-only instruction on limiter.set (\"retune already-running transfers now\", not a persisted setting), so it never comes back from get.",
|
||||||
"request": {
|
"request": {
|
||||||
"jsonrpc": "2.0",
|
"jsonrpc": "2.0",
|
||||||
"id": 52,
|
"id": 52,
|
||||||
@@ -12,11 +12,11 @@
|
|||||||
"id": 52,
|
"id": 52,
|
||||||
"result": {
|
"result": {
|
||||||
"enabled": false,
|
"enabled": false,
|
||||||
"globalBps": 2097152,
|
"globalBps": 2097152
|
||||||
"applyToRunning": false
|
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
"assertions": [
|
"assertions": [
|
||||||
"enabled false means no throttling regardless of globalBps"
|
"enabled false means no throttling regardless of globalBps",
|
||||||
|
"applyToRunning is absent, not false: it's meaningless outside a limiter.set call"
|
||||||
]
|
]
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -64,6 +64,19 @@ std::string lower(std::string s) {
|
|||||||
return s;
|
return s;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// scheme://host[:port] of `url`, with no path/query/fragment -- what docs/04 §7's "403
|
||||||
|
// after redirect: retry once with the original referrer" retries with as the Referer
|
||||||
|
// header. Empty on an unparseable URL (the caller just won't get a referrer retry).
|
||||||
|
std::string origin_of(std::string_view url) {
|
||||||
|
auto s = net::split_url(url);
|
||||||
|
if (!s.valid)
|
||||||
|
return {};
|
||||||
|
std::string out = s.scheme + "://" + s.host;
|
||||||
|
if (s.port)
|
||||||
|
out += ":" + std::to_string(*s.port);
|
||||||
|
return out;
|
||||||
|
}
|
||||||
|
|
||||||
} // namespace
|
} // namespace
|
||||||
|
|
||||||
// What to do once every worker has drained (see DownloadTaskState::begin_drain_locked).
|
// What to do once every worker has drained (see DownloadTaskState::begin_drain_locked).
|
||||||
@@ -88,6 +101,7 @@ struct SegWorker {
|
|||||||
bool needs_auth = false;
|
bool needs_auth = false;
|
||||||
bool wrong_status = false;
|
bool wrong_status = false;
|
||||||
bool range_bad = false;
|
bool range_bad = false;
|
||||||
|
bool forbidden = false; // 403 -- docs/04 §7's "retry once with the original referrer"
|
||||||
bool auth_handshake = false; // saw a 401/407 and let libcurl resend with credentials
|
bool auth_handshake = false; // saw a 401/407 and let libcurl resend with credentials
|
||||||
std::string resp_etag, resp_last_modified; // captured on a wrong_status 200, for demote
|
std::string resp_etag, resp_last_modified; // captured on a wrong_status 200, for demote
|
||||||
std::optional<ErrorInfo> flush_error;
|
std::optional<ErrorInfo> flush_error;
|
||||||
@@ -135,6 +149,16 @@ struct DownloadTaskState : std::enable_shared_from_this<DownloadTaskState> {
|
|||||||
std::optional<std::uint64_t> total_size;
|
std::optional<std::uint64_t> total_size;
|
||||||
std::string origin_host;
|
std::string origin_host;
|
||||||
|
|
||||||
|
// docs/04 §7's "403 after redirect: retry once with the original referrer" -- many
|
||||||
|
// CDNs 403 a bare/foreign Referer. Starts as spec.referrer (the browser's, verbatim);
|
||||||
|
// start_worker_locked() sends this, not spec.referrer directly, so a 403 retry can
|
||||||
|
// override it (to the download URL's own origin) without touching what the caller
|
||||||
|
// actually asked for. referrer_retried bounds it to exactly once per task -- a second
|
||||||
|
// 403 with a same-origin Referer already set is a real, honest failure
|
||||||
|
// (Error::forbidden), not something a referrer swap can fix.
|
||||||
|
std::string effective_referrer;
|
||||||
|
bool referrer_retried = false;
|
||||||
|
|
||||||
std::unique_ptr<segment::Segmenter> seg;
|
std::unique_ptr<segment::Segmenter> seg;
|
||||||
std::unique_ptr<io::SparseFile> file;
|
std::unique_ptr<io::SparseFile> file;
|
||||||
std::unordered_map<std::uint32_t, std::unique_ptr<SegWorker>> workers;
|
std::unordered_map<std::uint32_t, std::unique_ptr<SegWorker>> workers;
|
||||||
@@ -164,7 +188,7 @@ struct DownloadTaskState : std::enable_shared_from_this<DownloadTaskState> {
|
|||||||
std::vector<std::function<void()>> deferred;
|
std::vector<std::function<void()>> deferred;
|
||||||
|
|
||||||
DownloadTaskState(TaskHost &h, TaskId i, DownloadSpec s, DownloadCallbacks c)
|
DownloadTaskState(TaskHost &h, TaskId i, DownloadSpec s, DownloadCallbacks c)
|
||||||
: host(h), id(i), spec(std::move(s)), cbs(std::move(c)) {}
|
: host(h), id(i), spec(std::move(s)), cbs(std::move(c)), effective_referrer(spec.referrer) {}
|
||||||
|
|
||||||
// --- deferred callbacks -------------------------------------------------------------
|
// --- deferred callbacks -------------------------------------------------------------
|
||||||
void defer(std::function<void()> fn) {
|
void defer(std::function<void()> fn) {
|
||||||
@@ -286,7 +310,7 @@ void DownloadTaskState::restart_probe(bool with_auth) {
|
|||||||
pr.url = spec.url;
|
pr.url = spec.url;
|
||||||
pr.headers = spec.headers;
|
pr.headers = spec.headers;
|
||||||
pr.cookies = spec.cookies;
|
pr.cookies = spec.cookies;
|
||||||
pr.referrer = spec.referrer;
|
pr.referrer = effective_referrer;
|
||||||
pr.user_agent = spec.user_agent;
|
pr.user_agent = spec.user_agent;
|
||||||
pr.proxy = spec.proxy;
|
pr.proxy = spec.proxy;
|
||||||
if (with_auth)
|
if (with_auth)
|
||||||
@@ -299,12 +323,37 @@ void DownloadTaskState::restart_probe(bool with_auth) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
void DownloadTaskState::on_probe_result(Result<net::ProbeResult> r) {
|
void DownloadTaskState::on_probe_result(Result<net::ProbeResult> r) {
|
||||||
|
bool retry_probe_with_referrer = false;
|
||||||
{
|
{
|
||||||
std::unique_lock lk(mu);
|
std::unique_lock lk(mu);
|
||||||
if (retired.load() || is_terminal(state))
|
if (retired.load() || is_terminal(state))
|
||||||
return;
|
return;
|
||||||
if (!r.has_value()) {
|
if (!r.has_value()) {
|
||||||
fail_locked(std::move(r).error());
|
ErrorInfo e = std::move(r).error();
|
||||||
|
// docs/04 §7's referrer retry applies here too: a probe (HEAD, or the
|
||||||
|
// ranged-GET fallback when HEAD is refused -- probe.cpp) can be the request
|
||||||
|
// that actually gets 403'd, before any segment worker exists to retry it
|
||||||
|
// (net::Prober builds its own request from ProbeRequest::referrer, not
|
||||||
|
// through start_worker_locked() -- see restart_probe()'s use of
|
||||||
|
// effective_referrer below). Same one-shot bound via referrer_retried as the
|
||||||
|
// worker-level retry (seg_finished's w->forbidden branch) shares.
|
||||||
|
if (e.code == Error::forbidden && !referrer_retried) {
|
||||||
|
referrer_retried = true;
|
||||||
|
effective_referrer = origin_of(spec.url);
|
||||||
|
retry_probe_with_referrer = true;
|
||||||
|
} else if (e.code == Error::forbidden) {
|
||||||
|
// Already retried with the origin referrer and still 403 -- not something
|
||||||
|
// another blind retry fixes (an expired signed URL, a private resource).
|
||||||
|
// Ask rather than fail outright, the same "ask, don't just fail" shape as
|
||||||
|
// wrong_status/range_bad/the worker-level 403 branch: refresh_url() is a
|
||||||
|
// no-op once the task is terminal, and tools/testserver's expiring-signed-
|
||||||
|
// url mode (also a bare 403, indistinguishable from any other without
|
||||||
|
// parsing the body -- CLAUDE.md §3, core never does) is meant to be
|
||||||
|
// recovered exactly that way.
|
||||||
|
auto_pause_locked(std::move(e), false, true);
|
||||||
|
} else {
|
||||||
|
fail_locked(std::move(e));
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
probe = std::move(r).value();
|
probe = std::move(r).value();
|
||||||
have_probe = true;
|
have_probe = true;
|
||||||
@@ -319,6 +368,8 @@ void DownloadTaskState::on_probe_result(Result<net::ProbeResult> r) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
flush_deferred();
|
flush_deferred();
|
||||||
|
if (retry_probe_with_referrer)
|
||||||
|
restart_probe(false);
|
||||||
}
|
}
|
||||||
|
|
||||||
void DownloadTaskState::finish_probe_locked() {
|
void DownloadTaskState::finish_probe_locked() {
|
||||||
@@ -472,7 +523,7 @@ void DownloadTaskState::start_worker_locked(std::uint32_t seg_idx) {
|
|||||||
req.url = current_url();
|
req.url = current_url();
|
||||||
req.headers = spec.headers;
|
req.headers = spec.headers;
|
||||||
req.cookies = spec.cookies;
|
req.cookies = spec.cookies;
|
||||||
req.referrer = spec.referrer;
|
req.referrer = effective_referrer;
|
||||||
req.user_agent = spec.user_agent;
|
req.user_agent = spec.user_agent;
|
||||||
req.proxy = spec.proxy;
|
req.proxy = spec.proxy;
|
||||||
req.auth = spec.auth;
|
req.auth = spec.auth;
|
||||||
@@ -552,6 +603,10 @@ net::DataAction DownloadTaskState::seg_head(std::uint32_t seg_idx, const net::Re
|
|||||||
w->range_bad = true;
|
w->range_bad = true;
|
||||||
return net::DataAction::abort;
|
return net::DataAction::abort;
|
||||||
}
|
}
|
||||||
|
if (h.status == 403) {
|
||||||
|
w->forbidden = true;
|
||||||
|
return net::DataAction::abort;
|
||||||
|
}
|
||||||
if (h.status >= 400)
|
if (h.status >= 400)
|
||||||
return net::DataAction::abort;
|
return net::DataAction::abort;
|
||||||
seg->set_segment_state(seg_idx, segment::SegState::downloading);
|
seg->set_segment_state(seg_idx, segment::SegState::downloading);
|
||||||
@@ -637,7 +692,7 @@ void DownloadTaskState::seg_finished(std::uint32_t seg_idx, Result<net::Transfer
|
|||||||
// (content-length-mismatch's honest-length lie, flaky-reset's tail, a proxy RST after
|
// (content-length-mismatch's honest-length lie, flaky-reset's tail, a proxy RST after
|
||||||
// the last byte). If the segment is fully covered, that's a success.
|
// the last byte). If the segment is fully covered, that's a success.
|
||||||
if (seg && !cancel_requested && !pause_requested && !w->needs_auth && !w->wrong_status &&
|
if (seg && !cancel_requested && !pause_requested && !w->needs_auth && !w->wrong_status &&
|
||||||
!w->flush_error && !w->range_bad) {
|
!w->flush_error && !w->range_bad && !w->forbidden) {
|
||||||
const std::uint64_t len = seg->segment_end(seg_idx) - seg->segment_start(seg_idx) + 1;
|
const std::uint64_t len = seg->segment_end(seg_idx) - seg->segment_start(seg_idx) + 1;
|
||||||
if (len != 0 && seg->segment_completed(seg_idx) >= len) {
|
if (len != 0 && seg->segment_completed(seg_idx) >= len) {
|
||||||
r = Result<net::TransferStats>(net::TransferStats{});
|
r = Result<net::TransferStats>(net::TransferStats{});
|
||||||
@@ -756,6 +811,35 @@ void DownloadTaskState::seg_finished(std::uint32_t seg_idx, Result<net::Transfer
|
|||||||
auto_pause_locked(ErrorInfo(Error::range_not_satisfiable, "416"), false, true);
|
auto_pause_locked(ErrorInfo(Error::range_not_satisfiable, "416"), false, true);
|
||||||
return done();
|
return done();
|
||||||
}
|
}
|
||||||
|
if (w->forbidden) {
|
||||||
|
// docs/04 §7: "403 after redirect: retry once with the original referrer -- many
|
||||||
|
// CDNs require it." Bare/foreign Referer is the common cause; origin_of() rebuilds
|
||||||
|
// it from the (possibly redirected) URL the response actually came from. Exactly
|
||||||
|
// once per task, not a backoff series -- a second 403 with a same-origin Referer
|
||||||
|
// already set isn't something another blind retry can fix (a private/expired
|
||||||
|
// resource, an expiring signed URL past its window, ...). That's not necessarily
|
||||||
|
// terminal, though: ask (same "ask, don't just fail outright" shape as
|
||||||
|
// wrong_status/range_bad above) rather than fail_locked() outright, specifically
|
||||||
|
// so DownloadHandle::refresh_url() -- do_refresh_url() is a no-op once the task is
|
||||||
|
// terminal -- stays usable for the case tools/testserver's README pairs it with:
|
||||||
|
// a caller that gets a fresh signed URL and hands it back.
|
||||||
|
release_slot();
|
||||||
|
if (!referrer_retried) {
|
||||||
|
referrer_retried = true;
|
||||||
|
effective_referrer = origin_of(current_url());
|
||||||
|
seg->set_segment_state(seg_idx, segment::SegState::stalled);
|
||||||
|
auto wp = weak_from_this();
|
||||||
|
host.schedule(std::chrono::steady_clock::now(), [wp, seg_idx] {
|
||||||
|
if (auto s = wp.lock())
|
||||||
|
s->retry_worker(seg_idx);
|
||||||
|
});
|
||||||
|
if (workers.empty())
|
||||||
|
transition(EngineState::retry_wait, std::nullopt);
|
||||||
|
} else {
|
||||||
|
auto_pause_locked(ErrorInfo(Error::forbidden, "403", w->http_status), false, true);
|
||||||
|
}
|
||||||
|
return done();
|
||||||
|
}
|
||||||
|
|
||||||
if (!r.has_value()) {
|
if (!r.has_value()) {
|
||||||
ErrorInfo e = std::move(r).error();
|
ErrorInfo e = std::move(r).error();
|
||||||
@@ -1215,6 +1299,7 @@ void DownloadTaskState::do_refresh_url(std::string url, std::vector<net::HeaderF
|
|||||||
net::ProbeRequest pr;
|
net::ProbeRequest pr;
|
||||||
pr.url = spec.url;
|
pr.url = spec.url;
|
||||||
pr.headers = spec.headers;
|
pr.headers = spec.headers;
|
||||||
|
pr.referrer = effective_referrer;
|
||||||
pr.auth = spec.auth;
|
pr.auth = spec.auth;
|
||||||
pr.proxy = spec.proxy;
|
pr.proxy = spec.proxy;
|
||||||
host.probe(std::move(pr), [wp](Result<net::ProbeResult> r) {
|
host.probe(std::move(pr), [wp](Result<net::ProbeResult> r) {
|
||||||
@@ -1224,10 +1309,43 @@ void DownloadTaskState::do_refresh_url(std::string url, std::vector<net::HeaderF
|
|||||||
std::unique_lock lk(s->mu);
|
std::unique_lock lk(s->mu);
|
||||||
if (s->retired.load() || is_terminal(s->state))
|
if (s->retired.load() || is_terminal(s->state))
|
||||||
return;
|
return;
|
||||||
if (r.has_value()) {
|
if (!r.has_value()) {
|
||||||
s->probe.effective_url = r.value().effective_url;
|
lk.unlock();
|
||||||
s->probe.etag = r.value().etag;
|
s->flush_deferred();
|
||||||
s->probe.last_modified = r.value().last_modified;
|
return; // still paused; the caller can retry refresh_url() or decide()
|
||||||
|
}
|
||||||
|
if (!s->have_probe) {
|
||||||
|
// The task's *first* probe never succeeded (e.g. this session's own
|
||||||
|
// expiring-signed-url path: 403, one referrer retry, still 403 -> ask rather
|
||||||
|
// than fail outright -- see on_probe_result() -- specifically so this branch
|
||||||
|
// exists to recover it). finish_probe_locked() is what actually registers the
|
||||||
|
// task with the budget and builds its Segmenter; nothing downstream of a
|
||||||
|
// partial field copy would ever start a worker without it.
|
||||||
|
s->probe = std::move(r).value();
|
||||||
|
s->have_probe = true;
|
||||||
|
s->awaiting_auth = false;
|
||||||
|
s->awaiting_decision = false;
|
||||||
|
s->finish_probe_locked();
|
||||||
|
lk.unlock();
|
||||||
|
s->flush_deferred();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
s->probe.effective_url = r.value().effective_url;
|
||||||
|
s->probe.etag = r.value().etag;
|
||||||
|
s->probe.last_modified = r.value().last_modified;
|
||||||
|
// refresh_url()'s own contract is "on a live OR PAUSED task, without losing
|
||||||
|
// progress" -- distinct from do_decide(restart), which discards progress. A task
|
||||||
|
// can be paused here for any of three reasons (a plain user pause, awaiting_auth,
|
||||||
|
// or awaiting_decision -- e.g. this session's own 403-after-referrer-retry path,
|
||||||
|
// or the pre-existing wrong_status/range_bad ones); apply_slot_target()'s guard
|
||||||
|
// blocks on awaiting_auth/awaiting_decision specifically, so leaving either set
|
||||||
|
// would have set_want() below recompute a target that nothing ever acts on --
|
||||||
|
// the caller's new URL re-probed successfully and then the task just sat there.
|
||||||
|
// Clear both and leave `paused` the same way do_decide(restart) does.
|
||||||
|
if (s->state == EngineState::paused) {
|
||||||
|
s->awaiting_auth = false;
|
||||||
|
s->awaiting_decision = false;
|
||||||
|
s->transition(EngineState::connecting, std::nullopt);
|
||||||
}
|
}
|
||||||
if (s->registered)
|
if (s->registered)
|
||||||
s->host.budget().set_want(s->id, s->want_slots());
|
s->host.budget().set_want(s->id, s->want_slots());
|
||||||
|
|||||||
@@ -28,7 +28,14 @@ namespace vdm::testing {
|
|||||||
|
|
||||||
class TestServer {
|
class TestServer {
|
||||||
public:
|
public:
|
||||||
TestServer() {
|
TestServer() : TestServer(1.0) {}
|
||||||
|
|
||||||
|
// loris_seconds overrides the dribble duration slow-loris mode uses (default matches the
|
||||||
|
// no-arg ctor's long-standing 1s). A test that needs curl's stall detector
|
||||||
|
// (CURLOPT_LOW_SPEED_TIME, hardcoded to 30s in download_task.cpp) to actually fire needs a
|
||||||
|
// dribble that outlasts that threshold, not the short one every other test relies on to
|
||||||
|
// keep runtime down.
|
||||||
|
explicit TestServer(double loris_seconds) {
|
||||||
const char *script = VDM_TESTSERVER_PY;
|
const char *script = VDM_TESTSERVER_PY;
|
||||||
if (!script || !*script || ::access(script, R_OK) != 0)
|
if (!script || !*script || ::access(script, R_OK) != 0)
|
||||||
return;
|
return;
|
||||||
@@ -50,8 +57,9 @@ class TestServer {
|
|||||||
int devnull = ::open("/dev/null", O_WRONLY);
|
int devnull = ::open("/dev/null", O_WRONLY);
|
||||||
if (devnull >= 0)
|
if (devnull >= 0)
|
||||||
::dup2(devnull, STDERR_FILENO);
|
::dup2(devnull, STDERR_FILENO);
|
||||||
|
std::string loris_str = std::to_string(loris_seconds);
|
||||||
::execlp("python3", "python3", script, "--port", "0", "--seed", "9", "--loris-seconds",
|
::execlp("python3", "python3", script, "--port", "0", "--seed", "9", "--loris-seconds",
|
||||||
"1", "--throttle-bps", "131072", static_cast<char *>(nullptr));
|
loris_str.c_str(), "--throttle-bps", "131072", static_cast<char *>(nullptr));
|
||||||
::_exit(127);
|
::_exit(127);
|
||||||
}
|
}
|
||||||
::close(pipefd[1]);
|
::close(pipefd[1]);
|
||||||
|
|||||||
@@ -124,6 +124,31 @@ std::string server_sha(TestServer &srv, const std::string &mode, const std::stri
|
|||||||
return out.substr(open + 1, close - open - 1);
|
return out.substr(open + 1, close - open - 1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Small, deliberately identical extraction to server_sha's: GET /<mode>/sign/<size>?ttl=N
|
||||||
|
// and pull the "url" field's value out of the {"url":..., "exp":...} JSON body.
|
||||||
|
std::string sign_url(TestServer &srv, const std::string &mode, const std::string &size,
|
||||||
|
int ttl_seconds) {
|
||||||
|
std::string url =
|
||||||
|
srv.url("/" + mode + "/sign/" + size + "?ttl=" + std::to_string(ttl_seconds));
|
||||||
|
std::string cmd = "curl -s '" + url + "'";
|
||||||
|
std::string out;
|
||||||
|
if (FILE *f = ::popen(cmd.c_str(), "r")) {
|
||||||
|
char buf[1024];
|
||||||
|
while (std::fgets(buf, sizeof buf, f))
|
||||||
|
out += buf;
|
||||||
|
::pclose(f);
|
||||||
|
}
|
||||||
|
auto q = out.find("\"url\"");
|
||||||
|
if (q == std::string::npos)
|
||||||
|
return {};
|
||||||
|
auto colon = out.find(':', q);
|
||||||
|
auto open = out.find('"', colon);
|
||||||
|
auto close = out.find('"', open + 1);
|
||||||
|
if (open == std::string::npos || close == std::string::npos)
|
||||||
|
return {};
|
||||||
|
return out.substr(open + 1, close - open - 1);
|
||||||
|
}
|
||||||
|
|
||||||
DownloadSpec spec_for(TestServer &srv, const std::string &urlpath, const std::string &save) {
|
DownloadSpec spec_for(TestServer &srv, const std::string &urlpath, const std::string &save) {
|
||||||
DownloadSpec s;
|
DownloadSpec s;
|
||||||
s.url = srv.url(urlpath);
|
s.url = srv.url(urlpath);
|
||||||
@@ -328,6 +353,30 @@ VT_TEST(engine_401_then_provide_auth_completes) {
|
|||||||
VT_CHECK_EQ(file_size(td.file("au.bin")), 1u * 1024 * 1024);
|
VT_CHECK_EQ(file_size(td.file("au.bin")), 1u * 1024 * 1024);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
VT_TEST(engine_401_digest_then_provide_auth_completes) {
|
||||||
|
// Same shape as engine_401_then_provide_auth_completes, but the challenge is HTTP
|
||||||
|
// Digest (qop=auth) rather than Basic. provide_auth() doesn't know or care which --
|
||||||
|
// http_client.cpp always asks libcurl for CURLAUTH_ANY (net::AuthScheme::any) and lets
|
||||||
|
// curl negotiate against whatever WWW-Authenticate the server actually sent -- so this
|
||||||
|
// exists purely to prove that's true end-to-end, not just at the unit level.
|
||||||
|
TestServer srv;
|
||||||
|
VT_REQUIRE(srv.available());
|
||||||
|
TmpDir td;
|
||||||
|
Recorder rec;
|
||||||
|
Engine eng;
|
||||||
|
DownloadHandle h;
|
||||||
|
auto cbs = rec.cbs(&h, "test", "test");
|
||||||
|
h = eng.start(spec_for(srv, "/401-digest/file/1M", td.file("dg.bin")), std::move(cbs));
|
||||||
|
rec.arm(h);
|
||||||
|
|
||||||
|
auto r = rec.wait();
|
||||||
|
VT_REQUIRE(r.has_value());
|
||||||
|
VT_CHECK(rec.auth_calls.load() >= 1);
|
||||||
|
VT_CHECK_EQ(file_size(td.file("dg.bin")), 1u * 1024 * 1024);
|
||||||
|
auto got = hash_file(td.file("dg.bin"), Checksum::Algo::sha256);
|
||||||
|
VT_CHECK_EQ(got.value(), server_sha(srv, "401-digest", "1M"));
|
||||||
|
}
|
||||||
|
|
||||||
// --- hostile-mode matrix: the four where a bug is silent corruption, not a visible
|
// --- hostile-mode matrix: the four where a bug is silent corruption, not a visible
|
||||||
// failure (docs/04 §5 "ask, never silently corrupt" / §7's failure-policy table). ---
|
// failure (docs/04 §5 "ask, never silently corrupt" / §7's failure-policy table). ---
|
||||||
|
|
||||||
@@ -458,6 +507,142 @@ VT_TEST(engine_content_length_mismatch_fails_honestly) {
|
|||||||
VT_CHECK_EQ(::access(td.file("clm.bin").c_str(), F_OK), -1); // never renamed into place
|
VT_CHECK_EQ(::access(td.file("clm.bin").c_str(), F_OK), -1); // never renamed into place
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --- remaining hostile-mode matrix (tools/testserver/README.md's mode table). ---
|
||||||
|
|
||||||
|
VT_TEST(engine_expiring_signed_url_recovers_via_refresh_url) {
|
||||||
|
// A signed URL past its ttl 403s (tools/testserver's own JSON body distinguishes
|
||||||
|
// "expired" from "bad signature", but core never parses response bodies -- CLAUDE.md
|
||||||
|
// §3 -- so both just read as a 403). The one automatic referrer retry (see
|
||||||
|
// engine_403_without_referer_retries_with_origin, below) can't fix an expired
|
||||||
|
// signature, so the second 403 asks -- via the same auto_pause_locked(..., false,
|
||||||
|
// true) "ask, don't just fail" path as wrong_status/range_bad -- rather than
|
||||||
|
// terminally failing outright, specifically so DownloadHandle::refresh_url() (its own
|
||||||
|
// contract: works "on a live or paused task", never on a terminal one) stays usable:
|
||||||
|
// the README pairs this mode with exactly that recovery.
|
||||||
|
TestServer srv;
|
||||||
|
VT_REQUIRE(srv.available());
|
||||||
|
TmpDir td;
|
||||||
|
Recorder rec;
|
||||||
|
Engine eng;
|
||||||
|
|
||||||
|
std::string expired = sign_url(srv, "expiring-signed-url", "64K", /*ttl=*/1);
|
||||||
|
VT_REQUIRE(!expired.empty());
|
||||||
|
std::this_thread::sleep_for(1500ms); // let the ttl actually pass before the first request
|
||||||
|
|
||||||
|
DownloadSpec s;
|
||||||
|
s.url = expired;
|
||||||
|
s.save_path = td.file("exp.bin");
|
||||||
|
auto h = eng.start(std::move(s), rec.cbs());
|
||||||
|
|
||||||
|
for (int i = 0; i < 300 && rec.decision_calls.load() == 0; ++i)
|
||||||
|
std::this_thread::sleep_for(20ms);
|
||||||
|
VT_REQUIRE(rec.decision_calls.load() >= 1);
|
||||||
|
VT_CHECK_EQ(h.state(), EngineState::paused);
|
||||||
|
|
||||||
|
std::string fresh = sign_url(srv, "expiring-signed-url", "64K", /*ttl=*/60);
|
||||||
|
VT_REQUIRE(!fresh.empty());
|
||||||
|
h.refresh_url(fresh);
|
||||||
|
|
||||||
|
auto r = rec.wait(60s);
|
||||||
|
VT_REQUIRE(r.has_value());
|
||||||
|
VT_CHECK_EQ(file_size(td.file("exp.bin")), 64u * 1024);
|
||||||
|
auto got = hash_file(td.file("exp.bin"), Checksum::Algo::sha256);
|
||||||
|
VT_CHECK_EQ(got.value(), server_sha(srv, "expiring-signed-url", "64K"));
|
||||||
|
}
|
||||||
|
|
||||||
|
VT_TEST(engine_403_without_referer_retries_with_origin) {
|
||||||
|
// docs/04 §7: "403 after redirect: retry once with the original referrer -- many CDNs
|
||||||
|
// require it." No spec.referrer is set here (the common case for anything not
|
||||||
|
// initiated from a browser page, e.g. `velox add <url>`), so the first attempt 403s;
|
||||||
|
// the engine's own retry supplies the download URL's own origin as Referer, which
|
||||||
|
// this mode accepts, and the download completes with no decision ever asked.
|
||||||
|
TestServer srv;
|
||||||
|
VT_REQUIRE(srv.available());
|
||||||
|
TmpDir td;
|
||||||
|
Recorder rec;
|
||||||
|
Engine eng;
|
||||||
|
auto h = eng.start(spec_for(srv, "/403-without-referer/file/128K", td.file("ref.bin")),
|
||||||
|
rec.cbs());
|
||||||
|
auto r = rec.wait(30s);
|
||||||
|
VT_REQUIRE(r.has_value());
|
||||||
|
VT_CHECK_EQ(rec.decision_calls.load(), 0); // recovered automatically, not asked
|
||||||
|
VT_CHECK_EQ(file_size(td.file("ref.bin")), 128u * 1024);
|
||||||
|
auto got = hash_file(td.file("ref.bin"), Checksum::Algo::sha256);
|
||||||
|
VT_CHECK_EQ(got.value(), server_sha(srv, "403-without-referer", "128K"));
|
||||||
|
}
|
||||||
|
|
||||||
|
VT_TEST(engine_redirect_chain_follows_to_completion) {
|
||||||
|
// 5 hops (tools/testserver's own --redirect-depth default) of a plain 302, query
|
||||||
|
// string preserved across each. No CORE-side logic needed for this one -- libcurl's
|
||||||
|
// own CURLOPT_FOLLOWLOCATION (RequestOptions::follow_redirects, already on) and
|
||||||
|
// CURLOPT_MAXREDIRS (default 20, well over 5) do the whole thing -- this is here as
|
||||||
|
// the end-to-end check that they're actually wired through both the probe and every
|
||||||
|
// segment worker's own request, not just one of the two.
|
||||||
|
TestServer srv;
|
||||||
|
VT_REQUIRE(srv.available());
|
||||||
|
TmpDir td;
|
||||||
|
Recorder rec;
|
||||||
|
Engine eng;
|
||||||
|
auto h = eng.start(spec_for(srv, "/redirect-chain/file/1M", td.file("rc.bin")), rec.cbs());
|
||||||
|
auto r = rec.wait(30s);
|
||||||
|
VT_REQUIRE(r.has_value());
|
||||||
|
VT_CHECK_EQ(file_size(td.file("rc.bin")), 1u * 1024 * 1024);
|
||||||
|
auto got = hash_file(td.file("rc.bin"), Checksum::Algo::sha256);
|
||||||
|
VT_CHECK_EQ(got.value(), server_sha(srv, "redirect-chain", "1M"));
|
||||||
|
}
|
||||||
|
|
||||||
|
VT_TEST(engine_slow_loris_stall_timeout_fires) {
|
||||||
|
// Status line, headers, and body dribbled out one byte at a time for --loris-seconds,
|
||||||
|
// then (if the dribble hasn't already been cut off) normal streaming -- a connection
|
||||||
|
// that's technically alive (bytes ARE arriving, just far too slowly) but must not be
|
||||||
|
// allowed to hang the task forever. http_client.cpp sets CURLOPT_LOW_SPEED_LIMIT/_TIME
|
||||||
|
// (RequestOptions::low_speed_bytes_per_sec/low_speed_secs, hardcoded in
|
||||||
|
// download_task.cpp to 1024 B/s for 30s) for exactly this.
|
||||||
|
//
|
||||||
|
// Every other test in this file uses TestServer's default 1s loris dribble to keep
|
||||||
|
// runtime down, but 1s is far shorter than curl's 30s low_speed_time: a 1s trickle
|
||||||
|
// followed by full-speed streaming never accumulates 30 CONSECUTIVE seconds under the
|
||||||
|
// floor, so curl would never actually abort it -- the download would just complete
|
||||||
|
// slightly late, which would make this test pass for the wrong reason (or not exercise
|
||||||
|
// the stall timeout at all). Explicitly ask for a dribble that outlasts the 30s
|
||||||
|
// threshold so the stall timeout is the thing actually observed firing, not assumed.
|
||||||
|
TestServer srv(40.0);
|
||||||
|
VT_REQUIRE(srv.available());
|
||||||
|
TmpDir td;
|
||||||
|
Recorder rec;
|
||||||
|
Engine eng;
|
||||||
|
auto s = spec_for(srv, "/slow-loris/file/64K", td.file("sl.bin"));
|
||||||
|
s.segments = 1;
|
||||||
|
s.max_retries = 1;
|
||||||
|
auto h = eng.start(std::move(s), rec.cbs());
|
||||||
|
auto r = rec.wait(60s); // stall timeout fires ~30s in; must resolve, not hang to 60s
|
||||||
|
VT_REQUIRE(!r.has_value());
|
||||||
|
VT_CHECK(is_retryable(r.error().code) || r.error().code == Error::max_retries_exhausted);
|
||||||
|
}
|
||||||
|
|
||||||
|
VT_TEST(engine_chunked_no_length_completes_single_segment) {
|
||||||
|
// No Content-Length anywhere (HEAD gets none either, since it's the same handler path)
|
||||||
|
// -- the probe can't know total_size or prove resumability, so this should take the
|
||||||
|
// exact same "unknown size, one plain-GET segment" path as engine_non_resumable_single_
|
||||||
|
// segment, just arriving there via a chunked body instead of a server that plainly
|
||||||
|
// refuses Range. No core-side work needed if that demotion is already size-agnostic;
|
||||||
|
// this is here to prove it, since every other test's server tells the probe the size
|
||||||
|
// up front.
|
||||||
|
TestServer srv;
|
||||||
|
VT_REQUIRE(srv.available());
|
||||||
|
TmpDir td;
|
||||||
|
Recorder rec;
|
||||||
|
Engine eng;
|
||||||
|
auto h = eng.start(spec_for(srv, "/chunked-no-length/file/2M", td.file("ch.bin")),
|
||||||
|
rec.cbs());
|
||||||
|
auto r = rec.wait();
|
||||||
|
VT_REQUIRE(r.has_value());
|
||||||
|
VT_CHECK_EQ(rec.decision_calls.load(), 0);
|
||||||
|
VT_CHECK_EQ(file_size(td.file("ch.bin")), 2u * 1024 * 1024);
|
||||||
|
auto got = hash_file(td.file("ch.bin"), Checksum::Algo::sha256);
|
||||||
|
VT_CHECK_EQ(got.value(), server_sha(srv, "chunked-no-length", "2M"));
|
||||||
|
}
|
||||||
|
|
||||||
// --- DAEMON-reported bug: Progress.speed_bps reads 0 for the whole life of a live
|
// --- DAEMON-reported bug: Progress.speed_bps reads 0 for the whole life of a live
|
||||||
// download while downloaded bytes visibly advance. DAEMON reads progress by polling
|
// download while downloaded bytes visibly advance. DAEMON reads progress by polling
|
||||||
// DownloadHandle::progress() (engine_port_core.hpp), not the on_progress push callback --
|
// DownloadHandle::progress() (engine_port_core.hpp), not the on_progress push callback --
|
||||||
|
|||||||
@@ -0,0 +1,107 @@
|
|||||||
|
# ADR 0018 — Nullable optional fields: absent vs. explicit null
|
||||||
|
|
||||||
|
**Status:** accepted · **Date:** 2026-09-13 · **Lane:** PROTO
|
||||||
|
**Prompted by:** a DAEMON report against `download.update`: the generated C++ parser gives
|
||||||
|
`VeloxDispatcher` no way to tell "the caller left this field alone" from "the caller wants
|
||||||
|
it cleared," so `download.update` and (the moment a nullable `SettingKey` exists)
|
||||||
|
`settings.set` can set a nullable field but never clear it back to `null`.
|
||||||
|
|
||||||
|
## Context
|
||||||
|
|
||||||
|
`download.update`'s `patch` object documents the convention plainly: "Only the present
|
||||||
|
fields change. An explicit null clears a nullable field." That is a deliberate, already-
|
||||||
|
committed wire contract — not something up for redesign here. The gap is one layer down:
|
||||||
|
`contracts/codegen/gen_cpp.py`'s `emit_field_parse` collapses "key absent" and "key present
|
||||||
|
with value `null`" to the same `std::nullopt`, on purpose, and the comment says so:
|
||||||
|
|
||||||
|
> Absent and null mean the same thing: the field is not set. A client that omits a
|
||||||
|
> nullable field and one that sends null are treated identically on purpose.
|
||||||
|
|
||||||
|
That collapse is *correct* for the common case — most nullable-optional fields are on
|
||||||
|
create-style params (`DownloadSpec.saveDir`, `.categoryId`, …) where there is no existing
|
||||||
|
value to distinguish "never set" from "explicitly cleared" in the first place; either way
|
||||||
|
the daemon just uses a default. It is wrong specifically for **patch-style** params, where
|
||||||
|
a field can already hold a value and the caller needs to say which of two different things
|
||||||
|
they mean: "leave it" or "clear it."
|
||||||
|
|
||||||
|
The schema IR (`schema_ir.py`) already tracks `required` and `nullable` as two independent
|
||||||
|
booleans per `Field`, so the information needed to make this distinction exists all the way
|
||||||
|
through parsing — `emit_field_parse` just doesn't act on it. Only `download.update`'s
|
||||||
|
`patch` object is affected today (`filename`, `saveDir`, `categoryId`, `queueId`,
|
||||||
|
`description`, `segments`, `bufferBytes`, `checksum` — all eight of its fields are
|
||||||
|
nullable-and-optional with exactly this "leave vs. clear" meaning). No `SettingKey` is
|
||||||
|
nullable yet, so `settings.set` has no live instance of the bug, but the same shape
|
||||||
|
(`values` patches an existing bag) means the first nullable settings key will hit the exact
|
||||||
|
same gap.
|
||||||
|
|
||||||
|
## Decision
|
||||||
|
|
||||||
|
**A JSON-null-aware optional, opt in per field via a new `x-clearable: true` annotation —
|
||||||
|
not a blanket rule and not a companion "clear list" field.**
|
||||||
|
|
||||||
|
- New per-field schema annotation, `x-clearable: true`, valid only on a field whose type
|
||||||
|
already includes `null` (schema error otherwise — clearable implies nullable). Marks
|
||||||
|
"this field distinguishes absent from explicit null"; every other nullable-optional field
|
||||||
|
keeps today's collapse.
|
||||||
|
- The generated C++ type for a `x-clearable` field becomes `std::optional<std::optional<T>>`:
|
||||||
|
outer `nullopt` = absent (leave unchanged), outer engaged with an inner `nullopt` =
|
||||||
|
explicit `null` (clear it), outer engaged with an inner value = set it. One field, three
|
||||||
|
states, no parallel bitset to keep in sync and no second field to forget to check.
|
||||||
|
- `emit_field_parse` for such a field stops folding `is_null()` into "absent": absent skips
|
||||||
|
the assignment (outer stays `nullopt`); present-and-null assigns an engaged-but-empty
|
||||||
|
inner optional; present-and-valued parses normally into the inner optional. Every other
|
||||||
|
field's codegen (the `required`/`nullable`-but-not-`clearable` majority) is unchanged.
|
||||||
|
- TypeScript needs no generator change: `field?: T | null` already round-trips this exactly
|
||||||
|
the way JSON does — an omitted key serializes as absent, `null` serializes as `null`, and
|
||||||
|
`"field" in obj` / `obj.field === null` already distinguish the three states natively.
|
||||||
|
This gap is a C++-generator-only problem.
|
||||||
|
- Applies now to `download.update`'s eight `patch` fields. `Settings` gets no annotation
|
||||||
|
today (nothing nullable to mark); the day a nullable `SettingKey` is added, it gets
|
||||||
|
`x-clearable: true` in the same PR, not left to rediscover this ADR.
|
||||||
|
|
||||||
|
## Versioning
|
||||||
|
|
||||||
|
Per ADR 0015: this retypes a generated C++ field (`optional<T>` -> `optional<optional<T>>`)
|
||||||
|
with the wire byte-for-byte unchanged — a client sending the same JSON parses correctly
|
||||||
|
either way. **Minor bump, with a migration note** for anyone reading `patch.filename` et al.
|
||||||
|
directly (unwrap twice: check the outer, then the inner). Not major; `session.hello`'s
|
||||||
|
major-only check must not refuse a wire-compatible peer over a binding-only change.
|
||||||
|
|
||||||
|
## Consequences
|
||||||
|
|
||||||
|
- `on_download_update` (DAEMON, not this lane) can finally implement "explicit null
|
||||||
|
clears": read the outer optional for presence, the inner for clear-vs-value, exactly the
|
||||||
|
three states the schema already promised.
|
||||||
|
- The collapse comment in `emit_field_parse` stays as the default behavior and gets a
|
||||||
|
pointer to this ADR for the opt-in exception, instead of being read as an oversight.
|
||||||
|
- Implementation (schema annotation support in `schema_ir.py`, the `gen_cpp.py` emission
|
||||||
|
change above, regenerating `core/generated/`, the `x-clearable: true` annotations on
|
||||||
|
`download.update`'s eight fields, the VERSION bump and migration note) is **not** done in
|
||||||
|
this change — recorded here so DAEMON isn't blocked on relitigating the design, tracked as
|
||||||
|
its own PROTO PR per the normal contracts process (schema + regenerated code + fixtures +
|
||||||
|
VERSION bump together, CLAUDE.md §2).
|
||||||
|
|
||||||
|
## Alternatives rejected
|
||||||
|
|
||||||
|
**An explicit clear list** (e.g. `patch.clearFields: ["categoryId", …]`, plain non-nullable
|
||||||
|
`optional<T>` fields otherwise). Rejected: the wire contract "an explicit null clears a
|
||||||
|
nullable field" is already written into `download.update`'s schema description and is what
|
||||||
|
DAEMON built against — this would be a real, disruptive wire redesign to route around a
|
||||||
|
generator gap, not a fix for it. It also doesn't compose: every patch-shaped object gains a
|
||||||
|
second array to keep in sync with the first, by hand, forever.
|
||||||
|
|
||||||
|
**A parallel "which fields were present" bitset** (struct of `optional<T>` fields plus a
|
||||||
|
sibling presence-flags struct or bitset). Rejected: two things to check per field instead
|
||||||
|
of one, and nothing stops a caller from reading the optional and forgetting the presence
|
||||||
|
bit — exactly the class of bug this ADR exists to close.
|
||||||
|
|
||||||
|
**Apply the tri-state to every `nullable && !required` field automatically**, using the IR
|
||||||
|
flags already present, no annotation needed. Rejected: `emit_field_parse` only backs
|
||||||
|
`parse<T>()`, used for *params* types the daemon receives — but the conformance C++ runner
|
||||||
|
also instantiates `parse<T>()` for **result** types (round-tripping golden fixtures), and
|
||||||
|
plenty of those are nullable-optional with no patch semantics at all (`TaskSummary.effectiveUrl`,
|
||||||
|
"null until the first probe succeeds" — a plain nullable value, not a leave-or-clear
|
||||||
|
choice). Blanket application would retype those too, forcing every read site across the
|
||||||
|
daemon that already does `if (summary.effectiveUrl)` into an unwanted double-unwrap for a
|
||||||
|
distinction that field doesn't have. Opt-in keeps the blast radius at exactly the fields
|
||||||
|
that need it.
|
||||||
@@ -0,0 +1,45 @@
|
|||||||
|
# ADR 0019 — The live-`veloxd` conformance runner is a required check, as-is
|
||||||
|
|
||||||
|
**Status:** accepted · **Date:** 2026-09-12 · **Lane:** PKG/QA
|
||||||
|
|
||||||
|
## Context
|
||||||
|
|
||||||
|
`tests/conformance/run.sh` step 3b (ADR 0014's `conformance` ctest, already required
|
||||||
|
per `.github/BRANCH_PROTECTION.md`) replays every fixture against a real, isolated
|
||||||
|
`veloxd` it builds and starts — not just mockd, which only proves the TS client and the
|
||||||
|
fixtures agree with each other. This is the runner that can catch `veloxd` disagreeing
|
||||||
|
with its own contract, and it is unconditional in `run.sh` (`set -euo pipefail`, no
|
||||||
|
skip flag): it already runs, and already blocks, inside the `conformance` job.
|
||||||
|
|
||||||
|
What was open was whether to treat that as a settled, defended gate or as something
|
||||||
|
still provisional while `daemon/docs/deferrals.md`'s D1-D4b stub handlers were excused
|
||||||
|
via `veloxd-xfail.json`. PROTO's update: 57/57 fixtures pass on `main`, and the xfail
|
||||||
|
allowlist is down to 18 entries from 34 as DAEMON lands the deferred handlers behind
|
||||||
|
them.
|
||||||
|
|
||||||
|
## Decision
|
||||||
|
|
||||||
|
The live-`veloxd` conformance runner stays required — no change to CI is needed, since
|
||||||
|
it already runs inside the already-required `conformance` job (ADR 0014). What this ADR
|
||||||
|
records is the standing: PKG/QA is not carving out an exception, a `continue-on-error`,
|
||||||
|
or a separate advisory job for it while the xfail list shrinks. A regression here fails
|
||||||
|
the same required check a schema mismatch would.
|
||||||
|
|
||||||
|
Verified, not assumed: `tests/conformance/veloxd-xfail.json` has 18 entries as of this
|
||||||
|
ADR (`python3 -c "import json; print(len(json.load(open('tests/conformance/veloxd-xfail.json'))))"`).
|
||||||
|
Each remaining entry excuses one still-stubbed handler on `deferrals.md`'s D-list, not a
|
||||||
|
real disagreement between `veloxd` and its contract — `tests/conformance/README.md`
|
||||||
|
already draws that line (an entry for anything else is a `run.sh`-detected "xfail entry
|
||||||
|
unexpectedly passed" or a straight failure, not a quiet pass).
|
||||||
|
|
||||||
|
## Consequences
|
||||||
|
|
||||||
|
- No `ci.yml` or `BRANCH_PROTECTION.md` change: `conformance` was already listed
|
||||||
|
required, and this runner was already inside it.
|
||||||
|
- The xfail list is a visible, shrinking number, not a static allowance — as DAEMON
|
||||||
|
clears more of `deferrals.md`'s D-list, entries come out of
|
||||||
|
`tests/conformance/veloxd-xfail.json`, and `replay.ts` fails loudly (per
|
||||||
|
`applyXfail`'s "unexpectedly passed" check) if one is left in after its handler ships.
|
||||||
|
- Nothing here changes who owns what: `veloxd-xfail.json` and `run.sh` stay PROTO's;
|
||||||
|
PKG/QA's role is the branch-protection policy this ADR confirms, not the runner
|
||||||
|
itself.
|
||||||
@@ -19,7 +19,12 @@ import {
|
|||||||
} from './context-menus.js';
|
} from './context-menus.js';
|
||||||
import { MediaBridge, notifyTab } from './media-bridge.js';
|
import { MediaBridge, notifyTab } from './media-bridge.js';
|
||||||
import { createTransport, transportStorage, type TransportStatus, type VeloxTransport } from './transport/index.js';
|
import { createTransport, transportStorage, type TransportStatus, type VeloxTransport } from './transport/index.js';
|
||||||
import type { CaptureOfferParams, CaptureRules, DownloadSpec } from '../shared/protocol/index.js';
|
import {
|
||||||
|
SESSION_SUBSCRIBE_PARAMS_EVENTS_ITEM_VALUES,
|
||||||
|
type CaptureOfferParams,
|
||||||
|
type CaptureRules,
|
||||||
|
type DownloadSpec,
|
||||||
|
} from '../shared/protocol/index.js';
|
||||||
|
|
||||||
let transport: VeloxTransport | undefined;
|
let transport: VeloxTransport | undefined;
|
||||||
let rules: CaptureRules = DEFAULT_CAPTURE_RULES;
|
let rules: CaptureRules = DEFAULT_CAPTURE_RULES;
|
||||||
@@ -75,10 +80,31 @@ async function refreshRules(): Promise<void> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* "Nothing is delivered until this is called" (session.subscribe's own description) —
|
||||||
|
* without it, event.task.progress et al. never reach this connection at all, no matter
|
||||||
|
* how many listeners bridge.ts registers locally. Requests the whole set every time
|
||||||
|
* because any popup/options document could open at any moment and none of them narrow
|
||||||
|
* per-tab; a fresh connection (first connect, or after a drop) starts with nothing
|
||||||
|
* subscribed until this runs again.
|
||||||
|
*/
|
||||||
|
async function subscribeToEvents(): Promise<void> {
|
||||||
|
try {
|
||||||
|
await mustTransport().call('session.subscribe', {
|
||||||
|
events: [...SESSION_SUBSCRIBE_PARAMS_EVENTS_ITEM_VALUES],
|
||||||
|
});
|
||||||
|
} catch {
|
||||||
|
// Best-effort; a reconnect (or the next event.settings.changed-driven refresh) retries.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function onTransportState(status: TransportStatus): void {
|
function onTransportState(status: TransportStatus): void {
|
||||||
const detail = status.fatal ?? (status.needsPairing ? 'needs pairing' : '');
|
const detail = status.fatal ?? (status.needsPairing ? 'needs pairing' : '');
|
||||||
console.debug(`[velox] transport ${status.state}${detail ? ` — ${detail}` : ''}`);
|
console.debug(`[velox] transport ${status.state}${detail ? ` — ${detail}` : ''}`);
|
||||||
if (status.state === 'connected') void refreshRules();
|
if (status.state === 'connected') {
|
||||||
|
void refreshRules();
|
||||||
|
void subscribeToEvents();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
async function setOverride(override: 'auto' | 'ws' | 'uds'): Promise<void> {
|
async function setOverride(override: 'auto' | 'ws' | 'uds'): Promise<void> {
|
||||||
|
|||||||
@@ -0,0 +1,389 @@
|
|||||||
|
// Runs the transport, the capture hook, and the popup event path against a REAL veloxd
|
||||||
|
// — not FakeDaemon. Everything else in this suite is faithful to the documented wire
|
||||||
|
// protocol, but "faithful" isn't "real"; this is what actually proves it.
|
||||||
|
//
|
||||||
|
// Requires VELOXD_BIN (path to a built veloxd) in the environment. Skips itself with a
|
||||||
|
// clear message otherwise, so `npm test` and CI (no daemon binary lying around) are
|
||||||
|
// unaffected. Run it like:
|
||||||
|
//
|
||||||
|
// VELOXD_BIN=/path/to/build/dev/bin/veloxd npx vitest run tests/live
|
||||||
|
//
|
||||||
|
// Each veloxd instance gets its own scratch XDG_RUNTIME_DIR/XDG_DATA_HOME/
|
||||||
|
// XDG_CONFIG_HOME/HOME (main.cpp's single-instance lock is keyed to the runtime dir, so
|
||||||
|
// this can run alongside another developer's or CI's own veloxd on the same machine).
|
||||||
|
// VELOX_PAIR_AUTO=1 stands in for the GUI's Allow-prompt approver during dev/test
|
||||||
|
// (rpc/pairing.hpp's EnvAutoApprover) — pairing itself is exercised for real, only the
|
||||||
|
// human click is stubbed.
|
||||||
|
import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
|
||||||
|
import { mkdtempSync, mkdirSync, rmSync } from 'node:fs';
|
||||||
|
import { readFile } from 'node:fs/promises';
|
||||||
|
import { tmpdir } from 'node:os';
|
||||||
|
import { dirname, join, resolve } from 'node:path';
|
||||||
|
import { fileURLToPath } from 'node:url';
|
||||||
|
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
|
||||||
|
import { WebSocket as WsClient } from 'ws';
|
||||||
|
|
||||||
|
import { RpcError, TransportClosedError } from '../../src/background/transport/types.js';
|
||||||
|
import { WebSocketTransport, type WebSocketCtor, type WebSocketTransportDeps } from '../../src/background/transport/websocket.js';
|
||||||
|
import { CaptureHook } from '../../src/background/capture/index.js';
|
||||||
|
import type { OnHeadersReceivedDetails } from '../../src/background/capture/index.js';
|
||||||
|
import type { CaptureRules, DownloadSpec, TaskProgressEvent, TaskStateEvent } from '../../src/shared/protocol/index.js';
|
||||||
|
|
||||||
|
const VELOXD_BIN = process.env.VELOXD_BIN;
|
||||||
|
const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '../../..');
|
||||||
|
const TESTSERVER_PY = join(REPO_ROOT, 'tools/testserver/testserver.py');
|
||||||
|
|
||||||
|
// A fixed moz-extension origin, used both as session.pair's extensionId and as the WS
|
||||||
|
// upgrade's Origin header — real Firefox sets the latter itself; ws's client needs it
|
||||||
|
// spelled out (docs/05 §4: the daemon refuses the upgrade without a moz-extension:// Origin).
|
||||||
|
const EXTENSION_ID = '11111111-2222-3333-4444-555555555555';
|
||||||
|
const ORIGIN = `moz-extension://${EXTENSION_ID}`;
|
||||||
|
|
||||||
|
class OriginWebSocket extends WsClient {
|
||||||
|
constructor(url: string) {
|
||||||
|
super(url, { origin: ORIGIN });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
const CTOR = OriginWebSocket as unknown as WebSocketCtor;
|
||||||
|
|
||||||
|
function sleep(ms: number): Promise<void> {
|
||||||
|
return new Promise((r) => setTimeout(r, ms));
|
||||||
|
}
|
||||||
|
|
||||||
|
async function waitFor(cond: () => Promise<boolean> | boolean, timeoutMs: number, what: string): Promise<void> {
|
||||||
|
const deadline = Date.now() + timeoutMs;
|
||||||
|
for (;;) {
|
||||||
|
if (await cond()) return;
|
||||||
|
if (Date.now() > deadline) throw new Error(`timed out waiting for ${what}`);
|
||||||
|
await sleep(50);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
interface VeloxdInstance {
|
||||||
|
proc: ChildProcessWithoutNullStreams;
|
||||||
|
scratch: string;
|
||||||
|
wsPort: number;
|
||||||
|
/** True once the process has actually exited, by signal or otherwise. Node only sets
|
||||||
|
* `proc.exitCode` for a normal exit — a signal-killed process reports its death via
|
||||||
|
* `signalCode` and an `exit` event instead, never a non-null `exitCode`. */
|
||||||
|
hasExited(): boolean;
|
||||||
|
kill(signal?: NodeJS.Signals): void;
|
||||||
|
}
|
||||||
|
|
||||||
|
async function startVeloxd(bin: string): Promise<VeloxdInstance> {
|
||||||
|
const scratch = mkdtempSync(join(tmpdir(), 'velox-live-'));
|
||||||
|
const runtime = join(scratch, 'rt');
|
||||||
|
const data = join(scratch, 'data');
|
||||||
|
const config = join(scratch, 'cfg');
|
||||||
|
const home = join(scratch, 'home');
|
||||||
|
mkdirSync(runtime, { mode: 0o700 });
|
||||||
|
mkdirSync(data, { recursive: true });
|
||||||
|
mkdirSync(config, { recursive: true });
|
||||||
|
mkdirSync(join(home, 'Downloads'), { recursive: true });
|
||||||
|
|
||||||
|
const proc = spawn(bin, [], {
|
||||||
|
env: {
|
||||||
|
...process.env,
|
||||||
|
VELOX_PAIR_AUTO: '1',
|
||||||
|
XDG_RUNTIME_DIR: runtime,
|
||||||
|
XDG_DATA_HOME: data,
|
||||||
|
XDG_CONFIG_HOME: config,
|
||||||
|
HOME: home,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
let exited = false;
|
||||||
|
proc.on('exit', () => {
|
||||||
|
exited = true;
|
||||||
|
});
|
||||||
|
let stderr = '';
|
||||||
|
proc.stderr.on('data', (d) => {
|
||||||
|
stderr += String(d);
|
||||||
|
});
|
||||||
|
|
||||||
|
const portFile = join(runtime, 'velox', 'ws.port');
|
||||||
|
try {
|
||||||
|
await waitFor(async () => {
|
||||||
|
if (exited) throw new Error(`veloxd exited early (code ${proc.exitCode}, signal ${proc.signalCode}): ${stderr}`);
|
||||||
|
try {
|
||||||
|
await readFile(portFile);
|
||||||
|
return true;
|
||||||
|
} catch {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}, 10_000, 'veloxd to write ws.port');
|
||||||
|
} catch (e) {
|
||||||
|
proc.kill('SIGKILL');
|
||||||
|
rmSync(scratch, { recursive: true, force: true });
|
||||||
|
throw e;
|
||||||
|
}
|
||||||
|
const wsPort = Number((await readFile(portFile, 'utf8')).trim());
|
||||||
|
|
||||||
|
return {
|
||||||
|
proc,
|
||||||
|
scratch,
|
||||||
|
wsPort,
|
||||||
|
hasExited: () => exited,
|
||||||
|
kill(signal: NodeJS.Signals = 'SIGTERM') {
|
||||||
|
proc.kill(signal);
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
interface TestServerInstance {
|
||||||
|
proc: ChildProcessWithoutNullStreams;
|
||||||
|
baseUrl: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
async function startTestServer(): Promise<TestServerInstance> {
|
||||||
|
const proc = spawn('python3', [TESTSERVER_PY, '--port', '0'], {});
|
||||||
|
let stdout = '';
|
||||||
|
let port: number | null = null;
|
||||||
|
proc.stdout.on('data', (d) => {
|
||||||
|
stdout += String(d);
|
||||||
|
const m = /^(\d+)\s*$/m.exec(stdout);
|
||||||
|
if (m) port = Number(m[1]);
|
||||||
|
});
|
||||||
|
await waitFor(() => port !== null, 5_000, 'testserver to print its port');
|
||||||
|
const baseUrl = `http://127.0.0.1:${port}`;
|
||||||
|
await waitFor(async () => {
|
||||||
|
try {
|
||||||
|
const res = await fetch(`${baseUrl}/__health`);
|
||||||
|
return res.ok;
|
||||||
|
} catch {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}, 5_000, 'testserver /__health');
|
||||||
|
return { proc, baseUrl };
|
||||||
|
}
|
||||||
|
|
||||||
|
function memDeps(init: { token?: string | null } = {}): { deps: WebSocketTransportDeps; store: { token: string | null } } {
|
||||||
|
const store = { token: init.token ?? null };
|
||||||
|
return {
|
||||||
|
store,
|
||||||
|
deps: {
|
||||||
|
getToken: async () => store.token,
|
||||||
|
setToken: async (t) => {
|
||||||
|
store.token = t;
|
||||||
|
},
|
||||||
|
getCachedPort: async () => null,
|
||||||
|
setCachedPort: async () => undefined,
|
||||||
|
extensionId: EXTENSION_ID,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function makeTransport(port: number, deps: WebSocketTransportDeps, extra: Partial<WebSocketTransportDeps> = {}): WebSocketTransport {
|
||||||
|
return new WebSocketTransport({
|
||||||
|
...deps,
|
||||||
|
...extra,
|
||||||
|
webSocketCtor: CTOR,
|
||||||
|
portRange: { start: port, end: port },
|
||||||
|
openTimeoutMs: 2000,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
const maybeDescribe = VELOXD_BIN ? describe : describe.skip;
|
||||||
|
|
||||||
|
if (!VELOXD_BIN) {
|
||||||
|
console.warn('tests/live/real-veloxd.test.ts: VELOXD_BIN not set — skipping (see file header).');
|
||||||
|
}
|
||||||
|
|
||||||
|
maybeDescribe('WebSocketTransport against a real veloxd', () => {
|
||||||
|
let daemon: VeloxdInstance;
|
||||||
|
let testserver: TestServerInstance;
|
||||||
|
// The mid-test fail-open case kills `daemon` and a later test starts a replacement —
|
||||||
|
// every scratch dir that ever existed gets cleaned up here, not just the last one.
|
||||||
|
const allScratchDirs: string[] = [];
|
||||||
|
|
||||||
|
async function freshVeloxd(): Promise<VeloxdInstance> {
|
||||||
|
const d = await startVeloxd(VELOXD_BIN!);
|
||||||
|
allScratchDirs.push(d.scratch);
|
||||||
|
return d;
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
daemon = await freshVeloxd();
|
||||||
|
testserver = await startTestServer();
|
||||||
|
}, 20_000);
|
||||||
|
|
||||||
|
afterAll(() => {
|
||||||
|
daemon?.kill('SIGKILL');
|
||||||
|
testserver?.proc.kill('SIGKILL');
|
||||||
|
for (const dir of allScratchDirs) rmSync(dir, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('session.hello without a token surfaces NotPaired / needsPairing', async () => {
|
||||||
|
const { deps } = memDeps();
|
||||||
|
const t = makeTransport(daemon.wsPort, deps, { autoPair: false });
|
||||||
|
await expect(t.connect()).rejects.toBeInstanceOf(RpcError);
|
||||||
|
expect(t.status.needsPairing).toBe(true);
|
||||||
|
t.disconnect();
|
||||||
|
});
|
||||||
|
|
||||||
|
let pairedToken: string;
|
||||||
|
|
||||||
|
it('pairs (VELOX_PAIR_AUTO=1 stands in for the human Allow click) and hellos with the issued token', async () => {
|
||||||
|
const { deps, store } = memDeps();
|
||||||
|
const t = makeTransport(daemon.wsPort, deps); // autoPair: true (default)
|
||||||
|
await t.connect();
|
||||||
|
expect(t.state).toBe('connected');
|
||||||
|
expect(t.status.daemonVersion).toBeTruthy();
|
||||||
|
expect(store.token).toBeTruthy();
|
||||||
|
pairedToken = store.token!;
|
||||||
|
t.disconnect();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('the pairing token survives a reconnect: a fresh transport reuses it with no fresh pairing', async () => {
|
||||||
|
const { deps } = memDeps({ token: pairedToken });
|
||||||
|
// autoPair: false — if this succeeds at all, it can only be because the stored
|
||||||
|
// token from the previous test was accepted outright, not because this transport
|
||||||
|
// silently re-paired.
|
||||||
|
const t = makeTransport(daemon.wsPort, deps, { autoPair: false });
|
||||||
|
await t.connect();
|
||||||
|
expect(t.state).toBe('connected');
|
||||||
|
t.disconnect();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('a wrong token is rejected, and repeating it rate-limits the next pairing attempt', async () => {
|
||||||
|
// Five failed session.hello attempts from this origin (ws_server.cpp records a
|
||||||
|
// rate-limiter failure on every not-paired hello, not only on a failed session.pair)
|
||||||
|
// exhausts the window; the sixth thing this origin tries — a pairing attempt — gets
|
||||||
|
// RateLimited rather than a fresh token.
|
||||||
|
for (let i = 0; i < 5; i += 1) {
|
||||||
|
const { deps } = memDeps({ token: 'not-the-real-token' });
|
||||||
|
const t = makeTransport(daemon.wsPort, deps, { autoPair: false });
|
||||||
|
await expect(t.connect()).rejects.toBeInstanceOf(RpcError);
|
||||||
|
t.disconnect();
|
||||||
|
}
|
||||||
|
|
||||||
|
const { deps } = memDeps(); // no token -> autoPair kicks in -> session.pair
|
||||||
|
const t = makeTransport(daemon.wsPort, deps);
|
||||||
|
await expect(t.connect()).rejects.toBeInstanceOf(RpcError);
|
||||||
|
expect(t.status.needsPairing).toBe(true);
|
||||||
|
expect(t.status.retryAfterSec).toBeGreaterThan(0);
|
||||||
|
t.disconnect();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('download.add creates a real task the engine picks up', async () => {
|
||||||
|
const { deps } = memDeps({ token: pairedToken });
|
||||||
|
const t = makeTransport(daemon.wsPort, deps, { autoPair: false });
|
||||||
|
await t.connect();
|
||||||
|
try {
|
||||||
|
const spec: DownloadSpec = { url: `${testserver.baseUrl}/plain/file/64K`, filename: 'plain-download.bin' };
|
||||||
|
const added = await t.call('download.add', spec);
|
||||||
|
expect(added.taskId).toBeTruthy();
|
||||||
|
|
||||||
|
await waitFor(async () => {
|
||||||
|
const detail = await t.call('download.get', { taskId: added.taskId });
|
||||||
|
const state = detail.summary.state;
|
||||||
|
return state === 'complete' || state === 'downloading' || state === 'verifying';
|
||||||
|
}, 10_000, 'the task to leave the queued state');
|
||||||
|
} finally {
|
||||||
|
t.disconnect();
|
||||||
|
}
|
||||||
|
}, 15_000);
|
||||||
|
|
||||||
|
it('capture.offer end to end: the real capture path takes a monitored download, ignores its own duplicate, and fails open when the daemon dies mid-offer', async () => {
|
||||||
|
const { deps } = memDeps({ token: pairedToken });
|
||||||
|
const t = makeTransport(daemon.wsPort, deps, { autoPair: false });
|
||||||
|
await t.connect();
|
||||||
|
|
||||||
|
const rules: CaptureRules = await t.call('capture.getRules', {});
|
||||||
|
expect(rules.monitoredExtensions).toContain('zip'); // seeded default (0001_initial.sql)
|
||||||
|
|
||||||
|
const hook = new CaptureHook({
|
||||||
|
offer: (params, opts) => t.call('capture.offer', params, opts),
|
||||||
|
stash: { take: () => undefined, peek: () => undefined },
|
||||||
|
getCookies: async () => [],
|
||||||
|
getRules: () => rules,
|
||||||
|
origin: ORIGIN,
|
||||||
|
});
|
||||||
|
|
||||||
|
// A throttled URL so the task the first offer creates is still active (not yet
|
||||||
|
// complete) when the dedupe offer for the same URL follows immediately after.
|
||||||
|
const url = `${testserver.baseUrl}/throttled/file/512K`;
|
||||||
|
const details: OnHeadersReceivedDetails = {
|
||||||
|
requestId: 'live-1',
|
||||||
|
url,
|
||||||
|
method: 'GET',
|
||||||
|
type: 'other',
|
||||||
|
statusCode: 200,
|
||||||
|
tabId: 1,
|
||||||
|
responseHeaders: [{ name: 'content-disposition', value: 'attachment; filename="live-capture.zip"' }],
|
||||||
|
};
|
||||||
|
|
||||||
|
const first = await hook.handle(details);
|
||||||
|
expect(first).toEqual({ cancel: true }); // the daemon took it — Firefox never starts its own download
|
||||||
|
|
||||||
|
const list = await t.call('download.list', { filter: { query: 'live-capture' } });
|
||||||
|
expect(list.items.length).toBeGreaterThan(0);
|
||||||
|
const task = list.items[0]!;
|
||||||
|
expect(task.categoryId).toBe('programs'); // "zip" routes to the built-in Programs category
|
||||||
|
expect(task.saveDir).toContain('Downloads/Programs');
|
||||||
|
|
||||||
|
// Same URL again, task still active: the daemon's own dedupe (has_active_duplicate)
|
||||||
|
// says Ignore, so the hook proceeds instead of cancelling a second time.
|
||||||
|
const dup = await hook.handle({ ...details, requestId: 'live-2' });
|
||||||
|
expect(dup).toEqual({});
|
||||||
|
|
||||||
|
// Now kill the daemon mid-offer and prove fail-open holds against the REAL binary,
|
||||||
|
// not just FakeDaemon: the hook must still resolve to {} (Firefox downloads
|
||||||
|
// normally) well inside its own 750 ms budget.
|
||||||
|
daemon.kill('SIGKILL');
|
||||||
|
await waitFor(() => daemon.hasExited(), 5_000, 'veloxd to actually die');
|
||||||
|
|
||||||
|
const started = Date.now();
|
||||||
|
const afterDeath = await hook.handle({ ...details, requestId: 'live-3', url: `${url}?after-death=1` });
|
||||||
|
const elapsedMs = Date.now() - started;
|
||||||
|
expect(afterDeath).toEqual({}); // fail open — never {cancel: true} with a dead daemon
|
||||||
|
expect(elapsedMs).toBeLessThan(900); // budget is 750ms; the hook's own timer bounds this
|
||||||
|
|
||||||
|
t.disconnect();
|
||||||
|
}, 20_000);
|
||||||
|
|
||||||
|
it('event.task.progress reaches a subscribed client (the popup\'s own path)', async () => {
|
||||||
|
if (daemon.hasExited()) {
|
||||||
|
// The previous test kills the daemon on purpose to prove fail-open; start a fresh
|
||||||
|
// one so this test still exercises the real event path end to end.
|
||||||
|
daemon = await freshVeloxd();
|
||||||
|
}
|
||||||
|
const { deps } = memDeps();
|
||||||
|
const t = makeTransport(daemon.wsPort, deps); // fresh daemon instance -> fresh pairing
|
||||||
|
await t.connect();
|
||||||
|
try {
|
||||||
|
await t.call('session.subscribe', {
|
||||||
|
events: ['event.task.added', 'event.task.state', 'event.task.progress'],
|
||||||
|
});
|
||||||
|
|
||||||
|
const progressEvents: TaskProgressEvent[] = [];
|
||||||
|
const stateEvents: TaskStateEvent[] = [];
|
||||||
|
t.on('event.task.progress', (p) => progressEvents.push(p as TaskProgressEvent));
|
||||||
|
t.on('event.task.state', (p) => stateEvents.push(p as TaskStateEvent));
|
||||||
|
|
||||||
|
const spec: DownloadSpec = { url: `${testserver.baseUrl}/throttled/file/1M`, filename: 'progress-check.bin' };
|
||||||
|
const added = await t.call('download.add', spec);
|
||||||
|
|
||||||
|
// /throttled defaults to 1 MiB/s, so a 1 MiB file takes ~1s — long enough that at
|
||||||
|
// least one 4 Hz progress tick (event.task.progress's documented cap) lands before
|
||||||
|
// it completes, exactly the path popup/store.ts consumes in the real extension.
|
||||||
|
await waitFor(
|
||||||
|
() => progressEvents.some((e) => e.tasks.some((row) => row.taskId === added.taskId)),
|
||||||
|
8_000,
|
||||||
|
'a live event.task.progress tick for our task',
|
||||||
|
);
|
||||||
|
expect(stateEvents.some((e) => e.taskId === added.taskId)).toBe(true);
|
||||||
|
} finally {
|
||||||
|
t.disconnect();
|
||||||
|
}
|
||||||
|
}, 15_000);
|
||||||
|
|
||||||
|
it('fail-open also holds through the transport itself: a call against a dead socket rejects, never hangs past its deadline', async () => {
|
||||||
|
const { deps } = memDeps();
|
||||||
|
const t = makeTransport(daemon.wsPort, deps);
|
||||||
|
await t.connect();
|
||||||
|
t.disconnect(); // closes the socket without telling the daemon anything is wrong
|
||||||
|
await expect(t.call('capture.offer', { url: 'https://example.com/x.zip', method: 'GET', tabUrl: '' }, { timeoutMs: 200 })).rejects.toBeInstanceOf(
|
||||||
|
TransportClosedError,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -160,11 +160,18 @@ if [ -z "$EXTERNAL_UDS" ] && [ -z "$EXTERNAL_WS" ]; then
|
|||||||
for _ in $(seq 1 50); do [ -S "$VUDS" ] && break; sleep 0.2; done
|
for _ in $(seq 1 50); do [ -S "$VUDS" ] && break; sleep 0.2; done
|
||||||
[ -S "$VUDS" ] || { echo "veloxd did not start:"; cat "$WORK/veloxd.log"; exit 1; }
|
[ -S "$VUDS" ] || { echo "veloxd did not start:"; cat "$WORK/veloxd.log"; exit 1; }
|
||||||
|
|
||||||
# saveTo.allowedRoots defaults to ["~/Downloads"]; download.add.json (fixture) asks
|
# saveTo.allowedRoots defaults to ["~/Downloads"]; download.add's own isolated
|
||||||
# for a saveDir under $HOME/Downloads, so both that and download.add's own isolated
|
# downloads dir needs to be an allowed root too, or every download.add fixture fails
|
||||||
# downloads dir need to be allowed roots, or every download.add fixture fails -32011
|
# -32011 before the point of this runner is even reached. $HOME/Downloads stays in
|
||||||
# before the point of this runner is even reached. settings.set is itself a D3 stub,
|
# the list alongside it: a few fixtures still set an explicit saveDir there
|
||||||
# so this is written straight into the isolated velox.db rather than over the wire.
|
# (category.upsert.json, download.update.json) rather than take the default.
|
||||||
|
# capture.minSizeBytes defaults to 0 (nothing is ever "too small"), which makes
|
||||||
|
# errors/capture.offer.ignore.json's below-minimum-size case impossible to reach
|
||||||
|
# against a fresh daemon; raised here so that fixture's scenario is actually
|
||||||
|
# reachable. Written straight into the isolated velox.db, before veloxd has any RPC
|
||||||
|
# session to write it through: settings.set is real now (D9), but this has to be in
|
||||||
|
# place before the very first fixture runs, and setup happens before any connection
|
||||||
|
# exists.
|
||||||
python3 - "$VXDG/data/velox/velox.db" "$VXDG/downloads" "$HOME/Downloads" <<'PY'
|
python3 - "$VXDG/data/velox/velox.db" "$VXDG/downloads" "$HOME/Downloads" <<'PY'
|
||||||
import json, sqlite3, sys
|
import json, sqlite3, sys
|
||||||
db_path, isolated_downloads, home_downloads = sys.argv[1:4]
|
db_path, isolated_downloads, home_downloads = sys.argv[1:4]
|
||||||
@@ -174,6 +181,11 @@ db.execute(
|
|||||||
"ON CONFLICT(key) DO UPDATE SET value = excluded.value",
|
"ON CONFLICT(key) DO UPDATE SET value = excluded.value",
|
||||||
("saveTo.allowedRoots", json.dumps([isolated_downloads, home_downloads])),
|
("saveTo.allowedRoots", json.dumps([isolated_downloads, home_downloads])),
|
||||||
)
|
)
|
||||||
|
db.execute(
|
||||||
|
"INSERT INTO settings(key, value) VALUES(?, ?) "
|
||||||
|
"ON CONFLICT(key) DO UPDATE SET value = excluded.value",
|
||||||
|
("capture.minSizeBytes", json.dumps(1000000)),
|
||||||
|
)
|
||||||
db.execute(
|
db.execute(
|
||||||
"INSERT INTO settings(key, value) VALUES(?, ?) "
|
"INSERT INTO settings(key, value) VALUES(?, ?) "
|
||||||
"ON CONFLICT(key) DO UPDATE SET value = excluded.value",
|
"ON CONFLICT(key) DO UPDATE SET value = excluded.value",
|
||||||
|
|||||||
@@ -1,40 +1,26 @@
|
|||||||
[
|
[
|
||||||
{ "fixture": "contracts/fixtures/download.refreshUrl.json", "reason": "D3: stub handler, -32603" },
|
{ "fixture": "contracts/fixtures/grabber.harvest.json", "reason": "D3: stub handler, -32603 (M4 territory per deferrals.md)" },
|
||||||
{ "fixture": "contracts/fixtures/download.update.json", "reason": "D3: stub handler, -32603" },
|
{ "fixture": "contracts/fixtures/grabber.start.json", "reason": "D3: stub handler, -32603 (M4 territory per deferrals.md)" },
|
||||||
|
{ "fixture": "contracts/fixtures/grabber.status.json", "reason": "D3: stub handler, -32603 (M4 territory per deferrals.md)" },
|
||||||
|
|
||||||
{ "fixture": "contracts/fixtures/rules.list.json", "reason": "D3: stub handler, -32603" },
|
{ "fixture": "contracts/fixtures/media.addVariant.json", "reason": "D3: stub handler, -32603 (M4 territory per deferrals.md)" },
|
||||||
{ "fixture": "contracts/fixtures/rules.upsert.json", "reason": "D3: stub handler, -32603" },
|
{ "fixture": "contracts/fixtures/media.listVariants.json", "reason": "D3: stub handler, -32603 (M4 territory per deferrals.md)" },
|
||||||
|
|
||||||
{ "fixture": "contracts/fixtures/settings.get.json", "reason": "D3: stub handler, -32603" },
|
|
||||||
{ "fixture": "contracts/fixtures/settings.set.json", "reason": "D3: stub handler, -32603" },
|
|
||||||
|
|
||||||
{ "fixture": "contracts/fixtures/limiter.get.json", "reason": "D3: stub handler, -32603" },
|
|
||||||
{ "fixture": "contracts/fixtures/limiter.set.json", "reason": "D3: stub handler, -32603" },
|
|
||||||
|
|
||||||
{ "fixture": "contracts/fixtures/schedule.get.json", "reason": "D3: stub handler, -32603" },
|
|
||||||
{ "fixture": "contracts/fixtures/schedule.set.json", "reason": "D3: stub handler, -32603" },
|
|
||||||
|
|
||||||
{ "fixture": "contracts/fixtures/queue.reorder.json", "reason": "D3: stub handler, -32603" },
|
|
||||||
|
|
||||||
{ "fixture": "contracts/fixtures/grabber.harvest.json", "reason": "D3: stub handler, -32603" },
|
|
||||||
{ "fixture": "contracts/fixtures/grabber.start.json", "reason": "D3: stub handler, -32603" },
|
|
||||||
{ "fixture": "contracts/fixtures/grabber.status.json", "reason": "D3: stub handler, -32603" },
|
|
||||||
|
|
||||||
{ "fixture": "contracts/fixtures/media.addVariant.json", "reason": "D3: stub handler, -32603" },
|
|
||||||
{ "fixture": "contracts/fixtures/media.listVariants.json", "reason": "D3: stub handler, -32603" },
|
|
||||||
|
|
||||||
{ "fixture": "contracts/fixtures/capture.getRules.json", "reason": "D3: stub handler, -32603 -- DAEMON is filing capture.offer next" },
|
|
||||||
{ "fixture": "contracts/fixtures/capture.offer.take.json", "reason": "D3: stub handler, -32603 -- DAEMON is filing capture.offer next" },
|
|
||||||
{ "fixture": "contracts/fixtures/errors/capture.offer.ignore.json", "reason": "D3: stub handler, -32603 -- DAEMON is filing capture.offer next" },
|
|
||||||
|
|
||||||
{ "fixture": "contracts/fixtures/errors/download.provideAuth.not-found.json", "reason": "real bug: on_download_provideAuth (dispatcher.cpp) never checks the task exists -- TaskActionPort::provide_auth returns false for an unknown id, which the handler folds into a normal {ok:false} result instead of -32010" },
|
{ "fixture": "contracts/fixtures/errors/download.provideAuth.not-found.json", "reason": "real bug: on_download_provideAuth (dispatcher.cpp) never checks the task exists -- TaskActionPort::provide_auth returns false for an unknown id, which the handler folds into a normal {ok:false} result instead of -32010" },
|
||||||
|
|
||||||
{ "fixture": "contracts/fixtures/category.list.json", "reason": "documented gap (deferrals.md D3a note): categories table has no mimeTypes/sortOrder columns, so category.upsert accepts them but category.list never echoes mimeTypes back" },
|
{ "fixture": "contracts/fixtures/category.list.json", "reason": "documented gap (deferrals.md D3a note): categories table has no mimeTypes/sortOrder columns, so category.upsert accepts them but category.list never echoes mimeTypes back" },
|
||||||
|
|
||||||
|
{ "fixture": "contracts/fixtures/capture.getRules.json", "reason": "not a bug: capture.monitoredMimeTypes defaults to [] (store/settings.cpp's kDefaults) on a fresh daemon; the golden's non-empty example illustrates a configured one" },
|
||||||
|
|
||||||
{ "fixture": "contracts/fixtures/download.probe.json", "reason": "not a bug: requiresAuth is optional-and-omitted-when-false (schema doesn't require it); the golden shows it because that fixture's probe hit a 401, this run's doesn't" },
|
{ "fixture": "contracts/fixtures/download.probe.json", "reason": "not a bug: requiresAuth is optional-and-omitted-when-false (schema doesn't require it); the golden shows it because that fixture's probe hit a 401, this run's doesn't" },
|
||||||
{ "fixture": "contracts/fixtures/download.get.json", "reason": "not a bug: effectiveUrl is 'null until the first probe succeeds' (schema) and omitted rather than sent as null; our bound $taskId is a fresh, never-started task, so it's never been probed -- the golden depicts an in-progress download instead" },
|
{ "fixture": "contracts/fixtures/download.get.json", "reason": "not a bug: effectiveUrl is 'null until the first probe succeeds' (schema) and omitted rather than sent as null; our bound $taskId is a fresh, never-started task, so it's never been probed -- the golden depicts an in-progress download instead" },
|
||||||
{ "fixture": "contracts/fixtures/download.list.json", "reason": "same as download.get.json: effectiveUrl omitted for our never-started bound tasks, golden depicts an in-progress download" },
|
{ "fixture": "contracts/fixtures/download.list.json", "reason": "same as download.get.json: effectiveUrl omitted for our never-started bound tasks, golden depicts an in-progress download" },
|
||||||
{ "fixture": "contracts/fixtures/session.hello.json", "reason": "not a bug: capabilities is genuinely empty because media/grabber/Secret Service aren't implemented yet; the golden's ['media','grabber','secretservice'] illustrates a future daemon, not this one" },
|
{ "fixture": "contracts/fixtures/download.update.json", "reason": "not a bug: etaSeconds is only known for a task the engine has probed/is running; our bound $taskId is a fresh, never-started task, so it's absent -- same class as download.get.json's effectiveUrl" },
|
||||||
|
{ "fixture": "contracts/fixtures/session.hello.json", "reason": "not a bug: capabilities is genuinely empty because media/grabber aren't implemented yet (capture/Secret Service's parts of it now are); the golden's example list illustrates a future daemon, not this one" },
|
||||||
|
|
||||||
{ "fixture": "contracts/fixtures/queue.start.json", "reason": "not a bug: startedTaskIds is empty because nothing is a member of queue 'main' in this isolated run; the golden depicts a queue with real membership" },
|
{ "fixture": "contracts/fixtures/queue.start.json", "reason": "not a bug: startedTaskIds is empty because nothing is a member of queue 'main' in this isolated run; the golden depicts a queue with real membership" },
|
||||||
|
{ "fixture": "contracts/fixtures/queue.reorder.json", "reason": "not a bug: the fixture's taskIds are two literal ids that only ever existed in a seeded mock; queue 'main' has no members at all in this isolated run, so any non-empty list is correctly rejected as not a permutation of (empty) membership" },
|
||||||
|
{ "fixture": "contracts/fixtures/rules.list.json", "reason": "not a bug: no rule is ever seeded in a fresh daemon; the golden depicts a configured rule set" },
|
||||||
|
{ "fixture": "contracts/fixtures/schedule.set.json", "reason": "documented gap (deferrals.md D3f note): nextRunAt is deliberately left unset -- computing it needs DST-aware next-transition logic sched/schedule_window.hpp doesn't have yet" },
|
||||||
{ "fixture": "contracts/fixtures/category.remove.json", "reason": "not a bug: reassignedTaskIds is empty because nothing was ever filed under the 'firmware' category this run creates; the golden depicts a category with real membership" }
|
{ "fixture": "contracts/fixtures/category.remove.json", "reason": "not a bug: reassignedTaskIds is empty because nothing was ever filed under the 'firmware' category this run creates; the golden depicts a category with real membership" }
|
||||||
]
|
]
|
||||||
|
|||||||
+46
-72
@@ -88,88 +88,62 @@ manual testing.
|
|||||||
nowhere to run. GUI owns the harness; PKG/QA owns the CI job. This is the wiring contract
|
nowhere to run. GUI owns the harness; PKG/QA owns the CI job. This is the wiring contract
|
||||||
so the two halves meet without another round trip.
|
so the two halves meet without another round trip.
|
||||||
|
|
||||||
### What PKG/QA needs from GUI
|
### What GUI built, and the invocation contract
|
||||||
|
|
||||||
A driver invoked as `gui/tests/dod/run.sh <gate> [--json <path>]` (exact path TBD by GUI),
|
`gui/tests/dod/run.sh <gate> [--json <path>]` (`gate` one of `scroll-60fps` / `rss-flat`
|
||||||
headless-capable (Xvfb or offscreen `QT_QPA_PLATFORM`), with:
|
/ `unhappy-path`), backed by `gui/tests/dod/dod_harness.cpp` (target `gui-dod-harness`).
|
||||||
|
Headless by default — `run.sh` sets `QT_QPA_PLATFORM=offscreen` itself, so the CI jobs
|
||||||
|
below need no Xvfb.
|
||||||
|
|
||||||
| `<gate>` | Pass / fail condition | Budget |
|
| `<gate>` | Pass / fail condition | Budget |
|
||||||
|---|---|---|
|
|---|---|---|
|
||||||
| `scroll-60fps` | `mockd --tasks 10000`, scripted fling scroll; **fail** if p99 frame > 16.6 ms | per-PR |
|
| `scroll-60fps` | `mockd --tasks 10000`, scripted fling scroll; **fail** if p99 frame > 16.6 ms (×4 under an ASan/UBSan build — `VELOX_DOD_FRAME_BUDGET_MS` overrides outright) | per-PR |
|
||||||
| `rss-flat` | `mockd --tasks 10000` + progress events, 10 min; **fail** if RSS growth > a fixed slack (GUI picks the number, states it) | nightly |
|
| `rss-flat` | `mockd --tasks 10000` + progress events, 10 min; **fail** if RSS growth past warm-up exceeds a 20 MiB slack (`VELOX_DOD_RSS_SLACK_KIB` overrides) | nightly |
|
||||||
| `unhappy-path` | `mockd --slow` / `--flaky <f>` / `--drop-connection <s>`; **fail** on crash, on watchdog-detected hang, or if connection state never returns to `Connected` | per-PR |
|
| `unhappy-path` | Three phases (`--slow` / `--flaky <f>` / `--drop-connection <s>`), one `mockd` restart each; **fail** on crash, on the harness's own watchdog firing (75 s), or if connection state never (re)reaches `Connected` | per-PR |
|
||||||
|
|
||||||
Contract:
|
Contract, as built: exit `0` pass, non-zero fail; a hang is the harness's own watchdog
|
||||||
|
converting itself into a non-zero exit (`3`), never something CI needs `timeout(1)`
|
||||||
|
around; `--json` writes one result object per gate (`unhappy-path` merges its three
|
||||||
|
phases into one file); no network, no writes outside a tempdir, no leaked child process
|
||||||
|
on any exit path (`run.sh`'s `trap cleanup EXIT INT TERM`).
|
||||||
|
|
||||||
* exit `0` pass, non-zero fail; a hang is the harness's own watchdog to catch and turn
|
### CI jobs — wired
|
||||||
into a non-zero exit, not something CI should have to `timeout(1)` around.
|
|
||||||
* `--json` writes one machine-readable result file (measured p99, RSS series, recovery
|
|
||||||
time) so the job can upload it as an artifact and a regression is a diff, not a re-run.
|
|
||||||
* no network, no writes outside a tempdir, no leaked child processes on failure.
|
|
||||||
|
|
||||||
### CI job — pre-drafted, add once the harness path is fixed
|
`gui-dod` (per-PR: `scroll-60fps` + `unhappy-path`) and `gui-dod-nightly` (`rss-flat`,
|
||||||
|
`schedule`/`workflow_dispatch` only) are live in `ci.yml`. Both bootstrap, build
|
||||||
|
`gui-dod-harness`, `npm ci` in `tools/mockd`, run `gui/tests/dod/run.sh`, and upload the
|
||||||
|
`--json` output as an artifact — no Xvfb step, since `run.sh` already runs offscreen.
|
||||||
|
|
||||||
```yaml
|
### Forced red, once per gate, before wiring it required
|
||||||
gui-dod:
|
|
||||||
# Per-PR GUI gates. The 10-minute rss-flat gate is in gui-dod-nightly, not here.
|
|
||||||
runs-on: ubuntu-latest
|
|
||||||
steps:
|
|
||||||
- uses: actions/checkout@v4
|
|
||||||
- name: Bootstrap toolchain
|
|
||||||
run: sudo ./tools/bootstrap.sh
|
|
||||||
- uses: actions/setup-node@v4
|
|
||||||
with:
|
|
||||||
node-version: '22' # tools/mockd
|
|
||||||
- name: Configure + build
|
|
||||||
run: |
|
|
||||||
cmake --preset dev
|
|
||||||
cmake --build --preset dev --target velox-gui
|
|
||||||
- name: Install mockd
|
|
||||||
run: cd tools/mockd && npm ci
|
|
||||||
- name: Xvfb + gates
|
|
||||||
run: |
|
|
||||||
sudo apt-get install -y --no-install-recommends xvfb
|
|
||||||
xvfb-run -a gui/tests/dod/run.sh scroll-60fps --json scroll.json # TODO(GUI): path
|
|
||||||
xvfb-run -a gui/tests/dod/run.sh unhappy-path --json unhappy.json
|
|
||||||
- uses: actions/upload-artifact@v4
|
|
||||||
if: always()
|
|
||||||
with:
|
|
||||||
name: gui-dod-${{ github.run_id }}
|
|
||||||
path: "*.json"
|
|
||||||
|
|
||||||
gui-dod-nightly:
|
| Gate | Forced via | Observed |
|
||||||
if: github.event_name == 'schedule'
|
|---|---|---|
|
||||||
runs-on: ubuntu-latest
|
| `scroll-60fps` | `VELOX_DOD_FRAME_BUDGET_MS=0.01` | `FAIL: p99=50.73 ms mean=31.34 ms max=52.88 ms budget=0.01 ms over 240 steps, 10000 rows`, exit 1 |
|
||||||
steps:
|
| `rss-flat` | `VELOX_DOD_RSS_SLACK_KIB=-999999999` (guarantees `growthKiB > slack` regardless of actual RSS behavior that run) | `FAIL: growth=13644 KiB slack=-999999999 KiB over 15s (warmup 1s), 10000 rows`, exit 1 |
|
||||||
- uses: actions/checkout@v4
|
| `unhappy-path` | ran `gui-dod-harness` directly (bypassing `run.sh`, which always starts a working `mockd`) against a `--sock` path with nothing listening | 75 s of `Connecting`/`Reconnecting`, then `FAIL: dod_harness watchdog fired — hung past 75s`, exit 3 |
|
||||||
- name: Bootstrap toolchain
|
|
||||||
run: sudo ./tools/bootstrap.sh
|
|
||||||
- uses: actions/setup-node@v4
|
|
||||||
with:
|
|
||||||
node-version: '22'
|
|
||||||
- name: Configure + build
|
|
||||||
run: |
|
|
||||||
cmake --preset dev
|
|
||||||
cmake --build --preset dev --target velox-gui
|
|
||||||
- run: cd tools/mockd && npm ci
|
|
||||||
- name: RSS soak (10 min)
|
|
||||||
run: |
|
|
||||||
sudo apt-get install -y --no-install-recommends xvfb
|
|
||||||
xvfb-run -a gui/tests/dod/run.sh rss-flat --json rss.json
|
|
||||||
- uses: actions/upload-artifact@v4
|
|
||||||
if: always()
|
|
||||||
with:
|
|
||||||
name: gui-dod-rss-${{ github.run_id }}
|
|
||||||
path: rss.json
|
|
||||||
```
|
|
||||||
|
|
||||||
`gui-dod-nightly` needs `if: github.event_name == 'schedule'` (the `nightly-integration`
|
The real (non-forced) runs all pass live: `scroll-60fps` p99 51.28 ms against a 66.4 ms
|
||||||
job below already added that trigger to `ci.yml` — `cron: '17 3 * * *'` — so this no
|
budget (ASan/UBSan build), `rss-flat` growth 13504 KiB against the 20480 KiB slack over a
|
||||||
longer needs its own).
|
15 s smoke duration, `unhappy-path` all three phases `PASS` with `finalConnected=true`.
|
||||||
|
|
||||||
|
### Known gap: `unhappy-path`'s drop-connection phase doesn't drop anything
|
||||||
|
|
||||||
|
Filed by GUI in `gui/docs/proto-requests-m1.md`: `mockd --drop-connection` only works
|
||||||
|
over WebSocket — `startUds()` never wires the periodic-drop timer `startWs()` has, so the
|
||||||
|
UDS transport (GUI/CLI/nmhost's only one) never sees a connection actually die. GUI's own
|
||||||
|
live verification: 45 s observing a `--drop-connection 5` mockd over UDS, `stateChanged`
|
||||||
|
never fires. The phase still runs and its JSON honestly records `sawDisruption: false`
|
||||||
|
rather than silently passing as if it proved something — that's what the real run above
|
||||||
|
shows, `pass: true` alongside `sawDisruption: false`, so a reviewer reading the JSON sees
|
||||||
|
exactly how much this phase currently covers.
|
||||||
|
|
||||||
|
This is PROTO's fix, not PKG/QA's or GUI's to route around locally — coordinating on it
|
||||||
|
rather than patching mockd from this lane. Once PROTO lands `dropEverySec` on `startUds`,
|
||||||
|
`unhappy-path`'s drop-connection phase starts exercising a real drop and this note comes
|
||||||
|
out; until then `gui-dod` stays required as specified (crash/hang/never-reconnects still
|
||||||
|
catch real regressions), just not yet catching a swallowed real disconnect.
|
||||||
|
|
||||||
### Status
|
### Status
|
||||||
|
|
||||||
Blocked on GUI's harness. Not urgent (GUI M1 DoD, not M0). When GUI files the follow-up
|
Wired and required: `gui-dod` runs per-PR, `gui-dod-nightly` on schedule. Revisit once
|
||||||
with the real `run.sh` path and the `rss-flat` slack number, PKG/QA drops the `TODO(GUI)`
|
PROTO's UDS `--drop-connection` fix lands (see above).
|
||||||
markers and marks `gui-dod` required. The `schedule:` trigger `gui-dod-nightly` needs is
|
|
||||||
already in `ci.yml`.
|
|
||||||
|
|||||||
@@ -288,7 +288,10 @@ export class Dispatcher {
|
|||||||
}
|
}
|
||||||
|
|
||||||
case 'limiter.get':
|
case 'limiter.get':
|
||||||
return state.limiter;
|
// applyToRunning is a write-only instruction on limiter.set ("retune already-
|
||||||
|
// running transfers now"), not a persisted setting, so get never echoes it back
|
||||||
|
// (contracts/fixtures/limiter.get.json).
|
||||||
|
return { enabled: state.limiter.enabled, globalBps: state.limiter.globalBps };
|
||||||
|
|
||||||
case 'limiter.set': {
|
case 'limiter.set': {
|
||||||
state.limiter = {
|
state.limiter = {
|
||||||
|
|||||||
Reference in New Issue
Block a user