Skip to content

feat: support LARKSUITE_CLI_PROFILE env var for multi-app selection #445

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
meijing0114 wants to merge 2 commits into
base: main
Choose a base branch
from
+277 −17
Open

feat: support LARKSUITE_CLI_PROFILE env var for multi-app selection #445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95ef221
feat: support LARKSUITE_CLI_PROFILE env var for multi-app selection
meijing0114 Apr 13, 2026
79cf353
feat: bootstrap reads LARKSUITE_CLI_PROFILE env var as --profile fall…
meijing0114 Apr 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion cmd/bootstrap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ package cmd
import (
"errors"
"io"
"os"

"github.com/larksuite/cli/internal/cmdutil"
"github.com/larksuite/cli/internal/envvars"
"github.com/spf13/pflag"
)

Expand All @@ -26,5 +28,9 @@ func BootstrapInvocationContext(args []string) (cmdutil.InvocationContext, error
if err := fs.Parse(args); err != nil && !errors.Is(err, pflag.ErrHelp) {
return cmdutil.InvocationContext{}, err
}
return cmdutil.InvocationContext{Profile: globals.Profile}, nil
profile := globals.Profile
if profile == "" {
profile = os.Getenv(envvars.CliProfile)
}
return cmdutil.InvocationContext{Profile: profile}, nil
Comment on lines +31 to +35
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In github.com/spf13/pflag, does FlagSet.StringVar set the destination variable to the provided default value immediately when the flag is registered?

💡 Result:

Yes, in github.com/spf13/pflag, FlagSet.StringVar sets the destination variable to the provided default value immediately when the flag is registered.

Citations:

🏁 Script executed:

#!/bin/bash
# Examine the relevant files to trace the profile initialization flow

echo "=== cmd/bootstrap.go (lines 31-35) ==="
sed -n '25,40p' cmd/bootstrap.go

echo ""
echo "=== cmd/root.go (to see where GlobalOptions is initialized) ==="
grep -n "GlobalOptions" cmd/root.go | head -20

echo ""
echo "=== Full cmd/root.go around line 92-110 ==="
sed -n '85,115p' cmd/root.go

echo ""
echo "=== cmd/global_flags.go (RegisterGlobalFlags function) ==="
cat cmd/global_flags.go

echo ""
echo "=== cmd/bootstrap.go (full context) ==="
cat cmd/bootstrap.go

Repository: larksuite/cli

Length of output: 3125

The env-selected profile is overwritten when flags are registered in root.go.

The bootstrap correctly extracts the profile from the environment and passes it via inv.Profile. However, in cmd/root.go, a new GlobalOptions is created with that profile value, then immediately passed to RegisterGlobalFlags. When RegisterGlobalFlags calls fs.StringVar(&opts.Profile, "profile", "", ...), the hardcoded "" default immediately overwrites opts.Profile back to empty. Any code that later reads globals.Profile will miss the environment variable.

Fix 
diff --git a/cmd/global_flags.go b/cmd/global_flags.go
@@
 func RegisterGlobalFlags(fs *pflag.FlagSet, opts *GlobalOptions) {
-	fs.StringVar(&opts.Profile, "profile", "", "use a specific profile")
+	fs.StringVar(&opts.Profile, "profile", opts.Profile, "use a specific profile")
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@cmd/bootstrap.go` around lines 31 - 35, The profile value extracted into
InvocationContext.Profile is being lost because RegisterGlobalFlags uses
fs.StringVar(&opts.Profile, "profile", "", ...) which overwrites the pre-set
opts.Profile with an empty default; update the flag registration so the default
value is the current opts.Profile (e.g. fs.StringVar(&opts.Profile, "profile",
opts.Profile, ...)) when creating GlobalOptions in root initialization (ensure
the GlobalOptions/Profile passed from bootstrap is preserved and not clobbered
by the flag registration in RegisterGlobalFlags).

}
22 changes: 22 additions & 0 deletions cmd/bootstrap_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,28 @@ func TestBootstrapInvocationContext_ShortHelp(t *testing.T) {
}
}

func TestBootstrapInvocationContext_EnvVarFallback(t *testing.T) {
t.Setenv("LARKSUITE_CLI_PROFILE", "env-profile")
inv, err := BootstrapInvocationContext([]string{"auth", "status"})
if err != nil {
t.Fatalf("BootstrapInvocationContext() error = %v", err)
}
if inv.Profile != "env-profile" {
t.Fatalf("profile = %q, want %q", inv.Profile, "env-profile")
}
}

func TestBootstrapInvocationContext_FlagOverridesEnvVar(t *testing.T) {
t.Setenv("LARKSUITE_CLI_PROFILE", "env-profile")
inv, err := BootstrapInvocationContext([]string{"--profile", "flag-profile", "auth", "status"})
if err != nil {
t.Fatalf("BootstrapInvocationContext() error = %v", err)
}
if inv.Profile != "flag-profile" {
t.Fatalf("profile = %q, want %q", inv.Profile, "flag-profile")
}
}

func TestBootstrapInvocationContext_HelpWithProfile(t *testing.T) {
inv, err := BootstrapInvocationContext([]string{"--profile", "target", "--help"})
if err != nil {
Expand Down
71 changes: 64 additions & 7 deletions internal/core/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"strings"
"unicode/utf8"

"github.com/larksuite/cli/internal/envvars"
"github.com/larksuite/cli/internal/keychain"
"github.com/larksuite/cli/internal/output"
"github.com/larksuite/cli/internal/validate"
Expand Down Expand Up @@ -86,6 +87,52 @@ func (m *MultiAppConfig) CurrentAppConfig(profileOverride string) *AppConfig {
return nil
}

// FindAppByID searches apps by appId and returns a pointer to the matching AppConfig.
// Returns a ConfigError listing available IDs if not found.
func FindAppByID(apps []AppConfig, appID string) (*AppConfig, error) {
for i := range apps {
if apps[i].AppId == appID {
return &apps[i], nil
}
}
ids := make([]string, len(apps))
for i, a := range apps {
ids[i] = a.AppId
}
return nil, &ConfigError{
Code: 2,
Type: "config",
Message: fmt.Sprintf("app %q not found in config; available: %s", appID, strings.Join(ids, ", ")),
Hint: "check LARKSUITE_CLI_PROFILE or run `lark-cli config init`",
}
}
Comment on lines +90 to +108
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 FindAppByID is exported but has no production callers

FindAppByID is only referenced in its own unit tests. The existing MultiAppConfig.FindApp already handles lookup-by-appId (as a second-pass after name matching). If this is scaffolding for future use, a brief comment would help; if it is not needed, removing it would keep the public API surface minimal.


// ActiveApp returns the active app from a MultiAppConfig.
// Resolution priority: LARKSUITE_CLI_PROFILE env var > CurrentApp field > Apps[0].
func ActiveApp(multi *MultiAppConfig) (*AppConfig, error) {
if len(multi.Apps) == 0 {
return nil, &ConfigError{Code: 2, Type: "config", Message: "no apps configured", Hint: "run `lark-cli config init`"}
}
if envProfile := os.Getenv(envvars.CliProfile); envProfile != "" {
app := multi.CurrentAppConfig(envProfile)
if app == nil {
return nil, &ConfigError{
Code: 2,
Type: "config",
Message: fmt.Sprintf("profile %q not found", envProfile),
Hint: fmt.Sprintf("available profiles: %s", formatProfileNames(multi.ProfileNames())),
}
}
return app, nil
}
// Fall back to CurrentAppConfig's own resolution (CurrentApp field > Apps[0]).
app := multi.CurrentAppConfig("")
if app == nil {
return nil, &ConfigError{Code: 2, Type: "config", Message: "no apps configured", Hint: "run `lark-cli config init`"}
Comment on lines +128 to +131
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don’t collapse a stale currentApp into no apps configured.

When Apps is non-empty and CurrentApp points to a deleted profile, CurrentAppConfig("") returns nil intentionally. This branch now reports "no apps configured", which is misleading and hides the actual remediation path.

Suggested fix 
  // Fall back to CurrentAppConfig's own resolution (CurrentApp field > Apps[0]).
  app := multi.CurrentAppConfig("")
  if app == nil {
-		return nil, &ConfigError{Code: 2, Type: "config", Message: "no apps configured", Hint: "run `lark-cli config init`"}
+		return nil, &ConfigError{
+			Code:    2,
+			Type:    "config",
+			Message: fmt.Sprintf("profile %q not found", multi.CurrentApp),
+			Hint:    fmt.Sprintf("available profiles: %s", formatProfileNames(multi.ProfileNames())),
+		}
  }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@internal/core/config.go` around lines 128 - 131, Current logic treats a nil
return from multi.CurrentAppConfig("") as "no apps configured", which is
misleading when Apps is non-empty but CurrentApp refers to a deleted profile;
update the branch around CurrentAppConfig("") to detect this case: if
multi.CurrentAppConfig("") returns nil and len(multi.Apps) > 0 (or
multi.CurrentApp != ""), return a ConfigError with a distinct message (e.g.,
"current app profile not found") and a hint guiding the user to select or
re-create the profile (instead of "no apps configured"); reference the
CurrentAppConfig function and the multi.CurrentApp/multi.Apps fields to locate
where to add this conditional and new ConfigError.

}
return app, nil
}

