Skip to content

feat: add mail signature write shortcuts#1304

Open
cnzakii wants to merge 1 commit into
larksuite:mainfrom
cnzakii:feat/5bdc7c2
Open

feat: add mail signature write shortcuts#1304
cnzakii wants to merge 1 commit into
larksuite:mainfrom
cnzakii:feat/5bdc7c2

Conversation

@cnzakii

@cnzakii cnzakii commented Jun 6, 2026

Copy link
Copy Markdown

Generated by the harness-coding skill.

  • Branch: feat/5bdc7c2
  • Target: main

Sprints

ID Title Status Commit
S4 Add +signature-create/update/delete shortcuts + registration + skill docs passed e63d4b8
S6 Synthesize transport contract for larksuite/cli passed bd07859

Source specs


This MR was created autonomously. Quality gates were enforced by the repo's own pre-commit hooks.

Summary by CodeRabbit

  • New Features

    • Added mail signature management with commands to create, update, and delete signatures.
    • HTML content support with automatic image handling: local images are uploaded and converted to cid: references.
    • Update command uses full-replace semantics; unspecified fields are cleared.
  • Documentation

    • Added comprehensive guides for all signature management commands with usage examples and parameter descriptions.

Add create, update, and delete shortcuts for personal mail signatures, including inline image upload and cid rewrite reuse from the template pipeline.

sprint: S4
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Codex seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown

Worried about impact? Review this PR in Change Stack to explore blast radius before you approve or request changes.

Review Change Stack

📝 Walkthrough

Walkthrough

This PR implements three new mail signature management commands (+signature-create, +signature-update, +signature-delete) with shared infrastructure for content resolution, image uploading to Drive with CID rewriting, API communication, error handling, and comprehensive test coverage, plus user-facing documentation.

Changes

Mail Signature CRUD Shortcuts

Layer / File(s) Summary
Shared signature write infrastructure
shortcuts/mail/mail_signature_write.go
Helper functions for content/device resolution, HTML image parsing and upload to Drive with CID rewriting, API calls (create/update/delete), response extraction, error decoration with user-facing hints, and formatted output.
Create, Update, Delete shortcuts
shortcuts/mail/mail_signature_create.go, shortcuts/mail/mail_signature_update.go, shortcuts/mail/mail_signature_delete.go, shortcuts/mail/shortcuts.go
Three parallel shortcut implementations with DryRun/Validate/Execute handlers; create POSTs new signature, update PUTs with full-replace warning, delete DELETEs by ID; all registered in shortcut registry.
Test coverage
shortcuts/mail/mail_signature_write_test.go
Metadata validation, create/update/delete end-to-end tests with stubbed HTTP endpoints, input validation error messages, and error decoration behavior.
User documentation
skill-template/domains/mail.md, skills/lark-mail/SKILL.md, skills/lark-mail/references/lark-mail-signature-*.md
Domain guidance on bot mailbox constraints and full-replace semantics, SKILL.md updates, and dedicated reference pages for each command with examples, parameters, responses, and error codes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • larksuite/cli#642: Both implement local HTML <img src> path parsing, Drive upload, CID generation, and cid: rewriting—in this PR for mail signatures and the related PR for mail templates.

Suggested labels

domain/mail, size/L

Suggested reviewers

  • chanthuang
  • infeng

Poem

🐰 A rabbit hops through mailboxes bright,
Creating signatures, deleting with might!
Images rewritten to cid: so fine,
Three shortcuts now dance in perfect line! 📧✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is largely incomplete against the repository template, missing required sections: Summary, Changes list, Test Plan, and Related Issues. Add required sections: a 1–3 sentence Summary of motivation/scope, a bulleted Changes list, a Test Plan with verification steps, and a Related Issues section (or explicitly state 'None').
Docstring Coverage ⚠️ Warning Docstring coverage is 5.56% 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 'feat: add mail signature write shortcuts' clearly and concisely summarizes the primary change—the addition of new mail signature write commands (create, update, delete).
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@github-actions github-actions Bot added domain/mail PR touches the mail domain size/L Large or sensitive change across domains or core paths labels Jun 6, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)
shortcuts/mail/mail_signature_write_test.go (1)

1-246: ⚠️ Potential issue | 🟠 Major

Add dry-run E2E coverage for mail signature shortcuts

The dry-run E2E suite under tests/cli_e2e/ has no test files (and no dry-run test content) referencing MailSignature or +signature-create / +signature-update / +signature-delete, so the new shortcuts lack the required --dry-run request-structure coverage. Add corresponding dry-run E2E tests under tests/cli_e2e/mail/ (or tests/cli_e2e/dryrun/) that validate payloads without real API calls.

