test#1658
Closed
maojinze7-prog wants to merge 0 commit into
Closed
Conversation
📝 WalkthroughWalkthroughA new Changeswhoami command
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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
Additional details and impacted files@@ Coverage Diff @@
## main #1658 +/- ##
==========================================
+ Coverage 74.71% 74.73% +0.02%
==========================================
Files 810 811 +1
Lines 81839 81928 +89
==========================================
+ Hits 61142 61228 +86
- Misses 16131 16134 +3
Partials 4566 4566 ☔ 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@8a3276fc0bbe0c887b00bf8870c61ad155d8600f🧩 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: 3
🤖 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_test.go`:
- Around line 186-195: The malformed-config test only checks for a non-nil
error, so it can miss regressions in the typed error contract. Update the
`whoamiRun` error-path assertion in `whoami_test.go` to inspect the returned
error with `errs.ProblemOf`, and verify the expected `category` and `subtype`
for the `SubtypeInvalidConfig` case. Keep the existing `whoamiRun` /
`whoamiOptions` setup, but make the test fail if the typed metadata is rewritten
or lost.
In `@cmd/whoami/whoami.go`:
- Around line 140-149: The whoami command currently ignores unexpected
positional arguments instead of rejecting them; update the cobra.Command setup
in whoamiRun/command construction to add Args validation that returns
errs.NewValidationError(errs.SubtypeInvalidArgument, ...) when extra args are
provided. Make sure the validation is wired through the whoami command path so
RunE still calls runF or whoamiRun only after args are checked, and add a test
in whoami_test.go that covers the invalid-argument failure case.
- Around line 143-147: Propagate the Cobra command context through the whoami
execution path instead of always creating a background context. Update the RunE
path and the whoamiRun function so it uses cmd.Context() (and passes it into
identitydiag.Diagnose) when running --verify, preserving cancellation and
deadlines from the parent command. Keep the existing runF override behavior
intact while ensuring the default whoamiRun flow receives the command context.
🪄 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: becffa63-4e53-4f56-93d0-49d177a88016
📒 Files selected for processing (3)
cmd/build.gocmd/whoami/whoami.gocmd/whoami/whoami_test.go
Comment on lines
+186
to
+195
| t.Run("malformed config passes through error", func(t *testing.T) { | ||
| f, _, _, _ := cmdutil.TestFactory(t, nil) | ||
| f.Config = func() (*core.CliConfig, error) { | ||
| return nil, errs.NewConfigError(errs.SubtypeInvalidConfig, "malformed") | ||
| } | ||
| err := whoamiRun(&whoamiOptions{Factory: f}) | ||
| if err == nil { | ||
| t.Fatal("expected non-nil error for invalid config, got nil") | ||
| } | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert the typed config error contract here.
Line 191 only checks that an error is returned, so this test will miss regressions where whoamiRun rewrites SubtypeInvalidConfig into a different typed error. In *_test.go error-path cases, please assert the returned problem metadata with errs.ProblemOf (at least category and subtype). As per coding guidelines, "Error-path tests must assert typed metadata via errs.ProblemOf (category / subtype / param) and cause preservation, not message substrings alone".
🧪 Proposed assertion upgrade
t.Run("malformed config passes through error", func(t *testing.T) {
f, _, _, _ := cmdutil.TestFactory(t, nil)
f.Config = func() (*core.CliConfig, error) {
return nil, errs.NewConfigError(errs.SubtypeInvalidConfig, "malformed")
}
err := whoamiRun(&whoamiOptions{Factory: f})
if err == nil {
t.Fatal("expected non-nil error for invalid config, got nil")
}
+ p, ok := errs.ProblemOf(err)
+ if !ok {
+ t.Fatalf("expected typed error, got %T", err)
+ }
+ if p.Subtype != errs.SubtypeInvalidConfig {
+ t.Errorf("subtype = %q, want %q", p.Subtype, errs.SubtypeInvalidConfig)
+ }
})🤖 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_test.go` around lines 186 - 195, The malformed-config test
only checks for a non-nil error, so it can miss regressions in the typed error
contract. Update the `whoamiRun` error-path assertion in `whoami_test.go` to
inspect the returned error with `errs.ProblemOf`, and verify the expected
`category` and `subtype` for the `SubtypeInvalidConfig` case. Keep the existing
`whoamiRun` / `whoamiOptions` setup, but make the test fail if the typed
metadata is rewritten or lost.
Sources: Coding guidelines, Learnings
Comment on lines
+140
to
+149
| cmd := &cobra.Command{ | ||
| Use: "whoami", | ||
| Short: "Show the current effective identity (profile, app, bot/user, token status)", | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| if runF != nil { | ||
| return runF(opts) | ||
| } | ||
| return whoamiRun(opts) | ||
| }, | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- whoami.go ---'
sed -n '1,240p' cmd/whoami/whoami.go
echo
echo '--- whoami_test.go ---'
sed -n '1,260p' cmd/whoami/whoami_test.go
echo
echo '--- search for Args validation in cmd ---'
rg -n 'Args:\s*func|NoArgs|ExactArgs|MinimumNArgs|MaximumNArgs' cmd --glob '*.go'
echo
echo '--- search for whoami tests referencing args ---'
rg -n 'args|positional|unexpected|validation|invalid argument' cmd/whoami --glob '*_test.go'Repository: larksuite/cli
Length of output: 14324
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- whoami-related files ---'
git ls-files 'cmd/whoami/*'
echo
echo '--- search whoami command references ---'
rg -n 'whoami' cmd --glob '*.go'
echo
echo '--- search for cobra Args helpers in similar commands ---'
rg -n 'NoArgs|ExactArgs|MinimumNArgs|MaximumNArgs|Args:\s*func' cmd --glob '*.go'Repository: larksuite/cli
Length of output: 3440
Reject unexpected positional arguments.
whoami currently accepts and ignores extra args; add Args validation with errs.NewValidationError(errs.SubtypeInvalidArgument, ...) and cover the failure in cmd/whoami/whoami_test.go.
🤖 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 140 - 149, The whoami command currently
ignores unexpected positional arguments instead of rejecting them; update the
cobra.Command setup in whoamiRun/command construction to add Args validation
that returns errs.NewValidationError(errs.SubtypeInvalidArgument, ...) when
extra args are provided. Make sure the validation is wired through the whoami
command path so RunE still calls runF or whoamiRun only after args are checked,
and add a test in whoami_test.go that covers the invalid-argument failure case.
Source: Coding guidelines
Comment on lines
+143
to
+147
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| if runF != nil { | ||
| return runF(opts) | ||
| } | ||
| return whoamiRun(opts) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files cmd/whoami/whoami.go internal | sed -n '1,120p'
echo '--- whoami.go ---'
cat -n cmd/whoami/whoami.go | sed -n '1,240p'
echo '--- Diagnose references ---'
rg -n "func Diagnose|identitydiag\.Diagnose|Diagnose\(" -S .Repository: larksuite/cli
Length of output: 11753
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- internal/identitydiag/diagnostics.go ---'
cat -n internal/identitydiag/diagnostics.go | sed -n '1,260p'
echo '--- cmd/doctor/doctor.go ---'
cat -n cmd/doctor/doctor.go | sed -n '1,200p'
echo '--- cmd/auth/status.go ---'
cat -n cmd/auth/status.go | sed -n '1,140p'Repository: larksuite/cli
Length of output: 21001
Propagate cmd.Context() into whoamiRun. identitydiag.Diagnose already accepts a context, but whoamiRun always uses context.Background(). That drops cancellation/deadlines for --verify, so Ctrl-C or parent cancellation can still leave the probe running until the internal timeout expires.
🤖 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 143 - 147, Propagate the Cobra command
context through the whoami execution path instead of always creating a
background context. Update the RunE path and the whoamiRun function so it uses
cmd.Context() (and passes it into identitydiag.Diagnose) when running --verify,
preserving cancellation and deadlines from the parent command. Keep the existing
runF override behavior intact while ensuring the default whoamiRun flow receives
the command context.
maojinze7-prog
force-pushed
the
feat/whoami-command
branch
from
8a3276f to
ebb0b6f
Compare
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
1 participant
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.
No description provided.