From a0298286c0cf4a20d797810d14e179fbab8582b6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 5 May 2026 06:52:54 +0100 Subject: [PATCH] fix: harden live metadata and device playback paths --- CHANGELOG.md | 3 + cmd/spogo/main.go | 23 ++++++ cmd/spogo/main_test.go | 8 ++ internal/spotify/client.go | 5 ++ internal/spotify/client_test.go | 65 +++++++++++++++- internal/spotify/connect_commands.go | 27 ++++++- internal/spotify/connect_extract_items.go | 9 ++- internal/spotify/connect_extract_metadata.go | 13 ++++ internal/spotify/connect_mapping_test.go | 64 ++++++++++++++++ internal/spotify/connect_playback_test.go | 80 ++++++++++++++++++++ internal/spotify/mapper.go | 46 +++++++++++ 11 files changed, 338 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b9b91f..7180d00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ - Fix Connect liked-track listing via `fetchLibraryTracks` with Web API fallback on payload drift (`#16`, thanks @masonc15) - Fix Connect play when no device is active by falling back to Web API playback (`#21`, thanks @prashanthbala) - Fix Connect volume changes by sending the volume endpoint as `PUT` (`#24`, thanks @cavit99) +- Fix sparse status/search metadata so track artists and albums are populated consistently across engines. +- Fix Connect `--device` playback when no device is active without falling back to rate-limited Web API playback. +- Fix `auth paste --no-input` by accepting the documented flag order. ## 0.3.0 - 2026-03-08 diff --git a/cmd/spogo/main.go b/cmd/spogo/main.go index f51ddab..6ef9513 100644 --- a/cmd/spogo/main.go +++ b/cmd/spogo/main.go @@ -34,6 +34,7 @@ func run(args []string, out io.Writer, errOut io.Writer) int { _, _ = fmt.Fprintln(errOut, err) return 2 } + args = normalizeArgs(args) kctx, err := parser.Parse(args) if exitCode >= 0 { return exitCode @@ -63,3 +64,25 @@ func run(args []string, out io.Writer, errOut io.Writer) int { } return 0 } + +func normalizeArgs(args []string) []string { + if len(args) == 0 { + return args + } + front := make([]string, 0, 1) + rest := make([]string, 0, len(args)) + for _, arg := range args { + if arg == "--no-input" { + front = append(front, arg) + continue + } + rest = append(rest, arg) + } + if len(front) == 0 { + return args + } + normalized := make([]string, 0, len(args)) + normalized = append(normalized, front...) + normalized = append(normalized, rest...) + return normalized +} diff --git a/cmd/spogo/main_test.go b/cmd/spogo/main_test.go index 38c0312..2292394 100644 --- a/cmd/spogo/main_test.go +++ b/cmd/spogo/main_test.go @@ -91,6 +91,14 @@ func TestRunAuthStatus(t *testing.T) { } } +func TestNormalizeArgsMovesNoInput(t *testing.T) { + got := normalizeArgs([]string{"auth", "paste", "--no-input", "--cookie-path", "cookies.json"}) + want := []string{"--no-input", "auth", "paste", "--cookie-path", "cookies.json"} + if fmt.Sprint(got) != fmt.Sprint(want) { + t.Fatalf("got %v, want %v", got, want) + } +} + func TestMain(t *testing.T) { origArgs := os.Args origExit := exitFunc diff --git a/internal/spotify/client.go b/internal/spotify/client.go index ce50c88..74529fa 100644 --- a/internal/spotify/client.go +++ b/internal/spotify/client.go @@ -176,6 +176,11 @@ func (c *Client) Playback(ctx context.Context) (PlaybackStatus, error) { if raw.Item.ID != "" { item := mapTrack(raw.Item) status.Item = &item + if itemNeedsTrackMetadata(status.Item) { + if full, err := c.GetTrack(ctx, status.Item.ID); err == nil { + mergeItemMetadata(status.Item, full) + } + } } return status, nil } diff --git a/internal/spotify/client_test.go b/internal/spotify/client_test.go index 42e6f68..7b09fb7 100644 --- a/internal/spotify/client_test.go +++ b/internal/spotify/client_test.go @@ -84,11 +84,74 @@ func TestPlaybackNoContent(t *testing.T) { func TestPlaybackWithItem(t *testing.T) { handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/me/player": + payload := playbackResponse{ + IsPlaying: true, + ProgressMS: 1000, + Device: deviceItem{Name: "Desk"}, + Item: trackItem{ID: "t1", Name: "Song", Artists: []artistRef{{Name: "Artist"}}}, + } + _ = json.NewEncoder(w).Encode(payload) + default: + w.WriteHeader(http.StatusNotFound) + } + }) + client, closeFn := newTestClient(t, handler) + defer closeFn() + status, err := client.Playback(context.Background()) + if err != nil { + t.Fatalf("playback: %v", err) + } + if status.Item == nil || status.Item.Name != "Song" { + t.Fatalf("expected item") + } +} + +func TestPlaybackHydratesSparseItem(t *testing.T) { + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/me/player": + payload := playbackResponse{ + IsPlaying: true, + Item: trackItem{ID: "t1", URI: "spotify:track:t1", Name: "Song"}, + } + _ = json.NewEncoder(w).Encode(payload) + case "/tracks/t1": + payload := trackItem{ + ID: "t1", + URI: "spotify:track:t1", + Name: "Song", + Album: albumRef{Name: "Album"}, + Artists: []artistRef{{Name: "Artist"}}, + } + _ = json.NewEncoder(w).Encode(payload) + default: + w.WriteHeader(http.StatusNotFound) + } + }) + client, closeFn := newTestClient(t, handler) + defer closeFn() + status, err := client.Playback(context.Background()) + if err != nil { + t.Fatalf("playback: %v", err) + } + if status.Item == nil || len(status.Item.Artists) != 1 || status.Item.Artists[0] != "Artist" || status.Item.Album != "Album" { + t.Fatalf("expected hydrated item: %#v", status.Item) + } +} + +func TestPlaybackKeepsSparseItemWhenHydrationFails(t *testing.T) { + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/me/player" { + w.WriteHeader(http.StatusTooManyRequests) + return + } payload := playbackResponse{ IsPlaying: true, ProgressMS: 1000, Device: deviceItem{Name: "Desk"}, - Item: trackItem{ID: "t1", Name: "Song", Artists: []artistRef{{Name: "Artist"}}}, + Item: trackItem{ID: "t1", URI: "spotify:track:t1", Name: "Song"}, } _ = json.NewEncoder(w).Encode(payload) }) diff --git a/internal/spotify/connect_commands.go b/internal/spotify/connect_commands.go index 40cb288..506f309 100644 --- a/internal/spotify/connect_commands.go +++ b/internal/spotify/connect_commands.go @@ -10,7 +10,13 @@ import ( func (c *ConnectClient) playback(ctx context.Context) (PlaybackStatus, error) { return withConnectState(ctx, c, func(state connectState) (PlaybackStatus, error) { - return mapPlaybackStatus(state), nil + status := mapPlaybackStatus(state) + if itemNeedsTrackMetadata(status.Item) { + if full, err := c.trackInfo(ctx, status.Item.ID); err == nil { + mergeItemMetadata(status.Item, full) + } + } + return status, nil }) } @@ -44,7 +50,11 @@ func (c *ConnectClient) transferViaWebAPI(ctx context.Context, deviceID string) func (c *ConnectClient) play(ctx context.Context, uri string) error { return withConnectStateErr(ctx, c, func(state connectState) error { if state.activeDeviceID == "" { - return c.playViaWebAPI(ctx, uri) + if targetID := resolveConnectTargetDeviceID(state, c.device); targetID != "" { + state.activeDeviceID = targetID + } else { + return c.playViaWebAPI(ctx, uri) + } } if uri == "" { return c.sendPlayerCommand(ctx, state, "resume", nil) @@ -175,6 +185,19 @@ func connectTransferSourceID(state connectState) string { return fromID } +func resolveConnectTargetDeviceID(state connectState, selector string) string { + selector = strings.TrimSpace(selector) + if selector == "" { + return "" + } + for _, device := range mapDevices(state) { + if strings.EqualFold(device.ID, selector) || strings.EqualFold(device.Name, selector) { + return device.ID + } + } + return "" +} + func playCommandPayload(uri string) map[string]any { command := map[string]any{ "endpoint": "play", diff --git a/internal/spotify/connect_extract_items.go b/internal/spotify/connect_extract_items.go index f360088..03c5394 100644 --- a/internal/spotify/connect_extract_items.go +++ b/internal/spotify/connect_extract_items.go @@ -69,8 +69,13 @@ func extractItem(value any, kind string) (Item, bool) { } item.URL = fmt.Sprintf("https://open.spotify.com/%s/%s", item.Type, item.ID) item.Artists = extractArtistNames(m) - if album := extractAlbumName(m); album != "" { - item.Album = album + if len(item.Artists) == 0 && item.Type == "track" { + item.Artists = findFirstArtistNames(m) + } + if item.Type == "track" { + if album := extractAlbumName(m); album != "" { + item.Album = album + } } item.Explicit = getBool(m, "explicit") item.DurationMS = getInt(m, "duration_ms") diff --git a/internal/spotify/connect_extract_metadata.go b/internal/spotify/connect_extract_metadata.go index 1b854ce..b584d7b 100644 --- a/internal/spotify/connect_extract_metadata.go +++ b/internal/spotify/connect_extract_metadata.go @@ -60,6 +60,19 @@ func appendArtistNames(artists *[]string, entries []any) { } } +func findFirstArtistNames(value any) []string { + var artists []string + walkMap(value, func(m map[string]any) { + if len(artists) > 0 { + return + } + if found := extractArtistNames(m); len(found) > 0 { + artists = found + } + }) + return artists +} + func artistNameFromValue(value any) string { m, ok := value.(map[string]any) if !ok { diff --git a/internal/spotify/connect_mapping_test.go b/internal/spotify/connect_mapping_test.go index dea0df2..cb4f208 100644 --- a/internal/spotify/connect_mapping_test.go +++ b/internal/spotify/connect_mapping_test.go @@ -232,6 +232,70 @@ func TestExtractItemOtherArtistsItems(t *testing.T) { } } +func TestExtractSearchTrackItemNestedArtists(t *testing.T) { + payload := map[string]any{ + "data": map[string]any{ + "searchV2": map[string]any{ + "tracksV2": map[string]any{ + "totalCount": 1, + "items": []any{ + map[string]any{ + "item": map[string]any{ + "data": map[string]any{ + "uri": "spotify:track:abc", + "name": "Song", + "artists": map[string]any{ + "items": []any{ + map[string]any{"profile": map[string]any{"name": "Artist"}}, + }, + }, + "albumOfTrack": map[string]any{"name": "Album"}, + }, + }, + }, + }, + }, + }, + }, + } + items, total := extractSearchItems(payload, "track") + if total != 1 || len(items) != 1 { + t.Fatalf("unexpected items: %#v total=%d", items, total) + } + if len(items[0].Artists) != 1 || items[0].Artists[0] != "Artist" || items[0].Album != "Album" { + t.Fatalf("unexpected metadata: %#v", items[0]) + } +} + +func TestExtractPlaylistDoesNotLeakNestedTrackAlbum(t *testing.T) { + payload := map[string]any{ + "data": map[string]any{ + "playlistV2": map[string]any{ + "uri": "spotify:playlist:p1", + "name": "Playlist", + "content": map[string]any{ + "items": []any{ + map[string]any{"itemV2": map[string]any{"data": map[string]any{ + "track": map[string]any{ + "uri": "spotify:track:t1", + "name": "Song", + "album": map[string]any{"name": "Album"}, + }, + }}}, + }, + }, + }, + }, + } + item, ok := extractItemFromPayload(payload, "playlist") + if !ok { + t.Fatalf("expected playlist") + } + if item.Album != "" { + t.Fatalf("playlist leaked album metadata: %#v", item) + } +} + func TestExtractItemFromPayloadPrefersTrackUnion(t *testing.T) { payload := map[string]any{ "data": map[string]any{ diff --git a/internal/spotify/connect_playback_test.go b/internal/spotify/connect_playback_test.go index 08fd929..8a807de 100644 --- a/internal/spotify/connect_playback_test.go +++ b/internal/spotify/connect_playback_test.go @@ -108,6 +108,48 @@ func TestConnectPlaybackCommands(t *testing.T) { } } +func TestConnectPlaybackHydratesSparseTrack(t *testing.T) { + statePayload := map[string]any{ + "devices": map[string]any{ + "device-1": map[string]any{"name": "Desk", "device_type": "computer"}, + }, + "player_state": map[string]any{ + "is_paused": true, + "track": map[string]any{ + "uri": "spotify:track:t1", + "name": "Song", + }, + }, + "active_device_id": "device-1", + } + transport := roundTripperFunc(func(req *http.Request) (*http.Response, error) { + switch { + case req.Method == http.MethodPut && strings.Contains(req.URL.Path, "/devices/hobs_"): + return jsonResponse(http.StatusOK, statePayload), nil + case req.URL.Query().Get("operationName") == "getTrack": + return jsonResponse(http.StatusOK, map[string]any{ + "data": map[string]any{"track": map[string]any{ + "uri": "spotify:track:t1", + "name": "Song", + "artists": []any{map[string]any{"name": "Artist"}}, + "album": map[string]any{"name": "Album"}, + }}, + }), nil + default: + return textResponse(http.StatusNotFound, "missing"), nil + } + }) + client := newRegisteredConnectClientForTests(transport) + client.hashes.hashes["getTrack"] = "hash" + status, err := client.Playback(context.Background()) + if err != nil { + t.Fatalf("playback: %v", err) + } + if status.Item == nil || len(status.Item.Artists) != 1 || status.Item.Artists[0] != "Artist" || status.Item.Album != "Album" { + t.Fatalf("expected hydrated item: %#v", status.Item) + } +} + func TestConnectPlaybackActiveDeviceFromDevices(t *testing.T) { statePayload := map[string]any{ "devices": map[string]any{ @@ -238,6 +280,44 @@ func TestConnectPlayFallsBackToWebAPIWithoutActiveDevice(t *testing.T) { } } +func TestConnectPlayUsesConfiguredDeviceWithoutActiveDevice(t *testing.T) { + statePayload := map[string]any{ + "devices": map[string]any{ + "device-1": map[string]any{ + "name": "Desk", + "device_type": "computer", + }, + }, + "player_state": map[string]any{ + "is_paused": true, + }, + } + var sawConnectPlay bool + transport := roundTripperFunc(func(req *http.Request) (*http.Response, error) { + switch { + case req.Method == http.MethodPut && strings.Contains(req.URL.Path, "/devices/hobs_"): + return jsonResponse(http.StatusOK, statePayload), nil + case req.Method == http.MethodPost && strings.Contains(req.URL.Path, "/player/command/from/device/to/device-1"): + sawConnectPlay = true + return textResponse(http.StatusOK, "ok"), nil + case req.Method == http.MethodPut && req.URL.Path == "/v1/me/player/play": + t.Fatalf("unexpected web play fallback") + return nil, errors.New("unexpected web play fallback") + default: + return textResponse(http.StatusNotFound, "missing"), nil + } + }) + client := newRegisteredConnectClientForTests(transport) + client.device = "Desk" + + if err := client.Play(context.Background(), "spotify:track:abc"); err != nil { + t.Fatalf("play: %v", err) + } + if !sawConnectPlay { + t.Fatalf("expected connect play") + } +} + func TestSendPlayerCommandMissingDevice(t *testing.T) { client := newConnectClientForTests(roundTripperFunc(func(req *http.Request) (*http.Response, error) { return textResponse(http.StatusOK, ""), nil diff --git a/internal/spotify/mapper.go b/internal/spotify/mapper.go index bb87bc0..5fa4e05 100644 --- a/internal/spotify/mapper.go +++ b/internal/spotify/mapper.go @@ -150,3 +150,49 @@ func externalURL(urls map[string]string) string { } return "" } + +func itemNeedsTrackMetadata(item *Item) bool { + if item == nil || item.ID == "" { + return false + } + if item.Type != "" && item.Type != "track" { + return false + } + return item.Name == "" || len(item.Artists) == 0 || item.Album == "" +} + +func mergeItemMetadata(dst *Item, src Item) { + if dst == nil { + return + } + if dst.ID == "" { + dst.ID = src.ID + } + if dst.URI == "" { + dst.URI = src.URI + } + if dst.Name == "" { + dst.Name = src.Name + } + if dst.Type == "" { + dst.Type = src.Type + } + if dst.URL == "" { + dst.URL = src.URL + } + if len(dst.Artists) == 0 { + dst.Artists = src.Artists + } + if dst.Album == "" { + dst.Album = src.Album + } + if dst.DurationMS == 0 { + dst.DurationMS = src.DurationMS + } + if !dst.Explicit { + dst.Explicit = src.Explicit + } + if !dst.IsPlayable { + dst.IsPlayable = src.IsPlayable + } +}