feat: support vc agent active meetings#1469
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new ChangesVCMeetingListActive feature and bot auth expansion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@c86547dd13341bb76e34d09ba42e37744f85fb5c🧩 Skill updatenpx skills add larksuite/cli#hzc/vc-agent-uat-tat-active-meeting -y -g |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1469 +/- ##
==========================================
+ Coverage 73.35% 73.41% +0.05%
==========================================
Files 750 751 +1
Lines 69264 69420 +156
==========================================
+ Hits 50811 50962 +151
+ Misses 14713 14709 -4
- Partials 3740 3749 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
skills/lark-vc-agent/SKILL.md (1)
73-73: 💤 Low valueMinor grammar refinement at line 73.
The phrase "不要把任务完成当作离会指令" may be clearer as "不应因任务完成而执行离会" or similar restructuring. The current 把-construction feels slightly forced; consider rewording to directly express "do not treat task completion as a signal to call leave."
🤖 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-vc-agent/SKILL.md` at line 73, The phrase "不要把任务完成当作离会指令" at line 73 of SKILL.md uses a somewhat awkward 把-construction that could be clearer. Restructure this phrase to more directly express the intent "do not execute the meeting-leave command simply because a task is complete" by using a more natural Chinese construction such as "不应因任务完成而执行离会" or similar rephrasing that flows better and more clearly conveys that task completion alone should not trigger the leave instruction. Ensure the reworded phrase maintains the exact same meaning and guidance for the agent's behavior.
🤖 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/vc/vc_meeting_list_active.go`:
- Around line 89-99: The `validateMeetingListActiveUserID` function validates
the `--user-id` parameter at lines 97-99 regardless of whether running as a user
or bot, but it should be completely ignored when running as user identity.
Modify the function so that the validation call to `common.ValidateUserIDTyped`
at lines 97-99 only executes when `runtime.IsBot()` returns true. When running
as a user, return nil early to skip validation entirely, allowing invalid
`--user-id` values to be silently ignored as intended.
In `@shortcuts/vc/vc_meeting_test.go`:
- Around line 623-645: In the TestMeetingListActive_Validate_BotRequiresUserID
and TestMeetingListActive_Validate_UserIDOpenIDFormat test functions, replace
the substring-based error assertions (the strings.Contains checks on
err.Error()) with typed metadata assertions. Use errs.ProblemOf(err) to assert
the category and subtype of the error, and use errors.As(err, &ve) where ve is a
*errs.ValidationError to access and assert the Param field. This ensures the
tests validate the structured error information rather than relying on message
text that could change.
---
Nitpick comments:
In `@skills/lark-vc-agent/SKILL.md`:
- Line 73: The phrase "不要把任务完成当作离会指令" at line 73 of SKILL.md uses a somewhat
awkward 把-construction that could be clearer. Restructure this phrase to more
directly express the intent "do not execute the meeting-leave command simply
because a task is complete" by using a more natural Chinese construction such as
"不应因任务完成而执行离会" or similar rephrasing that flows better and more clearly conveys
that task completion alone should not trigger the leave instruction. Ensure the
reworded phrase maintains the exact same meaning and guidance for the agent's
behavior.
🪄 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: 9a6d686b-7178-4c2f-86ec-0bbc05a7ab70
📒 Files selected for processing (12)
shortcuts/vc/shortcuts.goshortcuts/vc/vc_meeting_events.goshortcuts/vc/vc_meeting_events_test.goshortcuts/vc/vc_meeting_join.goshortcuts/vc/vc_meeting_leave.goshortcuts/vc/vc_meeting_list_active.goshortcuts/vc/vc_meeting_test.goskills/lark-vc-agent/SKILL.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-events.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-join.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-leave.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-list-active.md
c53c5b5 to
b031120
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
shortcuts/vc/vc_meeting_list_active.go (1)
89-101:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
--user-idvalidation still executes under user identity.Lines 97-99 validate the format of
--user-idregardless of whether running as user or bot. When--as user, this flag should be completely ignored (no validation). The current code allows invalid--user-idvalues to fail validation even when the flag has no effect.The recommended fix is to check
runtime.IsBot()first and return nil early for user identity, so validation only runs for bot identity.🤖 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/vc/vc_meeting_list_active.go` around lines 89 - 101, The validateMeetingListActiveUserID function validates the user-id flag even when running as a regular user, where the flag should be completely ignored. Restructure the function to check runtime.IsBot() first and return nil immediately for user identity before any validation occurs. This ensures the format validation of the user-id (performed by the common.ValidateUserIDTyped call) only executes when running as a bot, preventing invalid user-id values from failing validation when the flag has no effect.
🤖 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.
Duplicate comments:
In `@shortcuts/vc/vc_meeting_list_active.go`:
- Around line 89-101: The validateMeetingListActiveUserID function validates the
user-id flag even when running as a regular user, where the flag should be
completely ignored. Restructure the function to check runtime.IsBot() first and
return nil immediately for user identity before any validation occurs. This
ensures the format validation of the user-id (performed by the
common.ValidateUserIDTyped call) only executes when running as a bot, preventing
invalid user-id values from failing validation when the flag has no effect.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d183482c-c745-42ed-a25f-8931902557cd
📒 Files selected for processing (12)
shortcuts/vc/shortcuts.goshortcuts/vc/vc_meeting_events.goshortcuts/vc/vc_meeting_events_test.goshortcuts/vc/vc_meeting_join.goshortcuts/vc/vc_meeting_leave.goshortcuts/vc/vc_meeting_list_active.goshortcuts/vc/vc_meeting_test.goskills/lark-vc-agent/SKILL.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-events.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-join.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-leave.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-list-active.md
✅ Files skipped from review due to trivial changes (1)
- skills/lark-vc-agent/references/lark-vc-agent-meeting-leave.md
🚧 Files skipped from review as they are similar to previous changes (7)
- shortcuts/vc/vc_meeting_leave.go
- shortcuts/vc/vc_meeting_events_test.go
- skills/lark-vc-agent/references/lark-vc-agent-meeting-join.md
- skills/lark-vc-agent/references/lark-vc-agent-meeting-list-active.md
- shortcuts/vc/vc_meeting_join.go
- shortcuts/vc/vc_meeting_events.go
- shortcuts/vc/shortcuts.go
b031120 to
be97a6e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shortcuts/vc/vc_meeting_test.go (1)
816-835: ⚡ Quick winPreserve the underlying API failure in this error-path test.
This only locks in
Category/Subtype. IfExecuteis expected to wrap the API failure, add anerrors.Is/errors.Unwrapassertion so cause preservation is covered too.🤖 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/vc/vc_meeting_test.go` around lines 816 - 835, The test TestMeetingListActive_Execute_APIError validates the Category and Subtype of the returned error but does not verify that the underlying API failure is preserved through error wrapping. Add an errors.Is or errors.Unwrap assertion after the existing subtype check to ensure the error chain contains the underlying API error, confirming that Execute properly wraps and preserves the original API failure rather than losing it in the error handling chain.Source: Coding guidelines
🤖 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-vc-agent/SKILL.md`:
- Around line 73-80: In the SKILL.md file section 4 about
`+meeting-list-active`, correct the field name reference to use the flat
`meeting_id` field instead of the nested `meeting.id` notation. The
`+meeting-list-active` command returns a flat structure with a `meeting_id`
field, not a nested object, so update any references in the documentation to
accurately reflect this flat field name to prevent users from attempting to
access a non-existent nested field.
---
Nitpick comments:
In `@shortcuts/vc/vc_meeting_test.go`:
- Around line 816-835: The test TestMeetingListActive_Execute_APIError validates
the Category and Subtype of the returned error but does not verify that the
underlying API failure is preserved through error wrapping. Add an errors.Is or
errors.Unwrap assertion after the existing subtype check to ensure the error
chain contains the underlying API error, confirming that Execute properly wraps
and preserves the original API failure rather than losing it in the error
handling chain.
🪄 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: d3a2f9c9-23ff-4dd1-a937-d31088edee4e
📒 Files selected for processing (12)
shortcuts/vc/shortcuts.goshortcuts/vc/vc_meeting_events.goshortcuts/vc/vc_meeting_events_test.goshortcuts/vc/vc_meeting_join.goshortcuts/vc/vc_meeting_leave.goshortcuts/vc/vc_meeting_list_active.goshortcuts/vc/vc_meeting_test.goskills/lark-vc-agent/SKILL.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-events.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-join.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-leave.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-list-active.md
✅ Files skipped from review due to trivial changes (4)
- skills/lark-vc-agent/references/lark-vc-agent-meeting-list-active.md
- shortcuts/vc/vc_meeting_join.go
- shortcuts/vc/vc_meeting_events.go
- skills/lark-vc-agent/references/lark-vc-agent-meeting-leave.md
🚧 Files skipped from review as they are similar to previous changes (5)
- shortcuts/vc/vc_meeting_leave.go
- shortcuts/vc/shortcuts.go
- shortcuts/vc/vc_meeting_list_active.go
- skills/lark-vc-agent/references/lark-vc-agent-meeting-join.md
- shortcuts/vc/vc_meeting_events_test.go
There was a problem hiding this comment.
♻️ Duplicate comments (1)
skills/lark-vc-agent/SKILL.md (1)
75-75:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
meeting_id(notmeeting.id) for+meeting-list-activeoutput.Line 75 currently implies
+meeting-list-activeyieldsmeeting.id, but that shortcut returns flatmeeting_id; this can mislead downstream+meeting-leaveusage.Suggested doc fix
-1. 只有用户明确要求机器人退出 / 离开 / 结束参会时,才用 `+meeting-leave --meeting-id <从 +meeting-join 或 +meeting-list-active 拿到的 meeting.id>`;不应因任务完成而执行离会。 +1. 只有用户明确要求机器人退出 / 离开 / 结束参会时,才用 `+meeting-leave --meeting-id <从 +meeting-join 的 meeting.id 或 +meeting-list-active 的 meeting_id 获取>`;不应因任务完成而执行离会。🤖 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-vc-agent/SKILL.md` at line 75, The documentation at line 75 in SKILL.md incorrectly references the output field from `+meeting-list-active` as `meeting.id` when it should be `meeting_id`. Update the text to replace `meeting.id` with `meeting_id` to accurately reflect that `+meeting-list-active` returns a flat structure with `meeting_id` rather than a nested object with `meeting.id`. This ensures users understand the correct field name to use when passing the meeting ID to `+meeting-leave`.
🤖 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.
Duplicate comments:
In `@skills/lark-vc-agent/SKILL.md`:
- Line 75: The documentation at line 75 in SKILL.md incorrectly references the
output field from `+meeting-list-active` as `meeting.id` when it should be
`meeting_id`. Update the text to replace `meeting.id` with `meeting_id` to
accurately reflect that `+meeting-list-active` returns a flat structure with
`meeting_id` rather than a nested object with `meeting.id`. This ensures users
understand the correct field name to use when passing the meeting ID to
`+meeting-leave`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7166eee-dadd-4221-8584-cb66fb7bc88f
📒 Files selected for processing (5)
shortcuts/vc/vc_meeting_list_active.goshortcuts/vc/vc_meeting_test.goskills/lark-vc-agent/SKILL.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-events.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-list-active.md
✅ Files skipped from review due to trivial changes (1)
- skills/lark-vc-agent/references/lark-vc-agent-meeting-events.md
🚧 Files skipped from review as they are similar to previous changes (2)
- skills/lark-vc-agent/references/lark-vc-agent-meeting-list-active.md
- shortcuts/vc/vc_meeting_test.go
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/lark-vc-agent/SKILL.md (1)
75-75:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDisambiguate field names for
+meeting-joinvs+meeting-list-activein section 3.Line 75 refers to "meeting.id" for both
+meeting-joinand+meeting-list-active, but they return different field names:
+meeting-joinreturns nestedmeeting.id(as documented in section 1, line 48)+meeting-list-activereturns flatmeeting_id(as documented in section 4, line 82)This mirrors the prior review feedback that flagged the
meeting.idvsmeeting_iddistinction to prevent users from copying a non-existent nested field.📝 Proposed fix
-1. 只有用户明确要求机器人退出 / 离开 / 结束参会时,才用 `+meeting-leave --meeting-id <从 +meeting-join 或 +meeting-list-active 拿到的 meeting.id>`;不应因任务完成而执行离会。 +1. 只有用户明确要求机器人退出 / 离开 / 结束参会时,才用 `+meeting-leave --meeting-id <从 +meeting-join 返回的 meeting.id 或从 +meeting-list-active 返回的 meeting_id>`;不应因任务完成而执行离会。🤖 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-vc-agent/SKILL.md` at line 75, In section 3, line 75, disambiguate the field name reference in the `+meeting-leave --meeting-id` instruction to account for different return structures from the two commands. Clarify that when the meeting ID comes from `+meeting-join` (documented in section 1, line 48), use the nested `meeting.id` field, and when it comes from `+meeting-list-active` (documented in section 4, line 82), use the flat `meeting_id` field. This prevents users from attempting to reference non-existent field names depending on which command they used to retrieve the meeting identifier.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.
Outside diff comments:
In `@skills/lark-vc-agent/SKILL.md`:
- Line 75: In section 3, line 75, disambiguate the field name reference in the
`+meeting-leave --meeting-id` instruction to account for different return
structures from the two commands. Clarify that when the meeting ID comes from
`+meeting-join` (documented in section 1, line 48), use the nested `meeting.id`
field, and when it comes from `+meeting-list-active` (documented in section 4,
line 82), use the flat `meeting_id` field. This prevents users from attempting
to reference non-existent field names depending on which command they used to
retrieve the meeting identifier.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 79763808-19bb-4a86-91f0-7f36659d8c34
📒 Files selected for processing (3)
shortcuts/vc/vc_meeting_list_active.goshortcuts/vc/vc_meeting_test.goskills/lark-vc-agent/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (2)
- shortcuts/vc/vc_meeting_list_active.go
- shortcuts/vc/vc_meeting_test.go
Summary
Adds VC agent shortcut support for UAT/TAT meeting flows, including active meeting discovery for the current or target user.
Changes
vc +meeting-list-activefor/open-apis/vc/v1/bots/user_active_meeting+meeting-join,+meeting-leave, and+meeting-eventsto run with bot identityou_...open_id for TAT+meeting-list-active --user-idlark-vc-agentskill docs for active meeting discovery, event reading, join/leave flow, pagination, and UAT/TAT identity boundariesTest Plan
git diff --checkgo test ./shortcuts/vcgo build ./...Related Issues
Summary by CodeRabbit
New Features
+meeting-list-activeto list active VC meetings, including enhanced pretty output and guidance when multiple meetings are found.+meeting-list-activewith required--user-idfor authorization and accurate scoping.Documentation
--as user) vs TAT (--as bot) identity boundaries, includingmeeting_idsourcing.Bug Fixes
+meeting-events,+meeting-join, and+meeting-leaveto include both user and bot.Tests