Skip to content

fix: 修复 CodeRabbit webhook 反馈#527

Merged
ding113 merged 1 commit intodevfrom
fix/coderabbit-webhook-fixes
Jan 4, 2026
Merged

fix: 修复 CodeRabbit webhook 反馈#527
ding113 merged 1 commit intodevfrom
fix/coderabbit-webhook-fixes

Conversation

@ding113
Copy link
Owner

@ding113 ding113 commented Jan 3, 2026

Summary

This PR addresses code quality feedback from CodeRabbit reviews on recent webhook-related PRs, focusing on type safety, i18n consistency, and test documentation.

Related PRs:


Problem

CodeRabbit identified several code quality and consistency issues in the recent webhook feature implementation:

  1. Inconsistent logging - Dashboard migration dialog used console.warn instead of unified logger
  2. Missing i18n - Webhook target dialog showed hardcoded error messages instead of translation keys
  3. Type safety issues - Notification settings action used as any and inconsistent return types
  4. Type mismatch - customTemplate had frontend/backend type inconsistency (string vs Record)
  5. Missing test comments - Unit test intent not clearly documented

Solution

Fixes Applied

Issue Fix Files Changed
Console logging Replace console.warn with unified logger.warn() webhook-migration-dialog.tsx
Hardcoded errors Convert to i18n keys, add translations for 5 languages webhook-target-dialog.tsx, messages/*/settings.json
Type assertions Remove as any, unify updateNotificationSettingsAction to return ActionResult<T> notifications.ts, route.ts, hooks.ts
Type consistency Make customTemplate accept both string | Record<string, unknown> in server action webhook-targets.ts, route.ts
Test documentation Add comment explaining test intent internal-url-allowed.test.ts

New i18n Keys

Added to all 5 locales (en/ja/ru/zh-CN/zh-TW):

"notifications.targetDialog.errors": {
  "headersInvalidJson": "Headers must be valid JSON",
  "headersMustBeObject": "Headers must be a JSON object",
  "headersValueMustBeString": "Header values must be strings"
}

Type Safety Improvements

Before:

// Inconsistent return type
return { success: true, data: result };

// Type assertion
await updateNotificationSettingsAction(payload as any);

After:

// Unified ActionResult<T>
return { ok: true, data: result };

// Properly typed payload
const payload: UpdatePayload = { ... };
await updateNotificationSettingsAction(payload);

Changes

Core Changes

  • src/actions/notifications.ts - Unify return type to ActionResult<NotificationSettings>
  • src/actions/webhook-targets.ts - Support both string and object for customTemplate
  • src/app/[locale]/settings/notifications/_lib/hooks.ts - Remove as any, properly type all callbacks

Supporting Changes

  • messages/*/settings.json (5 files) - Add errors section for webhook target validation
  • src/app/[locale]/dashboard/_components/webhook-migration-dialog.tsx - Use unified logger
  • src/app/[locale]/settings/notifications/_components/webhook-target-dialog.tsx - Use i18n for errors
  • src/app/api/actions/[...route]/route.ts - Update schema to accept string | Record for customTemplate
  • tests/unit/actions/internal-url-allowed.test.ts - Add test intent comment

Testing

Validation

✅ bun run lint          # Biome checks passed
✅ bun run typecheck     # TypeScript checks passed
✅ bun run test          # All tests passed
✅ bun run build         # Production build successful
✅ bun run test --coverage  # Coverage improved

Coverage Improvement

Metric Before After Change
Statements 64.97% 65.03% +0.06%
Branches 51.00% 51.14% +0.14%
Functions 59.80% 59.95% +0.15%
Lines 65.81% 65.88% +0.07%

Checklist

  • Code follows project conventions
  • Self-review completed
  • Tests pass locally
  • i18n translations complete (5 languages)
  • Type safety improved (removed as any)
  • Coverage maintained/improved

Original Description (Chinese)

