fix: make the --yes self-approval ban reachable on every surface by luozhixiong01 · Pull Request #2151 · larksuite/cli

luozhixiong01 · 2026-08-03T08:07:21Z

Summary

lark-cli tells agents that --yes asserts the user confirmed, so an agent must never add it on its own. That sentence lives in writeRisk, and writeRisk was reachable from only one of the two paths that rendered a Risk: line into shortcut help. Every shortcut on that path happens to be read-level, so the ban had never rendered once anywhere in the CLI — 0 of 53 high-risk-write shortcuts carried it, including drive +delete, base +record-delete and wiki +delete-space.

Worse, on 14 of those commands a hand-written tip filled the vacuum with the opposite rule:

This is a high-risk write command. If the user explicitly requested it and the target is unambiguous, pass --yes without asking again.

Its precondition is that the user requested the operation — satisfied on the default path of any delete request — so it converts a request into consent, which is exactly what the framework ban forbids. Those tips predate the framework wording by roughly a month; they were each domain's own answer to a question the framework had not yet answered.

This PR makes the Risk: line have exactly one producer and one location across all 678 commands, deletes the tips that contradict it, and single-sources the ban itself so the four surfaces that mention --yes can no longer drift apart.

To be precise about scope: agent self-approval was never entirely unguarded — skills/lark-shared/SKILL.md already forbade appending --yes without consent. What was missing is the framework's own wording on the surface an agent reads when inspecting a command's contract.

Changes

One producer for the risk line

internal/cmdutil/risk.go — new exported RiskLine(cmd) (string, bool). The ban clause is gated on cmd.Flags().Lookup("yes") != nil alone, so it reaches every command offering the gate and no command that does not, independent of declared risk level. The parenthetical differs between the two gate shapes: at high-risk-write the framework refuses the whole command without --yes, whereas at lower levels --yes authorizes one destructive step while the rest runs unguarded.
cmd/service/affordance.go — PrepareMethodHelp and PrepareShortcutHelp no longer write Risk/Tips into Long; resolved tips move onto the command via cmdutil.SetTips, preserving the existing overlay-wins precedence. writeRisk lost all callers and is deleted.
cmd/root.go — neither the method nor the shortcut branch returns early, so both render Risk/Tips through the shared bottom-of-help append. The domain branch is untouched: domain help carries no Risk: line, only a prose legend.

One wording for the ban — internal/core gains YesSelfApprovalBan, embedded by all four surfaces in their own context rather than reworded per surface:

Surface	Before	After
command help	full ban (unreachable)	full ban, on every gated command
`internal/schema` `yes` property	"Pass `--yes` only after the user has explicitly confirmed"	embeds the ban
exit-10 hint (`internal/cmdutil/confirm.go`)	"add `--yes` to confirm"	"add `--yes` to confirm; " + ban
`shortcuts/apps/apps_env.go`	copy of the above	embeds the ban

The exit-10 hint is what an agent reads at the moment it decides whether to retry, and it was the weakest of the four.

Tips that contradicted the ban — removed the baseHighRiskYesTip constant (15 references across 14 base commands, plus the file it lived in) and two wiki tips. The base +role-delete tip was two clauses joined by a comma — a +role-get disambiguation hint plus a --yes instruction — so only the second clause is dropped and the hint is kept. Deliberately kept: tips whose precondition is already confirmed (base +base-block-delete), the strongest existing wording (apps +role-delete), and every --yes-bearing Example: line — those are complete runnable invocations, and stripping --yes would leave an agent to hit exit 10 and read the weakest copy instead.

A guard against regrowth — TestRiskLine_EveryYesGateCarriesTheBan walks the real command tree from Build() and asserts the invariant in both directions: every command registering --yes renders the ban, and no command without the flag does. It fails loudly if the set of sub-high-risk-write gates ever empties, since that is the coverage a level-keyed predicate would silently lose.

