[codex] support bot menu events#1765
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 processing and registration for ChangesBot menu v6 event support
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant EventConsumer
participant GlobalRegistry
participant processBotMenu
participant BotMenuOutput
EventConsumer->>GlobalRegistry: lookup application.bot.menu_v6
GlobalRegistry->>processBotMenu: process raw event payload
processBotMenu->>BotMenuOutput: flatten and normalize fields
processBotMenu-->>EventConsumer: marshaled output
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
github-actions
Bot
added
the
size/M
3795f41 to
0d8576b
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the lark-cli event pipeline by adding a new Application-domain event key (application.bot.menu_v6) so bots can consume custom menu push events with a flattened, jq-friendly output shape.
Changes:
- Added a new
events/applicationdomain and registeredapplication.bot.menu_v6with a processed (flattened) output schema. - Implemented payload processing to surface
event_key, header metadata, menu timestamp, and operator identifiers at the top level (with timestamp fallback behavior). - Added unit tests for registration/metadata and for payload parsing + malformed-payload passthrough, and documented the new EventKey in the
lark-eventskill docs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| skills/lark-event/SKILL.md | Adds the Application topic entry pointing to the new Application EventKey reference. |
| skills/lark-event/references/lark-event-application.md | Documents application.bot.menu_v6, flattened fields, and jq filtering examples. |
| events/register.go | Registers the new Application event domain in the global event registry. |
| events/application/register.go | Defines the application.bot.menu_v6 KeyDefinition (schema + auth + required console events). |
| events/application/menu.go | Implements payload flattening + timestamp fallback and tolerant timestamp parsing. |
| events/application/menu_test.go | Adds unit coverage for definition metadata, registry integration, parsing, fallback behavior, and malformed passthrough. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
events/application/menu.go (1)
16-16: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider hardcoding
Typeinstead of trusting the payload.The
Typefield doc says it's "always application.bot.menu_v6", but the value is copied fromenvelope.Header.EventType(payload-controlled) rather than the localeventTypeBotMenuV6constant. If the header ever arrives empty/mismatched, downstream--jqfilters on.typewould silently break the documented guarantee.♻️ Proposed fix
out := &BotMenuOutput{ - Type: envelope.Header.EventType, + Type: eventTypeBotMenuV6, EventID: envelope.Header.EventID,Also applies to: 64-64
🤖 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/application/menu.go` at line 16, The menu event Type is currently being populated from envelope.Header.EventType instead of the local eventTypeBotMenuV6 constant, which makes the documented “always application.bot.menu_v6” guarantee depend on payload data. Update the application menu event construction in menu.go so Type is set directly from eventTypeBotMenuV6 (and not copied from the envelope header), ensuring the emitted JSON always matches the documented value even when the incoming header is empty or mismatched.
🤖 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 `@events/application/menu.go`:
- Line 16: The menu event Type is currently being populated from
envelope.Header.EventType instead of the local eventTypeBotMenuV6 constant,
which makes the documented “always application.bot.menu_v6” guarantee depend on
payload data. Update the application menu event construction in menu.go so Type
is set directly from eventTypeBotMenuV6 (and not copied from the envelope
header), ensuring the emitted JSON always matches the documented value even when
the incoming header is empty or mismatched.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4d874fd5-a12c-4282-920c-f3cd5c41877b
📒 Files selected for processing (6)
events/application/menu.goevents/application/menu_test.goevents/application/register.goevents/register.goskills/lark-event/SKILL.mdskills/lark-event/references/lark-event-application.md
0d8576b to
7ddd654
Compare
|
@LXJ0000 The feature direction is reasonable, and the overall implementation fits the existing event consume framework. The relevant package tests have passed. The main issue is the time unit handling for Please fix this before merging: normalize Also, the current GitHub Checks only show CodeRabbit and sync-pr-labels. Please run the regular CI checks as well. |
|
Thanks for the review. Fixed in Changes made:
Local verification:
I also tried the regular CI unit-test command locally: go test -v -race -count=1 -timeout=5m ./cmd/... ./internal/... ./shortcuts/... ./extension/...It ran through the suite but exits non-zero because of an unrelated existing This failure is outside the changed |
|
@sang-neo03 辛苦帮忙再 review 一下～～ |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@3ba85edd3b20229817068aa7cb038beab08ed80c🧩 Skill updatenpx skills add LXJ0000/cli#codex/support-bot-menu-event -y -g |
Summary
Adds
application.bot.menu_v6support tolark-cli eventso bots can consume custom menu push events over the existing event pipeline. The output is flattened around the menuevent_keyand operator identifiers so callers can filter events such asstart_evalwith--jq.Changes
application.bot.menu_v6registration and processed output schema.event_key, header metadata, menu timestamp, and operator open/union/user IDs.lark-eventskill reference with a--jqfiltering example.Test Plan
go test ./events/application ./eventslark-cli event consume application.bot.menu_v6 --as bot --jq 'select(.event_key == "start_eval")'flow receives a real bot menu click event.Related Issues
Summary by CodeRabbit
application.bot.menu_v6Application event, producing flattened output with event/header identifiers, menu metadata, operator details, and normalized timestamps.