fix(cache): report correct deleted count for key and key+ref deletions

Previously, deleting caches by key or key+ref could misreport the number of deleted entries, especially when multiple caches matched the criteria. This change ensures the actual number of deleted caches is reported by consuming the API response's total_count field.

pkg/cmd/cache/delete/delete.go

@@ -164,33 +164,27 @@ func deleteCaches(opts *DeleteOptions, client *api.Client, repo ghrepo.Interface
 	cs := opts.IO.ColorScheme()
 	repoName := ghrepo.FullName(repo)
 	opts.IO.StartProgressIndicator()
-	base := fmt.Sprintf("repos/%s/actions/caches", repoName)
 
+	totalDeleted := 0
 	for _, cache := range toDelete {
-		// TODO(babakks): We use two different endpoints here which have different
-		// response schemas:
+		// We use two different endpoints with different response schemas:
 		//
 		//   1. /repos/OWNER/REPO/actions/caches/ID (for deleting by cache ID)
 		//      - returns HTTP 204 (NO CONTENT) on success
 		//   2. /repos/OWNER/REPO/actions/caches?key=KEY[&ref=REF] (for deleting by cache key, and optionally a ref)
 		//      - returns HTTP 200 on success including information about the deleted caches
 		//
-		// So, if/when we decided to use the data in the response body we need
-		// to be careful with parsing. Probably want to split these API calls
-		// into separate functions.
+		// The API calls are split into separate functions to handle the different response handling.
 
-		path := ""
+		var count int
+		var err error
 		if id, ok := parseCacheID(cache); ok {
-			path = fmt.Sprintf("%s/%d", base, id)
+			err = deleteCacheByID(client, repo, id)
+			count = 1
 		} else {
-			path = fmt.Sprintf("%s?key=%s", base, url.QueryEscape(cache))
-
-			if opts.Ref != "" {
-				path += fmt.Sprintf("&ref=%s", url.QueryEscape(opts.Ref))
-			}
+			count, err = deleteCacheByKey(client, repo, cache, opts.Ref)
 		}
 
-		err := client.REST(repo.RepoHost(), "DELETE", path, nil, nil)
 		if err != nil {
 			var httpErr api.HTTPError
 			if errors.As(err, &httpErr) {
@@ -207,17 +201,38 @@ func deleteCaches(opts *DeleteOptions, client *api.Client, repo ghrepo.Interface
 			opts.IO.StopProgressIndicator()
 			return err
 		}
+
+		totalDeleted += count
 	}
 
 	opts.IO.StopProgressIndicator()
 
 	if opts.IO.IsStdoutTTY() {
-		fmt.Fprintf(opts.IO.Out, "%s Deleted %s from %s\n", cs.SuccessIcon(), text.Pluralize(len(toDelete), "cache"), repoName)
+		fmt.Fprintf(opts.IO.Out, "%s Deleted %s from %s\n", cs.SuccessIcon(), text.Pluralize(totalDeleted, "cache"), repoName)
 	}
 
 	return nil
 }
 
+func deleteCacheByID(client *api.Client, repo ghrepo.Interface, id int) error {
+	path := fmt.Sprintf("repos/%s/actions/caches/%d", ghrepo.FullName(repo), id)
+	return client.REST(repo.RepoHost(), "DELETE", path, nil, nil)
+}
+
+func deleteCacheByKey(client *api.Client, repo ghrepo.Interface, key, ref string) (int, error) {
+	path := fmt.Sprintf("repos/%s/actions/caches?key=%s", ghrepo.FullName(repo), url.QueryEscape(key))
+	if ref != "" {
+		path += fmt.Sprintf("&ref=%s", url.QueryEscape(ref))
+	}
+	var payload shared.CachePayload
+	err := client.REST(repo.RepoHost(), "DELETE", path, nil, &payload)
+	if err != nil {
+		return 0, err
+	}
+
+	return payload.TotalCount, nil
+}
+
 func parseCacheID(arg string) (int, bool) {
 	id, err := strconv.Atoi(arg)
 	return id, err == nil

pkg/cmd/cache/delete/delete_test.go

@@ -235,13 +235,30 @@ func TestDeleteRun(t *testing.T) {
 					httpmock.QueryMatcher("DELETE", "repos/OWNER/REPO/actions/caches", url.Values{
 						"key": []string{"a weird＿cache+key"},
 					}),
-					// The response is a JSON object but we don't need it here.
-					httpmock.StatusStringResponse(200, "{}"),
+					httpmock.JSONResponse(shared.CachePayload{
+						TotalCount: 1,
+					}),
 				)
 			},
 			tty:        true,
 			wantStdout: "✓ Deleted 1 cache from OWNER/REPO\n",
 		},
+		{
+			name: "deletes multiple caches by key",
+			opts: DeleteOptions{Identifier: "shared-cache-key"},
+			stubs: func(reg *httpmock.Registry) {
+				reg.Register(
+					httpmock.QueryMatcher("DELETE", "repos/OWNER/REPO/actions/caches", url.Values{
+						"key": []string{"shared-cache-key"},
+					}),
+					httpmock.JSONResponse(shared.CachePayload{
+						TotalCount: 5,
+					}),
+				)
+			},
+			tty:        true,
+			wantStdout: "✓ Deleted 5 caches from OWNER/REPO\n",
+		},
 		{
 			name: "no caches to delete when deleting all",
 			opts: DeleteOptions{DeleteAll: true},
@@ -299,8 +316,9 @@ func TestDeleteRun(t *testing.T) {
 						"key": []string{"cache-key"},
 						"ref": []string{"refs/heads/main"},
 					}),
-					// The response is a JSON object but we don't need it here.
-					httpmock.StatusStringResponse(200, "{}"),
+					httpmock.JSONResponse(shared.CachePayload{
+						TotalCount: 1,
+					}),
 				)
 			},
 			tty:        true,
@@ -315,13 +333,31 @@ func TestDeleteRun(t *testing.T) {
 						"key": []string{"cache-key"},
 						"ref": []string{"refs/heads/main"},
 					}),
-					// The response is a JSON object but we don't need it here.
-					httpmock.StatusStringResponse(200, "{}"),
+					httpmock.JSONResponse(shared.CachePayload{
+						TotalCount: 1,
+					}),
 				)
 			},
 			tty:        false,
 			wantStdout: "",
 		},
+		{
+			name: "deletes multiple caches by key and ref",
+			opts: DeleteOptions{Identifier: "cache-key", Ref: "refs/heads/feature"},
+			stubs: func(reg *httpmock.Registry) {
+				reg.Register(
+					httpmock.QueryMatcher("DELETE", "repos/OWNER/REPO/actions/caches", url.Values{
+						"key": []string{"cache-key"},
+						"ref": []string{"refs/heads/feature"},
+					}),
+					httpmock.JSONResponse(shared.CachePayload{
+						TotalCount: 3,
+					}),
+				)
+			},
+			tty:        true,
+			wantStdout: "✓ Deleted 3 caches from OWNER/REPO\n",
+		},
 		{
 			// As of now, the API returns HTTP 404 for invalid or non-existent refs.
 			name: "cache key exists but ref is invalid/not-found",