fix(auth): make agent authorization resumable #1867
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,7 +61,7 @@ var loginMsgZh = &loginMsg{ | |
|
|
||
| OpenURL: "在浏览器中打开以下链接进行认证:\n\n", | ||
| WaitingAuth: "等待用户授权...", | ||
| AgentTimeoutHint: "[AI agent] 此命令最长阻塞约 10 分钟，等待用户在浏览器内完成授权。请确保 runner 的 timeout >= 600s。若你的 harness 或 agent 工具只会把最终回复发给用户，请改用 \"lark-cli auth login --no-wait --json\" 拿到 verification_url，把 verification_url 作为本轮最终消息发给用户并结束本轮；等用户回复已完成授权后，再在后续步骤运行 \"lark-cli auth login --resume\" 续上轮询。CLI 会在本机保存 pending device code，不要把它写入回复或任务评论。**不要在同一轮里展示 URL 后立刻阻塞执行 --resume**，也不要短 timeout 反复重试；如果 pending 授权过期，再发起一条新的链接。**必须生成二维码并展示**: 你必须调用 lark-cli auth qrcode 将 verification_url 转为二维码，这是必须步骤，不要跳过。优先生成 PNG 二维码（--output）；仅当用户明确要求时才使用 ASCII（--ascii）。**关键：生成后必须在回复中展示图片**，仅生成文件不算完成，须用 image 标签或内联图片等方式将图片包含在回复中。**展示顺序**: 先输出 URL，再将二维码图片置于 URL 下方完整展示。**URL 输出规则**: 将 verification_url 视为不可修改的 opaque string，不要做任何修改（包括 URL 编码/解码、添加空格或标点）。", | ||
| AuthSuccess: "已收到授权确认，正在获取用户信息并校验授权结果...", | ||
| LoginSuccess: "授权成功! 用户: %s (%s)", | ||
| AuthorizedUser: "当前授权账号: %s (%s)", | ||
|
|
@@ -97,7 +97,7 @@ var loginMsgEn = &loginMsg{ | |
|
|
||
| OpenURL: "Open this URL in your browser to authenticate:\n\n", | ||
| WaitingAuth: "Waiting for user authorization...", | ||
| AgentTimeoutHint: "[AI agent] This command blocks for up to ~10 minutes while waiting for the user to authorize in their browser. Make sure your runner's timeout is >= 600s. If your harness or agent tool only delivers final turn messages, use \"lark-cli auth login --no-wait --json\" to get verification_url, present it to the user exactly as the final message of this turn, then end the turn; after the user replies that they authorized, run \"lark-cli auth login --resume\" in a later step to resume polling. The CLI stores the pending device code locally; do not put it in a response or task comment. **Do NOT show the URL and then immediately block on --resume in the same turn**, and do not retry with a short timeout; start a fresh flow only if the pending authorization expires.**MUST generate QR code AND display it:** You MUST call lark-cli auth qrcode to convert verification_url into a QR code. This is a required step, do NOT skip it. Prefer PNG QR code (--output); use ASCII (--ascii) only when the user explicitly requests it.**CRITICAL: You MUST include the QR image in your response.** Generating the file alone is NOT enough—use image tags, inline images, or file attachments to display it.**Display order:** Output the URL first, then place the QR code image below the URL.**URL Output Rules:** Treat verification_url as an opaque string that cannot be modified. Do NOT URL-encode/decode or add spaces/punctuation.", | ||
| AuthSuccess: "Authorization confirmed, fetching user info and validating granted scopes...", | ||
| LoginSuccess: "Authorization successful! User: %s (%s)", | ||
| AuthorizedUser: "Authorized account: %s (%s)", | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -101,12 +101,12 @@ func TestLoginMsg_FormatStrings(t *testing.T) { | |
| // TestAgentTimeoutHint_CarriesKeyInfo guards the contract that the synchronous | ||
| // auth-login output tells AI agents three things: (a) this command blocks for | ||
| // minutes — set a long runner timeout, (b) the alternative is the --no-wait + | ||
| // --resume split-flow, and (c) non-streaming harnesses must end the turn | ||
| // after presenting the URL instead of blocking in the same turn. | ||
| func TestAgentTimeoutHint_CarriesKeyInfo(t *testing.T) { | ||
| for _, lang := range []i18n.Lang{i18n.LangZhCN, i18n.LangEnUS} { | ||
| hint := getLoginMsg(lang).AgentTimeoutHint | ||
| for _, want := range []string{"--no-wait", "--resume", "turn"} { | ||
| if lang == i18n.LangZhCN && want == "turn" { | ||
| want = "本轮" | ||
| } | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,6 +9,7 @@ import ( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "os" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "path/filepath" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "regexp" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "time" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| larkauth "github.com/larksuite/cli/internal/auth" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/larksuite/cli/internal/core" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -22,6 +23,13 @@ type loginScopeCacheRecord struct { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RequestedScope string `json:"requested_scope"` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type pendingLoginRecord struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DeviceCode string `json:"device_code"` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| AppID string `json:"app_id"` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RequestedScope string `json:"requested_scope"` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ExpiresAt int64 `json:"expires_at"` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // loginScopeCacheDir returns the directory used to persist auth login --no-wait | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // requested scopes keyed by device_code. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func loginScopeCacheDir() string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -33,6 +41,10 @@ func loginScopeCachePath(deviceCode string) string { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return filepath.Join(loginScopeCacheDir(), sanitizeLoginScopeCacheKey(deviceCode)+".json") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func pendingLoginPath(appID string) string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return filepath.Join(loginScopeCacheDir(), "latest-"+sanitizeLoginScopeCacheKey(appID)+".json") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // sanitizeLoginScopeCacheKey converts a device_code into a safe filename token. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func sanitizeLoginScopeCacheKey(deviceCode string) string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sanitized := loginScopeCacheSafeChars.ReplaceAllString(deviceCode, "_") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -54,6 +66,45 @@ func saveLoginRequestedScope(deviceCode, requestedScope string) error { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return validate.AtomicWrite(loginScopeCachePath(deviceCode), data, 0600) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // savePendingLogin persists the latest split-flow authorization so a later | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // agent turn can resume it without copying a device code through the model's | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // conversation context. The per-device scope record remains for backwards | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // compatibility with the explicit --device-code flow. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func savePendingLogin(deviceCode, appID, requestedScope string, expiresIn int) error { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err := saveLoginRequestedScope(deviceCode, requestedScope); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| record := pendingLoginRecord{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DeviceCode: deviceCode, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| AppID: appID, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RequestedScope: requestedScope, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ExpiresAt: time.Now().Add(time.Duration(expiresIn) * time.Second).Unix(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| data, err := json.Marshal(record) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return validate.AtomicWrite(pendingLoginPath(appID), data, 0600) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func loadPendingLogin(appID string) (*pendingLoginRecord, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| path := pendingLoginPath(appID) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| data, err := vfs.ReadFile(path) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var record pendingLoginRecord | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err := json.Unmarshal(data, &record); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _ = vfs.Remove(path) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if record.DeviceCode == "" || record.AppID != appID || record.ExpiresAt <= time.Now().Unix() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _ = vfs.Remove(path) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, os.ErrNotExist | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+101
to
+103
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 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return &record, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // loadLoginRequestedScope loads the cached requested scope string for a device_code. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // It returns an empty string if no cache entry exists. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func loadLoginRequestedScope(deviceCode string) (string, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -81,6 +132,20 @@ func removeLoginRequestedScope(deviceCode string) error { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+135
to
+146
There was a problem hiding this comment. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win Do not silently ignore pending-file read failures. If 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // shouldRemoveLoginRequestedScope indicates whether the requested-scope cache | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // should be removed after polling finishes. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func shouldRemoveLoginRequestedScope(result *larkauth.DeviceFlowResult) bool { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 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
--resumesetsLoginOptions.Resume.As per coding guidelines, every behavior change needs a test alongside the change.
🤖 Prompt for AI Agents
Source: Coding guidelines