Shared config with the MCP server, scopes for 3.5.2, logout revokes

cmd/backup.go

@@ -144,6 +144,8 @@ Example:
 			mediaData, err := c.Get("/media?limit=10000")
 			if err != nil {
 				fmt.Printf("failed: %v\n", err)
+				// A backup without its media is not a backup; say so instead of exiting 0
+				return fmt.Errorf("could not list media (%v); the token may lack media:read. Use --skip-media to back up documents only", err)
 			} else {
 				var mediaItems []struct {
 					ID       string `json:"id"`

cmd/clean.go

@@ -126,6 +126,7 @@ Or use --dry-run to preview what would be deleted first.
 		}
 
 		// Clean media (paginated, with rate-limit handling)
+		mediaStuck := 0
 		if !documentsOnly {
 			fmt.Print("Cleaning media files...\n")
 
@@ -158,6 +159,7 @@ Or use --dry-run to preview what would be deleted first.
 				}
 
 				totalMediaCount += len(mediaItems)
+				deletedBefore := totalMediaDeleted
 				for _, item := range mediaItems {
 					if dryRun {
 						continue
@@ -180,6 +182,14 @@ Or use --dry-run to preview what would be deleted first.
 				}
 
 				fmt.Printf("  Deleted %d media file(s) so far...\n", totalMediaDeleted)
+
+				// The listing always returns the first page, so a page in which nothing could be
+				// deleted would come back unchanged forever
+				if totalMediaDeleted == deletedBefore {
+					mediaStuck = len(mediaItems)
+					fmt.Fprintf(os.Stderr, "  Stopping: none of these %d media file(s) could be deleted. The token may lack media:delete; run 'trokky login' again with a current CLI.\n", len(mediaItems))
+					break
+				}
 			}
 
 			if totalMediaDeleted > 0 || totalMediaCount == 0 {
@@ -206,6 +216,9 @@ Or use --dry-run to preview what would be deleted first.
 		fmt.Printf("Mode:            %s\n", mode)
 		fmt.Println("──────────────────────────────────────────────────")
 
+		if mediaStuck > 0 {
+			return fmt.Errorf("%d media file(s) could not be deleted", mediaStuck)
+		}
 		return nil
 	},
 }

cmd/clean_media_test.go

