Extract shared bridge core helpers#3432
Conversation
Add integrations/bridge-core with shared pure helpers for env parsing, command parsing/action mapping, message splitting, runtime error compaction, active-turn detection, and preserved chat state. Re-export or wrap those helpers from Telegram, Feishu, WeCom, and Weixin bridge libs while keeping transport-specific identity, validation, keyboards, and protocol handling local. Add Weixin lib tests so its helper behavior is guarded.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request extracts shared helper functions from various chat bridges (Feishu, Telegram, WeCom, and Weixin) into a new centralized package, @codewhale/bridge-core, to reduce code duplication and improve maintainability. Unit tests have also been added to verify the shared functionality. The review feedback highlights three key areas for improvement: adding a length check in parseEnvText to prevent incorrectly stripping single-character quotes, using optional chaining on state in activeTurnBlock to avoid potential runtime errors when state is null, and restoring the original Telegram bridge behavior by adding 'channel' to directChatTypes in stripGroupPrefix.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if ( | ||
| (value.startsWith('"') && value.endsWith('"')) || | ||
| (value.startsWith("'") && value.endsWith("'")) | ||
| ) { | ||
| value = value.slice(1, -1); | ||
| } |
There was a problem hiding this comment.
In parseEnvText, if the value is a single quote or double quote character (e.g., " or '), the current logic will strip it to an empty string because startsWith and endsWith both match the same single character. Adding a length check ensures that only valid quoted strings (with at least an opening and a closing quote) are stripped.
| if ( | |
| (value.startsWith('"') && value.endsWith('"')) || | |
| (value.startsWith("'") && value.endsWith("'")) | |
| ) { | |
| value = value.slice(1, -1); | |
| } | |
| if ( | |
| value.length >= 2 && | |
| ((value.startsWith('"') && value.endsWith('"')) || | |
| (value.startsWith("'") && value.endsWith("'"))) | |
| ) { | |
| value = value.slice(1, -1); | |
| } |
| turnId: runningTurn.id || state.activeTurnId || "", | ||
| message: `Thread already has active turn ${ | ||
| runningTurn.id || state.activeTurnId || "(unknown)" | ||
| }. Wait for it to finish or send /interrupt.` |
There was a problem hiding this comment.
If state is explicitly passed as null to activeTurnBlock, accessing state.activeTurnId will throw a TypeError. Using optional chaining (state?.activeTurnId) prevents potential runtime crashes.
| turnId: runningTurn.id || state.activeTurnId || "", | |
| message: `Thread already has active turn ${ | |
| runningTurn.id || state.activeTurnId || "(unknown)" | |
| }. Wait for it to finish or send /interrupt.` | |
| turnId: runningTurn.id || state?.activeTurnId || "", | |
| message: `Thread already has active turn ${ | |
| runningTurn.id || state?.activeTurnId || "(unknown)" | |
| }. Wait for it to finish or send /interrupt.` |
| export function stripGroupPrefix(text, { chatType, requirePrefix, prefix }) { | ||
| const trimmed = String(text || "").trim(); | ||
| if (!trimmed) return { accepted: false, text: "" }; | ||
| if (!requirePrefix || !isGroupChat(chatType)) { | ||
| return { accepted: true, text: trimmed }; | ||
| } | ||
| const marker = prefix || "/cw"; | ||
| if (trimmed === marker) return { accepted: true, text: "/help" }; | ||
| if (trimmed.startsWith(`${marker} `)) { | ||
| return { accepted: true, text: trimmed.slice(marker.length).trim() }; | ||
| } | ||
| return { accepted: false, text: "" }; | ||
| return coreStripGroupPrefix(text, { | ||
| chatType, | ||
| requirePrefix, | ||
| prefix: prefix || "/cw", | ||
| directChatTypes: ["private"] | ||
| }); |
There was a problem hiding this comment.
In Telegram, chatType can be "private", "group", "supergroup", or "channel". The original implementation accepted messages without a prefix if they were not group chats (i.e., private or channel). By specifying only "private" in directChatTypes, the new implementation will incorrectly require a prefix for channel posts when requirePrefix is enabled. Adding "channel" to directChatTypes preserves the original behavior.
| export function stripGroupPrefix(text, { chatType, requirePrefix, prefix }) { | |
| const trimmed = String(text || "").trim(); | |
| if (!trimmed) return { accepted: false, text: "" }; | |
| if (!requirePrefix || !isGroupChat(chatType)) { | |
| return { accepted: true, text: trimmed }; | |
| } | |
| const marker = prefix || "/cw"; | |
| if (trimmed === marker) return { accepted: true, text: "/help" }; | |
| if (trimmed.startsWith(`${marker} `)) { | |
| return { accepted: true, text: trimmed.slice(marker.length).trim() }; | |
| } | |
| return { accepted: false, text: "" }; | |
| return coreStripGroupPrefix(text, { | |
| chatType, | |
| requirePrefix, | |
| prefix: prefix || "/cw", | |
| directChatTypes: ["private"] | |
| }); | |
| export function stripGroupPrefix(text, { chatType, requirePrefix, prefix }) { | |
| return coreStripGroupPrefix(text, { | |
| chatType, | |
| requirePrefix, | |
| prefix: prefix || "/cw", | |
| directChatTypes: ["private", "channel"] | |
| }); | |
| } |
Move the duplicated JSON thread-map store into bridge-core with bridge-specific options for message dedupe, Telegram action tokens, and WeCom private state-file modes. Keep thin local subclasses where startup-order tests and bridge exports expect ThreadStore names. Verification: - npm --prefix integrations/bridge-core run check && npm --prefix integrations/bridge-core test - npm --prefix integrations/telegram-bridge run check && npm --prefix integrations/telegram-bridge test - npm --prefix integrations/feishu-bridge run check && npm --prefix integrations/feishu-bridge test - npm --prefix integrations/wecom-bridge run check && npm --prefix integrations/wecom-bridge test - npm --prefix integrations/weixin-bridge run check && npm --prefix integrations/weixin-bridge test
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
1 participant
Goal
Consolidate duplicated bridge logic from the Telegram, Feishu, WeCom, and Weixin Node integrations into a shared
integrations/bridge-corepackage while keeping each transport's protocol-specific behavior local.Changes
integrations/bridge-corewith shared helpers for:envFirst,parseList,parseBool,parseEnvText)ThreadStorefor JSON thread-map persistence:integrations/telegram-bridge/src/lib.mjsintegrations/telegram-bridge/src/index.mjsintegrations/feishu-bridge/src/lib.mjsintegrations/feishu-bridge/src/index.mjsintegrations/wecom-bridge/src/lib.mjsintegrations/weixin-bridge/src/lib.mjsintegrations/weixin-bridge/src/index.mjsThreadStorecoverage.Verification
Results:
Risks / Follow-ups