First step of ADR-008. internal/transcode answers one question: can this host
hardware-transcode, and with which encoder.
The rule the package exists to enforce is that a device node is not proof. A
node can exist while the driver cannot encode, and that produces the worst
outcome ADR-008 describes: a transcode that starts, looks like it works, and
then cannot keep up. So every candidate runs a real two-frame probe encode and
is dropped unless that encode exits cleanly.
This host is a perfect illustration and the reason the rule is not theoretical:
it has /dev/dri/renderD128 AND Nvidia nodes. The render node exists, but VAAPI
fails to initialise against it ("Failed to initialise VAAPI connection"). A
detection that trusted the node would have picked VAAPI and been wrong. The
probe rejected it and selected NVENC, which was verified independently with
ffmpeg outside this code.
The probe is injected, so every branch is testable with no hardware present,
including the important one: device present, probe fails. Verified red-green -
ignoring the probe result fails TestDetectProbeFails with "got vaapi, want no
capability".
One real bug found while proving it: the probe used testsrc=size=64x48, below
NVENC's minimum frame size, so a host with working NVENC would have reported no
capability at all. Raised to 320x240, still two frames.
Also added internal/transcode to the arch layer map as a layer-1 primitive
beside probe and engine, which the arch rule caught immediately.
Not wired into the engine registry or playback yet - that is the next step.
internal/discovery registers three service types on the listen port:
_vellum._tcp for native clients, and _jellyfin._tcp / _subsonic._tcp
because those ecosystems already have clients that browse the LAN. That
last part is the point: Infuse, Jellyfin Media Player and Symfonium can
find Vellum with no client-side work, because they are already looking for
those names.
TXT records carry api, ver, proto, types, tls and (when set) pub, so a
client that resolves one can tell what it found before connecting.
Definitions is a pure function of config and Start only registers what it
returns, so what is advertised is testable without a socket.
Two behaviours worth knowing:
- VELLUM_DISCOVERY=off stops advertising. It accepts off/no/false/0;
strconv.ParseBool would have silently ignored "off".
- A loopback-only listen address advertises nothing. That is the normal
shape behind a reverse proxy, and advertising it would put a port in the
LAN browse list that refuses every client that dials it. Verified
red-green: defeating the guard fails TestDefinitionsSkipsLoopbackListen.
zeroconf is the only new dependency. It logs to the stdlib logger, so a
host with no multicast-capable interface prints to stderr outside slog.
Not verified: real mDNS on a real LAN. There is no multicast here, so the
wire behaviour of zeroconf is untested - only the definitions it is handed.
Phase 3 built the seam; nothing stopped it being undone one convenient import at
a time. Two rules, both proven able to fail.
TestDatabaseSQLImportersAreDeclared freezes the blast radius of database/sql.
Twenty packages import it today and that set is now explicit. A new package that
needs its own SQL must be added deliberately, with a reason. The rule also fails
in the other direction — if a listed package stops importing database/sql, the
list must shrink, so it cannot rot into a meaningless allowlist.
TestThreadedConsumersStayOffSQL is the concrete regression guard: internal/
playback was moved onto catalog.Store, and this fails if it reaches for *sql.DB
again.
Both checks read non-test files only, so a package that needs database/sql
purely to build a test database is not forced onto the list.
Proven by breaking each on purpose:
- dropping internal/media from the list -> "imports database/sql but is not
in sqlImporters"
- adding internal/natsort (no SQL) -> "is listed ... but no longer
imports database/sql"
- re-adding database/sql to playback -> "undoes the seam"
All restored afterwards and the files verified unchanged.
Seven arch rules now, each able to fail. gofmt clean, build 0, vet 0, 25
packages pass.
Four extractions, each chosen because the two halves have different callers,
different failure modes or different testing needs. Not a line-count exercise.
internal/home out of internal/media
- media serves item RESOURCES (detail, file, cover, progress); home computes
AGGREGATES (the five shelves). The 2328-line package was doing both.
- media keeps POST /items/{id}/view because it is item-scoped and must stay in
the package that authorizes the item; it imports home.RecordView for that one
thing. home does not import media.
- Verified live: all five shelves non-empty, and two consecutive /home calls
return different explore ids, which is the non-determinism the Explore rewrite
exists for.
internal/scanner out of internal/library
- The scanner walks the filesystem and reconciles; library CRUD is an admin HTTP
surface. Different callers (a job vs a request), different failure modes.
- The registry deliberately STAYED in internal/library: three of its four callers
are not the scanner (modules register types, handlers serve the list, metadata
reads the group mode, subsonic calls CleanTitle). It is a property of the kind,
not of the scan. The one genuinely scanner-owned helper became
library.ExtensionsFor.
- Two tests moved with the code rather than being rewritten; they now exercise
the boundary by calling library.ListMissing across it.
- Verified live: first scan updates, rescan reports unchanged=N, so change
detection still short-circuits.
internal/providers out of internal/metadata
- Providers talk to third-party APIs; the enrich job owns the matching policy and
the database writes. The split's whole point is that a provider is testable
WITHOUT a database, so a DB-free test was added and OpenLibrary gained a
BaseURL override to make it possible.
- providers imports zero internal packages — a true leaf.
- The matching policy (searchTitle, artistHint, pickBest) is untouched. It exists
because real false matches were found.
dto packages out of internal/jellyfin and internal/subsonic
- The wire types and their mapping are now separate from the handler logic, so
the contract with clients we do not control is reviewable on its own.
- Both dto packages are PURE: zero references to sql.DB or net/http, verified by
grep. That is what makes them reviewable.
- No field name, tag or error code changed.
internal/arch did its job: it failed the build twice during this phase, once for
each new package that was not yet placed in a layer. That is the rule working,
not an obstacle.
Verified: gofmt clean, build exit 0, vet exit 0, 23 packages pass, svelte-check
0/0, and end to end against a running server — home shelves, a queued scan
reporting unchanged, Jellyfin /Items, and Subsonic ping.
Process note: one agent (dto) stopped mid-task and left the tree red — the dto
packages unlayered and its test still referencing constants that had moved. The
arch test caught it immediately. I finished it by hand: exported constant
references in the test, and both dto packages placed at layer 3.
Still open from Phase 1: internal/jellyfin has no test files, catalog is
item-centric while Subsonic is file-first, and subsonic loadAlbum now scopes to
the item's own library.
Motivation, from the evidence rather than from taste. Five packages each
implemented "load an item, then check the caller may read it": media, comics,
games, jellyfin (with its own dbItem type and its own SQL), and subsonic. Five
copies of one authorization rule means four can drift, and the failure mode is
not a crash — it is silently serving somebody a library they were never granted.
One implementation is one thing to review.
internal/catalog now owns item and file access, and the access check.
LoadUnchecked(ctx, db, id) -- no access check; jobs and tests only
LoadFor(ctx, db, u, id) -- load AND enforce the grant, in one call
Files / FilesFor / FileByID / FirstFile / FileCount
The unchecked variant is deliberately named so that reaching for it by accident
is hard: it is the dangerous call, so it should not have the friendlier name.
This came from the interface-design rule that a good interface makes the right
thing easy and the wrong thing hard.
The security property is tested, not asserted: a forbidden item and an absent
item return byte-identical errors, so no handler can leak the existence of
another user's library through a 403. That is now enforced in one place.
Migrated all five surfaces onto it. media, comics, games and ebooks went from
four duplicate loaders to zero. jellyfin's dbItem and loadDBItem are gone.
subsonic's per-item authorization moved while its album/artist aggregate
projections stayed put, because grouping items into albums is a presentation
concern, not a data-access one. The remaining raw SQL everywhere is browse and
aggregate queries; no per-item load decides access any more.
internal/arch enforces the layering instead of documenting it. Five rules, each
proven able to fail by deliberately breaking it:
- no upward imports across six explicit layers
- internal/core may depend only on migrations
- internal/catalog must not know about HTTP, or it stops being reusable
- modules must not import each other
- every package must be placed in a layer, so the map cannot go stale
A test that only ever passes is decoration, so each rule was verified by
introducing the violation and confirming the failure.
Verified: gofmt clean, go build ./... exit 0, go vet exit 0, 20 packages pass,
svelte-check 0/0. End to end against a running server: the vellum API serves
item detail with codec facts, an unknown item 404s, Jellyfin AuthenticateByName
mints a token and /Items browses, and Subsonic ping returns status ok. The
subsonic migration was additionally A/B'd byte-identical against the previous
implementation across ~15 endpoints, and Jellyfin/Subsonic both still return the
same error for a forbidden item as for an absent one.
Known follow-ups, recorded rather than silently skipped:
- internal/jellyfin has no test files at all. Its wire contract is the most
client-visible surface in the tree and is now verified only by hand.
- catalog is item-centric, but Subsonic addresses songs file-first (mf-<id>),
so it resolves file to item through its own projection. A catalog lookup
that takes a file id would close that.
- subsonic loadAlbum now scopes the projection to the item's own library
rather than all visible audio libraries. Equivalent by construction, since
an item's kind tracks its library's kind, but a hand-edited mismatch would
differ.
mithral