fix(apps): make cache-clear ask first, and make apps failures classifiable - #2415
Merged
Merged
Conversation
github-actions
Bot
added
the
size/M
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe change adds Spark error metadata, filters generic app hints for failed-precondition errors, and strengthens cache-clearing confirmation guidance. ChangesError classification and app hints
Cache operation safeguards
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This PR clarifies cache-clear confirmation behavior and classifies app failures for more reliable caller decisions; no actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@9fb2f5d8093ee3381b93977607b79db9663d6779🧩 Skill updatenpx skills add larksuite/cli#fix/apps-error-contract-cache-confirm -y -g |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2415 +/- ##
==========================================
- Coverage 76.41% 76.41% -0.01%
==========================================
Files 1047 1047
Lines 115504 115506 +2
==========================================
Hits 88260 88260
- Misses 20443 20444 +1
- Partials 6801 6802 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Asked to clear an app's online cache, an agent read `Risk: high-risk-write` from
--help and then supplied `--yes` itself on the first call, wiping production
cache without ever hitting the confirmation gate.
The CLI gate is fine: no --yes -> exit 10 confirmation_required, and --dry-run ->
exit 0 without triggering it. The wording was not. It only forbade appending
`--yes` *after* an exit-10, and said "已明确授权可直接带 --yes" without defining
authorization — so "clear my cache" read as authorization.
- `+cache-clear` gets a CAUTION block: never self-supply `--yes` on the first
call; without confirmation, either --dry-run or ask, then stop and wait. exit
10 is not a signal to retry with --yes.
- Add a zero-ambiguity table separating a *request* to clear ("clear the online
cache") from a *confirmation* ("我确认清 dev"), so blocking the accidental wipe
does not also kill the cases that were already correct: an explicit
confirmation still goes straight to `--yes`, and a request with no environment
named still has to ask instead of picking one.
- Note that online needs a confirmation phrase even when named explicitly.
`+cache-delete` gains the response field an agent has to read
(`deleted_key_count`): 0 means the key never existed, not "deleted
successfully", plus the get -> delete -> get chain needed to prove a delete took
effect — a single miss afterwards cannot tell the two apart.
SKILL.md: add +cache-clear to 禁止预授权判定底线, the one list a pre-authorized
run cannot skip; a reference-level rule alone would be bypassed there. The
routing table is left alone — no other row annotates risk, including
+file-delete, +role-delete and +member-remove.
chenxingyang1019
force-pushed
the
fix/apps-error-contract-cache-confirm
branch
from
August 20, 2026 06:47
a968233 to
00106cf
Compare
|
|
chenxingyang1019
force-pushed
the
fix/apps-error-contract-cache-confirm
branch
from
August 20, 2026 07:22
00106cf to
72e6384
Compare
`+db-execute` against a tenant that never activated Miaoda returns code 221800
"miaoda UAT not activated" with the hint "verify table/column names with
`+db-table-get` ... target the dev database with --environment dev". Neither step
can help: the failure is tenant-level, so a caller following the hint loops over
table lookups and env retries that fail identically.
Two causes. 221800 was unregistered, so it degraded to api/unknown — nothing in
the envelope distinguished "your tenant is not activated, stop" from "your SQL
was wrong, fix it and retry". And withAppsHint filled the caller's hint whenever
the server sent none, without looking at what failed: the hints are
command-scoped ("verify --app-id", "verify table/column names", "list releases"),
so every one of them describes the request, and the request is exactly what
failed_precondition says was fine.
Register 221800 as validation/failed_precondition (same shape as 400002465 "app
has no database yet") and gate the hint fallback on the subtype.
Blast radius is two codes, since that is all the subtype covers here:
- 221800 — now withheld; message and code still carry the meaning.
- 400002655 "no running container" — only when it reaches a non-observability
command; the observability pair rewrites it first, and "verify --app-id" was
never the fix for an undeployed app.
400002465 / 500002759 are intercepted by the isAppNoDatabaseError branch above
the gate, and 400002479 is served by withDBSyncHint, which does not delegate
here. The other 78 call sites take the original path for every input.
Gate on the one subtype, not on Category: this package asserts on purpose that an
authentication failure on +role-list (99991663) keeps the app-access hint and a
503 on credential issuance keeps the developer-access hint. Those hints are broad
enough to survive a caller-standing failure; only the precondition class is
misdescribed by construction. A test pins that, so widening the gate to Category
fails loudly instead of silently dropping those hints.
The gate is asserted on the real classification path (BuildAPIError -> the code
table -> withAppsHint), not only on a hand-built Problem. Constructing
SubtypeFailedPrecondition directly feeds the gate the input it wants and passes
whether or not 221800 is registered, so the registration itself has to be part of
what the test covers.
No recovery hint for 221800 — the activation path is a product procedure, and
guessing one is what made this failure misleading in the first place.
Five Spark business codes reached the CLI unregistered, so every one of them came
out as api/unknown with exit 1: a caller could not tell "your app id is wrong"
from "you lack permission" from "the upstream is having a bad minute", and the
exit code offered no way to branch either.
400002484 app not found -> validation/invalid_argument exit 2
400002467 no admin/developer perm -> authorization/permission_denied exit 3
500002761 ditto, pre-4xx renumber -> same
400000034 file not found/no access -> api/not_found exit 1
500000034 ditto, pre-4xx renumber -> same
400002467 is not file-specific: db commands (+db-table-list, +db-table-get,
+db-quota-get, +db-changelog-list) return it for an app the caller cannot access,
so registering it fixes both domains at once.
400002484 covers a well-formed id that does not exist AND a malformed one
("notanappid", "app_1" return it too), so the argument itself is the failure ->
invalid_argument, whose exit 2 separates "you passed the wrong id" from an
upstream fault. Environments that have not picked it up answer with 400002465
instead, conflating it with "app has no database yet"; the CLI cannot tell those
apart on the old code, so nothing here keys on that.
Both the current and the pre-4xx number are registered for each file failure.
The domain is moving its client-class errors from the 5xxxxxxxx band into
4xxxxxxxx, rolled out per environment, so both are live at once and dropping the
old one would silently return the un-migrated half to api/unknown — the same trap
that made the no-database recovery flow disappear when the server renumbered it
(see appNoDatabaseCode). The new number is not derivable from the old either:
500002761 became 400002467, tail digits included.
No hints added: permission_denied already has framework recovery wording, and a
domain-specific one would have to invent a remedy.
chenxingyang1019
force-pushed
the
fix/apps-error-contract-cache-confirm
branch
from
August 20, 2026 08:04
72e6384 to
9fb2f5d
Compare
liangshuo-1
approved these changes
Aug 20, 2026
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
3 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.
本 PR 做两件事，都指向同一个目标：让调用方（尤其是 agent）拿到的失败信封能据以决策。取代 #2361（同内容 + 新增 file/app 级错误码分类）。
一、
+cache-clear必须先确认再清（skill 文档）现象：用户说「帮我清一下应用 app_xxx 的 online 环境缓存」，agent 从
--help读到Risk: high-risk-write后，自己在首次调用就补上--yes，真的清掉了 online 缓存，全程没碰到确认关卡。根因不在 CLI 门禁——门禁本身是好的：不带
--yes→ exit 10confirmation_required；--dry-run→ exit 0 且不触发门禁。问题在文档措辞只约束了「撞到 exit 10 之后不要静默补--yes」，没禁止首次调用就预先带上；而「已明确授权可直接带--yes」又没定义什么叫「授权」，于是「帮我清一下缓存」被读成了授权。改动：
+cache-clear增加> [!CAUTION]：默认「先确认、后执行」，未确认时只能--dry-run预览或口头请求确认后停下等回话；并写明 exit 10 不是「补--yes重试」的信号。增加零歧义判据表，把「请求清理」和「确认清理」分开。这样在拦住误清的同时，不会把本来正确的行为一起判死：
--dry-run或停下请求确认--environment dev --yes+cache-delete小节点名响应字段deleted_key_count：0表示 key 原本就不存在，不得表述成「已成功删除」；并给出「删前 get → delete 得 1 → 删后 get miss」的证明链（只靠删后一次 miss 无法区分「删掉了」和「本来就没有」）。SKILL.md的「禁止预授权判定底线」新增+cache-clear——该清单即便用户已预授权也不豁免，这是二级文档管不到的场景。路由表未加风险标注：同表其它高危命令（+file-delete、+role-delete、+member-remove）都没有，保持一致。二、让 apps 域的失败可分类
2.1 hint 不再套到 precondition 类失败上
+db-execute打到一个从未开通妙搭的租户，返回221800 miaoda UAT not activated，hint 却是「verify table/column names… target the dev database with--environment dev」——两条都没用，agent 照做只会在查表和换环境之间空转，且每次都以同样的错误告终。withAppsHint的 hint 是命令级的（"verify --app-id"、"verify table/column names"、"list releases"），描述的都是请求；而failed_precondition的定义恰恰是「请求合法、但系统状态不满足，调用方要去改状态而不是重试」。给它套请求形状的建议，是构造性错误。所以：登记
221800为validation/failed_precondition，并把 hint 兜底按 subtype 门控。影响面是 2 个码，因为该 subtype 在本域只覆盖这些：
221800—— 现在不填 hint，message 和 code 仍承载语义；400002655「无运行容器」—— 仅当它流到非可观测命令时；可观测命令由withObservabilityHint先改写，而「verify --app-id」对一个没部署的 app 本来就不是解法。其余 precondition 码到不了门控：
400002465/500002759被上方isAppNoDatabaseError分支拦截，400002479由withDBSyncHint处理且不委派到这里。另外 78 个调用点在任何输入下都走原路径。只按这一个 subtype 门控、不按 Category：本包刻意断言了
+role-list的认证失败（99991663）要保留 app-access hint、credential 签发的 503 要保留 developer-access hint。那些 hint 措辞足够宽，套在「调用方身份」类失败上仍成立；只有 precondition 这一类是构造性地被描述错。2.2 登记 5 个 file / app 级业务码
这些码此前全部落
api/unknown+ exit 1——调用方分不出「app id 写错」「没权限」「上游抖了」，退出码也无法分流：400002484app not foundvalidation/invalid_argument400002467/500002761无 admin/developer 权限authorization/permission_denied400000034/500000034文件不存在api/not_found三点说明：
400002467不是 file 专属：db 命令（+db-table-list/-table-get/-quota-get/-changelog-list）打无权限 app 也返回它，所以登记一次同时修好两个域。400002484选invalid_argument而非not_found：它对格式非法的 id（notanappid、app_1）同样返回，说明失败在入参本身，而不是「资源存在与否」。归为 validation 后退出码是 2，把「你传的 id 不对」与「上游故障」分开。api/unknown——这正是服务端换号那次把 no-database 恢复流程静默搞没的坑（见appNoDatabaseCode的注释）。号也不能按前缀推算：500002761迁移后是400002467，尾号一起变了。未加任何域内 hint：
permission_denied已有框架层的恢复文案，再写一条域内专属的就得猜 remedy，而「猜出来的 hint」正是 2.1 要修的那类问题。验证
internal/errclass、shortcuts/apps、shortcuts/apps/gitcred、internal/output、errs、cmd全绿。withAppsHint内部两个分支的先后顺序——400002465「app 无数据库」本身就是failed_precondition，它的 message 改写与专属 hint 必须继续优先于新加的门控，否则这条恢复流程会被门控吞掉。api/unknown、api/server_error、authentication/token_invalid等类别仍然照常填充命令级 hint，防止后来者把门控从 subtype 扩大到 Category、破掉本包已有的两条刻意契约。Summary by CodeRabbit
Bug Fixes
Safety
Documentation