Merge pull request #27 from aaearon/feat/verbose-logging-all-commands

feat: add verbose logging to all commands

Author: aaearon

CHANGELOG.md

@@ -4,6 +4,16 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Added
+
+- Verbose logging (`--verbose`/`-v`) now produces output for all commands, not just those using `SCAAccessService`
+- Commands `update`, `version`, `login`, `logout`, `configure`, and `favorites` now emit SDK-format verbose logs (`grant | timestamp | INFO | message`)
+
+### Changed
+
+- Migrated 4 ad-hoc `[verbose]`/`Warning:` messages in `fetchStatusData`, `buildDirectoryNameMap`, `buildWorkspaceNameMap`, and `fetchEligibility` to use the SDK logger for consistent format
+- Removed `errWriter io.Writer` parameter from `fetchStatusData`, `buildDirectoryNameMap`, `buildWorkspaceNameMap`, and `fetchGroupsEligibility` (verbose output now goes through SDK logger, not injected writer)
+
 ## [0.4.0] - 2026-02-19
 
 ### Added

CLAUDE.md

@@ -77,6 +77,8 @@ Custom `SCAAccessService` follows SDK conventions:
 ## Verbose / Logging
 - `--verbose` / `-v` global flag wired via `PersistentPreRunE` in `cmd/root.go`
 - Calls `config.EnableVerboseLogging("INFO")` (sets `IDSEC_LOG_LEVEL=INFO`) or `config.DisableVerboseLogging()` (sets `IDSEC_LOG_LEVEL=CRITICAL`)
+- `cmdLogger` interface in `cmd/verbose.go` — `Info(msg string, v ...interface{})`, satisfied by `*common.IdsecLogger`
+- `log` package-level var in `cmd/verbose.go` — all commands use `log.Info(...)` for verbose output; tests swap with `spyLogger`
 - `loggingClient` in `internal/sca/logging_client.go` decorates `httpClient`, logging method/route/status/duration at INFO, response headers at DEBUG with Authorization redaction
 - `NewSCAAccessService()` wraps ISP client with `loggingClient` using `common.GetLogger("grant", -1)` (dynamic level from env)
 - `NewSCAAccessServiceWithClient()` (test constructor) does not wrap — tests don't need logging

cmd/configure.go

@@ -104,6 +104,7 @@ func runConfigure(cmd *cobra.Command, saver profileSaver, tenantURL, username st
 	}
 
 	// Save SDK profile
+	log.Info("Saving profile...")
 	if err := saver.SaveProfile(profile); err != nil {
 		return fmt.Errorf("failed to save profile: %w", err)
 	}
@@ -124,6 +125,7 @@ func runConfigure(cmd *cobra.Command, saver profileSaver, tenantURL, username st
 	}
 
 	// Save app config
+	log.Info("Saving config...")
 	cfgPath, err := config.ConfigPath()
 	if err != nil {
 		return err

cmd/configure_test.go

@@ -248,6 +248,41 @@ func TestConfigureCommand(t *testing.T) {
 	}
 }
 
