feat: detect competing compression plugins during init - #86
Conversation
- Add check_competing_plugins() called at start of agentflare init - Scans agent config files for lex-temple, cc-md markers - Emits warning to stderr when competitor detected - AGENTFLARE_IGNORE_CONFLICTS=true to suppress - Closes ponytail PR audit ticket #72
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a preflight conflict check to the init flow in src/init.rs. A new check_competing_plugins function scans agent configuration files for known competitor plugin markers, warning on stderr if found, and can be disabled via an environment variable. ChangesCompeting plugin conflict detection
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/init.rs (2)
436-441: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueSubstring marker matching can false-positive.
Matching
markersvialower.contains(m)on the raw lowercased config content means any incidental substring (e.g. a path segment or comment containing"cc-md") triggers the warning, even without an actual competing plugin installed. Given this is only a warning (not a hard failure) the risk is low, but a more targeted check (e.g. looking for the marker as a JSON key/value or word-bounded match) would reduce noise.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/init.rs` around lines 436 - 441, The marker check in the config scan is too broad because `lower.contains(m)` on raw content can match incidental substrings and produce false warnings. Update the logic in `src/init.rs` around the `read_to_string`/`markers.iter().any(...)` block to use a more targeted match, such as word-bounded or JSON key/value-aware detection, so only actual plugin markers trigger the warning.
406-420: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRedundant work: env var and file reads repeated per competitor.
AGENTFLARE_IGNORE_CONFLICTSis checked insidescan_agent_configs, which is called once per competitor entry — so the env var lookup and (on a hit) the config file read happen multiple times perinitcall instead of once. Hoisting theAGENTFLARE_IGNORE_CONFLICTScheck and the config-file read up intocheck_competing_plugins(reading each relevant config file once, then matching all competitor markers against the cached content) avoids the duplicate I/O.♻️ Sketch of hoisting the read/env-check
fn check_competing_plugins(agent: &str) { + if std::env::var("AGENTFLARE_IGNORE_CONFLICTS").is_ok() { + return; + } let competitors: &[(&str, &[&str])] = &[ ("lex-temple", &["lex-temple", "lex_temple"]), ("cc-md", &["cc-md", "cc_md"]), ];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/init.rs` around lines 406 - 420, The repeated env-var lookup and config-file reads happen because check_competing_plugins calls scan_agent_configs once per competitor. Hoist the AGENTFLARE_IGNORE_CONFLICTS check into check_competing_plugins, read each relevant config file only once, cache its contents, and then match all competitor markers against that cached content before emitting the warning. Keep the existing scan_agent_configs behavior only if needed elsewhere, but avoid per-competitor I/O in check_competing_plugins.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/init.rs`:
- Around line 436-441: The marker check in the config scan is too broad because
`lower.contains(m)` on raw content can match incidental substrings and produce
false warnings. Update the logic in `src/init.rs` around the
`read_to_string`/`markers.iter().any(...)` block to use a more targeted match,
such as word-bounded or JSON key/value-aware detection, so only actual plugin
markers trigger the warning.
- Around line 406-420: The repeated env-var lookup and config-file reads happen
because check_competing_plugins calls scan_agent_configs once per competitor.
Hoist the AGENTFLARE_IGNORE_CONFLICTS check into check_competing_plugins, read
each relevant config file only once, cache its contents, and then match all
competitor markers against that cached content before emitting the warning. Keep
the existing scan_agent_configs behavior only if needed elsewhere, but avoid
per-competitor I/O in check_competing_plugins.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c21b5c44-5bdc-4b9b-835c-43a8678d9f5c
📒 Files selected for processing (1)
src/init.rs
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary by CodeRabbit