Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
  samiandClaude Sonnet 5
|
b5c6e1f47d |
@@ -506,12 +506,7 @@ 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 --
|
||||
# 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).
|
||||
# a nullable field and one that sends null are treated identically on purpose.
|
||||
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);")
|
||||
|
||||
@@ -21,7 +21,7 @@ fixtures/
|
||||
|
||||
```jsonc
|
||||
{
|
||||
"name": "download.add — add an ISO for later, into the Programs category",
|
||||
"name": "download.add — start an ISO now, 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,27 +81,3 @@ 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.
|
||||
|
||||
@@ -1,23 +1,22 @@
|
||||
{
|
||||
"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. 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",
|
||||
"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.",
|
||||
"request": {
|
||||
"jsonrpc": "2.0",
|
||||
"id": 42,
|
||||
"method": "capture.offer",
|
||||
"params": {
|
||||
"url": "https://example.org/dl/ubuntu-26.04-desktop-amd64.iso",
|
||||
"url": "https://releases.ubuntu.com/26.04/ubuntu-26.04-desktop-amd64.iso",
|
||||
"method": "GET",
|
||||
"tabUrl": "https://example.org/26.04/",
|
||||
"tabUrl": "https://releases.ubuntu.com/26.04/",
|
||||
"headers": {
|
||||
"User-Agent": "Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:154.0) Gecko/20100101 Firefox/154.0",
|
||||
"Referer": "https://example.org/26.04/",
|
||||
"Referer": "https://releases.ubuntu.com/26.04/",
|
||||
"Accept": "*/*"
|
||||
},
|
||||
"cookies": [],
|
||||
"contentType": "application/octet-stream",
|
||||
"contentLength": 5242880,
|
||||
"contentLength": 6228541440,
|
||||
"contentDisposition": "attachment; filename=\"ubuntu-26.04-desktop-amd64.iso\"",
|
||||
"filename": "ubuntu-26.04-desktop-amd64.iso",
|
||||
"origin": "moz-extension://11111111-2222-3333-4444-555555555555"
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"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.",
|
||||
"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.",
|
||||
"request": {
|
||||
"jsonrpc": "2.0",
|
||||
"id": 11,
|
||||
@@ -8,9 +8,10 @@
|
||||
"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": "later"
|
||||
"startMode": "now"
|
||||
}
|
||||
},
|
||||
"response": {
|
||||
@@ -18,13 +19,13 @@
|
||||
"id": 11,
|
||||
"result": {
|
||||
"taskId": "$uuid",
|
||||
"state": "paused",
|
||||
"state": "connecting",
|
||||
"duplicate": null
|
||||
}
|
||||
},
|
||||
"assertions": [
|
||||
"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)",
|
||||
"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",
|
||||
"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",
|
||||
"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.",
|
||||
"description": "globalBps still carries the last configured value so the GUI can restore it when the user re-enables the limit.",
|
||||
"request": {
|
||||
"jsonrpc": "2.0",
|
||||
"id": 52,
|
||||
@@ -12,11 +12,11 @@
|
||||
"id": 52,
|
||||
"result": {
|
||||
"enabled": false,
|
||||
"globalBps": 2097152
|
||||
"globalBps": 2097152,
|
||||
"applyToRunning": false
|
||||
}
|
||||
},
|
||||
"assertions": [
|
||||
"enabled false means no throttling regardless of globalBps",
|
||||
"applyToRunning is absent, not false: it's meaningless outside a limiter.set call"
|
||||
"enabled false means no throttling regardless of globalBps"
|
||||
]
|
||||
}
|
||||
|
||||
@@ -1,107 +0,0 @@
|
||||
# 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.
|
||||
@@ -19,7 +19,12 @@ import {
|
||||
} from './context-menus.js';
|
||||
import { MediaBridge, notifyTab } from './media-bridge.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 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 {
|
||||
const detail = status.fatal ?? (status.needsPairing ? 'needs pairing' : '');
|
||||
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> {
|
||||
|
||||
@@ -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,18 +160,11 @@ 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'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.
|
||||
# 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.
|
||||
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]
|
||||
@@ -181,11 +174,6 @@ 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",
|
||||
|
||||
@@ -1,26 +1,40 @@
|
||||
[
|
||||
{ "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/download.refreshUrl.json", "reason": "D3: stub handler, -32603" },
|
||||
{ "fixture": "contracts/fixtures/download.update.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/media.listVariants.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/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/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/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/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" }
|
||||
]
|
||||
|
||||
@@ -288,10 +288,7 @@ export class Dispatcher {
|
||||
}
|
||||
|
||||
case 'limiter.get':
|
||||
// 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 };
|
||||
return state.limiter;
|
||||
|
||||
case 'limiter.set': {
|
||||
state.limiter = {
|
||||
|
||||
Reference in New Issue
Block a user