feat(navigation): enhance episode navigation logic and improve error handling

- Update GetNextEpisode and GetPreviousEpisode to validate against actual episode list.
- Refactor ListAllEpisodes to return real episode numbers instead of fabricated values.
- Introduce regression tests for SuperFlix and AllAnime source handling.
- Implement error handling improvements in player menu actions to prevent unintended auto-advances.

Author: alvarorichard

internal/playback/allanime_navigation.go

@@ -3,6 +3,7 @@ package playback
 
 import (
 	"fmt"
+	"slices"
 	"strconv"
 	"strings"
 	"sync"
@@ -61,13 +62,17 @@ func (nav *AllAnimeNavigator) GetNextEpisode(currentEpisode string) (string, err
 		return "", fmt.Errorf("invalid current episode number: %w", err)
 	}
 
+	// Validate against the actual episode list instead of assuming a
+	// contiguous 1..N numbering: lists starting at "0" (or with gaps) made
+	// the old len()-based check accept phantom episodes past the last one.
 	next := current + 1
-	if next > len(nav.episodes) {
+	target := strconv.Itoa(next)
+	if !slices.Contains(nav.episodes, target) {
 		return "", fmt.Errorf("no next episode available (current: %d, total: %d)", current, len(nav.episodes))
 	}
 
 	util.Debugf("Navigating to next episode from=%d to=%d", current, next)
-	return strconv.Itoa(next), nil
+	return target, nil
 }
 
 // GetPreviousEpisode returns the previous episode before the current one
@@ -77,13 +82,16 @@ func (nav *AllAnimeNavigator) GetPreviousEpisode(currentEpisode string) (string,
 		return "", fmt.Errorf("invalid current episode number: %w", err)
 	}
 
+	// Same list-membership check as GetNextEpisode: allows "0" when the
+	// source really has an episode 0 and rejects anything not in the list.
 	prev := current - 1
-	if prev < 1 {
+	target := strconv.Itoa(prev)
+	if !slices.Contains(nav.episodes, target) {
 		return "", fmt.Errorf("no previous episode available (current: %d)", current)
 	}
 
 	util.Debugf("Navigating to previous episode from=%d to=%d", current, prev)
-	return strconv.Itoa(prev), nil
+	return target, nil
 }
 
 // GetTotalEpisodes returns the total number of episodes
@@ -94,30 +102,51 @@ func (nav *AllAnimeNavigator) GetTotalEpisodes() int {
 // ListAllEpisodes returns all available episode numbers
 func (nav *AllAnimeNavigator) ListAllEpisodes() []string {
 	result := make([]string, len(nav.episodes))
-	for i := range nav.episodes {
-		result[i] = strconv.Itoa(i + 1)
-	}
+	copy(result, nav.episodes)
 	return result
 }
 
 // Helper function to check if anime is from AllAnime source
 func isAllAnimeSource(anime *models.Anime) bool {
-	if anime.Source == "AllAnime" {
-		return true
+	// An explicit source is authoritative. The length heuristic below used to
+	// run even when Source was set, misrouting SuperFlix/SFlix (short numeric
+	// TMDB IDs as URLs) into AllAnime navigation.
+	if anime.Source != "" {
+		return anime.Source == "AllAnime"
 	}
 
 	if strings.Contains(anime.URL, "allanime") {
 		return true
 	}
 
-	// Check if URL is a short ID (AllAnime typically uses short IDs)
-	if len(anime.URL) < 30 &&
-		strings.ContainsAny(anime.URL, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789") &&
-		!strings.Contains(anime.URL, "http") {
-		return true
+	// AllAnime IDs are short opaque alphanumeric tokens containing at least
+	// one letter (purely numeric IDs belong to other sources).
+	return anime.URL != "" &&
+		len(anime.URL) < 30 &&
+		isAlphanumericID(anime.URL) &&
+		strings.ContainsAny(anime.URL, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz")
+}
+
+// isAlphanumericID reports whether s is non-empty and contains only ASCII
+// letters and digits.
+func isAlphanumericID(s string) bool {
+	for _, r := range s {
+		isAlnum := ('a' <= r && r <= 'z') || ('A' <= r && r <= 'Z') || ('0' <= r && r <= '9')
+		if !isAlnum {
+			return false
+		}
 	}
+	return s != ""
+}
 
-	return false
+// allAnimePathWords are URL path segments that precede the anime ID in full
+// AllAnime URLs and must never be mistaken for the ID itself.
+var allAnimePathWords = map[string]bool{
+	"anime":   true,
+	"bangumi": true,
+	"watch":   true,
+	"serie":   true,
+	"series":  true,
 }
 
 // Helper function to extract AllAnime ID from URL
@@ -127,12 +156,27 @@ func extractAllAnimeID(url string) string {
 		return url
 	}
 
-	// Extract ID from full AllAnime URLs if needed
+	// Extract the ID from full AllAnime URLs. The old scan returned the first
+	// "long alphanumeric" segment, which for any https URL was the scheme
+	// ("https:") — poisoning the navigator cache key and every episode fetch.
 	if strings.Contains(url, "allanime") {
-		parts := strings.SplitSeq(url, "/")
-		for part := range parts {
-			if len(part) > 5 && len(part) < 30 &&
-				strings.ContainsAny(part, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789") {
+		parts := strings.Split(url, "/")
+
+		// Prefer the segment right after a known path word (/anime/<id>/...)
+		for i, part := range parts {
+			if allAnimePathWords[part] && i+1 < len(parts) &&
+				len(parts[i+1]) < 30 && isAlphanumericID(parts[i+1]) {
+				return parts[i+1]
+			}
+		}
+
+		// Fallback: first plausible ID segment, skipping scheme and domain
+		for _, part := range parts {
+			if part == "" || strings.HasSuffix(part, ":") ||
+				strings.Contains(part, ".") || allAnimePathWords[part] {
+				continue
+			}
+			if len(part) > 5 && len(part) < 30 && isAlphanumericID(part) {
 				return part
 			}
 		}

internal/playback/allanime_navigation_test.go

@@ -20,6 +20,13 @@ func TestIsAllAnimeSource(t *testing.T) {
 		{"short id", &models.Anime{URL: "hHjXnUTda"}, true},
 		{"http url unrelated", &models.Anime{URL: "https://animefire.io/x"}, false},
 		{"empty", &models.Anime{}, false},
+		// Regression: explicit non-AllAnime source must win even when the URL
+		// is a short ID — SuperFlix/SFlix use numeric TMDB IDs as URLs and
+		// were misrouted into AllAnime navigation.
+		{"superflix numeric id", &models.Anime{Source: "SuperFlix", URL: "1234"}, false},
+		{"sflix short id", &models.Anime{Source: "SFlix", URL: "abc123"}, false},
+		{"other source with allanime-like id", &models.Anime{Source: "AnimeFire", URL: "hHjXnUTda"}, false},
+		{"numeric-only id without source", &models.Anime{URL: "12345"}, false},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -37,7 +44,10 @@ func TestExtractAllAnimeID(t *testing.T) {
 		want string
 	}{
 		{"short id", "hHjXnUTda", "hHjXnUTda"},
-		{"allanime url returns first long alphanumeric segment", "https://allanime.to/anime/abc123XYZ/title", "https:"},
+		// Regression: the old scan returned "https:" (the scheme) for any full
+		// URL, poisoning the navigator cache key and every episode fetch.
+		{"allanime url extracts id segment", "https://allanime.to/anime/abc123XYZ/title", "abc123XYZ"},
+		{"allanime bangumi url extracts id segment", "https://allanime.to/bangumi/xyz789AB/some-title", "xyz789AB"},
 		{"non-allanime", "https://example.com/x", "https://example.com/x"},
 	}
 	for _, tt := range tests {
@@ -86,9 +96,38 @@ func TestAllAnimeNavigator_GetTotalEpisodes(t *testing.T) {
 
 func TestAllAnimeNavigator_ListAllEpisodes(t *testing.T) {
 	t.Parallel()
-	nav := &AllAnimeNavigator{episodes: []string{"a", "b", "c"}}
+	// Must return the real episode numbers from the source, not fabricated
+	// 1..N values that lie for lists starting at "0" or containing specials.
+	nav := &AllAnimeNavigator{episodes: []string{"0", "1", "5.5"}}
 	list := nav.ListAllEpisodes()
-	assert.Equal(t, []string{"1", "2", "3"}, list)
+	assert.Equal(t, []string{"0", "1", "5.5"}, list)
+}
+
+func TestAllAnimeNavigator_GetNextEpisode_ZeroBasedList(t *testing.T) {
+	t.Parallel()
+	// List starts at "0": 3 entries but last real episode is "2". The old
+	// len()-based check accepted phantom episode "3" (3 <= len(3)).
+	nav := &AllAnimeNavigator{animeID: "x", episodes: []string{"0", "1", "2"}}
+
+	next, err := nav.GetNextEpisode("1")
+	require.NoError(t, err)
+	assert.Equal(t, "2", next)
+
+	_, err = nav.GetNextEpisode("2")
+	assert.Error(t, err, "episode 3 does not exist in a 0-based list of 3 entries")
+}
+
+func TestAllAnimeNavigator_GetPreviousEpisode_ZeroBasedList(t *testing.T) {
+	t.Parallel()
+	nav := &AllAnimeNavigator{animeID: "x", episodes: []string{"0", "1", "2"}}
+
+	// Episode "0" exists in the list, so previous from "1" must reach it.
+	prev, err := nav.GetPreviousEpisode("1")
+	require.NoError(t, err)
+	assert.Equal(t, "0", prev)
+
+	_, err = nav.GetPreviousEpisode("0")
+	assert.Error(t, err)
 }
 
 func TestNewAllAnimeNavigator_RejectsNonAllAnime(t *testing.T) {

internal/playback/input.go

@@ -12,6 +12,12 @@ type menuItem struct {
 	Value string
 }
 
+// findMenuFunc is a package-level indirection over tui.Find so tests can
+// drive GetUserInput without opening a TUI.
+var findMenuFunc = func(items []menuItem, itemFunc func(i int) string, opts ...fuzzyfinder.Option) (int, error) {
+	return tui.Find(items, itemFunc, opts...)
+}
+
 // GetUserInput shows post-playback menu. Pass isMovie=true for movies to show
 // a simplified menu without episode navigation options.
 func GetUserInput(isMovie ...bool) string {
@@ -38,12 +44,15 @@ func GetUserInput(isMovie ...bool) string {
 		}
 	}
 
-	idx, err := tui.Find(items, func(i int) string {
+	idx, err := findMenuFunc(items, func(i int) string {
 		return items[i].Label
 	}, fuzzyfinder.WithPromptString("What would you like to do next? "))
 	if err != nil {
+		// A broken or aborted menu must not auto-advance: returning "n" here
+		// made HandleSeries/HandleMovie auto-play forever on non-TTY
+		// terminals and turned Esc into "next episode".
 		util.Errorf("Error showing menu: %v", err)
-		return "n" // Default to next episode on error
+		return "q"
 	}
 
 	return items[idx].Value

internal/playback/input_test.go

@@ -0,0 +1,68 @@
+package playback
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/ktr0731/go-fuzzyfinder"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// swapFindMenu replaces the menu finder seam for the duration of the test.
+// Tests using it mutate a package global, so they must not run in parallel.
+func swapFindMenu(t *testing.T, fn func(items []menuItem, itemFunc func(i int) string, opts ...fuzzyfinder.Option) (int, error)) {
+	t.Helper()
+	orig := findMenuFunc
+	findMenuFunc = fn
+	t.Cleanup(func() { findMenuFunc = orig })
+}
+
+func TestGetUserInput_MenuErrorDoesNotAutoAdvance(t *testing.T) {
+	// Regression: on menu failure (non-TTY, Esc, broken terminal) GetUserInput
+	// returned "n", which made HandleSeries/HandleMovie auto-play the next
+	// episode forever with zero user input. Failure must quit.
+	swapFindMenu(t, func(_ []menuItem, _ func(i int) string, _ ...fuzzyfinder.Option) (int, error) {
+		return 0, errors.New("failed to open TTY")
+	})
+	assert.Equal(t, "q", GetUserInput(), "menu failure must map to quit, never to next-episode")
+	assert.Equal(t, "q", GetUserInput(true), "movie menu failure must map to quit, never to replay")
+}
+
+func TestGetUserInput_SeriesMenuMapping(t *testing.T) {
+	wantLabels := []string{"Next episode", "Previous episode", "Select episode", "Change anime", "← Back", "Quit"}
+	wantValues := []string{"n", "p", "e", "c", "back", "q"}
+
+	for i, wantValue := range wantValues {
+		var gotLabels []string
+		swapFindMenu(t, func(items []menuItem, itemFunc func(i int) string, _ ...fuzzyfinder.Option) (int, error) {
+			gotLabels = nil
+			for j := range items {
+				gotLabels = append(gotLabels, itemFunc(j))
+			}
+			return i, nil
+		})
+		got := GetUserInput()
+		require.Equal(t, wantLabels, gotLabels)
+		assert.Equal(t, wantValue, got, "label %q must map to %q", wantLabels[i], wantValue)
+	}
+}
+
+func TestGetUserInput_MovieMenuMapping(t *testing.T) {
+	wantLabels := []string{"Replay movie", "Change movie", "← Back", "Quit"}
+	wantValues := []string{"n", "c", "back", "q"}
+
+	for i, wantValue := range wantValues {
+		var gotLabels []string
+		swapFindMenu(t, func(items []menuItem, itemFunc func(i int) string, _ ...fuzzyfinder.Option) (int, error) {
+			gotLabels = nil
+			for j := range items {
+				gotLabels = append(gotLabels, itemFunc(j))
+			}
+			return i, nil
+		})
+		got := GetUserInput(true)
+		require.Equal(t, wantLabels, gotLabels)
+		assert.Equal(t, wantValue, got, "label %q must map to %q", wantLabels[i], wantValue)
+	}
+}

internal/playback/navigation_test.go

@@ -126,6 +126,19 @@ func TestHandleUserNavigationEnhanced_AllAnime_UsesEnhancedHandler(t *testing.T)
 	assert.Equal(t, 2, n)
 }
 
+func TestHandleUserNavigationEnhanced_SuperFlixShortID_NotRoutedToAllAnime(t *testing.T) {
+	t.Parallel()
+	// Regression: SuperFlix/SFlix use short numeric TMDB IDs as URLs. The old
+	// isAllAnimeSource heuristic matched any short non-http URL, sending these
+	// sources into AllAnime navigation (wrong navigator + network fetch).
+	eps := []models.Episode{{URL: "u1", Number: "1", Num: 1}, {URL: "u2", Number: "2", Num: 2}}
+	anime := &models.Anime{Source: "SuperFlix", URL: "1234"}
+	url, num, n := handleUserNavigationEnhanced("n", eps, 1, 2, anime)
+	assert.Equal(t, "u2", url)
+	assert.Equal(t, "2", num)
+	assert.Equal(t, 2, n)
+}
+
 // --- handleAllAnimeNavigation ---
 
 func TestHandleAllAnimeNavigation_NoCurrentEpisode_Fallback(t *testing.T) {