Docs — skills/lark-shared/SKILL.md said the risk line appears at the top of shortcut help. That was already wrong before this change (430 of 433 rendered it at the tail) and is corrected without anchoring to Global Flags:, which is hidden under a single-app config.

Behaviour changes worth calling out

245 raw method commands now render Risk: at the end of --help instead of near the top. This is the largest user-visible change here.
Three write-level commands now carry the ban: apps +env-set, drive +push and drive +pull implement a real --yes gate while declaring write. --yes there authorizes deleting Drive files, deleting local files, and writing the online environment respectively, so an agent that self-approves destroys data exactly as it would on a high-risk-write command. Two of the three had the ban nowhere at all before this change — their gates reject via a validation error, which carries no hint — and apps +env-set had it only in the runtime exit-10 hint, never in the help an agent reads while deciding.
lark-cli update renders a bare Risk: high-risk-write. It is annotated high-risk-write but registers no --yes flag and implements no gate, so the old wording pointed agents at a flag that does not exist. The annotation is deliberately left alone — it is consumed by the policy engine and by ByWrite extension hooks. Whether that command should gain a real gate or drop to write is filed below.

Test Plan

make unit-test passed
go build ./..., go vet, and gofmt -l are clean; golangci-lint run --new-from-rev=origin/main reports 0 issues
Full --help sweep over all 433 visible shortcuts: commands carrying the ban 0/53 -> 53/53; Risk: rendered before Usage: 3 -> 0; Tips bullet indents converged to a single width; shortcut count and risk distribution unchanged (52 high-risk-write / 204 write / 177 read)
Whole-tree audit over all 869 command nodes: of the 95 registering --yes, 95 render the ban (was 0); of the 774 without it, 0 do. TestRiskLine_EveryYesGateCarriesTheBan asserts both directions on every run, so the audit is no longer a one-off sweep
Regression boundary: domain help verified byte-identical against the base commit. Raw method help deliberately differs -- all 245 methods moved Risk: from the top to the tail, which is the point of the change; verified across six command shapes (shortcut, high-risk method, read method, overlay shortcut, hand-written command, domain group) that the only delta is that relocation
manual verification: lark-cli drive +delete --help, lark-cli im messages delete --help, lark-cli im chats get --help, lark-cli contact +get-user --help, lark-cli update --help, lark-cli im --help

The 53rd guarded shortcut is sheets +delete-sheet, hidden from sheets --help; a sweep driven by domain listings cannot see it, so the count was confirmed by walking the command tree.

Not covered: a probe of whether an agent copy-pastes a --yes-bearing Example: line rather than reading the ban. Filed below.

Related Issues

N/A

Found while auditing --help output. Follow-ups worth filing separately:

cmd/update/update.go is now the only high-risk-write command in the CLI with no --yes protocol at all. An agent that has learned "high-risk-write implies --yes" and finds no such flag may reasonably conclude the command is ungated -- and it does install software. It should either gain a real gate (cmdutil.RequireConfirmation already exists) or drop to write. Note that dropping it weakens MaxRisk policy interception, which is the only reason it was annotated in the first place.
Root --help still states unconditionally that high-risk-write needs --yes, which is false for update.
drive +push / drive +pull reject via a validation error, not the confirmation envelope, so they exit with the generic validation code rather than 10 and carry no structured risk / action fields. Their own error text calls the operation high-risk. Reconciling their declared level with the gate they actually implement is a behaviour change and belongs in its own PR; this one only makes the help honest about what they do.

Summary by CodeRabbit

New Features
- Help output now consistently displays risk warnings and safety guidance at the end of command details.
- High-risk operations clearly state that --yes requires explicit user confirmation.
- Overlay-provided tips are displayed consistently and take precedence where applicable.
Bug Fixes
- Corrected duplicated or misplaced guidance in method and shortcut help.
- Improved consistency of risk warnings across command variations.
Documentation
- Updated operation guidance to clarify confirmation requirements and warning placement.

