Audit of the principles in CLAUDE.md and docs/architecture/ against the actual code. Principles with a working automated check (poison-tolerant locking, Android source sync, one-directional playback state, graceful backend init, reachability-from-traffic) all held up. The two that drifted are exactly the two whose checks were broken or too narrow: - traceability-gate-repair: CI divided by hardcoded denominators (UR/39, IR/24, DR/48, JA/3, total 114) while requirements.md had grown to 211, reporting 158% coverage — the 50% threshold was unreachable and the job could not fail. - req-coverage-script-removal: check-req-coverage.sh reports "1 requirement" and prints "all requirements have implementations". - scoped-search-boundary-implementation: the founding boundary incident was specced but never built; the leak is still live. - boundary-tripwire-hardening: check:boundary passes on that same leak — the pattern is anchored to the query site, so a named const evades it. - player-facade-enforcement: 52 direct commands.player* call sites outside the facade, and no automated check at all. Each spec follows SPEC-TEMPLATE.md with a filled-in Layer assignment table and is checked against SPEC-REVIEW-CHECKLIST.md.
10 KiB
Spec: Harden the frontend boundary tripwire
Status: Implemented Requirements: DR-094 UX spec: n/a — developer tooling. Supersedes / revises: revises the detection rule in scripts/check-frontend-boundary.sh; the boundary policy in scoped-search-boundary.md is unchanged.
Summary
bun run check:boundary passes on a tree that contains the exact leak it was
built to catch. It matches a multi-type array only when written inline at the
query site, so assigning the same array to a named const evades it entirely —
which is how searchScope.ts has kept a
category→item-type mapping through every green CI run. This spec broadens the
match to item-type array literals anywhere in src/, and resolves the handful
of legitimate hits that broadening surfaces.
Motivation
The current pattern is anchored to the includeItemTypes: key:
PATTERN='includeItemTypes:[[:space:]]*\[[^]]*,[^]]*\]'
The live leak is not written that way:
// src/lib/utils/searchScope.ts:29 — invisible to the tripwire
const SCOPE_ITEM_TYPES = { music: ["MusicAlbum", "MusicArtist", "Audio", "Playlist"], … };
The taxonomy and the query are one indirection apart, and the grep only sees the
query. The script's own header is admirably honest that it is "a TRIPWIRE, NOT A
PROOF" — but the gap here is not a subtle judgment call it was designed to
defer to human review. It is the crudest form of the violation, one const
away from the shape it does match, in the very file the founding incident was
written about.
Broadening the pattern to any item-type array literal finds it, with a manageable number of other hits (measured, not estimated):
| Site | Verdict |
|---|---|
searchScope.ts:30,32 |
🔴 The leak. Removed by scoped-search-boundary-implementation.md. |
PersonDetailView.svelte:30 |
Already allowlisted, with a recorded reason. |
DownloadedBrowse.svelte:95 |
Borderline — ["MusicAlbum","Series","Season","BoxSet"].includes(item.type) as an "is this a container?" predicate. |
GenericMediaListPage.svelte:298 |
Borderline — ["MusicAlbum","MusicArtist","Audio","Playlist"].includes(config.itemType) as a music-styling predicate. |
6 hits in *.test.ts |
Excluded; tests legitimately name types. |
Four non-test sites total. This is a tractable change, not a boil-the-ocean one.
Layer assignment
Tooling only — no application logic, nothing crosses IPC. The two borderline application sites do get a layer decision, below.
| Logic / responsibility | Layer | Why it belongs there |
|---|---|---|
Detecting item-type array literals in src/ |
Build tooling (scripts/) |
Static analysis of repo source; belongs beside the existing check. |
"Is this item a container?" (DownloadedBrowse) |
Rust (recommended) | Containers-vs-leaves is Jellyfin structure, and the set grows when Jellyfin adds a container type — the litmus test's "yes". Prefer a MediaItem.isContainer boolean from the backend over a type-set predicate in a component. |
"Is this music content?" (GenericMediaListPage) |
Frontend, allowlisted | Selects a grid style. It reads config.itemType, a value the page already declares about itself, and changes only if the UI is redesigned — the litmus test's "no". Single-type presentation is explicitly not the target of the rule. |
DownloadedBrowse defaults to Rust per the checklist's borderline rule; see
Out of scope for why the migration itself is deferred rather than bundled.
Design
1. Broaden the pattern
Replace the key-anchored pattern with one matching an array literal of two or more known Jellyfin item types, wherever it appears:
# Two or more adjacent item-type string literals inside a bracket.
TYPES='Movie|Series|Episode|Audio|MusicAlbum|MusicArtist|MusicVideo|Season|BoxSet|Playlist|Book|AudioBook|Video|Person|Folder|CollectionFolder|TvChannel|LiveTvChannel'
PATTERN="\[[[:space:]]*\"($TYPES)\"[[:space:]]*,[[:space:]]*\"($TYPES)\""
Properties worth stating, because each is a deliberate trade:
- Not anchored to any key, so a named const, a function return, a
Recordvalue, or an inline query all match equally. - Requires two adjacent type literals, preserving the existing and correct
carve-out that single-type presentation (
itemType: "Movie") is legitimate. - Requires string literals, so
item.type === "Audio"(display inspection) still does not match. - Explicit type list, not
[A-Z][a-z]+, so arbitrary string arrays (["High","Low"],["Songs","Albums"]) do not produce noise.
Keep grep -rInE, the *.test.* exclusion, and the allowlist mechanism as they
are — all three work.
2. Resolve the surfaced sites
PersonDetailView.svelte— already allowlisted; entry unchanged.GenericMediaListPage.svelte— add to the allowlist with the reason from the layer table (grid styling over a self-declareditemType).DownloadedBrowse.svelte— add to the allowlist with aTODOnaming the preferred fix (backendisContainer). An allowlist entry that records a known-borderline decision is honest; silently broadening the pattern to miss it would not be.searchScope.ts— not allowlisted. It is the leak, and scoped-search-boundary-implementation.md deletes it.
3. 🔴 Sequencing
This spec must land after Stage 1 of
scoped-search-boundary-implementation.md.
Hardening the tripwire first turns master red on a violation with no fix
available, and the only ways out are reverting the hardening or allowlisting the
leak — the second of which is exactly how a boundary rule dies.
4. Keep the allowlist honest
The script already warns that a growing allowlist means the boundary is eroding. This change takes it from 1 entry to 3, which is close to that line. Add a hard cap so drift is caught mechanically rather than by whoever notices:
MAX_ALLOWLIST=4
if [ "${#ALLOWLIST[@]}" -gt "$MAX_ALLOWLIST" ]; then
echo "❌ Allowlist has ${#ALLOWLIST[@]} entries (max $MAX_ALLOWLIST)."
echo " Push taxonomy into Rust instead of appending here."
exit 1
fi
The cap is deliberately just above the current count: the next exception forces a conversation instead of a one-line append.
5. Restate the limits
The header's "tripwire, not a proof" caveat stays and gets sharper. The broadened pattern still cannot see:
- a type set built at run time (
[...musicTypes, "Playlist"]), - types split across variables (
const A = "Audio"; [A, B]), - taxonomy expressed as a
switchor chained||rather than an array.
The spec-review checklist remains the real gate. This raises the floor; it does not close the class.
Out of scope
- Migrating
DownloadedBrowseto a backendisContainerflag. It touchesMediaItem,bindings.ts, and the offline path — its own spec. Allowlisted with a TODO here so it is recorded, not forgotten. - The scoped-search fix itself — scoped-search-boundary-implementation.md.
- Detecting the run-time-construction cases listed above.
- Extending the check to Rust or Kotlin (the rule is about
src/). - Changing the boundary policy in CLAUDE.md.
Acceptance criteria
- With
searchScope.tsreverted to its leaking form,bun run check:boundaryfails and namessrc/lib/utils/searchScope.ts. This is the criterion that proves the fix — verify it explicitly before landing. - On the post-fix tree,
bun run check:boundarypasses. - A newly introduced
const X = ["Movie", "Series"]in any non-testsrc/file fails the check (regression test for the const-indirection evasion). itemType: "Movie"anditem.type === "Audio"do not trip the check.["High", "Low"]and other non-item-type arrays do not trip it.- Test files are still excluded (the 6 known test hits stay silent).
- The allowlist has exactly 3 entries, each with a written reason; a 5th
entry fails the check via
MAX_ALLOWLIST. - The script header still states it is a tripwire, not a proof, and names the evasions it cannot see.
bun run checkandbun run testpass.bun run test:allpasses.
Testing
The script is bash and has no test harness. Verify by construction — each is a temporary edit, run, revert:
- Reintroduce the
SCOPE_ITEM_TYPESconst → must fail. - Add
const T = ["Movie","Series"]to a scratch.sveltefile → must fail. - Add the same to a
.test.tsfile → must pass (exclusion holds). - Add
itemType: "Movie"→ must pass. - Add a 5th allowlist entry → must fail on the cap.
Record the five results in the PR description. A grep-based gate that has never been observed failing is indistinguishable from one that cannot fail — which is the precise condition this whole spec exists to correct.
TRACES
Allocate in requirements.md:
- DR-094 — "Frontend boundary tripwire detects Jellyfin item-type array
literals anywhere in
src/(not only inline at anincludeItemTypes:query site), so a category→type mapping cannot evade the check via a named const; allowlist is capped to force taxonomy into Rust rather than accumulating exceptions." Category: Tooling. Status: Done on merge.
Shell scripts carry no TRACES: comment convention in this repo; reference
DR-094 in the script header comment instead.
Notes for the implementer
- A parallel Claude session may be active in this repo —
git diffbefore "repairing" unexpected changes (CLAUDE.md §Gotchas). - Land after Stage 1 of the scoped-search fix — see §3. This is the one
ordering constraint that will break
masterif ignored. - Test the regex against the current tree before committing:
grep -rInE "$PATTERN" src/ | grep -v '\.test\.'should return exactly the four sites in the Motivation table. - The
TYPESlist will need occasional extension as Jellyfin adds types. That is acceptable for a tripwire — an unlisted type produces a false negative, never a false positive, so the check degrades safely.