@@ -0,0 +1,102 @@
+package cmd
+
+import (
+	"bytes"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+func runClean(t *testing.T, args ...string) (string, string, error) {
+	t.Helper()
+	t.Setenv("HOME", t.TempDir())
+	t.Setenv("TROKKY_URL", "")
+	t.Setenv("TROKKY_TOKEN", "")
+	t.Setenv("TROKKY_INSTANCE", "")
+	for name, def := range map[string]string{"collections": "", "media-only": "false", "documents-only": "false", "dry-run": "false", "confirm": "false"} {
+		if err := cleanCmd.Flags().Set(name, def); err != nil {
+			t.Fatal(err)
+		}
+	}
+	var stdout, stderr bytes.Buffer
+	rootCmd.SetOut(&stdout)
+	rootCmd.SetErr(&stderr)
+	defer func() {
+		rootCmd.SetOut(nil)
+		rootCmd.SetErr(nil)
+	}()
+	rootCmd.SetArgs(append([]string{"clean"}, args...))
+	var err error
+	rawOut, rawErr := captureProcessOutput(t, func() {
+		err = rootCmd.Execute()
+	})
+	return stdout.String() + rawOut, stderr.String() + rawErr, err
+}
+
+// A token that can list media but not delete it gets the same first page back forever;
+// clean must stop with an error instead of looping.
+func TestCleanStopsWhenMediaCannotBeDeleted(t *testing.T) {
+	rec := &recorder{}
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		rec.record(r)
+		w.Header().Set("Content-Type", "application/json")
+		switch {
+		case r.Method == http.MethodGet && apiPath(r) == "/media":
+			io.WriteString(w, `{"success":true,"data":[{"id":"m1"},{"id":"m2"}]}`)
+		case r.Method == http.MethodDelete:
+			w.WriteHeader(http.StatusForbidden)
+			io.WriteString(w, `{"success":false,"error":{"message":"media:delete required"}}`)
+		default:
+			w.WriteHeader(http.StatusNotFound)
+			io.WriteString(w, `{"success":false}`)
+		}
+	}))
+	defer server.Close()
+
+	done := make(chan error, 1)
+	go func() {
+		_, _, err := runClean(t, "--media-only", "--confirm", "--url", server.URL+"/api", "--token", "t")
+		done <- err
+	}()
+	select {
+	case err := <-done:
+		if err == nil || !strings.Contains(err.Error(), "could not be deleted") {
+			t.Fatalf("err = %v, want a media deletion failure", err)
+		}
+	case <-time.After(60 * time.Second):
+		t.Fatal("clean did not stop; it loops on media it cannot delete")
+	}
+	if n := rec.countPath("/media"); n != 1 {
+		t.Errorf("listed media %d times, want 1", n)
+	}
+}
+
+// A backup that cannot list media must fail rather than write an archive without media.
+func TestBackupFailsWhenMediaCannotBeListed(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		switch {
+		case r.Method == http.MethodGet && apiPath(r) == "/collections":
+			io.WriteString(w, testCollectionsResponse)
+		case r.Method == http.MethodGet && strings.HasPrefix(apiPath(r), "/collections/"):
+			io.WriteString(w, `{"success":true,"data":{"documents":[]}}`)
+		case apiPath(r) == "/media":
+			w.WriteHeader(http.StatusForbidden)
+			io.WriteString(w, `{"success":false,"error":{"message":"media:read required"}}`)
+		default:
+			w.WriteHeader(http.StatusNotFound)
+			io.WriteString(w, `{"success":false}`)
+		}
+	}))
+	defer server.Close()
+
+	out := filepath.Join(t.TempDir(), "b.zip")
+	_, _, err := runBackup(t, "--output", out, "--url", server.URL+"/api", "--token", "t")
+	if err == nil || !strings.Contains(err.Error(), "could not list media") {
+		t.Fatalf("err = %v, want the media listing failure", err)
+	}
+}

cmd/login.go

@@ -32,6 +32,13 @@ Example:
 		}
 		setDefault, _ := cmd.Flags().GetBool("set-default")
 
+		// Logging in again to the same instance replaces its session; a different site under
+		// the same name would silently take its place
+		if existing, err := config.GetInstance(name); err == nil && existing != nil &&
+			config.NormalizeBaseURL(existing.URL) != baseURL {
+			return fmt.Errorf("an instance named %q already exists (%s); choose another with --name", name, existing.URL)
+		}
+
 		fmt.Println()
 		fmt.Println("Trokky CLI Login")
 		fmt.Println("────────────────────────────────────────")

cmd/logout.go

@@ -1,9 +1,11 @@
 package cmd
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/spf13/cobra"
+	"github.com/trokky/cli/internal/auth"
 	"github.com/trokky/cli/internal/config"
 )
 
@@ -46,6 +48,14 @@ Example:
 			}
 		}
 
+		// Revoke the sign-in on the instance first, so it also disappears from its Studio's
+		// connected applications; best effort, an unreachable instance must not block logout
+		inst, _ := config.GetInstance(name)
+		var revokeErr error
+		if inst != nil && inst.AuthType == config.AuthTypeOAuth2 && (inst.RefreshToken != "" || inst.Token != "") {
+			revokeErr = auth.RevokeToken(*inst)
+		}
+
 		removed, err := config.RemoveInstance(name)
 		if err != nil {
 			return fmt.Errorf("failed to save config: %w", err)
@@ -55,6 +65,12 @@ Example:
 		}
 
 		fmt.Printf("✓ Logged out from %q\n", name)
