feat: support project-level profile binding#1504
Closed
ennann wants to merge 1 commit into
Closed
Conversation
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (22)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (13)
📝 WalkthroughWalkthroughIntroduces project-level profile binding for the Lark CLI. A ChangesProject-level profile binding
Sequence Diagram(s)sequenceDiagram
rect rgba(70, 130, 180, 0.5)
Note over CLI,BootstrapInvocationContext: Startup
end
participant CLI
participant BootstrapInvocationContext
participant core as core.ResolveProjectProfile
participant NewDefault as cmdutil.NewDefault
participant DefaultAccountProvider
participant MultiAppConfig
CLI->>BootstrapInvocationContext: args (no --profile flag)
BootstrapInvocationContext->>core: walk up to .git, find .lark-cli/config.json
core-->>BootstrapInvocationContext: ProjectProfileBinding{Profile, Path}
BootstrapInvocationContext-->>NewDefault: InvocationContext{ProfileSourceProject, ProfileConfigPath}
NewDefault->>DefaultAccountProvider: NewDefaultAccountProviderWithSource(kc, profile, ProjectSource, configPath)
rect rgba(200, 80, 80, 0.5)
Note over DefaultAccountProvider,MultiAppConfig: ResolveAccount
end
DefaultAccountProvider->>MultiAppConfig: LoadMultiAppConfig()
alt profile missing from config
DefaultAccountProvider-->>CLI: ProjectProfileNotFoundError (hint: profile list)
else profile found
DefaultAccountProvider-->>CLI: Account credentials
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
size/L
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds project-local profile binding support so the CLI can resolve an “effective profile” from a repo’s .lark-cli/config.json, expose it via new profile subcommands, and fail closed with a typed config error when a project-bound profile is missing.
Changes:
- Introduces project config discovery/read/write helpers and related tests (
internal/core/project_config.go+ tests). - Propagates
ProfileSource/ProfileConfigPaththrough invocation/factory/credential resolution and adds “project profile missing” fail-closed behavior. - Adds
profile current|bind|unbindcommands and extends bootstrap logic to read project bindings (with selective skipping for certain commands).
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/credential/integration_test.go | Adds an integration test asserting project-bound missing profile fails closed with *core.ConfigError. |
| internal/credential/default_provider.go | Extends default account provider to retain profile source/path and emit project-specific “not found” errors early. |
| internal/core/project_config_test.go | Adds tests for project config discovery, read/write, validation, and removal semantics. |
| internal/core/project_config.go | Implements .lark-cli/config.json project binding discovery/updating/removal and the ProjectProfileNotFoundError helper. |
| internal/core/config.go | Introduces core.ProfileSource and constants (cli, project, global). |
| internal/cmdutil/factory_default.go | Threads ProfileSource and ProfileConfigPath into credential provider construction. |
| internal/cmdutil/factory.go | Extends InvocationContext to carry profile source and originating config path. |
| cmd/profile/project_bind.go | Adds profile bind and profile unbind commands to manage project profile bindings. |
| cmd/profile/profile_test.go | Adds tests for bind/unbind/current behavior and typed error expectations. |
| cmd/profile/profile.go | Registers new `profile current |
| cmd/profile/current.go | Adds profile current command to show effective profile (including source and config path). |
| cmd/config/strict_mode.go | Uses a shared helper to return a project-aware “no active profile” error. |
| cmd/config/show.go | Uses the project-aware “no active profile” error helper. |
| cmd/config/default_as.go | Uses the project-aware “no active profile” error helper. |
| cmd/config/active_profile.go | Adds helper to produce project-aware “no active profile” errors. |
| cmd/bootstrap_test.go | Adds tests for project profile resolution precedence and malformed config behavior. |
| cmd/bootstrap.go | Adds project profile resolution during bootstrap with heuristics to skip lookup for certain commands. |
| cmd/auth/logout.go | Returns ProjectProfileNotFoundError when a project-bound active profile is missing. |
| cmd/auth/list.go | Returns ProjectProfileNotFoundError when a project-bound active profile is missing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
54
to
100
| func skipProjectProfileLookup(args []string) bool { | ||
| for _, arg := range args { | ||
| if arg == "-h" || arg == "--help" { | ||
| return true | ||
| } | ||
| } | ||
| parts := firstPositionalArgs(args, 2) | ||
| if len(parts) == 0 { | ||
| return false | ||
| } | ||
| switch parts[0] { | ||
| case "completion", "__complete", "__completeNoDesc": | ||
| return true | ||
| case "profile": | ||
| return len(parts) < 2 || parts[1] != "current" | ||
| case "config": | ||
| return len(parts) >= 2 && (parts[1] == "bind" || parts[1] == "init" || parts[1] == "remove") | ||
| default: | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| func firstPositionalArgs(args []string, limit int) []string { | ||
| var out []string | ||
| skipNext := false | ||
| for _, arg := range args { | ||
| if skipNext { | ||
| skipNext = false | ||
| continue | ||
| } | ||
| if arg == "--" { | ||
| break | ||
| } | ||
| if arg == "--profile" { | ||
| skipNext = true | ||
| continue | ||
| } | ||
| if strings.HasPrefix(arg, "--profile=") || strings.HasPrefix(arg, "-") { | ||
| continue | ||
| } | ||
| out = append(out, arg) | ||
| if len(out) >= limit { | ||
| return out | ||
| } | ||
| } | ||
| return out | ||
| } |
Comment on lines
+50
to
+57
| source := f.Invocation.ProfileSource | ||
| if source == "" { | ||
| source = core.ProfileSourceGlobal | ||
| } | ||
| configPath := core.GetConfigPath() | ||
| if source == core.ProfileSourceProject { | ||
| configPath = f.Invocation.ProfileConfigPath | ||
| } |
Comment on lines
70
to
+72
| func NewDefaultAccountProvider(kc func() keychain.KeychainAccess, profile string) *DefaultAccountProvider { | ||
| return NewDefaultAccountProviderWithSource(kc, profile, "", "") | ||
| } |
Comment on lines
+72
to
+75
| app := multi.CurrentAppConfig(f.Invocation.Profile) | ||
| if app == nil && f.Invocation.Profile != "" && f.Invocation.ProfileSource == core.ProfileSourceProject { | ||
| return core.ProjectProfileNotFoundError(f.Invocation.Profile, f.Invocation.ProfileConfigPath, multi.ProfileNames()) | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/auth/list.go`:
- Around line 73-75: The check at lines 73-75 for ProjectProfileNotFoundError is
unreachable because the function returns early at line 48 when len(multi.Apps)
== 0. Move the project profile validation logic that checks if app is nil and
f.Invocation.Profile is set with ProfileSourceProject before or into the early
return condition at line 48, so that a missing project-bound profile properly
returns ProjectProfileNotFoundError instead of exiting with success. This
ensures the fail-closed behavior matches how
internal/credential/default_provider.go handles missing profiles.
In `@cmd/auth/logout.go`:
- Around line 62-64: The project-bound missing profile validation on lines 62-64
is unreachable because the code returns early when len(multi.Apps) == 0 (at line
48), breaking the fail-closed behavior for project-sourced profiles. Move the
project profile validation check (the condition checking for app == nil with
ProfileSource == core.ProfileSourceProject and the call to
core.ProjectProfileNotFoundError) to execute before the early return on
len(multi.Apps) == 0, ensuring that project-sourced profile validation always
occurs regardless of whether apps are empty.
In `@cmd/profile/current.go`:
- Around line 42-48: The error handling after the CurrentAppConfig call needs to
distinguish between CLI and Project profile sources. When app is nil and
f.Invocation.ProfileSource is CLI (not ProfileSourceProject), this represents
invalid user input and should return
errs.NewValidationError(errs.SubtypeInvalidArgument, ...) with
.WithParam("--profile") instead of the current ConfigError. Keep the existing
ProjectProfileNotFoundError branch for the project-source case unchanged, and
only return the ConfigError when neither CLI nor project source conditions apply
(or when the profile is empty).
In `@cmd/profile/profile_test.go`:
- Around line 118-120: The test
TestProfileBindRun_InvalidProfileReturnsTypedError lacks config directory
isolation, which can cause cross-test state leakage. Before calling
cmdutil.TestFactory(t, nil), add a call to t.Setenv("LARKSUITE_CLI_CONFIG_DIR",
t.TempDir()) to set up an isolated temporary config directory for this test,
matching the pattern used in other tests in this file.
🪄 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: 86c22449-ed5d-4ecd-9d11-7d7e3fa9c7b3
📒 Files selected for processing (19)
cmd/auth/list.gocmd/auth/logout.gocmd/bootstrap.gocmd/bootstrap_test.gocmd/config/active_profile.gocmd/config/default_as.gocmd/config/show.gocmd/config/strict_mode.gocmd/profile/current.gocmd/profile/profile.gocmd/profile/profile_test.gocmd/profile/project_bind.gointernal/cmdutil/factory.gointernal/cmdutil/factory_default.gointernal/core/config.gointernal/core/project_config.gointernal/core/project_config_test.gointernal/credential/default_provider.gointernal/credential/integration_test.go
Comment on lines
+42
to
+48
| app := multi.CurrentAppConfig(f.Invocation.Profile) | ||
| if app == nil { | ||
| if f.Invocation.Profile != "" && f.Invocation.ProfileSource == core.ProfileSourceProject { | ||
| return core.ProjectProfileNotFoundError(f.Invocation.Profile, f.Invocation.ProfileConfigPath, multi.ProfileNames()) | ||
| } | ||
| return errs.NewConfigError(errs.SubtypeNotConfigured, "no active profile").WithHint("run: lark-cli profile list") | ||
| } |
There was a problem hiding this comment.
Return a validation error when an explicit CLI profile is missing.
If f.Invocation.ProfileSource is CLI and CurrentAppConfig(...) returns nil, this is an invalid --profile input, not a SubtypeNotConfigured state. Please classify it as errs.NewValidationError(errs.SubtypeInvalidArgument, ...) with .WithParam("--profile") (and keep the project-source branch as-is).
Suggested patch
app := multi.CurrentAppConfig(f.Invocation.Profile)
if app == nil {
- if f.Invocation.Profile != "" && f.Invocation.ProfileSource == core.ProfileSourceProject {
- return core.ProjectProfileNotFoundError(f.Invocation.Profile, f.Invocation.ProfileConfigPath, multi.ProfileNames())
- }
+ if f.Invocation.Profile != "" {
+ if f.Invocation.ProfileSource == core.ProfileSourceProject {
+ return core.ProjectProfileNotFoundError(f.Invocation.Profile, f.Invocation.ProfileConfigPath, multi.ProfileNames())
+ }
+ if f.Invocation.ProfileSource == core.ProfileSourceCLI {
+ return errs.NewValidationError(errs.SubtypeInvalidArgument, "profile %q not found", f.Invocation.Profile).
+ WithParam("--profile").
+ WithHint("run: lark-cli profile list")
+ }
+ }
return errs.NewConfigError(errs.SubtypeNotConfigured, "no active profile").WithHint("run: lark-cli profile list")
}As per coding guidelines, cmd/**/*.go user flag/argument validation failures should use errs.NewValidationError(errs.SubtypeInvalidArgument, ...).WithParam("--flag").
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/profile/current.go` around lines 42 - 48, The error handling after the
CurrentAppConfig call needs to distinguish between CLI and Project profile
sources. When app is nil and f.Invocation.ProfileSource is CLI (not
ProfileSourceProject), this represents invalid user input and should return
errs.NewValidationError(errs.SubtypeInvalidArgument, ...) with
.WithParam("--profile") instead of the current ConfigError. Keep the existing
ProjectProfileNotFoundError branch for the project-source case unchanged, and
only return the ConfigError when neither CLI nor project source conditions apply
(or when the profile is empty).
Source: Coding guidelines
ennann
force-pushed
the
codex/project-profile-config
branch
from
5199a3c to
02da6d9
Compare
8 tasks
Author
|
Superseded by #1506. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds project-local profile binding via
.lark-cli/config.json, so commands can automatically use the profile selected for the current repository without requiring--profileon every invocation. Explicit--profilestill takes precedence, and the project file stores only the profile name, not credentials or login state.Supersedes #1502 with a cleaner single-commit branch and the finalized project config path.
Changes
profile bind <profile>,profile unbind, andprofile current..lark-cli/config.jsonduring bootstrap, stopping at the Git root.bytedance,team-prod, andlark-boeprofile names.Test Plan
go test ./cmd ./cmd/profile ./cmd/config ./cmd/auth ./internal/core ./internal/cmdutil ./internal/credentialgo vet ./...gofmt -l .go mod tidy && git diff --exit-code -- go.mod go.sumgo run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=origin/main(0 issues; existing gocritic disabled-check warning only)make buildprofile bind team-prod,profile current,--profile bytedance profile current, andprofile unbindmake unit-testcurrently fails in existingshortcuts/minutesdownload tests becauseexample.compresigned download URLs are blocked as local/internal hosts; related packages and all new project-profile tests pass.Related Issues
中文说明
这个 PR 增加项目级 profile 绑定能力。项目可以通过
.lark-cli/config.json记录当前仓库默认使用的 profile，之后普通命令不需要重复传--profile；显式传入--profile时仍然优先使用命令行参数。项目配置文件只保存 profile 名称，不保存 app secret、token 或登录态。示例里包含
bytedance、team-prod、lark-boe这类租户/环境命名，用来说明 profile 名称本身可以承载团队内部环境语义。Summary by CodeRabbit
Release Notes
New Features
profile bindto set a project-level profile, andprofile unbindto remove it.profile currentto show the active profile along with its source and resolved config path.--profileoverrides project settings.Bug Fixes