fix(claude-code): catch up to v2.1.117 + tighten clippy gate - #756
Merged
Conversation
…olor field) Closes #745. Triage of upstream Claude Code releases v2.1.91 through v2.1.117 (27-release window, 20 with notes) surfaced three structural changes that were causing agnix to false-positive on valid Claude Code configs: - Monitor tool (added v2.1.98) is now in KNOWN_AGENT_TOOLS, so an agent with tools: [Monitor] no longer triggers CC-AG-009/CC-AG-010. - xhigh effort level (added v2.1.111 for Opus 4.7) is now in VALID_EFFORT_VALUES (agent) and VALID_EFFORT_LEVELS (skill), so effort: xhigh no longer triggers CC-AG-014/CC-SK-018. - color field (display color, predates this window but exposed during upstream cross-check) is now a typed field on AgentSchema and listed in KNOWN_AGENT_FIELDS, so color: blue no longer triggers CC-AG-019. Documentation: bumped verified_on for affected rules to 2026-04-22; added a note on CC-AG-011 and CC-AG-019 that hooks/mcpServers in agent frontmatter also apply to --agent main-thread sessions (v2.1.116/v2.1.117); bumped RESEARCH-TRACKING.md Last Reviewed for Claude Code. Tests added: test_monitor_tool_accepted, test_color_field_accepted, and extended test_effort_valid_values / test_cc_sk_018_valid_effort_values to cover xhigh. Verified: cargo test --workspace passes (4500+ tests, 0 failures); cargo clippy --workspace --all-features -- -D warnings clean. Out of scope (will land in a follow-up PR before #747): output-style validator (new file type, .claude/output-styles/*.md with the keep-coding-instructions field added in v2.1.94).
The CI clippy step and local pre-push hook ran `cargo clippy --workspace --all-features` without `--all-targets`, so warnings in test, bench, and example code never failed the gate. Adding `--all-targets` exposed eight latent warnings, all fixed in this commit: - agnix-cli/src/main.rs: items_after_test_module - moved `mod resolve_fix_mode_tests` to end of file - agnix-cli/src/sarif.rs: unnecessary_literal_unwrap on intentional fallback test - marked `#[allow]` since the test exercises that exact path - agnix-cli/tests/kiro_fixture_inventory.rs: needless `<'a>` lifetime - dropped - agnix-core/benches/validation.rs: deprecated `criterion::black_box` -> `std::hint::black_box` - agnix-core/src/rules/cross_platform.rs: needless `&content` (already &str) - agnix-core/src/rules/project_level.rs: needless `&` in two `.contains()` calls - agnix-core/tests/api_contract.rs: `Ok(42).unwrap()` on intentional contract test - marked `#[allow]` - agnix-lsp/src/backend/tests.rs: unnecessary `(i + 2) as i32` cast - agnix-lsp/tests/lsp_integration.rs: unused `DiagnosticLevel` and `LintConfig` imports - removed Verified: `cargo clippy --workspace --all-targets --all-features -- -D warnings` passes with these changes.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates Claude Code support to version 2.1.117, introducing the Monitor tool, xhigh effort level, and color field. It also expands Clippy linting to include tests and benchmarks, resolving several latent warnings across the codebase. Review feedback highlights opportunities to improve the project's architecture by deduplicating effort level constants and configuration files to maintain a single source of truth, as well as increasing test coverage for the newly added Claude Code rules.
| "https://code.claude.com/docs/en/sub-agents" | ||
| ], | ||
| "verified_on": "2026-02-07", | ||
| "verified_on": "2026-04-22", |
Contributor
There was a problem hiding this comment.
The files crates/agnix-rules/rules.json and knowledge-base/rules.json are identical and are being manually synchronized in this PR (as noted in the PR description regarding the parity test). This violates the principle of having a single source of truth. Consider refactoring the project so that one file is the source of truth and the other is either a symbolic link or a generated copy, or have both components reference the same path.
References
- Instead of merging and deduplicating data from multiple sources, refactor the code to have a single source of truth to avoid duplication in the first place.
|
|
||
| /// Valid effort values per CC-AG-014 | ||
| const VALID_EFFORT_VALUES: &[&str] = &["low", "medium", "high", "max"]; | ||
| const VALID_EFFORT_VALUES: &[&str] = &["low", "medium", "high", "xhigh", "max"]; |
Contributor
There was a problem hiding this comment.
The VALID_EFFORT_VALUES list is identical to VALID_EFFORT_LEVELS defined in crates/agnix-core/src/schemas/skill.rs. To maintain a single source of truth, consider referencing the existing constant instead of duplicating the list.
Suggested change
| const VALID_EFFORT_VALUES: &[&str] = &["low", "medium", "high", "xhigh", "max"]; | |
| const VALID_EFFORT_VALUES: &[&str] = crate::schemas::skill::VALID_EFFORT_LEVELS; |
References
- Instead of merging and deduplicating data from multiple sources, refactor the code to have a single source of truth to avoid duplication in the first place.
Comment on lines
+3809
to
+3813
| assert_eq!( | ||
| d.iter().filter(|x| x.rule == "CC-AG-009").count(), | ||
| 0, | ||
| "Monitor tool should not trigger CC-AG-009 (added in Claude Code v2.1.98)" | ||
| ); |
Contributor
There was a problem hiding this comment.
The PR description mentions that the Monitor tool should no longer trigger CC-AG-009 or CC-AG-010. This test only verifies CC-AG-009. Consider adding a check for CC-AG-010 to ensure both conditions are covered.
assert_eq!(
d.iter().filter(|x| x.rule == "CC-AG-009").count(),
0,
"Monitor tool should not trigger CC-AG-009 (added in Claude Code v2.1.98)"
);
assert_eq!(
d.iter().filter(|x| x.rule == "CC-AG-010").count(),
0,
"Monitor tool should not trigger CC-AG-010"
);| #[test] | ||
| fn test_effort_valid_values() { | ||
| for e in &["low", "medium", "high", "max"] { | ||
| for e in &["low", "medium", "high", "xhigh", "max"] { |
Contributor
There was a problem hiding this comment.
Instead of hardcoding the list of valid effort values in the test, consider using the VALID_EFFORT_VALUES constant (or the shared VALID_EFFORT_LEVELS constant) to ensure consistency and reduce maintenance overhead.
Suggested change
| for e in &["low", "medium", "high", "xhigh", "max"] { | |
| for e in crate::schemas::skill::VALID_EFFORT_LEVELS { |
References
- Instead of merging and deduplicating data from multiple sources, refactor the code to have a single source of truth to avoid duplication in the first place.
| #[test] | ||
| fn test_cc_sk_018_valid_effort_values() { | ||
| for effort in &["low", "medium", "high", "max"] { | ||
| for effort in &["low", "medium", "high", "xhigh", "max"] { |
Contributor
There was a problem hiding this comment.
Instead of hardcoding the list of valid effort values in the test, consider using the VALID_EFFORT_LEVELS constant to ensure consistency and reduce maintenance overhead.
Suggested change
| for effort in &["low", "medium", "high", "xhigh", "max"] { | |
| for effort in crate::schemas::skill::VALID_EFFORT_LEVELS { |
References
- Instead of merging and deduplicating data from multiple sources, refactor the code to have a single source of truth to avoid duplication in the first place.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates agnix’s Claude Code validation baselines and schemas to match Claude Code v2.1.117 (fixing several false-positives), and hardens CI/local hooks by running Clippy across all targets/features so warnings in tests/benches/examples fail the build.
Changes:
- Extend Claude Code agent/skill validation to accept
Monitor,effort: xhigh, and agentcolor, plus corresponding tests/docs/rules metadata updates. - Tighten Clippy gating in CI and
pre-pushtocargo clippy --workspace --all-targets --all-features -- -D warnings. - Fix newly surfaced Clippy warnings across core/cli/lsp tests, benches, and docs.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/pre-push-rust | Run clippy with --all-targets --all-features before tests. |
| .github/workflows/ci.yml | Make CI clippy cover all targets/features and deny warnings. |
| crates/agnix-core/src/rules/agent.rs | Accept Monitor tool, xhigh effort, and color field; add regression tests. |
| crates/agnix-core/src/schemas/agent.rs | Add typed color field to agent schema. |
| crates/agnix-core/src/schemas/skill.rs | Add xhigh to valid skill effort levels. |
| crates/agnix-core/src/rules/skill/tests.rs | Extend CC-SK-018 tests to include xhigh. |
| crates/agnix-core/src/rules/cross_platform.rs | Minor refactor to avoid needless borrows. |
| crates/agnix-core/src/rules/project_level.rs | Minor refactor to avoid needless borrows in assertions. |
| crates/agnix-core/tests/api_contract.rs | Adjust contract test to satisfy clippy (with targeted allow). |
| crates/agnix-core/benches/validation.rs | Use std::hint::black_box to avoid deprecated criterion API. |
| crates/agnix-cli/src/sarif.rs | Tweak tests to satisfy clippy and pin fallback behavior. |
| crates/agnix-cli/tests/kiro_fixture_inventory.rs | Remove unnecessary lifetime to satisfy clippy. |
| crates/agnix-cli/src/main.rs | Move resolve_fix_mode tests to satisfy clippy module-order lint. |
| crates/agnix-lsp/src/backend/tests.rs | Remove unnecessary cast in LSP stress test versioning. |
| crates/agnix-lsp/tests/lsp_integration.rs | Remove unused imports surfaced by all-targets clippy. |
| knowledge-base/VALIDATION-RULES.md | Document xhigh and add notes about main-thread --agent behavior. |
| knowledge-base/RESEARCH-TRACKING.md | Update Claude Code “Last Reviewed” date to 2026-04-22. |
| knowledge-base/rules.json | Bump verified_on dates for affected Claude Code rules. |
| crates/agnix-rules/rules.json | Keep packaged rules.json in sync with knowledge-base rules. |
| CHANGELOG.md | Record the Claude Code catch-up and clippy gate tightening. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| use agnix_core::{CoreError, LintError, LintResult, ValidationError}; | ||
|
|
||
| // LintResult<T> must accept Ok values. | ||
| #[allow(clippy::unnecessary_literal_unwrap)] // intentional: contract test pinning the alias |
There was a problem hiding this comment.
The #[allow(clippy::unnecessary_literal_unwrap)] on the let ok: LintResult<u32> = Ok(42); line doesn’t suppress any lint (there’s no unwrap/unwrap_or call on that statement). Consider removing it (or moving the allow to the specific unwrap call only) to avoid misleading future readers about what’s being suppressed.
Suggested change
| #[allow(clippy::unnecessary_literal_unwrap)] // intentional: contract test pinning the alias |
| <a id="cc-ag-019"></a> | ||
| ### CC-AG-019 [LOW] Unknown Agent Frontmatter Field | ||
| **Requirement**: Agent frontmatter fields MAY be validated against known set | ||
| **Requirement**: Agent frontmatter fields MAY be validated against known set. As of Claude Code v2.1.117, agent-frontmatter `mcpServers` is also loaded for main-thread sessions launched via `--agent`, in addition to subagent spawning. |
There was a problem hiding this comment.
The added note in CC-AG-019 calls out mcpServers being loaded for --agent main-thread sessions, but this rule is about unknown agent frontmatter fields. Consider rephrasing the note to be about agent-frontmatter loading in general (with mcpServers as an example), or moving this mcpServers/--agent detail to a more directly related rule section, to keep the rule docs tightly aligned with what CC-AG-019 checks.
Suggested change
| **Requirement**: Agent frontmatter fields MAY be validated against known set. As of Claude Code v2.1.117, agent-frontmatter `mcpServers` is also loaded for main-thread sessions launched via `--agent`, in addition to subagent spawning. | |
| **Requirement**: Agent frontmatter fields MAY be validated against a known set. Recognized agent-frontmatter fields are loaded by Claude Code; for example, as of Claude Code v2.1.117, `mcpServers` is a supported agent-frontmatter field. |
Rust 1.95 / clippy 1.95 in CI flagged 11 more errors (pattern `refs.sort_by(|a, b| b.start_byte.cmp(&a.start_byte))` -> use `sort_by_key(|b| std::cmp::Reverse(b.start_byte))`) that the previous clippy bundle commit did not catch because the local toolchain was 1.92. 10 sites in agnix-core/src/fixes.rs and 1 in agnix-core/src/rules/xml.rs. Pure mechanical refactor, no behavior change. Verified: cargo clippy --workspace --all-targets --all-features -- -D warnings now passes; 3409 lib tests pass.
Collaborator
Author
|
Per-comment reply table for round 1
Plus two CI-blocking fixes that landed before the comments (not from reviewer feedback):
Pushing all of the above now. |
Reviewer feedback (PR #756 round 1): - Gemini #2: dedupe agent VALID_EFFORT_VALUES with skill VALID_EFFORT_LEVELS; agent.rs now imports from schemas::skill so the two stay in lock-step. - Gemini #3: extend test_monitor_tool_accepted to also cover CC-AG-010 (disallowedTools list) since both validators share KNOWN_AGENT_TOOLS. - Gemini #4: drop redundant test_effort_valid_values; the pre-existing test_cc_ag_014_all_valid_effort_values already iterates VALID_EFFORT_LEVELS. - Gemini #5: skill tests.rs now iterates VALID_EFFORT_LEVELS instead of hardcoding the list. - Copilot #6: drop redundant #[allow(unnecessary_literal_unwrap)] on the Ok(42) declaration line in api_contract.rs (the actual unwrap is on the next line and already has its own allow). - Copilot #7: move the v2.1.116/v2.1.117 --agent-main-thread scope note out of CC-AG-011 and CC-AG-019 (which check structure, not loading semantics); consolidate as a single scope note at the top of the SUBAGENTS section. CI fix (Linux/macOS only): - agnix-lsp/tests/lsp_integration.rs: gate `use agnix_core::{DiagnosticLevel, LintConfig}` with #[cfg(unix)] so it follows the unix-gated test that uses it. The import was removed by the clippy bundle (Windows-local clippy thought it was unused since the test doesn't compile on Windows), but Linux/macOS CI does compile the test and needed the import. Verified locally: cargo clippy --workspace --all-targets --all-features -- -D warnings clean; 3408 lib tests pass; rules.json parity test passes.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - **Docusaurus** - bumped `@docusaurus/core` and `@docusaurus/preset-classic` from 3.9.2 to 3.10.0. | ||
|
|
||
| ### Fixed | ||
| - **Claude Code v2.1.90 -> v2.1.117 catch-up** (#745) - `Monitor` (the new built-in tool added in v2.1.98 for streaming events from background scripts) is now in `KNOWN_AGENT_TOOLS`, so `tools: [Monitor]` in agent frontmatter no longer false-positives CC-AG-009/CC-AG-010. `xhigh` (the new effort level for Opus 4.7, added in v2.1.111) is now in `VALID_EFFORT_VALUES` and `VALID_EFFORT_LEVELS`, so `effort: xhigh` no longer false-positives CC-AG-014/CC-SK-018. The agent `color` field (display color for the task list and transcript) is now a typed field on `AgentSchema` and listed in `KNOWN_AGENT_FIELDS`, so `color: blue` no longer false-positives CC-AG-019. Documentation: bumped `verified_on` for CC-AG-009/010/011/014/019 and CC-SK-018 to 2026-04-22; added a note on CC-AG-011 and CC-AG-019 that those agent fields now also fire/load for `--agent` main-thread sessions (v2.1.116/v2.1.117); bumped `RESEARCH-TRACKING.md` "Last Reviewed" for Claude Code from 2026-02-05 to 2026-04-22. |
There was a problem hiding this comment.
CHANGELOG entry mentions xhigh being added to VALID_EFFORT_VALUES, but the implementation now uses schemas::skill::VALID_EFFORT_LEVELS for both agent and skill effort validation (and there is no VALID_EFFORT_VALUES symbol left in the codebase). Update this line to reference the actual constant(s) used so the changelog stays accurate.
…EFFORT_VALUES Per Copilot review (PR #756 round 2): the CHANGELOG entry referenced `VALID_EFFORT_VALUES` and CC-AG-011/CC-AG-019 notes that no longer exist after the round-1 dedup and the CC-AG-019 note rephrasing. Update to match the actual implementation: shared `schemas::skill::VALID_EFFORT_LEVELS` constant + scope note at the SUBAGENTS section header.
This was referenced Apr 22, 2026
avifenesh
added a commit
that referenced
this pull request
Apr 22, 2026
* feat(claude-code): add output-style validator (CC-OS-001..005) Surfaced during the Claude Code v2.1.117 triage (#745) but kept out of that PR per "no follow-ups" rule -- output-style files are a green-field new file type, not a fix. ## What's new - New \`FileType::ClaudeOutputStyle\` for \`.claude/output-styles/*.md\` and \`~/.claude/output-styles/*.md\` - New \`OutputStyleSchema\` and \`OutputStyleValidator\` in agnix-core - 5 new validation rules: - CC-OS-001 (LOW): Missing or empty \`description\` - CC-OS-002 (HIGH): \`keep-coding-instructions\` is not a YAML boolean (the spec requires bool; reject string \`"yes"\`, number \`1\`, \`null\`) - CC-OS-003 (MEDIUM): Unknown frontmatter key (signals typo or wrong file type -- valid keys are \`name\`, \`description\`, \`keep-coding-instructions\`) - CC-OS-004 (MEDIUM): Empty body after closing \`---\` (style with no instructions is dead config) - CC-OS-005 (LOW): \`name\` exceeds 64 characters (truncates in UI) - All rules: non-autofix (no obvious correct rewrite for any of them) - 6 fixtures (1 valid + 5 invalid, one per rule) Total rules: 399 -> 404. Validator count: 73 -> 74. FileType variants: 42 -> 43. ## Why now Claude Code added the \`keep-coding-instructions\` field in v2.1.94 and the output-style file type itself predates that. agnix has been silently ignoring these files. With the v2.1.117 catch-up complete (PR #756), this fills the last gap surfaced by that triage. ## Touchpoints Primary (12): file_types/{types,detection}, schemas/{output_style,mod}, rules/{output_style,mod}, registry, rules.json (both copies), VALIDATION-RULES.md, CHANGELOG, fixtures. Secondary (3): config.rs (\`RuleConfig.output_styles\`), config/rule_filter.rs (\`CC-OS-\` prefix branch), rule_parity.rs (3 separate insertions). Generated (forced regen): website/docs/rules/generated/cc-ag-{009,010,011,014,019}.md and cc-sk-018.md picked up the verified_on bumps from PR #756; cdx-pl-005 through cdx-pl-013 picked up other pre-existing drift. The \`docs_website_parity\` test forces regen on any rules.json edit. ## Verified locally - \`cargo check --workspace\`: clean - \`cargo test -p agnix-core --lib\`: 3440 passed (+32 new) - \`cargo test --workspace --all-targets\`: every suite ok (iai bench needs iai-callgrind-runner; pre-existing local env issue; CI has it) - \`cargo clippy --workspace --all-targets --all-features -- -D warnings\`: clean - \`cargo test -p agnix-rules --tests\`: 47 + 4 parity tests pass * style: cargo fmt the new output_style files CI's format check caught fmt drift in crates/agnix-core/src/{rules,schemas}/output_style.rs (the implementation didn't run `cargo fmt` before reporting done). Pure mechanical reformat, no behavior change. * fix: address review comments + add CC-OS-006 for parse errors Reviewer feedback (PR #757 round 1, all 9 comments addressed): Gemini medium x3: - output_style.rs: split CC-OS-002 (was overloaded) - parse errors now use new CC-OS-006 [HIGH] Invalid Output Style Frontmatter Syntax. CC-OS-002 back to its narrow scope (non-bool keep-coding-instructions). +3 tests. - schemas/output_style.rs: refactored manual serde_yaml::Mapping extraction into #[derive(Deserialize)] + #[serde(rename = "keep-coding-instructions")]. Added a tiny deserialize_with helper to preserve YAML null as Some(Value::Null) for CC-OS-002 detection (otherwise serde collapses null into None, indistinguishable from absent). - Removed stale comment about hyphens in serde rename. Copilot x6 (all about the missing categories.claude-output-styles entry): - Added claude-output-styles to the categories map in rules.json (both copies); regenerated website docs picked up the friendly label and the bumped categoryCount. - Reworded VALIDATION-RULES.md output-styles section: file type predates v2.1.94; v2.1.94 specifically added keep-coding-instructions. - Updated cli_integration.rs comment '~385 rules' -> '~405 rules'. Plus a fmt fix that landed in the previous commit (30accd2). Verified: cargo clippy --workspace --all-targets --all-features -- -D warnings clean; 3443 lib tests pass (+3 new for CC-OS-006); rules.json parity tests pass; docs regenerated to 405 pages. Total rules: 404 -> 405. Validators: 73 -> 74 (unchanged from previous push).
This was referenced Apr 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Triages issue #745 (the watcher-opened bump for Claude Code v2.1.90 → v2.1.117) and bundles a CI clippy hardening that was sitting in working-tree limbo.
Two semantic commits
1.
fix(claude-code): catch up to v2.1.117(#745)Triage of upstream Claude Code releases v2.1.91 through v2.1.117 (27-release window, 20 with notes) surfaced three structural changes that were causing agnix to false-positive on valid Claude Code configs:
Monitortool (added v2.1.98) → added toKNOWN_AGENT_TOOLS. Agents withtools: [Monitor]no longer trigger CC-AG-009 / CC-AG-010.xhigheffort level (added v2.1.111 for Opus 4.7) → added toVALID_EFFORT_VALUES(agent) andVALID_EFFORT_LEVELS(skill).effort: xhighno longer triggers CC-AG-014 / CC-SK-018.coloragent field → typed onAgentSchemaand listed inKNOWN_AGENT_FIELDS.color: blueno longer triggers CC-AG-019. Predates this window but exposed during upstream cross-check.Docs: bumped
verified_onfor CC-AG-009/010/011/014/019 and CC-SK-018 to 2026-04-22; added a note on CC-AG-011 / CC-AG-019 that those fields now also fire/load for--agentmain-thread sessions (v2.1.116 / v2.1.117); bumpedRESEARCH-TRACKING.md"Last Reviewed" for Claude Code from 2026-02-05 to 2026-04-22.Tests: added
test_monitor_tool_accepted,test_color_field_accepted; extendedtest_effort_valid_valuesandtest_cc_sk_018_valid_effort_valuesto coverxhigh.2.
chore(ci): tighten clippy gate to --all-targets.github/workflows/ci.ymlandscripts/pre-push-rustnow runcargo clippy --workspace --all-targets --all-features -- -D warnings, so warnings in test, bench, and example code fail the build. Adding--all-targetsexposed eight latent warnings, all fixed in the same commit (see the commit message for the full list).Out of scope
Output-style validator (new file type for
.claude/output-styles/*.mdwith thekeep-coding-instructionsfield added in v2.1.94) lands in the immediate follow-up PR before triage moves to issue #747 (Codex CLI). That PR is a green-field new validator, not a fix, so it benefits from its own review focus.Test plan
cargo test --workspace- 4500+ tests pass, 0 failurescargo clippy --workspace --all-targets --all-features -- -D warningscleanrules.jsonparity test passes (bothknowledge-base/rules.jsonandcrates/agnix-rules/rules.jsonsynced)Closes #745.