From 4e177ec809ea4fe2695b174ef1570763fc4aea6c Mon Sep 17 00:00:00 2001 From: sami Date: Sat, 12 Sep 2026 16:56:35 +0400 Subject: [PATCH] proto: stop conformance from downloading real files, ADR the null-clearing gap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. download.add.json had startMode "now" against a real, large (~6 GB) Ubuntu ISO with saveDir hardcoded to /home/sami/Downloads/Programs. Against a real veloxd (tests/conformance/run.sh) that's a real download into the real user's real home, every single run — it had already happened twice. startMode -> "later" (exercises the add path, hands nothing to the engine) and saveDir is dropped entirely (resolves to saveTo.defaultDir instead, checked against allowedRoots the same way). Documented the rule this fixture was breaking in contracts/fixtures/README.md so it doesn't happen a third time. Auditing the rest for the same shape (now real: capture.offer, D7) found a second, subtler instance: capture.offer.take.json's "take" admits a real, immediately-started task the same way download.add does, and the "Programs" category's saveDir is a migration-seeded builtin (~/Downloads/Programs) that no isolated test setup can redirect -- so even after pointing the URL at example.org (RFC 2606), a real ~6 GB sparse .veloxpart still landed in the real home on the declared Content-Length alone. Shrunk to a plausible-but-small 5 MiB. Also scoped to "transport": "uds" -- a real "take" persists an active task, so replaying the same fixture again on the second live transport against the same shared daemon was hitting capture.offer's own dedupe-by-URL and failing on a missing taskId, not a bug. download.add's other real-URL siblings (errors/*.invalid-path, *.invalid-params, *.disk-full) all fail before admission or are requires-gated; left alone. 2. ADR 0018: DAEMON can set a nullable field through download.update / settings.set but never clear it back to null, because the generated C++ parser collapses "absent" and "explicit null" to the same std::nullopt for every optional field (contracts/codegen/gen_cpp.py, on purpose, and correct for create-style params -- just wrong for patch-style ones, which is the only place the schema documents "explicit null clears"). Decision: an opt-in x-clearable schema annotation makes just those fields std::optional> in C++ (TS already round-trips this natively); not a blanket rule (would retype response fields like TaskSummary.effectiveUrl that have no clear-vs-absent distinction to make), not an explicit clear-list field (would redesign a wire contract DAEMON already built against just to route around a generator gap). Recorded, not implemented here -- that's its own PROTO PR (schema annotations + gen_cpp.py + gen_ts.py + regeneration + a minor VERSION bump per ADR 0015), not bundled into a fixture-safety pass. Left a pointer to the ADR at the generator comment it concerns. 3. Re-verified every xfail entry against current deferrals.md rather than trust the reasons already on file: D7/D8 (capture.offer/ getRules), D3d/e/f/g/h/i (rules, queue.reorder, schedule, limiter, download.update/refreshUrl) and D9 (settings) have all closed since the list was last pruned, so most of it was stale. Removed everything that now cleanly passes; kept and re-reasoned everything that doesn't: - errors/download.provideAuth.not-found.json stays, as asked: real bug, on_download_provideAuth never checks the task exists. - category.list.json (mimeTypes -- documented D3a gap), schedule.set.json (nextRunAt -- documented D3f gap): unchanged in substance, reason text was already accurate. - download.probe/get/list/update.json, session.hello.json, queue.start/reorder.json, category.remove.json: not bugs -- each golden depicts a richer lifecycle/config state (a probed download, real queue or category membership, media/grabber capabilities) than this harness's fresh, never-started bound tasks and empty isolated DB can produce. - limiter.get.json: real fixture bug, not a daemon one -- applyToRunning is a write-only instruction on limiter.set, on_limiter_get never returns it; the golden shouldn't have had it either. Fixed the fixture and tools/mockd's own limiter.get, which had the same field hardcoded into its in-memory state independent of the fixture file. - grabber.*/media.*: still genuinely stub (M4 territory). Only remaining unexpected-pass surfaced while re-verifying (errors/capture.offer.ignore.json, always "take" instead of "ignore") traced to capture.minSizeBytes defaulting to 0 on a fresh daemon, making its below-minimum-size scenario unreachable -- not a bug, so raised the setting in run.sh's isolated seeding instead of xfailing it. ctest -L conformance: green, 100% (2/2), ~87s. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01SFeUKLbdHizrJjLBeK7ffz --- contracts/codegen/gen_cpp.py | 7 +- contracts/fixtures/README.md | 26 ++++- contracts/fixtures/capture.offer.take.json | 11 +- contracts/fixtures/download.add.json | 13 +-- contracts/fixtures/limiter.get.json | 8 +- ...optional-fields-absent-vs-explicit-null.md | 107 ++++++++++++++++++ tests/conformance/run.sh | 22 +++- tests/conformance/veloxd-xfail.json | 40 +++---- tools/mockd/src/dispatch.ts | 5 +- 9 files changed, 188 insertions(+), 51 deletions(-) create mode 100644 docs/adr/0018-nullable-optional-fields-absent-vs-explicit-null.md diff --git a/contracts/codegen/gen_cpp.py b/contracts/codegen/gen_cpp.py index 402fda8..987a49a 100644 --- a/contracts/codegen/gen_cpp.py +++ b/contracts/codegen/gen_cpp.py @@ -506,7 +506,12 @@ def emit_field_parse(f: Field, indent: str) -> list[str]: f'{i} const auto it = j.find("{f.name}");'] if f.optional: # 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 += emit_value_parse(f.type, "(*it)", "val", "fp", i + " ") o.append(f"{i} out.{m} = std::move(val);") diff --git a/contracts/fixtures/README.md b/contracts/fixtures/README.md index 9aee282..c15d5be 100644 --- a/contracts/fixtures/README.md +++ b/contracts/fixtures/README.md @@ -21,7 +21,7 @@ fixtures/ ```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.", "transport": "uds", // optional: replay only on this transport "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 Firefox download normally. A download manager that eats downloads when its daemon is down 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. diff --git a/contracts/fixtures/capture.offer.take.json b/contracts/fixtures/capture.offer.take.json index ff3b13a..f5db860 100644 --- a/contracts/fixtures/capture.offer.take.json +++ b/contracts/fixtures/capture.offer.take.json @@ -1,22 +1,23 @@ { "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": { "jsonrpc": "2.0", "id": 42, "method": "capture.offer", "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", - "tabUrl": "https://releases.ubuntu.com/26.04/", + "tabUrl": "https://example.org/26.04/", "headers": { "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": "*/*" }, "cookies": [], "contentType": "application/octet-stream", - "contentLength": 6228541440, + "contentLength": 5242880, "contentDisposition": "attachment; filename=\"ubuntu-26.04-desktop-amd64.iso\"", "filename": "ubuntu-26.04-desktop-amd64.iso", "origin": "moz-extension://11111111-2222-3333-4444-555555555555" diff --git a/contracts/fixtures/download.add.json b/contracts/fixtures/download.add.json index 9733715..ad1a9f2 100644 --- a/contracts/fixtures/download.add.json +++ b/contracts/fixtures/download.add.json @@ -1,6 +1,6 @@ { - "name": "download.add \u2014 start an ISO now, into the Programs category", - "description": "The ordinary add path. saveDir is canonicalized and checked against the allowed roots before anything is written.", + "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. 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": { "jsonrpc": "2.0", "id": 11, @@ -8,10 +8,9 @@ "params": { "url": "https://releases.ubuntu.com/26.04/ubuntu-26.04-desktop-amd64.iso", "filename": "ubuntu-26.04-desktop-amd64.iso", - "saveDir": "/home/sami/Downloads/Programs", "categoryId": "programs", "segments": 8, - "startMode": "now" + "startMode": "later" } }, "response": { @@ -19,13 +18,13 @@ "id": 11, "result": { "taskId": "$uuid", - "state": "connecting", + "state": "paused", "duplicate": null } }, "assertions": [ - "the .veloxpart file is created sparse and preallocated at the final size", - "saveDir resolves inside saveTo.allowedRoots, or the call fails -32011 having written nothing", + "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", + "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" ] } diff --git a/contracts/fixtures/limiter.get.json b/contracts/fixtures/limiter.get.json index e618c9c..e7a75c7 100644 --- a/contracts/fixtures/limiter.get.json +++ b/contracts/fixtures/limiter.get.json @@ -1,6 +1,6 @@ { "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": { "jsonrpc": "2.0", "id": 52, @@ -12,11 +12,11 @@ "id": 52, "result": { "enabled": false, - "globalBps": 2097152, - "applyToRunning": false + "globalBps": 2097152 } }, "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" ] } diff --git a/docs/adr/0018-nullable-optional-fields-absent-vs-explicit-null.md b/docs/adr/0018-nullable-optional-fields-absent-vs-explicit-null.md new file mode 100644 index 0000000..ef5333b --- /dev/null +++ b/docs/adr/0018-nullable-optional-fields-absent-vs-explicit-null.md @@ -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>`: + 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` -> `optional>`) +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` 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` 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()`, used for *params* types the daemon receives — but the conformance C++ runner +also instantiates `parse()` 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. diff --git a/tests/conformance/run.sh b/tests/conformance/run.sh index 4199b50..1db6943 100755 --- a/tests/conformance/run.sh +++ b/tests/conformance/run.sh @@ -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 [ -S "$VUDS" ] || { echo "veloxd did not start:"; cat "$WORK/veloxd.log"; exit 1; } - # saveTo.allowedRoots defaults to ["~/Downloads"]; download.add.json (fixture) asks - # for a saveDir under $HOME/Downloads, so both that and download.add's own isolated - # downloads dir need to be allowed roots, or every download.add fixture fails -32011 - # before the point of this runner is even reached. settings.set is itself a D3 stub, - # so this is written straight into the isolated velox.db rather than over the wire. + # saveTo.allowedRoots defaults to ["~/Downloads"]; download.add's own isolated + # downloads dir needs to be an allowed root too, or every download.add fixture fails + # -32011 before the point of this runner is even reached. $HOME/Downloads stays in + # the list alongside it: a few fixtures still set an explicit saveDir there + # (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' import json, sqlite3, sys 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", ("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( "INSERT INTO settings(key, value) VALUES(?, ?) " "ON CONFLICT(key) DO UPDATE SET value = excluded.value", diff --git a/tests/conformance/veloxd-xfail.json b/tests/conformance/veloxd-xfail.json index cde996f..59e33b7 100644 --- a/tests/conformance/veloxd-xfail.json +++ b/tests/conformance/veloxd-xfail.json @@ -1,40 +1,26 @@ [ - { "fixture": "contracts/fixtures/download.refreshUrl.json", "reason": "D3: stub handler, -32603" }, - { "fixture": "contracts/fixtures/download.update.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/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/rules.upsert.json", "reason": "D3: stub handler, -32603" }, - - { "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/media.addVariant.json", "reason": "D3: stub handler, -32603 (M4 territory per deferrals.md)" }, + { "fixture": "contracts/fixtures/media.listVariants.json", "reason": "D3: stub handler, -32603 (M4 territory per deferrals.md)" }, { "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/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.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/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.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" } ] diff --git a/tools/mockd/src/dispatch.ts b/tools/mockd/src/dispatch.ts index 5664315..9c29fbd 100644 --- a/tools/mockd/src/dispatch.ts +++ b/tools/mockd/src/dispatch.ts @@ -288,7 +288,10 @@ export class Dispatcher { } 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': { state.limiter = {