Move review preferences from project settings to local preferences

cmd/entire/cli/recap.go

@@ -15,6 +15,7 @@ import (
 	"golang.org/x/term"
 
 	"github.com/entireio/cli/cmd/entire/cli/api"
+	"github.com/entireio/cli/cmd/entire/cli/auth"
 	"github.com/entireio/cli/cmd/entire/cli/gitremote"
 	"github.com/entireio/cli/cmd/entire/cli/interactive"
 	"github.com/entireio/cli/cmd/entire/cli/paths"
@@ -122,9 +123,17 @@ func runRecap(ctx context.Context, w, errW io.Writer, f *recapFlags) error {
 	if err != nil {
 		return err
 	}
-	client, err := NewAuthenticatedAPIClient(f.insecureHTTP)
+	client, err := newRecapClient(f.insecureHTTP)
 	if err != nil {
-		fmt.Fprintln(errW, "Sign in with `entire login` to use `entire recap`.")
+		var keyringErr *keyringReadError
+		switch {
+		case errors.Is(err, api.ErrInsecureHTTP):
+			fmt.Fprintf(errW, "ENTIRE_API_BASE_URL is set to an insecure http:// URL (%s). Use https:// for production, or pass --insecure-http-auth for local dev.\n", api.BaseURL())
+		case errors.As(err, &keyringErr):
+			fmt.Fprintf(errW, "Could not read your auth token from the system keyring: %v. Running `entire login` may not help — the keyring may be locked or inaccessible. Check your OS keychain settings.\n", keyringErr.Cause)
+		default:
+			return err
+		}
 		return NewSilentError(err)
 	}
 	rangeKey := f.rangeKey()
@@ -154,6 +163,34 @@ func runRecap(ctx context.Context, w, errW io.Writer, f *recapFlags) error {
 	return nil
 }
 
+// keyringReadError marks a failure to read the auth token from the system
+// keyring (locked, permission denied, etc.) — distinct from "no token saved",
+// which keyring.ErrNotFound resolves to (token=="", err==nil) upstream.
+type keyringReadError struct{ Cause error }
+
+func (e *keyringReadError) Error() string {
+	return "read auth token from keyring: " + e.Cause.Error()
+}
+func (e *keyringReadError) Unwrap() error { return e.Cause }
+
+// newRecapClient does not gate on a missing token; FetchMeRecap surfaces 401s
+// via recapLoadErrorMessage so flag effects (--week, --agent, ...) and the
+// real auth error are not collapsed into one "sign in" hint. A keyring read
+// failure is surfaced as *keyringReadError so the caller can show a targeted
+// message instead of misattributing it to a missing login.
+func newRecapClient(insecureHTTP bool) (*api.Client, error) {
+	token, err := auth.LookupCurrentToken()
+	if err != nil {
+		return nil, &keyringReadError{Cause: err}
+	}
+	if token != "" && !insecureHTTP {
+		if err := api.RequireSecureURL(api.BaseURL()); err != nil {
+			return nil, fmt.Errorf("base URL check: %w", err)
+		}
+	}
+	return api.NewClient(token), nil
+}
+
 func handleRecapFetchError(w io.Writer, err error) error {
 	if shouldShowRecapLoadErrorMessage(err) {
 		fmt.Fprintln(w, recapLoadErrorMessage(err))

cmd/entire/cli/recap_errors.go

@@ -37,12 +37,25 @@ func recapLoadErrorMessage(err error) string {
 			return err.Error()
 		}
 	}
+	if host, ok := dnsNotFoundHost(err); ok {
+		return fmt.Sprintf("Could not resolve API host %q (DNS lookup failed). Check ENTIRE_API_BASE_URL — the host may be misspelled or the env var may be pointing at a non-existent server. Details: %v", host, err)
+	}
 	if isRecapNetworkError(err) {
 		return fmt.Sprintf("Could not reach entire.io. Check your internet connection and ENTIRE_API_BASE_URL if you use a custom API host. Details: %v", err)
 	}
 	return err.Error()
 }
 
+// dnsNotFoundHost reports an NXDOMAIN-style failure, distinguishing "host
+// doesn't exist" from generic connectivity loss.
+func dnsNotFoundHost(err error) (string, bool) {
+	var dnsErr *net.DNSError
+	if errors.As(err, &dnsErr) && dnsErr.IsNotFound {
+		return dnsErr.Name, true
+	}
+	return "", false
+}
+
 func recapErrorDetail(err *api.HTTPError) string {
 	if strings.TrimSpace(err.Message) != "" {
 		return fmt.Sprintf("HTTP %d: %s", err.StatusCode, err.Message)

cmd/entire/cli/recap_test.go

@@ -143,6 +143,27 @@ func TestRecapFlags_ColorEnabled(t *testing.T) {
 	}
 }
 
+func TestKeyringReadError_PreservesCauseAndMatchesAs(t *testing.T) {
+	t.Parallel()
+
+	cause := errors.New("keychain locked")
+	err := error(&keyringReadError{Cause: cause})
+
+	if !errors.Is(err, cause) {
+		t.Fatalf("errors.Is should match wrapped cause; got false for %v", err)
+	}
+	var keyringErr *keyringReadError
+	if !errors.As(err, &keyringErr) {
+		t.Fatalf("errors.As should extract *keyringReadError; got false for %v", err)
+	}
+	if !errors.Is(keyringErr.Cause, cause) {
+		t.Fatalf("Cause = %v, want %v", keyringErr.Cause, cause)
+	}
+	if !strings.Contains(err.Error(), "keychain locked") {
+		t.Fatalf("Error() should include cause text; got %q", err.Error())
+	}
+}
+
 func TestRunRecap_PrerequisiteErrorsUseErrorWriter(t *testing.T) {
 	tmpDir := t.TempDir()
 	t.Chdir(tmpDir)
@@ -314,6 +335,25 @@ func TestRecapLoadErrorMessage_NetworkError(t *testing.T) {
 	}
 }
 
+func TestRecapLoadErrorMessage_DNSNotFound(t *testing.T) {
+	t.Parallel()
+
+	nxdomain := &net.DNSError{Name: "no-token-here.example.com", Err: "no such host", IsNotFound: true}
+	got := recapLoadErrorMessage(fmt.Errorf("me/recap get: %w", nxdomain))
+	if strings.Contains(got, "Check your internet connection") {
+		t.Fatalf("NXDOMAIN should not blame internet connection:\n%s", got)
+	}
+	for _, want := range []string{
+		"Could not resolve API host",
+		"no-token-here.example.com",
+		"ENTIRE_API_BASE_URL",
+	} {
+		if !strings.Contains(got, want) {
+			t.Fatalf("message missing %q:\n%s", want, got)
+		}
+	}
+}
+
 func TestRecapLoadErrorMessage_ContextCancellation(t *testing.T) {
 	t.Parallel()
 

cmd/entire/cli/review/cmd.go

@@ -156,6 +156,15 @@ Subcommands:
 			if modes > 1 {
 				return errors.New("--edit, --findings, and --fix are mutually exclusive")
 			}
+			if err := maybePromptReviewSettingsMigration(
+				ctx,
+				cmd.OutOrStdout(),
+				cmd.ErrOrStderr(),
+				interactive.IsTerminalWriter(cmd.OutOrStdout()) && interactive.CanPromptInteractively(),
+				deps.PromptYN,
+			); err != nil {
+				return err
+			}
 			if edit {
 				_, err := RunReviewConfigPicker(ctx, cmd.OutOrStdout(), deps.GetAgentsWithHooksInstalled)
 				return err

cmd/entire/cli/review/migration.go

@@ -0,0 +1,153 @@
+package review
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"fmt"
+	"io"
+	"os"
+
+	"github.com/entireio/cli/cmd/entire/cli/jsonutil"
+	"github.com/entireio/cli/cmd/entire/cli/paths"
+	"github.com/entireio/cli/cmd/entire/cli/settings"
+)
+
+type projectReviewSettings struct {
+	path        string
+	raw         map[string]json.RawMessage
+	review      json.RawMessage
+	fixAgent    json.RawMessage
+	hasReview   bool
+	hasFixAgent bool
+}
+
+func maybePromptReviewSettingsMigration(
+	ctx context.Context,
+	out io.Writer,
+	errOut io.Writer,
+	canPrompt bool,
+	promptYN func(context.Context, string, bool) (bool, error),
+) error {
+	project, ok, err := loadProjectReviewSettings(ctx)
+	if err != nil {
+		return err
+	}
+	if !ok {
+		return nil
+	}
+
+	if !canPrompt {
+		fmt.Fprintln(errOut, "Review preferences are stored in project settings (.entire/settings.json). Run `entire review --edit` interactively to move them to local preferences.")
+		return nil
+	}
+
+	if promptYN == nil {
+		promptYN = realPromptYN
+	}
+	migrate, err := promptYN(ctx, "Review preferences are stored in project settings (.entire/settings.json). Move them to local preferences now?", false)
+	if err != nil {
+		return fmt.Errorf("review settings migration prompt: %w", err)
+	}
+	if !migrate {
+		return nil
+	}
+
+	if err := migrateProjectReviewSettings(ctx, project); err != nil {
+		return err
+	}
+	fmt.Fprintln(out, "Moved review preferences from project settings to local preferences.")
+	return nil
+}
+
+func loadProjectReviewSettings(ctx context.Context) (*projectReviewSettings, bool, error) {
+	path, err := paths.AbsPath(ctx, settings.EntireSettingsFile)
+	if err != nil {
+		path = settings.EntireSettingsFile
+	}
+
+	data, err := os.ReadFile(path) //nolint:gosec // path is resolved from repo settings path
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil, false, nil
+		}
+		return nil, false, fmt.Errorf("reading project settings for review migration: %w", err)
+	}
+
+	raw := map[string]json.RawMessage{}
+	if err := json.Unmarshal(data, &raw); err != nil {
+		return nil, false, fmt.Errorf("parsing project settings for review migration: %w", err)
+	}
+
+	reviewRaw, hasReview := raw["review"]
+	fixAgentRaw, hasFixAgent := raw["review_fix_agent"]
+	if !hasReview && !hasFixAgent {
+		return nil, false, nil
+	}
+	return &projectReviewSettings{
+		path:        path,
+		raw:         raw,
+		review:      reviewRaw,
+		fixAgent:    fixAgentRaw,
+		hasReview:   hasReview,
+		hasFixAgent: hasFixAgent,
+	}, true, nil
+}
+
+func migrateProjectReviewSettings(ctx context.Context, project *projectReviewSettings) error {
+	if project == nil {
+		return nil
+	}
+
+	prefs, err := settings.LoadClonePreferences(ctx)
+	if err != nil {
+		return fmt.Errorf("load review preferences for migration: %w", err)
+	}
+	if prefs == nil {
+		prefs = &settings.ClonePreferences{}
+	}
+
+	preferencesChanged := false
+	if project.hasReview && len(prefs.Review) == 0 && !isJSONNull(project.review) {
+		var review map[string]settings.ReviewConfig
+		if err := json.Unmarshal(project.review, &review); err != nil {
+			return fmt.Errorf("parsing project review settings: %w", err)
+		}
+		if len(review) > 0 {
+			prefs.Review = review
+			preferencesChanged = true
+		}
+	}
+	if project.hasFixAgent && prefs.ReviewFixAgent == "" && !isJSONNull(project.fixAgent) {
+		var fixAgent string
+		if err := json.Unmarshal(project.fixAgent, &fixAgent); err != nil {
+			return fmt.Errorf("parsing project review_fix_agent: %w", err)
+		}
+		if fixAgent != "" {
+			prefs.ReviewFixAgent = fixAgent
+			preferencesChanged = true
+		}
+	}
+
+	if preferencesChanged {
+		if err := settings.SaveClonePreferences(ctx, prefs); err != nil {
+			return fmt.Errorf("save review preferences for migration: %w", err)
+		}
+	}
+
+	delete(project.raw, "review")
+	delete(project.raw, "review_fix_agent")
+	data, err := jsonutil.MarshalIndentWithNewline(project.raw, "", "  ")
+	if err != nil {
+		return fmt.Errorf("marshal project settings after review migration: %w", err)
+	}
+	//nolint:gosec // G306: settings file is config, not secrets; 0o644 is appropriate
+	if err := os.WriteFile(project.path, data, 0o644); err != nil {
+		return fmt.Errorf("write project settings after review migration: %w", err)
+	}
+	return nil
+}
+
+func isJSONNull(raw json.RawMessage) bool {
+	return bytes.Equal(bytes.TrimSpace(raw), []byte("null"))
+}