feat: add task event consumer#1510
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds the ChangesTask Event: task.task.update_user_access_v2
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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 |
|
got it !
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
skills/lark-task/references/lark-task-subscribe-event.md (1)
105-106: 💤 Low valueOptional: Reduce repetition of "wants to" phrasing in workflow steps.
Lines 105–106 use "wants to" twice in close succession. Consider rewording for variety:
- Line 105: "If the user wants to listen now" → "If listening immediately is the goal" or "To start listening now"
- Line 106: "If the user only wants to register the subscription" → "To only register the subscription"
This improves readability without changing meaning.
📝 Suggested revision
1. Confirm whether the user wants to subscribe with `user` identity or `bot` identity. -2. If the user wants to listen now, execute `lark-cli event consume task.task.update_user_access_v2 --as <identity>`. -3. If the user only wants to register the subscription, execute `lark-cli task +subscribe-event --as <identity>`. +2. To start listening now, execute `lark-cli event consume task.task.update_user_access_v2 --as <identity>`. +3. To only register the subscription, execute `lark-cli task +subscribe-event --as <identity>`. 4. Report whether the subscription or consumer startup succeeded, and clarify which identity it applies to.🤖 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-task/references/lark-task-subscribe-event.md` around lines 105 - 106, The workflow steps contain repetitive "wants to" phrasing that reduces readability. Reword the two conditional statements in the event subscription instructions to eliminate this repetition. For the step about listening immediately (currently "If the user wants to listen now"), revise to use a more direct phrasing like "To start listening now" or "If listening immediately is the goal". For the step about registration (currently "If the user only wants to register the subscription"), change to "To only register the subscription". These rewording maintain the meaning while improving clarity and variety in the documentation flow.Source: Linters/SAST tools
🤖 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 `@events/task/preconsume_test.go`:
- Around line 58-70: The TestTaskSubscriptionPreConsumeRequiresRuntime test
function only asserts the error category but does not validate the subtype and
param fields of the typed error returned by taskSubscriptionPreConsume. After
the existing category assertion on line 64, add additional error assertions to
check the p.Subtype field against the expected subtype value and verify the
p.Param field if applicable to ensure error classification regressions are
caught according to the coding guidelines.
In `@events/task/preconsume.go`:
- Around line 21-23: The error handling in the rt.CallAPI call needs to be
updated to preserve the typed error contract. Instead of returning raw errors
directly, check if the returned error from rt.CallAPI is already a typed error
using the appropriate type assertion or check, and if it is, pass it through
unchanged. For untyped errors, wrap them using errs.NewNetworkError with
errs.SubtypeNetworkTransport as the subtype, and chain the original error using
the .WithCause method to preserve the underlying error information before
returning.
In `@skills/lark-event/references/lark-event-task.md`:
- Line 34: The task_subscribe_event.go file currently only implements a POST
endpoint for subscribing to task events at
/open-apis/task/v2/task_v2/task_subscription with no corresponding DELETE
endpoint for unsubscribing. Add a DELETE endpoint to task_subscribe_event.go
that mirrors the unsubscribe pattern used in the peer event handlers
(whiteboard, vc, minutes, mail implementations) to enable proper subscription
cleanup on graceful shutdown. This will prevent repeated subscription calls when
the consumer is re-run.
---
Nitpick comments:
In `@skills/lark-task/references/lark-task-subscribe-event.md`:
- Around line 105-106: The workflow steps contain repetitive "wants to" phrasing
that reduces readability. Reword the two conditional statements in the event
subscription instructions to eliminate this repetition. For the step about
listening immediately (currently "If the user wants to listen now"), revise to
use a more direct phrasing like "To start listening now" or "If listening
immediately is the goal". For the step about registration (currently "If the
user only wants to register the subscription"), change to "To only register the
subscription". These rewording maintain the meaning while improving clarity and
variety in the documentation flow.
🪄 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: 845a6276-f14d-4d34-857a-a230ea7b7e92
📒 Files selected for processing (12)
cmd/event/list_test.gocmd/event/schema_test.goevents/register.goevents/task/native.goevents/task/preconsume.goevents/task/preconsume_test.goevents/task/register.goevents/task/register_test.goskills/lark-event/SKILL.mdskills/lark-event/references/lark-event-task.mdskills/lark-task/SKILL.mdskills/lark-task/references/lark-task-subscribe-event.md
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)
events/task/preconsume_test.go (1)
75-83:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStrengthen pass-through API error assertions (metadata + identity).
This error-path test currently checks only
errors.Is. A wrapped/reclassified error could still pass. Assert typed metadata viaerrs.ProblemOfand direct identity (err == wantErr) to lock the passthrough contract.Suggested patch
func TestTaskSubscriptionPreConsumePassesThroughAPIError(t *testing.T) { wantErr := errs.NewValidationError(errs.SubtypeFailedPrecondition, "subscription already exists") rt := &stubAPIClient{err: wantErr} _, err := taskSubscriptionPreConsume(context.Background(), rt, nil) + if err != wantErr { + t.Fatalf("err identity changed: got %T %v, want original %T %v", err, err, wantErr, wantErr) + } if !errors.Is(err, wantErr) { t.Fatalf("err = %v, want %v", err, wantErr) } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed error, got %T: %v", err, err) + } + if p.Category != errs.CategoryValidation { + t.Errorf("category = %s, want %s", p.Category, errs.CategoryValidation) + } + if p.Subtype != errs.SubtypeFailedPrecondition { + t.Errorf("subtype = %s, want %s", p.Subtype, errs.SubtypeFailedPrecondition) + } }As per coding guidelines:
**/*_test.go: Error-path tests must assert typed metadata viaerrs.ProblemOf(category/subtype/param) and cause preservation, not message substrings alone.🤖 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 `@events/task/preconsume_test.go` around lines 75 - 83, The test TestTaskSubscriptionPreConsumePassesThroughAPIError needs stronger assertions to verify the error passed through from taskSubscriptionPreConsume is not wrapped or reclassified. In addition to the existing errors.Is check, add assertions using errs.ProblemOf to verify the typed metadata of the error (ensuring the category and subtype match the original wantErr), and consider adding a direct identity check with err == wantErr to enforce that the exact error object is returned without modification. This ensures the passthrough contract is properly locked.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.
Outside diff comments:
In `@events/task/preconsume_test.go`:
- Around line 75-83: The test
TestTaskSubscriptionPreConsumePassesThroughAPIError needs stronger assertions to
verify the error passed through from taskSubscriptionPreConsume is not wrapped
or reclassified. In addition to the existing errors.Is check, add assertions
using errs.ProblemOf to verify the typed metadata of the error (ensuring the
category and subtype match the original wantErr), and consider adding a direct
identity check with err == wantErr to enforce that the exact error object is
returned without modification. This ensures the passthrough contract is properly
locked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf8783b0-20b2-4240-a874-d181bb76e559
📒 Files selected for processing (3)
events/task/preconsume.goevents/task/preconsume_test.goskills/lark-task/references/lark-task-subscribe-event.md
✅ Files skipped from review due to trivial changes (1)
- skills/lark-task/references/lark-task-subscribe-event.md
🚧 Files skipped from review as they are similar to previous changes (1)
- events/task/preconsume.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@b4dc062ce02f944b26dfde1e68fe265c8d7f9c56🧩 Skill updatenpx skills add ILUO/cli#feat/task-event-consume -y -g |
Summary
Add the Lark Task v2 event consumer for
task.task.update_user_access_v2using the existing EventKey registry, bus daemon, and schema pipeline. This also removes the legacytask +subscribe-eventshortcut so Task event subscription is exposed through the unifiedevent consumeflow only.Changes
task.task.update_user_access_v2..eventand schema metadata for.event.task_guid,.event.event_types, and.header.event_id.event_typesenum values andtask_guidformat.POST /open-apis/task/v2/task_v2/task_subscription?user_id_type=open_id.task +subscribe-eventshortcut, tests, and skill reference docs.lark-eventandlark-taskdocs to useevent consume task.task.update_user_access_v2.Test Plan
go test ./shortcuts/task ./events/... ./cmd/event/... -count=1go test ./events/task -count=1go run . task --helpno longer shows+subscribe-eventgo run . event list --json | grep task.task.update_user_access_v2go run . event schema task.task.update_user_access_v2 --jsonnpxavailability: non-shortcuts/appspackages pass without shim;shortcuts/appspasses with a temporarynpxshimRelated Issues
Summary by CodeRabbit
New Features
task.task.update_user_access_v2event type for real-time task access update consumptionDocumentation
Deprecated