feat: support LARKSUITE_CLI_PROFILE env var for multi-app selection#445
feat: support LARKSUITE_CLI_PROFILE env var for multi-app selection#445meijing0114 wants to merge 2 commits intolarksuite:mainfrom
Conversation
Use LARKSUITE_CLI_PROFILE (not LARKSUITE_CLI_APP_ID which collides with the env credential provider) for profile-based app selection. The env var accepts both profile names and app IDs, consistent with --profile flag. - Add envvars.CliProfile constant - Rename standalone FindApp to FindAppByID to avoid shadowing the method - Use multi.CurrentAppConfig() in ActiveApp for name+appId resolution - Add test for CurrentApp fallback when env var is unset Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…back Commands that use f.Invocation.Profile directly (config show, auth logout, auth list, config default-as) now respect the env var. Priority: --profile flag > LARKSUITE_CLI_PROFILE env var > (none). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-actions
bot
added
the
size/L
📝 WalkthroughWalkthroughThe changes implement environment variable-based profile fallback for the CLI. When no Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds One subtle side-note worth awareness: Confidence Score: 5/5Safe to merge — all findings are P2 style/cleanup suggestions with no impact on the normal CLI code path. The env-var plumbing is correct and backward-compatible, test coverage is thorough, and no runtime errors or data-loss risks were identified. Remaining comments are a stale docstring and an unused exported function. internal/core/config.go — stale docstring on RequireConfigForProfile and unused FindAppByID export. Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as Execute()
participant Boot as BootstrapInvocationContext
participant Env as os.Getenv
participant Fac as NewDefault(inv)
participant DAP as DefaultAccountProvider
participant RCM as ResolveConfigFromMulti
participant AA as ActiveApp
CLI->>Boot: args (os.Args[1:])
Boot->>Boot: parse --profile flag
alt --profile flag set
Boot-->>Fac: InvocationContext{Profile: flagValue}
else flag empty
Boot->>Env: LARKSUITE_CLI_PROFILE
Boot-->>Fac: InvocationContext{Profile: envValue or ""}
end
Fac->>DAP: NewDefaultAccountProvider(kc, inv.Profile)
DAP->>RCM: ResolveConfigFromMulti(multi, kc, profile)
alt profileOverride != ""
RCM->>RCM: CurrentAppConfig(profileOverride)
else profileOverride == "" (direct RequireConfig path)
RCM->>AA: ActiveApp(multi)
AA->>Env: LARKSUITE_CLI_PROFILE
alt env var set
AA-->>RCM: app selected by env var
else env var empty
AA-->>RCM: CurrentApp field or Apps[0]
end
end
RCM-->>DAP: CliConfig
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // 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`", | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/bootstrap.go`:
- Around line 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).
In `@internal/core/config.go`:
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a13ec1b5-cedf-407b-960a-dfd6d398c2b3
📒 Files selected for processing (5)
cmd/bootstrap.gocmd/bootstrap_test.gointernal/core/config.gointernal/core/config_test.gointernal/envvars/envvars.go
| profile := globals.Profile | ||
| if profile == "" { | ||
| profile = os.Getenv(envvars.CliProfile) | ||
| } | ||
| return cmdutil.InvocationContext{Profile: profile}, nil |
There was a problem hiding this comment.
🧩 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:
- 1: https://github.com/spf13/pflag/blob/master/string.go
- 2: Flag default assignments should be lazily applied spf13/pflag#257
- 3: https://github.com/spf13/pflag/commits/master/
🏁 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.goRepository: 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).
| // 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`"} |
There was a problem hiding this comment.
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.
1 participant
Summary
LARKSUITE_CLI_PROFILEenvironment variable to select the active app profile without the--profileCLI flagFindAppByID()andActiveApp()helpers tointernal/core/config.gofor env-var-aware profile resolutionBootstrapInvocationContextso all commands (includingconfig show,auth logout,auth list,config default-as) respect itMotivation
When running multiple lark-cli bots on the same machine (e.g. one for an AI agent platform, one for Claude Code), each agent needs its own app configuration. The
--profileflag works for interactive use, but agents run in fixed environments where an environment variable is the natural selector.Resolution Priority
--profileflag >LARKSUITE_CLI_PROFILEenv var >config.CurrentAppfield >Apps[0]Usage
Backward Compatibility
100%. Users who don't set the env var get identical behavior.
Test plan
FindAppByIDfound/not-found testsActiveAppwith env var / without / not-found testsRequireConfigintegration tests with env var selectionBootstrapInvocationContextenv var fallback + flag-overrides-env testsconfig showwith/without env var🤖 Generated with Claude Code
Summary by CodeRabbit
LARKSUITE_CLI_PROFILEenvironment variable.