Include mailbox_id in mail +triage output for public mailbox support by xukuncx · Pull Request #1227 · larksuite/cli

xukuncx · 2026-06-02T13:00:59Z

Generated by the harness-coding skill.

Branch: feat/c5b5818
Target: main

Sprints

ID	Title	Status	Commit
S1	Preserve mailbox context in mail triage output	passed	`79be1c4`
S2	Synthesize transport contract for larksuite/cli	passed	`0aa9e96`

Source specs

lightweight-requirement.md

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

Summary by CodeRabbit

New Features
- mail +triage output now includes mailbox ID in results across all formats (JSON, data, and table).
Documentation
- Updated tip message to clarify how to read full message content using the mail +message command with mailbox parameter.
Tests
- Added test cases to verify metadata preservation in triage message processing.

When mail +triage reads messages from a public/shared mailbox, the structured output now includes the resolved mailbox_id so downstream consumers (e.g. mail +message) can use the correct mailbox instead of defaulting to /user_mailboxes/me. Changes: - Add mailbox_id to every message in JSON/data output - Add mailbox_id as a top-level field in JSON/data output - Add mailbox_id column to table output rows - Update tip text to include --mailbox flag with the resolved mailbox - Add unit tests for message builder field preservation sprint: S1

CLAassistant · 2026-06-02T13:01:07Z

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.

harness-agent 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 · 2026-06-02T13:01:13Z

📝 Walkthrough

Walkthrough

The mail +triage command now surfaces mailbox_id in all output formats. JSON/data outputs include mailbox_id at the top level and per-message; table output adds a mailbox_id column; and the help tip updates to show users how to use the resolved mailbox ID with mail +message --mailbox <mailbox>. Tests validate metadata field preservation.

Changes

Mailbox ID in Triage Output

Layer / File(s)	Summary
Add mailbox_id to triage output formats `shortcuts/mail/mail_triage.go`	Triage output now includes `mailbox_id` at the top level in json/data formats and in each message row in table format. The help tip is updated to instruct users to pass `--mailbox` with the resolved mailbox value when calling `mail +message`.
Test metadata field preservation `shortcuts/mail/mail_triage_test.go`	New tests verify that `buildTriageMessageMeta` and `buildTriageMessagesFromSearchItems` correctly preserve and extract message metadata fields (`message_id`, `subject`, `thread_id`, `from`, `create_time`) from input data.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

domain/mail, size/M

Suggested reviewers

chanthuang

Poem

📨 A rabbit hops through triage with glee,
Now mailbox IDs shown for all to see!
Each message labeled, each row complete,
No more confusion—metadata's neat! 🐰✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is incomplete. It lacks the required Summary, Changes, Test Plan, and Related Issues sections from the template, instead providing auto-generated metadata.	Restructure the description to follow the template: add a Summary section (1-3 sentences), list main Changes with bullet points, describe the Test Plan with checkboxes, and include a Related Issues section.
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.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 clearly and specifically describes the main change: adding mailbox_id to mail triage output to support public mailboxes.
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

🧹 Nitpick comments (2)

shortcuts/mail/mail_triage.go (1)
314-314: 💤 Low value

Consider shell-quoting the mailbox value for consistency.