🤖 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/mail/mail_signature_write_test.go` around lines 1 - 246, The PR is
missing dry-run E2E coverage for the new mail signature shortcuts; add E2E tests
that exercise MailSignatureCreate, MailSignatureUpdate and MailSignatureDelete
with --dry-run and assert the generated request payloads instead of performing
real API calls. Create test files under tests/cli_e2e/mail/ (or
tests/cli_e2e/dryrun/) that invoke the CLI with "+signature-create --dry-run
...", "+signature-update --dry-run ...", and "+signature-delete --dry-run ..."
and validate the produced request bodies and flags (name, content/content-file
mutex, device, signature-id, mailbox) match the expected structure, and ensure
the tests verify no network calls are made (dry-run mode) and that payloads
include full-replace behavior for update when content omitted. Reference the
shortcut symbols MailSignatureCreate, MailSignatureUpdate, MailSignatureDelete
when locating which behaviors/flags to cover.

Source: Learnings

🤖 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 `@skills/lark-mail/references/lark-mail-signature-create.md`:
- Line 58: Remove the irrelevant error code entry "找不到签名
ID(`15180302`,通常出现在更新/删除)" from the +signature-create documentation
(lark-mail-signature-create.md): locate the line referencing error code 15180302
and delete it so the create docs no longer list an update/delete-only error, and
ensure 15180302 remains documented only in the signature update/delete docs.

---

Outside diff comments:
In `@shortcuts/mail/mail_signature_write_test.go`:
- Around line 1-246: The PR is missing dry-run E2E coverage for the new mail
signature shortcuts; add E2E tests that exercise MailSignatureCreate,
MailSignatureUpdate and MailSignatureDelete with --dry-run and assert the
generated request payloads instead of performing real API calls. Create test
files under tests/cli_e2e/mail/ (or tests/cli_e2e/dryrun/) that invoke the CLI
with "+signature-create --dry-run ...", "+signature-update --dry-run ...", and
"+signature-delete --dry-run ..." and validate the produced request bodies and
flags (name, content/content-file mutex, device, signature-id, mailbox) match
the expected structure, and ensure the tests verify no network calls are made
(dry-run mode) and that payloads include full-replace behavior for update when
content omitted. Reference the shortcut symbols MailSignatureCreate,
MailSignatureUpdate, MailSignatureDelete when locating which behaviors/flags to
cover.
🪄 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

Run ID: 66db5b00-a286-4868-a8af-f72db411d482

📥 Commits

Reviewing files that changed from the base of the PR and between 5788a6c and e63d4b8.

📒 Files selected for processing (11)
  • shortcuts/mail/mail_signature_create.go
  • shortcuts/mail/mail_signature_delete.go
  • shortcuts/mail/mail_signature_update.go
  • shortcuts/mail/mail_signature_write.go
  • shortcuts/mail/mail_signature_write_test.go
  • shortcuts/mail/shortcuts.go
  • skill-template/domains/mail.md
  • skills/lark-mail/SKILL.md
  • skills/lark-mail/references/lark-mail-signature-create.md
  • skills/lark-mail/references/lark-mail-signature-delete.md
  • skills/lark-mail/references/lark-mail-signature-update.md

## 错误提示

- 重名(`15180303`):换一个 `--name`,或改用 `+signature-update` 更新已有签名。
- 找不到签名 ID(`15180302`,通常出现在更新/删除):先运行 `lark-cli mail +signature` 获取真实 ID。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove irrelevant error code from create documentation.

Error code 15180302 ("找不到签名 ID") with the note "通常出现在更新/删除" (usually appears in update/delete) is not relevant to the +signature-create command. Create operations POST new signatures and cannot encounter "signature not found" errors. This error code belongs only in the update and delete documentation.

📝 Suggested fix
 ## 错误提示
 
 - 重名(`15180303`):换一个 `--name`,或改用 `+signature-update` 更新已有签名。
-- 找不到签名 ID(`15180302`,通常出现在更新/删除):先运行 `lark-cli mail +signature` 获取真实 ID。
 - 权限不足(`15180305`):检查 `mail:user_mailbox.message:modify` scope、邮箱权限,以及 bot 身份是否传了显式 `--mailbox`。
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- 找不到签名 ID(`15180302`,通常出现在更新/删除):先运行 `lark-cli mail +signature` 获取真实 ID。
## 错误提示
- 重名(`15180303`):换一个 `--name`,或改用 `+signature-update` 更新已有签名。
- 权限不足(`15180305`):检查 `mail:user_mailbox.message:modify` scope、邮箱权限,以及 bot 身份是否传了显式 `--mailbox`
🤖 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 `@skills/lark-mail/references/lark-mail-signature-create.md` at line 58, Remove
the irrelevant error code entry "找不到签名 ID(`15180302`,通常出现在更新/删除)" from the
+signature-create documentation (lark-mail-signature-create.md): locate the line
referencing error code 15180302 and delete it so the create docs no longer list
an update/delete-only error, and ensure 15180302 remains documented only in the
signature update/delete docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/mail PR touches the mail domain size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants