Files
jellytau/docs/specs/boundary-tripwire-hardening.md
dtourolle 75bae2556c docs(specs): design-principles audit — five remediation specs
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.
2026-07-30 10:29:54 +02:00

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 Record value, 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.svelteadd to the allowlist with the reason from the layer table (grid styling over a self-declared itemType).
  • DownloadedBrowse.svelteadd to the allowlist with a TODO naming the preferred fix (backend isContainer). An allowlist entry that records a known-borderline decision is honest; silently broadening the pattern to miss it would not be.
  • searchScope.tsnot 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 switch or 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 DownloadedBrowse to a backend isContainer flag. It touches MediaItem, 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.ts reverted to its leaking form, bun run check:boundary fails and names src/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:boundary passes.
  • A newly introduced const X = ["Movie", "Series"] in any non-test src/ file fails the check (regression test for the const-indirection evasion).
  • itemType: "Movie" and item.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 check and bun run test pass.
  • bun run test:all passes.

Testing

The script is bash and has no test harness. Verify by construction — each is a temporary edit, run, revert:

  1. Reintroduce the SCOPE_ITEM_TYPES const → must fail.
  2. Add const T = ["Movie","Series"] to a scratch .svelte file → must fail.
  3. Add the same to a .test.ts file → must pass (exclusion holds).
  4. Add itemType: "Movie"must pass.
  5. 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 an includeItemTypes: 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 diff before "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 master if 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 TYPES list 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.