fix: select most specific launcher instead of first match

pkg/helpers/paths.go

@@ -365,9 +365,24 @@ func getPathExt(path string) string {
 	return base[lastDot:]
 }
 
+// launcherSpecificity scores how specific a launcher's matching criteria are.
+// Higher scores indicate more specific matches which should take priority.
+func launcherSpecificity(l *platforms.Launcher) int {
+	score := 0
+	if len(l.Schemes) > 0 {
+		score += 1000
+	}
+	if len(l.Folders) > 0 {
+		score += 100
+	}
+	if l.SystemID != "" {
+		score += 10
+	}
+	return score
+}
+
 // FindLauncher takes a path and tries to find the best possible match for a
-// launcher, taking into account any allowlist restrictions. Returns the
-// launcher to be used.
+// launcher, taking into account specificity and allowlist restrictions.
 func FindLauncher(
 	cfg *config.Instance,
 	pl platforms.Platform,
@@ -380,8 +395,24 @@ func FindLauncher(
 		return platforms.Launcher{}, errors.New("no launcher found for: " + path)
 	}
 
-	// TODO: must be some better logic to picking this!
-	launcher := launchers[0]
+	best := 0
+	bestScore := launcherSpecificity(&launchers[0])
+	for i := 1; i < len(launchers); i++ {
+		score := launcherSpecificity(&launchers[i])
+		if score > bestScore {
+			best = i
+			bestScore = score
+		}
+	}
+
+	launcher := launchers[best]
+
+	log.Debug().
+		Str("path", path).
+		Str("launcher", launcher.ID).
+		Int("specificity", bestScore).
+		Int("candidates", len(launchers)).
+		Msg("selected launcher by specificity")
 
 	if launcher.AllowListOnly && !cfg.IsLauncherFileAllowed(path) {
 		return platforms.Launcher{}, errors.New("file not allowed: " + path)

pkg/helpers/paths_launcher_test.go

@@ -301,3 +301,267 @@ func TestGetSystemPaths_CustomLauncherAbsolutePath(t *testing.T) {
 	// Verify PathIsLauncher works for a file in this directory
 	assert.True(t, PathIsLauncher(cfg, mockPlatform, &launchers[0], filepath.Join(tmpDir, "game.iso")))
 }
+
+func TestLauncherSpecificity(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		launcher platforms.Launcher
+		want     int
+	}{
+		{
+			name:     "generic: no schemes, folders, or system",
+			launcher: platforms.Launcher{ID: "Generic", Extensions: []string{".sh"}},
+			want:     0,
+		},
+		{
+			name:     "system only",
+			launcher: platforms.Launcher{ID: "SNES", SystemID: "SNES", Extensions: []string{".sfc"}},
+			want:     10,
+		},
+		{
+			name:     "folders only",
+			launcher: platforms.Launcher{ID: "FolderOnly", Folders: []string{"roms"}},
+			want:     100,
+		},
+		{
+			name: "folders and system",
+			launcher: platforms.Launcher{
+				ID: "Ports", SystemID: "Ports",
+				Folders: []string{"ports"}, Extensions: []string{".sh"},
+			},
+			want: 110,
+		},
+		{
+			name:     "scheme only",
+			launcher: platforms.Launcher{ID: "SchemeOnly", Schemes: []string{"custom"}},
+			want:     1000,
+		},
+		{
+			name:     "scheme and system",
+			launcher: platforms.Launcher{ID: "Steam", SystemID: "Steam", Schemes: []string{"steam"}},
+			want:     1010,
+		},
+		{
+			name:     "all criteria",
+			launcher: platforms.Launcher{ID: "Full", SystemID: "X", Folders: []string{"x"}, Schemes: []string{"x"}},
+			want:     1110,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got := launcherSpecificity(&tt.launcher)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+func TestFindLauncher_SpecificOverGeneric(t *testing.T) {
+	mockPlatform := mocks.NewMockPlatform()
+	mockPlatform.On("Settings").Return(platforms.Settings{})
+	mockPlatform.On("RootDirs", mock.AnythingOfType("*config.Instance")).Return([]string{"/userdata/roms"})
+
+	genericLauncher := platforms.Launcher{
+		ID:         "Generic",
+		Extensions: []string{".sh"},
+	}
+	portsLauncher := platforms.Launcher{
+		ID:         "Ports",
+		SystemID:   "Ports",
+		Folders:    []string{"ports"},
+		Extensions: []string{".sh"},
+	}
+
+	mockPlatform.On("Launchers", mock.AnythingOfType("*config.Instance")).Return(
+		[]platforms.Launcher{genericLauncher, portsLauncher})
+
+	cfg := &config.Instance{}
+
+	originalCache := GlobalLauncherCache
+	testCache := &LauncherCache{}
+	testCache.Initialize(mockPlatform, cfg)
+	GlobalLauncherCache = testCache
+	defer func() { GlobalLauncherCache = originalCache }()
+
+	launcher, err := FindLauncher(cfg, mockPlatform, "/userdata/roms/ports/Stardew Valley.sh")
+	require.NoError(t, err)
+	assert.Equal(t, "Ports", launcher.ID, "specific Ports launcher should win over Generic")
+}
+
+func TestFindLauncher_SchemeHighestPriority(t *testing.T) {
+	mockPlatform := mocks.NewMockPlatform()
+	mockPlatform.On("Settings").Return(platforms.Settings{})
+	mockPlatform.On("RootDirs", mock.AnythingOfType("*config.Instance")).Return([]string{})
+
+	genericLauncher := platforms.Launcher{
+		ID:         "Generic",
+		Extensions: []string{".sh"},
+	}
+	steamLauncher := platforms.Launcher{
+		ID:       "Steam",
+		SystemID: "Steam",
+		Schemes:  []string{"steam"},
+	}
+
+	mockPlatform.On("Launchers", mock.AnythingOfType("*config.Instance")).Return(
+		[]platforms.Launcher{genericLauncher, steamLauncher})
+
+	cfg := &config.Instance{}
+
+	originalCache := GlobalLauncherCache
+	testCache := &LauncherCache{}
+	testCache.Initialize(mockPlatform, cfg)
+	GlobalLauncherCache = testCache
+	defer func() { GlobalLauncherCache = originalCache }()
+
+	launcher, err := FindLauncher(cfg, mockPlatform, "steam://rungameid/413150")
+	require.NoError(t, err)
+	assert.Equal(t, "Steam", launcher.ID, "scheme-based launcher should have highest priority")
+}
+
+func TestFindLauncher_FolderSystemOverSystemOnly(t *testing.T) {
+	mockPlatform := mocks.NewMockPlatform()
+	mockPlatform.On("Settings").Return(platforms.Settings{})
+	mockPlatform.On("RootDirs", mock.AnythingOfType("*config.Instance")).Return([]string{"/roms"})
+
+	systemOnly := platforms.Launcher{
+		ID:         "SystemOnly",
+		SystemID:   "NES",
+		Extensions: []string{".nes"},
+	}
+	folderAndSystem := platforms.Launcher{
+		ID:         "FolderAndSystem",
+		SystemID:   "NES",
+		Folders:    []string{"nes"},
+		Extensions: []string{".nes"},
+	}
+
+	mockPlatform.On("Launchers", mock.AnythingOfType("*config.Instance")).Return(
+		[]platforms.Launcher{systemOnly, folderAndSystem})
+
+	cfg := &config.Instance{}
+
+	originalCache := GlobalLauncherCache
+	testCache := &LauncherCache{}
+	testCache.Initialize(mockPlatform, cfg)
+	GlobalLauncherCache = testCache
+	defer func() { GlobalLauncherCache = originalCache }()
+
+	launcher, err := FindLauncher(cfg, mockPlatform, "/roms/nes/mario.nes")
+	require.NoError(t, err)
+	assert.Equal(t, "FolderAndSystem", launcher.ID, "folder+system launcher should beat system-only")
+}
+
+func TestFindLauncher_GenericAloneStillWorks(t *testing.T) {
+	mockPlatform := mocks.NewMockPlatform()
+	mockPlatform.On("Settings").Return(platforms.Settings{})
+	mockPlatform.On("RootDirs", mock.AnythingOfType("*config.Instance")).Return([]string{})
+
+	genericLauncher := platforms.Launcher{
+		ID:         "Generic",
+		Extensions: []string{".sh"},
+	}
+
+	mockPlatform.On("Launchers", mock.AnythingOfType("*config.Instance")).Return(
+		[]platforms.Launcher{genericLauncher})
+
+	cfg := &config.Instance{}
+
+	originalCache := GlobalLauncherCache
+	testCache := &LauncherCache{}
+	testCache.Initialize(mockPlatform, cfg)
+	GlobalLauncherCache = testCache
+	defer func() { GlobalLauncherCache = originalCache }()
+
+	launcher, err := FindLauncher(cfg, mockPlatform, "/some/path/script.sh")
+	require.NoError(t, err)
+	assert.Equal(t, "Generic", launcher.ID, "generic launcher should work when it's the only match")
+}
+
+func TestFindLauncher_NoMatch(t *testing.T) {
+	mockPlatform := mocks.NewMockPlatform()
+	mockPlatform.On("Settings").Return(platforms.Settings{})
+	mockPlatform.On("RootDirs", mock.AnythingOfType("*config.Instance")).Return([]string{})
+
+	mockPlatform.On("Launchers", mock.AnythingOfType("*config.Instance")).Return(
+		[]platforms.Launcher{
+			{ID: "NES", SystemID: "NES", Folders: []string{"nes"}, Extensions: []string{".nes"}},
+		})
+
+	cfg := &config.Instance{}
+
+	originalCache := GlobalLauncherCache
+	testCache := &LauncherCache{}
+	testCache.Initialize(mockPlatform, cfg)
+	GlobalLauncherCache = testCache
+	defer func() { GlobalLauncherCache = originalCache }()
+
+	_, err := FindLauncher(cfg, mockPlatform, "/some/random/file.xyz")
+	assert.Error(t, err, "should return error when no launcher matches")
+}
+
+func TestFindLauncher_AllowListBlocksAfterSpecificity(t *testing.T) {
+	mockPlatform := mocks.NewMockPlatform()
+	mockPlatform.On("Settings").Return(platforms.Settings{})
+	mockPlatform.On("RootDirs", mock.AnythingOfType("*config.Instance")).Return([]string{})
+
+	// AllowListOnly launcher wins by specificity but blocks due to allowlist
+	launcher := platforms.Launcher{
+		ID:            "Generic",
+		Extensions:    []string{".sh"},
+		AllowListOnly: true,
+	}
+
+	mockPlatform.On("Launchers", mock.AnythingOfType("*config.Instance")).Return(
+		[]platforms.Launcher{launcher})
+
+	cfg := &config.Instance{}
+
+	originalCache := GlobalLauncherCache
+	testCache := &LauncherCache{}
+	testCache.Initialize(mockPlatform, cfg)
+	GlobalLauncherCache = testCache
+	defer func() { GlobalLauncherCache = originalCache }()
+
+	_, err := FindLauncher(cfg, mockPlatform, "/path/script.sh")
+	require.Error(t, err, "allowlist should block even after specificity selection")
+	assert.Contains(t, err.Error(), "file not allowed")
+}
+
+func TestFindLauncher_TieBreaksToFirstMatch(t *testing.T) {
+	mockPlatform := mocks.NewMockPlatform()
+	mockPlatform.On("Settings").Return(platforms.Settings{})
+	mockPlatform.On("RootDirs", mock.AnythingOfType("*config.Instance")).Return([]string{"/roms"})
+
+	// Two launchers with identical specificity (both have Folders+SystemID = 110)
+	first := platforms.Launcher{
+		ID:         "First",
+		SystemID:   "NES",
+		Folders:    []string{"nes"},
+		Extensions: []string{".nes"},
+	}
+	second := platforms.Launcher{
+		ID:         "Second",
+		SystemID:   "NES",
+		Folders:    []string{"nes"},
+		Extensions: []string{".nes"},
+	}
+
+	mockPlatform.On("Launchers", mock.AnythingOfType("*config.Instance")).Return(
+		[]platforms.Launcher{first, second})
+
+	cfg := &config.Instance{}
+
+	originalCache := GlobalLauncherCache
+	testCache := &LauncherCache{}
+	testCache.Initialize(mockPlatform, cfg)
+	GlobalLauncherCache = testCache
+	defer func() { GlobalLauncherCache = originalCache }()
+
+	launcher, err := FindLauncher(cfg, mockPlatform, "/roms/nes/mario.nes")
+	require.NoError(t, err)
+	assert.Equal(t, "First", launcher.ID, "on tie, first match should win")
+}

pkg/platforms/batocera/platform.go

@@ -633,18 +633,6 @@ func (p *Platform) Launchers(cfg *config.Instance) []platforms.Launcher {
 		kodi.NewKodiAlbumLauncher(),
 		kodi.NewKodiArtistLauncher(),
 		kodi.NewKodiTVShowLauncher(),
-		platforms.Launcher{
-			ID:            "Generic",
-			Extensions:    []string{".sh"},
-			AllowListOnly: true,
-			Launch: func(_ *config.Instance, path string, _ *platforms.LaunchOptions) (*os.Process, error) {
-				err := exec.CommandContext(context.Background(), path).Start()
-				if err != nil {
-					return nil, fmt.Errorf("failed to start command: %w", err)
-				}
-				return nil, nil //nolint:nilnil // Command launches don't return a process handle
-			},
-		},
 	)
 
 	for folder, info := range esde.SystemMap {
@@ -746,6 +734,19 @@ func (p *Platform) Launchers(cfg *config.Instance) []platforms.Launcher {
 		})
 	}
 
+	launchers = append(launchers, platforms.Launcher{
+		ID:            "Generic",
+		Extensions:    []string{".sh"},
+		AllowListOnly: true,
+		Launch: func(_ *config.Instance, path string, _ *platforms.LaunchOptions) (*os.Process, error) {
+			err := exec.CommandContext(context.Background(), path).Start()
+			if err != nil {
+				return nil, fmt.Errorf("failed to start command: %w", err)
+			}
+			return nil, nil //nolint:nilnil // Command launches don't return a process handle
+		},
+	})
+
 	return append(helpers.ParseCustomLaunchers(p, cfg.CustomLaunchers()), launchers...)
 }