RiskLine warned every high-risk-write command not to self-approve via --yes, but `update` is annotated high-risk-write without registering a --yes flag or any confirmation step. The sentence asserts the user confirmed via --yes, which is only true when the flag and gate exist; condition it on cmd.Flags().Lookup("yes") != nil so update.go and any future high-risk-write-without-gate command render the bare Risk line instead of pointing an agent at a flag that doesn't exist. Also narrows the writeRisk comment in affordance.go, which claimed the RiskLine wording was shared by "any other surface" — untrue, since internal/cmdutil/confirm.go and internal/schema/assembler.go each have independent --yes wording.

Update the risk-line test suites to match the corrected contract: a high-risk-write command only carries the confirmation guardrail when it actually registers --yes. Splits the old single assertion into a with-flag case (guardrail present) and a without-flag case shaped like `update` (bare Risk line, no --yes mention). Also adds TestHelpFunc_OverlayShortcutRendersRiskAndTipsOnce, which drives a real affordance overlay through service.PrepareShortcutHelp and the shared installTipsHelpFunc page-tail append together — the combination that previously double-rendered Risk and Tips. Nothing covered that path before.

The framework now renders a Risk: high-risk-write guardrail line on every high-risk-write shortcut's help page, stating --yes may only be passed after the user has confirmed. Several base/wiki shortcuts carried hand-written Tips telling the agent it could pass --yes on its own initiative once a request "seemed" explicit or unambiguous, which directly contradicts that guardrail and, being the last line on the page, would win. Remove the baseHighRiskYesTip constant (and its 14 usages) and the equivalent wiki delete-command tips, leaving the framework guardrail as the single source of truth for when --yes may be passed. Update the two base shortcut tests that asserted the now-removed wording to instead check the actual guardrail render path.

PrepareMethodHelp no longer folds Risk (writeRisk, now dead and deleted) or the affordance overlay's Tips into the top of Long. Tips resolution mirrors PrepareShortcutHelp: the overlay's tips win when present, otherwise the command's declarative tips are kept, and either way they move onto the command via cmdutil.SetTips for the shared bottom-of-help append. installTipsHelpFunc no longer returns early after PrepareMethodHelp, so every method command's Risk/Tips render after Usage: in the same place and wording as shortcuts. Domain help is unaffected (it carries a prose risk legend, not a Risk: line, and keeps its early return).

…ma and the exit-10 hint The "agent must not add --yes on its own" guardrail existed as four independently-worded copies (help RiskLine, schema yes-property description, confirm.go hint, apps_env.go hint), and the weakest of them was exactly the one an agent reads at the moment its command actually gets rejected. Added core.YesSelfApprovalBan as the single wording and had all four surfaces embed it in their own context instead of restating it.

…iption Concatenating core.YesSelfApprovalBan (lowercase-led, meant to follow a semicolon) after a period left a sentence-cased splice: "...if absent or false. the agent must NOT...". Switched the join to a semicolon so the schema description reads as one clean sentence.

coderabbitai · 2026-08-03T08:07:29Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 24d7e9bf-5f2d-407f-a438-bcebd8b85e6e

📥 Commits

Reviewing files that changed from the base of the PR and between bcc390e and cf2ecb0.

📒 Files selected for processing (30)

cmd/root.go
cmd/root_risk_help_test.go
cmd/service/affordance.go
cmd/service/affordance_test.go
internal/cmdutil/confirm.go
internal/cmdutil/confirm_test.go
internal/cmdutil/risk.go
internal/cmdutil/risk_test.go
internal/core/risk.go
internal/schema/assembler.go
shortcuts/apps/apps_env.go
shortcuts/base/base_advperm_disable.go
shortcuts/base/base_form_delete.go
shortcuts/base/base_form_questions_delete.go
shortcuts/base/base_form_submit.go
shortcuts/base/base_role_delete.go
shortcuts/base/base_role_update.go
shortcuts/base/base_shortcuts_test.go
shortcuts/base/dashboard_block_delete.go
shortcuts/base/dashboard_delete.go
shortcuts/base/field_delete.go
shortcuts/base/field_update.go
shortcuts/base/high_risk.go
shortcuts/base/record_delete.go
shortcuts/base/record_upload_attachment.go
shortcuts/base/table_delete.go
shortcuts/base/view_delete.go
shortcuts/wiki/wiki_delete.go
shortcuts/wiki/wiki_node_delete.go
skills/lark-shared/SKILL.md

