fix(auth): make agent authorization resumable#1867
Closed
irinakk wants to merge 1 commit into
Closed
Conversation
github-actions
Bot
added
the
size/L
📝 WalkthroughWalkthroughThe CLI adds ChangesAuthentication resume flow
Managed profile initialization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant AuthLogin
participant PendingLoginCache
participant DeviceAuthorization
Agent->>AuthLogin: Run --no-wait --json
AuthLogin->>PendingLoginCache: Save pending authorization
Agent->>AuthLogin: Run --resume later
AuthLogin->>PendingLoginCache: Load latest valid authorization
AuthLogin->>DeviceAuthorization: Poll with saved device code
DeviceAuthorization-->>AuthLogin: Return authorization result
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Author
|
Closing for now; this public PR was opened prematurely before the intended review and coordination. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
cmd/auth/login_scope_cache_test.go (1)
98-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse standard-library APIs for test fixture setup.
Creating fixtures through
vfsmakes this test circular because it exercises the same abstraction used by production. Useos.MkdirAllandos.WriteFile; retainingvfs.Statfor the assertion is fine.Based on learnings, test fixtures under
t.TempDir()must use standard-library filesystem APIs rather thanvfs.Proposed fix
- if err := vfs.MkdirAll(loginScopeCacheDir(), 0700); err != nil { + if err := os.MkdirAll(loginScopeCacheDir(), 0700); err != nil { t.Fatalf("MkdirAll() error = %v", err) } - if err := vfs.WriteFile(pendingLoginPath(record.AppID), data, 0600); err != nil { + if err := os.WriteFile(pendingLoginPath(record.AppID), data, 0600); err != nil {🤖 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/auth/login_scope_cache_test.go` around lines 98 - 102, Update the fixture setup around loginScopeCacheDir and pendingLoginPath to use os.MkdirAll and os.WriteFile instead of vfs.MkdirAll and vfs.WriteFile, preserving the existing permissions and fatal error handling. Keep vfs.Stat for the assertion.Source: Learnings
🤖 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/login_scope_cache.go`:
- Around line 101-103: Update the expiration branch in the scope-cache lookup to
remove both the latest app record and the corresponding device-scoped record
before returning os.ErrNotExist. Extend the expiration test to verify that both
cache files are deleted.
- Around line 135-146: Update removePendingLogin to preserve vfs.ReadFile errors
when the pending-login file exists but cannot be read, while treating
os.ErrNotExist as benign. Propagate the read error through firstErr when no
earlier cleanup error has been recorded, and keep the existing record-matching
removal behavior unchanged.
In `@cmd/auth/login.go`:
- Around line 90-92: Add command-level flag-parsing coverage for the `--resume`
option in the login command tests, asserting that parsing `--resume` sets
`LoginOptions.Resume` to true. Keep the existing parsing coverage unchanged and
place the assertion alongside the other login flag tests.
- Around line 294-296: Update the savePendingLogin error path in the login
command so --no-wait returns a typed storage error that preserves the underlying
cause instead of logging a warning and continuing; keep existing behavior for
other modes. Add an error-path test verifying the returned error type and
wrapped cause metadata.
- Around line 139-149: Update the missing-pending-state branch in the
opts.Resume flow to return
errs.NewValidationError(errs.SubtypeFailedPrecondition, ...) instead of an
authentication/unknown error, preserving the existing recovery hint. Add
metadata assertions using errs.ProblemOf to verify the returned problem has the
expected validation category and failed-precondition subtype.
In `@cmd/config/init_guard_test.go`:
- Around line 23-44: Update TestGuardAgentWorkspace_ManagedProfileRefuses to
inspect the typed problem metadata with errs.ProblemOf, asserting CategoryConfig
and SubtypeNotConfigured (and preserving the existing cause assertion pattern if
applicable). Retain the current error-type and message/hint checks, but make the
test validate typed metadata rather than relying only on strings.
---
Nitpick comments:
In `@cmd/auth/login_scope_cache_test.go`:
- Around line 98-102: Update the fixture setup around loginScopeCacheDir and
pendingLoginPath to use os.MkdirAll and os.WriteFile instead of vfs.MkdirAll and
vfs.WriteFile, preserving the existing permissions and fatal error handling.
Keep vfs.Stat for the assertion.
🪄 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: d05da9b8-155d-4e8a-8b66-9a6842b1f1ab
📒 Files selected for processing (10)
cmd/auth/auth_test.gocmd/auth/login.gocmd/auth/login_messages.gocmd/auth/login_messages_test.gocmd/auth/login_scope_cache.gocmd/auth/login_scope_cache_test.gocmd/auth/login_test.gocmd/config/init.gocmd/config/init_guard_test.goskills/lark-shared/SKILL.md
Comment on lines
+101
to
+103
| if record.DeviceCode == "" || record.AppID != appID || record.ExpiresAt <= time.Now().Unix() { | ||
| _ = vfs.Remove(path) | ||
| return nil, os.ErrNotExist |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Remove the expired flow’s per-device scope record too.
Expiration deletes only latest-<app>.json, leaving the device-scoped cache indefinitely. Clean both records and extend the expiration test to assert both files disappear.
Proposed fix
- if record.DeviceCode == "" || record.AppID != appID || record.ExpiresAt <= time.Now().Unix() {
+ if record.DeviceCode == "" || record.AppID != appID {
_ = vfs.Remove(path)
return nil, os.ErrNotExist
}
+ if record.ExpiresAt <= time.Now().Unix() {
+ _ = removeLoginRequestedScope(record.DeviceCode)
+ _ = vfs.Remove(path)
+ return nil, os.ErrNotExist
+ }📝 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
| if record.DeviceCode == "" || record.AppID != appID || record.ExpiresAt <= time.Now().Unix() { | |
| _ = vfs.Remove(path) | |
| return nil, os.ErrNotExist | |
| if record.DeviceCode == "" || record.AppID != appID { | |
| _ = vfs.Remove(path) | |
| return nil, os.ErrNotExist | |
| } | |
| if record.ExpiresAt <= time.Now().Unix() { | |
| _ = removeLoginRequestedScope(record.DeviceCode) | |
| _ = vfs.Remove(path) | |
| return nil, os.ErrNotExist | |
| } |
🤖 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/auth/login_scope_cache.go` around lines 101 - 103, Update the expiration
branch in the scope-cache lookup to remove both the latest app record and the
corresponding device-scoped record before returning os.ErrNotExist. Extend the
expiration test to verify that both cache files are deleted.
Comment on lines
+135
to
+146
| func removePendingLogin(deviceCode, appID string) error { | ||
| firstErr := removeLoginRequestedScope(deviceCode) | ||
| path := pendingLoginPath(appID) | ||
| if data, err := vfs.ReadFile(path); err == nil { | ||
| var record pendingLoginRecord | ||
| if json.Unmarshal(data, &record) == nil && record.DeviceCode == deviceCode { | ||
| if err := vfs.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) && firstErr == nil { | ||
| firstErr = err | ||
| } | ||
| } | ||
| } | ||
| return firstErr |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Do not silently ignore pending-file read failures.
If vfs.ReadFile fails for reasons other than absence, cleanup returns success and leaves stale resumable state behind. Preserve that error so the command can warn the caller.
Proposed fix
path := pendingLoginPath(appID)
- if data, err := vfs.ReadFile(path); err == nil {
+ data, err := vfs.ReadFile(path)
+ if err != nil {
+ if !errors.Is(err, os.ErrNotExist) && firstErr == nil {
+ firstErr = err
+ }
+ return firstErr
+ }
+ {
var record pendingLoginRecord
if json.Unmarshal(data, &record) == nil && record.DeviceCode == deviceCode {📝 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
| func removePendingLogin(deviceCode, appID string) error { | |
| firstErr := removeLoginRequestedScope(deviceCode) | |
| path := pendingLoginPath(appID) | |
| if data, err := vfs.ReadFile(path); err == nil { | |
| var record pendingLoginRecord | |
| if json.Unmarshal(data, &record) == nil && record.DeviceCode == deviceCode { | |
| if err := vfs.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) && firstErr == nil { | |
| firstErr = err | |
| } | |
| } | |
| } | |
| return firstErr | |
| func removePendingLogin(deviceCode, appID string) error { | |
| firstErr := removeLoginRequestedScope(deviceCode) | |
| path := pendingLoginPath(appID) | |
| data, err := vfs.ReadFile(path) | |
| if err != nil { | |
| if !errors.Is(err, os.ErrNotExist) && firstErr == nil { | |
| firstErr = err | |
| } | |
| return firstErr | |
| } | |
| { | |
| var record pendingLoginRecord | |
| if json.Unmarshal(data, &record) == nil && record.DeviceCode == deviceCode { | |
| if err := vfs.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) && firstErr == nil { | |
| firstErr = err | |
| } | |
| } | |
| } | |
| return firstErr |
🤖 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/auth/login_scope_cache.go` around lines 135 - 146, Update
removePendingLogin to preserve vfs.ReadFile errors when the pending-login file
exists but cannot be read, while treating os.ErrNotExist as benign. Propagate
the read error through firstErr when no earlier cleanup error has been recorded,
and keep the existing record-matching removal behavior unchanged.
Comment on lines
+90
to
+92
| cmd.Flags().BoolVar(&opts.NoWait, "no-wait", false, "initiate device authorization and return immediately; use --resume to complete later") | ||
| cmd.Flags().StringVar(&opts.DeviceCode, "device-code", "", "poll and complete authorization with a device code from a previous --no-wait call") | ||
| cmd.Flags().BoolVar(&opts.Resume, "resume", false, "resume the latest pending authorization created by --no-wait") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add direct parsing coverage for --resume.
The new Cobra flag is not exercised by the supplied flag-parsing test. Add a command-level assertion that --resume sets LoginOptions.Resume.
As per coding guidelines, every behavior change needs a test alongside the change.
🤖 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/auth/login.go` around lines 90 - 92, Add command-level flag-parsing
coverage for the `--resume` option in the login command tests, asserting that
parsing `--resume` sets `LoginOptions.Resume` to true. Keep the existing parsing
coverage unchanged and place the assertion alongside the other login flag tests.
Source: Coding guidelines
Comment on lines
+139
to
+149
| if opts.Resume { | ||
| if opts.DeviceCode != "" || opts.NoWait || opts.Scope != "" || opts.Recommend || len(opts.Domains) > 0 || len(opts.Exclude) > 0 { | ||
| return errs.NewValidationError(errs.SubtypeInvalidArgument, "--resume cannot be combined with authorization request options or --device-code").WithParam("--resume") | ||
| } | ||
| pending, err := loadPendingLogin(config.AppID) | ||
| if err != nil { | ||
| if errors.Is(err, os.ErrNotExist) { | ||
| return errs.NewAuthenticationError(errs.SubtypeUnknown, "no unexpired pending authorization to resume"). | ||
| WithHint("start a fresh split flow with `lark-cli auth login --scope <scope> --no-wait --json`, send its verification URL to the user, then run `lark-cli auth login --resume` only after the user confirms authorization") | ||
| } | ||
| return errs.NewInternalError(errs.SubtypeStorage, "failed to load pending authorization: %v", err).WithCause(err) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Classify missing pending state as a failed precondition.
--resume is valid input, but no resumable state exists. Returning AuthenticationError/Unknown breaks the typed command contract; use ValidationError/FailedPrecondition and retain the recovery hint. Add metadata assertions using errs.ProblemOf.
As per coding guidelines, valid requests made in the wrong system state must use errs.NewValidationError(errs.SubtypeFailedPrecondition, ...).WithHint(...).
Proposed fix
- return errs.NewAuthenticationError(errs.SubtypeUnknown, "no unexpired pending authorization to resume").
+ return errs.NewValidationError(errs.SubtypeFailedPrecondition, "no unexpired pending authorization to resume").
WithHint("start a fresh split flow with `lark-cli auth login --scope <scope> --no-wait --json`, send its verification URL to the user, then run `lark-cli auth login --resume` only after the user confirms authorization")📝 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
| if opts.Resume { | |
| if opts.DeviceCode != "" || opts.NoWait || opts.Scope != "" || opts.Recommend || len(opts.Domains) > 0 || len(opts.Exclude) > 0 { | |
| return errs.NewValidationError(errs.SubtypeInvalidArgument, "--resume cannot be combined with authorization request options or --device-code").WithParam("--resume") | |
| } | |
| pending, err := loadPendingLogin(config.AppID) | |
| if err != nil { | |
| if errors.Is(err, os.ErrNotExist) { | |
| return errs.NewAuthenticationError(errs.SubtypeUnknown, "no unexpired pending authorization to resume"). | |
| WithHint("start a fresh split flow with `lark-cli auth login --scope <scope> --no-wait --json`, send its verification URL to the user, then run `lark-cli auth login --resume` only after the user confirms authorization") | |
| } | |
| return errs.NewInternalError(errs.SubtypeStorage, "failed to load pending authorization: %v", err).WithCause(err) | |
| if opts.Resume { | |
| if opts.DeviceCode != "" || opts.NoWait || opts.Scope != "" || opts.Recommend || len(opts.Domains) > 0 || len(opts.Exclude) > 0 { | |
| return errs.NewValidationError(errs.SubtypeInvalidArgument, "--resume cannot be combined with authorization request options or --device-code").WithParam("--resume") | |
| } | |
| pending, err := loadPendingLogin(config.AppID) | |
| if err != nil { | |
| if errors.Is(err, os.ErrNotExist) { | |
| return errs.NewValidationError(errs.SubtypeFailedPrecondition, "no unexpired pending authorization to resume"). | |
| WithHint("start a fresh split flow with `lark-cli auth login --scope <scope> --no-wait --json`, send its verification URL to the user, then run `lark-cli auth login --resume` only after the user confirms authorization") | |
| } | |
| return errs.NewInternalError(errs.SubtypeStorage, "failed to load pending authorization: %v", err).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/auth/login.go` around lines 139 - 149, Update the missing-pending-state
branch in the opts.Resume flow to return
errs.NewValidationError(errs.SubtypeFailedPrecondition, ...) instead of an
authentication/unknown error, preserving the existing recovery hint. Add
metadata assertions using errs.ProblemOf to verify the returned problem has the
expected validation category and failed-precondition subtype.
Source: Coding guidelines
Comment on lines
+294
to
296
| if err := savePendingLogin(authResp.DeviceCode, config.AppID, finalScope, authResp.ExpiresIn); err != nil { | ||
| fmt.Fprintf(f.IOStreams.ErrOut, "[lark-cli] [WARN] auth login: failed to cache requested scopes: %v\n", err) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Fail --no-wait when resumable state cannot be persisted.
The command currently returns success and instructs the agent to use --resume, even though that follow-up is guaranteed to fail. Return a typed storage error with the underlying cause, and add an error-path metadata/cause test.
Proposed fix
if opts.NoWait {
if err := savePendingLogin(authResp.DeviceCode, config.AppID, finalScope, authResp.ExpiresIn); err != nil {
- fmt.Fprintf(f.IOStreams.ErrOut, "[lark-cli] [WARN] auth login: failed to cache requested scopes: %v\n", err)
+ return errs.NewInternalError(errs.SubtypeStorage,
+ "failed to save pending authorization: %v", err).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
| if err := savePendingLogin(authResp.DeviceCode, config.AppID, finalScope, authResp.ExpiresIn); err != nil { | |
| fmt.Fprintf(f.IOStreams.ErrOut, "[lark-cli] [WARN] auth login: failed to cache requested scopes: %v\n", err) | |
| } | |
| if err := savePendingLogin(authResp.DeviceCode, config.AppID, finalScope, authResp.ExpiresIn); err != nil { | |
| return errs.NewInternalError(errs.SubtypeStorage, | |
| "failed to save pending authorization: %v", err).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/auth/login.go` around lines 294 - 296, Update the savePendingLogin error
path in the login command so --no-wait returns a typed storage error that
preserves the underlying cause instead of logging a warning and continuing; keep
existing behavior for other modes. Add an error-path test verifying the returned
error type and wrapped cause metadata.
Source: Coding guidelines
Comment on lines
+23
to
+44
| func TestGuardAgentWorkspace_ManagedProfileRefuses(t *testing.T) { | ||
| clearAgentEnv(t) | ||
| t.Setenv("LARKSUITE_CLI_MANAGED_CONFIG", "1") | ||
|
|
||
| err := guardAgentWorkspace(&ConfigInitOptions{}) | ||
| if err == nil { | ||
| t.Fatal("expected refusal for a host-managed profile, got nil") | ||
| } | ||
| var cfgErr *errs.ConfigError | ||
| if !errors.As(err, &cfgErr) { | ||
| t.Fatalf("error type = %T, want *errs.ConfigError", err) | ||
| } | ||
| if !strings.Contains(cfgErr.Message, "managed by the Agent host") { | ||
| t.Errorf("message must identify host-managed config; got %q", cfgErr.Message) | ||
| } | ||
| if strings.Contains(cfgErr.Hint, "config bind") { | ||
| t.Errorf("managed profiles must not be redirected to config bind; got %q", cfgErr.Hint) | ||
| } | ||
| if !strings.Contains(cfgErr.Hint, "failed to provision") { | ||
| t.Errorf("hint must identify the host provisioning failure; got %q", cfgErr.Hint) | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert typed error subtype, not just message substrings.
Per the coding guidelines, error-path tests must assert typed metadata via errs.ProblemOf (category / subtype / param) and cause preservation, not message substrings alone. TestGuardAgentWorkspace_ManagedProfileRefuses checks the error type via errors.As and verifies message/hint substrings, but does not assert Subtype == errs.SubtypeNotConfigured (or Category == CategoryConfig). Add a subtype assertion to satisfy the guideline.
🛡️ Proposed fix: add subtype assertion
if !errors.As(err, &cfgErr) {
t.Fatalf("error type = %T, want *errs.ConfigError", err)
}
+ if cfgErr.Subtype != errs.SubtypeNotConfigured {
+ t.Errorf("subtype = %q, want %q", cfgErr.Subtype, errs.SubtypeNotConfigured)
+ }
if !strings.Contains(cfgErr.Message, "managed by the Agent host") {📝 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
| func TestGuardAgentWorkspace_ManagedProfileRefuses(t *testing.T) { | |
| clearAgentEnv(t) | |
| t.Setenv("LARKSUITE_CLI_MANAGED_CONFIG", "1") | |
| err := guardAgentWorkspace(&ConfigInitOptions{}) | |
| if err == nil { | |
| t.Fatal("expected refusal for a host-managed profile, got nil") | |
| } | |
| var cfgErr *errs.ConfigError | |
| if !errors.As(err, &cfgErr) { | |
| t.Fatalf("error type = %T, want *errs.ConfigError", err) | |
| } | |
| if !strings.Contains(cfgErr.Message, "managed by the Agent host") { | |
| t.Errorf("message must identify host-managed config; got %q", cfgErr.Message) | |
| } | |
| if strings.Contains(cfgErr.Hint, "config bind") { | |
| t.Errorf("managed profiles must not be redirected to config bind; got %q", cfgErr.Hint) | |
| } | |
| if !strings.Contains(cfgErr.Hint, "failed to provision") { | |
| t.Errorf("hint must identify the host provisioning failure; got %q", cfgErr.Hint) | |
| } | |
| } | |
| func TestGuardAgentWorkspace_ManagedProfileRefuses(t *testing.T) { | |
| clearAgentEnv(t) | |
| t.Setenv("LARKSUITE_CLI_MANAGED_CONFIG", "1") | |
| err := guardAgentWorkspace(&ConfigInitOptions{}) | |
| if err == nil { | |
| t.Fatal("expected refusal for a host-managed profile, got nil") | |
| } | |
| var cfgErr *errs.ConfigError | |
| if !errors.As(err, &cfgErr) { | |
| t.Fatalf("error type = %T, want *errs.ConfigError", err) | |
| } | |
| if cfgErr.Subtype != errs.SubtypeNotConfigured { | |
| t.Errorf("subtype = %q, want %q", cfgErr.Subtype, errs.SubtypeNotConfigured) | |
| } | |
| if !strings.Contains(cfgErr.Message, "managed by the Agent host") { | |
| t.Errorf("message must identify host-managed config; got %q", cfgErr.Message) | |
| } | |
| if strings.Contains(cfgErr.Hint, "config bind") { | |
| t.Errorf("managed profiles must not be redirected to config bind; got %q", cfgErr.Hint) | |
| } | |
| if !strings.Contains(cfgErr.Hint, "failed to provision") { | |
| t.Errorf("hint must identify the host provisioning failure; got %q", cfgErr.Hint) | |
| } | |
| } |
🤖 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/config/init_guard_test.go` around lines 23 - 44, Update
TestGuardAgentWorkspace_ManagedProfileRefuses to inspect the typed problem
metadata with errs.ProblemOf, asserting CategoryConfig and SubtypeNotConfigured
(and preserving the existing cause assertion pattern if applicable). Retain the
current error-type and message/hint checks, but make the test validate typed
metadata rather than relying only on strings.
Source: Coding guidelines
|
yilin.wang seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
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
auth login --no-waitreturnsauth login --resumeso a later agent turn can continue an unexpired flow without carrying the device code in conversation contextconfig initin managed environments unless--force-initis explicitly providedWhy
Agent-hosted sessions can end while a device authorization command is still pending. Requiring the next turn to recover a device code from conversation context makes the flow fragile and can lead to repeated authorization links. Managed hosts also need a clear guard against creating a second app configuration when provisioning is temporarily unavailable.
Test plan
GOTOOLCHAIN=go1.24.3 make unit-testGOTOOLCHAIN=go1.24.3 go vet ./...gofmt -l .GOTOOLCHAIN=go1.24.3 go mod tidywith nogo.mod/go.sumdiffgolangci-lint v2.1.6 run --new-from-rev=origin/mainSummary by CodeRabbit
New Features
--no-wait --json, followed later by--resume.--force-initavailable as an override.Bug Fixes