Skip to content

feat(lark-im): support UAT for forward and add threads.forward#689

Open
chenxingtong-bytedance wants to merge 1 commit intolarksuite:mainfrom
chenxingtong-bytedance:feat/support_uat_forward_msg
Open

feat(lark-im): support UAT for forward and add threads.forward#689
chenxingtong-bytedance wants to merge 1 commit intolarksuite:mainfrom
chenxingtong-bytedance:feat/support_uat_forward_msg

Conversation

@chenxingtong-bytedance
Copy link
Copy Markdown
Contributor

@chenxingtong-bytedance chenxingtong-bytedance commented Apr 28, 2026

Support IM forward APIs in UAT/user identity scenarios by updating messages.forward identity metadata and adding threads.forward API resource
documentation. Also add E2E coverage for message and thread forwarding workflows.

Changes

  • Update lark-im skill docs so messages.forward supports both user and bot identities.
  • Add threads.forward to IM API resources.
  • Add IM forward scope mapping for im:message / im:message.send_as_user.
  • Add TestIM_MessageForwardWorkflowAsUser E2E coverage for im messages forward and im threads forward.
  • Update IM E2E coverage docs from 9/29 to 11/30 covered leaf commands.

Test Plan

  • make unit-test
  • go test ./tests/cli_e2e/im -run TestIM_MessageForwardWorkflowAsUser -count=1
  • Manually verify lark im messages forward --as user
  • Manually verify lark im threads forward --as user

Related Issues

N/A

Summary by CodeRabbit

  • Documentation

    • Updated API documentation for message and thread forwarding to reflect support for both user and bot identities.
  • Chores

    • Updated default infrastructure endpoints for non-Lark brands.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

📝 Walkthrough

Walkthrough

Documentation and E2E tests were updated to reflect that IM message and thread forwarding support both user and bot identities; added a user-mode CLI E2E test and minor endpoint default host changes.

Changes

Cohort / File(s) Summary
IM API Docs
skills/lark-im/SKILL.md
Updated im.messages.forward caller identities from bot-only to user and bot; added im.threads.forward doc indicating user and bot support.
CLI E2E Tests & Coverage
tests/cli_e2e/im/coverage.md, tests/cli_e2e/im/message_forward_workflow_test.go
Added TestIM_MessageForwardWorkflowAsUser test and helpers; updated coverage docs to mark im messages forward and im threads forward covered and document behavior and conditional skipping when permissions absent.
CLI Defaults / Endpoints
cmd/root.go, internal/core/types.go
Changed non-Lark default admin-console base domain and default Open/Accounts/MCP endpoints to use pre-domain (*.feishu-pre.cn) instead of previous domain (*.feishu.cn) for non-BrandLark brands.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Suggested reviewers

  • YangJunzhou-01
  • sammi-bytedance

Poem

🐇 I hopped a change from bot to user,

forwarding threads with tiny fur,
Tests that bounce and docs that sing,
Permissions checked before they spring,
A cheerful hop — a rabbit's little cheer! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% 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 directly addresses the main changes: adding UAT/user identity support for forward APIs and introducing threads.forward functionality, which aligns with the core objectives.
Description check ✅ Passed The description covers all required template sections with concrete details about changes, test plan items, and addresses the PR objectives comprehensively.
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/im PR touches the im domain size/M Single-domain feat or fix with limited business impact labels Apr 28, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
tests/cli_e2e/im/coverage.md (1)

13-16: Minor readability nit: vary repeated “proves …” sentence starts.

The summary bullets read a bit repetitive; a small rephrase would improve scanability.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/cli_e2e/im/coverage.md` around lines 13 - 16, The three bullet lines
repeating "proves" should be reworded for readability: update
TestIM_MessageGetWorkflowAsUser to start with "Validates user message
readback..." (reference: `TestIM_MessageGetWorkflowAsUser` and "batch get
message as user"), change TestIM_MessageReplyWorkflowAsBot to "Exercises
threaded reply flow..." (reference: `TestIM_MessageReplyWorkflowAsBot`, `reply
to message in thread as bot`, `im +threads-messages-list`), and change
TestIM_MessageForwardWorkflowAsUser to "Demonstrates UAT-backed API
forwarding..." (reference: `TestIM_MessageForwardWorkflowAsUser`, `im messages
forward`, `im threads forward`); leave the blocked area sentence as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@skills/lark-im/SKILL.md`:
- Around line 108-110: The permission table is missing a row for threads.forward
and its scope mapping is inconsistent with the API doc: add a permission entry
for "threads.forward" in the table and include the exact scope(s) used (e.g.,
im:message.send_as_user) in the same row's scopes column or notes; if mappings
differ by identity (user vs bot), state those differences in the row notes (for
example: "user: im:message.send_as_user; bot: im:message.send") so the table
aligns with the API doc and the PR's forward mapping.

