feat(notification): add Feishu webhook support#490
Conversation
重构 webhook 通知系统,支持飞书和微信两种平台。 - 引入 StructuredMessage 抽象层实现多平台渲染 - 添加飞书卡片消息渲染器 - 统一消息模板(熔断、成本、排行榜) - 支持指数退避重试 BREAKING CHANGE: WeChatBot API 已替换为 WebhookNotifier Signed-off-by: Kevin Cui <bh@bugs.cc>
Summary of ChangesHello @BlackHole1, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在全面升级和优化系统的通知功能。通过引入一个抽象的 Webhook 通知框架,它使得系统能够更灵活地与各种第三方消息平台集成,而不仅仅局限于企业微信。这一改进不仅增加了对飞书等新平台的支持,还通过统一的消息结构和渲染逻辑,确保了通知内容在不同平台上的展示一致性和可维护性。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review Summary
This is a well-executed refactoring that introduces a structured notification system supporting both WeChat and Feishu platforms. The code quality is high with proper abstraction, comprehensive unit tests, and adherence to project standards.
PR Size: XL
- Lines changed: 1,805 (1,453 additions, 352 deletions)
- Files changed: 34
Note: This XL PR could potentially be split into:
- Core abstraction layer (types, notifier, utils)
- WeChat migration (refactor existing code)
- Feishu support (new renderer)
- Templates refactoring
- Test coverage
However, the changes are tightly coupled and the current scope is manageable given the quality.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean (SSRF protection maintained)
- Error handling - Clean (proper try-catch with logging and retry)
- Type safety - Clean (no
anyusage, proper TypeScript types) - Documentation accuracy - Clean (comments are accurate and helpful)
- Test coverage - Excellent (comprehensive unit tests for new modules)
- Code clarity - Good (well-structured abstraction)
Highlights
Strengths:
- Clean abstraction with
StructuredMessageinterface enabling multi-platform rendering - Registry pattern for renderers maintains extensibility
- Comprehensive unit test coverage for all new modules
- Proper error handling with retry logic and logging
- Maintains SSRf protection from existing implementation
- Consistent with existing codebase patterns (see
src/lib/wechat/bot.ts) - i18n updates for all 5 locales
- Breaking change clearly documented in PR description
Architecture:
The new notification system follows the existing patterns in the codebase:
- Similar error handling pattern to
WeChatBot.sendMarkdown()(lines 47-97 in bot.ts) - Retry logic abstraction mirrors the queue processor approach
- Type safety maintained throughout
Automated review by Claude AI
There was a problem hiding this comment.
Additional Comments (1)
-
tests/unit/webhook/notifier.test.ts, line 47-48 (link)syntax: test expects error message "Unsupported webhook URL" but the actual code throws "Unsupported webhook hostname: ${url.hostname}" (see
src/lib/webhook/notifier.ts:46)
34 files reviewed, 1 comment
There was a problem hiding this comment.
Pull request overview
This pull request refactors the webhook notification system to support multiple platforms (Feishu and WeChat), replacing the previous WeChat-only implementation. The refactoring introduces a structured message abstraction layer with platform-specific renderers, enabling cleaner separation of concerns and easier extensibility.
Key Changes:
- Introduced
StructuredMessageabstraction layer for platform-independent message construction - Added Feishu card message renderer alongside the existing WeChat markdown renderer
- Unified message templates for circuit breaker alerts, cost warnings, and leaderboard notifications
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/lib/webhook/notifier.ts |
New webhook notifier with provider auto-detection and retry logic |
src/lib/webhook/renderers/wechat.ts |
WeChat markdown renderer implementation |
src/lib/webhook/renderers/feishu.ts |
Feishu card renderer implementation |
src/lib/webhook/templates/circuit-breaker.ts |
Structured circuit breaker alert message template |
src/lib/webhook/templates/cost-alert.ts |
Structured cost alert message template |
src/lib/webhook/templates/daily-leaderboard.ts |
Structured daily leaderboard message template |
src/lib/webhook/templates/test-messages.ts |
Test message builder for webhook validation |
src/lib/webhook/types.ts |
Type definitions for structured messages and webhook interfaces |
src/lib/webhook/utils/retry.ts |
Retry utility with exponential backoff |
src/lib/webhook/utils/date.ts |
Date formatting utility for China timezone |
src/lib/notification/notification-queue.ts |
Updated to use new webhook system and structured messages |
src/actions/notifications.ts |
Updated test webhook action to accept notification type parameter |
src/app/[locale]/settings/notifications/page.tsx |
Updated UI to pass notification type to test webhook function |
src/lib/constants/notification.constants.ts |
Centralized notification job type constants |
src/lib/wechat/bot.ts |
Removed old WeChat-specific bot implementation |
src/lib/wechat/message-templates.ts |
Removed old WeChat-specific message templates |
tests/unit/webhook/ |
Comprehensive test coverage for new webhook system |
messages/*/settings.json |
Updated UI strings to reflect generic webhook support |
src/drizzle/schema.ts |
Updated comment to reflect generic webhook configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ], | ||
| totalRequests: 270, | ||
| totalCost: 22.7, | ||
| }); |
There was a problem hiding this comment.
The switch statement is missing a default case. While TypeScript's exhaustiveness checking ensures all NotificationJobType values are handled, adding a default case that throws an error would provide better runtime safety and clearer error messages if an unexpected value is passed.
| }); | |
| }); | |
| default: { | |
| const exhaustiveCheck: never = type; | |
| throw new Error(`Unhandled NotificationJobType in buildTestMessage: ${exhaustiveCheck}`); | |
| } |
| } | ||
|
|
||
| export function buildCostAlertMessage(data: CostAlertData): StructuredMessage { | ||
| const usagePercent = (data.currentCost / data.quotaLimit) * 100; |
There was a problem hiding this comment.
Division by zero error: If quotaLimit is 0, line 10 will result in a division by zero, producing Infinity or NaN. This would cause incorrect percentage calculations and display. Add validation to handle the case where quotaLimit is zero or negative.
| describe("buildCostAlertMessage", () => { | ||
| it("should create structured message for user cost alert", () => { | ||
| const data: CostAlertData = { | ||
| targetType: "user", | ||
| targetName: "张三", | ||
| targetId: 100, | ||
| currentCost: 8.5, | ||
| quotaLimit: 10, | ||
| threshold: 0.8, | ||
| period: "本周", | ||
| }; | ||
|
|
||
| const message = buildCostAlertMessage(data); | ||
|
|
||
| expect(message.header.level).toBe("warning"); | ||
| expect(message.header.icon).toBe("💰"); | ||
| expect(message.header.title).toContain("成本预警"); | ||
|
|
||
| const sectionsStr = JSON.stringify(message.sections); | ||
| expect(sectionsStr).toContain("张三"); | ||
| expect(sectionsStr).toContain("8.5"); | ||
| expect(sectionsStr).toContain("本周"); | ||
| }); | ||
|
|
||
| it("should create structured message for provider cost alert", () => { | ||
| const data: CostAlertData = { | ||
| targetType: "provider", | ||
| targetName: "GPT-4", | ||
| targetId: 1, | ||
| currentCost: 950, | ||
| quotaLimit: 1000, | ||
| threshold: 0.9, | ||
| period: "本月", | ||
| }; | ||
|
|
||
| const message = buildCostAlertMessage(data); | ||
|
|
||
| expect(message.header.level).toBe("warning"); | ||
| const sectionsStr = JSON.stringify(message.sections); | ||
| expect(sectionsStr).toContain("供应商"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing test coverage for the division by zero edge case. The tests should include a case where quotaLimit is 0 or negative to ensure the function handles these edge cases gracefully without producing Infinity or NaN values.
|
|
||
| it("should use exponential backoff", async () => { | ||
| const delays: number[] = []; | ||
| const originalSetTimeout = globalThis.setTimeout; |
There was a problem hiding this comment.
Unused variable originalSetTimeout.
Signed-off-by: Kevin Cui <bh@bugs.cc>
Signed-off-by: Kevin Cui <bh@bugs.cc>
重构 webhook 通知系统,支持飞书和微信两种平台。
Resolves #485 - Implements custom Webhook notification support, enabling Feishu (飞书) in addition to WeChat Work
English Summary
This PR refactors the notification system from WeChat Work-only to a multi-platform webhook architecture, adding support for both WeChat Work (企业微信) and Feishu (飞书/Lark).
Problem
Previously, the system only supported WeChat Work webhooks (
WeChatBotclass), limiting users who use other enterprise collaboration platforms like Feishu or DingTalk.Solution
Introduced a platform-agnostic notification architecture:
StructuredMessageabstraction - platform-independent message representationWeChatRenderer) and Feishu (FeishuCardRenderer)Core Changes
New Architecture
Breaking Changes
WeChatBotclassWebhookNotifiernew WebhookNotifier(webhookUrl)sendWeChatNotification()sendWebhookMessage()buildCircuitBreakerAlert()buildCircuitBreakerMessage()buildCostAlert()buildCostAlertMessage()buildDailyLeaderboard()buildDailyLeaderboardMessage()src/lib/wechat/bot.tssrc/lib/webhook/notifier.tssrc/lib/wechat/message-templates.tssrc/lib/webhook/templates/Note: Webhook URLs are auto-detected by hostname:
qyapi.weixin.qq.com→ WeChat Work rendereropen.feishu.cn→ Feishu rendererTest Coverage
tests/unit/webhook/notifier.test.ts- Core notification logictests/unit/webhook/renderers/feishu.test.ts- Feishu card renderingtests/unit/webhook/renderers/wechat.test.ts- WeChat markdown renderingtests/unit/webhook/templates/templates.test.ts- All message templatestests/unit/webhook/utils/retry.test.ts- Retry mechanismUI Changes
circuitBreaker→circuit-breakertypeparameter totestWebhookAction()for template-specific testingDatabase
企业微信机器人通知配置→Webhook 通知配置Description enhanced by Claude AI
Greptile Summary
This PR refactors the notification system from WeChat-specific implementation to a multi-platform webhook system that supports both Feishu and WeChat.
Key Changes:
StructuredMessageabstraction layer that decouples business logic from platform-specific renderingFeishuCardRendererfor Feishu interactive card messagesWeChatRendererfor WeChat markdown messagesNotificationQueuefromsendWeChatNotificationtosendWebhookMessageWeChatBotclass and WeChat-specific message templatesBreaking Changes:
WeChatBotAPI replaced withWebhookNotifierbuildCircuitBreakerAlert→buildCircuitBreakerMessage)Issues Found:
Confidence Score: 4/5
tests/unit/webhook/notifier.test.tsneeds test assertion fix for error messageImportant Files Changed
Sequence Diagram
sequenceDiagram participant User participant NotificationQueue participant WebhookNotifier participant Renderer participant Template participant WebhookAPI User->>NotificationQueue: addNotificationJob(type, webhookUrl, data) NotificationQueue->>NotificationQueue: queue.add(job) Note over NotificationQueue: Job Processing (with retry) NotificationQueue->>Template: buildMessage(data) Template-->>NotificationQueue: StructuredMessage NotificationQueue->>WebhookNotifier: send(message) WebhookNotifier->>WebhookNotifier: detectProvider() WebhookNotifier->>Renderer: createRenderer(providerType) Renderer-->>WebhookNotifier: renderer instance WebhookNotifier->>Renderer: render(message) alt Feishu Renderer->>Renderer: FeishuCardRenderer.render() Renderer-->>WebhookNotifier: Interactive Card JSON else WeChat Renderer->>Renderer: WeChatRenderer.render() Renderer-->>WebhookNotifier: Markdown JSON end WebhookNotifier->>WebhookNotifier: withRetry(doSend, maxRetries=3) loop Retry up to 3 times WebhookNotifier->>WebhookAPI: POST webhook with payload alt Success Response WebhookAPI-->>WebhookNotifier: {errcode: 0} or {code: 0} WebhookNotifier-->>NotificationQueue: {success: true} else Error Response WebhookAPI-->>WebhookNotifier: Error code WebhookNotifier->>WebhookNotifier: throw Error Note over WebhookNotifier: Exponential backoff delay end end NotificationQueue-->>User: Job completed or failed