You can close this window.
") - shutdownOnce.Do(func() { //nolint:contextcheck // decoupled from outer ctx intentionally - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - go func() { defer cancel(); server.Shutdown(ctx) }() //nolint:errcheck // best-effort shutdown - }) - return - } - - if state != expectedState { - errCh <- fmt.Errorf("state mismatch: CSRF protection failed") - fmt.Fprint(w, "State mismatch.
") - shutdownOnce.Do(func() { //nolint:contextcheck // decoupled from outer ctx intentionally - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - go func() { defer cancel(); server.Shutdown(ctx) }() //nolint:errcheck // best-effort shutdown - }) - return - } - - codeCh <- code - fmt.Fprint(w, "You can close this window.
") - shutdownOnce.Do(func() { //nolint:contextcheck // decoupled from outer ctx intentionally - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - go func() { defer cancel(); server.Shutdown(ctx) }() //nolint:errcheck // best-effort shutdown - }) - }) - - go server.Serve(listener) //nolint:errcheck // server.Serve returns ErrServerClosed on Shutdown - - // Try to open browser automatically unless --no-browser was specified - if opts.BrowserLauncher != nil { - if err := opts.BrowserLauncher(authURL); err != nil { - // Fall back to printing URL if browser open fails - opts.log("\nCouldn't open browser automatically.\nOpen this URL in your browser:\n" + authURL + "\n\nWaiting for authentication...") - } else { - opts.log("\nOpening browser for authentication...") - opts.log("If the browser doesn't open, visit: " + authURL + "\n\nWaiting for authentication...") - } - } else { - opts.log("\nOpen this URL in your browser:\n" + authURL + "\n\nWaiting for authentication...") - } - - // Wait for callback or timeout - select { - case code := <-codeCh: - return code, nil - case err := <-errCh: - return "", err - case <-ctx.Done(): - return "", ctx.Err() - case <-time.After(5 * time.Minute): - return "", fmt.Errorf("authentication timeout waiting for callback on %s", listenAddr) - } -} - func (m *Manager) exchangeCode(ctx context.Context, cfg *oauth.Config, oauthType, code, codeVerifier string, clientCreds *ClientCredentials, opts *LoginOptions) (*Credentials, error) { exchanger := oauth.NewExchanger(m.httpClient) @@ -670,29 +611,6 @@ func (m *Manager) exchangeCode(ctx context.Context, cfg *oauth.Config, oauthType return creds, nil } -// PKCE helpers - -func generateCodeVerifier() string { - b := make([]byte, 32) - if _, err := rand.Read(b); err != nil { - panic("crypto/rand failed: " + err.Error()) - } - return base64.RawURLEncoding.EncodeToString(b) -} - -func generateCodeChallenge(verifier string) string { - h := sha256.Sum256([]byte(verifier)) - return base64.RawURLEncoding.EncodeToString(h[:]) -} - -func generateState() string { - b := make([]byte, 16) - if _, err := rand.Read(b); err != nil { - panic("crypto/rand failed: " + err.Error()) - } - return base64.RawURLEncoding.EncodeToString(b) -} - // openBrowser opens the specified URL in the default browser. func openBrowser(url string) error { return hostutil.OpenBrowser(url) diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 5fd923c0a..0ab252690 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -2,8 +2,6 @@ package auth import ( "context" - "crypto/sha256" - "encoding/base64" "encoding/json" "fmt" "io" @@ -21,6 +19,13 @@ import ( "github.com/basecamp/basecamp-cli/internal/config" ) +// newTestStore creates a file-backed credential store for testing. +func newTestStore(t *testing.T, dir string) *Store { + t.Helper() + t.Setenv("BASECAMP_NO_KEYRING", "1") + return NewStore(dir) +} + func TestNewStore(t *testing.T) { tmpDir := t.TempDir() store := NewStore(tmpDir) @@ -31,9 +36,7 @@ func TestNewStore(t *testing.T) { func TestStoreFileBackend(t *testing.T) { tmpDir := t.TempDir() - - // Force file backend by creating store with useKeyring=false - store := &Store{useKeyring: false, fallbackDir: tmpDir} + store := newTestStore(t, tmpDir) origin := "https://test.example.com" creds := &Credentials{ @@ -49,13 +52,6 @@ func TestStoreFileBackend(t *testing.T) { err := store.Save(origin, creds) require.NoError(t, err, "Save failed") - // Verify file was created with correct permissions - credFile := filepath.Join(tmpDir, "credentials.json") - info, err := os.Stat(credFile) - require.NoError(t, err, "Credentials file not created") - perms := info.Mode().Perm() - assert.Equal(t, os.FileMode(0600), perms, "File permissions mismatch") - // Load credentials loaded, err := store.Load(origin) require.NoError(t, err, "Load failed") @@ -71,7 +67,7 @@ func TestStoreFileBackend(t *testing.T) { func TestStoreMultipleOrigins(t *testing.T) { tmpDir := t.TempDir() - store := &Store{useKeyring: false, fallbackDir: tmpDir} + store := newTestStore(t, tmpDir) // Save credentials for two different origins origin1 := "https://origin1.example.com" @@ -95,7 +91,7 @@ func TestStoreMultipleOrigins(t *testing.T) { func TestStoreDelete(t *testing.T) { tmpDir := t.TempDir() - store := &Store{useKeyring: false, fallbackDir: tmpDir} + store := newTestStore(t, tmpDir) origin := "https://delete-test.example.com" creds := &Credentials{AccessToken: "to-be-deleted", ExpiresAt: time.Now().Unix() + 3600} @@ -111,77 +107,13 @@ func TestStoreDelete(t *testing.T) { func TestStoreLoadMissing(t *testing.T) { tmpDir := t.TempDir() - store := &Store{useKeyring: false, fallbackDir: tmpDir} + store := newTestStore(t, tmpDir) // Load non-existent origin should fail _, err := store.Load("https://nonexistent.example.com") assert.Error(t, err, "Load should fail for non-existent origin") } -func TestKeyFunction(t *testing.T) { - tests := []struct { - origin string - expected string - }{ - {"https://3.basecampapi.com", "basecamp::https://3.basecampapi.com"}, - {"http://localhost:3000", "basecamp::http://localhost:3000"}, - {"https://custom.example.com", "basecamp::https://custom.example.com"}, - } - - for _, tt := range tests { - t.Run(tt.origin, func(t *testing.T) { - result := key(tt.origin) - assert.Equal(t, tt.expected, result) - }) - } -} - -func TestGenerateCodeVerifier(t *testing.T) { - // Generate multiple verifiers to check they're unique - verifiers := make(map[string]bool) - for range 10 { - v := generateCodeVerifier() - - // Should be base64url encoded (no padding) - assert.NotEmpty(t, v, "generateCodeVerifier returned empty string") - - // Check uniqueness - assert.False(t, verifiers[v], "generateCodeVerifier produced duplicate: %s", v) - verifiers[v] = true - - // Should be ~43 characters (32 bytes base64url encoded) - assert.True(t, len(v) >= 40 && len(v) <= 50, "generateCodeVerifier length = %d, expected ~43", len(v)) - } -} - -func TestGenerateCodeChallenge(t *testing.T) { - verifier := "test_code_verifier_12345" - - challenge := generateCodeChallenge(verifier) - - // Manually compute expected challenge - h := sha256.Sum256([]byte(verifier)) - expected := base64.RawURLEncoding.EncodeToString(h[:]) - - assert.Equal(t, expected, challenge) -} - -func TestGenerateState(t *testing.T) { - // Generate multiple states to check they're unique - states := make(map[string]bool) - for range 10 { - s := generateState() - - assert.NotEmpty(t, s, "generateState returned empty string") - - assert.False(t, states[s], "generateState produced duplicate: %s", s) - states[s] = true - - // Should be ~22 characters (16 bytes base64url encoded) - assert.True(t, len(s) >= 20 && len(s) <= 25, "generateState length = %d, expected ~22", len(s)) - } -} - func TestNewManager(t *testing.T) { cfg := &config.Config{ BaseURL: "https://3.basecampapi.com", @@ -213,7 +145,7 @@ func TestIsAuthenticatedWithEnvToken(t *testing.T) { } manager := NewManager(cfg, http.DefaultClient) // Use file backend with empty temp dir to ensure no stored creds - manager.store = &Store{useKeyring: false, fallbackDir: tmpDir} + manager.store = newTestStore(t, tmpDir) // Without env token os.Unsetenv("BASECAMP_TOKEN") @@ -243,7 +175,7 @@ func TestIsAuthenticatedWithStoredCreds(t *testing.T) { BaseURL: "https://3.basecampapi.com", } manager := NewManager(cfg, http.DefaultClient) - manager.store = &Store{useKeyring: false, fallbackDir: tmpDir} + manager.store = newTestStore(t, tmpDir) // Without stored creds assert.False(t, manager.IsAuthenticated(), "Should not be authenticated without stored credentials") @@ -267,7 +199,7 @@ func TestGetUserID(t *testing.T) { BaseURL: "https://3.basecampapi.com", } manager := NewManager(cfg, http.DefaultClient) - manager.store = &Store{useKeyring: false, fallbackDir: tmpDir} + manager.store = newTestStore(t, tmpDir) // Save credentials with user ID creds := &Credentials{ @@ -288,7 +220,7 @@ func TestSetUserID(t *testing.T) { BaseURL: "https://3.basecampapi.com", } manager := NewManager(cfg, http.DefaultClient) - manager.store = &Store{useKeyring: false, fallbackDir: tmpDir} + manager.store = newTestStore(t, tmpDir) // Save initial credentials creds := &Credentials{ @@ -314,7 +246,7 @@ func TestLogout(t *testing.T) { BaseURL: "https://3.basecampapi.com", } manager := NewManager(cfg, http.DefaultClient) - manager.store = &Store{useKeyring: false, fallbackDir: tmpDir} + manager.store = newTestStore(t, tmpDir) // Save credentials creds := &Credentials{ @@ -396,11 +328,11 @@ func TestClientCredentialsJSON(t *testing.T) { } func TestUsingKeyring(t *testing.T) { - store := &Store{useKeyring: true, fallbackDir: "/tmp"} - assert.True(t, store.UsingKeyring(), "UsingKeyring() should be true") + tmpDir := t.TempDir() - store = &Store{useKeyring: false, fallbackDir: "/tmp"} - assert.False(t, store.UsingKeyring(), "UsingKeyring() should be false") + // With keyring disabled, UsingKeyring returns false + store := newTestStore(t, tmpDir) + assert.False(t, store.UsingKeyring(), "UsingKeyring() should be false when BASECAMP_NO_KEYRING is set") } func TestLaunchpadURL_InsecureRejected(t *testing.T) { @@ -649,7 +581,7 @@ func TestRegisterBC3Client_UsesResolvedRedirectURI(t *testing.T) { m := &Manager{ cfg: config.Default(), httpClient: srv.Client(), - store: &Store{useKeyring: false, fallbackDir: tmpDir}, + store: newTestStore(t, tmpDir), } opts := &LoginOptions{RedirectURI: "http://localhost:7777/cb"} @@ -677,7 +609,7 @@ func TestRegisterBC3Client_CustomRedirectNotPersisted(t *testing.T) { m := &Manager{ cfg: config.Default(), httpClient: srv.Client(), - store: &Store{useKeyring: false, fallbackDir: tmpDir}, + store: newTestStore(t, tmpDir), } opts := &LoginOptions{RedirectURI: "http://localhost:7777/cb"} @@ -704,7 +636,7 @@ func TestRegisterBC3Client_DefaultRedirectPersisted(t *testing.T) { m := &Manager{ cfg: config.Default(), httpClient: srv.Client(), - store: &Store{useKeyring: false, fallbackDir: tmpDir}, + store: newTestStore(t, tmpDir), } opts := &LoginOptions{RedirectURI: defaultRedirectURI} @@ -731,7 +663,7 @@ func TestLoadClientCredentials_BC3_CustomRedirect_SkipsStoredClient(t *testing.T m := &Manager{ cfg: config.Default(), httpClient: srv.Client(), - store: &Store{useKeyring: false, fallbackDir: tmpDir}, + store: newTestStore(t, tmpDir), } // Pre-populate client.json @@ -747,9 +679,9 @@ func TestLoadClientCredentials_BC3_CustomRedirect_SkipsStoredClient(t *testing.T assert.Equal(t, "dcr-fresh", creds.ClientID, "should use DCR result, not stored client") } -func TestAtomicCredentialWrite_OverwriteExisting(t *testing.T) { +func TestCredentialWrite_OverwriteExisting(t *testing.T) { tmpDir := t.TempDir() - store := &Store{useKeyring: false, fallbackDir: tmpDir} + store := newTestStore(t, tmpDir) origin := "https://test.example.com" // Write initial credentials @@ -761,26 +693,18 @@ func TestAtomicCredentialWrite_OverwriteExisting(t *testing.T) { } require.NoError(t, store.Save(origin, creds1)) - // Overwrite with new credentials (exercises the Windows pre-remove path) + // Overwrite with new credentials creds2 := &Credentials{ AccessToken: "token-2", RefreshToken: "refresh-2", ExpiresAt: time.Now().Unix() + 7200, OAuthType: "bc3", } - require.NoError(t, store.Save(origin, creds2), "overwrite of existing credential file must succeed") + require.NoError(t, store.Save(origin, creds2), "overwrite of existing credential must succeed") // Verify the new value persists loaded, err := store.Load(origin) require.NoError(t, err) assert.Equal(t, "token-2", loaded.AccessToken) assert.Equal(t, "bc3", loaded.OAuthType) - - // Verify no stale temp files left behind - entries, err := os.ReadDir(tmpDir) - require.NoError(t, err) - for _, e := range entries { - assert.False(t, filepath.Ext(e.Name()) == ".tmp", - "stale temp file left behind: %s", e.Name()) - } } diff --git a/internal/auth/keyring.go b/internal/auth/keyring.go index 745783c3e..afd30077a 100644 --- a/internal/auth/keyring.go +++ b/internal/auth/keyring.go @@ -4,14 +4,8 @@ import ( "encoding/json" "fmt" "os" - "path/filepath" - "runtime" - "github.com/zalando/go-keyring" -) - -const ( - serviceName = "basecamp" + "github.com/basecamp/cli/credstore" ) // Credentials holds OAuth tokens and metadata. @@ -25,207 +19,51 @@ type Credentials struct { UserID string `json:"user_id,omitempty"` } -// Store handles credential storage, preferring system keychain. +// Store wraps credstore.Store with typed Credentials marshaling. type Store struct { - useKeyring bool - fallbackDir string + inner *credstore.Store } // NewStore creates a credential store. func NewStore(fallbackDir string) *Store { - // Skip keyring for tests or when explicitly disabled - if os.Getenv("BASECAMP_NO_KEYRING") != "" { - return &Store{useKeyring: false, fallbackDir: fallbackDir} - } - - // Test if keyring is available - testKey := "basecamp::test" - err := keyring.Set(serviceName, testKey, "test") - if err == nil { - _ = keyring.Delete(serviceName, testKey) // Best-effort cleanup - return &Store{useKeyring: true, fallbackDir: fallbackDir} + s := credstore.NewStore(credstore.StoreOptions{ + ServiceName: "basecamp", + DisableEnvVar: "BASECAMP_NO_KEYRING", + FallbackDir: fallbackDir, + }) + if w := s.FallbackWarning(); w != "" { + fmt.Fprintf(os.Stderr, "warning: %s\n", w) } - fmt.Fprintf(os.Stderr, "warning: system keyring unavailable, credentials stored in plaintext at %s\n", - filepath.Join(fallbackDir, "credentials.json")) - return &Store{useKeyring: false, fallbackDir: fallbackDir} -} - -// key returns the keyring key for an origin. -func key(origin string) string { - return fmt.Sprintf("basecamp::%s", origin) + return &Store{inner: s} } // Load retrieves credentials for the given origin. func (s *Store) Load(origin string) (*Credentials, error) { - if s.useKeyring { - return s.loadFromKeyring(origin) - } - return s.loadFromFile(origin) -} - -// Save stores credentials for the given origin. -func (s *Store) Save(origin string, creds *Credentials) error { - if s.useKeyring { - return s.saveToKeyring(origin, creds) - } - return s.saveToFile(origin, creds) -} - -// Delete removes credentials for the given origin. -func (s *Store) Delete(origin string) error { - if s.useKeyring { - return keyring.Delete(serviceName, key(origin)) - } - return s.deleteFile(origin) -} - -// Keyring methods - -func (s *Store) loadFromKeyring(origin string) (*Credentials, error) { - data, err := keyring.Get(serviceName, key(origin)) + data, err := s.inner.Load(origin) if err != nil { - return nil, fmt.Errorf("credentials not found: %w", err) + return nil, err } - var creds Credentials - if err := json.Unmarshal([]byte(data), &creds); err != nil { + if err := json.Unmarshal(data, &creds); err != nil { return nil, fmt.Errorf("invalid credentials: %w", err) } return &creds, nil } -func (s *Store) saveToKeyring(origin string, creds *Credentials) error { +// Save stores credentials for the given origin. +func (s *Store) Save(origin string, creds *Credentials) error { data, err := json.Marshal(creds) if err != nil { return err } - return keyring.Set(serviceName, key(origin), string(data)) -} - -// File fallback methods - -func (s *Store) credentialsPath() string { - return filepath.Join(s.fallbackDir, "credentials.json") + return s.inner.Save(origin, data) } -func (s *Store) loadAllFromFile() (map[string]*Credentials, error) { - data, err := os.ReadFile(s.credentialsPath()) - if err != nil { - if os.IsNotExist(err) { - return make(map[string]*Credentials), nil - } - return nil, err - } - - var all map[string]*Credentials - if err := json.Unmarshal(data, &all); err != nil { - return nil, err - } - return all, nil -} - -func (s *Store) saveAllToFile(all map[string]*Credentials) error { - if err := os.MkdirAll(s.fallbackDir, 0700); err != nil { - return err - } - - data, err := json.MarshalIndent(all, "", " ") - if err != nil { - return err - } - - // Atomic write with randomized temp file name - tmpFile, err := os.CreateTemp(s.fallbackDir, "credentials-*.json.tmp") - if err != nil { - return err - } - tmpPath := tmpFile.Name() - - if _, err := tmpFile.Write(data); err != nil { - tmpFile.Close() - os.Remove(tmpPath) - return err - } - if err := tmpFile.Chmod(0600); err != nil { - tmpFile.Close() - os.Remove(tmpPath) - return err - } - if err := tmpFile.Close(); err != nil { - os.Remove(tmpPath) - return err - } - // Unix: rename atomically replaces the destination. - // Windows: rename fails when destination exists. Try rename first to - // preserve the old file on unrelated errors; only remove+retry on failure. - destPath := s.credentialsPath() - if err := os.Rename(tmpPath, destPath); err != nil { - if runtime.GOOS == "windows" { - _ = os.Remove(destPath) - return os.Rename(tmpPath, destPath) - } - os.Remove(tmpPath) // Clean up stale temp on failure - return err - } - return nil -} - -func (s *Store) loadFromFile(origin string) (*Credentials, error) { - all, err := s.loadAllFromFile() - if err != nil { - return nil, err - } - - creds, ok := all[origin] - if !ok { - return nil, fmt.Errorf("credentials not found for %s", origin) - } - return creds, nil -} - -func (s *Store) saveToFile(origin string, creds *Credentials) error { - all, err := s.loadAllFromFile() - if err != nil { - return err - } - - all[origin] = creds - return s.saveAllToFile(all) -} - -func (s *Store) deleteFile(origin string) error { - all, err := s.loadAllFromFile() - if err != nil { - return err - } - - delete(all, origin) - return s.saveAllToFile(all) -} +// Delete removes credentials for the given origin. +func (s *Store) Delete(origin string) error { return s.inner.Delete(origin) } // MigrateToKeyring migrates credentials from file to keyring. -func (s *Store) MigrateToKeyring() error { - if !s.useKeyring { - return nil // Keyring not available - } - - all, err := s.loadAllFromFile() - if err != nil { - return nil //nolint:nilerr // No file to migrate is not an error - } - - for origin, creds := range all { - if err := s.saveToKeyring(origin, creds); err != nil { - return fmt.Errorf("failed to migrate %s: %w", origin, err) - } - } - - // Remove the plaintext file after successful migration - _ = os.Remove(s.credentialsPath()) // Best-effort cleanup - return nil -} +func (s *Store) MigrateToKeyring() error { return s.inner.MigrateToKeyring() } // UsingKeyring returns true if the store is using the system keyring. -func (s *Store) UsingKeyring() bool { - return s.useKeyring -} +func (s *Store) UsingKeyring() bool { return s.inner.UsingKeyring() } diff --git a/internal/output/codes.go b/internal/output/codes.go index 11d9cade0..44fbe1e47 100644 --- a/internal/output/codes.go +++ b/internal/output/codes.go @@ -1,51 +1,32 @@ // Package output provides JSON/Markdown output formatting and error handling. package output -// Exit codes matching the Bash implementation. +import clioutput "github.com/basecamp/cli/output" + +// Exit codes matching the Bash implementation (re-exported from shared module). const ( - ExitOK = 0 // Success - ExitUsage = 1 // Invalid arguments or flags - ExitNotFound = 2 // Resource not found - ExitAuth = 3 // Not authenticated - ExitForbidden = 4 // Access denied (scope issue) - ExitRateLimit = 5 // Rate limited (429) - ExitNetwork = 6 // Connection/DNS/timeout error - ExitAPI = 7 // Server returned error - ExitAmbiguous = 8 // Multiple matches for name + ExitOK = clioutput.ExitOK + ExitUsage = clioutput.ExitUsage + ExitNotFound = clioutput.ExitNotFound + ExitAuth = clioutput.ExitAuth + ExitForbidden = clioutput.ExitForbidden + ExitRateLimit = clioutput.ExitRateLimit + ExitNetwork = clioutput.ExitNetwork + ExitAPI = clioutput.ExitAPI + ExitAmbiguous = clioutput.ExitAmbiguous ) -// Error codes for JSON envelope. +// Error codes for JSON envelope (re-exported from shared module). const ( - CodeUsage = "usage" - CodeNotFound = "not_found" - CodeAuth = "auth_required" - CodeForbidden = "forbidden" - CodeRateLimit = "rate_limit" - CodeNetwork = "network" - CodeAPI = "api_error" - CodeAmbiguous = "ambiguous" + CodeUsage = clioutput.CodeUsage + CodeNotFound = clioutput.CodeNotFound + CodeAuth = clioutput.CodeAuth + CodeForbidden = clioutput.CodeForbidden + CodeRateLimit = clioutput.CodeRateLimit + CodeNetwork = clioutput.CodeNetwork + CodeAPI = clioutput.CodeAPI + CodeAmbiguous = clioutput.CodeAmbiguous ) // ExitCodeFor returns the exit code for a given error code. -func ExitCodeFor(code string) int { - switch code { - case CodeUsage: - return ExitUsage - case CodeNotFound: - return ExitNotFound - case CodeAuth: - return ExitAuth - case CodeForbidden: - return ExitForbidden - case CodeRateLimit: - return ExitRateLimit - case CodeNetwork: - return ExitNetwork - case CodeAPI: - return ExitAPI - case CodeAmbiguous: - return ExitAmbiguous - default: - return ExitAPI - } -} +func ExitCodeFor(code string) int { return clioutput.ExitCodeFor(code) } diff --git a/internal/output/envelope.go b/internal/output/envelope.go index 2a91c79dd..26a019cf9 100644 --- a/internal/output/envelope.go +++ b/internal/output/envelope.go @@ -1,17 +1,31 @@ package output import ( - "bytes" "encoding/json" "fmt" "io" "os" "strings" + clioutput "github.com/basecamp/cli/output" + "github.com/basecamp/basecamp-cli/internal/observability" "github.com/basecamp/basecamp-cli/internal/presenter" ) +// NormalizeData converts json.RawMessage and other types to standard Go types. +func NormalizeData(data any) any { return clioutput.NormalizeData(data) } + +// TruncationNotice returns a notice string if results may be truncated. +func TruncationNotice(count, defaultLimit int, all bool, explicitLimit int) string { + return clioutput.TruncationNotice(count, defaultLimit, all, explicitLimit) +} + +// TruncationNoticeWithTotal returns a truncation notice using totalCount from the API. +func TruncationNoticeWithTotal(count, totalCount int) string { + return clioutput.TruncationNoticeWithTotal(count, totalCount) +} + // Response is the success envelope for JSON output. type Response struct { OK bool `json:"ok"` @@ -257,69 +271,6 @@ func (w *Writer) writeCount(v any) error { return nil } -// NormalizeData converts json.RawMessage and other types to standard Go types. -func NormalizeData(data any) any { - // Handle json.RawMessage by unmarshaling it - if raw, ok := data.(json.RawMessage); ok { - var unmarshaled any - if err := unmarshalPreservingNumbers(raw, &unmarshaled); err == nil { - return normalizeUnmarshaled(unmarshaled) - } - return data - } - - // Handle typed structs/slices by marshaling then unmarshaling - // This converts struct types to map[string]any - switch data.(type) { - case []map[string]any, map[string]any, []any: - return data // Already normalized - case nil: - return data - default: - // Try to convert via JSON round-trip - b, err := json.Marshal(data) - if err != nil { - return data - } - var unmarshaled any - if err := unmarshalPreservingNumbers(b, &unmarshaled); err != nil { - return data - } - return normalizeUnmarshaled(unmarshaled) - } -} - -// unmarshalPreservingNumbers decodes JSON using UseNumber so numeric values -// remain as json.Number instead of being converted to float64. This preserves -// precision for large integer IDs that exceed 53-bit float64 range. -func unmarshalPreservingNumbers(data []byte, v any) error { - dec := json.NewDecoder(bytes.NewReader(data)) - dec.UseNumber() - return dec.Decode(v) -} - -// normalizeUnmarshaled converts []any to []map[string]any if all elements are maps. -func normalizeUnmarshaled(v any) any { - switch d := v.(type) { - case []any: - // Check if all elements are maps, convert to []map[string]any - if len(d) == 0 { - return []map[string]any{} - } - maps := make([]map[string]any, 0, len(d)) - for _, item := range d { - if m, ok := item.(map[string]any); ok { - maps = append(maps, m) - } else { - return v // Mixed types, return as-is - } - } - return maps - default: - return v - } -} - // writeStyled outputs ANSI styled terminal output. func (w *Writer) writeStyled(v any) error { // Schema-aware presenter is opt-in: only activates when a command @@ -376,50 +327,6 @@ func WithNotice(s string) ResponseOption { return func(r *Response) { r.Notice = s } } -// TruncationNotice returns a notice string if results may be truncated. -// Returns empty string if no truncation warning is needed. -// Parameters: -// - count: number of results returned -// - defaultLimit: the default limit for this resource type (e.g., 100) -// - all: whether --all flag was used -// - explicitLimit: limit set via --limit flag (0 if not set) -func TruncationNotice(count, defaultLimit int, all bool, explicitLimit int) string { - // No notice if --all was used (user explicitly requested everything) - if all { - return "" - } - - // Determine the effective limit - limit := defaultLimit - if explicitLimit > 0 { - limit = explicitLimit - } - - // No notice if no limit was applied (defaultLimit=0 and no explicit limit) - if limit == 0 { - return "" - } - - // If count equals the limit, results are likely truncated - if count > 0 && count >= limit { - return fmt.Sprintf("Showing %d results (use --all for complete list)", count) - } - - return "" -} - -// TruncationNoticeWithTotal returns a truncation notice when results are truncated. -// Uses totalCount from API's X-Total-Count header to show accurate counts. -// Returns empty string if no truncation or totalCount is 0 (unavailable). -func TruncationNoticeWithTotal(count, totalCount int) string { - // No notice if total count unavailable or all results returned - if totalCount == 0 || count >= totalCount { - return "" - } - - return fmt.Sprintf("Showing %d of %d results (use --all for complete list)", count, totalCount) -} - // WithBreadcrumbs adds breadcrumbs to the response. func WithBreadcrumbs(b ...Breadcrumb) ResponseOption { return func(r *Response) { r.Breadcrumbs = append(r.Breadcrumbs, b...) } diff --git a/internal/output/envelope_benchmark_test.go b/internal/output/envelope_benchmark_test.go index de0a4f10c..0ba9d5f20 100644 --- a/internal/output/envelope_benchmark_test.go +++ b/internal/output/envelope_benchmark_test.go @@ -75,49 +75,6 @@ func BenchmarkNormalizeData(b *testing.B) { }) } -// BenchmarkNormalizeUnmarshaled benchmarks array type conversion -func BenchmarkNormalizeUnmarshaled(b *testing.B) { - b.Run("all_maps", func(b *testing.B) { - data := []any{ - map[string]any{"id": 1, "name": "A"}, - map[string]any{"id": 2, "name": "B"}, - map[string]any{"id": 3, "name": "C"}, - } - b.ResetTimer() - for i := 0; i < b.N; i++ { - normalizeUnmarshaled(data) - } - }) - - b.Run("mixed_types", func(b *testing.B) { - data := []any{ - map[string]any{"id": 1}, - "string value", - 42, - } - b.ResetTimer() - for i := 0; i < b.N; i++ { - normalizeUnmarshaled(data) - } - }) - - b.Run("empty_array", func(b *testing.B) { - data := []any{} - b.ResetTimer() - for i := 0; i < b.N; i++ { - normalizeUnmarshaled(data) - } - }) - - b.Run("non_array", func(b *testing.B) { - data := map[string]any{"id": 123} - b.ResetTimer() - for i := 0; i < b.N; i++ { - normalizeUnmarshaled(data) - } - }) -} - // BenchmarkWriteJSON benchmarks JSON output writing func BenchmarkWriteJSON(b *testing.B) { b.Run("simple_response", func(b *testing.B) { diff --git a/internal/output/errors.go b/internal/output/errors.go index 8ca03d8fc..1e220e317 100644 --- a/internal/output/errors.go +++ b/internal/output/errors.go @@ -1,60 +1,31 @@ package output -import ( - "errors" - "fmt" -) +import clioutput "github.com/basecamp/cli/output" // Error is a structured error with code, message, and optional hint. -type Error struct { - Code string - Message string - Hint string - HTTPStatus int - Retryable bool - Cause error -} - -func (e *Error) Error() string { - if e.Hint != "" { - return fmt.Sprintf("%s: %s", e.Message, e.Hint) - } - return e.Message -} +// Type alias — zero-cost, full compatibility with errors.As. +type Error = clioutput.Error -func (e *Error) Unwrap() error { - return e.Cause -} - -// ExitCode returns the appropriate exit code for this error. -func (e *Error) ExitCode() int { - return ExitCodeFor(e.Code) -} - -// Error constructors for common cases. - -func ErrUsage(msg string) *Error { - return &Error{Code: CodeUsage, Message: msg} -} - -func ErrUsageHint(msg, hint string) *Error { - return &Error{Code: CodeUsage, Message: msg, Hint: hint} -} +// Generic error constructors (re-exported from shared module). +func ErrUsage(msg string) *Error { return clioutput.ErrUsage(msg) } +func ErrUsageHint(msg, hint string) *Error { return clioutput.ErrUsageHint(msg, hint) } func ErrNotFound(resource, identifier string) *Error { - return &Error{ - Code: CodeNotFound, - Message: fmt.Sprintf("%s not found: %s", resource, identifier), - } + return clioutput.ErrNotFound(resource, identifier) } - func ErrNotFoundHint(resource, identifier, hint string) *Error { - return &Error{ - Code: CodeNotFound, - Message: fmt.Sprintf("%s not found: %s", resource, identifier), - Hint: hint, - } + return clioutput.ErrNotFoundHint(resource, identifier, hint) +} +func ErrForbidden(msg string) *Error { return clioutput.ErrForbidden(msg) } +func ErrRateLimit(retryAfter int) *Error { return clioutput.ErrRateLimit(retryAfter) } +func ErrNetwork(cause error) *Error { return clioutput.ErrNetwork(cause) } +func ErrAPI(status int, msg string) *Error { return clioutput.ErrAPI(status, msg) } +func ErrAmbiguous(resource string, matches []string) *Error { + return clioutput.ErrAmbiguous(resource, matches) } +func AsError(err error) *Error { return clioutput.AsError(err) } + +// App-specific error constructors with basecamp-cli hints. func ErrAuth(msg string) *Error { return &Error{ @@ -64,14 +35,6 @@ func ErrAuth(msg string) *Error { } } -func ErrForbidden(msg string) *Error { - return &Error{ - Code: CodeForbidden, - Message: msg, - HTTPStatus: 403, - } -} - func ErrForbiddenScope() *Error { return &Error{ Code: CodeForbidden, @@ -80,60 +43,3 @@ func ErrForbiddenScope() *Error { HTTPStatus: 403, } } - -func ErrRateLimit(retryAfter int) *Error { - hint := "Try again later" - if retryAfter > 0 { - hint = fmt.Sprintf("Try again in %d seconds", retryAfter) - } - return &Error{ - Code: CodeRateLimit, - Message: "Rate limited", - Hint: hint, - HTTPStatus: 429, - Retryable: true, - } -} - -func ErrNetwork(cause error) *Error { - return &Error{ - Code: CodeNetwork, - Message: "Network error", - Hint: cause.Error(), - Retryable: true, - Cause: cause, - } -} - -func ErrAPI(status int, msg string) *Error { - return &Error{ - Code: CodeAPI, - Message: msg, - HTTPStatus: status, - } -} - -func ErrAmbiguous(resource string, matches []string) *Error { - hint := "Be more specific" - if len(matches) > 0 && len(matches) <= 5 { - hint = fmt.Sprintf("Did you mean: %v", matches) - } - return &Error{ - Code: CodeAmbiguous, - Message: fmt.Sprintf("Ambiguous %s", resource), - Hint: hint, - } -} - -// AsError attempts to convert an error to an *Error. -func AsError(err error) *Error { - var e *Error - if errors.As(err, &e) { - return e - } - return &Error{ - Code: CodeAPI, - Message: err.Error(), - Cause: err, - } -}