In `@tests/cli_e2e/im/message_forward_workflow_test.go`:
- Around line 17-94: TestIM_MessageForwardWorkflowAsUser leaves created messages
behind; capture the created IDs (the initial messageID from
sendDirectMessageToUser, the threadID from findThreadIDForMessage, and the
forwardedIDs parsed from result.Stdout in both forward steps) and register
cleanup via t.Cleanup to delete them after the test completes. Specifically,
after obtaining messageID, threadID and each forwardedID (variables messageID,
threadID, forwardedID), call the same CLI/API delete operations used elsewhere
(e.g., the "im messages delete" / thread delete endpoint via clie2e.RunCmd)
inside t.Cleanup so removal always runs even on failures; ensure you handle
empty IDs and ignore not-found errors.
- Around line 21-29: This test performs bot-based setup (calls
sendDirectMessageToUser(..., "bot") and later uses the im messages-reply flow)
but only calls clie2e.SkipWithoutUserToken(t); update the test to guard
bot-specific steps by calling clie2e.SkipWithoutBotToken(t) (or a combined
bot-or-user skip helper) before invoking sendDirectMessageToUser and before the
im +messages-reply section so the test will be skipped cleanly in user-only
environments; apply the same change to the second bot-setup block referenced
(the section around sendDirectMessageToUser/im +messages-reply at lines 55-63).

---