💤 Files with no reviewable changes (16)

shortcuts/base/field_update.go
shortcuts/wiki/wiki_node_delete.go
shortcuts/base/record_upload_attachment.go
shortcuts/base/base_form_submit.go
shortcuts/wiki/wiki_delete.go
shortcuts/base/field_delete.go
shortcuts/base/table_delete.go
shortcuts/base/record_delete.go
shortcuts/base/dashboard_block_delete.go
shortcuts/base/base_form_delete.go
shortcuts/base/view_delete.go
shortcuts/base/base_role_update.go
shortcuts/base/high_risk.go
shortcuts/base/base_form_questions_delete.go
shortcuts/base/dashboard_delete.go
shortcuts/base/base_advperm_disable.go

🚧 Files skipped from review as they are similar to previous changes (14)

internal/core/risk.go
internal/schema/assembler.go
shortcuts/base/base_role_delete.go
cmd/service/affordance.go
internal/cmdutil/confirm_test.go
skills/lark-shared/SKILL.md
shortcuts/apps/apps_env.go
shortcuts/base/base_shortcuts_test.go
internal/cmdutil/confirm.go
internal/cmdutil/risk.go
cmd/root_risk_help_test.go
cmd/service/affordance_test.go
cmd/root.go
internal/cmdutil/risk_test.go

📝 Walkthrough

Walkthrough

The change centralizes risk and confirmation guidance in command help. Method and shortcut affordances now render tips separately from descriptions. Generic shortcut --yes tips were removed. Shared self-approval guidance now covers help, prompts, schemas, tests, and documentation.

Changes

Risk-aware command help

Layer / File(s)	Summary
Shared risk and confirmation contracts `internal/core/risk.go`, `internal/cmdutil/risk.go`, `internal/cmdutil/confirm.go`, `internal/schema/assembler.go`, `shortcuts/apps/apps_env.go`, related tests	`RiskLine` formats risk guidance. Confirmation messages include the shared self-approval restriction.
Unified affordance and help rendering `cmd/root.go`, `cmd/service/affordance.go`, `cmd/root_risk_help_test.go`, `cmd/service/affordance_test.go`	Method and shortcut help stores tips on commands and renders Risk and Tips through the shared bottom-of-help path. Tests cover ordering, deduplication, flag conditions, and command-tree invariants.
Shortcut guidance migration `shortcuts/base/`, `shortcuts/wiki/`, `skills/lark-shared/SKILL.md`	Generic high-risk `--yes` tips were removed from shortcuts. Shortcut tests and skill guidance now use the shared warning location and wording.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant RootHelp
  participant AffordanceService
  participant RiskLine
  RootHelp->>AffordanceService: prepare method or shortcut help
  AffordanceService->>RootHelp: return Long text and resolved Tips
  RootHelp->>RiskLine: format command risk
  RiskLine-->>RootHelp: return Risk line
  RootHelp-->>RootHelp: render Risk and Tips at help output end

Possibly related issues

Reduce duplicated context in bundled Lark Skills by relying on CLI schemas and risk gates #2140 — Addresses the same --yes confirmation policy and risk guidance behavior.

Suggested reviewers: liangshuo-1

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 46.15% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes making the --yes self-approval ban reachable across all command surfaces.
Description check	✅ Passed	The description covers the summary, changes, test plan, and related issues with detailed implementation and verification information.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/unify-shortcut-help-risk-rendering

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

