fix(vc): align meeting query scopes by identity#1850
Conversation
github-actions
Bot
added
domain/vc
|
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:
📝 WalkthroughWalkthroughMeeting event and active-meeting shortcuts now validate runtime authorization using either supported meeting-query scope. Static scope metadata, validation logic, tests, and identity-specific permission guidance were updated. ChangesVC meeting scope validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant VCMeetingShortcut
participant checkMeetingQueryAnyScope
participant meetingQueryTokenResolver
participant PermissionError
VCMeetingShortcut->>checkMeetingQueryAnyScope: validate meeting query access
checkMeetingQueryAnyScope->>meetingQueryTokenResolver: resolve identity and granted scopes
checkMeetingQueryAnyScope->>PermissionError: return missing-scope error when no candidate matches
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
shortcuts/vc/vc_meeting_events_test.go (1)
421-437: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an error-path test for
checkMeetingQueryAnyScope.The new test verifies
ScopesForIdentityreturns empty andhasAnyGrantedScopeaccepts/rejects the right scopes, but doesn't exercise thecheckMeetingQueryAnyScopeerror path — where no candidate scope is granted and the function returns a typed*errs.PermissionError. Per coding guidelines, error-path tests must assert typed metadata viaerrs.ProblemOf(category/subtype/param) and cause preservation.Consider adding a test that constructs a
RuntimeContextwith a credential provider returning a token with no meeting-query scopes, callscheckMeetingQueryAnyScope, and asserts the returned error is a*errs.PermissionErrorwithCategory == errs.CategoryAuthorization,Subtype == errs.SubtypeMissingScope, and populatedMissingScopes/Identity/Hint.🤖 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_events_test.go` around lines 421 - 437, Add an error-path test for checkMeetingQueryAnyScope that builds a RuntimeContext whose credential provider returns a token without meeting-query scopes, then invokes the check directly. Assert the error via errs.ProblemOf and verify it is a *errs.PermissionError with Category authorization, Subtype missing-scope, populated MissingScopes, Identity, and Hint, while also confirming the underlying cause is preserved.Source: Coding guidelines
shortcuts/vc/vc_meeting_test.go (1)
611-627: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame error-path test gap applies here.
The
hasAnyGrantedScopeassertions in this test are identical to those inTestMeetingEvents_UsesCustomAnyScopeCheck— they verify the shared helper rather thanVCMeetingListActive-specific behavior. The same suggestion to add an error-path test forcheckMeetingQueryAnyScopeapplies. If a shared test for the helper is added (e.g., in ahelpers_test.go), the duplicatedhasAnyGrantedScopeassertions could be removed from both shortcut test files.🤖 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 611 - 627, Update TestMeetingListActive_UsesCustomAnyScopeCheck to cover the error path of checkMeetingQueryAnyScope, verifying errors are returned or handled correctly for invalid scope input. Move the shared hasAnyGrantedScope assertions into a dedicated helper test (such as helpers_test.go) and remove the duplicated assertions from this test and TestMeetingEvents_UsesCustomAnyScopeCheck.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/references/lark-vc-agent-meeting-events.md`:
- Around line 16-21: Clarify the permissions table in the meeting-events
documentation: the listed scopes are recommendations, not identity-exclusive
requirements. State that runtime authorization in the relevant
permission-checking helper accepts either vc:meeting.meetingevent:read or
vc:meeting.bot.join:write for both --as user and --as bot.
- Around line 18-21: Replace the unexplained “UAT” and “TAT” labels in the
permissions table with the documented identity names “用户身份” and “应用身份,” while
retaining the corresponding --as user and --as bot examples and permission
values.
In `@skills/lark-vc-agent/references/lark-vc-agent-meeting-list-active.md`:
- Around line 11-14: Remove the unexplained “UAT” and “TAT” abbreviations from
the permissions table, using “用户身份” and “应用身份” consistently in the scenario
labels while preserving the existing command references and permission
recommendations.
- Around line 9-14: Update the permissions table in the meeting-list
documentation to state that both listed scopes are supported for either identity
(`--as user` and `--as bot`), while retaining the current scopes as
recommendations. Clarify that the command is authorized when either candidate
scope is granted, and avoid implying that each identity requires only its
corresponding scope.
---
Nitpick comments:
In `@shortcuts/vc/vc_meeting_events_test.go`:
- Around line 421-437: Add an error-path test for checkMeetingQueryAnyScope that
builds a RuntimeContext whose credential provider returns a token without
meeting-query scopes, then invokes the check directly. Assert the error via
errs.ProblemOf and verify it is a *errs.PermissionError with Category
authorization, Subtype missing-scope, populated MissingScopes, Identity, and
Hint, while also confirming the underlying cause is preserved.
In `@shortcuts/vc/vc_meeting_test.go`:
- Around line 611-627: Update TestMeetingListActive_UsesCustomAnyScopeCheck to
cover the error path of checkMeetingQueryAnyScope, verifying errors are returned
or handled correctly for invalid scope input. Move the shared hasAnyGrantedScope
assertions into a dedicated helper test (such as helpers_test.go) and remove the
duplicated assertions from this test and
TestMeetingEvents_UsesCustomAnyScopeCheck.
🪄 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: 8c7cb651-60cd-4255-852d-b4039612ae9b
📒 Files selected for processing (7)
shortcuts/vc/helpers.goshortcuts/vc/vc_meeting_events.goshortcuts/vc/vc_meeting_events_test.goshortcuts/vc/vc_meeting_list_active.goshortcuts/vc/vc_meeting_test.goskills/lark-vc-agent/references/lark-vc-agent-meeting-events.mdskills/lark-vc-agent/references/lark-vc-agent-meeting-list-active.md
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@f57d98ccf751d7dd623bbd8630de28b7630cca86🧩 Skill updatenpx skills add larksuite/cli#feat/lark-cli-vc-meeting-query-dual-scope-narrow -y -g |
There was a problem hiding this comment.
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/vc/helpers_test.go`:
- Around line 43-68: Add a `Category` assertion in
`assertMeetingQueryPermissionError` alongside the existing `Subtype` validation,
comparing `pe.Category` against the expected permission-error category constant
used by the surrounding tests or error definitions. Keep the existing
`errors.As`-based `PermissionError` checks and metadata validations intact.
🪄 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: 1ebe58ac-cf30-4420-bbc2-739b1b45145f
📒 Files selected for processing (2)
shortcuts/vc/helpers.goshortcuts/vc/helpers_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/vc/helpers.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1850 +/- ##
==========================================
+ Coverage 74.58% 74.73% +0.14%
==========================================
Files 861 887 +26
Lines 89971 92644 +2673
==========================================
+ Hits 67106 69235 +2129
- Misses 17669 18050 +381
- Partials 5196 5359 +163 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
8049df1 to
f83ee72
Compare
f83ee72 to
20ea90e
Compare
f0f280e to
a56dda9
Compare
2 participants
Summary
This change aligns VC meeting query shortcuts with the actual permission model used by the upstream APIs. Instead of relying on framework-level identity scope lists,
+meeting-list-activeand+meeting-eventsnow perform VC-local validation so eithervc:meeting.meetingevent:readorvc:meeting.bot.join:writecan satisfy the local preflight without changing shared framework behavior.Changes
UserScopesandBotScopesfrom+meeting-list-activeand+meeting-eventsValidatepathsTest Plan
lark-cli vcflow works as expectedgo test ./shortcuts/vcvc +meeting-list-active --as usersucceeds whenvc:meeting.meetingevent:readis granted and returns an active meetingmeeting_idvc +meeting-list-active --as bot --user-id <user_open_id>succeeds whenvc:meeting.meetingevent:readis available to the meeting query flowvc +meeting-list-active --as bot --user-id <user_open_id>succeeds whenvc:meeting.bot.join:writeis available to the meeting query flowvc +meeting-events --as bot --meeting-id <meeting_id>reaches the remote API without a localmissing_scopefailure whenvc:meeting.meetingevent:readis available; before the bot joined the meeting, the API returnedbot is not in the meetingvc +meeting-join --as bot --meeting-number <meeting_no>succeeds aftervc:meeting.bot.join:writeis appliedvc +meeting-events --as bot --meeting-id <meeting_id>succeeds after the bot joins the meeting and returns meeting activity eventsRelated Issues
Summary by CodeRabbit