feat(base): add identity priority strategy and error handling#505
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDefaults base operations to Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Dev / CLI
participant Auth as lark-cli (auth)
participant Skill as Skill Runtime
participant Shared as lark-shared
Dev->>Auth: lark-cli auth login --domain base
Dev->>Skill: request --as user
Skill-->>Dev: permission error (contains escalation hint?)
alt escalation hint present
Dev->>Skill: guide user to elevate scope (auth login --scope) and retry --as user
Skill-->>Dev: success / permission granted
else no escalation hint and error != 91403
Dev->>Skill: retry request --as bot (once)
alt bot success
Skill-->>Dev: success
else bot permission error
Dev->>Shared: route to lark-shared permission-handling flow
end
else error is 91403
Skill-->>Dev: permission error (stop, route to lark-shared)
Dev->>Shared: route to lark-shared permission-handling flow
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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
domain/base
|
|
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/lark-base/SKILL.md`:
- Around line 267-271: Add an explicit first-branch exception for error code
91403 in the permission-handling flow so it never triggers any retry logic:
update the execution rules in SKILL.md (the permission fallback algorithm around
the “--as user” / “--as bot” retry guidance) to check for code 91403 first and
immediately stop retries/redirect to the lark-shared guidance for resolution,
before the current user-vs-bot retry branching; reference the 91403 rule when
editing the block that currently describes the user fallback, bot one-time
retry, and final stop behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Establish user-first identity selection with graceful degradation to bot, and add no-retry rule for error code 91403 (permission denied on Base). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
wangqiucheng2bd
force-pushed
the
feat/base-identity-priority-and-error-handling
branch
from
9c8d210 to
ba0d7ec
Compare
Move non-retryable error code check (e.g. 91403) to a dedicated step before the user/bot fallback decision, resolving conflicting instructions between the error table and the execution rules. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
skills/lark-base/SKILL.md (1)
267-271:⚠️ Potential issue | 🟠 Major
91403no-retry rule is not enforced in the main fallback algorithm.Line 335 says
91403must not retry, but Lines 267-271 currently allow a bot fallback retry for permission errors without guidance. This creates conflicting operator instructions. Please add an explicit first-branch exception for91403in the execution rules (before any user/bot retry decision).📋 Suggested doc patch
1. 所有操作默认先用 `--as user`。 -2. 若 user 身份返回权限错误,检查错误响应中是否包含 `permission_violations` / `hint` 等提权引导信息： +2. 若 user 身份返回权限错误，先判断是否为 `91403`： + - **`91403`**：**不要重试**，直接按 `lark-shared` 权限不足处理流程引导用户解决。 + - 其他权限错误：再检查错误响应中是否包含 `permission_violations` / `hint` 等提权引导信息： - **有提权引导**：按 `lark-shared` 权限不足处理流程，先引导用户完成 user 身份提权（`auth login --scope`）；确认提权成功后，以 `--as user` 重试。 - **无提权引导**（如资源级无访问权限、非 scope 不足）：切换到 `--as bot` 重试**一次**。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/lark-base/SKILL.md` around lines 267 - 271, Add an explicit first-branch exception for error code 91403 before the user/bot retry logic: detect 91403 immediately and do NOT retry (neither as user nor as bot), instead stop further retries and invoke the lark-shared permission-resolution flow; update the execution rules near the "所有操作默认先用 `--as user`" / the user→bot fallback section so 91403 is checked first and documented as a no-retry rule.
🧹 Nitpick comments (1)
skills/lark-base/SKILL.md (1)
269-269: Consider clarifying where the scope value comes from.The instruction mentions
auth login --scopebut doesn't explicitly state that the specific scope should come from the error response'shintorpermission_violationsfields. While the phrase "按lark-shared权限不足处理流程" implies this, making it explicit would reduce ambiguity.✏️ Optional clarification
- - **有提权引导**：按 `lark-shared` 权限不足处理流程，先引导用户完成 user 身份提权（`auth login --scope`）；确认提权成功后，以 `--as user` 重试。 + - **有提权引导**：按 `lark-shared` 权限不足处理流程，根据错误响应中的 `hint` 或 `permission_violations` 字段引导用户完成 user 身份提权（`auth login --scope <具体scope>`）；确认提权成功后，以 `--as user` 重试。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/lark-base/SKILL.md` at line 269, Clarify that the scope value for the auth command should be taken from the error response (e.g., the "hint" or "permission_violations" fields): update the `lark-shared` insufficient-permission flow text to explicitly say "run `auth login --scope <scope_from_error>` where <scope_from_error> is the scope shown in the error's `hint` or `permission_violations`, then confirm elevation and retry with `--as user`"; reference the existing terms `auth login --scope`, `hint`, `permission_violations`, and `--as user` so readers know exactly where to obtain the scope.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@skills/lark-base/SKILL.md`:
- Around line 267-271: Add an explicit first-branch exception for error code
91403 before the user/bot retry logic: detect 91403 immediately and do NOT retry
(neither as user nor as bot), instead stop further retries and invoke the
lark-shared permission-resolution flow; update the execution rules near the
"所有操作默认先用 `--as user`" / the user→bot fallback section so 91403 is checked first
and documented as a no-retry rule.
---
Nitpick comments:
In `@skills/lark-base/SKILL.md`:
- Line 269: Clarify that the scope value for the auth command should be taken
from the error response (e.g., the "hint" or "permission_violations" fields):
update the `lark-shared` insufficient-permission flow text to explicitly say
"run `auth login --scope <scope_from_error>` where <scope_from_error> is the
scope shown in the error's `hint` or `permission_violations`, then confirm
elevation and retry with `--as user`"; reference the existing terms `auth login
--scope`, `hint`, `permission_violations`, and `--as user` so readers know
exactly where to obtain the scope.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
skills/lark-base/SKILL.md (1)
268-268: Consider clarifying whether91403is the only non-retryable error code.Line 268 uses "不可重试错误码（如
91403）" (non-retryable error codes such as91403), which suggests there may be multiple non-retryable codes. However, throughout the document only91403is mentioned (line 336). If91403is currently the only non-retryable error code, consider using more specific wording like "是否为91403" (whether it is91403). If other codes exist or are planned, they should be documented in the error table at line 336 for completeness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/lark-base/SKILL.md` at line 268, Update the wording in the sentence that currently reads "不可重试错误码（如 `91403`）" to be explicit: if `91403` is the only non-retryable code, change it to "是否为 `91403`" (or similar) to avoid implying others; if there are additional non-retryable codes, list them here (or refer to the error table) and add them to the error table where `91403` is documented so the set of non-retryable codes is consistent and complete across SKILL.md.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@skills/lark-base/SKILL.md`:
- Line 268: Update the wording in the sentence that currently reads "不可重试错误码（如
`91403`）" to be explicit: if `91403` is the only non-retryable code, change it
to "是否为 `91403`" (or similar) to avoid implying others; if there are additional
non-retryable codes, list them here (or refer to the error table) and add them
to the error table where `91403` is documented so the set of non-retryable codes
is consistent and complete across SKILL.md.
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@3d01e20d1bd7f1a69321f0503522be12526f7b41🧩 Skill updatenpx skills add wangqiucheng2bd/cli#feat/base-identity-priority-and-error-handling -y -g |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
skills/lark-base/SKILL.md (1)
267-274: Execution rules correctly implement 91403 first-check, but wording could be more precise.The logic flow properly enforces the no-retry rule for error code 91403 before any fallback logic, addressing the previous review concern. However, Line 268 uses "不可重试错误码（如
91403）" (non-retryable error codes such as 91403), which suggests there may be multiple codes in this category. If 91403 is currently the only non-retryable error code for Base operations, consider rewording to "不可重试错误码（91403）" for clarity. If additional codes exist, they should be explicitly documented here or referenced.📝 Suggested clarification
If 91403 is the only non-retryable code:
-2. 若 user 身份返回权限错误，先判断是否为**不可重试错误码**（如 `91403`）。若是，**立即停止**，不做任何重试或降级，直接按 `lark-shared` 权限不足处理流程引导用户解决。 +2. 若 user 身份返回权限错误，先判断是否为**不可重试错误码 `91403`**。若是，**立即停止**，不做任何重试或降级，直接按 `lark-shared` 权限不足处理流程引导用户解决。If there are other non-retryable codes, list them explicitly:
-2. 若 user 身份返回权限错误，先判断是否为**不可重试错误码**（如 `91403`）。若是，**立即停止**，不做任何重试或降级，直接按 `lark-shared` 权限不足处理流程引导用户解决。 +2. 若 user 身份返回权限错误，先判断是否为**不可重试错误码**（`91403`、`XXXXX` 等）。若是，**立即停止**，不做任何重试或降级，直接按 `lark-shared` 权限不足处理流程引导用户解决。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/lark-base/SKILL.md` around lines 267 - 274, The wording "不可重试错误码（如 `91403`）" is ambiguous; update the SKILL.md text to either explicitly state the single non-retryable code as "不可重试错误码（`91403`）" if 91403 is the only one, or enumerate all non-retryable codes in that clause (e.g., list additional codes) so the rule is unambiguous; locate and edit the phrase "不可重试错误码（如 `91403`）" in the execution rules block and replace it accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@skills/lark-base/SKILL.md`:
- Around line 267-274: The wording "不可重试错误码（如 `91403`）" is ambiguous; update the
SKILL.md text to either explicitly state the single non-retryable code as
"不可重试错误码（`91403`）" if 91403 is the only one, or enumerate all non-retryable
codes in that clause (e.g., list additional codes) so the rule is unambiguous;
locate and edit the phrase "不可重试错误码（如 `91403`）" in the execution rules block and
replace it accordingly.
wangqiucheng2bd
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #505 +/- ##
=======================================
Coverage ? 59.05%
=======================================
Files ? 384
Lines ? 32636
Branches ? 0
=======================================
Hits ? 19274
Misses ? 11553
Partials ? 1809 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4 participants
Summary
Identity priority strategy (section 3.5)
--as userfor all Base operationsauth login --scope, retry as user after success--as botonceError code 91403 (section 5)
lark-sharedpermission handling flowTest plan
Summary by CodeRabbit