Skip to content

Commit a31cd58

Browse files
authored
feat(agnix-rules): derive tool mappings from rules.json (#176) (#178)
* feat(agnix-rules): derive tool mappings from rules.json at compile time - Enhanced build.rs to extract VALID_TOOLS and TOOL_RULE_PREFIXES from rules.json - VALID_TOOLS: unique tools from evidence.applies_to.tool field - TOOL_RULE_PREFIXES: mapping of (prefix, tool) for tool-specific rules - Added helper functions: valid_tools(), get_tool_for_prefix(), get_prefixes_for_tool() - Added is_valid_tool() and normalize_tool_name() for tool validation - All mappings generated at compile time with zero runtime cost * build: add agnix-rules as workspace dependency - Add agnix-rules to [workspace.dependencies] in root Cargo.toml - Add agnix-rules.workspace = true to agnix-core dependencies * fix(agnix-rules): exclude mixed-tool prefixes from mapping Prefixes where some rules have a tool and others have empty applies_to are now excluded from TOOL_RULE_PREFIXES. This prevents incorrect filtering where generic rules would be blocked. For example: CC-MEM- has some rules with "claude-code" and some with empty applies_to (making them generic), so CC-MEM- is not in the mapping. * refactor(config): use derived tool mappings from agnix-rules Replace hardcoded tool_mapping in is_rule_for_tools() with TOOL_RULE_PREFIXES derived from rules.json at compile time. - Use agnix_rules::TOOL_RULE_PREFIXES for prefix-to-tool lookup - Add backward compat: "copilot" alias for "github-copilot" - Maintain existing behavior for generic rules (apply to all tools) * refactor(cli): use agnix_rules::valid_tools() in rule_parity test Replace hardcoded tool list with valid_tools() derived from rules.json. This ensures the test automatically stays in sync with actual tool data. * test: add review-requested test coverage Add tests for critical and high severity review findings: - test_tool_rule_prefixes_consistency: verifies every tool in TOOL_RULE_PREFIXES is also in VALID_TOOLS (no orphaned entries) - test_is_tool_alias_case_sensitivity: verifies "Copilot" and "COPILOT" match "github-copilot" via case-insensitive alias - test_mixed_tool_prefix_as: verifies AS- returns None (all generic) - test_mixed_tool_prefix_cc_mem: verifies CC-MEM- returns None (mixed) - test_consistent_tool_prefix_cc_hk: verifies CC-HK- returns Some * fix: address all review findings (medium/low severity) - Add doc comment to is_tool_alias() explaining why only github-copilot has an alias (backward compatibility with configs using 'copilot') - Add comment noting TOOL_RULE_PREFIXES is small (~6 entries) so linear search is acceptable, but could use HashMap if it grows - Add tests for get_prefixes_for_tool() edge cases: - Empty string as tool (returns empty Vec) - Unknown tool (returns empty Vec) - Tool with multiple prefixes (claude-code returns multiple) - Add tests for is_tool_alias() edge cases: - Unknown alias returns false - Canonical name 'github-copilot' is NOT treated as alias of itself - Add tests for get_tool_for_prefix() edge cases: - Empty prefix returns None - Unknown prefix returns None - Add test verifying empty tool strings are treated as generic (not added to VALID_TOOLS or TOOL_RULE_PREFIXES) - Add clippy allow attribute to rule_parity.rs for test patterns * docs: add CHANGELOG entry for tool mappings derivation * style: apply cargo fmt * fix: address Gemini review feedback - Use rsplit_once in extract_rule_prefix for efficiency and to handle edge cases like trailing hyphens correctly - Make get_prefixes_for_tool case-insensitive for API consistency - Simplify test_is_tool_alias_case_sensitivity with loop iteration
1 parent 3969abf commit a31cd58

9 files changed

Lines changed: 660 additions & 19 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
9191
- Added validation pipeline tests for AGENTS.md path collection and files_checked counter (#83)
9292

9393
### Changed
94+
- Tool mappings derived from rules.json at compile time (#176)
95+
- VALID_TOOLS and TOOL_RULE_PREFIXES now extracted from rules.json evidence metadata
96+
- New helper functions in agnix-rules: valid_tools(), get_tool_for_prefix(), get_prefixes_for_tool()
97+
- Config tools array validation uses derived mappings instead of hardcoded list
98+
- Backward compatibility maintained with "copilot" alias for "github-copilot"
99+
- Zero runtime cost - all mappings resolved at compile time
94100
- Narrowed agnix-core public API surface (#85)
95101
- Made `parsers`, `rules`, `schemas`, and `file_utils` modules private
96102
- Re-exported `Validator` trait for custom validator implementations

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ keywords = ["agent", "linter", "claude", "skills", "mcp"]
1919
categories = ["development-tools", "command-line-utilities"]
2020

2121
[workspace.dependencies]
22+
# Internal crates
23+
agnix-rules = { path = "crates/agnix-rules", version = "0.1.0" }
24+
2225
# Core dependencies
2326
serde = { version = "1", features = ["derive"] }
2427
serde_json = "1"

crates/agnix-cli/tests/rule_parity.rs

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
//! 3. Covered by test fixtures in tests/fixtures/
77
//! 4. Have valid evidence metadata
88
9+
// Allow common test patterns that clippy flags but are intentional in tests
10+
#![allow(clippy::field_reassign_with_default)]
11+
912
use regex::Regex;
1013
use serde::Deserialize;
1114
use std::collections::{BTreeSet, HashMap};
@@ -666,13 +669,8 @@ fn test_evidence_test_coverage_accuracy() {
666669
fn test_applies_to_tool_values() {
667670
let rules_index = load_rules_json();
668671

669-
let valid_tools = [
670-
"claude-code",
671-
"cursor",
672-
"github-copilot",
673-
"windsurf",
674-
// Add more as needed
675-
];
672+
// Use valid_tools derived from rules.json at compile time
673+
let valid_tools = agnix_rules::valid_tools();
676674

677675
for rule in &rules_index.rules {
678676
if let Some(ref tool) = rule.evidence.applies_to.tool {
@@ -686,3 +684,47 @@ fn test_applies_to_tool_values() {
686684
}
687685
}
688686
}
687+
688+
// ============================================================================
689+
// Tool Mapping Consistency Tests (Review-requested coverage)
690+
// ============================================================================
691+
692+
#[test]
693+
fn test_tool_rule_prefixes_consistency() {
694+
// Every tool in TOOL_RULE_PREFIXES must also exist in VALID_TOOLS
695+
// This ensures no orphaned tools or prefixes exist
696+
let valid_tools = agnix_rules::valid_tools();
697+
698+
for (prefix, tool) in agnix_rules::TOOL_RULE_PREFIXES {
699+
assert!(
700+
valid_tools.contains(tool),
701+
"Tool '{}' from prefix '{}' is not in VALID_TOOLS. \
702+
TOOL_RULE_PREFIXES and VALID_TOOLS must be consistent.",
703+
tool,
704+
prefix
705+
);
706+
}
707+
}
708+
709+
#[test]
710+
fn test_is_tool_alias_case_sensitivity() {
711+
// Test that tool alias matching is case insensitive
712+
// "Copilot" (mixed case) and "COPILOT" (uppercase) should both
713+
// be recognized as valid tools via the alias mechanism
714+
715+
// The is_tool_alias function is private, but we can test through
716+
// LintConfig::is_rule_enabled which uses it internally
717+
718+
use agnix_core::LintConfig;
719+
720+
let aliases = ["Copilot", "COPILOT", "copilot"];
721+
for alias in aliases {
722+
let mut config = LintConfig::default();
723+
config.tools = vec![alias.to_string()];
724+
assert!(
725+
config.is_rule_enabled("COP-001"),
726+
"Alias '{}' should match 'github-copilot' and enable COP-* rules",
727+
alias
728+
);
729+
}
730+
}

crates/agnix-core/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ keywords = ["agent", "linter", "validation", "mcp", "skills"]
1212
categories = ["development-tools", "parser-implementations"]
1313

1414
[dependencies]
15+
agnix-rules.workspace = true
1516
serde.workspace = true
1617
serde_json.workspace = true
1718
serde_yaml.workspace = true

crates/agnix-core/src/config.rs

Lines changed: 70 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -332,26 +332,44 @@ impl LintConfig {
332332

333333
/// Check if a rule applies based on the tools array
334334
fn is_rule_for_tools(&self, rule_id: &str) -> bool {
335-
// Tool-specific rule prefixes and their corresponding tool names
336-
let tool_mapping: &[(&str, &[&str])] = &[
337-
("CC-", &["claude-code"]), // Claude Code specific rules
338-
("COP-", &["copilot"]), // GitHub Copilot specific rules
339-
("CUR-", &["cursor"]), // Cursor specific rules
340-
];
341-
342-
for (prefix, tool_names) in tool_mapping {
335+
// Use TOOL_RULE_PREFIXES derived from rules.json at compile time.
336+
// Note: TOOL_RULE_PREFIXES is small (~6 entries), so linear search is acceptable.
337+
// If this grows significantly larger, consider using a HashMap for O(1) lookups.
338+
for (prefix, tool) in agnix_rules::TOOL_RULE_PREFIXES {
343339
if rule_id.starts_with(prefix) {
344-
// Check if any of the required tools is in the tools list
345-
return tool_names
340+
// Check if the required tool is in the tools list (case-insensitive)
341+
// Also accept backward-compat aliases (e.g., "copilot" for "github-copilot")
342+
return self
343+
.tools
346344
.iter()
347-
.any(|t| self.tools.iter().any(|u| u.eq_ignore_ascii_case(t)));
345+
.any(|t| t.eq_ignore_ascii_case(tool) || Self::is_tool_alias(t, tool));
348346
}
349347
}
350348

351349
// Generic rules (AS-*, XML-*, REF-*, XP-*, AGM-*, MCP-*, PE-*) apply to all tools
352350
true
353351
}
354352

353+
/// Check if a user-provided tool name is a backward-compatible alias
354+
/// for the canonical tool name from rules.json.
355+
///
356+
/// Currently only "github-copilot" has an alias ("copilot"). This exists for
357+
/// backward compatibility: early versions of agnix used the shorter "copilot"
358+
/// name in configs, and we need to continue supporting that for existing users.
359+
/// The canonical names in rules.json use the full "github-copilot" to match
360+
/// the official tool name from GitHub's documentation.
361+
///
362+
/// Note: This function does NOT treat canonical names as aliases of themselves.
363+
/// For example, "github-copilot" is NOT an alias for "github-copilot" - that's
364+
/// handled by the direct eq_ignore_ascii_case comparison in is_rule_for_tools().
365+
fn is_tool_alias(user_tool: &str, canonical_tool: &str) -> bool {
366+
// Backward compatibility: accept short names as aliases
367+
match canonical_tool {
368+
"github-copilot" => user_tool.eq_ignore_ascii_case("copilot"),
369+
_ => false,
370+
}
371+
}
372+
355373
/// Check if a rule's category is enabled
356374
fn is_category_enabled(&self, rule_id: &str) -> bool {
357375
match rule_id {
@@ -1651,4 +1669,45 @@ exclude = []
16511669
// Other CC-* rules should still work
16521670
assert!(config.is_rule_enabled("CC-AG-001"));
16531671
}
1672+
1673+
// ===== is_tool_alias Edge Case Tests =====
1674+
1675+
#[test]
1676+
fn test_is_tool_alias_unknown_alias_returns_false() {
1677+
// Unknown aliases should return false
1678+
assert!(!LintConfig::is_tool_alias("unknown", "github-copilot"));
1679+
assert!(!LintConfig::is_tool_alias("gh-copilot", "github-copilot"));
1680+
assert!(!LintConfig::is_tool_alias("", "github-copilot"));
1681+
}
1682+
1683+
#[test]
1684+
fn test_is_tool_alias_canonical_name_not_alias_of_itself() {
1685+
// Canonical name "github-copilot" is NOT treated as an alias of itself.
1686+
// This is by design - canonical names match via direct comparison in
1687+
// is_rule_for_tools(), not through the alias mechanism.
1688+
assert!(!LintConfig::is_tool_alias(
1689+
"github-copilot",
1690+
"github-copilot"
1691+
));
1692+
assert!(!LintConfig::is_tool_alias(
1693+
"GitHub-Copilot",
1694+
"github-copilot"
1695+
));
1696+
}
1697+
1698+
#[test]
1699+
fn test_is_tool_alias_copilot_is_alias_for_github_copilot() {
1700+
// "copilot" is an alias for "github-copilot" (backward compatibility)
1701+
assert!(LintConfig::is_tool_alias("copilot", "github-copilot"));
1702+
assert!(LintConfig::is_tool_alias("Copilot", "github-copilot"));
1703+
assert!(LintConfig::is_tool_alias("COPILOT", "github-copilot"));
1704+
}
1705+
1706+
#[test]
1707+
fn test_is_tool_alias_no_aliases_for_other_tools() {
1708+
// Other tools have no aliases defined
1709+
assert!(!LintConfig::is_tool_alias("claude", "claude-code"));
1710+
assert!(!LintConfig::is_tool_alias("cc", "claude-code"));
1711+
assert!(!LintConfig::is_tool_alias("cur", "cursor"));
1712+
}
16541713
}

crates/agnix-rules/build.rs

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,13 @@
22
//!
33
//! Generates Rust code from rules.json at compile time.
44
//! Supports both local crate builds (crates.io) and workspace builds (development).
5+
//!
6+
//! Generated constants:
7+
//! - `RULES_DATA`: All rule (id, name) tuples
8+
//! - `VALID_TOOLS`: Unique tool names from evidence.applies_to.tool
9+
//! - `TOOL_RULE_PREFIXES`: Mapping of (prefix, tool) for tool-specific rules
510
11+
use std::collections::{BTreeMap, BTreeSet};
612
use std::env;
713
use std::fs;
814
use std::path::{Path, PathBuf};
@@ -156,10 +162,122 @@ fn main() {
156162
));
157163
}
158164

165+
generated_code.push_str("];\n\n");
166+
167+
// =========================================================================
168+
// Extract unique tools from evidence.applies_to.tool
169+
// =========================================================================
170+
let mut tools: BTreeSet<String> = BTreeSet::new();
171+
172+
for rule in rules_array {
173+
if let Some(tool) = rule
174+
.get("evidence")
175+
.and_then(|e| e.get("applies_to"))
176+
.and_then(|a| a.get("tool"))
177+
.and_then(|t| t.as_str())
178+
{
179+
if !tool.is_empty() {
180+
tools.insert(tool.to_string());
181+
}
182+
}
183+
}
184+
185+
// Generate VALID_TOOLS constant
186+
generated_code
187+
.push_str("/// Valid tool names derived from rules.json evidence.applies_to.tool.\n");
188+
generated_code.push_str("/// \n");
189+
generated_code
190+
.push_str("/// These are the tools that have at least one rule specifically for them.\n");
191+
generated_code.push_str("pub const VALID_TOOLS: &[&str] = &[\n");
192+
for tool in &tools {
193+
generated_code.push_str(&format!(" \"{}\",\n", escape_str(tool)));
194+
}
195+
generated_code.push_str("];\n\n");
196+
197+
// =========================================================================
198+
// Derive prefix-to-tool mappings from rule IDs
199+
// =========================================================================
200+
// Group rules by their prefix and track:
201+
// 1. Which tools are specified for rules with this prefix
202+
// 2. Whether any rule with this prefix has NO tool (making it generic)
203+
#[derive(Default)]
204+
struct PrefixInfo {
205+
tools: BTreeSet<String>,
206+
has_generic: bool, // true if any rule in this prefix has no tool specified
207+
}
208+
209+
let mut prefix_info: BTreeMap<String, PrefixInfo> = BTreeMap::new();
210+
211+
for rule in rules_array {
212+
let id = rule["id"].as_str().unwrap_or("");
213+
let tool = rule
214+
.get("evidence")
215+
.and_then(|e| e.get("applies_to"))
216+
.and_then(|a| a.get("tool"))
217+
.and_then(|t| t.as_str());
218+
219+
if let Some(prefix) = extract_rule_prefix(id) {
220+
let info = prefix_info.entry(prefix).or_default();
221+
if let Some(tool_name) = tool {
222+
if !tool_name.is_empty() {
223+
info.tools.insert(tool_name.to_string());
224+
} else {
225+
info.has_generic = true;
226+
}
227+
} else {
228+
// No tool specified = generic rule, applies to all tools
229+
info.has_generic = true;
230+
}
231+
}
232+
}
233+
234+
// Only include prefixes that:
235+
// 1. Have exactly one tool specified AND
236+
// 2. Have NO generic rules (all rules specify that tool)
237+
generated_code.push_str("/// Mapping of rule ID prefixes to their associated tools.\n");
238+
generated_code.push_str("/// \n");
239+
generated_code.push_str(
240+
"/// Derived from rules.json: for each prefix, this is the tool that all rules\n",
241+
);
242+
generated_code
243+
.push_str("/// with that prefix apply to. Only includes prefixes where ALL rules\n");
244+
generated_code
245+
.push_str("/// consistently specify the same tool (excludes generic prefixes).\n");
246+
generated_code.push_str("pub const TOOL_RULE_PREFIXES: &[(&str, &str)] = &[\n");
247+
248+
for (prefix, info) in &prefix_info {
249+
// Only include if: exactly one tool AND no generic rules
250+
if info.tools.len() == 1 && !info.has_generic {
251+
let tool = info.tools.iter().next().unwrap();
252+
generated_code.push_str(&format!(
253+
" (\"{}\", \"{}\"),\n",
254+
escape_str(prefix),
255+
escape_str(tool)
256+
));
257+
}
258+
}
159259
generated_code.push_str("];\n");
160260

161261
// Write to OUT_DIR
162262
let out_dir = env::var("OUT_DIR").unwrap();
163263
let dest_path = Path::new(&out_dir).join("rules_data.rs");
164264
fs::write(&dest_path, generated_code).expect("Failed to write generated rules");
165265
}
266+
267+
/// Extract the rule prefix from a rule ID.
268+
///
269+
/// Examples:
270+
/// - "CC-HK-001" -> "CC-HK-"
271+
/// - "COP-001" -> "COP-"
272+
/// - "AS-001" -> "AS-"
273+
fn extract_rule_prefix(rule_id: &str) -> Option<String> {
274+
// Find the last hyphen. If it exists and is followed by only digits,
275+
// we've found our prefix. This is more efficient than splitting into a vector
276+
// and correctly handles edge cases like trailing hyphens.
277+
if let Some((prefix, suffix)) = rule_id.rsplit_once('-') {
278+
if !suffix.is_empty() && suffix.chars().all(|c| c.is_ascii_digit()) {
279+
return Some(format!("{}-", prefix));
280+
}
281+
}
282+
None
283+
}

0 commit comments

Comments
 (0)