fix(sources): never hand over to a leaving source; browser checks per connection

Local review round 4 (Codex + Greptile):

- Removing the active source handed over to its neighbour even when that
  neighbour's own delete was still running ("Removing…"), activating a
  source about to disappear. replacementAfterRemoval (playlist shared
  util) picks the nearest row that is not leaving, on the Sources page and
  in the switcher; SourceRemovalService also refuses a leaving replacement.
- In the browser build a "Check again" for an account the user then
  edited could finish last and overwrite the new account's status under
  the same playlist ID. Portal statuses are now keyed by playlist and
  connection fingerprint, so the old check lands under the old key.
- The switcher scrolled the highlighted option into view through a
  selector built from the playlist ID, which a restored ID with "." or
  ":" broke (or made throw). It now finds the row by its exact DOM ID.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.spec.ts

@@ -194,6 +194,16 @@ describe('PlaylistSwitcherPanelComponent', () => {
         expect(closed).toHaveBeenCalled();
     });
 
+    it('moves through options whose IDs a selector would misread', () => {
+        const restored = { ...ALPHA, _id: 'odd:id.one', title: 'Aardvark' };
+        playlists.set([restored, BRAVO]);
+        fixture.detectChanges();
+        expect(() => press('ArrowDown')).not.toThrow();
+        expect(input().getAttribute('aria-activedescendant')).toBe(
+            'playlist-switcher-all-odd:id.one'
+        );
+    });
+
     it('filters by name or host as you type and highlights the first match', () => {
         type('charlie.example');
         expect(optionIds()).toEqual(['playlist-switcher-all-charlie']);

libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.ts

@@ -27,6 +27,7 @@ import {
     orderFailingFirst,
     PlaylistContextFacade,
     PlaylistRecentHistoryService,
+    replacementAfterRemoval,
     SourceBucketOf,
     sourceSortName,
     SourcesTypeFilter,
@@ -280,22 +281,21 @@ export class PlaylistSwitcherPanelComponent {
      * from Undo, which then takes the focus and returns it to the trigger.
      */
     remove(playlist: PlaylistMeta): void {
-        const all = this.groups().all.map((option) => option.playlist);
-        const index = all.findIndex((item) => item._id === playlist._id);
+        const next = replacementAfterRemoval(
+            this.groups().all.map((option) => option.playlist),
+            playlist._id,
+            (row) => this.removal.progress.isDeleting(row._id)
+        );
         this.close();
         // Closing put the focus back on the trigger.
         const returnFocusTo = this.document.activeElement;
-        this.removal.remove(
-            playlist,
-            all[index + 1] ?? all[index - 1] ?? null,
-            {
-                returnFocusTo:
-                    returnFocusTo instanceof HTMLElement &&
-                    returnFocusTo !== this.document.body
-                        ? returnFocusTo
-                        : null,
-            }
-        );
+        this.removal.remove(playlist, next, {
+            returnFocusTo:
+                returnFocusTo instanceof HTMLElement &&
+                returnFocusTo !== this.document.body
+                    ? returnFocusTo
+                    : null,
+        });
     }
 
     /** A row menu gives the focus back to the search, unless closing. */
@@ -337,7 +337,13 @@ export class PlaylistSwitcherPanelComponent {
     private scrollHighlightIntoView(): void {
         const id = this.activeOptionId();
         const list = this.list().nativeElement;
-        const option = id ? list.querySelector<HTMLElement>(`#${id}`) : null;
+        // By exact ID, not a selector: a restored playlist ID may hold "."
+        // or ":", which a selector would misread.
+        const option = id
+            ? this.rowElements()
+                  .map((row) => row.nativeElement as HTMLElement)
+                  .find((element) => element.id === id)
+            : undefined;
         if (!option) {
             return;
         }

libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.spec.ts

@@ -206,6 +206,14 @@ describe('RecentPlaylistsComponent', () => {
         expect(ids()).toEqual(['file', 'm3u', 'xtream', 'url']);
     });
 
+    it('hands over past a source whose delete is still running', () => {
+        (
+            removal.progress as { isDeleting: (id: string) => boolean }
+        ).isDeleting = (id) => id === 'xtream';
+        component.remove(M3U);
+        expect(removal.remove).toHaveBeenCalledWith(M3U, FILE);
+    });
+
     it('applies the type and status filters together', () => {
         viewState.setType('m3u');
         expect(ids()).toEqual(['m3u', 'file']);

libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.ts

@@ -31,6 +31,7 @@ import {
     filterSources,
     orderFailingFirst,
     PlaylistContextFacade,
+    replacementAfterRemoval,
     SourceBucketOf,
     sourceSortName,
     SourcesViewStateService,
@@ -201,9 +202,11 @@ export class RecentPlaylistsComponent {
         if (this.refresh.isRefreshing(playlist._id)) {
             return;
         }
-        const rows = this.visiblePlaylists();
-        const index = rows.findIndex((item) => item._id === playlist._id);
-        const next = rows[index + 1] ?? rows[index - 1] ?? null;
+        const next = replacementAfterRemoval(
+            this.visiblePlaylists(),
+            playlist._id,
+            (row) => this.removal.progress.isDeleting(row._id)
+        );
         this.removal.remove(playlist, next);
     }
 

libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.spec.ts

@@ -209,6 +209,17 @@ describe('SourceRemovalService', () => {
         expect(context.activatePlaylist).toHaveBeenCalledWith('a');
     });
 
+    it('does not hand over to a source that is being deleted', () => {
+        resolvedPlaylistId.set('a');
+        service.progress.track('b', {
+            operation: 'delete-playlist',
+            status: 'started',
+        });
+        service.remove(playlist('a'), playlist('b'));
+        expect(context.selectPlaylist).not.toHaveBeenCalled();
+        expect(toasts[0].message).toBe('WORKSPACE.SOURCES.REMOVED');
+    });
+
     it('leaves a switch the user made after the delete alone on Undo', () => {
         resolvedPlaylistId.set('a');
         service.remove(playlist('a'), playlist('b'));

libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.ts

@@ -105,7 +105,14 @@ export class SourceRemovalService {
         const wasOnItsRoute =
             this.playlistContext.routePlaylistId() === playlist._id;
         const previousUrl = this.router.url;
-        const replacement = wasActive ? next : null;
+        // A source that is leaving too cannot take over.
+        const replacement =
+            wasActive &&
+            next &&
+            !this.isPending(next._id) &&
+            !this.progress.isDeleting(next._id)
+                ? next
+                : null;
         this.setPending(playlist._id, true);
 
         if (replacement) {

libs/playlist/shared/util/src/lib/source-display/source-list.spec.ts

@@ -6,6 +6,7 @@ import {
     filterSources,
     matchesSourceQuery,
     orderFailingFirst,
+    replacementAfterRemoval,
     SourceBucketOf,
 } from './source-list';
 import {
@@ -98,6 +99,26 @@ describe('source list filters', () => {
     });
 });
 
+describe('replacementAfterRemoval', () => {
+    const rows = ['a', 'b', 'c', 'd'].map((id) => ({ _id: id }));
+    const none = () => false;
+
+    it('hands over to the next row, or the previous one at the end', () => {
+        expect(replacementAfterRemoval(rows, 'b', none)?._id).toBe('c');
+        expect(replacementAfterRemoval(rows, 'd', none)?._id).toBe('c');
+        expect(replacementAfterRemoval([{ _id: 'a' }], 'a', none)).toBeNull();
+    });
+
+    it('skips rows that are leaving too', () => {
+        const deleting = (row: { _id: string }) =>
+            row._id === 'c' || row._id === 'd';
+        expect(replacementAfterRemoval(rows, 'b', deleting)?._id).toBe('a');
+        expect(
+            replacementAfterRemoval(rows, 'a', (row) => row._id !== 'a')
+        ).toBeNull();
+    });
+});
+
 describe('SourcesViewStateService', () => {
     const create = () => {
         TestBed.configureTestingModule({

libs/playlist/shared/util/src/lib/source-display/source-list.ts

@@ -122,3 +122,30 @@ export function orderFailingFirst(
     }
     return [...failing, ...rest];
 }
+
+/**
+ * The source that takes over when `removedId` leaves a list: the nearest
+ * row after it, else before it, skipping rows that are leaving too (a
+ * delete still running). Null when no other source can take over.
+ */
+export function replacementAfterRemoval<T extends { _id: string }>(
+    rows: readonly T[],
+    removedId: string,
+    leaving: (row: T) => boolean
+): T | null {
+    const index = rows.findIndex((row) => row._id === removedId);
+    if (index === -1) {
+        return null;
+    }
+    for (let next = index + 1; next < rows.length; next += 1) {
+        if (!leaving(rows[next])) {
+            return rows[next];
+        }
+    }
+    for (let previous = index - 1; previous >= 0; previous -= 1) {
+        if (!leaving(rows[previous])) {
+            return rows[previous];
+        }
+    }
+    return null;
+}

libs/portal/shared/data-access/src/lib/source-status.service.spec.ts

@@ -232,6 +232,26 @@ describe('SourceStatusService', () => {
             });
         });
 
+        it("keeps an edited account's status from the old account's slow check", async () => {
+            let finishOld!: (status: string) => void;
+            portal.checkPortalStatus
+                .mockReturnValueOnce(new Promise((r) => (finishOld = r)))
+                .mockResolvedValueOnce('active');
+            const service = create(false);
+            service.recheck(xtream);
+
+            const edited = { ...xtream, password: 'changed' } as PlaylistMeta;
+            service.ensureChecked(edited, new AbortController().signal);
+            await Promise.resolve();
+            await Promise.resolve();
+            expect(service.statusOf(edited)?.reachability).toBe('ok');
+
+            finishOld('inactive');
+            await Promise.resolve();
+            await Promise.resolve();
+            expect(service.statusOf(edited)?.reachability).toBe('ok');
+        });
+
         it('drops a result that lands after the list closed', async () => {
             const service = create(false);
             const controller = new AbortController();

libs/portal/shared/data-access/src/lib/source-status.service.ts

@@ -57,6 +57,11 @@ export class SourceStatusService {
     private readonly injector = inject(Injector);
     private readonly history = inject(SourceHealthHistoryService);
     private readonly portalStatus = inject(PortalStatusService);
+    /**
+     * Browser-build portal statuses by playlist and connection
+     * (`portalKey`): a slow check of an account the user has since edited
+     * lands under the old key and cannot overwrite the new account's.
+     */
     private readonly portalStatuses = signal<ReadonlyMap<string, PortalStatus>>(
         new Map()
     );
@@ -77,7 +82,7 @@ export class SourceStatusService {
             return null;
         }
         if (!this.runtime.supportsSourceHealth) {
-            const status = this.portalStatuses().get(playlist._id);
+            const status = this.portalStatuses().get(portalKey(playlist));
             return status
                 ? {
                       reachability: sourceReachabilityFromPortalStatus(status),
@@ -147,16 +152,17 @@ export class SourceStatusService {
         fresh: boolean,
         signal?: AbortSignal
     ): Promise<void> {
-        const { _id, serverUrl, username, password } = playlist;
+        const { serverUrl, username, password } = playlist;
+        const key = portalKey(playlist);
         const cached = fresh
             ? null
             : this.portalStatus.getCachedStatus(serverUrl, username, password);
         if (cached) {
-            this.setPortalStatus(_id, cached);
+            this.setPortalStatus(key, cached);
             return;
         }
-        if (fresh || !this.portalStatuses().has(_id)) {
-            this.setPortalStatus(_id, 'checking');
+        if (fresh || !this.portalStatuses().has(key)) {
+            this.setPortalStatus(key, 'checking');
         }
         let status: PortalStatus;
         try {
@@ -172,23 +178,28 @@ export class SourceStatusService {
         if (signal?.aborted) {
             // A slow portal must not land after the list has closed; drop a
             // placeholder so the next opening checks again.
-            if (this.portalStatuses().get(_id) === 'checking') {
-                this.setPortalStatus(_id, null);
+            if (this.portalStatuses().get(key) === 'checking') {
+                this.setPortalStatus(key, null);
             }
             return;
         }
-        this.setPortalStatus(_id, status);
+        this.setPortalStatus(key, status);
     }
 
-    private setPortalStatus(id: string, status: PortalStatus | null): void {
+    private setPortalStatus(key: string, status: PortalStatus | null): void {
         this.portalStatuses.update((current) => {
             const next = new Map(current);
             if (status) {
-                next.set(id, status);
+                next.set(key, status);
             } else {
-                next.delete(id);
+                next.delete(key);
             }
             return next;
         });
     }
 }
+
+/** A playlist's portal status key: its ID and its connection. */
+function portalKey(playlist: PlaylistMeta): string {
+    return `${playlist._id}\u0000${sourceHealthFingerprint(playlist)}`;
+}