CLAassistant · 2026-08-03T08:07:30Z

All committers have signed the CLA.

codecov · 2026-08-03T08:13:14Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.56%. Comparing base (6e5308a) to head (cf2ecb0).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2151      +/-   ##
==========================================
+ Coverage   75.25%   75.56%   +0.31%     
==========================================
  Files         916      931      +15     
  Lines       97167    99363    +2196     
==========================================
+ Hits        73121    75087    +1966     
- Misses      18433    18544     +111     
- Partials     5613     5732     +119

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The ban clause was conditioned on level == high-risk-write && has --yes, which silently skipped three commands that implement a real --yes gate while declaring write: apps +env-set, drive +push and drive +pull. There --yes authorizes writing the online environment, deleting Drive files and deleting local files, so an agent that self-approves destroys data exactly as it would on a high-risk-write command. Two of the three carried the ban nowhere at all: their gates reject with a validation error, which has no hint field. apps +env-set carried it only in the runtime exit-10 hint, never in the help an agent reads while deciding whether to add the flag. Condition on the presence of --yes alone. The parenthetical still differs by shape, because the two mean different things: at high-risk-write the framework refuses the whole command, whereas at lower levels --yes authorizes one destructive step while the rest runs unguarded. Add TestRiskLine_EveryYesGateCarriesTheBan, which walks the real tree from Build() and asserts both directions over all 869 nodes: 95 register --yes and all 95 render the ban; the other 774 render none. It also fails if the set of sub-high-risk-write gates ever empties, since that is the coverage a level-keyed predicate loses without any test noticing.

The removed tip joined two clauses with a comma: a disambiguation hint and a --yes instruction. Only the second contradicted the framework ban, but the whole line went, taking with it the only pointer to +role-get for an ambiguous role target. Restore the hint without the --yes clause.

github-actions · 2026-08-03T09:32:33Z

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@cf2ecb0e0a6ce882f4cc83d770be1f53bf26bcdd

🧩 Skill update

npx skills add larksuite/cli#fix/unify-shortcut-help-risk-rendering -y -g

coderabbitai

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

shortcuts/base/base_shortcuts_test.go (1)
230-250: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Assert removal of the generic high-risk tip.

The test comment requires shortcut Tips not to restate the RiskLine guidance, but the assertions inspect only RiskLine. Reintroducing the generic baseHighRiskYesTip would still pass. Assert that the generic confirmation wording is absent from cmdutil.GetTips(cmd), while allowing command-specific examples that contain --yes.

As per coding guidelines, contract tests must assert the changed behavior directly so reverting the implementation causes failure.
🤖 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 `@shortcuts/base/base_shortcuts_test.go` around lines 230 - 250, Update
TestBaseHighRiskShortcutsTipsGuideAgents to inspect cmdutil.GetTips(cmd) and
assert the generic baseHighRiskYesTip confirmation wording is absent, while
retaining the existing RiskLine agent-guidance assertion. Allow command-specific
tips or examples containing --yes; the test must fail if the generic high-risk
tip is reintroduced.
Source: Coding guidelines

🤖 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/service/affordance_test.go`:
- Around line 122-125: In the affected test setup before calling
cmdutil.TestFactory, use t.Setenv to set LARKSUITE_CLI_CONFIG_DIR to
t.TempDir(). Keep the existing factory and command construction unchanged while
ensuring this test uses an isolated configuration directory.
- Around line 211-229: Extend the existing `PrepareShortcutHelp` test for `both`
to assert that the overlay tip is absent from `both.Long`, while preserving the
current `cmdutil.GetTips` replacement assertion. Ensure the assertion fails if
`"overlay wins"` is rendered in the command’s long help text.

In `@internal/cmdutil/risk.go`:
- Around line 85-89: The write-level `--yes` branch in `RiskLine` lacks direct
unit coverage. Add a test case to `TestRiskLine_LowerLevelsRenderBare` or a
focused new test that registers a `--yes` flag on the command, calls `RiskLine`
for the applicable lower risk level, and asserts the result contains both
`"--yes authorizes a destructive step"` and `core.YesSelfApprovalBan`.

---

Outside diff comments:
In `@shortcuts/base/base_shortcuts_test.go`:
- Around line 230-250: Update TestBaseHighRiskShortcutsTipsGuideAgents to
inspect cmdutil.GetTips(cmd) and assert the generic baseHighRiskYesTip
confirmation wording is absent, while retaining the existing RiskLine
agent-guidance assertion. Allow command-specific tips or examples containing
--yes; the test must fail if the generic high-risk tip is reintroduced.

🪄 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 Plus

Run ID: 52620ea9-419b-4f73-bac9-35e6b480cf6d

📥 Commits

Reviewing files that changed from the base of the PR and between 427cbd6 and 0e15873.

📒 Files selected for processing (30)

cmd/root.go
cmd/root_risk_help_test.go
cmd/service/affordance.go
cmd/service/affordance_test.go
internal/cmdutil/confirm.go
internal/cmdutil/confirm_test.go
internal/cmdutil/risk.go
internal/cmdutil/risk_test.go
internal/core/risk.go
internal/schema/assembler.go
shortcuts/apps/apps_env.go
shortcuts/base/base_advperm_disable.go
shortcuts/base/base_form_delete.go
shortcuts/base/base_form_questions_delete.go
shortcuts/base/base_form_submit.go
shortcuts/base/base_role_delete.go
shortcuts/base/base_role_update.go
shortcuts/base/base_shortcuts_test.go
shortcuts/base/dashboard_block_delete.go
shortcuts/base/dashboard_delete.go
shortcuts/base/field_delete.go
shortcuts/base/field_update.go
shortcuts/base/high_risk.go
shortcuts/base/record_delete.go
shortcuts/base/record_upload_attachment.go
shortcuts/base/table_delete.go
shortcuts/base/view_delete.go
shortcuts/wiki/wiki_delete.go
shortcuts/wiki/wiki_node_delete.go
skills/lark-shared/SKILL.md

💤 Files with no reviewable changes (16)

shortcuts/base/high_risk.go
shortcuts/base/base_role_update.go
shortcuts/base/dashboard_block_delete.go
shortcuts/wiki/wiki_delete.go
shortcuts/base/base_form_delete.go
shortcuts/base/view_delete.go
shortcuts/base/base_form_submit.go
shortcuts/base/field_update.go
shortcuts/base/base_advperm_disable.go
shortcuts/base/record_upload_attachment.go
shortcuts/base/field_delete.go
shortcuts/base/table_delete.go
shortcuts/wiki/wiki_node_delete.go
shortcuts/base/record_delete.go
shortcuts/base/dashboard_delete.go
shortcuts/base/base_form_questions_delete.go

luozhixiong01 · 2026-08-03T09:52:15Z

`e2e-live` is failing on `main`, not from this PR

e2e-live is red here, and results is its aggregate. The failure predates this branch:

commit	`e2e-live`
`6e5308a` — merge base of this branch	success
`427cbd6` — current `main` HEAD, the base GitHub compares against	failure

Evidence that this PR cannot be the cause:

One subtest fails, and only it: TestContact_LookupWorkflowAsBot/discover_user_via_api_as_bot, identical before and after the two review-fix commits pushed to this branch.
It asserts tenant data, not help text: the subtest runs api get /open-apis/contact/v3/users as bot and requires data.items.0.open_id to be non-empty. The failure is Should NOT be empty — the tenant returned an empty user list.
Zero file overlap: this PR touches 30 files under cmd/, internal/, shortcuts/{base,wiki,apps} and skills/lark-shared/. The failing test lives in tests/cli_e2e/contact/contact_lookup_workflow_test.go. Nothing here can reach an API response body.

Everything else is green: 21 checks pass, including unit-test, lint, coverage, deterministic-gate, deadcode, e2e-dry-run, security and all three CodeQL analyses.

Earlier fail entries with a 0s duration on this PR were run supersessions, not test failures — a force-push and a description edit each started a newer workflow run that cancelled the previous one.

Three gaps raised in review, all cases where a test would still pass after the implementation it covers was reverted: The write-level --yes branch of RiskLine had no direct unit test. The tree-wide invariant in package cmd reaches it only through the shortcuts that happen to have that shape today, so the contract was asserted by coincidence rather than by construction. Add a case that registers --yes at write level and pins all three properties: the ban is present, the wording is scoped to the gated step, and the high-risk-write phrasing is absent (claiming the whole command is gated would be false there). Verified by restoring the old level-keyed condition, which fails this test. TestPrepareShortcutHelp asserted that overlay tips land on the command but not that they leave Long. Rendering them in both places -- the exact defect this branch removes -- would have passed. Assert the absence too. TestPrepareMethodHelp built a factory without isolating the config dir, against the convention used by 114 test files in this repo including cmd/service/service_paginate_test.go. Point it at t.TempDir().

luozhixiong01 added 14 commits July 30, 2026 21:43

feat: add RiskLine as the single source of the help risk line

959e29d

refactor: render the affordance risk line through cmdutil.RiskLine

819dcbe

fix: keep shortcut risk and tips out of Long so they render once

0d47510

fix: restore the --yes guardrail on every high-risk-write shortcut help

9f63dae

docs: correct where the risk line appears in shortcut help

49254d1

docs: drop the environment-dependent anchor from the risk line hint

a1da197

fix: drop the base role-delete tip that frames --yes as the confirmation

1efe1c3

docs: sync the exit-10 hint example with the single-sourced ban

e316a85

luozhixiong01 added the bugfix Bug fixes label Aug 3, 2026

github-actions Bot added domain/base PR touches the base domain domain/ccm PR touches the ccm domain size/XL Architecture-level or global-impact change labels Aug 3, 2026

luozhixiong01 added 2 commits August 3, 2026 16:47

luozhixiong01 force-pushed the fix/unify-shortcut-help-risk-rendering branch from b93d220 to 0e15873 Compare August 3, 2026 08:59

luozhixiong01 marked this pull request as ready for review August 3, 2026 09:26

luozhixiong01 requested a review from liangshuo-1 as a code owner August 3, 2026 09:26

coderabbitai Bot reviewed Aug 3, 2026

View reviewed changes

Comment thread cmd/service/affordance_test.go

Comment thread cmd/service/affordance_test.go

Comment thread internal/cmdutil/risk.go

luozhixiong01 force-pushed the fix/unify-shortcut-help-risk-rendering branch from bcc390e to cf2ecb0 Compare August 3, 2026 10:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: make the --yes self-approval ban reachable on every surface - #2151

fix: make the --yes self-approval ban reachable on every surface#2151
luozhixiong01 wants to merge 17 commits into
mainfrom
fix/unify-shortcut-help-risk-rendering

luozhixiong01 commented Aug 3, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 3, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

CLAassistant commented Aug 3, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Aug 3, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 3, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

luozhixiong01 commented Aug 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

luozhixiong01 commented Aug 3, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Behaviour changes worth calling out

Test Plan

Related Issues

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

CLAassistant commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🚀 PR Preview Install Guide

🧰 CLI update

🧩 Skill update

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

luozhixiong01 commented Aug 3, 2026

e2e-live is failing on main, not from this PR

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

luozhixiong01 commented Aug 3, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 3, 2026 •

edited

Loading

CLAassistant commented Aug 3, 2026 •

edited

Loading

codecov Bot commented Aug 3, 2026 •

edited

Loading

github-actions Bot commented Aug 3, 2026 •

edited

Loading

`e2e-live` is failing on `main`, not from this PR