fix: channel-lark feature compilation errors#3032
fix: channel-lark feature compilation errors#3032gqf2008 wants to merge 1 commit intozeroclaw-labs:masterfrom
Conversation
Fix compilation errors when enabling the channel-lark feature: - Add platform field to LarkChannel struct (replaces use_feishu bool) - Add mention_only parameter to new_with_platform function - Implement from_lark_config and from_feishu_config methods - Fix test fixture with mention_only field Also fix import ordering in src/peripherals/serial.rs (cargo fmt). Validation: - cargo check --features channel-lark ✓ - cargo test --features channel-lark -- lark (48/48 passing) ✓ - cargo fmt --check ✓
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Platform Enum Refactoring src/channels/lark.rs |
Replaced use_feishu: bool field with platform: LarkPlatform enum. Added from_lark_config() and from_feishu_config() constructors for explicit platform selection. Updated from_config() to derive platform from legacy use_feishu flag. Modified new_with_platform() signature to include platform parameter. Tests updated to use new constructor methods. |
Import Reordering src/peripherals/serial.rs |
Relocated import statement; no functional changes. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
- [Bug]: cargo build --release --features channel-lark fails on v0.1.7 (and dev branch) — PR #1358 split broke LarkChannel #1991: Directly implements the
LarkPlatformenum and addsfrom_lark_config/from_feishu_configmethods that address the platform field replacement. - [Bug]: channel-lark feature fails to compile on Windows (v0.1.7 and main branch) #2230: The constructor method signatures and platform enum changes directly resolve compilation errors related to missing
mention_only/platformparameters and constructors. - [Bug]: Compilation error in lark.rs when building with channel-lark feature #1717: The platform enum replacement and new constructor methods address reported missing field/method signatures in compilation errors.
Suggested labels
size: S, risk: medium, channel, channel: lark, tests
Suggested reviewers
- JordanTheJet
- theonlyhennygod
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check | The description is mostly complete with a clear problem statement and validation evidence, but it is missing several required sections from the template (Labels, Change Metadata, Linked Issues, etc.). | Add required sections: Label Snapshot, Change Metadata, Linked Issue, Security Impact, Privacy and Data Hygiene, Compatibility, Human Verification, Side Effects, and Rollback Plan. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly summarizes the main change—fixing compilation errors for the channel-lark feature by refactoring the platform representation. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
Tip
Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Pull request overview
Fixes channel-lark feature build issues by refactoring Lark/Feishu endpoint selection into an explicit LarkPlatform on LarkChannel, updating constructors/config adapters accordingly, and aligning a test fixture with the new config shape.
Changes:
- Replace
use_feishu: boolonLarkChannelwithplatform: LarkPlatformand route base URLs / proxy keys via that enum. - Extend
new_with_platformto acceptmention_only, and add explicitfrom_lark_config/from_feishu_configconstructors (keepingfrom_configas the legacy compatibility path). - Apply
cargo fmtimport ordering adjustment insrc/peripherals/serial.rs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/peripherals/serial.rs |
Import ordering change from rustfmt (no behavior change). |
src/channels/lark.rs |
Platform refactor + constructor/config wiring updates; test fixture updated for mention_only. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub fn from_feishu_config(config: &crate::config::schema::FeishuConfig) -> Self { | ||
| let mut ch = Self::new_with_platform( | ||
| config.app_id.clone(), | ||
| config.app_secret.clone(), | ||
| config.verification_token.clone().unwrap_or_default(), | ||
| config.port, | ||
| config.allowed_users.clone(), | ||
| false, // mention_only not part of FeishuConfig | ||
| LarkPlatform::Feishu, |
There was a problem hiding this comment.
from_feishu_config hard-codes mention_only to false because FeishuConfig lacks this field. That creates a surprising feature-parity gap vs LarkConfig (and vs the legacy LarkConfig { use_feishu: true } path, which can enable mention_only). Consider adding mention_only to FeishuConfig as well, or otherwise documenting/centralizing this behavior so users can’t accidentally lose mention-only filtering when switching to [channels_config.feishu].
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/channels/lark.rs`:
- Around line 390-398: The Feishu conversion currently hard-codes mention_only:
false in from_feishu_config, which silently broadens group-response permissions;
update the config and constructor flow by adding a mention_only: Option<bool>
(or bool with default) to the FeishuConfig struct and propagate it into
LarkChannel creation inside from_feishu_config (i.e., replace the hard-coded
false with config.mention_only.unwrap_or(false) or config.mention_only); ensure
the FeishuConfig definition (schema::FeishuConfig) and any
deserialization/defaults are updated so callers can opt into mention-gating
rather than defaulting to permissive behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d28c1daa-796b-453d-b96a-aad9bedee3d1
📒 Files selected for processing (2)
src/channels/lark.rssrc/peripherals/serial.rs
| pub fn from_feishu_config(config: &crate::config::schema::FeishuConfig) -> Self { | ||
| let mut ch = Self::new_with_platform( | ||
| config.app_id.clone(), | ||
| config.app_secret.clone(), | ||
| config.verification_token.clone().unwrap_or_default(), | ||
| config.port, | ||
| config.allowed_users.clone(), | ||
| false, // mention_only not part of FeishuConfig | ||
| LarkPlatform::Feishu, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'FeishuConfig definition and related fields:\n'
rg -n -C4 --type rust '\bstruct\s+FeishuConfig\b|\bmention_only\b' src
printf '\nLark/Feishu constructor call sites:\n'
rg -n -C3 --type rust '\bfrom_feishu_config\s*\(|\bfrom_lark_config\s*\(|\bfrom_config\s*\(' srcRepository: zeroclaw-labs/zeroclaw
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '=== FeishuConfig struct definition ===\n'
sed -n '3207,3230p' src/config/schema.rs
printf '\n=== from_feishu_config wiring/calls ===\n'
rg -n 'from_feishu_config|LarkChannel::from_feishu_config' --type rust
printf '\n=== LarkConfig struct mention_only field ===\n'
sed -n '3175,3190p' src/config/schema.rsRepository: zeroclaw-labs/zeroclaw
Length of output: 2273
Feishu configuration lacks mention-gating support; don't silently default to permissive behavior.
FeishuConfig struct (lines 3207–3230 in src/config/schema.rs) does not have a mention_only field, whereas LarkConfig does. Line 397 of src/channels/lark.rs hard-codes mention_only: false when constructing a LarkChannel from Feishu config. This means every Feishu-configured bot will respond to all allowed group messages without requiring an explicit @-mention, inconsistent with Lark bots and contrary to the intent expressed in should_respond_in_group().
Per coding guidelines, "never silently broaden permissions/capabilities; document fallback behavior when fallback is intentional and safe." Either:
- Add
mention_onlyfield toFeishuConfigso users can opt into group-mention gating, or - Explicitly error if mention-gating is unsupported, rather than silently defaulting to the more permissive behavior.
If accepting the permissive default is intentional (e.g., Feishu API does not support group-mention filtering), document that trade-off explicitly in the code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/channels/lark.rs` around lines 390 - 398, The Feishu conversion currently
hard-codes mention_only: false in from_feishu_config, which silently broadens
group-response permissions; update the config and constructor flow by adding a
mention_only: Option<bool> (or bool with default) to the FeishuConfig struct and
propagate it into LarkChannel creation inside from_feishu_config (i.e., replace
the hard-coded false with config.mention_only.unwrap_or(false) or
config.mention_only); ensure the FeishuConfig definition (schema::FeishuConfig)
and any deserialization/defaults are updated so callers can opt into
mention-gating rather than defaulting to permissive behavior.
|
Thanks for the fix! This PR introduces Could you:
Happy to re-review once those are in. |
|
Fixed the issues raised by @rikitrader:
Verified: |
|
Thanks — I only noticed this PR after opening #3081, so I closed mine as a duplicate. Small correction to my follow-up note: the Concretely: I verified that behavior locally on macOS while testing a Feishu-enabled build. |
|
Closing as stale — this PR has had no activity for over 2 weeks and is significantly behind |
Fix compilation errors when enabling the channel-lark feature:
Also fix import ordering in src/peripherals/serial.rs (cargo fmt).
Validation
Summary by CodeRabbit