修复点:

  • dashboard webhook 迁移弹窗:使用统一 logger(替换 console.warn)。
  • Webhook 目标弹窗:Headers 解析错误改为 i18n key,并补齐 5 语言翻译。
  • 通知设置 hooks:统一 updateNotificationSettingsAction 返回 ActionResult(ok/data/error),移除 as any 与多处类型断言。
  • Webhook target:customTemplate 在前后端统一为 JSON 对象(Record),server action 兼容 string/object 两种输入。
  • 单测:按建议补充用例意图注释。

验证:bun run lint/typecheck/test/build/test --coverage
覆盖率:All files(Stmts/Branch/Funcs/Lines)由 64.97/51.00/59.80/65.81 提升到 65.03/51.14/59.95/65.88


Description enhanced by Claude AI

@coderabbitai
Copy link

coderabbitai bot commented Jan 3, 2026

📝 Walkthrough

高层概述

此PR为Webhook目标的自定义HTTP头部JSON验证添加了多语言错误消息,更新了通知设置和Webhook目标操作的类型签名,并增强了客户端验证逻辑以支持翻译错误消息和灵活的数据结构。

更改

文件分组 更改摘要
国际化消息文件
messages/en/settings.json, messages/ja/settings.json, messages/ru/settings.json, messages/zh-CN/settings.json, messages/zh-TW/settings.json
在所有语言的设置JSON中的targetDialog.errors下添加三个验证错误消息(headersInvalidJsonheadersMustBeObjectheadersValueMustBeString),用于自定义头部JSON验证。
通知操作
src/actions/notifications.ts
updateNotificationSettingsAction的返回类型从Promise<{ success: boolean; ... }>更改为Promise<ActionResult<NotificationSettings>>;用ok替换success标志;添加生产环境调度逻辑和开发模式日志记录。
Webhook目标操作
src/actions/webhook-targets.ts
parseCustomTemplate函数签名从(value: string | null | undefined)更改为(value: unknown);添加新的CustomTemplateSchemaZod联合类型;更新BaseTargetSchemacustomTemplate字段以接受新的schema。
日志改进
src/app/[locale]/dashboard/_components/webhook-migration-dialog.tsx
console.warn替换为结构化的logger.warn调用,保持现有行为。
Webhook目标对话框组件
src/app/[locale]/settings/notifications/_components/webhook-target-dialog.tsx
添加parseTemplateJsonparseHeadersJson函数,支持翻译错误消息;当providerType"custom"时,仅解析customTemplate;增强头部JSON验证,包括对象类型和值类型检查。
通知Hooks
src/app/[locale]/settings/notifications/_lib/hooks.ts
更新WebhookTargetCreateInput.customTemplate类型为Record<string, unknown> | null;将updateSettingscreateTargetupdateTargetdeleteTargettestTarget等函数的返回类型更新为显式的Promise<ClientActionResult<T>>;重构updateSettings逻辑为单一统一的操作调用;createTarget现在刷新设置。
API路由
src/app/api/actions/[...route]/route.ts
WebhookTargetCreateSchema.customTemplate从纯字符串改为允许字符串(已修整)或JSON对象(字符串到未知值的记录)的联合类型;简化updateNotificationSettingsRoute以直接委托给操作。
单元测试
tests/unit/actions/internal-url-allowed.test.ts
在测试中添加单行内联注释,无代码逻辑更改。

预估代码审查工作量

🎯 4 (Complex) | ⏱️ ~60 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed PR标题虽然使用了中文,但内容明确指向修复CodeRabbit webhook反馈相关的问题,与变更集内容高度相关,涵盖了主要改进方向。
Description check ✅ Passed PR描述与changeset高度相关,详细列举了五个修复点及其对应的代码改动,包括日志统一、i18n翻译、类型统一、兼容性改进和测试注释补充。
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/coderabbit-webhook-fixes

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @ding113, 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!

此拉取请求旨在修复 CodeRabbit webhook 反馈中的多项问题,通过引入国际化错误信息、统一日志记录、标准化 Action 返回结果以及统一 customTemplate 的数据结构,全面提升了系统的健壮性、用户体验和代码质量。这些改进使得 webhook 配置更加稳定,错误提示更加清晰,并减少了潜在的类型不一致问题。

