Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Kiro agent validation capabilities by introducing new rules that govern subagent invocation and tool scope consistency. It integrates a dedicated validator for these rules into the system, ensuring that Kiro agent configurations adhere to best practices for inter-agent communication and security. The changes also involve comprehensive updates to documentation and internal rule tracking to reflect the expanded validation coverage. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 463ff6554e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Code Review
This pull request introduces two new validation rules, KR-AG-006 and KR-AG-007, for Kiro agent configurations, along with a new KiroAgentValidator. The implementation of the validator and its associated tests are well-done. The changes also include updates to documentation, tests, and configuration files across the project to reflect the addition of these new rules.
I've found a couple of minor issues:
- The new Kiro agent rules are tied to the
kiro_steeringconfiguration flag, which could be confusing. They should probably have their ownkiro_agentsflag for independent control. - There are some inconsistencies in the reported number of auto-fixable rules across different documentation files.
Overall, this is a good contribution that expands the linter's capabilities. Addressing the minor issues will improve consistency and usability.
There was a problem hiding this comment.
Pull request overview
Adds new cross-agent validation for Kiro .kiro/agents/*.json files, introducing rules to detect unknown subagent references and tool-scope mismatches between an orchestrator agent and its referenced subagents. The PR also wires the new rules/category through registry/config/tests and refreshes generated docs/site data + rule counts.
Changes:
- Implement
KiroAgentValidatorwith KR-AG-006 and KR-AG-007 cross-file checks for.kiro/agents/*.json. - Register the validator for
FileType::KiroAgentand update rule parity/CI-gate tests + locales. - Refresh
rules.json/ docs / website data and update total rule & category counts across docs.
Reviewed changes
Copilot reviewed 26 out of 53 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| website/src/data/siteData.json | Updates website rule/category counts to include the new Kiro agent rules/category. |
| website/docs/rules/index.md | Updates rules index counts and adds KR-AG-006/007 entries to the rules table. |
| website/docs/rules/generated/oc-sk-001.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/oc-009.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/oc-008.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/oc-007.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/oc-006.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/oc-004.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/oc-003.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/oc-002.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/oc-001.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/kr-ag-007.md | Adds generated documentation page for KR-AG-007. |
| website/docs/rules/generated/kr-ag-006.md | Adds generated documentation page for KR-AG-006. |
| website/docs/rules/generated/cur-016.md | Refreshes generated rule doc content (Verified On, source URL, examples). |
| website/docs/rules/generated/cur-015.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/cur-014.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/cur-013.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/cur-012.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/cur-011.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/cur-010.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/cur-009.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/cur-008.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/cur-007.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/cur-006.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/cur-005.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/cur-004.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/cur-003.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/cur-002.md | Refreshes generated “Verified On” metadata. |
| website/docs/rules/generated/cur-001.md | Refreshes generated “Verified On” metadata. |
| scripts/check-rule-counts.py | Adds kiro-agents category label + mapping for count checks. |
| locales/zh-CN.yml | Adds localized messages/suggestions for KR-AG-006 and KR-AG-007. |
| locales/es.yml | Adds localized messages/suggestions for KR-AG-006 and KR-AG-007. |
| locales/en.yml | Adds localized messages/suggestions for KR-AG-006 and KR-AG-007. |
| knowledge-base/rules.json | Adds KR-AG-006/007 rule entries and a new kiro-agents category; bumps totals/last_updated. |
| knowledge-base/VALIDATION-RULES.md | Documents KR-AG-006/007 and updates totals/coverage tables. |
| knowledge-base/README.md | Updates referenced rule count to 233. |
| knowledge-base/INDEX.md | Updates referenced rule count and adds “Kiro Agents” to category breakdown. |
| editors/vscode/README.md | Updates referenced total rule count to 233. |
| crates/agnix-rules/rules.json | Updates packaged rules catalog with KR-AG-006/007 and category metadata. |
| crates/agnix-mcp/tests/mcp_tests.rs | Updates rule_count() expectation to 233. |
| crates/agnix-lsp/README.md | Updates referenced total rule count to 233. |
| crates/agnix-core/src/rules/mod.rs | Exposes new kiro_agent rules module. |
| crates/agnix-core/src/rules/kiro_agent.rs | Implements KiroAgentValidator and unit tests for KR-AG-006/007. |
| crates/agnix-core/src/registry.rs | Registers KiroAgentValidator for FileType::KiroAgent and updates registry counts/tests. |
| crates/agnix-core/src/config/rule_filter.rs | Adds KR-AG-* prefix handling for category enablement filtering. |
| crates/agnix-cli/tests/rule_parity.rs | Includes KR-AG-* in rule prefix extraction and adds fixture coverage mapping for kiro-agents. |
| crates/agnix-cli/tests/kiro_ci_gate.rs | Updates CI-gate assertions for Kiro surface to include KR-AG-* and .kiro/agents/*.json. |
| crates/agnix-cli/tests/docs_website_parity.rs | Updates Kiro docs parity checks to include KR-AG-* and .kiro/agents/*.json. |
| SPEC.md | Updates top-level counts and adds “Kiro Agents” to validated surfaces. |
| README.md | Updates Supported Tools table row for Kiro to include KR-AG-* and .kiro/agents/*.json. |
| CLAUDE.md | Updates referenced rule and validator counts. |
| CHANGELOG.md | Updates referenced rule counts for LSP/VS Code integration bullets. |
| AGENTS.md | Updates referenced rule and validator counts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Addressed all review feedback in commit 712637d.\n\nImplemented:\n- Prompt-only @subagent extraction (including *Prompt fields), with new tests for non-prompt false positives.\n- Case-insensitive .kiro/agents discovery for both fast-path and upward search.\n- allowedTools presence semantics: allowedTools is authoritative even when empty.\n- 1-based KR-AG line/column reporting + coverage test.\n- Dedicated rules.kiro_agents config toggle and independent gating tests.\n- Doc consistency updates (auto-fix totals = 96) and generator label fix for "Kiro Agents" in website index/category output.\n\nValidation rerun:\n- cargo test -p agnix-core kiro_agent -- --nocapture\n- cargo test -p agnix-core config::tests:: -- --nocapture\n- cargo test -p agnix-cli --test docs_website_parity -- --nocapture\n- cargo test -p agnix-cli --test rule_parity -- --nocapture\n- cargo test -p agnix-mcp --test mcp_tests rules_tests::test_rules_count -- --nocapture\n- python3 scripts/check-rule-counts.py |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 712637dc58
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Follow-up feedback addressed in commit 68b50c6:
Validation rerun:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 56 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn prompt_field_regex() -> &'static regex::Regex { | ||
| static RE: OnceLock<regex::Regex> = OnceLock::new(); | ||
| RE.get_or_init(|| { | ||
| regex::Regex::new(r#"(?is)"(?P<key>[A-Za-z0-9_]+)"\s*:\s*"(?P<value>(?:\\.|[^"\\])*)""#) | ||
| .expect("prompt field regex must compile") | ||
| }) | ||
| } | ||
|
|
There was a problem hiding this comment.
extract_prompt_agent_mentions() relies on prompt_field_regex() scanning the raw JSON text. Because this regex does not understand JSON structure, it can match sequences like "prompt": "..." that appear inside other string values (e.g., a prompt that contains a JSON example), which can produce false KR-AG-006/KR-AG-007 diagnostics.
Consider extracting prompt fields from the parsed serde_json::Value (walk objects/arrays and only consider keys that equal/ends_with "prompt") and then scanning those string values for @agent mentions. If you still want accurate locations, add a targeted span-finder for the specific prompt string token rather than a whole-file regex.
| let Ok(raw) = fs.read_to_string(&entry.path) else { | ||
| continue; | ||
| }; | ||
| let Ok(value) = serde_json::from_str::<Value>(&raw) else { | ||
| continue; | ||
| }; | ||
|
|
||
| let explicit_name = value.get("name").and_then(Value::as_str); | ||
| let fallback_name = entry.path.file_stem().and_then(|stem| stem.to_str()); |
There was a problem hiding this comment.
load_agent_index() skips agent files that fail read_to_string or JSON parsing. This means a referenced subagent file can exist on disk but still be treated as “unknown”, causing KR-AG-006 false positives (especially while a subagent file is mid-edit / temporarily invalid JSON).
To avoid incorrect KR-AG-006 reports, consider indexing agent names from the filename (and/or the raw "name" field when present) even when JSON parsing fails, and treat tool scope as unknown in that case (so KR-AG-007 can be skipped).
| let Ok(raw) = fs.read_to_string(&entry.path) else { | |
| continue; | |
| }; | |
| let Ok(value) = serde_json::from_str::<Value>(&raw) else { | |
| continue; | |
| }; | |
| let explicit_name = value.get("name").and_then(Value::as_str); | |
| let fallback_name = entry.path.file_stem().and_then(|stem| stem.to_str()); | |
| // Fallback agent name from the filename (used even if JSON reading/parsing fails). | |
| let fallback_name = entry.path.file_stem().and_then(|stem| stem.to_str()); | |
| // Read the file; on failure, still index the agent by filename with unknown tool scope. | |
| let raw = match fs.read_to_string(&entry.path) { | |
| Ok(raw) => raw, | |
| Err(_) => { | |
| if let Some(name) = fallback_name { | |
| let normalized_name = normalize_agent_name(name); | |
| if !normalized_name.is_empty() { | |
| let null_value = Value::Null; | |
| // Keep first observed definition for deterministic conflict handling. | |
| index | |
| .entry(normalized_name) | |
| .or_insert_with(|| AgentInfo { | |
| tools: extract_tools(&null_value), | |
| has_explicit_tool_scope: has_explicit_tool_scope(&null_value), | |
| }); | |
| } | |
| } | |
| continue; | |
| } | |
| }; | |
| // Parse the JSON; on failure, still index the agent by filename with unknown tool scope. | |
| let value = match serde_json::from_str::<Value>(&raw) { | |
| Ok(value) => value, | |
| Err(_) => { | |
| if let Some(name) = fallback_name { | |
| let normalized_name = normalize_agent_name(name); | |
| if !normalized_name.is_empty() { | |
| let null_value = Value::Null; | |
| // Keep first observed definition for deterministic conflict handling. | |
| index | |
| .entry(normalized_name) | |
| .or_insert_with(|| AgentInfo { | |
| tools: extract_tools(&null_value), | |
| has_explicit_tool_scope: has_explicit_tool_scope(&null_value), | |
| }); | |
| } | |
| } | |
| continue; | |
| } | |
| }; | |
| let explicit_name = value.get("name").and_then(Value::as_str); |
| #[test] | ||
| fn test_kr_ag_mentions_only_counted_from_prompt_fields() { | ||
| let temp = tempfile::TempDir::new().unwrap(); | ||
| let agents_dir = temp.path().join(".kiro").join("agents"); | ||
| fs::create_dir_all(&agents_dir).unwrap(); | ||
|
|
||
| let orchestrator = agents_dir.join("orchestrator.json"); | ||
| write_agent( | ||
| &orchestrator, | ||
| r#"{ | ||
| "name": "orchestrator", | ||
| "description": "Contact @missing-agent for docs", | ||
| "prompt": "Run local checks only" | ||
| }"#, | ||
| ); | ||
|
|
||
| let diagnostics = validate(&orchestrator); | ||
| assert!( | ||
| diagnostics | ||
| .iter() | ||
| .all(|diagnostic| diagnostic.rule != "KR-AG-006"), | ||
| "KR-AG-006 should ignore mentions outside prompt fields: {:?}", | ||
| diagnostics | ||
| ); | ||
| } |
There was a problem hiding this comment.
There’s no unit test covering the case where a prompt contains an embedded JSON/code sample that includes a "prompt": "..." snippet. With the current whole-file regex approach, that scenario is prone to false positives.
Add a regression test where the agent’s actual prompt string contains a JSON example mentioning @missing-agent, and assert KR-AG-006 does not fire based on that embedded snippet.
Summary
KiroAgentValidatorfor.kiro/agents/*.jsonand implementKR-AG-006/KR-AG-007FileType::KiroAgentand update rule/category parity wiring (rules.json, docs, locale keys, README tool surface)Validation
cargo test -p agnix-core kiro_agent -- --nocapturecargo test -p agnix-cli --test rule_parity -- --nocapturecargo test -p agnix-cli --test docs_website_parity -- --nocapturecargo test -p agnix-mcp --test mcp_tests rules_tests::test_rules_count -- --nocapturepython3 scripts/check-rule-counts.pyCloses #604