diff --git a/CHANGELOG.md b/CHANGELOG.md index 7180d00..aa5a3d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - 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. +- Fix playlist add/remove 429s by using Connect playlist mutations with writable-playlist checks and fallback coverage across engines (`#20`). ## 0.3.0 - 2026-03-08 diff --git a/internal/app/context_factory.go b/internal/app/context_factory.go index e4d65fe..fb7b560 100644 --- a/internal/app/context_factory.go +++ b/internal/app/context_factory.go @@ -81,6 +81,9 @@ func (c *Context) newAppleScriptClient(source cookies.Source) (spotify.API, erro var fallback spotify.API if webClient, webErr := c.newWebClient(source); webErr == nil { fallback = webClient + if connectClient, connectErr := c.newConnectClient(source); connectErr == nil { + fallback = spotify.NewPlaybackFallbackClient(webClient, connectClient) + } } return spotify.NewAppleScriptClient(spotify.AppleScriptOptions{Fallback: fallback}) } diff --git a/internal/spotify/connect.go b/internal/spotify/connect.go index 76d2b5c..a5eb1d5 100644 --- a/internal/spotify/connect.go +++ b/internal/spotify/connect.go @@ -193,12 +193,22 @@ func (c *ConnectClient) CreatePlaylist(ctx context.Context, name string, public, } func (c *ConnectClient) AddTracks(ctx context.Context, playlistID string, uris []string) error { + if err := c.addTracks(ctx, playlistID, uris); err == nil { + return nil + } else if errors.Is(err, errPlaylistNotWritable) { + return err + } return withWebFallback(c, func(web *Client) error { return web.AddTracks(ctx, playlistID, uris) }) } func (c *ConnectClient) RemoveTracks(ctx context.Context, playlistID string, uris []string) error { + if err := c.removeTracks(ctx, playlistID, uris); err == nil { + return nil + } else if errors.Is(err, errPlaylistNotWritable) { + return err + } return withWebFallback(c, func(web *Client) error { return web.RemoveTracks(ctx, playlistID, uris) }) diff --git a/internal/spotify/connect_playlist_mutation.go b/internal/spotify/connect_playlist_mutation.go new file mode 100644 index 0000000..6bd5bee --- /dev/null +++ b/internal/spotify/connect_playlist_mutation.go @@ -0,0 +1,131 @@ +package spotify + +import ( + "context" + "errors" + "fmt" +) + +var errPlaylistNotWritable = errors.New("playlist is not writable") + +func (c *ConnectClient) addTracks(ctx context.Context, playlistID string, uris []string) error { + if err := c.ensurePlaylistWritable(ctx, playlistID); err != nil { + return err + } + _, err := c.graphQL(ctx, "addToPlaylist", map[string]any{ + "playlistUri": "spotify:playlist:" + playlistID, + "playlistItemUris": uris, + "newPosition": map[string]any{ + "moveType": "TOP_OF_PLAYLIST", + "fromUid": nil, + }, + }) + return err +} + +func (c *ConnectClient) removeTracks(ctx context.Context, playlistID string, uris []string) error { + if err := c.ensurePlaylistWritable(ctx, playlistID); err != nil { + return err + } + uids, err := c.playlistTrackUIDs(ctx, playlistID, uris) + if err != nil { + return err + } + _, err = c.graphQL(ctx, "removeFromPlaylist", map[string]any{ + "playlistUri": "spotify:playlist:" + playlistID, + "uids": uids, + }) + return err +} + +func (c *ConnectClient) playlistTrackUIDs(ctx context.Context, playlistID string, uris []string) ([]string, error) { + if len(uris) == 0 { + return nil, fmt.Errorf("track uri required") + } + need := map[string]int{} + for _, uri := range uris { + need[uri]++ + } + uids := make([]string, 0, len(uris)) + offset := 0 + const limit = 100 + for len(uids) < len(uris) { + payload, err := c.graphQL(ctx, "fetchPlaylist", playlistTrackVariables(playlistID, limit, offset)) + if err != nil { + return nil, err + } + found, total := extractPlaylistTrackUIDs(payload, need) + uids = append(uids, found...) + if total <= 0 || offset+limit >= total { + break + } + offset += limit + } + if len(uids) != len(uris) { + return nil, fmt.Errorf("playlist items not found for removal") + } + return uids, nil +} + +func extractPlaylistTrackUIDs(payload map[string]any, need map[string]int) ([]string, int) { + content, ok := getMap(payload, "data", "playlistV2", "content") + if !ok { + return nil, 0 + } + rawItems, _ := content["items"].([]any) + uids := make([]string, 0) + for _, raw := range rawItems { + m, ok := raw.(map[string]any) + if !ok { + continue + } + wrapper, ok := m["itemV2"].(map[string]any) + if !ok { + continue + } + uid := getString(wrapper, "uid") + if uid == "" { + uid = getString(m, "uid") + } + dataM, _ := wrapper["data"].(map[string]any) + uri := playlistTrackURI(dataM) + if uid == "" || uri == "" || need[uri] <= 0 { + continue + } + need[uri]-- + uids = append(uids, uid) + } + return uids, getInt(content, "totalCount") +} + +func playlistTrackURI(data map[string]any) string { + if data == nil { + return "" + } + if uri := getString(data, "uri"); uri != "" { + return uri + } + if track, ok := data["track"].(map[string]any); ok { + if uri := getString(track, "uri"); uri != "" { + return uri + } + } + return findFirstURI(data, "track") +} + +func (c *ConnectClient) ensurePlaylistWritable(ctx context.Context, playlistID string) error { + payload, err := c.graphQL(ctx, "playlistPermissions", map[string]any{ + "uri": "spotify:playlist:" + playlistID, + }) + if err != nil { + return err + } + caps, ok := getMap(payload, "data", "playlistV2", "currentUserCapabilities") + if !ok { + return fmt.Errorf("playlist permissions missing") + } + if !getBool(caps, "canEditItems") { + return fmt.Errorf("%w: %s", errPlaylistNotWritable, playlistID) + } + return nil +} diff --git a/internal/spotify/connect_playlist_mutation_test.go b/internal/spotify/connect_playlist_mutation_test.go new file mode 100644 index 0000000..5a0e480 --- /dev/null +++ b/internal/spotify/connect_playlist_mutation_test.go @@ -0,0 +1,210 @@ +package spotify + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "testing" +) + +func TestConnectAddTracksUsesPathfinderMutation(t *testing.T) { + var variables map[string]any + transport := roundTripperFunc(func(req *http.Request) (*http.Response, error) { + switch req.URL.Query().Get("operationName") { + case "playlistPermissions": + return jsonResponse(http.StatusOK, playlistWritablePayload(true)), nil + case "addToPlaylist": + if err := json.Unmarshal([]byte(req.URL.Query().Get("variables")), &variables); err != nil { + t.Fatalf("variables: %v", err) + } + return jsonResponse(http.StatusOK, map[string]any{"data": map[string]any{"addToPlaylist": true}}), nil + default: + return textResponse(http.StatusNotFound, "missing"), nil + } + }) + client := newConnectClientForTests(transport) + client.hashes.hashes["playlistPermissions"] = "hash" + client.hashes.hashes["addToPlaylist"] = "hash" + + err := client.AddTracks(context.Background(), "p1", []string{"spotify:track:t1"}) + if err != nil { + t.Fatalf("add tracks: %v", err) + } + if got := getString(variables, "playlistUri"); got != "spotify:playlist:p1" { + t.Fatalf("playlistUri = %q", got) + } + uris, _ := variables["playlistItemUris"].([]any) + if len(uris) != 1 || uris[0] != "spotify:track:t1" { + t.Fatalf("playlistItemUris = %#v", variables["playlistItemUris"]) + } + position, _ := variables["newPosition"].(map[string]any) + if got := getString(position, "moveType"); got != "TOP_OF_PLAYLIST" { + t.Fatalf("moveType = %q", got) + } +} + +func TestConnectRemoveTracksUsesResolvedPlaylistUIDs(t *testing.T) { + operations := []string{} + var removeVariables map[string]any + transport := roundTripperFunc(func(req *http.Request) (*http.Response, error) { + op := req.URL.Query().Get("operationName") + operations = append(operations, op) + switch op { + case "playlistPermissions": + return jsonResponse(http.StatusOK, playlistWritablePayload(true)), nil + case "fetchPlaylist": + return jsonResponse(http.StatusOK, map[string]any{ + "data": map[string]any{"playlistV2": map[string]any{"content": map[string]any{ + "totalCount": 2, + "items": []any{ + map[string]any{"itemV2": map[string]any{ + "uid": "uid-1", + "data": map[string]any{"track": map[string]any{"uri": "spotify:track:t1"}}, + }}, + map[string]any{"itemV2": map[string]any{ + "uid": "uid-2", + "data": map[string]any{"track": map[string]any{"uri": "spotify:track:t2"}}, + }}, + }, + }}}, + }), nil + case "removeFromPlaylist": + if err := json.Unmarshal([]byte(req.URL.Query().Get("variables")), &removeVariables); err != nil { + t.Fatalf("variables: %v", err) + } + return jsonResponse(http.StatusOK, map[string]any{"data": map[string]any{"removeFromPlaylist": true}}), nil + default: + return textResponse(http.StatusNotFound, "missing"), nil + } + }) + client := newConnectClientForTests(transport) + client.hashes.hashes["playlistPermissions"] = "hash" + client.hashes.hashes["fetchPlaylist"] = "hash" + client.hashes.hashes["removeFromPlaylist"] = "hash" + + err := client.RemoveTracks(context.Background(), "p1", []string{"spotify:track:t2"}) + if err != nil { + t.Fatalf("remove tracks: %v", err) + } + if len(operations) != 3 || operations[0] != "playlistPermissions" || operations[1] != "fetchPlaylist" || operations[2] != "removeFromPlaylist" { + t.Fatalf("operations = %#v", operations) + } + if got := getString(removeVariables, "playlistUri"); got != "spotify:playlist:p1" { + t.Fatalf("playlistUri = %q", got) + } + uids, _ := removeVariables["uids"].([]any) + if len(uids) != 1 || uids[0] != "uid-2" { + t.Fatalf("uids = %#v", removeVariables["uids"]) + } +} + +func TestConnectRemoveTracksFindsUIDOnLaterPlaylistPage(t *testing.T) { + fetches := 0 + transport := roundTripperFunc(func(req *http.Request) (*http.Response, error) { + switch req.URL.Query().Get("operationName") { + case "playlistPermissions": + return jsonResponse(http.StatusOK, playlistWritablePayload(true)), nil + case "fetchPlaylist": + fetches++ + var vars map[string]any + if err := json.Unmarshal([]byte(req.URL.Query().Get("variables")), &vars); err != nil { + t.Fatalf("variables: %v", err) + } + items := []any{map[string]any{"itemV2": map[string]any{ + "uid": "uid-other", + "data": map[string]any{"track": map[string]any{"uri": "spotify:track:other"}}, + }}} + if getInt(vars, "offset") == 100 { + items = []any{map[string]any{"itemV2": map[string]any{ + "uid": "uid-target", + "data": map[string]any{"track": map[string]any{"uri": "spotify:track:target"}}, + }}} + } + return jsonResponse(http.StatusOK, map[string]any{ + "data": map[string]any{"playlistV2": map[string]any{"content": map[string]any{ + "totalCount": 150, + "items": items, + }}}, + }), nil + case "removeFromPlaylist": + return jsonResponse(http.StatusOK, map[string]any{"data": map[string]any{"removeFromPlaylist": true}}), nil + default: + return textResponse(http.StatusNotFound, "missing"), nil + } + }) + client := newConnectClientForTests(transport) + client.hashes.hashes["playlistPermissions"] = "hash" + client.hashes.hashes["fetchPlaylist"] = "hash" + client.hashes.hashes["removeFromPlaylist"] = "hash" + + err := client.RemoveTracks(context.Background(), "p1", []string{"spotify:track:target"}) + if err != nil { + t.Fatalf("remove tracks: %v", err) + } + if fetches != 2 { + t.Fatalf("fetches = %d", fetches) + } +} + +func TestConnectAddTracksFallsBackToWeb(t *testing.T) { + webCalled := false + transport := roundTripperFunc(func(req *http.Request) (*http.Response, error) { + return textResponse(http.StatusInternalServerError, "fail"), nil + }) + client := newConnectClientForTests(transport) + client.hashes.hashes["playlistPermissions"] = "hash" + client.hashes.hashes["addToPlaylist"] = "hash" + client.web = mustNewWebClientForPlaylistMutationTest(t, func(w http.ResponseWriter, r *http.Request) { + webCalled = true + w.WriteHeader(http.StatusNoContent) + }) + + err := client.AddTracks(context.Background(), "p1", []string{"spotify:track:t1"}) + if err != nil { + t.Fatalf("add tracks fallback: %v", err) + } + if !webCalled { + t.Fatalf("expected web fallback") + } +} + +func TestConnectAddTracksRejectsNonWritablePlaylist(t *testing.T) { + webCalled := false + transport := roundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Query().Get("operationName") != "playlistPermissions" { + return textResponse(http.StatusNotFound, "missing"), nil + } + return jsonResponse(http.StatusOK, playlistWritablePayload(false)), nil + }) + client := newConnectClientForTests(transport) + client.hashes.hashes["playlistPermissions"] = "hash" + client.hashes.hashes["addToPlaylist"] = "hash" + client.web = mustNewWebClientForPlaylistMutationTest(t, func(w http.ResponseWriter, r *http.Request) { + webCalled = true + w.WriteHeader(http.StatusNoContent) + }) + + err := client.AddTracks(context.Background(), "p1", []string{"spotify:track:t1"}) + if !errors.Is(err, errPlaylistNotWritable) { + t.Fatalf("expected not writable error, got %v", err) + } + if webCalled { + t.Fatalf("did not expect web fallback") + } +} + +func playlistWritablePayload(writable bool) map[string]any { + return map[string]any{ + "data": map[string]any{"playlistV2": map[string]any{ + "currentUserCapabilities": map[string]any{"canEditItems": writable}, + }}, + } +} + +func mustNewWebClientForPlaylistMutationTest(t *testing.T, handler http.HandlerFunc) *Client { + t.Helper() + client, closeFn := newTestClient(t, handler) + t.Cleanup(closeFn) + return client +} diff --git a/internal/spotify/fallback.go b/internal/spotify/fallback.go index 02b4c4c..1da1b76 100644 --- a/internal/spotify/fallback.go +++ b/internal/spotify/fallback.go @@ -208,9 +208,13 @@ func (c *fallbackClient) CreatePlaylist(ctx context.Context, name string, public } func (c *fallbackClient) AddTracks(ctx context.Context, playlistID string, uris []string) error { - return c.web.AddTracks(ctx, playlistID, uris) + return fallbackVoid(c, true, func(api API) error { + return api.AddTracks(ctx, playlistID, uris) + }) } func (c *fallbackClient) RemoveTracks(ctx context.Context, playlistID string, uris []string) error { - return c.web.RemoveTracks(ctx, playlistID, uris) + return fallbackVoid(c, true, func(api API) error { + return api.RemoveTracks(ctx, playlistID, uris) + }) } diff --git a/internal/spotify/fallback_test.go b/internal/spotify/fallback_test.go index 79c14c4..ec3b06e 100644 --- a/internal/spotify/fallback_test.go +++ b/internal/spotify/fallback_test.go @@ -15,6 +15,8 @@ type apiStub struct { libraryModifyFn func(context.Context, string, []string, string) error followedArtistsFn func(context.Context, int, string) ([]Item, int, string, error) artistTopTracksFn func(context.Context, string, int) ([]Item, error) + addTracksFn func(context.Context, string, []string) error + removeTracksFn func(context.Context, string, []string) error } func (a apiStub) Search(ctx context.Context, kind, query string, limit, offset int) (SearchResult, error) { @@ -186,13 +188,19 @@ func (a apiStub) CreatePlaylist(context.Context, string, bool, bool) (Item, erro return Item{}, nil } -func (a apiStub) AddTracks(context.Context, string, []string) error { +func (a apiStub) AddTracks(ctx context.Context, playlistID string, uris []string) error { a.note("AddTracks") + if a.addTracksFn != nil { + return a.addTracksFn(ctx, playlistID, uris) + } return nil } -func (a apiStub) RemoveTracks(context.Context, string, []string) error { +func (a apiStub) RemoveTracks(ctx context.Context, playlistID string, uris []string) error { a.note("RemoveTracks") + if a.removeTracksFn != nil { + return a.removeTracksFn(ctx, playlistID, uris) + } return nil } @@ -330,6 +338,31 @@ func TestFallbackPauseOnRateLimit(t *testing.T) { } } +func TestFallbackPlaylistWritesOnRateLimit(t *testing.T) { + ctx := context.Background() + calls := map[string]int{} + web := apiStub{ + calls: calls, + addTracksFn: func(context.Context, string, []string) error { + return APIError{Status: 429, Message: "rate limit"} + }, + removeTracksFn: func(context.Context, string, []string) error { + return APIError{Status: 429, Message: "rate limit"} + }, + } + connect := apiStub{calls: calls} + client := NewPlaybackFallbackClient(web, connect) + if err := client.AddTracks(ctx, "p1", []string{"spotify:track:t1"}); err != nil { + t.Fatalf("add tracks: %v", err) + } + if err := client.RemoveTracks(ctx, "p1", []string{"spotify:track:t1"}); err != nil { + t.Fatalf("remove tracks: %v", err) + } + if calls["AddTracks"] != 2 || calls["RemoveTracks"] != 2 { + t.Fatalf("unexpected calls: %#v", calls) + } +} + func TestFallbackDelegatesToWeb(t *testing.T) { ctx := context.Background() calls := map[string]int{}