Highlights

  • Webhook 错误信息国际化: Webhook 目标弹窗中的 Headers 解析错误现在使用 i18n 键,并已补齐五种语言的翻译,提升了用户体验。
  • 统一日志记录: Dashboard Webhook 迁移弹窗中,console.warn 已被替换为统一的 logger,确保日志记录的一致性。
  • 通知设置 Action 结果标准化: 通知设置相关的 updateNotificationSettingsAction 现在统一返回 ActionResult(包含 ok/data/error 字段),并移除了多余的 as any 和类型断言,增强了类型安全性。
  • CustomTemplate 数据结构统一: Webhook target 的 customTemplate 在前后端统一为 JSON 对象(Record 类型),并且服务器端 Action 兼容处理字符串和对象两种输入格式。
  • 单元测试用例意图注释: 根据建议,为单元测试补充了用例意图注释,提高了测试代码的可读性和可维护性。
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@github-actions github-actions bot added bug Something isn't working area:i18n labels Jan 3, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Jan 3, 2026

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces enhanced validation and internationalization for custom headers and custom templates within webhook notification settings. New i18n error messages for invalid JSON, non-object JSON, and non-string header values have been added across multiple language files (English, Japanese, Russian, Simplified Chinese, Traditional Chinese). The parseCustomTemplate function was refactored to accept both JSON strings and direct objects, with its Zod schema updated to reflect this change. Correspondingly, the UI's parseHeadersJson and a new parseTemplateJson function now leverage these new i18n messages for improved error feedback. Additionally, several action functions and related hooks were refactored to consistently use a standardized ActionResult type for their return values, replacing success with ok and explicitly typing promises for better type safety. A logging statement was also updated to use a dedicated logger, and a test comment was clarified.

@github-actions github-actions bot added the size/M Medium PR (< 500 lines) label Jan 3, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
src/app/api/actions/[...route]/route.ts (1)

907-915: Webhook 目标创建的 customTemplate 入参兼容 string 与对象

WebhookTargetCreateSchemacustomTemplate 改为 z.union([z.string().trim(), z.record(z.string(), z.unknown())]).optional().nullable(),这与服务端 CustomTemplateSchema / parseCustomTemplate 的设计一致,既支持 API 客户端直接传对象,也兼容前端以 JSON 字符串形式提交,方便后续逐步统一为 JSON 对象形态。

目前 schema 在 route 与 actions 中分别定义,未来如果有需要可以考虑在 actions 中导出公共 schema 以减少重复定义、避免演进时两处形状不一致的风险。

src/actions/notifications.ts (1)

14-61: 统一使用 ActionResult 返回通知设置更新结果

updateNotificationSettingsAction 改为返回 Promise<ActionResult<NotificationSettings>>,并在鉴权失败、更新异常等路径上统一返回 { ok: false, error },成功则返回 { ok: true, data },这与其他 admin actions 以及 API 文档说明的响应格式完全对齐,有利于前端复用同一套处理逻辑。

可以考虑在 catch 分支里补充一条 logger.error 日志(类似 webhook-targets actions 的做法),方便排查更新失败时的具体原因,但这属可选增强,不影响当前功能正确性。

src/actions/webhook-targets.ts (1)

28-49: parseCustomTemplate 支持 string 与对象,但可考虑统一 JSON 解析报错信息

当前 parseCustomTemplate

  • 接受 unknown,对 null / undefined 返回 null
  • 对 string:trim 后若为空返回 null,否则直接 JSON.parse,再校验必须为“非数组对象”,否则抛出 "自定义模板必须是 JSON 对象"
  • 对对象:若非数组则直接视为合法模板;
  • 其他类型抛出同样的错误。

这样可以很好地兼容:

  • API 客户端直接传 JSON 对象;
  • 前端传 JSON 字符串(由 route schema 校验为 string)。

