test - #1620
Closed
maojinze7-prog wants to merge 0 commit into
Closed
Conversation
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughIdentity selection is centralized in ChangesIdentity selection and whoami command
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (58.33%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1620 +/- ##
==========================================
- Coverage 74.64% 74.62% -0.03%
==========================================
Files 806 807 +1
Lines 81386 81521 +135
==========================================
+ Hits 60752 60831 +79
- Misses 16101 16154 +53
- Partials 4533 4536 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@bb6fdc8b33f45e8d2a2d89d7115248497e2ae172🧩 Skill updatenpx skills add larksuite/cli#feat/whoami-command -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmd/whoami/whoami.go (1)
36-41: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winThread the command context instead of
context.Background().With
--verify,whoamiRuncallsidentitydiag.Diagnosewhich performs network I/O. Usingcontext.Background()means the root command's context (and therefore Ctrl+C/parent cancellation) never reaches the verification HTTP call. TheRunEclosure has access tocmd.Context(); consider passing it through towhoamiRun.♻️ Proposed change
- RunE: func(cmd *cobra.Command, args []string) error { - if runF != nil { - return runF(opts) - } - return whoamiRun(opts) - }, + RunE: func(cmd *cobra.Command, args []string) error { + if runF != nil { + return runF(opts) + } + return whoamiRun(cmd.Context(), opts) + },-func whoamiRun(opts *WhoamiOptions) error { +func whoamiRun(ctx context.Context, opts *WhoamiOptions) error { f := opts.Factory ... - diagnostics := identitydiag.Diagnose(context.Background(), f, cfg, opts.Verify) + diagnostics := identitydiag.Diagnose(ctx, f, cfg, opts.Verify)Also applies to: 69-69
🤖 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/whoami/whoami.go` around lines 36 - 41, The whoami command is dropping the Cobra command context, so cancellation from Ctrl+C/parent commands does not reach the verification network call. Update the RunE closure on the whoami command to pass cmd.Context() through to whoamiRun, and adjust whoamiRun (and any related helpers it calls, such as the verify/identitydiag.Diagnose path) to accept and use that context instead of context.Background().
🤖 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/whoami/whoami.go`:
- Around line 55-62: The error handling in whoami’s config load path only
special-cases errs.ConfigError and returns all other f.Config() failures
directly, which breaks the typed-error contract. In whoami.go, update the cfg,
err := f.Config() branch to wrap any unclassified error with
errs.NewInternalError(errs.SubtypeUnknown, ...) and attach the original error
via WithCause(err), while keeping the existing errors.As(err, &cfgErr) path for
renderNotConfigured.
---
Nitpick comments:
In `@cmd/whoami/whoami.go`:
- Around line 36-41: The whoami command is dropping the Cobra command context,
so cancellation from Ctrl+C/parent commands does not reach the verification
network call. Update the RunE closure on the whoami command to pass
cmd.Context() through to whoamiRun, and adjust whoamiRun (and any related
helpers it calls, such as the verify/identitydiag.Diagnose path) to accept and
use that context instead of context.Background().
🪄 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: e7fb53b1-a5e9-4694-80fe-a7be74975473
📒 Files selected for processing (7)
cmd/auth/status.gocmd/build.gocmd/whoami/whoami.gocmd/whoami/whoami_test.gointernal/identitydiag/diagnostics.gointernal/identitydiag/diagnostics_test.goskills/lark-shared/SKILL.md
Comment on lines
+55
to
+62
| cfg, err := f.Config() | ||
| if err != nil { | ||
| var cfgErr *errs.ConfigError | ||
| if errors.As(err, &cfgErr) { | ||
| return renderNotConfigured(opts, cfgErr) | ||
| } | ||
| return err | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect what error types f.Config() can return.
ast-grep run --pattern 'func ($_ $_) Config() (*core.CliConfig, error) { $$$ }' --lang go internal/cmdutil 2>/dev/null
rg -nP 'Config\s+func\(\)\s*\(\*core\.CliConfig,\s*error\)' --type=go internal/cmdutil -C3Repository: larksuite/cli
Length of output: 757
🏁 Script executed:
#!/bin/bash
# Find where Factory struct is instantiated and Config field is assigned
rg -n 'Factory{.*Config' --type=go -A5 --include='*.go'
# Also search for .Config = assignment patterns
rg -n '\.Config\s*=\s*func' --type=go -B2 -A10
# Search for implementations of config loading functions
ast-grep run --pattern 'func $_() (*core.CliConfig, error) { $$$ }' --lang go . --limit 10Repository: larksuite/cli
Length of output: 593
🏁 Script executed:
#!/bin/bash
# Find where Factory struct is instantiated and Config field is assigned
rg -n 'Factory{.*Config' --type=go -A5
# Also search for .Config = assignment patterns
rg -n '\.Config\s*=\s*func' --type=go -B2 -A10
# Search for implementations of config loading functions
ast-grep run --pattern 'func $_() (*core.CliConfig, error) { $$$ }' --lang go . --limit 10Repository: larksuite/cli
Length of output: 258
🏁 Script executed:
#!/bin/bash
# Fix regex syntax and search for Factory struct initialization
rg -n 'Factory{' --type=go -A10 | head -100
# Search for Config field assignment with lambda
rg -n 'Config:' --type=go -B2 -A10
# Find all functions returning (*core.CliConfig, error)
ast-grep run --pattern 'func $NAME() (*core.CliConfig, error) { $$$ }' --lang go .Repository: larksuite/cli
Length of output: 231
🏁 Script executed:
#!/bin/bash
# Search for Factory struct initialization (escape braces)
rg -n 'Factory\{' --type=go -A15 | head -150
# Search for Config field assignment
rg -n 'Config:' --type=go -B2 -A15 | head -150
# Find all functions returning (*core.CliConfig, error)
ast-grep run --pattern 'func $NAME() (*core.CliConfig, error) { $$$ }' --lang go .Repository: larksuite/cli
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Search for Factory struct initialization more broadly
rg -n 'Factory' --type=go | grep -E '(=|return|&)' | head -50
# Look at the factory.go file to see how Config is typically set
cat -n internal/cmdutil/factory.go | head -80
# Search for any function that loads config
rg -n 'LoadConfig|loadConfig|readConfig' --type=go -i | head -30Repository: larksuite/cli
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Read the factory.go file to see the Factory struct and see if there are NewFactory or similar functions
cat -n internal/cmdutil/factory.go
# Search for file names containing "factory" to find related files
fd -t f --extension go -g '*factory*'
# Look for any place where Factory is used or created
rg -n 'NewFactory|NewCmdFactory|MakeFactory' --type=goRepository: larksuite/cli
Length of output: 10173
🏁 Script executed:
#!/bin/bash
# Search for usages of Factory struct initialization to find where Config is assigned
rg -n 'Factory{' --type=go -A20
# Search for patterns like ".Config = " or ".Config:"
rg -n '\.Config\s*[:=]' --type=go -B2 -A5 | head -100
# Look for the main entry point to see how Factory is created
rg -n 'func main|NewInvocationContext|NewFactory' --type=go | head -30
# Find cmd/root.go or similar initialization files
fd -t f -g '*root*' --extension go cmdRepository: larksuite/cli
Length of output: 231
🏁 Script executed:
#!/bin/bash
# Escape braces properly for rg
rg -n 'Factory\{' --type=go -A20
# Search for patterns like ".Config =" or ".Config:"
rg -n '\.Config\s*[:=]' --type=go -B2 -A5 | head -100
# Look for the main entry point to see how Factory is created
rg -n 'func main|NewInvocationContext|NewFactory' --type=go | head -30
# Find cmd/root.go or similar initialization files
fd -t f -g '*root*' --extension go cmdRepository: larksuite/cli
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Search for where Factory is initialized (look for struct literals)
rg -n 'func.*Config.*func' --type=go -B2 -A10 | head -100
# Find main.go or root command files
fd -t f 'main.go' cmd
fd -t f 'root.go' cmd
# Read the cmd/root.go or main file to see Factory initialization
cat -n cmd/main.go 2>/dev/null || cat -n cmd/root.go 2>/dev/null || find cmd -name '*.go' -exec sh -c 'echo "=== {} ===" && head -80 {}' \; | head -200Repository: larksuite/cli
Length of output: 151
🏁 Script executed:
#!/bin/bash
# List all Go files in cmd directory to find initialization code
find cmd -name '*.go' -type f
# Search for NewInvocationContext or where Factory fields are set
rg -n 'InvocationContext' --type=go | head -30
# Look for any file that constructs Factory struct
rg -n 'Config.*:.*func' --type=go -B2 -A10 | head -100Repository: larksuite/cli
Length of output: 3741
🏁 Script executed:
#!/bin/bash
# Read cmd/root.go to see how Factory is initialized
cat -n cmd/root.go | head -150
# Search for where the Config func is assigned in Factory
rg -n 'Config:' --type=go -B2 -A15 | head -200
# Look for factory initialization patterns without braces in pattern
rg -n 'func \(\w+ \*Factory\) Config' --type=go -A30Repository: larksuite/cli
Length of output: 5887
🏁 Script executed:
#!/bin/bash
# Continue reading cmd/root.go to find buildInternal function which likely creates Factory
cat -n cmd/root.go | sed -n '150,250p'
# Search for buildInternal function definition
rg -n 'func buildInternal' --type=go -A50 | head -80
# Search for where Config is assigned in Factory struct literal
rg -n 'Config:' --type=go -B5 -A20 | head -150Repository: larksuite/cli
Length of output: 4932
🏁 Script executed:
#!/bin/bash
# Search for buildInternal function which creates the Factory
rg -n 'func buildInternal' --type=go -A80 | head -120
# Also search for where the Config function is actually assigned/defined
rg -n '\.Config\s*=\s*func' --type=go -B5 -A15
# Look for factory_test.go or other files that might show how Config is set
cat -n internal/cmdutil/factory_test.go | head -100Repository: larksuite/cli
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Search for buildInternal definition more broadly
ast-grep run --pattern 'func $FUNC($CONTEXT, $INV) $TYP { $$$ }' --lang go . | head -20
# Look for the file containing buildInternal
rg -l 'buildInternal' --type=go
# Search for any pattern that assigns Config to a function
rg -n 'Config.*func\(\)' --type=go -B2 -A3 | head -80Repository: larksuite/cli
Length of output: 1480
🏁 Script executed:
#!/bin/bash
# Find the file containing buildInternal
rg -l 'buildInternal' --type=go
# Read cmd/build.go which likely contains buildInternal
cat -n cmd/build.go | head -150Repository: larksuite/cli
Length of output: 159
🏁 Script executed:
#!/bin/bash
# Read cmd/build.go which likely contains buildInternal
cat -n cmd/build.go | head -200
# Search for where Config field is assigned
rg -n 'factory.Config' --type=go -B5 -A5
# Look for factory assignment patterns
rg -n 'f.Config' --type=go -B5 -A5 | head -100Repository: larksuite/cli
Length of output: 9093
Wrap unclassified errors from f.Config() to satisfy the typed-error contract.
Per the coding guidelines for cmd/**/*.go, unclassified lower-layer errors used as final output must be wrapped with errs.NewInternalError(errs.SubtypeUnknown, ...).WithCause(err). The current code only handles *errs.ConfigError explicitly; any other error returned from f.Config() is passed bare, violating the contract.
if err != nil {
var cfgErr *errs.ConfigError
if errors.As(err, &cfgErr) {
return renderNotConfigured(opts, cfgErr)
}
- return err
+ return errs.NewInternalError(errs.SubtypeUnknown, "failed to load configuration").WithCause(err)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| cfg, err := f.Config() | |
| if err != nil { | |
| var cfgErr *errs.ConfigError | |
| if errors.As(err, &cfgErr) { | |
| return renderNotConfigured(opts, cfgErr) | |
| } | |
| return err | |
| } | |
| cfg, err := f.Config() | |
| if err != nil { | |
| var cfgErr *errs.ConfigError | |
| if errors.As(err, &cfgErr) { | |
| return renderNotConfigured(opts, cfgErr) | |
| } | |
| return errs.NewInternalError(errs.SubtypeUnknown, "failed to load configuration").WithCause(err) | |
| } |
🤖 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/whoami/whoami.go` around lines 55 - 62, The error handling in whoami’s
config load path only special-cases errs.ConfigError and returns all other
f.Config() failures directly, which breaks the typed-error contract. In
whoami.go, update the cfg, err := f.Config() branch to wrap any unclassified
error with errs.NewInternalError(errs.SubtypeUnknown, ...) and attach the
original error via WithCause(err), while keeping the existing errors.As(err,
&cfgErr) path for renderNotConfigured.
Source: Coding guidelines
|
|
maojinze7-prog
force-pushed
the
feat/whoami-command
branch
from
June 27, 2026 09:58
bb6fdc8 to
8a268aa
Compare
github-actions
Bot
added
size/M
maojinze7-prog
changed the title
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
2 participants
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
Add a top-level
lark-cli whoamicommand that shows the currently effective identity at a glance: the active profile, app, the effective identity (bot/user), and token status.The underlying identity data already existed via
internal/identitydiag, but it was only reachable throughauth status— JSON-only, without the active profile, and not discoverable under awhoaminame. This command aggregates profile + app + effective identity + token status into a single, human-readable view (with--jsonfor structured output and--verifyfor an optional online check), following the near-universalwhoamiCLI convention (gh,aws sts,gcloud,kubectl).Changes
whoamicommand (cmd/whoami/):--jsonfor structured output (mirrors theauth statuspayload plus a newprofilefield);--verifyfor optional online token verification.identity: none); exits 0 whenever it successfully renders state (includingnone/ not-configured), mirroring thewhoamiconvention.appId,openId,userName,scope, expiry, and token status enums (the same field set asauth status).identitydiag.EffectiveIdentityhelper sowhoamiandauth statusagree on which identity is in effect (no behavior change toauth status; existing regression tests cover it).lark-sharedskill doc.Test Plan
cmd/whoami,internal/identitydiag): JSON output, human-readable output,noneand not-configured handling,--verify, no-secret-leakage, and profile-line rendering; plus anauth statusbackward-compatibility regression for the shared-helper extraction.go build ./...,go vet, andgofmtall clean.Related Issues
N/A
Summary by CodeRabbit
whoamicommand to display the current effective identity and related token/verification status.--jsonfor structured output and--verifyto perform联网校验.whoamiin the root command set so it’s available from the main CLI.