Nitpick comments:
In `@tests/cli_e2e/im/coverage.md`:
- Around line 13-16: The three bullet lines repeating "proves" should be
reworded for readability: update TestIM_MessageGetWorkflowAsUser to start with
"Validates user message readback..." (reference:
`TestIM_MessageGetWorkflowAsUser` and "batch get message as user"), change
TestIM_MessageReplyWorkflowAsBot to "Exercises threaded reply flow..."
(reference: `TestIM_MessageReplyWorkflowAsBot`, `reply to message in thread as
bot`, `im +threads-messages-list`), and change
TestIM_MessageForwardWorkflowAsUser to "Demonstrates UAT-backed API
forwarding..." (reference: `TestIM_MessageForwardWorkflowAsUser`, `im messages
forward`, `im threads forward`); leave the blocked area sentence as-is.
🪄 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: 659263d7-3593-45fc-b1ee-6b64823fdc68

📥 Commits

Reviewing files that changed from the base of the PR and between c09b03f and 43005e5.

📒 Files selected for processing (3)
  • skills/lark-im/SKILL.md
  • tests/cli_e2e/im/coverage.md
  • tests/cli_e2e/im/message_forward_workflow_test.go

Comment thread skills/lark-im/SKILL.md
Comment on lines +108 to +110
### threads

- `forward` — 转发话题。Identity: supports `user` and `bot`.
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 | 🟠 Major

Complete forward scope mapping in the permission table.

threads.forward is documented under API resources, but there is no corresponding permission row. Also, forward mapping in this PR includes im:message.send_as_user, which is not reflected in the table. Please add/align these entries so operators configure scopes correctly.

Suggested doc update shape
 | `messages.forward` | `im:message` |
+| `threads.forward` | `im:message` |

If your actual auth mapping differs by identity, document that explicitly (e.g., user path with im:message.send_as_user) in the same table row notes.

Also applies to: 124-147

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/lark-im/SKILL.md` around lines 108 - 110, The permission table is
missing a row for threads.forward and its scope mapping is inconsistent with the
API doc: add a permission entry for "threads.forward" in the table and include
the exact scope(s) used (e.g., im:message.send_as_user) in the same row's scopes
column or notes; if mappings differ by identity (user vs bot), state those
differences in the row notes (for example: "user: im:message.send_as_user; bot:
im:message.send") so the table aligns with the API doc and the PR's forward
mapping.

Comment on lines +17 to +94
func TestIM_MessageForwardWorkflowAsUser(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
t.Cleanup(cancel)

clie2e.SkipWithoutUserToken(t)

suffix := clie2e.GenerateSuffix()
messageText := "im-forward-msg-" + suffix
replyText := "im-forward-reply-" + suffix

selfOpenID := getSelfOpenID(t, ctx)
chatID, messageID := sendDirectMessageToUser(t, ctx, selfOpenID, messageText, "bot")

t.Run("forward message with api command as user", func(t *testing.T) {
result, err := clie2e.RunCmd(ctx, clie2e.Request{
Args: []string{"im", "messages", "forward"},
DefaultAs: "user",
Params: map[string]any{
"message_id": messageID,
"receive_id_type": "chat_id",
"uuid": "msg-forward-" + suffix,
},
Data: map[string]any{
"receive_id": chatID,
},
})
require.NoError(t, err)
skipIfMissingIMForwardPermission(t, result)
result.AssertExitCode(t, 0)
result.AssertStdoutStatus(t, 0)

forwardedID := gjson.Get(result.Stdout, "data.message_id").String()
require.NotEmpty(t, forwardedID, "stdout:\n%s", result.Stdout)
require.NotEqual(t, messageID, forwardedID, "stdout:\n%s", result.Stdout)
require.Equal(t, chatID, gjson.Get(result.Stdout, "data.chat_id").String(), "stdout:\n%s", result.Stdout)
})

var threadID string
t.Run("create thread fixture as bot", func(t *testing.T) {
result, err := clie2e.RunCmd(ctx, clie2e.Request{
Args: []string{"im", "+messages-reply",
"--message-id", messageID,
"--text", replyText,
"--reply-in-thread",
},
DefaultAs: "bot",
})
require.NoError(t, err)
result.AssertExitCode(t, 0)
result.AssertStdoutStatus(t, true)

threadID = findThreadIDForMessage(t, ctx, chatID, messageID, "bot")
})

t.Run("forward thread with api command as user", func(t *testing.T) {
result, err := clie2e.RunCmd(ctx, clie2e.Request{
Args: []string{"im", "threads", "forward"},
DefaultAs: "user",
Params: map[string]any{
"thread_id": threadID,
"receive_id_type": "chat_id",
"uuid": "thread-forward-" + suffix,
},
Data: map[string]any{
"receive_id": chatID,
},
})
require.NoError(t, err)
skipIfMissingIMForwardPermission(t, result)
result.AssertExitCode(t, 0)
result.AssertStdoutStatus(t, 0)

forwardedID := gjson.Get(result.Stdout, "data.message_id").String()
require.NotEmpty(t, forwardedID, "stdout:\n%s", result.Stdout)
require.Equal(t, chatID, gjson.Get(result.Stdout, "data.chat_id").String(), "stdout:\n%s", result.Stdout)
require.Equal(t, "merge_forward", gjson.Get(result.Stdout, "data.msg_type").String(), "stdout:\n%s", result.Stdout)
})
}
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 | 🟠 Major

Add fixture cleanup to keep this live E2E test self-contained.

The workflow creates source/reply/forwarded messages but does not include cleanup, so test artifacts are left behind in the target chat.

As per coding guidelines: Live E2E tests must be self-contained with create/use/cleanup flows and placed in tests/cli_e2e/<domain>/.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/cli_e2e/im/message_forward_workflow_test.go` around lines 17 - 94,
TestIM_MessageForwardWorkflowAsUser leaves created messages behind; capture the
created IDs (the initial messageID from sendDirectMessageToUser, the threadID
from findThreadIDForMessage, and the forwardedIDs parsed from result.Stdout in
both forward steps) and register cleanup via t.Cleanup to delete them after the
test completes. Specifically, after obtaining messageID, threadID and each
forwardedID (variables messageID, threadID, forwardedID), call the same CLI/API
delete operations used elsewhere (e.g., the "im messages delete" / thread delete
endpoint via clie2e.RunCmd) inside t.Cleanup so removal always runs even on
failures; ensure you handle empty IDs and ignore not-found errors.

Comment on lines +21 to +29
clie2e.SkipWithoutUserToken(t)

suffix := clie2e.GenerateSuffix()
messageText := "im-forward-msg-" + suffix
replyText := "im-forward-reply-" + suffix

selfOpenID := getSelfOpenID(t, ctx)
chatID, messageID := sendDirectMessageToUser(t, ctx, selfOpenID, messageText, "bot")

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 | 🟠 Major

Add a bot-token precheck for bot-based fixture steps.

This test uses bot identity for setup (sendDirectMessageToUser(..., "bot") and im +messages-reply) but only calls clie2e.SkipWithoutUserToken(t). In user-only envs this will fail instead of skipping cleanly.

Suggested patch
 func TestIM_MessageForwardWorkflowAsUser(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
 	t.Cleanup(cancel)

 	clie2e.SkipWithoutUserToken(t)
+	clie2e.SkipWithoutBotToken(t)

Also applies to: 55-63

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/cli_e2e/im/message_forward_workflow_test.go` around lines 21 - 29, This
test performs bot-based setup (calls sendDirectMessageToUser(..., "bot") and
later uses the im messages-reply flow) but only calls
clie2e.SkipWithoutUserToken(t); update the test to guard bot-specific steps by
calling clie2e.SkipWithoutBotToken(t) (or a combined bot-or-user skip helper)
before invoking sendDirectMessageToUser and before the im +messages-reply
section so the test will be skipped cleanly in user-only environments; apply the
same change to the second bot-setup block referenced (the section around
sendDirectMessageToUser/im +messages-reply at lines 55-63).

  - Update messages.forward identity to support `user` and `bot`
  - Add threads.forward entry under threads API resources
  - Add forward APIs -> `im:message`, `im:message.send_as_user` scope mapping

Change-Id: I2e33b0d78d72fd067ba3916095479f9b336e7eb9
@chenxingtong-bytedance chenxingtong-bytedance force-pushed the feat/support_uat_forward_msg branch from 43005e5 to 9904add Compare April 28, 2026 12:01
@github-actions github-actions Bot added size/L Large or sensitive change across domains or core paths and removed size/M Single-domain feat or fix with limited business impact labels Apr 28, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

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)
cmd/root.go (1)

