Skip to content

[auto-research] 01KVWPVKPPB9A5CE0W0C28VNSX: lark cli auth - tdx - #1591

Closed
yballul-bytedance wants to merge 1 commit into
mainfrom
auto-research-sync/01KVWPVKPPB9A5CE0W0C28VNSX/mr-573-395863ea
Closed

[auto-research] 01KVWPVKPPB9A5CE0W0C28VNSX: lark cli auth - tdx#1591
yballul-bytedance wants to merge 1 commit into
mainfrom
auto-research-sync/01KVWPVKPPB9A5CE0W0C28VNSX/mr-573-395863ea

Conversation

@yballul-bytedance

@yballul-bytedance yballul-bytedance commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Source

Refs

  • Codebase base: de40fa645ffec37379853526d28895c20162288e
  • Codebase head: 395863ea8ef983d0cce7bd89c0f57c9960edebfe
  • GitHub base: 40a09c8957b25c66e160dbfdde330c7c74373273
  • Sync branch: auto-research-sync/01KVWPVKPPB9A5CE0W0C28VNSX/mr-573-395863ea

Sync Result

Worker mechanically applied the Codebase MR patch with git apply and created a sync commit.

Checks

  • git apply --check: passed
  • git diff --check: passed

Summary by CodeRabbit

  • Documentation

    • Expanded authentication guidance to cover login/status/logout flows, identity (user vs bot), domain vs scope permissions, missing-scope handling, authorization revocation, and interpreting notice payloads.
    • Added “认证任务速查” with intent → preferred command guidance, including logout semantics and bot-permission troubleshooting.
    • Updated update-notice behavior guidance and documented environment settings for stable JSON output.
  • Tests

    • Tightened logout success JSON assertions to confirm only expected fields are present, including verifying the message field is omitted.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 43e5c8c1-6697-43f3-8d18-fdc4b3b3cf85

📥 Commits

Reviewing files that changed from the base of the PR and between da2bdd3 and bcbec3b.

📒 Files selected for processing (2)
  • cmd/auth/logout_test.go
  • skills/lark-shared/SKILL.md

📝 Walkthrough

Walkthrough

The logout JSON success test now asserts that message is omitted, and the shared lark-cli skill doc expands auth guidance and revises _notice.update handling.

Changes

Auth logout and guidance

Layer / File(s) Summary
Logout JSON output check
cmd/auth/logout_test.go
The JSON-mode logout success test now fails if the stdout payload includes message.
Auth quick reference
skills/lark-shared/SKILL.md
The skill description expands the auth scope, and a new quick-reference section adds intent-to-command guidance for login, status, logout, domain/scope authorization, bot permission checks, and _notice handling.
_notice update guidance
skills/lark-shared/SKILL.md
The update-check section changes _notice.update handling to defer update messaging unless the user asks about updates/version/notice, while keeping the stable JSON environment-variable guidance.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • larksuite/cli#884: Its lark-cli update / _notice.update handling is directly related to the updated notice guidance in skills/lark-shared/SKILL.md.

Suggested labels

domain/ccm

Suggested reviewers

  • liangshuo-1
  • MaxHuang22

Poem

A bunny hopped with a JSON cheer,
No stray message could appear.
The auth guide now shines so neat,
With _notice kept tidy and sweet. 🐇

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description omits the required Summary, Changes, Test Plan, and Related Issues sections from the repository template. Add the template sections with a brief summary, bullet list of changes, verification steps, and related issues link or None.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title references the auth sync task and is related to the change, though it does not clearly summarize the specific modifications.
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
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch auto-research-sync/01KVWPVKPPB9A5CE0W0C28VNSX/mr-573-395863ea

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.

@github-actions github-actions Bot added the size/L Large or sensitive change across domains or core paths label Jun 26, 2026
@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.75%. Comparing base (ba51d48) to head (bcbec3b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1591   +/-   ##
=======================================
  Coverage   74.75%   74.75%           
=======================================
  Files         800      800           
  Lines       80459    80459           
=======================================
+ Hits        60147    60150    +3     
+ Misses      15857    15854    -3     
  Partials     4455     4455           

☔ 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.

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#auto-research-sync/01KVWPVKPPB9A5CE0W0C28VNSX/mr-573-395863ea -y -g

…ut success payload

- Broaden lark-shared SKILL description to cover auth login/status/logout, identity switching, business-domain permissions, scope handling, revoking authorization, and _notice JSON output.

- Add an authentication quick-reference table for common intents (full/domain/scope login, status verification, logout, bot scope guidance, revoking server-side authorization) and document LARKSUITE_CLI_NO_UPDATE_NOTIFIER / LARKSUITE_CLI_NO_SKILLS_NOTIFIER for stable machine-readable JSON output.

- Soften the _notice.update handling rule so agents complete the current request first instead of interrupting to repeatedly check help; keep `lark-cli update` as the canonical update path.

- Extend TestAuthLogoutRun_JSONMode_Success_WritesStdoutOnly to assert that the success payload omits the `message` field, alongside the existing `reason` absence check.
@yballul-bytedance
yballul-bytedance force-pushed the auto-research-sync/01KVWPVKPPB9A5CE0W0C28VNSX/mr-573-395863ea branch from da2bdd3 to bcbec3b Compare June 26, 2026 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant