Refactor GitHub event transformation pipeline for Feishu integration #7
Conversation
WalkthroughConsolidates GitHub-to-Feishu notification transformation by removing a dedicated card-generation script and merging its functionality into the main transform script. Updates the workflow to use the consolidated script with revised environment variables, centralizing event processing and card payload construction logic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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: 3
🧹 Nitpick comments (2)
.github/scripts/transform-message.ts (2)
47-48: Consider using a more specific type instead ofany.The
userparameter is typed asany, which bypasses type checking. Consider usingGitHubUser | null | undefinedfor better type safety.Apply this diff to improve type safety:
-const createUserLink = (user: any) => +const createUserLink = (user: GitHubUser | null | undefined) => user ? createLink(user.html_url, user.login) : "无";
148-248: Consistent pattern across all event handlers with similar non-null assertion concerns.All event handlers follow a uniform structure, which is good for maintainability. However, they all use non-null assertions on potentially undefined fields. Consider adding runtime checks or using optional chaining with fallbacks to prevent runtime errors.
For example, for the pull_request handler:
pull_request: ({ event: { pull_request } }, actionText) => ({ - title: `GitHub PR ${actionText}：${pull_request?.title}`, + title: `GitHub PR ${actionText}：${pull_request?.title || "未知 PR"}`, elements: [ { tag: "markdown", content: [ - createContentItem("链接：", createLink(pull_request!.html_url)), + createContentItem("链接：", pull_request?.html_url ? createLink(pull_request.html_url) : "无"),Apply similar patterns to other handlers to improve resilience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/scripts/transform-message-for-card.ts(0 hunks).github/scripts/transform-message.ts(3 hunks).github/workflows/Lark-notification.yml(2 hunks)
💤 Files with no reviewable changes (1)
- .github/scripts/transform-message-for-card.ts
🔇 Additional comments (8)
.github/scripts/transform-message.ts (6)
24-43: LGTM! Action text mapping is clear and comprehensive.The ACTION_TEXT_MAP provides a well-organized mapping of GitHub action types to their Chinese labels, and getActionText handles missing actions gracefully with a sensible fallback.
45-45: LGTM! Markdown link format is correct.The simplified createLink function correctly generates Markdown-style links.
74-75: LGTM! Content item formatting is clean.The function correctly formats content items with proper fallback for missing values.
87-123: LGTM! Push event handler includes proper fallbacks.The handler correctly handles missing head_commit scenarios with appropriate fallback values for both URL and message.
252-267: LGTM! Error handling is robust.The processEvent function properly validates handler existence and provides clear error messages with proper error chaining.
279-295: LGTM! Card structure follows Feishu schema 2.0 format.The card construction correctly uses the Feishu/Lark card schema with proper header and body elements.
.github/workflows/Lark-notification.yml (2)
44-44: Good security improvement: Using GitHub Secrets instead of environment variables.The change from environment variables to GitHub Secrets (
LARK_CHATBOT_HOOK_URL) is a security best practice for handling sensitive webhook URLs. Ensure that this secret is properly configured in the repository settings.
30-31: Old script file has been successfully removed.The transition from
transform-message-for-card.tstotransform-message.tsis clean—the old script no longer exists in the repository and no references to it remain in the codebase.
| const sanitizeMarkdown = (text: string): string => | ||
| text | ||
| // Remove code blocks | ||
| .replace(/```[\s\S]*?```/g, "[代码块]") | ||
| // Remove inline code | ||
| .replace(/`[^`]+`/g, (match) => match.slice(1, -1)) | ||
| // Convert images to link text | ||
| .replace(/!\[([^\]]*)\]\(([^)]+)\)/g, "🖼️ [$1]($2)") | ||
| // Convert ### headers to bold | ||
| .replace(/^###\s+(.+)$/gm, "**$1**") | ||
| // Convert ## headers to bold | ||
| .replace(/^##\s+(.+)$/gm, "**$1**") | ||
| // Convert # headers to bold | ||
| .replace(/^#\s+(.+)$/gm, "**$1**") | ||
| // Remove HTML comments | ||
| .replace(/<!--[\s\S]*?-->/g, "") | ||
| // Remove HTML tags (keep content) | ||
| .replace(/<[^>]+>/g, "") | ||
| // Remove excess empty lines | ||
| .replace(/\n{3,}/g, "\n\n") | ||
| // Truncate long content | ||
| .slice(0, 800) + (text.length > 800 ? "\n..." : ""); |
There was a problem hiding this comment.
Minor: Truncation check uses original text length.
The function slices the sanitized text to 800 characters but checks text.length > 800 (the original text length) to decide whether to append the ellipsis. This could result in the ellipsis appearing when it shouldn't, or not appearing when it should.
Apply this diff to fix the truncation logic:
const sanitizeMarkdown = (text: string): string =>
- text
+ {
+ const sanitized = text
// Remove code blocks
.replace(/```[\s\S]*?```/g, "[代码块]")
// Remove inline code
.replace(/`[^`]+`/g, (match) => match.slice(1, -1))
// Convert images to link text
.replace(/!\[([^\]]*)\]\(([^)]+)\)/g, "🖼️ [$1]($2)")
// Convert ### headers to bold
.replace(/^###\s+(.+)$/gm, "**$1**")
// Convert ## headers to bold
.replace(/^##\s+(.+)$/gm, "**$1**")
// Convert # headers to bold
.replace(/^#\s+(.+)$/gm, "**$1**")
// Remove HTML comments
.replace(/<!--[\s\S]*?-->/g, "")
// Remove HTML tags (keep content)
.replace(/<[^>]+>/g, "")
// Remove excess empty lines
- .replace(/\n{3,}/g, "\n\n")
- // Truncate long content
- .slice(0, 800) + (text.length > 800 ? "\n..." : "");
+ .replace(/\n{3,}/g, "\n\n");
+ return sanitized.slice(0, 800) + (sanitized.length > 800 ? "\n..." : "");
+};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sanitizeMarkdown = (text: string): string => | |
| text | |
| // Remove code blocks | |
| .replace(/```[\s\S]*?```/g, "[代码块]") | |
| // Remove inline code | |
| .replace(/`[^`]+`/g, (match) => match.slice(1, -1)) | |
| // Convert images to link text | |
| .replace(/!\[([^\]]*)\]\(([^)]+)\)/g, "🖼️ [$1]($2)") | |
| // Convert ### headers to bold | |
| .replace(/^###\s+(.+)$/gm, "**$1**") | |
| // Convert ## headers to bold | |
| .replace(/^##\s+(.+)$/gm, "**$1**") | |
| // Convert # headers to bold | |
| .replace(/^#\s+(.+)$/gm, "**$1**") | |
| // Remove HTML comments | |
| .replace(/<!--[\s\S]*?-->/g, "") | |
| // Remove HTML tags (keep content) | |
| .replace(/<[^>]+>/g, "") | |
| // Remove excess empty lines | |
| .replace(/\n{3,}/g, "\n\n") | |
| // Truncate long content | |
| .slice(0, 800) + (text.length > 800 ? "\n..." : ""); | |
| const sanitizeMarkdown = (text: string): string => { | |
| const sanitized = text | |
| // Remove code blocks | |
| .replace(/ |
🤖 Prompt for AI Agents
In .github/scripts/transform-message.ts around lines 51 to 72 the truncation
logic slices the sanitized string but checks text.length (original input) to
decide whether to append "…", which can misreport truncation; change the chain
to assign the result of all the .replace calls to a local variable (e.g.,
sanitized), then return sanitized.slice(0, 800) + (sanitized.length > 800 ?
"\n..." : ""); ensure the function closes correctly and no longer uses
text.length for the ellipsis check.
| issues: ({ event: { issue } }, actionText) => ({ | ||
| title: `GitHub issue ${actionText}：${issue?.title}`, | ||
| content: [ | ||
| [createText("链接："), createLink(issue!.html_url)], | ||
| [ | ||
| createText("作者："), | ||
| createLink(issue!.user!.html_url!, issue!.user!.login), | ||
| ], | ||
| [ | ||
| createText("指派："), | ||
| issue?.assignee | ||
| ? createLink(issue.assignee.html_url!, issue.assignee.login) | ||
| : createText("无"), | ||
| ], | ||
| [ | ||
| createText("标签："), | ||
| createText(issue?.labels?.map(({ name }) => name).join(", ") || "无"), | ||
| ], | ||
| [createText("里程碑："), createText(issue?.milestone?.title || "无")], | ||
| [createText("描述："), createText(issue?.body || "无")], | ||
| elements: [ | ||
| { | ||
| tag: "markdown", | ||
| content: [ | ||
| createContentItem("链接：", createLink(issue!.html_url)), | ||
| createContentItem("作者：", createUserLink(issue!.user!)), | ||
| createContentItem( | ||
| "指派：", | ||
| issue?.assignee ? createUserLink(issue.assignee) : "无", | ||
| ), | ||
| createContentItem( | ||
| "标签：", | ||
| issue?.labels?.map(({ name }) => name).join(", ") || "无", | ||
| ), | ||
| createContentItem("里程碑：", issue?.milestone?.title || "无"), | ||
| createContentItem("描述：", issue?.body || "无"), | ||
| ].join("\n"), | ||
| }, | ||
| ], | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Explore the structure of the file and look at the type definitions
cd /tmp && git ls-files .github/scripts/ 2>/dev/null | head -20 || echo "Finding the file..."
find . -name "transform-message.ts" -o -name "*.ts" -path "*/.github/scripts/*" 2>/dev/null | head -5Repository: Open-Source-Bazaar/feishu-action
Length of output: 58
🏁 Script executed:
#!/bin/bash
# Find the transform-message.ts file and examine its content
fd -t f "transform-message" . 2>/dev/null || find . -name "*transform*" -type f 2>/dev/null | grep -E "\.(ts|js)$" | head -10Repository: Open-Source-Bazaar/feishu-action
Length of output: 58
🏁 Script executed:
#!/bin/bash
# List all tracked files in the repository
git ls-files | head -30
echo "---"
# Search for TypeScript files
find . -type f -name "*.ts" | head -20Repository: Open-Source-Bazaar/feishu-action
Length of output: 504
🏁 Script executed:
#!/bin/bash
# Read the transform-message.ts file to understand the type definitions
cat -n .github/scripts/transform-message.tsRepository: Open-Source-Bazaar/feishu-action
Length of output: 10393
🏁 Script executed:
#!/bin/bash
# Check how the event handlers are called and if there's any type narrowing
grep -n "event_name\|eventHandlers\[" .github/scripts/transform-message.ts
# Also check how the event types are imported and what guarantees they have
grep -n "octokit\|webhook" .github/scripts/transform-message.tsRepository: Open-Source-Bazaar/feishu-action
Length of output: 608
🏁 Script executed:
#!/bin/bash
# Check the @octokit/openapi-types to understand the issue type
grep -r "webhook-issues-opened" . 2>/dev/null || echo "Type definition not in repo"
# Check package.json to see what version of octokit is used
cat package.json | grep -A5 -B5 "octokit"Repository: Open-Source-Bazaar/feishu-action
Length of output: 174
Use type narrowing or fix the type definition for the issue field.
The issue field is typed as optional but accessed with non-null assertions. While GitHub's webhook API guarantees the issue field is always present in issues events, the type definition doesn't reflect this. Either narrow the type when handling the issues event or mark issue as required (non-optional) since the GitHub issues webhook event always includes the issue object. This removes the need for non-null assertions and improves type safety.
🤖 Prompt for AI Agents
In .github/scripts/transform-message.ts around lines 125 to 146, the code uses
non-null assertions on issue (issue!, issue.user!) while the event typing marks
issue optional; remove the assertions and either narrow the type at the callsite
(add a runtime guard that returns/throws if issue is missing before accessing
properties) or update the event type definition so issue is non-optional for
issue events; after fixing the type, access issue fields without "!" and adjust
any downstream typing accordingly to eliminate unsafe assertions.
| const event = JSON.parse((await stdin()) || "{}") as GitHubAction; | ||
| const zh_cn = processEvent(event); | ||
| const result = processEvent(event); | ||
|
|
||
| if (zh_cn) console.log(JSON.stringify({ post: { zh_cn } })); | ||
| else | ||
| if (!result) { | ||
| throw new Error( | ||
| `Unsupported ${event.event_name} event & ${event.action} action` | ||
| `Unsupported ${event.event_name} event & ${event.action} action`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Dead code: The result validation is unnecessary.
The if (!result) check at line 273 is unreachable. The processEvent function either returns a valid result object or throws an error—it never returns a falsy value. This check can be safely removed.
Apply this diff to remove the dead code:
const event = JSON.parse((await stdin()) || "{}") as GitHubAction;
const result = processEvent(event);
-if (!result) {
- throw new Error(
- `Unsupported ${event.event_name} event & ${event.action} action`,
- );
-}
-
const card = {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const event = JSON.parse((await stdin()) || "{}") as GitHubAction; | |
| const zh_cn = processEvent(event); | |
| const result = processEvent(event); | |
| if (zh_cn) console.log(JSON.stringify({ post: { zh_cn } })); | |
| else | |
| if (!result) { | |
| throw new Error( | |
| `Unsupported ${event.event_name} event & ${event.action} action` | |
| `Unsupported ${event.event_name} event & ${event.action} action`, | |
| ); | |
| } | |
| const event = JSON.parse((await stdin()) || "{}") as GitHubAction; | |
| const result = processEvent(event); |
🤖 Prompt for AI Agents
In .github/scripts/transform-message.ts around lines 270 to 277, the if
(!result) check and its throw are dead code because processEvent either returns
a valid result or throws; remove the entire conditional block (the if statement
and its throw) so the code simply parses the event, calls processEvent, and uses
the returned result without the unreachable validation.
hu-qi
changed the title
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.