// FindApp looks up an app by name, then by appId. Returns nil if not found.
// Name match takes priority: if profile A has Name "X" and profile B has AppId "X",
// FindApp("X") returns profile A.
Expand Down Expand Up @@ -239,14 +286,24 @@ func RequireConfigForProfile(kc keychain.KeychainAccess, profileOverride string)

// ResolveConfigFromMulti resolves a single-app config from an already-loaded MultiAppConfig.
// This avoids re-reading the config file when the caller has already loaded it.
// Resolution priority: profileOverride > LARKSUITE_CLI_PROFILE env var > config.CurrentApp > Apps[0].
func ResolveConfigFromMulti(raw *MultiAppConfig, kc keychain.KeychainAccess, profileOverride string) (*CliConfig, error) {
app := raw.CurrentAppConfig(profileOverride)
if app == nil {
return nil, &ConfigError{
Code: 2,
Type: "config",
Message: fmt.Sprintf("profile %q not found", profileOverride),
Hint: fmt.Sprintf("available profiles: %s", formatProfileNames(raw.ProfileNames())),
var app *AppConfig
if profileOverride != "" {
app = raw.CurrentAppConfig(profileOverride)
if app == nil {
return nil, &ConfigError{
Code: 2,
Type: "config",
Message: fmt.Sprintf("profile %q not found", profileOverride),
Hint: fmt.Sprintf("available profiles: %s", formatProfileNames(raw.ProfileNames())),
}
}
} else {
var err error
app, err = ActiveApp(raw)
if err != nil {
return nil, err
}
}

Expand Down
192 changes: 183 additions & 9 deletions internal/core/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,174 @@ func TestMultiAppConfig_RoundTrip(t *testing.T) {
}
}

// noopKeychain satisfies keychain.KeychainAccess for tests (returns empty, no error).
type noopKeychain struct{}

func (n *noopKeychain) Get(service, account string) (string, error) { return "", nil }
func (n *noopKeychain) Set(service, account, value string) error { return nil }
func (n *noopKeychain) Remove(service, account string) error { return nil }

func TestFindAppByID_Found(t *testing.T) {
apps := []AppConfig{
{AppId: "cli_aaa", Brand: BrandFeishu},
{AppId: "cli_bbb", Brand: BrandLark},
}
got, err := FindAppByID(apps, "cli_bbb")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if got.AppId != "cli_bbb" {
t.Errorf("AppId = %q, want %q", got.AppId, "cli_bbb")
}
}

func TestFindAppByID_NotFound(t *testing.T) {
apps := []AppConfig{
{AppId: "cli_aaa", Brand: BrandFeishu},
}
_, err := FindAppByID(apps, "cli_zzz")
if err == nil {
t.Fatal("expected error for missing app")
}
var cfgErr *ConfigError
if !errors.As(err, &cfgErr) {
t.Fatalf("expected ConfigError, got %T", err)
}
}

func TestActiveApp_EnvVar(t *testing.T) {
t.Setenv("LARKSUITE_CLI_PROFILE", "cli_bbb")
multi := &MultiAppConfig{
Apps: []AppConfig{
{AppId: "cli_aaa"},
{AppId: "cli_bbb"},
},
}
got, err := ActiveApp(multi)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if got.AppId != "cli_bbb" {
t.Errorf("AppId = %q, want %q", got.AppId, "cli_bbb")
}
}

func TestActiveApp_FallsBackToFirst(t *testing.T) {
t.Setenv("LARKSUITE_CLI_PROFILE", "")
multi := &MultiAppConfig{
Apps: []AppConfig{
{AppId: "cli_aaa"},
{AppId: "cli_bbb"},
},
}
got, err := ActiveApp(multi)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if got.AppId != "cli_aaa" {
t.Errorf("AppId = %q, want %q", got.AppId, "cli_aaa")
}
}

func TestActiveApp_FallsBackToCurrentApp(t *testing.T) {
t.Setenv("LARKSUITE_CLI_PROFILE", "")
multi := &MultiAppConfig{
CurrentApp: "cli_bbb",
Apps: []AppConfig{
{AppId: "cli_aaa"},
{AppId: "cli_bbb"},
},
}
got, err := ActiveApp(multi)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if got.AppId != "cli_bbb" {
t.Errorf("AppId = %q, want %q", got.AppId, "cli_bbb")
}
}

func TestActiveApp_EnvVarNotFound(t *testing.T) {
t.Setenv("LARKSUITE_CLI_PROFILE", "cli_missing")
multi := &MultiAppConfig{
Apps: []AppConfig{{AppId: "cli_aaa"}},
}
_, err := ActiveApp(multi)
if err == nil {
t.Fatal("expected error for missing app")
}
}

func TestRequireConfig_EnvVarSelectsApp(t *testing.T) {
tmp := t.TempDir()
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", tmp)
t.Setenv("LARKSUITE_CLI_PROFILE", "cli_bbb")

config := &MultiAppConfig{
Apps: []AppConfig{
{AppId: "cli_aaa", AppSecret: PlainSecret("sec_a"), Brand: BrandFeishu, Users: []AppUser{}},
{AppId: "cli_bbb", AppSecret: PlainSecret("sec_b"), Brand: BrandLark, Users: []AppUser{{UserOpenId: "ou_123", UserName: "bob"}}},
},
}
if err := SaveMultiAppConfig(config); err != nil {
t.Fatalf("save: %v", err)
}

got, err := RequireConfig(&noopKeychain{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if got.AppID != "cli_bbb" {
t.Errorf("AppID = %q, want %q", got.AppID, "cli_bbb")
}
if got.UserOpenId != "ou_123" {
t.Errorf("UserOpenId = %q, want %q", got.UserOpenId, "ou_123")
}
}

func TestRequireConfig_DefaultsToFirstApp(t *testing.T) {
tmp := t.TempDir()
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", tmp)
t.Setenv("LARKSUITE_CLI_PROFILE", "")

config := &MultiAppConfig{
Apps: []AppConfig{
{AppId: "cli_first", AppSecret: PlainSecret("sec"), Brand: BrandFeishu, Users: []AppUser{}},
},
}
if err := SaveMultiAppConfig(config); err != nil {
t.Fatalf("save: %v", err)
}

got, err := RequireConfig(&noopKeychain{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if got.AppID != "cli_first" {
t.Errorf("AppID = %q, want %q", got.AppID, "cli_first")
}
}

func TestRequireConfig_EnvVarNotFound(t *testing.T) {
tmp := t.TempDir()
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", tmp)
t.Setenv("LARKSUITE_CLI_PROFILE", "cli_missing")

config := &MultiAppConfig{
Apps: []AppConfig{
{AppId: "cli_aaa", AppSecret: PlainSecret("sec"), Brand: BrandFeishu, Users: []AppUser{}},
},
}
if err := SaveMultiAppConfig(config); err != nil {
t.Fatalf("save: %v", err)
}

_, err := RequireConfig(&noopKeychain{})
if err == nil {
t.Fatal("expected error for missing app")
}
}

func TestResolveConfigFromMulti_RejectsSecretKeyMismatch(t *testing.T) {
raw := &MultiAppConfig{
Apps: []AppConfig{
Expand Down Expand Up @@ -164,27 +332,33 @@ func TestResolveConfigFromMulti_MatchingKeychainRefPassesValidation(t *testing.T
}
}

func TestResolveConfigFromMulti_DoesNotUseEnvProfileFallback(t *testing.T) {
t.Setenv("LARKSUITE_CLI_PROFILE", "missing")
func TestResolveConfigFromMulti_ProfileOverrideTakesPrecedenceOverEnv(t *testing.T) {
t.Setenv("LARKSUITE_CLI_PROFILE", "env-profile")

raw := &MultiAppConfig{
CurrentApp: "active",
Apps: []AppConfig{
{
Name: "active",
AppId: "cli_active",
AppSecret: PlainSecret("secret"),
Name: "env-profile",
AppId: "cli_env",
AppSecret: PlainSecret("secret_env"),
Brand: BrandFeishu,
},
{
Name: "explicit",
AppId: "cli_explicit",
AppSecret: PlainSecret("secret_explicit"),
Brand: BrandFeishu,
},
},
}

cfg, err := ResolveConfigFromMulti(raw, nil, "")
// When profileOverride is set, it should be used instead of the env var.
cfg, err := ResolveConfigFromMulti(raw, nil, "explicit")
if err != nil {
t.Fatalf("ResolveConfigFromMulti() error = %v", err)
}
if cfg.ProfileName != "active" {
t.Fatalf("ResolveConfigFromMulti() profile = %q, want %q", cfg.ProfileName, "active")
if cfg.ProfileName != "explicit" {
t.Fatalf("ResolveConfigFromMulti() profile = %q, want %q", cfg.ProfileName, "explicit")
}
}

Expand Down
1 change: 1 addition & 0 deletions internal/envvars/envvars.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ const (
CliTenantAccessToken = "LARKSUITE_CLI_TENANT_ACCESS_TOKEN"
CliDefaultAs = "LARKSUITE_CLI_DEFAULT_AS"
CliStrictMode = "LARKSUITE_CLI_STRICT_MODE"
CliProfile = "LARKSUITE_CLI_PROFILE"
)
Loading