小建议:可以在 JSON.parse 外包一层 try/catch,把语法错误也统一转换为 "自定义模板必须是 JSON 对象",避免向调用方暴露底层 SyntaxError 文案,同时与形状校验失败时的错误消息保持一致:

let parsed: unknown;
try {
  parsed = JSON.parse(trimmed);
} catch {
  throw new Error("自定义模板必须是 JSON 对象");
}

这属于体验改进,不影响当前逻辑正确性,因为外层 action 已有 try/catch 包装。

src/app/[locale]/settings/notifications/_lib/hooks.ts (1)

171-227: updateSettings 统一构造 payload 并直接消费 ActionResult

updateSettings 现在:

  • 通过 type UpdatePayload = Parameters<typeof updateNotificationSettingsAction>[0]; 自动对齐后端 UpdateNotificationSettingsInput 形状,减少手写类型漂移的风险;
  • 仅在 patch 中字段存在时才写入 payload,避免无意义的覆盖;
  • 对 Webhook URL 做 trim() 并把空字符串归一为 null,与后端存储类型兼容;
  • costAlertThreshold 从 number 转为 string 传递给后端,符合仓储层 string | null 约定;
  • 直接使用 updateNotificationSettingsAction{ ok, data, error } 返回值,失败路径返回自定义的 ClientActionResult<void>,成功时用返回的最新配置更新本地 settings

这让前端与后端的更新契约非常清晰,避免多处零散的字段更新调用。

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to Reviews > Disable Cache setting

📥 Commits

Reviewing files that changed from the base of the PR and between 83290bc and 134feee.

📒 Files selected for processing (12)
  • messages/en/settings.json
  • messages/ja/settings.json
  • messages/ru/settings.json
  • messages/zh-CN/settings.json
  • messages/zh-TW/settings.json
  • src/actions/notifications.ts
  • src/actions/webhook-targets.ts
  • src/app/[locale]/dashboard/_components/webhook-migration-dialog.tsx
  • src/app/[locale]/settings/notifications/_components/webhook-target-dialog.tsx
  • src/app/[locale]/settings/notifications/_lib/hooks.ts
  • src/app/api/actions/[...route]/route.ts
  • tests/unit/actions/internal-url-allowed.test.ts
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{ts,tsx,js,jsx,json}

📄 CodeRabbit inference engine (CLAUDE.md)

Use 2-space indentation in all code files

Files:

  • tests/unit/actions/internal-url-allowed.test.ts
  • src/app/[locale]/dashboard/_components/webhook-migration-dialog.tsx
  • messages/zh-CN/settings.json
  • messages/en/settings.json
  • messages/ru/settings.json
  • src/actions/webhook-targets.ts
  • src/actions/notifications.ts
  • messages/ja/settings.json
  • messages/zh-TW/settings.json
  • src/app/[locale]/settings/notifications/_components/webhook-target-dialog.tsx
  • src/app/api/actions/[...route]/route.ts
  • src/app/[locale]/settings/notifications/_lib/hooks.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Use double quotes for strings instead of single quotes
Use trailing commas in multi-line structures
Enforce maximum line length of 100 characters
Use path alias @/* to reference files from ./src/* directory

**/*.{ts,tsx,js,jsx}: Use Biome for linting and formatting with 2-space indent, double quotes, trailing commas, and 100 character max line length
Use path alias @/* to reference files in ./src/* directory

Files:

  • tests/unit/actions/internal-url-allowed.test.ts
  • src/app/[locale]/dashboard/_components/webhook-migration-dialog.tsx
  • src/actions/webhook-targets.ts
  • src/actions/notifications.ts
  • src/app/[locale]/settings/notifications/_components/webhook-target-dialog.tsx
  • src/app/api/actions/[...route]/route.ts
  • src/app/[locale]/settings/notifications/_lib/hooks.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use TypeScript strict mode for type safety
Use readonly or const assertions for immutable data structures

Files:

  • tests/unit/actions/internal-url-allowed.test.ts
  • src/app/[locale]/dashboard/_components/webhook-migration-dialog.tsx
  • src/actions/webhook-targets.ts
  • src/actions/notifications.ts
  • src/app/[locale]/settings/notifications/_components/webhook-target-dialog.tsx
  • src/app/api/actions/[...route]/route.ts
  • src/app/[locale]/settings/notifications/_lib/hooks.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use Vitest for unit testing with Node environment, coverage thresholds: 50% lines/functions, 40% branches

Files:

  • tests/unit/actions/internal-url-allowed.test.ts
**/*.test.ts

