fix(651): review round 8 — put the selection-id predicate at the boundary, not the site

Round 7 added the int32 check inside `isSearchPickerOption` — the place the defect was found
— which left every other door into editor state open. A malformed successful response
carrying `1.5` or `2147483648` still entered `draft` through list-backed options and through
the selection restored from the detail read, keeping Save enabled and sending a value the API
cannot bind, while the identical value arriving via SearchPicker was correctly rejected.

The predicate now lives once, in `web/src/api/selectionId.ts`, and sits on every path by
which an id from the wire becomes editor state. The class crosses all three screens, not just
the one the finding named, so all three are covered:
- RerunCollectionsScreen: `toPickerOptions` (3 list branches) + `draftFromRerun`
- PlaylistsScreen: `toPickerOptions` (3 list branches) + `draftFromItem` (4 id fields)
- FillerPresetsScreen: `draftFromPreset` (5 id fields) + the collection-family browse options
- pickers.tsx: `isSearchPickerOption` now delegates rather than carrying its own copy

An unbindable id is treated as ABSENT, never coerced — rounding 1.5 to 1 would submit a
DIFFERENT record — so it surfaces as "no selection" with Save disabled and a visible reason;
an option that cannot be selected safely is dropped rather than rendered. Five regressions
assert zero writes are reachable via each previously-unguarded path.

Also corrects two of my own test descriptions, per the review: the padded-ETag test is a
regression guard rather than a round-7 defect demonstration (Headers strips outer whitespace
before the app sees it), and the late-settlement test guards the abort/race COMPOSITION —
what it actually fails is an abort-only implementation whose fetch ignores its signal, which
is why its stub ignores `init.signal`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-07-27 04:25:14 +02:00
co-authored by Claude Opus 5
parent e605e4006a
commit 39c4e8df0a
10 changed files with 159 additions and 34 deletions
+1
View File
@@ -17,6 +17,7 @@ export * from './imageFolders';
export * from './languages';
export * from './libraries';
export * from './libraryBrowse';
export * from './selectionId';
export * from './logs';
export * from './maintenance';
export * from './mediaDetail';
+24
View File
@@ -0,0 +1,24 @@
// A *selection id* is any id an editor stores and later submits as an entity reference —
// `selectedId` on a rerun collection, `collectionId`/`mediaItemId`/… on a playlist item or filler
// preset. The API binds every one of them as a 32-bit integer, so a value outside that domain is
// not merely odd: it renders and commits happily and then fails on write.
//
// This lives in ONE place on purpose. #651 round 7 added the check inside the search picker's
// option validator — the site where the defect was found — leaving list-backed options and the
// selection restored from a detail read unguarded, so the identical malformed value entered editor
// state through a different door (round 8). The predicate belongs at the BOUNDARY the class
// crosses: every path by which an id from the wire becomes editor state.
export const INT32_MIN = -2_147_483_648;
export const INT32_MAX = 2_147_483_647;
export function isSelectionId(value: unknown): value is number {
return typeof value === 'number' && Number.isInteger(value) && value >= INT32_MIN && value <= INT32_MAX;
}
// Normalize an id arriving from the wire into editor state. Anything the API cannot bind is treated
// as ABSENT rather than carried: the editor then shows "no selection" with Save disabled — visible
// and honest — instead of a value that looks selected and fails on submit. Never silently coerce
// (rounding 1.5 to 1 would submit a *different* record).
export function selectionIdOrNull(value: unknown): null | number {
return isSelectionId(value) ? value : null;
}