+		var refused *auth.RevokeRefusedError
+		if errors.As(revokeErr, &refused) {
+			fmt.Printf("  The instance did not accept the revocation (HTTP %d); revoke the access in its Studio under Account > Connected applications.\n", refused.Status)
+		} else if revokeErr != nil {
+			fmt.Println("  The instance could not be reached to revoke the sign-in; revoke it in its Studio under Account > Connected applications.")
+		}
 		return nil
 	},
 }

cmd/logout_test.go

@@ -0,0 +1,148 @@
+package cmd
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"sync/atomic"
+	"testing"
+
+	"github.com/trokky/cli/internal/config"
+)
+
+func runLogout(t *testing.T, name string) (string, error) {
+	t.Helper()
+	var err error
+	out, _ := captureProcessOutput(t, func() {
+		rootCmd.SetArgs([]string{"logout", name, "--force"})
+		err = rootCmd.Execute()
+	})
+	return out, err
+}
+
+func addTestInstance(t *testing.T, name string, inst config.InstanceConfig) {
+	t.Helper()
+	if err := config.AddInstance(name, inst, false); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestLogout_RevokesABrowserSignInThenForgetsIt(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	var body map[string]string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/api/auth/revoke" {
+			t.Errorf("unexpected request to %s", r.URL.Path)
+		}
+		json.NewDecoder(r.Body).Decode(&body)
+	}))
+	defer server.Close()
+	addTestInstance(t, "site", config.InstanceConfig{URL: server.URL + "/api", Token: "a", RefreshToken: "r", AuthType: config.AuthTypeOAuth2, ClientID: "trokky-cli"})
+
+	out, err := runLogout(t, "site")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if body["token"] != "r" || body["client_id"] != "trokky-cli" {
+		t.Fatalf("revocation sent %v", body)
+	}
+	if strings.Contains(out, "revoke") {
+		t.Fatalf("a successful revocation needs no note: %q", out)
+	}
+	if inst, _ := config.GetInstance("site"); inst != nil {
+		t.Fatal("instance still configured")
+	}
+}
+
+func TestLogout_ForgetsAnUnreachableOrRefusingInstanceAndSaysWhy(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	addTestInstance(t, "gone", config.InstanceConfig{URL: "http://127.0.0.1:1/api", RefreshToken: "r", AuthType: config.AuthTypeOAuth2})
+	out, err := runLogout(t, "gone")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(out, "could not be reached") {
+		t.Fatalf("output %q", out)
+	}
+	if inst, _ := config.GetInstance("gone"); inst != nil {
+		t.Fatal("an unreachable instance must still be forgotten")
+	}
+
+	old := httptest.NewServer(http.NotFoundHandler())
+	defer old.Close()
+	addTestInstance(t, "old", config.InstanceConfig{URL: old.URL + "/api", RefreshToken: "r", AuthType: config.AuthTypeOAuth2})
+	out, err = runLogout(t, "old")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(out, "did not accept the revocation (HTTP 404)") {
+		t.Fatalf("output %q", out)
+	}
+	if inst, _ := config.GetInstance("old"); inst != nil {
+		t.Fatal("a refusing instance must still be forgotten")
+	}
+}
+
+func TestLogout_DoesNotRevokeAnAPIToken(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	var calls atomic.Int32
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { calls.Add(1) }))
+	defer server.Close()
+	addTestInstance(t, "tok", config.InstanceConfig{URL: server.URL + "/api", Token: "api-token", AuthType: config.AuthTypeAPIToken})
+
+	out, err := runLogout(t, "tok")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if calls.Load() != 0 || strings.Contains(out, "revoke") {
+		t.Fatalf("an API token is not an OAuth grant: %d calls, output %q", calls.Load(), out)
+	}
+}
+
+// A browser sign-in that stopped working says how to sign in again
+func TestCommand_Unauthorized_OAuthInstance_HintsAtLogin(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	t.Setenv("TROKKY_URL", "")
+	t.Setenv("TROKKY_TOKEN", "")
+	t.Setenv("TROKKY_INSTANCE", "")
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusUnauthorized)
+		w.Write([]byte(`{"success":false,"error":{"message":"Invalid or expired authentication token"}}`))
+	}))
+	defer server.Close()
+	addTestInstance(t, "site", config.InstanceConfig{URL: server.URL + "/api", Token: "a", AuthType: config.AuthTypeOAuth2})
+
+	var err error
+	captureProcessOutput(t, func() {
+		// rootCmd keeps flags from earlier tests: clear the ones that would override the instance
+		rootCmd.SetArgs([]string{"documents", "list", "posts", "--instance", "site", "--url", "", "--token", "", "-q"})
+		err = rootCmd.Execute()
+	})
+	if err == nil || !strings.Contains(err.Error(), "trokky login "+server.URL+"/api --name site") {
+		t.Fatalf("err = %v", err)
+	}
+}
+
+// An API token does not come from a sign-in, so its 401 is not answered with "trokky login"
+func TestCommand_Unauthorized_APIToken_NoLoginHint(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	t.Setenv("TROKKY_URL", "")
+	t.Setenv("TROKKY_TOKEN", "")
+	t.Setenv("TROKKY_INSTANCE", "")
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusUnauthorized)
+		w.Write([]byte(`{"success":false,"error":{"message":"Invalid API token"}}`))
+	}))
+	defer server.Close()
+	addTestInstance(t, "tok", config.InstanceConfig{URL: server.URL + "/api", Token: "t", AuthType: config.AuthTypeAPIToken})
+
+	var err error
+	captureProcessOutput(t, func() {
+		rootCmd.SetArgs([]string{"documents", "list", "posts", "--instance", "tok", "--url", "", "--token", "", "-q"})
+		err = rootCmd.Execute()
+	})
+	if err == nil || !strings.Contains(err.Error(), "Invalid API token") || strings.Contains(err.Error(), "trokky login") {
+		t.Fatalf("err = %v", err)
+	}
+}