312-317: ⚠️ Potential issue | 🟠 Major

Add tests for the brand-based host selection in enrichPermissionError.

The code at lines 312–316 switches the permission-console host based on brand (open.feishu-pre.cn for non-lark, open.larksuite.com for lark), changing user-facing guidance in error output. Current tests in cmd/root_test.go validate URL escaping but do not explicitly test this host selection logic. Per the coding guideline: Every behavior change must have an accompanying test.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/root.go` around lines 312 - 317, Add unit tests for enrichPermissionError
that assert the permission-console host selection based on cfg.Brand: create two
test cases (cfg.Brand == "lark" and a non-lark brand) and verify the returned
error message or consoleURL contains "open.larksuite.com" for lark and
"open.feishu-pre.cn" for non-lark, and still preserves URL-escaped AppID/scopes;
locate enrichPermissionError usage and the host/consoleURL construction in
cmd/root.go and add assertions in cmd/root_test.go that call
enrichPermissionError (or the public wrapper that emits the message) and check
the resulting string includes the expected host and escaped parameters.
♻️ Duplicate comments (3)
tests/cli_e2e/im/message_forward_workflow_test.go (2)

21-29: ⚠️ Potential issue | 🟠 Major

Guard bot-based fixture steps with a bot-token precheck.

The workflow uses bot identity in setup (sendDirectMessageToUser(..., "bot") and +messages-reply) but only checks for user token. Add clie2e.SkipWithoutBotToken(t) before bot-backed steps so environments without bot token skip cleanly.

Also applies to: 55-63

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/cli_e2e/im/message_forward_workflow_test.go` around lines 21 - 29, The
test uses bot-backed setup steps but only calls clie2e.SkipWithoutUserToken(t);
add a bot-token precheck by calling clie2e.SkipWithoutBotToken(t) immediately
before any bot-identity operations (e.g., before sendDirectMessageToUser(t, ctx,
selfOpenID, messageText, "bot") and before the block that posts message replies
via messages-reply); update both occurrences (the initial setup and the later
reply-related block) so environments without a bot token are skipped cleanly.

17-94: ⚠️ Potential issue | 🟠 Major

Add cleanup for created IM artifacts to keep the live E2E self-contained.

This flow creates source/reply/forwarded messages but does not register cleanup. Please collect created IDs and delete them in t.Cleanup (best-effort, ignore not-found).

