Carry --profile in suggested follow-up commands

internal/cli/followup_profile_test.go

@@ -0,0 +1,114 @@
+package cli
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/basecamp/basecamp-cli/internal/commands"
+	"github.com/basecamp/basecamp-cli/internal/output"
+)
+
+// A suggested follow-up is pasted into another shell, which may resolve a
+// different profile. However this run chose its profile, the follow-up names it.
+func TestFollowUpNamesTheProfileHoweverItWasChosen(t *testing.T) {
+	cases := map[string]struct {
+		config   func(url string) string
+		env      string
+		args     []string
+		followUp string
+	}{
+		"no profiles": {
+			config: func(url string) string {
+				return fmt.Sprintf(`{"base_url": %q, "account_id": "99999"}`, url)
+			},
+			followUp: "basecamp unspotlight 456",
+		},
+		"default_profile": {
+			config:   profilesConfig("staging"),
+			followUp: "basecamp unspotlight 456 --profile staging",
+		},
+		"BASECAMP_PROFILE": {
+			config:   profilesConfig("production"),
+			env:      "staging",
+			followUp: "basecamp unspotlight 456 --profile staging",
+		},
+		"--profile": {
+			config:   profilesConfig("production"),
+			args:     []string{"--profile", "staging"},
+			followUp: "basecamp unspotlight 456 --profile staging",
+		},
+	}
+	for name, c := range cases {
+		t.Run(name, func(t *testing.T) {
+			var requested []string
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				requested = append(requested, r.Method+" "+r.URL.Path)
+				w.Header().Set("Content-Type", "application/json")
+				w.WriteHeader(http.StatusCreated)
+				_, _ = w.Write([]byte(`{"id":456,"type":"Message","title":"Kickoff"}`))
+			}))
+			t.Cleanup(srv.Close)
+
+			isolateRootTest(t)
+			t.Setenv("HOME", t.TempDir())
+			t.Setenv("BASECAMP_TOKEN", "fake")
+			t.Setenv("BASECAMP_PROFILE", c.env)
+			t.Setenv("BASECAMP_ACCOUNT_ID", "")
+			t.Setenv("BASECAMP_BASE_URL", "")
+			dir := filepath.Join(os.Getenv("XDG_CONFIG_HOME"), "basecamp")
+			require.NoError(t, os.MkdirAll(dir, 0o700))
+			require.NoError(t, os.WriteFile(filepath.Join(dir, "config.json"),
+				[]byte(c.config(srv.URL)), 0o600))
+
+			root := NewRootCmd()
+			root.AddCommand(commands.NewSpotlightCmd())
+			root.SetArgs(append(c.args, "spotlight", "456", "--hints", "--json"))
+			root.SetOut(&bytes.Buffer{})
+			root.SetErr(&bytes.Buffer{})
+			stdout := captureStdout(t, func() { require.NoError(t, root.Execute()) })
+
+			var envelope output.Response
+			require.NoError(t, json.Unmarshal(stdout, &envelope), string(stdout))
+			assert.Equal(t, []string{"POST /99999/recordings/456/spotlight.json"}, requested)
+			assert.Contains(t, envelope.Breadcrumbs, output.Breadcrumb{
+				Action: "unspotlight", Cmd: c.followUp, Description: "Remove the spotlight",
+			})
+		})
+	}
+}
+
+func profilesConfig(defaultProfile string) func(url string) string {
+	return func(url string) string {
+		return fmt.Sprintf(`{"default_profile": %q, "profiles": {
+			"production": {"base_url": %q, "account_id": "11111"},
+			"staging": {"base_url": %q, "account_id": "99999"}}}`, defaultProfile, url, url)
+	}
+}
+
+func captureStdout(t *testing.T, run func()) []byte {
+	t.Helper()
+	reader, writer, err := os.Pipe()
+	require.NoError(t, err)
+	orig := os.Stdout
+	os.Stdout = writer
+	t.Cleanup(func() { os.Stdout = orig })
+	done := make(chan []byte)
+	go func() {
+		out, _ := io.ReadAll(reader)
+		done <- out
+	}()
+	run()
+	os.Stdout = orig
+	writer.Close()
+	return <-done
+}

internal/commands/comment.go

@@ -17,7 +17,6 @@ import (
 	"github.com/basecamp/basecamp-sdk/go/pkg/basecamp"
 
 	"github.com/basecamp/basecamp-cli/internal/appctx"
-	"github.com/basecamp/basecamp-cli/internal/config"
 	"github.com/basecamp/basecamp-cli/internal/editor"
 	"github.com/basecamp/basecamp-cli/internal/hostutil"
 	"github.com/basecamp/basecamp-cli/internal/output"
@@ -316,13 +315,7 @@ You can pass either a comment ID or a Basecamp URL:
 			}
 
 			app := appctx.FromContext(cmd.Context())
-
-			// Whether the account is persistently available (on-disk config or env)
-			// before resolution. When it isn't — adopted from the URL, resolved
-			// interactively, or passed via the process-local --account flag — the
-			// advertised follow-ups must spell out --account so they remain runnable
-			// in a fresh process.
-			persistentAccount := hasPersistentAccount(app.Config)
+			scope := newFollowUpScope(app)
 
 			// Same safe front door as `thread`: classify, guard the account, and
 			// validate the ID — so a plain recording URL or wrong-account URL is
@@ -331,7 +324,7 @@ You can pass either a comment ID or a Basecamp URL:
 			if err != nil {
 				return err
 			}
-			accountArg := replyAccountArg(persistentAccount, app.Config.AccountID)
+			scopeArgs := scope.args(app.Config.AccountID)
 
 			comment, err := app.Account().Comments().Get(cmd.Context(), commentID)
 			if err != nil {
@@ -376,14 +369,14 @@ You can pass either a comment ID or a Basecamp URL:
 			breadcrumbs := make([]output.Breadcrumb, 0, 2)
 			breadcrumbs = append(breadcrumbs, output.Breadcrumb{
 				Action:      "update",
-				Cmd:         fmt.Sprintf("basecamp comments update %d <text>%s", commentID, accountArg),
+				Cmd:         fmt.Sprintf("basecamp comments update %d <text>%s", commentID, scopeArgs),
 				Description: "Update comment",
 			})
 			if comment.Parent != nil && comment.Parent.ID != 0 {
 				m["reply_target"] = replyTarget(comment.Parent.ID, app.Config.AccountID)
 				breadcrumbs = append(breadcrumbs, output.Breadcrumb{
 					Action:      "reply",
-					Cmd:         fmt.Sprintf("basecamp comments create %d <text>%s", comment.Parent.ID, accountArg),
+					Cmd:         fmt.Sprintf("basecamp comments create %d <text>%s", comment.Parent.ID, scopeArgs),
 					Description: "Reply on the parent recording",
 				})
 			}
@@ -451,16 +444,12 @@ func runCommentsThread(cmd *cobra.Command, arg string, all bool, window int) err
 	// and `comments show` are equally safe: a plain recording URL / collection is
 	// rejected toward `basecamp show`, a wrong-account URL and a non-positive ID
 	// are rejected before any API call.
-	persistentAccount := hasPersistentAccount(app.Config)
+	scope := newFollowUpScope(app)
 	triggerID, err := resolveCommentTrigger(cmd, app, arg)
 	if err != nil {
 		return err
 	}
-	// When the account isn't persistently available (adopted from the URL,
-	// resolved interactively, or passed via the process-local --account flag), the
-	// advertised follow-ups must carry --account so they stay runnable in a fresh
-	// process.
-	accountArg := replyAccountArg(persistentAccount, app.Config.AccountID)
+	scopeArgs := scope.args(app.Config.AccountID)
 
 	// Step 1 — resolve the focus comment.
 	trigger, err := app.Account().Comments().Get(cmd.Context(), triggerID)
@@ -555,13 +544,13 @@ func runCommentsThread(cmd *cobra.Command, arg string, all bool, window int) err
 	if len(trigger.ContentAttachments) > 0 {
 		breadcrumbs = append(breadcrumbs, output.Breadcrumb{
 			Action:      "download",
-			Cmd:         fmt.Sprintf("basecamp attachments download %d --type comment%s", trigger.ID, accountArg),
+			Cmd:         fmt.Sprintf("basecamp attachments download %d --type comment%s", trigger.ID, scopeArgs),
 			Description: "Download comment attachments",
 		})
 	}
 	breadcrumbs = append(breadcrumbs, output.Breadcrumb{
 		Action:      "reply",
-		Cmd:         fmt.Sprintf("basecamp comments create %s <text>%s", parentIDStr, accountArg),
+		Cmd:         fmt.Sprintf("basecamp comments create %s <text>%s", parentIDStr, scopeArgs),
 		Description: "Reply on the parent recording",
 	})
 
@@ -668,37 +657,6 @@ func resolveCommentTrigger(cmd *cobra.Command, app *appctx.App, arg string) (int
 	return id, nil
 }
 
-// hasPersistentAccount reports whether the account is available to a fresh
-// process without re-specifying it — i.e. backed by an on-disk config layer or
-// an exported env var. A flag- or prompt-sourced account is process-local: it
-// lives only for this invocation, so an advertised follow-up copied into a new
-// process would fail to resolve it. Call this BEFORE resolveCommentTrigger, so a
-// URL-adopted / interactively-resolved account (AccountID empty at entry) also
-// reads as non-persistent.
-func hasPersistentAccount(cfg *config.Config) bool {
-	if cfg.AccountID == "" {
-		return false
-	}
-	switch cfg.Sources["account_id"] {
-	case string(config.SourceFlag), string(config.SourcePrompt):
-		return false
-	default:
-		return true
-	}
-}
-
-// replyAccountArg returns " --account <id>" to append to an advertised
-// follow-up command when the account is not persistently available (adopted from
-// the URL, resolved interactively, or passed via the process-local --account
-// flag this run), so the command stays runnable in a fresh process; otherwise
-// "". A persistent account needs no echo — the follow-up inherits it.
-func replyAccountArg(persistentAccount bool, accountID string) string {
-	if persistentAccount || accountID == "" {
-		return ""
-	}
-	return " --account " + accountID
-}
-
 // replyTarget builds the machine reply-target contract: the parent recording to
 // reply on, plus the account it belongs to so a consumer can construct a
 // fully-qualified reply regardless of its own configuration.

internal/commands/comment_test.go

@@ -432,29 +432,29 @@ func TestCommentsShowRejectsZeroComment(t *testing.T) {
 
 // --- Iteration 2: account provenance in reply breadcrumbs -------------------
 
-func TestHasPersistentAccount(t *testing.T) {
-	mk := func(id, src string) *config.Config {
-		c := &config.Config{AccountID: id, Sources: map[string]string{}}
-		if src != "" {
-			c.Sources["account_id"] = src
+func TestFollowUpScopeNamesWhatAFreshProcessLacks(t *testing.T) {
+	args := func(activeProfile, accountID, accountSource string) string {
+		app := &appctx.App{Config: &config.Config{
+			ActiveProfile: activeProfile, AccountID: accountID, Sources: map[string]string{},
+		}}
+		if accountSource != "" {
+			app.Config.Sources["account_id"] = accountSource
 		}
-		return c
+		return newFollowUpScope(app).args("99999")
 	}
-	// No account → not persistent.
-	assert.False(t, hasPersistentAccount(mk("", "")))
-	// Process-local sources → not persistent (follow-ups must echo --account).
-	assert.False(t, hasPersistentAccount(mk("99999", string(config.SourceFlag))))
-	assert.False(t, hasPersistentAccount(mk("99999", string(config.SourcePrompt))))
-	// On-disk / exported-env sources → persistent (follow-ups inherit it).
-	assert.True(t, hasPersistentAccount(mk("99999", string(config.SourceGlobal))))
-	assert.True(t, hasPersistentAccount(mk("99999", string(config.SourceLocal))))
-	assert.True(t, hasPersistentAccount(mk("99999", string(config.SourceEnv))))
-}
-
-func TestReplyAccountArg(t *testing.T) {
-	assert.Equal(t, "", replyAccountArg(true, "99999"))                  // persistent → no echo
-	assert.Equal(t, "", replyAccountArg(false, ""))                      // nothing to echo
-	assert.Equal(t, " --account 99999", replyAccountArg(false, "99999")) // process-local → echo
+	// Account resolved after the scope was read (URL, picker) → named.
+	assert.Equal(t, " --account 99999", args("", "", ""))
+	// Process-local account → named.
+	assert.Equal(t, " --account 99999", args("", "99999", string(config.SourceFlag)))
+	assert.Equal(t, " --account 99999", args("", "99999", string(config.SourcePrompt)))
+	// Account a fresh process resolves again → inherited.
+	assert.Equal(t, "", args("", "99999", string(config.SourceGlobal)))
+	assert.Equal(t, "", args("", "99999", string(config.SourceLocal)))
+	assert.Equal(t, "", args("", "99999", string(config.SourceEnv)))
+	// The active profile is always named; its account is inherited through it.
+	assert.Equal(t, " --profile staging", args("staging", "99999", "profile"))
+	assert.Equal(t, " --profile staging --account 99999", args("staging", "99999", string(config.SourceFlag)))
+	assert.Equal(t, " --profile 'my profile' --account 99999", args("my profile", "", ""))
 }
 
 func TestCommentsShowFlagAccountEchoedInBreadcrumb(t *testing.T) {

internal/commands/helpers.go

@@ -349,6 +349,42 @@ func ensureAccount(cmd *cobra.Command, app *appctx.App) error {
 	return nil
 }
 
+// followUpScope is what an advertised follow-up command must name to act on the
+// same profile and account when run in a fresh process. The active profile is
+// always named: the shell it is pasted into may have another BASECAMP_PROFILE,
+// and a profile picked interactively is not picked again. An account given with
+// --account, adopted from a URL or picked interactively lasts only for this run,
+// so it is named too. Any other account comes back with the profile or config.
+type followUpScope struct {
+	profile          string
+	accountInherited bool
+}
+
+// newFollowUpScope reads the scope before the command resolves its account, so
+// an account adopted from a URL or picked interactively (empty at entry) reads
+// as process-local.
+func newFollowUpScope(app *appctx.App) followUpScope {
+	inherited := app.Config.AccountID != ""
+	switch app.Config.Sources["account_id"] {
+	case string(config.SourceFlag), string(config.SourcePrompt):
+		inherited = false
+	}
+	return followUpScope{profile: app.Config.ActiveProfile, accountInherited: inherited}
+}
+
+// args returns the flags to append to a follow-up command, such as
+// " --profile staging --account 123", or "" when it inherits both.
+func (s followUpScope) args(accountID string) string {
+	args := ""
+	if s.profile != "" {
+		args += " --profile " + richtext.ShellQuote(s.profile)
+	}
+	if !s.accountInherited && accountID != "" {
+		args += " --account " + accountID
+	}
+	return args
+}
+
 // ensureProject resolves the project ID if not already configured.
 // This enables interactive prompts when --project flag and config are both missing.
 // The account must be resolved first (call ensureAccount before this).

internal/commands/spotlight.go

@@ -39,7 +39,7 @@ nothing. Requires edit access to the project.`,
 		Args: cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			app := appctx.FromContext(cmd.Context())
-			persistentAccount := hasPersistentAccount(app.Config)
+			scope := newFollowUpScope(app)
 			recordingID, err := spotlightTarget(cmd, app, args[0])
 			if err != nil {
 				return err
@@ -50,18 +50,18 @@ nothing. Requires edit access to the project.`,
 				return spotlightError(err)
 			}
 
-			accountArg := replyAccountArg(persistentAccount, app.Config.AccountID)
+			scopeArgs := scope.args(app.Config.AccountID)
 			return app.OK(recording,
 				output.WithSummary(fmt.Sprintf("Spotlighted #%d on its project page", recordingID)),
 				output.WithBreadcrumbs(
 					output.Breadcrumb{
 						Action:      "unspotlight",
-						Cmd:         fmt.Sprintf("basecamp unspotlight %d%s", recordingID, accountArg),
+						Cmd:         fmt.Sprintf("basecamp unspotlight %d%s", recordingID, scopeArgs),
 						Description: "Remove the spotlight",
 					},
 					output.Breadcrumb{
 						Action:      "show",
-						Cmd:         fmt.Sprintf("basecamp show %d%s", recordingID, accountArg),
+						Cmd:         fmt.Sprintf("basecamp show %d%s", recordingID, scopeArgs),
 						Description: "View item",
 					},
 				),
@@ -90,7 +90,7 @@ nothing. Requires edit access to the project.`,
 		Args: cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			app := appctx.FromContext(cmd.Context())
-			persistentAccount := hasPersistentAccount(app.Config)
+			scope := newFollowUpScope(app)
 			recordingID, err := spotlightTarget(cmd, app, args[0])
 			if err != nil {
 				return err
@@ -100,18 +100,18 @@ nothing. Requires edit access to the project.`,
 				return convertSDKError(err)
 			}
 
-			accountArg := replyAccountArg(persistentAccount, app.Config.AccountID)
+			scopeArgs := scope.args(app.Config.AccountID)
 			return app.OK(map[string]any{"id": recordingID, "spotlighted": false},
 				output.WithSummary(fmt.Sprintf("Spotlight removed from #%d", recordingID)),
 				output.WithBreadcrumbs(
 					output.Breadcrumb{
 						Action:      "spotlight",
-						Cmd:         fmt.Sprintf("basecamp spotlight %d%s", recordingID, accountArg),
+						Cmd:         fmt.Sprintf("basecamp spotlight %d%s", recordingID, scopeArgs),
 						Description: "Spotlight it again",
 					},
 					output.Breadcrumb{
 						Action:      "show",
-						Cmd:         fmt.Sprintf("basecamp show %d%s", recordingID, accountArg),
+						Cmd:         fmt.Sprintf("basecamp show %d%s", recordingID, scopeArgs),
 						Description: "View item",
 					},
 				),