cmd/main_test.go

@@ -0,0 +1,21 @@
+package cmd
+
+import (
+	"os"
+	"testing"
+)
+
+// Every test in this package runs against a throwaway HOME. RefreshAccessToken reads and
+// writes ~/.trokky/config.yaml under a lock, and a test that forgot overrideHome once
+// rewrote a developer's real config; this makes forgetting harmless.
+func TestMain(m *testing.M) {
+	home, err := os.MkdirTemp("", "trokky-cmd-test-home-")
+	if err != nil {
+		panic(err)
+	}
+	os.Setenv("HOME", home)
+	os.Setenv("USERPROFILE", home)
+	code := m.Run()
+	os.RemoveAll(home)
+	os.Exit(code)
+}

cmd/restore.go

@@ -229,6 +229,7 @@ Example:
 				if json.Unmarshal(mediaData, &mediaItems) != nil || len(mediaItems) == 0 {
 					break
 				}
+				deletedBefore := deletedMedia
 				for _, item := range mediaItems {
 					for attempt := 1; attempt <= 3; attempt++ {
 						_, err := c.Delete("/media/" + item.ID)
@@ -243,6 +244,13 @@ Example:
 					time.Sleep(100 * time.Millisecond)
 				}
 				fmt.Fprintf(out, "\r  Cleaning existing media... %d deleted", deletedMedia)
+
+				// The listing always returns the first page: if nothing on it could be deleted,
+				// it would come back unchanged forever
+				if deletedMedia == deletedBefore {
+					fmt.Fprintln(out)
+					return fmt.Errorf("could not delete existing media (%d file(s) refused); the token may lack media:delete: run 'trokky login' again with a current CLI, or restore without --clean", len(mediaItems))
+				}
 			}
 			fmt.Fprintf(out, "\r  Cleaning existing media... %d file(s) deleted\n", deletedMedia)
 		}