Validate mail shortcut mailbox IDs before requests by bubbmon233 · Pull Request #1442 · larksuite/cli

bubbmon233 · 2026-06-12T11:35:16Z

Generated by the harness-coding skill.

Branch: feat/35d1f60
Target: main

Sprints

ID	Title	Status	Commit
S1	Validate mail +message and +triage mailbox IDs before OAPI calls	passed	`93df9a0`
S2	Synthesize transport contract for larksuite/cli	passed	`69f335b`

Source specs

tech-design.md

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

Summary by CodeRabbit

Bug Fixes
- Mail shortcuts now validate user mailbox IDs during validation, accepting "me" or a precise email address and rejecting empty, padded, newline/control-char, display-name, comma-separated, or malformed values.
Tests
- Added validation tests ensuring invalid mailbox inputs fail early and valid mailboxes still produce expected dry-run output.

coderabbitai · 2026-06-12T11:35:30Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: aff58491-77ee-4920-a2f0-eeb86b8ccfe4

📥 Commits

Reviewing files that changed from the base of the PR and between 93df9a0 and 2a34ab8.

📒 Files selected for processing (1)

shortcuts/mail/mail_shortcut_validation_test.go

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

shortcuts/mail/mail_shortcut_validation_test.go

📝 Walkthrough

Walkthrough

Adds validateUserMailboxID helper and applies it in MailMessage and MailTriage Validate flows; unit and integration tests ensure invalid --mailbox values are rejected before runtime/HTTP output.

Changes

Mailbox ID validation for mail shortcuts

Layer / File(s)	Summary
Mailbox ID validation helper and unit tests `shortcuts/mail/helpers.go`, `shortcuts/mail/mail_shortcut_validation_test.go`	New `validateUserMailboxID` enforces `--mailbox` is `"me"` or an exact, control-character-free email address parsed via `net/mail` with no display name. Added test helper `assertValidationParamError` and unit tests covering valid and various invalid inputs.
MailMessage shortcut validation integration `shortcuts/mail/mail_message.go`, `shortcuts/mail/mail_shortcut_validation_test.go`	`MailMessage.Validate` now runs bot-mailbox check then validates the resolved user mailbox ID; integration test verifies invalid `--mailbox` stops validation with expected error and produces no stdout/HTTP interaction.
MailTriage shortcut validation integration `shortcuts/mail/mail_triage.go`, `shortcuts/mail/mail_shortcut_validation_test.go`	`MailTriage.Validate` applies the same two-stage validation; tests assert invalid mailboxes prevent dry-run API output and that valid explicit email mailboxes generate the expected `/open-apis/mail/v1/user_mailboxes/<mailbox>/messages` path.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

larksuite/cli#895: Related prior work around mailbox validation and bot mailbox checks used by MailMessage/MailTriage.

Suggested labels

domain/mail, size/M

Suggested reviewers

chanthuang
infeng

Poem

🐰 I hop through flags with careful eyes,
"me" or clean email—no sneaky disguise,
I trim the whites and ban the control,
So triage and messages behave as a whole,
A tidy mailbox makes the bunny wise.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The provided description deviates significantly from the repository's template; it lacks required sections (Summary, Changes, Test Plan with checkboxes, Related Issues) and instead provides internal metadata (branch, sprint info, source specs).	Restructure the description to follow the template: add a Summary section (1-3 sentences), enumerate Changes as bullet points, document Test Plan with checkbox items, and list Related Issues.
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.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 directly and clearly describes the main change: adding mailbox ID validation to mail shortcuts before making API requests.
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.}

coderabbitai

Actionable comments posted: 1

🤖 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 `@shortcuts/mail/mail_shortcut_validation_test.go`:
- Around line 184-256: Update the failing-branch tests to assert typed
validation metadata instead of only message substrings: replace or augment the
assertValidationError calls in TestValidateUserMailboxID (the invalid mailbox
loop), TestMailMessageInvalidMailboxValidationStopsBeforeHTTP, and
TestMailTriageInvalidMailboxValidationStopsDryRun by calling errs.ProblemOf(err)
and asserting its category/subtype match the validation error, and use
errors.As(err, &ve) with ve of type *errs.ValidationError to assert ve.Param ==
"--mailbox"; keep the existing substring checks only as optional extra info.
Ensure you reference validateUserMailboxID when validating unit tests and the
runMountedMailShortcut/MailTriage/MailMessage test branches so the typed
assertions run on the same error values.

🪄 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: 9f21db4e-93f0-4faf-9cfd-c852b8d3ee5c

📥 Commits

Reviewing files that changed from the base of the PR and between 69f335b and 93df9a0.

📒 Files selected for processing (4)

shortcuts/mail/helpers.go
shortcuts/mail/mail_message.go
shortcuts/mail/mail_shortcut_validation_test.go
shortcuts/mail/mail_triage.go

codecov · 2026-06-12T11:40:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.89%. Comparing base (69f335b) to head (2a34ab8).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1442      +/-   ##
==========================================
+ Coverage   72.86%   72.89%   +0.02%     
==========================================
  Files         738      738              
  Lines       69552    69569      +17     
==========================================
+ Hits        50679    50711      +32     
+ Misses      15057    15043      -14     
+ Partials     3816     3815       -1

☔ 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 · 2026-06-12T11:41:02Z

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add bubbmon233/cli#feat/35d1f60 -y -g

Change-Type: ci-fix

bubbmon233 · 2026-06-12T12:47:05Z

+		if err := validateBotMailboxNotMe(runtime); err != nil {
+			return err
+		}
+		return validateUserMailboxID("--mailbox", resolveMailboxID(runtime))


🤖 AI Review | [P2 正确性] bot 身份下显式空 mailbox 会绕过 me 校验

resolveMailboxID 会把空字符串归一成 me，但这里先调用的 validateBotMailboxNotMe 只检查原始 runtime.Str("mailbox") == "me"。因此 mail +message --as bot --mailbox "" 会通过 Validate，随后 dry-run/execute 继续生成 /user_mailboxes/me/...；+triage 同样有这个顺序。业务影响：bot 使用 tenant token 时无法解析 me，本应被本地参数校验拦截的请求会进入错误路径或误导 dry-run 输出。

修复建议: 先解析一次 mailboxID := resolveMailboxID(runtime)，并让 bot/me 校验基于解析后的值；同时补 --as bot --mailbox "" 的 +message 和 +triage 测试。

如有疑问或认为判断不准确，欢迎直接回复讨论。

bubbmon233 · 2026-06-12T12:47:35Z

🤖 AI Review | CR 汇总 | 可合入（1 个建议项）

增量审查：基于已有 1 条已解决评论，本次新增 1 条 P2 正确性建议。核心实现符合需求，但建议补齐 --as bot --mailbox "" 被归一成 me 后仍应拒绝的边界。

fix(mail): validate shortcut mailbox ids

93df9a0

github-actions Bot added domain/mail PR touches the mail domain size/M Single-domain feat or fix with limited business impact labels Jun 12, 2026

coderabbitai Bot reviewed Jun 12, 2026

View reviewed changes

Comment thread shortcuts/mail/mail_shortcut_validation_test.go

test(mail): assert mailbox validation metadata

2a34ab8

Change-Type: ci-fix

bubbmon233 commented Jun 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Validate mail shortcut mailbox IDs before requests#1442

Validate mail shortcut mailbox IDs before requests#1442
bubbmon233 wants to merge 2 commits into
larksuite:mainfrom
bubbmon233:feat/35d1f60

bubbmon233 commented Jun 12, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 12, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (2 warnings)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

codecov Bot commented Jun 12, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 12, 2026 •

edited

Loading

Uh oh!

bubbmon233 Jun 12, 2026

Uh oh!

bubbmon233 commented Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

bubbmon233 commented Jun 12, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sprints

Source specs

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (2 warnings)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Jun 12, 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 Jun 12, 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!

bubbmon233 Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

bubbmon233 commented Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

bubbmon233 commented Jun 12, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 12, 2026 •

edited

Loading

codecov Bot commented Jun 12, 2026 •

edited

Loading

github-actions Bot commented Jun 12, 2026 •

edited

Loading