Tw 5660 reduce over engineering Patch 1

CLAUDE.md

@@ -57,7 +57,7 @@ Go 1.24+ · Hexagonal architecture (CLI → Port → Adapter) · Cobra CLI · **
 | Hook enforcement | `.claude/HOOKS-CONFIG.md` |
 | Agent definitions | `.claude/agents/README.md` |
 
-**Env vars:** `NYLAS_DISABLE_KEYRING`, `NYLAS_API_KEY`, `NYLAS_CLIENT_ID`, `NYLAS_GRANT_ID`, `NYLAS_API_BASE_URL` — see `docs/DEVELOPMENT.md`
+**Env vars:** `NYLAS_DISABLE_KEYRING`, `NYLAS_API_KEY`, `NYLAS_CLIENT_ID`, `NYLAS_GRANT_ID`, `NYLAS_API_BASE_URL`, `NYLAS_API_TIMEOUT` (e.g. `120s`; or `nylas config set api.timeout`) — see `docs/DEVELOPMENT.md`
 
 **Credentials:** System keyring (service: `"nylas"`, keys: `client_id`, `api_key`, `client_secret`, `org_id`). Grant cache: `os.UserCacheDir()/nylas/grants.json`. Fallback: `~/.config/nylas/` with `NYLAS_DISABLE_KEYRING=true`.
 

internal/adapters/nylas/client.go

@@ -9,35 +9,19 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"slices"
 	"strconv"
 	"strings"
 	"sync"
 	"time"
 
-	"github.com/nylas/cli/internal/adapters/providers"
 	"github.com/nylas/cli/internal/domain"
 	"github.com/nylas/cli/internal/httputil"
 	"github.com/nylas/cli/internal/ports"
 	"github.com/nylas/cli/internal/version"
 	"golang.org/x/time/rate"
 )
 
-func init() {
-	// Register Nylas provider with the global registry
-	providers.Register("nylas", func(config providers.ProviderConfig) (ports.NylasClient, error) {
-		client := NewHTTPClient()
-
-		if config.BaseURL != "" {
-			client.SetBaseURL(config.BaseURL)
-		} else if config.Region != "" {
-			client.SetRegion(config.Region)
-		}
-
-		client.SetCredentials(config.ClientID, config.ClientSecret, config.APIKey)
-		return client, nil
-	})
-}
-
 const (
 	baseURLUS = domain.BaseURLUS
 	baseURLEU = domain.BaseURLEU
@@ -115,6 +99,16 @@ func (c *HTTPClient) ApplyConfig(cfg *domain.Config) {
 		return
 	}
 	c.baseURL = cfg.ResolveBaseURL()
+
+	// Apply the configured per-request API timeout. The shared DefaultClient is
+	// fixed at the default; when the install overrides it, give this client a
+	// matching http.Client so the transport-level cap tracks the request
+	// deadline instead of clipping (or out-living) it.
+	timeout := cfg.ResolveAPITimeout()
+	c.requestTimeout = timeout
+	if timeout != httputil.DefaultClientTimeout {
+		c.httpClient = httputil.NewClient(timeout)
+	}
 }
 
 // SetMaxRetries sets the maximum number of retries (for testing purposes).
@@ -498,15 +492,7 @@ func (c *HTTPClient) doJSONRequestInternalWithRetry(
 	}
 
 	// Validate status code
-	statusOK := false
-	for _, status := range acceptedStatuses {
-		if resp.StatusCode == status {
-			statusOK = true
-			break
-		}
-	}
-
-	if !statusOK {
+	if !slices.Contains(acceptedStatuses, resp.StatusCode) {
 		defer func() { _ = resp.Body.Close() }()
 		return nil, c.parseError(resp)
 	}

internal/adapters/nylas/security_test.go

@@ -5,7 +5,9 @@ package nylas
 
 import (
 	"testing"
+	"time"
 
+	"github.com/nylas/cli/internal/domain"
 	"github.com/nylas/cli/internal/httputil"
 )
 
@@ -116,6 +118,38 @@ func TestOTPExtractionSecurity(t *testing.T) {
 	})
 }
 
+// TestHTTPClient_ApplyConfigTimeout verifies the per-request timeout is wired
+// from config and that a non-default value also swaps in a matching http.Client
+// so the transport-level cap tracks the request deadline.
+func TestHTTPClient_ApplyConfigTimeout(t *testing.T) {
+	t.Run("default config keeps the shared client and default timeout", func(t *testing.T) {
+		t.Setenv("NYLAS_API_TIMEOUT", "")
+		client := NewHTTPClient()
+		client.ApplyConfig(&domain.Config{Region: "us"})
+		if client.requestTimeout != domain.TimeoutAPI {
+			t.Errorf("requestTimeout = %v, want default %v", client.requestTimeout, domain.TimeoutAPI)
+		}
+		if client.httpClient != httputil.DefaultClient {
+			t.Error("expected the shared DefaultClient for the default timeout")
+		}
+	})
+
+	t.Run("config api.timeout sets request timeout and a matching client", func(t *testing.T) {
+		t.Setenv("NYLAS_API_TIMEOUT", "")
+		client := NewHTTPClient()
+		client.ApplyConfig(&domain.Config{Region: "us", API: &domain.APIConfig{Timeout: "300s"}})
+		if want := 300 * time.Second; client.requestTimeout != want {
+			t.Errorf("requestTimeout = %v, want %v", client.requestTimeout, want)
+		}
+		if client.httpClient == httputil.DefaultClient {
+			t.Error("expected a dedicated client for a non-default timeout")
+		}
+		if want := 300 * time.Second; client.httpClient.Timeout != want {
+			t.Errorf("http client timeout = %v, want %v", client.httpClient.Timeout, want)
+		}
+	})
+}
+
 // TestHTTPClientSecurity tests HTTP client security.
 func TestHTTPClientSecurity(t *testing.T) {
 	t.Run("client_timeout_matches_server_ceiling", func(t *testing.T) {

internal/cli/agent/rule.go

@@ -148,7 +148,9 @@ func detachRuleFromAgentWorkspaces(ctx context.Context, client interface {
 	updatedWorkspaceIDs := make([]string, 0)
 
 	for _, workspace := range workspaces {
-		if !stringSliceContains(workspace.RulesIDs, ruleID) {
+		if !slices.ContainsFunc(workspace.RulesIDs, func(id string) bool {
+			return strings.TrimSpace(id) == strings.TrimSpace(ruleID)
+		}) {
 			continue
 		}
 
@@ -211,13 +213,3 @@ func rollbackWorkspaceRuleUpdates(ctx context.Context, client interface {
 	}
 	return nil
 }
-
-func stringSliceContains(items []string, value string) bool {
-	value = strings.TrimSpace(value)
-	for _, item := range items {
-		if strings.TrimSpace(item) == value {
-			return true
-		}
-	}
-	return false
-}

internal/cli/common/client.go

@@ -43,6 +43,10 @@ func GetNylasClient() (ports.NylasClient, error) {
 		cfg = &domain.Config{Region: "us"}
 	}
 
+	// Propagate the install's API timeout to CreateContext (the per-command
+	// deadline) so it matches the client this function builds.
+	SetAPITimeout(cfg.ResolveAPITimeout())
+
 	// First, check environment variables (highest priority)
 	apiKey := os.Getenv("NYLAS_API_KEY")
 	clientID := os.Getenv("NYLAS_CLIENT_ID")

internal/cli/common/context.go

@@ -2,15 +2,42 @@ package common
 
 import (
 	"context"
+	"sync/atomic"
 	"time"
 
 	"github.com/nylas/cli/internal/domain"
 )
 
-// CreateContext creates a context with the standard API timeout.
+// apiTimeoutNanos holds the resolved per-request API timeout (0 = use the
+// domain.TimeoutAPI default). It is set once at client construction
+// (GetNylasClient) from the install's config/env and read by CreateContext,
+// which runs on every command. atomic because spinner/background goroutines
+// may also create contexts.
+var apiTimeoutNanos atomic.Int64
+
+// SetAPITimeout records the resolved API timeout for subsequent CreateContext
+// calls. A non-positive value resets to the default.
+func SetAPITimeout(d time.Duration) {
+	if d <= 0 {
+		apiTimeoutNanos.Store(0)
+		return
+	}
+	apiTimeoutNanos.Store(int64(d))
+}
+
+// apiTimeout returns the configured API timeout, or the default if unset.
+func apiTimeout() time.Duration {
+	if n := apiTimeoutNanos.Load(); n > 0 {
+		return time.Duration(n)
+	}
+	return domain.TimeoutAPI
+}
+
+// CreateContext creates a context with the configured API timeout (see
+// SetAPITimeout; defaults to domain.TimeoutAPI).
 // Returns the context and a cancel function that should be deferred.
 func CreateContext() (context.Context, context.CancelFunc) {
-	return context.WithTimeout(context.Background(), domain.TimeoutAPI)
+	return context.WithTimeout(context.Background(), apiTimeout())
 }
 
 // CreateContextWithTimeout creates a context with a custom timeout.

internal/cli/common/context_test.go

@@ -4,9 +4,14 @@ import (
 	"context"
 	"testing"
 	"time"
+
+	"github.com/nylas/cli/internal/domain"
 )
 
 func TestCreateContext(t *testing.T) {
+	SetAPITimeout(0) // default
+	defer SetAPITimeout(0)
+
 	ctx, cancel := CreateContext()
 	defer cancel()
 
@@ -20,11 +25,37 @@ func TestCreateContext(t *testing.T) {
 		t.Error("CreateContext() context has no deadline")
 	}
 
-	// Check that deadline is approximately 90 seconds from now (TimeoutAPI)
-	expectedDeadline := time.Now().Add(90 * time.Second)
+	// Default deadline is TimeoutAPI from now.
+	expectedDeadline := time.Now().Add(domain.TimeoutAPI)
 	diff := expectedDeadline.Sub(deadline)
 	if diff < -1*time.Second || diff > 1*time.Second {
-		t.Errorf("CreateContext() deadline is %v, expected around 90s from now", deadline)
+		t.Errorf("CreateContext() deadline is %v, expected around %v from now", deadline, domain.TimeoutAPI)
+	}
+}
+
+func TestCreateContext_HonorsConfiguredTimeout(t *testing.T) {
+	SetAPITimeout(45 * time.Second)
+	defer SetAPITimeout(0)
+
+	ctx, cancel := CreateContext()
+	defer cancel()
+
+	deadline, ok := ctx.Deadline()
+	if !ok {
+		t.Fatal("CreateContext() context has no deadline")
+	}
+	diff := time.Until(deadline) - 45*time.Second
+	if diff < -1*time.Second || diff > 1*time.Second {
+		t.Errorf("CreateContext() deadline ~%v from now, expected ~45s", time.Until(deadline))
+	}
+
+	// Resetting to default restores TimeoutAPI.
+	SetAPITimeout(0)
+	ctx2, cancel2 := CreateContext()
+	defer cancel2()
+	d2, _ := ctx2.Deadline()
+	if diff := time.Until(d2) - domain.TimeoutAPI; diff < -1*time.Second || diff > 1*time.Second {
+		t.Errorf("after reset, deadline ~%v, expected ~%v", time.Until(d2), domain.TimeoutAPI)
 	}
 }
 

internal/cli/slack/files.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/nylas/cli/internal/cli/common"
 	"github.com/nylas/cli/internal/domain"
+	"github.com/nylas/cli/internal/httputil"
 )
 
 // newFilesCmd creates the files command group.
@@ -330,10 +331,9 @@ Examples:
 				return common.NewInputError(fmt.Sprintf("output path is a directory: %s", outputPath))
 			}
 
-			// Download the file with a dedicated long timeout: the command
-			// context carries the default API timeout, which would cut off
-			// large downloads mid-stream.
-			dlCtx, dlCancel := common.CreateContextWithTimeout(domain.TimeoutDownload)
+			// Download under the standard 120s client timeout (the Nylas
+			// server-side ceiling), matching email attachment downloads.
+			dlCtx, dlCancel := common.CreateContextWithTimeout(httputil.DefaultClientTimeout)
 			defer dlCancel()
 			reader, err := client.DownloadFile(dlCtx, file.DownloadURL)
 			if err != nil {

internal/cli/update/update.go

@@ -9,7 +9,6 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/nylas/cli/internal/cli/common"
-	"github.com/nylas/cli/internal/domain"
 	"github.com/nylas/cli/internal/version"
 )
 
@@ -171,23 +170,3 @@ func runUpdate(ctx context.Context, checkOnly, force, yes bool) error {
 
 	return nil
 }
-
-// CheckForUpdateAsync checks for updates in the background and prints a message if available.
-// This can be called during CLI startup for non-blocking update notifications.
-func CheckForUpdateAsync(currentVersion string) {
-	go func() {
-		ctx, cancel := common.CreateContextWithTimeout(domain.TimeoutQuickCheck)
-		defer cancel()
-
-		release, err := getLatestRelease(ctx)
-		if err != nil {
-			return // Silently ignore errors in async check
-		}
-
-		latestVersion := parseVersion(release.TagName)
-		if isUpdateAvailable(currentVersion, latestVersion) {
-			fmt.Printf("\nA new version of Nylas CLI is available: %s (current: %s)\n", latestVersion, currentVersion)
-			fmt.Println("Run 'nylas update' to upgrade.")
-		}
-	}()
-}