+func TestConfigureCommand_VerboseLogs(t *testing.T) {
+	spy := &spyLogger{}
+	oldLog := log
+	log = spy
+	defer func() { log = oldLog }()
+
+	tmpDir := t.TempDir()
+	cfgPath := filepath.Join(tmpDir, "config.yaml")
+	t.Setenv("GRANT_CONFIG", cfgPath)
+
+	cmd := NewConfigureCommandWithDeps(
+		&mockProfileSaver{},
+		"https://example.cyberark.cloud",
+		"test.user@example.com",
+	)
+	_, err := executeCommand(cmd)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	wantMessages := []string{"Saving profile", "Saving config"}
+	for _, want := range wantMessages {
+		found := false
+		for _, msg := range spy.messages {
+			if strings.Contains(msg, want) {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Errorf("expected log containing %q, got: %v", want, spy.messages)
+		}
+	}
+}
+
 func TestConfigureCommandIntegration(t *testing.T) {
 	// Test that configure command is properly registered
 	rootCmd := newTestRootCommand()

cmd/favorites.go

@@ -188,6 +188,9 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi
 
 	// Non-interactive mode requires name upfront
 	isNonInteractive := (target != "" && role != "") || (favType == config.FavoriteTypeGroups && group != "")
+	if isNonInteractive {
+		log.Info("Non-interactive mode: target=%q role=%q provider=%q group=%q", target, role, provider, group)
+	}
 	if isNonInteractive && name == "" {
 		if favType == config.FavoriteTypeGroups {
 			return fmt.Errorf("name is required when using --group flag\n\nUsage:\n  grant favorites add <name> --type groups --group <group>")
@@ -248,7 +251,7 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi
 
 		// Fetch groups eligibility (best-effort, enriched with directory names)
 		if groupsElig != nil {
-			groups, gErr := fetchGroupsEligibility(ctx, groupsElig, eligLister, cmd.ErrOrStderr())
+			groups, gErr := fetchGroupsEligibility(ctx, groupsElig, eligLister)
 			if gErr == nil {
 				for i := range groups {
 					items = append(items, selectionItem{kind: selectionGroup, group: &groups[i]})
@@ -293,6 +296,7 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi
 		}
 	}
 
+	log.Info("Saving favorite %q...", name)
 	if err := config.AddFavorite(cfg, name, fav); err != nil {
 		return fmt.Errorf("failed to add favorite: %w", err)
 	}
@@ -322,7 +326,7 @@ func addGroupFavorite(cmd *cobra.Command, name, group string, cfg *config.Config
 		ctx, cancel := context.WithTimeout(context.Background(), apiTimeout)
 		defer cancel()
 
-		groups, err := fetchGroupsEligibility(ctx, groupsElig, eligLister, cmd.ErrOrStderr())
+		groups, err := fetchGroupsEligibility(ctx, groupsElig, eligLister)
 		if err != nil {
 			return err
 		}
@@ -392,13 +396,15 @@ func newFavoritesRemoveCommand() *cobra.Command {
 }
 
 func runFavoritesList(cmd *cobra.Command, args []string) error {
+	log.Info("Loading config...")
 	cfg, _, err := config.LoadDefaultWithPath()
 	if err != nil {
 		return err
 	}
 
 	// List favorites
 	favorites := config.ListFavorites(cfg)
+	log.Info("Found %d favorite(s)", len(favorites))
 	if len(favorites) == 0 {
 		fmt.Fprintln(cmd.OutOrStdout(), "No favorites saved. Run 'grant favorites add' to create one.")
 		return nil
@@ -418,17 +424,20 @@ func runFavoritesList(cmd *cobra.Command, args []string) error {
 func runFavoritesRemove(cmd *cobra.Command, args []string) error {
 	name := args[0]
 
+	log.Info("Loading config...")
 	cfg, cfgPath, err := config.LoadDefaultWithPath()
 	if err != nil {
 		return err
 	}
 
 	// Remove favorite
+	log.Info("Removing favorite %q", name)
 	if err := config.RemoveFavorite(cfg, name); err != nil {
 		return err
 	}
 
 	// Save config
+	log.Info("Saving config...")
 	if err := config.Save(cfg, cfgPath); err != nil {
 		return fmt.Errorf("failed to save config: %w", err)
 	}

cmd/favorites_test.go

@@ -89,6 +89,125 @@ func TestFavoritesListCommand(t *testing.T) {
 	}
 }
 
+func TestFavoritesList_VerboseLogs(t *testing.T) {
+	spy := &spyLogger{}
+	oldLog := log
+	log = spy
+	defer func() { log = oldLog }()
+
+	tmpDir := t.TempDir()
+	configPath := filepath.Join(tmpDir, "config.yaml")
+	t.Setenv("GRANT_CONFIG", configPath)
+
+	cfg := config.DefaultConfig()
+	_ = config.AddFavorite(cfg, "dev", config.Favorite{
+		Provider: "azure",
+		Target:   "sub-123",
+		Role:     "Contributor",
+	})
+	_ = config.AddFavorite(cfg, "prod", config.Favorite{
+		Provider: "azure",
+		Target:   "sub-456",
+		Role:     "Reader",
+	})
+	_ = config.Save(cfg, configPath)
+
+	rootCmd := newTestRootCommand()
+	rootCmd.AddCommand(NewFavoritesCommand())
+	_, err := executeCommand(rootCmd, "favorites", "list")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	wantMessages := []string{"Loading config", "2 favorite"}
+	for _, want := range wantMessages {
+		found := false
+		for _, msg := range spy.messages {
+			if strings.Contains(msg, want) {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Errorf("expected log containing %q, got: %v", want, spy.messages)
+		}
+	}
+}
+
+func TestFavoritesRemove_VerboseLogs(t *testing.T) {
+	spy := &spyLogger{}
+	oldLog := log
+	log = spy
+	defer func() { log = oldLog }()
+
+	tmpDir := t.TempDir()
+	configPath := filepath.Join(tmpDir, "config.yaml")
+	t.Setenv("GRANT_CONFIG", configPath)
+
+	cfg := config.DefaultConfig()
+	_ = config.AddFavorite(cfg, "dev", config.Favorite{
+		Provider: "azure",
+		Target:   "sub-123",
+		Role:     "Contributor",
+	})
+	_ = config.Save(cfg, configPath)
+
+	rootCmd := newTestRootCommand()
+	rootCmd.AddCommand(NewFavoritesCommand())
+	_, err := executeCommand(rootCmd, "favorites", "remove", "dev")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	wantMessages := []string{"Loading config", "Removing favorite", "Saving config"}
+	for _, want := range wantMessages {
+		found := false
+		for _, msg := range spy.messages {
+			if strings.Contains(msg, want) {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Errorf("expected log containing %q, got: %v", want, spy.messages)
+		}
+	}
+}
+
+func TestFavoritesAddNonInteractive_VerboseLogs(t *testing.T) {
+	spy := &spyLogger{}
+	oldLog := log
+	log = spy
+	defer func() { log = oldLog }()
+
+	tmpDir := t.TempDir()
+	configPath := filepath.Join(tmpDir, "config.yaml")
+	t.Setenv("GRANT_CONFIG", configPath)
+	cfg := config.DefaultConfig()
+	_ = config.Save(cfg, configPath)
+
+	rootCmd := newTestRootCommand()
+	rootCmd.AddCommand(NewFavoritesCommand())
+	_, err := executeCommand(rootCmd, "favorites", "add", "myfav", "--target", "sub-123", "--role", "Contributor")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	wantMessages := []string{"Non-interactive mode", "Saving"}
+	for _, want := range wantMessages {
+		found := false
+		for _, msg := range spy.messages {
+			if strings.Contains(msg, want) {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Errorf("expected log containing %q, got: %v", want, spy.messages)
+		}
+	}
+}
+
 func TestFavoritesRemoveCommand(t *testing.T) {
 	tests := []struct {
 		name        string

cmd/helpers.go

@@ -3,7 +3,6 @@ package cmd
 import (
 	"context"
 	"fmt"
-	"io"
 	"strings"
 	"sync"
 
@@ -18,13 +17,12 @@ type statusData struct {
 
 // fetchStatusData fires sessions and all-CSP eligibility calls concurrently,
 // then joins results. A sessions error is fatal; eligibility errors are
-// gracefully degraded (empty nameMap entry, verbose warning).
+// gracefully degraded (empty nameMap entry, verbose warning via SDK logger).
 func fetchStatusData(
 	ctx context.Context,
 	sessionLister sessionLister,
 	eligLister eligibilityLister,
 	cspFilter *scamodels.CSP,
-	errWriter io.Writer,
 ) (*statusData, error) {
 	type eligResult struct {
 		csp     scamodels.CSP
@@ -76,9 +74,7 @@ func fetchStatusData(
 	nameMap := make(map[string]string)
 	for r := range eligResults {
 		if r.err != nil {
-			if verbose {
-				fmt.Fprintf(errWriter, "Warning: failed to fetch names for %s: %v\n", r.csp, r.err)
-			}
+			log.Info("failed to fetch names for %s: %v", r.csp, r.err)
 			continue
 		}
 		for _, t := range r.targets {
@@ -99,17 +95,17 @@ func fetchStatusData(
 // buildDirectoryNameMap fetches Azure cloud eligibility to resolve directoryId -> name.
 // It looks for DIRECTORY-type workspace entries whose workspaceId matches a directory ID.
 // Falls back to organizationId -> first workspace name if no DIRECTORY entries exist.
-// Errors are silently ignored (graceful degradation — groups display without directory context).
-func buildDirectoryNameMap(ctx context.Context, eligLister eligibilityLister, errWriter io.Writer) map[string]string {
+// Errors are logged via SDK logger (graceful degradation — groups display without directory context).
+func buildDirectoryNameMap(ctx context.Context, eligLister eligibilityLister) map[string]string {
 	nameMap := make(map[string]string)
 	if eligLister == nil {
 		return nameMap
 	}
 
 	resp, err := eligLister.ListEligibility(ctx, scamodels.CSPAzure)
 	if err != nil || resp == nil {
-		if verbose && err != nil {
-			fmt.Fprintf(errWriter, "Warning: failed to resolve directory names: %v\n", err)
+		if err != nil {
+			log.Info("failed to resolve directory names: %v", err)
 		}
 		return nameMap
 	}
@@ -136,9 +132,9 @@ func buildDirectoryNameMap(ctx context.Context, eligLister eligibilityLister, er
 
 // buildWorkspaceNameMap fetches eligibility for each unique CSP in sessions
 // concurrently and builds a workspaceID -> workspaceName map. Errors are
-// silently ignored (graceful degradation — the raw workspace ID is shown
+// logged via SDK logger (graceful degradation — the raw workspace ID is shown
 // as fallback).
-func buildWorkspaceNameMap(ctx context.Context, eligLister eligibilityLister, sessions []scamodels.SessionInfo, errWriter io.Writer) map[string]string {
+func buildWorkspaceNameMap(ctx context.Context, eligLister eligibilityLister, sessions []scamodels.SessionInfo) map[string]string {
 	nameMap := make(map[string]string)
 
 	// Collect unique CSPs
@@ -178,9 +174,7 @@ func buildWorkspaceNameMap(ctx context.Context, eligLister eligibilityLister, se
 
 	for r := range results {
 		if r.err != nil {
-			if verbose {
-				fmt.Fprintf(errWriter, "Warning: failed to fetch names for %s: %v\n", r.csp, r.err)
-			}
+			log.Info("failed to fetch names for %s: %v", r.csp, r.err)
 			continue
 		}
 		for _, t := range r.targets {