📄 CodeRabbit inference engine (AGENTS.md)

Ensure test database names contain 'test' keyword for safety validation

Files:

  • tests/unit/actions/internal-url-allowed.test.ts
src/**/*.{tsx,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{tsx,jsx}: Use lucide-react for icons, no custom SVGs
Use React's automatic escaping to prevent XSS vulnerabilities

Files:

  • src/app/[locale]/dashboard/_components/webhook-migration-dialog.tsx
  • src/app/[locale]/settings/notifications/_components/webhook-target-dialog.tsx
**/*.{tsx,json}

📄 CodeRabbit inference engine (AGENTS.md)

Use next-intl for internationalization with 5 locales: en, ja, ru, zh-CN, zh-TW

Files:

  • src/app/[locale]/dashboard/_components/webhook-migration-dialog.tsx
  • messages/zh-CN/settings.json
  • messages/en/settings.json
  • messages/ru/settings.json
  • messages/ja/settings.json
  • messages/zh-TW/settings.json
  • src/app/[locale]/settings/notifications/_components/webhook-target-dialog.tsx
messages/**/*.json

📄 CodeRabbit inference engine (CLAUDE.md)

Support 5 locales via next-intl: en, ja, ru, zh-CN, zh-TW with messages in messages/{locale}/*.json

Store message translations in messages/{locale}/*.json files

Files:

  • messages/zh-CN/settings.json
  • messages/en/settings.json
  • messages/ru/settings.json
  • messages/ja/settings.json
  • messages/zh-TW/settings.json
src/actions/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

src/actions/**/*.ts: Validate all user inputs with Zod schemas before processing
Use Server Actions in next-safe-action with OpenAPI generation for admin API endpoints
Use Next.js API Routes and Server Actions for admin operations and REST endpoints

Files:

  • src/actions/webhook-targets.ts
  • src/actions/notifications.ts
src/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.ts: Hash API keys using SHA-256 before storing in database, never store plaintext keys
Mask API keys and sensitive data in application logs
Validate required environment variables at startup with clear error messages

Files:

  • src/actions/webhook-targets.ts
  • src/actions/notifications.ts
  • src/app/api/actions/[...route]/route.ts
  • src/app/[locale]/settings/notifications/_lib/hooks.ts
src/{repository,actions}/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Avoid N+1 queries by using eager loading and batch queries for statistics

Files:

  • src/actions/webhook-targets.ts
  • src/actions/notifications.ts
src/app/api/actions/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Server Actions in src/app/api/actions/ must auto-generate OpenAPI 3.1.0 spec from Zod schemas and expose as REST endpoints

Files:

  • src/app/api/actions/[...route]/route.ts
src/app/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Implement Content-Security-Policy headers for XSS prevention

Files:

  • src/app/api/actions/[...route]/route.ts
  • src/app/[locale]/settings/notifications/_lib/hooks.ts
src/app/api/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Implement health check endpoint returning database and Redis status

Files:

  • src/app/api/actions/[...route]/route.ts
🧠 Learnings (5)
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/components/**/*.{tsx,jsx} : Use shadcn/ui component library for high-quality, accessible UI components

Applied to files:

  • src/app/[locale]/dashboard/_components/webhook-migration-dialog.tsx
📚 Learning: 2026-01-03T09:08:20.573Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T09:08:20.573Z
Learning: Applies to messages/**/*.json : Support 5 locales via next-intl: en, ja, ru, zh-CN, zh-TW with messages in `messages/{locale}/*.json`

Applied to files:

  • messages/zh-CN/settings.json
  • messages/en/settings.json
  • messages/ru/settings.json
  • messages/ja/settings.json
  • messages/zh-TW/settings.json
  • src/app/[locale]/settings/notifications/_components/webhook-target-dialog.tsx
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/actions/**/*.ts : Validate all user inputs with Zod schemas before processing

Applied to files:

  • src/actions/webhook-targets.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/actions/**/*.ts : Use Server Actions in `next-safe-action` with OpenAPI generation for admin API endpoints

Applied to files:

  • src/actions/notifications.ts
  • src/app/api/actions/[...route]/route.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/actions/**/*.ts : Use Next.js API Routes and Server Actions for admin operations and REST endpoints

Applied to files:

  • src/app/api/actions/[...route]/route.ts
🧬 Code graph analysis (3)
src/app/[locale]/dashboard/_components/webhook-migration-dialog.tsx (1)
src/lib/logger.ts (1)
  • logger (168-187)
src/actions/notifications.ts (3)
src/actions/types.ts (1)
  • ActionResult (31-31)
src/repository/notifications.ts (1)
  • NotificationSettings (11-34)
src/lib/auth.ts (1)
  • getSession (116-128)
src/app/[locale]/settings/notifications/_lib/hooks.ts (4)
src/actions/notifications.ts (1)
  • updateNotificationSettingsAction (30-61)
src/actions/webhook-targets.ts (4)
  • updateWebhookTargetAction (322-347)
  • deleteWebhookTargetAction (349-363)
  • NotificationType (79-79)
  • testWebhookTargetAction (365-422)
src/app/[locale]/settings/notifications/_lib/schemas.ts (1)
  • NotificationType (10-10)
src/repository/notification-bindings.ts (1)
  • NotificationType (8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: Greptile Review
  • GitHub Check: pr-review
  • GitHub Check: 🌐 API Tests
  • GitHub Check: pr-label
  • GitHub Check: pr-description
  • GitHub Check: check-codex-status
  • GitHub Check: Docker Build Test
  • GitHub Check: dev-build-deploy
🔇 Additional comments (12)
tests/unit/actions/internal-url-allowed.test.ts (1)

39-46: 测试意图注释补充得当

这条注释准确说明了用例关注的是“输入层允许内网 IP,但 provider 层因 hostname 不支持而失败”,方便后续维护理解测试语义,建议保留。

messages/zh-CN/settings.json (1)

1679-1685: 自定义 Headers 校验文案与多语言结构一致

新增的 headersInvalidJson / headersMustBeObject / headersValueMustBeString 语义清晰,位置在 notifications.targetDialog.errors 下也与其它 locale 对齐,有利于前端统一使用 t("notifications.targetDialog.errors.*") 进行提示。

src/app/[locale]/dashboard/_components/webhook-migration-dialog.tsx (1)

25-27: 迁移到统一 logger 的改动合理

console.warn 替换为 logger.warn 并引入统一日志封装,既保持了“获取设置失败不阻塞用户”的语义,又让错误出现在集中日志通道中,符合集中化观测的方向。

Also applies to: 137-140

messages/ja/settings.json (1)

423-427: Headers 校验日文文案与键结构正确

headersInvalidJson / headersMustBeObject / headersValueMustBeString 的翻译准确表达了 JSON 解析与类型要求,且放在 notifications.targetDialog.errors 下,方便与其他语言保持一一对应。

messages/zh-TW/settings.json (1)

423-427: 繁体 Headers 校验文案与其它语言对齐

新增的三条錯誤訊息鍵名和語義均與其他語言版本一致,放置在 notifications.targetDialog.errors 下也符合既有結構,方便前端統一使用與回退。

messages/en/settings.json (1)

432-436: 自定义 Headers 错误文案与前端校验逻辑已对齐

新增的 notifications.targetDialog.errors.* 三个 key 与前端 parseHeadersJson 中使用的 key 完全一致,英文描述准确清晰,能覆盖 JSON 不合法 / 非对象 / value 非字符串三类错误场景,没有发现命名或含义问题。

messages/ru/settings.json (1)

423-427: 俄文 Headers 错误文案与英文版语义一致

headersInvalidJson / headersMustBeObject / headersValueMustBeString 三个俄文翻译与英文含义一致,能清楚区分 JSON 解析失败、结构不是对象以及 value 不是字符串三种情况,满足多语言一致性要求。

src/app/api/actions/[...route]/route.ts (1)

790-841: 通知设置路由直接复用 ActionResult 风格的服务端 Action

这里直接把 notificationActions.updateNotificationSettingsAction 作为 createActionRoute 的执行函数,去掉了之前多余的包装层,统一使用 { ok, data, error } 约定,和前端 hooks 以及其他通知相关 action 的返回结构保持一致,没有发现权限或类型上的问题。

src/app/[locale]/settings/notifications/_components/webhook-target-dialog.tsx (1)

70-91: 本地 JSON 解析与 i18n 错误消息集成良好

  • parseTemplateJson:对空字符串直接返回 null,非空时做 JSON.parse 并校验为“非数组对象”,错误统一抛出 notifications.templateEditor.jsonInvalid,与已有文案 key 保持一致。
  • parseHeadersJson:细分三类错误:
    • 解析失败 → notifications.targetDialog.errors.headersInvalidJson
    • 非对象 / 数组 → ...headersMustBeObject
    • value 非字符串 → ...headersValueMustBeString
      与新加的 messages 中 key 完全一致,避免在组件里硬编码英文文案。
  • submit 中 payload 构造:
    • customTemplate 仅在 providerType === "custom" 时使用 parseTemplateJson 得到 Record<string, unknown> | null,与后端新的 customTemplate 类型对齐。
    • customHeaders 始终通过 parseHeadersJson 归一为 Record<string, string> | null,在前端就能拦截形状错误,减少后端 400。

整体上 JSON 校验责任前移到前端,并结合 i18n key 提供了清晰的错误提示,实现合理。

Also applies to: 93-121, 195-198

src/actions/webhook-targets.ts (1)

81-95: customTemplate 端到端类型统一与规范化逻辑合理

  • CustomTemplateSchema 定义为 string.trim() | record<string, unknown>,并在 BaseTargetSchema.customTemplate 中使用,确保:
    • HTTP 层(route)和 server action 层(这里)对 customTemplate 的形状约束一致;
    • 不会出现数组或其他基本类型混入。
  • normalizeTargetInput / normalizeTargetUpdateInput
    • providerType === "custom" 时统一通过 parseCustomTemplate 归一为 Record<string, unknown> | null
    • 使用两次 validateProviderConfig,第二次在 providerType === "custom" 时附带 customTemplate,强制自定义 Webhook 必须配置模板(否则抛 "自定义 Webhook 需要配置模板"),业务规则清晰;
    • 其它 providerType 分支合理设置 webhookUrl/telegramBotToken/dingtalkSecret/customHeaders 的互斥和归一化(如 Telegram 忽略 webhookUrl,自定义 provider 才保留 customHeaders)。

整体类型和归一化逻辑前后端一致,满足 PR 目标“customTemplate 在前后端统一为 JSON 对象(Record),server action 兼容 string 与 object 输入”的要求。

Also applies to: 105-157, 159-233

src/app/[locale]/settings/notifications/_lib/hooks.ts (2)

65-77: WebhookTargetCreateInput 的 customTemplate 类型与后端保持一致

WebhookTargetCreateInput.customTemplate 从 string 变更为 Record<string, unknown> | null,与:

  • 前端 WebhookTargetDialogparseTemplateJson 的返回值;
  • 后端 normalizeTargetInput / WebhookTargetcustomTemplate: Record<string, unknown> | null;

保持一致,避免再把原始 JSON 字符串透传到 server action,有利于在前端统一完成 JSON 解析与错误提示。


251-307: 目标 CRUD / 测试回调现在直接透传 ClientActionResult

createTarget / updateTarget / deleteTarget / testTarget 统一返回底层 action 的 ClientActionResult<T>,并在 ok 时按需刷新:

  • createTarget:刷新 targets、所有 bindings 以及 settings,确保刚创建的目标和自动切换模式(legacy → 多目标)立刻反映在 UI 中。
  • updateTarget / deleteTarget:刷新 targets 和所有 bindings,避免绑定列表与目标列表状态不一致。
  • testTarget:在测试成功时刷新 targets,使 lastTestResult / lastTestAt 即时更新。

这简化了调用方的错误判断逻辑(统一判断 ok),同时保证本地状态与服务端保持同步。

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

No significant issues identified in this PR.

PR Size: M

  • Lines changed: 258
  • Files changed: 12

Review Coverage

  • Logic and correctness - Clean
  • Security (OWASP Top 10) - Clean
  • Error handling - Clean
  • Type safety - Clean
  • Documentation accuracy - Clean
  • Test coverage - Adequate
  • Code clarity - Good

Automated review by Codex AI

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Reviewed PR #527 (fix: 修复 CodeRabbit webhook 反馈) across the 12 changed files and the full diff.
  • Applied the size label size/M (258 lines changed; 12 files).
  • Submitted a PR review comment with the “No significant issues identified in this PR” summary.
  • No inline review comments were posted because no diff-line issues met the ≥80 confidence threshold.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This PR successfully addresses code quality feedback from CodeRabbit reviews, focusing on type safety, i18n consistency, and logging standardization. After comprehensive analysis across all 6 review perspectives, no significant issues were identified.

PR Size: M

  • Lines changed: 258 (167 additions + 91 deletions)
  • Files changed: 12

Review Coverage

All changes have been validated against the following criteria:

  • Logic and correctness - Clean. Type unification properly maintains backward compatibility.
  • Security (OWASP Top 10) - Clean. No security vulnerabilities introduced.
  • Error handling - Clean. All JSON.parse calls are properly wrapped in try-catch blocks with i18n error messages.
  • Type safety - Improved. Successfully removed as any type assertion, unified ActionResult<T> return types, and added flexible string | Record support for customTemplate.
  • Documentation accuracy - Improved. Added test intent comment as requested.
  • i18n consistency - Improved. Replaced hardcoded error messages with translation keys across 5 locales.
  • Code clarity - Improved. Removed type assertions and simplified payload building in hooks.ts.

Analysis Details

Type Safety Improvements (src/actions/notifications.ts, src/app/[locale]/settings/notifications/_lib/hooks.ts):

  • ✅ Return type unified from { success: boolean, data?, error? } to ActionResult<T>
  • ✅ Removed as any type assertion in updateSettings callback
  • ✅ Properly typed payload construction instead of spreading with type assertion

i18n Consistency (webhook-target-dialog.tsx, messages/*/settings.json):

  • ✅ parseHeadersJson and parseTemplateJson now accept translation function
  • ✅ JSON parse errors properly wrapped in try-catch with i18n keys
  • ✅ All 5 locales updated consistently

Type Flexibility (src/actions/webhook-targets.ts):

  • ✅ parseCustomTemplate now handles both string | Record<string, unknown> inputs
  • ✅ Validation logic properly handles all input types
  • ✅ Backward compatible with existing string inputs

Logging Standardization (webhook-migration-dialog.tsx):

  • ✅ Replaced console.warn with unified logger.warn() with structured logging

Error Handling Validation:

  • ✅ All JSON.parse calls are wrapped in try-catch blocks
  • ✅ Error messages are properly localized
  • ✅ User feedback provided through toast notifications
  • ✅ No silent failures identified

Automated review by Claude AI - No issues found

@ding113 ding113 merged commit c786266 into dev Jan 4, 2026
19 checks passed
@github-project-automation github-project-automation bot moved this from Backlog to Done in Claude Code Hub Roadmap Jan 4, 2026
This was referenced Jan 6, 2026
@ding113 ding113 deleted the fix/coderabbit-webhook-fixes branch January 27, 2026 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:i18n bug Something isn't working size/M Medium PR (< 500 lines)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant