lane/proto
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  samiandClaude Sonnet 5
|
4e177ec809 |
proto: stop conformance from downloading real files, ADR the null-clearing gap
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<std::optional<T>> 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 <[email protected]>
Claude-Session: https://claude.ai/code/session_01SFeUKLbdHizrJjLBeK7ffz
|
||
|
samiandClaude Sonnet 5
|
e30d994d74 |
proto: fix conformance run.sh flakiness, prune the veloxd xfail list
Two run.sh fixes plus the xfail prune, all requested together:
1. VELOX_PAIR_AUTO=1 for the isolated veloxd. Pairing is the D1 dev stub
(EnvAutoApprover) and denies without it, so session.pair never issued
a token and the WS half of the veloxd step could never even connect.
2. WS_PORT was hardcoded to 52080 with no free-port search, so one leaked
mockd made every future run fail EADDRINUSE. free_port() binds :0 and
asks the kernel instead. The EXIT trap's stop() used `pkill -P "$pid"`,
which only reaps direct children — tsx's actual listener is often a
grandchild, which that missed and left holding the port. Every server
(mockd, slow mockd, veloxd) now launches under `setsid`, making it the
leader of its own process group, so stop() does `kill -TERM -"$pid"`
(a process-group kill) and reaches everything it spawned in one shot.
3. Pruned the xfail list now that D2, D4b and most of D3 have landed.
Pruning surfaced two more bugs than expected, both in the test harness
itself, not veloxd — worth recording since they were indistinguishable
from real daemon hangs until isolated:
- errors/session.hello.version-mismatch.json documents that the *server*
closes the connection after replying (correct, intended behavior). The
harness replays every fixture on one shared connection per transport,
so once this fixture ran, every later UDS fixture sent into the dead
socket and just sat there until its own timeout — including ones still
on the xfail list, which applyXfail waved through as "expected -32603"
regardless of the real reason. Fixed with a `closesConnection` fixture
flag: replay() reconnects (fresh session.hello) right after such a
fixture instead of leaving the rest of the run to time out one by one.
This is what was actually behind queue.*/session.*/download.remove
appearing to hang — none of them do; verified individually and via a
raw probe script before finding the real cause.
- category.remove.json (deletes the "firmware" category) sorted before
category.upsert.json (creates it) alphabetically, so it was failing
-32602 "no such category" against a fresh DB — never a daemon bug.
Added it to DESTRUCTIVE so it now replays after every other fixture.
Also fixed while verifying "confirm each really passes": download.addBatch.json's
`defaults.categoryId` was "compressed", a category nothing ever creates —
real veloxd correctly enforces the FK on tasks.category_id, so all three
batch items failed instead of the two expected. Changed to "programs" (a
migration-seeded builtin).
Of the 15 fixtures named for pruning, 10 turned out to cleanly pass and
are gone from the list entirely: download.pause/resume/start/cancel,
download.remove, download.addBatch, queue.upsert/stop, download.probe's
success path (D2, including errors/download.probe.probe-failed.json),
and category.upsert. Two do NOT cleanly pass and are kept, with reasons
rewritten to match what's actually happening now instead of the stale D3
text: download.probe.json (see below) and errors/download.provideAuth.not-found.json,
a real bug — on_download_provideAuth never checks the task exists, so an
unknown taskId gets a normal `{ok:false}` result instead of -32010.
Five more fixtures newly needed xfail entries to reach green, none of
them stubs:
- category.list.json — documented gap (deferrals.md's D3a note): the
categories table has no mimeTypes/sortOrder columns.
- download.probe.json, download.get.json, download.list.json,
session.hello.json — not bugs. Each golden depicts a richer lifecycle
state (a probed/in-progress download, a daemon with media/grabber/
Secret Service implemented) than this harness's bound tasks, which are
always fresh and never started, can produce. Optional/omit-if-absent
fields (effectiveUrl, requiresAuth, capabilities) are correctly absent;
the mismatch is against the golden's illustrative values, not the
contract.
- queue.start.json, category.remove.json — same class: startedTaskIds /
reassignedTaskIds are correctly empty because this run's queue/category
have no real membership.
`ctest -L conformance` is green: 100% (2/2), 81.7s (down from ~240s now
that pairing and the port/reconnect fixes remove the retries and the
5-10s timeouts the connection-death bug was producing).
One thing NOT fixed here, flagged for a follow-up decision rather than
touched mid-task: download.add.json's fixture is `startMode: "now"`
against a real, large (~6GB) Ubuntu ISO on the real internet, with
saveDir hardcoded to /home/sami/Downloads/Programs. Every run against a
real veloxd writes a real multi-GB file into that path — confirmed by
running this repeatedly during verification. Isolating the daemon's XDG
dirs doesn't isolate this. Worth its own change (startMode: "later"
would still exercise the add path without the transfer) but out of scope
for a fixture I wasn't asked to touch beyond what blocked this task.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01SFeUKLbdHizrJjLBeK7ffz
|
||
|
samiandClaude Sonnet 5
|
5b03eff926 |
proto: wire veloxd into the conformance suite's canonical entry point
run.sh's TS runner only ever started mockd; veloxd existed but nothing in
the ctest -L conformance path touched it, so mockd's always-valid fixtures
were the only thing ts/replay.ts ever saw. That let a real bug through:
veloxd's download.get can return segments: 0, which TaskSummary.segments
forbids (minimum 1, required) — nothing caught it.
Add a 3b step to run.sh (still the one canonical entry, per ADR 0014):
builds veloxd, starts it isolated (its own XDG_RUNTIME_DIR/XDG_DATA_HOME/
XDG_CONFIG_HOME), seeds saveTo.allowedRoots/defaultDir directly into the
isolated velox.db (settings.set is itself a stub, and the default
~/Downloads root doesn't isolate download.add's writes), then replays
every fixture against it over both transports.
Most handlers are still stubs (daemon/docs/deferrals.md D1-D4b). Fixtures
that hit them get an expected-failure entry in the new veloxd-xfail.json,
loaded by replay.ts's new --xfail flag. This is a maintained allowlist,
not a snapshot: a listed fixture that unexpectedly *passes* is flipped
back to a failure (applyXfail), so the list can only shrink as DAEMON
lands handlers, never rot into a list nobody rechecks. download.get's
segments: 0 is deliberately *not* on it — that's the regression this
step exists to catch.
Also hardened setupBindings: a server that can't even complete fixture
binding used to take the whole runner down with an uncaught exception
before a single fixture was checked. It's now a reported Outcome instead,
so the run still produces a coherent report. That robustness fix earned
its keep immediately: veloxd's download.add crashes on startMode
"later" (a valid, documented StartMode — "the File Info dialog's
Download Later button") with a SQLite CHECK constraint violation, because
migrations/0001_initial.sql's start_mode CHECK never had 'later' added to
it (and includes 'manual'/'auto', neither a contract value). That's a
second, more severe bug this wiring found, unrelated to segments: 0 and
currently blocking most of the veloxd run — filed for DAEMON in
tests/conformance/README.md, not fixed here (out of lane). capture.offer
and capture.getRules are also stubs but missing from deferrals.md's
D-list; xfailed with a note asking DAEMON to add the row.
Verified live once against a real, isolated veloxd before this session's
sandbox became persistently contended for veloxd's single-instance lock
(UID-scoped, not namespaced by XDG_RUNTIME_DIR — daemon/src/main.cpp;
documented as a caveat in the README): it built, started isolated, seeded
settings, connected over both transports, and surfaced the startMode bug
above as a real, non-xfailed failure — confirming the whole pipeline
including --xfail end to end. segments: 0 is confirmed by direct reading
of daemon/src/store/tasks.{hpp,cpp} (TaskRow::eff_segments defaults to 0,
copied verbatim into TaskSummary.segments) rather than by a second live
run reaching that specific fixture, since setup itself fails first on the
startMode bug above. mockd path re-verified green after these changes
(200/200, up from 196/196 — the new setup/$taskId outcomes are visible
and passing).
Recommendation for PKG: don't flip this required yet. The existing
`conformance` ctest entry is already a required check, and right now the
startMode bug fails most of the veloxd run, not just the one expected
segments: 0 case — merging as-is would block every lane's PRs on two
DAEMON bugs at once, one of them unrelated to what this task set out to
catch. Required once DAEMON lands a fix for startMode "later" at minimum;
segments: 0 can stay red for a while by design, same as any other tracked
regression.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01SFeUKLbdHizrJjLBeK7ffz
|
||
|
samiandClaude Sonnet 5
|
203d4a662f |
proto: wire the conformance suite into ctest so CI actually runs it
Verified PKG's landed CI (.github/workflows/ci.yml, db7650b/8f45815) against this lane's actual tree, per the standing instruction that conformance as a required check is this lane's DoD to verify, not PKG's. It wasn't running: the `conformance` job's presence-check looks for tests/conformance/CMakeLists.txt or tests/conformance/package.json, and neither existed -- the job was silently short-circuiting to a green "skipped" on every PR, forever. The M0 exit gate was not gating anything. tests/conformance/CMakeLists.txt registers one ctest entry, labeled "conformance", that shells out to run.sh -- the exact command tests/conformance/README.md tells a human to run locally, so there is one definition of "the suite passed", not a CMake-flavoured near-duplicate of it. cpp/CMakeLists.txt's existing conformance_cpp test gets the same label, for a lane iterating on core/generated/ who wants the fast native-only path. Fixed a second landmine found while wiring this: the root CMakeLists.txt only find_package(nlohmann_json)'s when daemon/CMakeLists.txt exists, since daemon is its real consumer -- but daemon hasn't landed yet, so add_subdirectory(tests/conformance) would have failed to configure the moment this file existed, on every machine, until daemon merges. Fixed inside tests/conformance/cpp/CMakeLists.txt with an if(NOT TARGET) guard rather than widening the root file's condition, which is PKG's to change. run.sh now installs its own Python deps (jsonschema, referencing) on demand: they aren't in tools/bootstrap.sh's apt list -- that's PKG's script, these are this suite's own dependency -- so a bare CI image would otherwise fail check_contract.py with an ImportError before this suite even started. Verified end to end: `cmake --preset dev && ctest --test-dir build/dev -R '^conformance$'` passes in 23.8s, exercising the exact command and label the CI job uses. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_012fgjnqFCS5h5L7gZTZo3rV |
||
|
samiandClaude Opus 5
|
53421d6cb8 |
proto: freeze the wire contract at 1.0.0
Schemas for the whole v1 surface: 38 methods, 9 events, 25 named types and the
JSON-RPC envelope, with x-privileged / x-transports / x-deadlineMs / x-errors
annotations that both generators emit as data rather than prose.
Four generators over one IR (contracts/codegen/schema_ir.py), so the C++ structs,
the TypeScript types and the OpenRPC document cannot disagree about what the
contract says:
gen_cpp.py -> core/generated/velox_proto.{hpp,cpp}
gen_ts.py -> extension/src/shared/protocol/
gen_openrpc.py -> contracts/openrpc.json
gen_cpp_conformance.py -> tests/conformance/cpp/fixture_dispatcher.hpp
Inbound parsing never throws: parse<T>() returns std::expected<T, ParseError> and
nlohmann's throwing ADL from_json is deliberately not emitted. Schema constraints
(minimum, maxLength, pattern, ...) become real runtime checks in both languages —
the daemon does not trust the extension and the extension does not trust the
daemon.
59 golden fixtures: a success case per method, 12 error cases, 9 events. Replayed
by tests/conformance/ against both the generated C++ and a live server over both
transports. tools/mockd serves the same fixtures with unhappy-path flags so the
GUI and EXT lanes never wait for veloxd.
run.sh also proves capture.offer fails open: with a daemon answering slower than
750 ms the client gives up and lets Firefox take the download.
core/generated/ is libveloxproto, a separate target from libveloxcore, which
still never sees JSON — see docs/adr/0009.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012fgjnqFCS5h5L7gZTZo3rV
|