As per coding guidelines: Live E2E tests must be self-contained with create/use/cleanup flows and placed in tests/cli_e2e/<domain>/.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/cli_e2e/im/message_forward_workflow_test.go` around lines 17 - 94, The
test TestIM_MessageForwardWorkflowAsUser creates messages and a thread
(messageID, forwardedID, threadID) but never deletes them; add a t.Cleanup block
that best-effort deletes the created artifacts after the test (ignore not-found
errors). Capture IDs returned from sendDirectMessageToUser, the first forward
result (data.message_id), and the thread-forward result (data.message_id) and
call the same CLI helper used elsewhere (clie2e.RunCmd) or existing helper
functions to delete messages/threads (use the same params/DefaultAs that created
them) in the cleanup; also remove any duplicate resources if both original and
forwarded IDs exist. Ensure cleanup references the symbols messageID,
forwardedID, threadID, sendDirectMessageToUser, findThreadIDForMessage, and
clie2e.RunCmd so reviewers can find where to add the deletion calls.
skills/lark-im/SKILL.md (1)

97-110: ⚠️ Potential issue | 🟠 Major

Forward permission table is still incomplete/inconsistent.

messages.forward and threads.forward are documented as user + bot, but the scope table only has messages.forward | im:message and no threads.forward row. Please add both forward rows with identity-specific scopes (including im:message.send_as_user for user path).

Suggested table shape
 | `messages.forward` | `im:message` |
+| `messages.forward` | `bot: im:message; user: im:message.send_as_user` |
+| `threads.forward`  | `bot: im:message; user: im:message.send_as_user` |

Also applies to: 124-147

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/lark-im/SKILL.md` around lines 97 - 110, Add missing scope table rows
for messages.forward and threads.forward and make them identity-specific: for
messages.forward include both im:message (bot) and im:message.send_as_user
(user); for threads.forward include both im:thread (bot) and
im:thread.send_as_user (user). Update the permissions table entries that
reference messages.forward and threads.forward so they list the appropriate
scopes per identity and ensure the narrative above (the bullets under messages
and threads) matches these new table rows.
🧹 Nitpick comments (1)
cmd/root.go (1)

312-315: Avoid duplicating brand→host mapping here.

This mapping now exists in both cmd/root.go and internal/core/types.go; deriving from core.ResolveOpenBaseURL(core.ParseBrand(cfg.Brand)) would reduce drift risk.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/root.go` around lines 312 - 315, The host selection in root.go duplicates
brand→host mapping; replace the inline branch that sets host based on cfg.Brand
with a call to the centralized resolver: use
core.ResolveOpenBaseURL(core.ParseBrand(cfg.Brand)) to derive the host (or base
URL) instead of hardcoding "open.feishu-pre.cn"/"open.larksuite.com"; update
references to the local host variable to use the returned value and remove the
duplicated mapping logic to avoid drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/core/types.go`:
- Around line 43-45: Update the test TestResolveEndpoints_EmptyDefaultsToFeishu
in internal/core/types_test.go to expect the new default domain "feishu-pre.cn"
instead of "feishu.cn"; specifically change assertions that compare resolved
endpoints (Open, Accounts, MCP) to expect "https://open.feishu-pre.cn",
"https://accounts.feishu-pre.cn", and "https://mcp.feishu-pre.cn" so they match
the defaults defined for Open, Accounts, and MCP in the types (the resolved
endpoints used by ResolveEndpoints/ResolveBrandDefaults logic).

---

Outside diff comments:
In `@cmd/root.go`:
- Around line 312-317: Add unit tests for enrichPermissionError that assert the
permission-console host selection based on cfg.Brand: create two test cases
(cfg.Brand == "lark" and a non-lark brand) and verify the returned error message
or consoleURL contains "open.larksuite.com" for lark and "open.feishu-pre.cn"
for non-lark, and still preserves URL-escaped AppID/scopes; locate
enrichPermissionError usage and the host/consoleURL construction in cmd/root.go
and add assertions in cmd/root_test.go that call enrichPermissionError (or the
public wrapper that emits the message) and check the resulting string includes
the expected host and escaped parameters.

---

Duplicate comments:
In `@skills/lark-im/SKILL.md`:
- Around line 97-110: Add missing scope table rows for messages.forward and
threads.forward and make them identity-specific: for messages.forward include
both im:message (bot) and im:message.send_as_user (user); for threads.forward
include both im:thread (bot) and im:thread.send_as_user (user). Update the
permissions table entries that reference messages.forward and threads.forward so
they list the appropriate scopes per identity and ensure the narrative above
(the bullets under messages and threads) matches these new table rows.

In `@tests/cli_e2e/im/message_forward_workflow_test.go`:
- Around line 21-29: The test uses bot-backed setup steps but only calls
clie2e.SkipWithoutUserToken(t); add a bot-token precheck by calling
clie2e.SkipWithoutBotToken(t) immediately before any bot-identity operations
(e.g., before sendDirectMessageToUser(t, ctx, selfOpenID, messageText, "bot")
and before the block that posts message replies via messages-reply); update both
occurrences (the initial setup and the later reply-related block) so
environments without a bot token are skipped cleanly.
- Around line 17-94: The test TestIM_MessageForwardWorkflowAsUser creates
messages and a thread (messageID, forwardedID, threadID) but never deletes them;
add a t.Cleanup block that best-effort deletes the created artifacts after the
test (ignore not-found errors). Capture IDs returned from
sendDirectMessageToUser, the first forward result (data.message_id), and the
thread-forward result (data.message_id) and call the same CLI helper used
elsewhere (clie2e.RunCmd) or existing helper functions to delete
messages/threads (use the same params/DefaultAs that created them) in the
cleanup; also remove any duplicate resources if both original and forwarded IDs
exist. Ensure cleanup references the symbols messageID, forwardedID, threadID,
sendDirectMessageToUser, findThreadIDForMessage, and clie2e.RunCmd so reviewers
can find where to add the deletion calls.

---

Nitpick comments:
In `@cmd/root.go`:
- Around line 312-315: The host selection in root.go duplicates brand→host
mapping; replace the inline branch that sets host based on cfg.Brand with a call
to the centralized resolver: use
core.ResolveOpenBaseURL(core.ParseBrand(cfg.Brand)) to derive the host (or base
URL) instead of hardcoding "open.feishu-pre.cn"/"open.larksuite.com"; update
references to the local host variable to use the returned value and remove the
duplicated mapping logic to avoid drift.
🪄 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: 0563c2c1-8af9-4b11-8815-d01634a5be2a

📥 Commits

Reviewing files that changed from the base of the PR and between 43005e5 and 9904add.

📒 Files selected for processing (5)
  • cmd/root.go
  • internal/core/types.go
  • skills/lark-im/SKILL.md
  • tests/cli_e2e/im/coverage.md
  • tests/cli_e2e/im/message_forward_workflow_test.go

Comment thread internal/core/types.go
Comment on lines +43 to +45
Open: "https://open.feishu-pre.cn",
Accounts: "https://accounts.feishu-pre.cn",
MCP: "https://mcp.feishu-pre.cn",
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 | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether endpoint-resolution behavior is covered by tests.
rg -n --type=go -C3 'Test.*ResolveEndpoints|ResolveEndpoints\(|ResolveOpenBaseURL\('

Repository: larksuite/cli

Length of output: 12646


🏁 Script executed:

sed -n '34,39p' internal/core/types_test.go

Repository: larksuite/cli

Length of output: 261


🏁 Script executed:

sed -n '32,48p' internal/core/types.go

Repository: larksuite/cli

Length of output: 527


Update endpoint tests to verify the default domain change to feishu-pre.cn.

The default endpoint for non-Lark brands changed from https://open.feishu.cn to https://open.feishu-pre.cn, but the test TestResolveEndpoints_EmptyDefaultsToFeishu in internal/core/types_test.go still expects the old .cn domain. Update the test assertions to expect .feishu-pre.cn for the default case.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/core/types.go` around lines 43 - 45, Update the test
TestResolveEndpoints_EmptyDefaultsToFeishu in internal/core/types_test.go to
expect the new default domain "feishu-pre.cn" instead of "feishu.cn";
specifically change assertions that compare resolved endpoints (Open, Accounts,
MCP) to expect "https://open.feishu-pre.cn", "https://accounts.feishu-pre.cn",
and "https://mcp.feishu-pre.cn" so they match the defaults defined for Open,
Accounts, and MCP in the types (the resolved endpoints used by
ResolveEndpoints/ResolveBrandDefaults logic).

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

Labels

domain/im PR touches the im 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.

1 participant