The hint on lines 304-312 uses shellQuote() for query, filter, and page-token values. For consistency and safety, apply the same pattern here:
fmt.Fprintf(runtime.IO().ErrOut, "tip: use mail +message --mailbox %s --message-id <id> to read full content\n", shellQuote(mailbox))
While mailbox is typically "me" or an email address (unlikely to contain shell metacharacters), quoting ensures the hint remains well-formed if the value ever includes spaces or special characters.
🤖 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_triage.go` at line 314, Update the fmt.Fprintf call that
prints the usage hint so the mailbox value is shell-quoted for consistency and
safety: replace the current fmt.Fprintf(..., mailbox) usage with
fmt.Fprintf(..., shellQuote(mailbox)) so the printed tip uses
shellQuote(mailbox); this touches the fmt.Fprintf call writing to
runtime.IO().ErrOut and uses the existing shellQuote function to ensure the
mailbox is safely quoted.
shortcuts/mail/mail_triage_test.go (1)
1408-1460: ⚖️ Poor tradeoff

Consider adding test coverage for mailbox_id in output formats.

The new tests correctly verify that buildTriageMessageMeta and buildTriageMessagesFromSearchItems preserve metadata fields. However, they don't test the actual feature introduced in this PR: that mailbox_id appears in the final output.

Consider adding tests that verify:

mailbox_id appears at the top level and in each message for json/data output (lines 270-274 in mail_triage.go)

mailbox_id appears in each row for table output (line 293 in mail_triage.go)

You could follow the pattern of existing dry-run tests or add a focused Execute test with a mock HTTP response.
🤖 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_triage_test.go` around lines 1408 - 1460, Add focused
tests that assert mailbox_id is included in outputs: extend or add tests that
call buildTriageMessageMeta and buildTriageMessagesFromSearchItems and verify
the returned map includes "mailbox_id" at the top level and inside each message;
additionally add an Execute-style test (or adapt an existing dry-run test) for
the table/json/data output paths to ensure "mailbox_id" is present in each
row/message for table output and at top-level for json/data output (exercise the
code path that formats outputs in mail_triage.go, referencing the formatting
logic used by the Execute function and the output branches that consume the
results of buildTriageMessageMeta/buildTriageMessagesFromSearchItems).

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

Nitpick comments:
In `@shortcuts/mail/mail_triage_test.go`:
- Around line 1408-1460: Add focused tests that assert mailbox_id is included in
outputs: extend or add tests that call buildTriageMessageMeta and
buildTriageMessagesFromSearchItems and verify the returned map includes
"mailbox_id" at the top level and inside each message; additionally add an
Execute-style test (or adapt an existing dry-run test) for the table/json/data
output paths to ensure "mailbox_id" is present in each row/message for table
output and at top-level for json/data output (exercise the code path that
formats outputs in mail_triage.go, referencing the formatting logic used by the
Execute function and the output branches that consume the results of
buildTriageMessageMeta/buildTriageMessagesFromSearchItems).

In `@shortcuts/mail/mail_triage.go`:
- Line 314: Update the fmt.Fprintf call that prints the usage hint so the
mailbox value is shell-quoted for consistency and safety: replace the current
fmt.Fprintf(..., mailbox) usage with fmt.Fprintf(..., shellQuote(mailbox)) so
the printed tip uses shellQuote(mailbox); this touches the fmt.Fprintf call
writing to runtime.IO().ErrOut and uses the existing shellQuote function to
ensure the mailbox is safely quoted.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6724f3d9-3cf1-49cd-8bbb-1ef4a55b37bd

📥 Commits

Reviewing files that changed from the base of the PR and between 6d7f8ba and 79be1c4.

📒 Files selected for processing (2)

shortcuts/mail/mail_triage.go
shortcuts/mail/mail_triage_test.go

github-actions · 2026-06-02T13:06:03Z

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add xukuncx/cli#feat/c5b5818 -y -g

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

coderabbitai Bot reviewed Jun 2, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include mailbox_id in mail +triage output for public mailbox support#1227

Include mailbox_id in mail +triage output for public mailbox support#1227
xukuncx wants to merge 1 commit into
larksuite:mainfrom
xukuncx:feat/c5b5818

xukuncx commented Jun 2, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

CLAassistant commented Jun 2, 2026

Uh oh!

coderabbitai Bot commented Jun 2, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (2 warnings)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

github-actions Bot commented Jun 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

xukuncx commented Jun 2, 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!

CLAassistant commented Jun 2, 2026

Uh oh!

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

Uh oh!

Walkthrough

Changes

Estimated code review effort

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!

github-actions Bot commented Jun 2, 2026

🚀 PR Preview Install Guide

🧰 CLI update

🧩 Skill update

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

xukuncx commented Jun 2, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 2, 2026 •

edited

Loading