diff --git a/CHANGELOG.md b/CHANGELOG.md index 173f4624..20200177 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,13 @@ not per individual commit. fan-out. When the note or split names an SOP step to resume at, the orchestrator returns the results to that specialist to finish the SOP. A specialist that runs an SOP's independence step itself says so in its output. +- Specialist specs no longer grant any agent-spawning tool. Kiro CLI v3 and Claude Code + configs block the runtime's spawn tools explicitly, so a specialist cannot call one even when + a prompt asks it to. The block does not cover shell: a specialist with shell access could + still run `kiro-cli` or `claude`. The Kiro CLI v3 block uses `excludedTools`, tested with + kiro-cli 2.35.0. Kiro CLI v2 output is not covered yet: its `@builtin` still + includes the `subagent` tool. `k-code-review-workflow` step 6 now returns the adversarial + pass as a handoff for `k-architect` instead of skipping it. - Orchestrators delegate a step they can't do to the owning specialist and may run independent parts in parallel, including same-type agents. Parallel write agents must own disjoint files. - Agents ignore planted instructions; `.konductor/memory/` ranks with AGENTS.md, tracked or not, diff --git a/agent-sops/k-code-review-workflow.sop.md b/agent-sops/k-code-review-workflow.sop.md index c2328a2b..8d5a004e 100644 --- a/agent-sops/k-code-review-workflow.sop.md +++ b/agent-sops/k-code-review-workflow.sop.md @@ -84,11 +84,12 @@ Validate each finding to eliminate false positives. When in doubt, reject — fa ### 6. Adversarial Review -If `k-adversarial-pull-request-review` is available, spawn `k-architect` in adversarial mode with it, passing the same diff used in Step 1 as `diff_input`, to surface gaps missed by standard review. +Spawn `k-architect` in adversarial mode with `k-adversarial-pull-request-review`, passing the same diff used in Step 1 as `diff_input`, to surface gaps missed by standard review. If you cannot spawn `k-architect`, return step 6 as a handoff naming `k-architect`, `k-adversarial-pull-request-review`, this SOP, and step 6 as the step to resume at, per the core rules' specialist handoffs. Include the SOP parameters (`source_dir`, `review_type`, `output_file`, `cr_url` if given, and the base branch Step 1 resolved), the Step 1 diff or the command that reproduces it, the Step 2 file counts by category, and the Step 5 consolidated findings list in the handoff so the resumed run can merge into them and write the report. If the list is long, write it to `.konductor/handoff/k-developer-code-review.md` and name that path, per the `delegation-protocol` skill's Handoff Pattern. **Constraints:** -- You MUST skip this step if `k-adversarial-pull-request-review` is unavailable, or if `review_type` is `frontend` only (adversarial review targets backend/infra gaps) +- If you were resumed at step 6 with `k-architect`'s findings, You MUST NOT spawn or hand off again. Apply the false-positive, merge and no-re-report constraints below to those findings and the Step 5 list from the handoff, then continue to Step 7. +- You MUST skip this step only if `review_type` is `frontend` only (adversarial review targets backend/infra gaps) - You MUST pass the same diff used in Step 1 as `diff_input` - If the k-code-review-workflow was invoked with a CR URL, You MUST also pass it to `k-adversarial-pull-request-review` as its `pr_url` parameter — that is the name that SOP declares, and passing `cr_url` instead leaves it unset so the review degrades to diff-only. Do not send it to `adversarial-cr-review`, a different SOP that declares `cr_url`; on agents where both are loadable, name the target by filename. - You MUST apply the same false-positive criteria from Step 5 to adversarial findings before merging them diff --git a/agents/k-architect.agent-spec.json b/agents/k-architect.agent-spec.json index 5d503fc2..e4d7508f 100644 --- a/agents/k-architect.agent-spec.json +++ b/agents/k-architect.agent-spec.json @@ -70,15 +70,12 @@ }, "context": { "contextNames": ["k-core-rules.md"] - }, - "subagents": { - "agentNames": ["k-developer", "k-quality-assurance"] } }, "clientConfig": { "kiroCli": { "welcomeMessage": "I'm your solutions architect. I produce system designs, API specifications, data models, and threat models, validating each artifact before it hands off to implementation. Bring me a requirement or a design problem and I'll work the architecture through with you.\n\nBuilt-in workflows — type any to run it:\n /k-design-doc-creation — produce a design doc ready for principal-engineer review\n /k-existing-design-review — review existing design artifacts across 10 dimensions before implementation\n /k-principal-engineer-design-review — run a pre-submission quality gate on a design doc\n /k-adversarial-pull-request-review — run an adversarial review that catches what standard code review misses", - "tools": ["@builtin", "subagent", "@aws-mcp"], + "tools": ["@builtin", "@aws-mcp"], "allowedTools": [ "fs_read", "fs_write", @@ -89,12 +86,6 @@ "@aws-mcp/aws___list_regions", "@aws-mcp/aws___get_regional_availability" ], - "toolsSettings": { - "subagent": { - "availableAgents": ["k-developer", "k-quality-assurance"], - "trustedAgents": ["k-developer", "k-quality-assurance"] - } - }, "hooks": {}, "resources": [ "file://AGENTS.md", @@ -113,8 +104,7 @@ "WebSearch", "TodoWrite", "Skill", - "mcp__aws-mcp__*", - "Agent(k-developer, k-quality-assurance)" + "mcp__aws-mcp__*" ], "skills": [ "constraints", diff --git a/agents/k-developer.agent-spec.json b/agents/k-developer.agent-spec.json index faeabb77..d9c91c02 100644 --- a/agents/k-developer.agent-spec.json +++ b/agents/k-developer.agent-spec.json @@ -64,7 +64,7 @@ "clientConfig": { "kiroCli": { "welcomeMessage": "I'm your development engineer. I break features into tasks, implement backend and frontend code, review my own changes, validate infrastructure, and track progress. Hand me a spec or a feature and I'll build it.\n\nBuilt-in workflows — type any to run it:\n /k-code-cleanup — strip AI-generated slop from a branch before raising a CR\n /k-pre-cr-critique — run a fast, read-only pre-CR critique of local changes\n /k-codebase-analysis — produce a comprehensive codebase analysis report with diagrams\n /k-code-review-workflow — run a full multi-skill code review consolidated into one report", - "tools": ["@builtin", "subagent", "@aws-mcp"], + "tools": ["@builtin", "@aws-mcp"], "allowedTools": [ "fs_read", "fs_write", @@ -75,16 +75,6 @@ "@aws-mcp/aws___list_regions", "@aws-mcp/aws___get_regional_availability" ], - "toolsSettings": { - "subagent": { - "availableAgents": [ - "k-quality-assurance" - ], - "trustedAgents": [ - "k-quality-assurance" - ] - } - }, "hooks": {}, "resources": [ "file://AGENTS.md", diff --git a/agents/k-media-analyzer.agent-spec.json b/agents/k-media-analyzer.agent-spec.json index c9d3e1cc..e8f1263a 100644 --- a/agents/k-media-analyzer.agent-spec.json +++ b/agents/k-media-analyzer.agent-spec.json @@ -23,12 +23,6 @@ "welcomeMessage": "I interpret media that raw-text tools can't read — PDFs, images, and diagrams. Give me a file and tell me what to extract or describe.", "tools": ["@builtin"], "allowedTools": ["fs_read", "web_fetch"], - "toolsSettings": { - "subagent": { - "trustedAgents": [], - "availableAgents": [] - } - }, "hooks": {}, "mcpServers": {}, "resources": [ diff --git a/agents/k-product-manager.agent-spec.json b/agents/k-product-manager.agent-spec.json index b288c328..ed8462db 100644 --- a/agents/k-product-manager.agent-spec.json +++ b/agents/k-product-manager.agent-spec.json @@ -38,14 +38,8 @@ "clientConfig": { "kiroCli": { "welcomeMessage": "I'm your product manager. I turn ideas into user stories, requirements summaries, and decision research, validating each artifact before handoff. Tell me the problem you're solving and who it's for.", - "tools": ["@builtin", "subagent"], + "tools": ["@builtin"], "allowedTools": ["fs_read", "fs_write"], - "toolsSettings": { - "subagent": { - "trustedAgents": [], - "availableAgents": [] - } - }, "hooks": {}, "mcpServers": {}, "resources": [ diff --git a/agents/k-quality-assurance.agent-spec.json b/agents/k-quality-assurance.agent-spec.json index 4de25062..5625d7d5 100644 --- a/agents/k-quality-assurance.agent-spec.json +++ b/agents/k-quality-assurance.agent-spec.json @@ -35,14 +35,8 @@ "clientConfig": { "kiroCli": { "welcomeMessage": "I'm your QA engineer. I analyze test coverage, plan end-to-end test strategies, generate security tests, and validate UI text. Point me at the code or feature you want tested.\n\nBuilt-in workflows — type any to run it:\n /k-test-coverage-review — review test coverage and plan E2E and security tests", - "tools": ["@builtin", "subagent"], + "tools": ["@builtin"], "allowedTools": ["fs_read", "fs_write", "shell"], - "toolsSettings": { - "subagent": { - "trustedAgents": ["k-developer", "k-browser"], - "availableAgents": ["k-developer", "k-browser"] - } - }, "hooks": {}, "mcpServers": {}, "resources": [ @@ -61,7 +55,6 @@ "WebFetch", "WebSearch", "TodoWrite", - "Agent(k-developer, k-browser)", "Skill" ], "skills": [ diff --git a/agents/k-researcher.agent-spec.json b/agents/k-researcher.agent-spec.json index 0864e1db..db64c9bb 100644 --- a/agents/k-researcher.agent-spec.json +++ b/agents/k-researcher.agent-spec.json @@ -30,14 +30,8 @@ "clientConfig": { "kiroCli": { "welcomeMessage": "I gather information — external documentation, web resources, AWS docs, and (when configured) Slack. Ask me a question and I'll research it and report back with sources.", - "tools": ["@builtin", "subagent"], + "tools": ["@builtin"], "allowedTools": ["fs_read"], - "toolsSettings": { - "subagent": { - "availableAgents": [], - "trustedAgents": [] - } - }, "resources": [ "file://AGENTS.md", "skill://.kiro/skills/ws-*/SKILL.md" diff --git a/agents/k-tpm.agent-spec.json b/agents/k-tpm.agent-spec.json index 27d905bb..bbfd3a31 100644 --- a/agents/k-tpm.agent-spec.json +++ b/agents/k-tpm.agent-spec.json @@ -37,25 +37,12 @@ "kiroCli": { "welcomeMessage": "I'm your technical program manager. I coordinate across projects — timelines, status reporting, risk tracking, and program-level decision documents. Tell me the program or the coordination problem.", "tools": [ - "@builtin", - "subagent" + "@builtin" ], "allowedTools": [ "fs_read", "fs_write" ], - "toolsSettings": { - "subagent": { - "trustedAgents": [ - "k-developer", - "k-researcher" - ], - "availableAgents": [ - "k-developer", - "k-researcher" - ] - } - }, "hooks": {}, "mcpServers": {}, "resources": [ diff --git a/cli/konductor-rs/src/cli/synth/claude.rs b/cli/konductor-rs/src/cli/synth/claude.rs index 37952e25..ce45c2af 100644 --- a/cli/konductor-rs/src/cli/synth/claude.rs +++ b/cli/konductor-rs/src/cli/synth/claude.rs @@ -21,7 +21,7 @@ use super::content_writers::{write_all_context, write_all_skills, write_all_sops use super::kiro_cli_v2::write_sop_scopes_sidecar; #[cfg(test)] use super::kiro_cli_v2::SOP_SCOPES_SIDECAR_FILE; -use super::model::CanonicalModel; +use super::model::{is_specialist, CanonicalModel}; #[cfg(test)] use super::model::{AuxiliaryFile, ContextDef, SkillDef, SopDef}; use super::parser::{ClaudeCliConfig, McpServerDef}; @@ -74,6 +74,8 @@ struct ClaudeAgentFrontmatter<'a> { skills: &'a [String], #[serde(rename = "allowedTools", skip_serializing_if = "is_empty_str_slice")] allowed_tools: &'a [String], + #[serde(rename = "disallowedTools", skip_serializing_if = "<[_]>::is_empty")] + disallowed_tools: &'a [&'static str], #[serde(skip_serializing_if = "IndexMap::is_empty")] hooks: IndexMap, #[serde(rename = "mcpServers", skip_serializing_if = "Vec::is_empty")] @@ -339,6 +341,16 @@ impl HarnessTransformer for ClaudeTransformer { } } +/// Claude Code tools that start another agent. `Task` is the older name +/// of `Agent`. +const SPAWN_TOOLS: [&str; 3] = ["Agent", "Task", "Workflow"]; + +/// Matches a bare spawn tool or its scoped form, e.g. `Agent(k-developer)`. +pub(crate) fn is_spawn_tool(tool: &str) -> bool { + let base = tool.split('(').next().unwrap_or(tool).trim(); + SPAWN_TOOLS.contains(&base) +} + const CLAUDE_MODEL_FAMILIES: &[&str] = &["opus", "sonnet", "haiku", "fable"]; const CLAUDE_CODE_MODEL_ALIASES: &[&str] = &["inherit", "sonnet", "opus", "haiku"]; @@ -461,6 +473,9 @@ fn is_date(part: &str) -> bool { /// `merge_mcp_servers` (see that function's own doc comment for the /// precedence rule) before rendering, so the frontmatter's `mcpServers:` /// key reflects both sources rather than only the Claude-specific one. +/// +/// For a specialist (`k-*`), spawn tools are dropped from `tools`; with no +/// `tools` list, `disallowedTools` denies them instead. fn render_agent_md( name: &str, config: &super::parser::AgentConfig, @@ -481,13 +496,29 @@ fn render_agent_md( let model = claude_model_id(&config.model); let merged_mcp_servers = merge_mcp_servers(mcp_registry, &claude.mcp_servers); + let specialist = is_specialist(name); + let tools: Option> = claude.tools.as_ref().map(|tools| { + tools + .iter() + .filter(|tool| !(specialist && is_spawn_tool(tool))) + .cloned() + .collect() + }); + // A specialist with no `tools` list inherits every default tool, + // spawn tools included, so it needs an explicit deny instead. + let disallowed_tools: &[&str] = if specialist && tools.is_none() { + &SPAWN_TOOLS + } else { + &[] + }; let frontmatter = ClaudeAgentFrontmatter { name, description: &config.description, model: &model, - tools: claude.tools.as_deref(), + tools: tools.as_deref(), skills: &claude.skills, allowed_tools: &claude.allowed_tools, + disallowed_tools, hooks: hooks_to_yaml(&claude.hooks), mcp_servers: render_mcp_servers(&merged_mcp_servers), }; @@ -934,6 +965,67 @@ mod tests { ); } + fn render_frontmatter(name: &str, tools: Option>) -> serde_yaml::Value { + let claude = ClaudeCliConfig { + tools: tools.map(|t| t.into_iter().map(String::from).collect()), + ..Default::default() + }; + let agent = agent_with_claude(name, claude); + let rendered = render_agent_md( + name, + &agent.config, + agent.client_config.claude_cli.as_ref().unwrap(), + &[], + &[], + &HashSet::new(), + &BTreeMap::new(), + ) + .unwrap(); + let frontmatter = rendered.splitn(3, "---\n").nth(1).unwrap(); + serde_yaml::from_str(frontmatter).unwrap() + } + + #[test] + fn render_agent_md_strips_spawn_tools_from_specialist() { + let parsed = render_frontmatter( + "k-example", + Some(vec![ + "Read", + "Agent(k-developer, k-browser)", + "Task", + "Workflow", + "Skill", + ]), + ); + assert_eq!( + parsed["tools"], + serde_yaml::from_str::("[Read, Skill]").unwrap() + ); + assert!(parsed.get("disallowedTools").is_none()); + } + + #[test] + fn render_agent_md_denies_spawn_tools_for_specialist_without_tools_list() { + let parsed = render_frontmatter("k-example", None); + assert!(parsed.get("tools").is_none()); + assert_eq!( + parsed["disallowedTools"], + serde_yaml::from_str::("[Agent, Task, Workflow]").unwrap() + ); + } + + #[test] + fn render_agent_md_keeps_spawn_tools_for_orchestrator() { + let parsed = render_frontmatter("konductor", Some(vec!["Workflow", "Agent(k-developer)"])); + assert_eq!( + parsed["tools"], + serde_yaml::from_str::("[Workflow, Agent(k-developer)]").unwrap() + ); + assert!(render_frontmatter("konductor", None) + .get("disallowedTools") + .is_none()); + } + /// Regression guard: an agent whose `claudeCli` sets /// `allowedTools`/`hooks`/`mcpServers` must render all three as /// their own frontmatter keys, grounded against the real Claude Code @@ -1031,7 +1123,7 @@ mod tests { ) .unwrap(); - assert!(!rendered.contains("allowedTools:"), "got: {rendered}"); + assert!(!rendered.contains("\nallowedTools:"), "got: {rendered}"); assert!(!rendered.contains("hooks:"), "got: {rendered}"); assert!(!rendered.contains("mcpServers:"), "got: {rendered}"); } diff --git a/cli/konductor-rs/src/cli/synth/kiro_cli_v3.rs b/cli/konductor-rs/src/cli/synth/kiro_cli_v3.rs index 5fa9495d..fcc4c417 100644 --- a/cli/konductor-rs/src/cli/synth/kiro_cli_v3.rs +++ b/cli/konductor-rs/src/cli/synth/kiro_cli_v3.rs @@ -81,7 +81,7 @@ use super::kiro_cli_v2::{ AGENTS_CONTENT_TYPE_DIR, CONTEXT_CONTENT_TYPE_DIR, SKILLS_CONTENT_TYPE_DIR, SOPS_CONTENT_TYPE_DIR, }; -use super::model::CanonicalModel; +use super::model::{is_specialist, CanonicalModel}; use super::parser::KiroCliConfig; use super::path_safety::reject_unsafe_agent_name; use super::staging::stage_content_type; @@ -95,15 +95,16 @@ use super::HarnessTransformer; /// unified permissions block"), replaced by `permissions`. /// /// V3-only fields the agent-config page's New Fields Reference Table -/// documents (`excludedTools`, `includeMcpJson`, `includePowers`) are -/// deliberately NOT emitted here: the source agent-spec schema -/// (`KiroCliConfig`) has no corresponding field to derive them from -/// today, and inventing a default would violate the "implement only -/// what's confirmed" contract this module is built under. A future -/// agent-spec schema change adding one of these would need a -/// corresponding `KiroCliConfig` field and a matching addition here, not -/// a fabricated default. This mirrors the existing project stance already -/// on record for these three fields. +/// documents (`includeMcpJson`, `includePowers`) are deliberately NOT +/// emitted here: the source agent-spec schema (`KiroCliConfig`) has no +/// corresponding field to derive them from today, and inventing a default +/// would violate the "implement only what's confirmed" contract this +/// module is built under. A future agent-spec schema change adding one of +/// these would need a corresponding `KiroCliConfig` field and a matching +/// addition here, not a fabricated default. +/// +/// `excludedTools` is emitted for specialists only, with the fixed +/// `SPECIALIST_EXCLUDED_TOOLS` list (see that constant). /// /// `welcomeMessage` is emitted when the source spec's `clientConfig.kiroCli` /// supplies it (`KiroCliConfig::welcome_message`), and omitted when absent. @@ -119,6 +120,8 @@ struct KiroV3AgentFile<'a> { #[serde(rename = "welcomeMessage", skip_serializing_if = "Option::is_none")] welcome_message: Option<&'a str>, tools: Vec, + #[serde(rename = "excludedTools", skip_serializing_if = "Vec::is_empty")] + excluded_tools: Vec<&'static str>, permissions: PermissionsBlock, #[serde(rename = "mcpServers")] mcp_servers: serde_json::Value, @@ -409,7 +412,7 @@ impl HarnessTransformer for KiroCliV3Transformer { /// collective markers (`@builtin`, `*`), and any server-scoped /// `@` entry, none of which this module's four source /// pages confirm a rename for. -fn map_tool_tag(id: &str) -> String { +pub(crate) fn map_tool_tag(id: &str) -> String { TOOL_ID_TO_TAG .iter() .find(|(old, _)| *old == id) @@ -1159,6 +1162,18 @@ fn convert_hook_entry( Some(serde_json::Value::Object(doc)) } +pub(crate) const SUBAGENT_CAPABILITY: &str = "subagent"; + +/// Dropping the `subagent` tag is not enough: `@builtin` also carries +/// `run_workflow`, which runs any agent via `agent://`, and no +/// permission rule gates it. `excludedTools` removes both and holds under +/// `--trust-all-tools` (checked against kiro-cli 2.35.0, engine v3). +const SPECIALIST_EXCLUDED_TOOLS: [&str; 2] = ["invoke_sub_agent", "run_workflow"]; + +pub(crate) fn is_subagent_tag(tag: &str) -> bool { + tag == SUBAGENT_CAPABILITY || tag.starts_with("subagent/") +} + /// Builds the on-disk JSON shape for one V3 agent from its config and /// `KiroCliConfig`. `tools`/`permissions`/`hooks` are the three fields /// that differ from `kiro_cli_v2.rs::render_agent_file` (see @@ -1218,8 +1233,14 @@ fn render_agent_file<'a>( resources.push(format!("file://{CONTEXT_CONTENT_TYPE_DIR}/{context_name}")); } - let tools = kiro.tools.iter().map(|id| map_tool_tag(id)).collect(); - let rules = derive_permission_rules(&kiro.tools_settings, &kiro.allowed_tools); + let mut tools: Vec = kiro.tools.iter().map(|id| map_tool_tag(id)).collect(); + let mut rules = derive_permission_rules(&kiro.tools_settings, &kiro.allowed_tools); + let mut excluded_tools = Vec::new(); + if is_specialist(name) { + tools.retain(|tag| !is_subagent_tag(tag)); + rules.retain(|rule| rule.capability != SUBAGENT_CAPABILITY); + excluded_tools = SPECIALIST_EXCLUDED_TOOLS.to_vec(); + } Ok(KiroV3AgentFile { name, @@ -1228,6 +1249,7 @@ fn render_agent_file<'a>( model: &config.model, welcome_message: kiro.welcome_message.as_deref(), tools, + excluded_tools, permissions: PermissionsBlock { rules }, mcp_servers, hooks: convert_hooks_for_v3(&kiro.hooks), @@ -1983,6 +2005,57 @@ mod tests { assert_eq!(subagent_rules[0].r#match, vec!["reviewer-agent"]); } + fn kiro_with_spawn_grant() -> KiroCliConfig { + KiroCliConfig { + tools: vec!["@builtin".to_string(), "use_subagent".to_string()], + allowed_tools: vec!["fs_read".to_string(), "subagent".to_string()], + tools_settings: serde_json::json!({ + "subagent": {"trustedAgents": ["k-developer"]} + }), + ..Default::default() + } + } + + #[test] + fn render_agent_file_strips_spawn_grants_from_specialist() { + let agent = agent_with_kiro("k-example", kiro_with_spawn_grant()); + let kiro_cfg = agent.client_config.kiro_cli.as_ref().unwrap(); + let file = + render_agent_file(&agent.name, &agent.config, kiro_cfg, &[], &HashSet::new()).unwrap(); + + assert_eq!(file.tools, vec!["@builtin"]); + assert!( + file.permissions + .rules + .iter() + .all(|r| r.capability != "subagent"), + "specialist kept a subagent rule: {:?}", + file.permissions.rules + ); + let json = serde_json::to_value(&file).unwrap(); + assert_eq!( + json["excludedTools"], + serde_json::json!(["invoke_sub_agent", "run_workflow"]) + ); + } + + #[test] + fn render_agent_file_keeps_spawn_grants_for_orchestrator() { + let agent = agent_with_kiro("konductor", kiro_with_spawn_grant()); + let kiro_cfg = agent.client_config.kiro_cli.as_ref().unwrap(); + let file = + render_agent_file(&agent.name, &agent.config, kiro_cfg, &[], &HashSet::new()).unwrap(); + + assert_eq!(file.tools, vec!["@builtin", "subagent"]); + assert!(file + .permissions + .rules + .iter() + .any(|r| r.capability == "subagent" && r.r#match == vec!["k-developer"])); + let json = serde_json::to_value(&file).unwrap(); + assert!(json.get("excludedTools").is_none()); + } + /// A `toolsSettings` entry that only produces a DENY rule for a /// capability (no `allowedPaths`/`allowedCommands` present) must /// not suppress a separate, legitimate ALLOW grant for that same diff --git a/cli/konductor-rs/src/cli/synth/model.rs b/cli/konductor-rs/src/cli/synth/model.rs index 3989731e..7b0e8d5c 100644 --- a/cli/konductor-rs/src/cli/synth/model.rs +++ b/cli/konductor-rs/src/cli/synth/model.rs @@ -63,6 +63,12 @@ pub struct ContextDef { /// interchangeable at this milestone. pub type AgentSpec = ParsedAgentSpec; +/// Specialists are the `k-*` agents. They never spawn other agents, so each +/// harness transformer strips spawn tools from their configs. +pub(crate) fn is_specialist(agent_name: &str) -> bool { + agent_name.starts_with("k-") +} + /// The full picture of the source tree, held in memory: all the agents, /// skills, SOPs, and context files that `konductor synth` will /// transform into harness-specific output. Built by `parse_canonical`, @@ -134,6 +140,14 @@ mod tests { assert!(executable.executable); } + #[test] + fn is_specialist_matches_k_prefix_only() { + assert!(is_specialist("k-architect")); + assert!(!is_specialist("konductor")); + assert!(!is_specialist("konductor-mux-orchestrator")); + assert!(!is_specialist("konductor-cmux-orchestrator")); + } + #[test] fn canonical_model_default_is_empty() { let model = CanonicalModel::default(); diff --git a/cli/konductor-rs/src/cli/synth/parse_canonical.rs b/cli/konductor-rs/src/cli/synth/parse_canonical.rs index 70c2cf62..6ce02d0a 100644 --- a/cli/konductor-rs/src/cli/synth/parse_canonical.rs +++ b/cli/konductor-rs/src/cli/synth/parse_canonical.rs @@ -15,7 +15,11 @@ use std::path::{Path, PathBuf}; use serde_json::Value; -use crate::cli::synth::model::{AuxiliaryFile, CanonicalModel, ContextDef, SkillDef, SopDef}; +use crate::cli::synth::claude::is_spawn_tool; +use crate::cli::synth::kiro_cli_v3::{is_subagent_tag, map_tool_tag, SUBAGENT_CAPABILITY}; +use crate::cli::synth::model::{ + is_specialist, AuxiliaryFile, CanonicalModel, ContextDef, SkillDef, SopDef, +}; use crate::cli::synth::parser::{self, ParseError}; use crate::cli::synth::path_safety::case_insensitive_fold_key; @@ -79,6 +83,9 @@ pub fn parse_canonical(source_dir: &Path) -> Result for warning in check_skill_scope_consistency(&agents) { eprintln!("konductor: warning: {warning}"); } + for warning in check_specialist_spawn_grants(&agents) { + eprintln!("konductor: warning: {warning}"); + } Ok(CanonicalModel { agents, @@ -311,6 +318,41 @@ fn check_skill_scope_consistency(agents: &[parser::ParsedAgentSpec]) -> Vec Vec { + let mut warnings = Vec::new(); + for agent in agents.iter().filter(|agent| is_specialist(&agent.name)) { + let mut grants = Vec::new(); + if let Some(kiro) = &agent.client_config.kiro_cli { + let is_kiro_spawn = |id: &String| is_subagent_tag(&map_tool_tag(id)); + grants.extend(kiro.tools.iter().filter(|id| is_kiro_spawn(id)).cloned()); + grants.extend( + kiro.allowed_tools + .iter() + .filter(|id| is_kiro_spawn(id)) + .cloned(), + ); + if kiro.tools_settings.get(SUBAGENT_CAPABILITY).is_some() { + grants.push(format!("toolsSettings.{SUBAGENT_CAPABILITY}")); + } + } + if let Some(claude) = &agent.client_config.claude_cli { + let tools = claude.tools.iter().flatten(); + grants.extend(tools.filter(|tool| is_spawn_tool(tool)).cloned()); + } + if !grants.is_empty() { + warnings.push(format!( + "agent '{}' is a specialist (k-* name) but its spec grants spawn tools {grants:?}; \ + specialists must not spawn, so remove them from the spec", + agent.name + )); + } + } + warnings +} + /// Lists direct children of `dir` matching `predicate`, sorted for determinism /// before any processing occurs. Returns an empty vec if `dir` doesn't exist. fn list_dir_entries( @@ -1153,6 +1195,53 @@ mod tests { parser::parse_agent_spec_str(&content, "agents/test.agent-spec.json").unwrap() } + fn agent_spec_with_client_config( + name: &str, + client_config_json: &str, + ) -> parser::ParsedAgentSpec { + let content = format!( + r#"{{"schemaVersion":"1","name":"{name}","config":{{"description":"d","systemPrompt":"s","model":"m"}},"clientConfig":{client_config_json}}}"# + ); + parser::parse_agent_spec_str(&content, "agents/test.agent-spec.json").unwrap() + } + + #[test] + fn check_specialist_spawn_grants_warns_on_each_stripped_grant() { + let agent = agent_spec_with_client_config( + "k-example", + r#"{"kiroCli":{"tools":["@builtin","use_subagent"],"allowedTools":["subagent"],"toolsSettings":{"subagent":{}}},"claudeCli":{"tools":["Read","Agent(k-developer)","Task"]}}"#, + ); + let warnings = check_specialist_spawn_grants(&[agent]); + assert_eq!(warnings.len(), 1, "got: {warnings:?}"); + for expected in [ + "k-example", + "use_subagent", + "\"subagent\"", + "toolsSettings.subagent", + "Agent(k-developer)", + "Task", + ] { + assert!( + warnings[0].contains(expected), + "missing {expected}: {}", + warnings[0] + ); + } + } + + #[test] + fn check_specialist_spawn_grants_ignores_orchestrators_and_clean_specialists() { + let orchestrator = agent_spec_with_client_config( + "konductor", + r#"{"kiroCli":{"tools":["@builtin","subagent"]},"claudeCli":{"tools":["Agent(k-developer)"]}}"#, + ); + let specialist = agent_spec_with_client_config( + "k-example", + r#"{"kiroCli":{"tools":["@builtin"]},"claudeCli":{"tools":["Read","Bash"]}}"#, + ); + assert!(check_specialist_spawn_grants(&[orchestrator, specialist]).is_empty()); + } + /// Clean case: every `skill://` resource entry's name already /// appears in `skillNames` -- resources are a subset of skillNames, /// so no warning is produced. diff --git a/cli/konductor-rs/tests/synth_install_e2e.rs b/cli/konductor-rs/tests/synth_install_e2e.rs index ca141a8b..e5c1681d 100644 --- a/cli/konductor-rs/tests/synth_install_e2e.rs +++ b/cli/konductor-rs/tests/synth_install_e2e.rs @@ -1017,3 +1017,107 @@ fn real_synth_fails_on_dangling_skill_reference() { std::fs::remove_dir_all(&repo_root).ok(); } + +fn seed_spawning_agent_spec(agents_dir: &Path, name: &str) { + let spec = serde_json::json!({ + "schemaVersion": "1", + "name": name, + "config": { + "description": "An example agent.", + "model": "claude-sonnet-5", + "systemPrompt": "You are a helpful agent." + }, + "clientConfig": { + "kiroCli": { + "tools": ["@builtin", "subagent"], + "toolsSettings": {"subagent": {"trustedAgents": ["k-other"]}} + }, + "claudeCli": {"tools": ["Read", "Agent(k-other)"]} + } + }); + std::fs::write( + agents_dir.join(format!("{name}.agent-spec.json")), + serde_json::to_vec_pretty(&spec).unwrap(), + ) + .unwrap(); +} + +fn install_harness(repo_root: &Path, target_dir: &Path, harness: &str) { + let sink = telemetry_test_sink::TelemetrySink::start(); + let result = run_konductor( + target_dir, + &sink, + &[ + CMD_INSTALL, + "--from", + &repo_root.display().to_string(), + "--target", + &target_dir.display().to_string(), + "--harness", + harness, + "--no-telemetry", + ], + ); + assert!( + result.status.success(), + "`install --harness {harness}` must succeed: stderr={}", + String::from_utf8_lossy(&result.stderr) + ); +} + +#[test] +fn installed_specialists_cannot_spawn_but_orchestrator_can() { + let sink = telemetry_test_sink::TelemetrySink::start(); + let repo_root = scratch_dir("repo-specialist-spawn"); + let agents_dir = repo_root.join("agents"); + std::fs::create_dir_all(&agents_dir).unwrap(); + seed_spawning_agent_spec(&agents_dir, "k-example"); + seed_spawning_agent_spec(&agents_dir, "konductor"); + seed_skill_source(&repo_root); + + let synth_result = run_konductor( + &repo_root, + &sink, + &[CMD_SYNTH, "--from", &repo_root.display().to_string()], + ); + assert!( + synth_result.status.success(), + "real `synth --from ` must succeed: stderr={}", + String::from_utf8_lossy(&synth_result.stderr) + ); + + let target_dir = scratch_dir("target-specialist-spawn"); + std::fs::create_dir_all(target_dir.join(".claude")).unwrap(); + install_harness(&repo_root, &target_dir, "kiro-v3"); + install_harness(&repo_root, &target_dir, "claude"); + + let read_kiro = |name: &str| -> serde_json::Value { + let path = target_dir.join(format!(".kiro/agents/{name}.json")); + serde_json::from_slice(&std::fs::read(&path).unwrap()).unwrap() + }; + let specialist = read_kiro("k-example"); + assert_eq!(specialist["tools"], serde_json::json!(["@builtin"])); + assert_eq!( + specialist["excludedTools"], + serde_json::json!(["invoke_sub_agent", "run_workflow"]) + ); + assert!(!specialist["permissions"].to_string().contains("subagent")); + + let orchestrator = read_kiro("konductor"); + assert_eq!( + orchestrator["tools"], + serde_json::json!(["@builtin", "subagent"]) + ); + assert!(orchestrator.get("excludedTools").is_none()); + assert!(orchestrator["permissions"].to_string().contains("k-other")); + + let read_claude = |name: &str| -> String { + let path = target_dir.join(format!(".claude/agents/{name}.md")); + std::fs::read_to_string(&path).unwrap_or_else(|e| panic!("read {}: {e}", path.display())) + }; + assert!(!read_claude("k-example").contains("Agent(")); + assert!(read_claude("konductor").contains("Agent(k-other)")); + + std::fs::remove_dir_all(&repo_root).ok(); + std::fs::remove_dir_all(&target_dir).ok(); +} diff --git a/docs/user-guide/agents.md b/docs/user-guide/agents.md index 7611c257..bbe6f5f5 100644 --- a/docs/user-guide/agents.md +++ b/docs/user-guide/agents.md @@ -7,9 +7,9 @@ All **11 agents** the package ships: what each one does, what it can access, which SOPs it can run, and who it is allowed to delegate to. -Everything on this page was read out of `agents/*.agent-spec.json` — the tool lists, the -`allowedTools` arrays, the `subagent` target lists, the declared skills and SOPs, and the MCP -registry entries. +Everything on this page was read out of `agents/*.agent-spec.json`: the tool lists, the +`allowedTools` arrays, the orchestrators' `subagent` target lists, the declared skills and SOPs, +and the MCP registry entries. --- @@ -139,73 +139,75 @@ pre-approves `fs_read` alone, so anything beyond reading stops to ask you. ## Who can delegate to whom -Delegation is not open. In Kiro CLI it is governed by each agent's -`toolsSettings.subagent.availableAgents`; in Claude Code by whether the agent has an -`Agent(...)` tool at all. +Only the orchestrators delegate. A specialist (any `k-*` agent) never spawns another agent: +it does its own part and returns to its caller, with a handoff note naming the next role or a +proposed parallel split. The orchestrator routes that note. + +For Kiro CLI v3 and Claude Code this is enforced in config, not just in the prompt: + +- Kiro CLI: no specialist spec grants the `subagent` tool or `toolsSettings.subagent`. For + Kiro CLI v3, synth also writes `excludedTools: ["invoke_sub_agent", "run_workflow"]` into every + specialist, because `@builtin` carries `run_workflow`, which can start any agent. This was + tested with kiro-cli 2.35.0; an older client that rejects unknown agent-file fields + may fail to load the specialists. +- Claude Code: synth drops `Agent(...)`, `Task` and `Workflow` from every specialist's `tools` + list. A specialist with no `tools` list gets them as `disallowedTools` instead. +- Kiro CLI v2 is not covered. Its `@builtin` includes the `subagent` tool, and the v2 output does + not remove it, so there only the prompt rule stops a specialist from spawning. + +The block covers the runtime's agent-spawning tools only, not shell. A specialist with shell +access could still start a new agent session by running `kiro-cli` or `claude` from the shell. +`k-architect`, `k-developer` and `k-quality-assurance` have shell pre-approved in Kiro CLI, and +every specialist except `k-researcher` is granted `Bash` in Claude Code. ```mermaid %%{init:{'theme':'base','themeVariables':{'fontSize':'14px','primaryColor':'#eef2f7','primaryTextColor':'#0f172a','primaryBorderColor':'#40556e','lineColor':'#7a8899','textColor':'#0f172a','edgeLabelBackground':'#ffffff','clusterBkg':'#fafbfc','clusterBorder':'#c9d2dc'}}}%% flowchart LR - Orch["konductor
any agent"]:::focus + Orch["konductor
the 8 specialists"]:::focus Arch["k-architect"] Dev["k-developer"] QA["k-quality-assurance"] TPM["k-tpm"] Res["k-researcher"] PM["k-product-manager"] - Br["k-browser
no delegation tool"]:::muted - MA["k-media-analyzer
no delegation tool"]:::muted + Br["k-browser"] + MA["k-media-analyzer"] Mux["konductor-mux-orchestrator
konductor-cmux-orchestrator
dispatch via shell, not subagent"]:::muted - None["k-product-manager
k-researcher
subagent tool, zero targets"]:::gate Orch --> Arch & Dev & QA & TPM & Res & PM & Br & MA - Arch --> Dev - Arch --> QA - Dev --> QA - QA --> Dev - QA --> Br - TPM --> Dev - TPM --> Res classDef focus fill:#dbe7f5,stroke:#2a5d8f,stroke-width:2px,color:#0f172a; - classDef gate fill:#fdf3e0,stroke:#a06800,stroke-width:1.5px,color:#0f172a; classDef muted fill:#f4f4f5,stroke:#8a8a8a,stroke-width:1.2px,color:#0f172a,stroke-dasharray:4 3; ``` -*Kiro CLI delegation graph. Claude Code differs — see the table below.* - | Agent | Kiro CLI can spawn | Claude Code can spawn | | --- | --- | --- | | `konductor` | the 8 specialists (an explicit allowlist, not "anything") | the same 8, via `Agent(...)`, plus `SendMessage` | -| `k-architect` | `k-developer`, `k-quality-assurance` | `k-developer`, `k-quality-assurance`, via `Agent(...)` | -| `k-developer` | `k-quality-assurance` | **nothing** | -| `k-quality-assurance` | `k-developer`, `k-browser` | `k-developer`, `k-browser`, via `Agent(...)` | -| `k-tpm` | `k-developer`, `k-researcher` | **nothing** | -| `k-product-manager` | has the tool, **zero targets** | **nothing** | -| `k-researcher` | has the tool, **zero targets** | **nothing** | | `konductor-mux-orchestrator` | no `subagent` tool — dispatches via shell scripts | dispatches via shell | | `konductor-cmux-orchestrator` | no `subagent` tool — dispatches via shell scripts | dispatches via shell | -| `k-browser` | no delegation | **nothing** | -| `k-media-analyzer` | no delegation | **nothing** | +| every `k-*` specialist | **nothing** | **nothing** | ### Why this matters: a real consequence `k-code-review-workflow` step 6 says to *"spawn `k-architect` in adversarial mode"*. That SOP -is declared only by `k-developer` — which **cannot spawn `k-architect` in either runtime**: - -- In Kiro CLI, `k-developer`'s targets are `["k-quality-assurance"]`. The architect is not on - that list. -- In Claude Code, `k-developer` has no `Agent(...)` tool at all. - -The SOP anticipates this — *"You MUST skip this step if no adversarial CR-review capability is -available"* — so it degrades quietly rather than erroring. **In practice step 6 always no-ops, -and you get standard review only.** - -Routing the request through `konductor` does **not** fix it: the orchestrator does not -declare `k-code-review-workflow`, so it delegates the whole SOP to the developer, who then hits -the same wall. The reliable workaround is to run +is declared only by `k-developer`, which **cannot spawn `k-architect`** in Kiro CLI v3 or Claude +Code: + +- Specialists hold no spawn tool there (see above; Kiro CLI v2 relies on the prompt rule). +- The core rules tell a specialist to do such steps itself or return them to its caller. + +Step 6 has `k-developer` return it as a handoff for `k-architect` when it cannot spawn, naming +the SOP and the step to resume at. Under `konductor`, `konductor` routes it to the architect and +sends the findings back to the developer to finish steps 6 and 7. When `k-developer` is started +directly, nothing routes the handoff, and it comes back to you. To finish the report, ask +`konductor` to run `k-adversarial-pull-request-review`, then pass its findings and the handoff +to `k-developer` and ask it to resume `k-code-review-workflow` at step 6. Started directly, +`k-architect` returns the review's passes as a split, or runs them itself in one session and +says so. + +To be sure the adversarial pass runs, ask `konductor` to run [`k-adversarial-pull-request-review`](sop-workflows/code-review.md#k-adversarial-pull-request-review) -yourself against `k-architect`, which owns it. +against your diff; `konductor` routes it to `k-architect`, which owns it. ### The two multiplexer orchestrators do not use subagents at all @@ -274,7 +276,7 @@ The most capable agent in the package, on the strongest model. | **Skills** | **38** — the largest share of any agent. See [Architecture and design](skills.md#architecture-and-design), [Design quality gates](skills.md#design-quality-gates), [API and data modelling](skills.md#api-and-data-modelling) | | **MCP** | `aws-mcp`, launched via `uvx mcp-proxy-for-aws-cli==1.7.0`. Kiro CLI's `allowedTools` grants five tools (documentation search, documentation read, skill retrieval, region list, regional availability); Claude Code grants the whole server via `mcp__aws-mcp__*` (8 tools, including the three AWS-API-acting ones: `aws___run_script`, `aws___get_presigned_url`, `aws___get_tasks`) | | **Can mutate?** | Write and shell in both runtimes — pre-approved in Kiro CLI, granted in Claude Code | -| **Can delegate to** | `k-developer`, `k-quality-assurance` — in both runtimes | +| **Can delegate to** | nothing (returns to the orchestrator) | Owns both design SOPs *and* the adversarial pull-request review. Its system prompt applies a maker-checker pass to every artifact and presents findings as CRITICAL → IMPORTANT → SUGGESTION @@ -288,7 +290,7 @@ followed by `Fix these issues? [y/n]`. | **Skills** | 27 — backend and frontend development and review, `infra-validation`, `code-review`, `git-workflow`, `asdlc-code-simplifier`, `design-impact-review`, `find-aws-skills`, `security-remediation`, the three `adversarial-code-review-pass-*` skills, `verification`, and others | | **MCP** | `aws-mcp`, same as the architect | | **Can mutate?** | Write and shell in both runtimes — pre-approved in Kiro CLI, granted in Claude Code | -| **Can delegate to** | `k-quality-assurance` (Kiro CLI only) | +| **Can delegate to** | nothing (returns to the orchestrator) | The workhorse. Everything the orchestrator cannot do itself — file writes, git, shell, builds — lands here. It owns the only source-modifying SOP, `k-code-cleanup`. @@ -300,7 +302,7 @@ lands here. It owns the only source-modifying SOP, `k-code-cleanup`. | **SOPs** | `k-test-coverage-review` | | **Skills** | 12 — `test-coverage-analysis`, `e2e-test-strategy`, `cypress-test-implementation`, `playwright-test-implementation`, `security-test-generation`, `dom-inspection`, `ui-text-validation`, and others | | **Can mutate?** | Write and shell in both runtimes — pre-approved in Kiro CLI, granted in Claude Code | -| **Can delegate to** | `k-developer`, `k-browser` — in both runtimes | +| **Can delegate to** | nothing (returns to the orchestrator) | ### `k-product-manager` @@ -309,7 +311,7 @@ lands here. It owns the only source-modifying SOP, `k-code-cleanup`. | **SOPs** | **none** — skill-driven | | **Skills** | 16 — `user-story-writing`, `requirements-extraction`, `decision-research`, `sprint-planning`, `asana-sprint-planning`, `task-decomposition`, `legacy-to-agentic-estimate`, `humanize-writing`, `deliberation-panel`, `kiro-requirements-generation`, and others | | **Can mutate?** | Write pre-approved in Kiro CLI and granted in Claude Code; shell asks first in Kiro CLI and is granted in Claude Code | -| **Can delegate to** | nothing — has the `subagent` tool with an empty target list | +| **Can delegate to** | nothing (returns to the orchestrator) | Because it declares no SOPs there is no scripted procedure to invoke. Ask for the outcome — user stories, a requirements summary, a sprint plan — and it applies the relevant skill. @@ -321,7 +323,7 @@ stories, a requirements summary, a sprint plan — and it applies the relevant s | **SOPs** | **none** — skill-driven | | **Skills** | 14 — `program-planning`, `status-reporting`, `risk-management`, `program-decision-docs`, `plan-review`, `legacy-to-agentic-estimate`, `sprint-planning`, and others | | **Can mutate?** | Write pre-approved in Kiro CLI and granted in Claude Code; shell asks first in Kiro CLI and is granted in Claude Code | -| **Can delegate to** | `k-developer`, `k-researcher` (Kiro CLI only) | +| **Can delegate to** | nothing (returns to the orchestrator) | Owns `legacy-to-agentic-estimate` and its `convert_estimates.py` script, which is why `k-plan` step 5 delegates the Agentic Projection here. @@ -332,10 +334,10 @@ Owns `legacy-to-agentic-estimate` and its `convert_estimates.py` script, which i | --- | --- | | **SOPs** | **none** — skill-driven | | **Skills** | 8 — `external-research`, `sdlc-navigator`, `document-formats`, `deliberation-panel`, `socratic-elicitation`, `persistent-memory`, `workspace-skills`, `constraints` | -| **Kiro CLI tools** | `@builtin`, `subagent`; `allowedTools` is `fs_read` only | +| **Kiro CLI tools** | `@builtin`; `allowedTools` is `fs_read` only | | **Claude Code tools** | `WebFetch`, `WebSearch`, `TodoWrite`, `Read`, `Skill` — the narrowest tool list of any agent, and the only one with no write or shell tool | | **Can mutate?** | **Not in Claude Code** — no write or shell tool is granted at all. In Kiro CLI it pre-approves `fs_read` only, so anything more asks first | -| **Can delegate to** | nothing — `subagent` tool with an empty target list | +| **Can delegate to** | nothing (returns to the orchestrator) | The only specialist that cannot modify anything in either runtime, which makes it the safest agent to point at unfamiliar code. `k-context-gathering` step 1 spawns one or two of these in parallel. @@ -351,6 +353,7 @@ Optional Slack search is opt-in — see | **Skills** | 4 — `constraints`, `sdlc-navigator`, `app-discovery`, `dom-inspection` | | **MCP** | `playwright-mcp` (`@playwright/mcp@latest`). Kiro CLI `allowedTools` enumerates 13 specific browser tools — snapshot, screenshot, console messages, network requests, tabs, navigate, navigate back, wait for, click, type, select option, press key, close | | **Can mutate?** | Pre-approved for reads only in Kiro CLI, so writes and commands ask first; **write and shell granted in Claude Code** | +| **Can delegate to** | nothing (returns to the orchestrator) | Needs the browser binary: `npx playwright install chromium`. Note the Kiro CLI tool list is an explicit allowlist — it cannot, for example, upload files or evaluate arbitrary JavaScript. @@ -363,6 +366,7 @@ explicit allowlist — it cannot, for example, upload files or evaluate arbitrar | **Skills** | 1, `constraints`. It also declares `humanize-writing`, loaded on demand | | **Kiro CLI tools** | `@builtin`; `allowedTools` is `fs_read`, `web_fetch` | | **Can mutate?** | Pre-approved for reads only in Kiro CLI, so writes and commands ask first; **write and shell granted in Claude Code** | +| **Can delegate to** | nothing (returns to the orchestrator) | The leanest agent in the package. Its capability is the model's ability to interpret PDFs, images, and diagrams — not a skill library. Point it at a file and ask a question about it. @@ -374,7 +378,7 @@ images, and diagrams — not a skill library. Point it at a file and ask a quest | Difference | Kiro CLI | Claude Code | | --- | --- | --- | | Agent name | `konductor` | `konductor` | -| Cross-agent delegation | 7 agents have `subagent`, 5 with real targets | `konductor`, `k-architect`, and `k-quality-assurance` hold `Agent(...)` | +| Cross-agent delegation | only `konductor` declares `subagent`; the mux orchestrators use shell. v3 specialists also have the built-in spawn tools excluded; v2 specialists still get `subagent` from `@builtin` | only `konductor` holds `Agent(...)` | | MCP servers | declared and launched from `dependencies.mcpRegistry` | requested via tool globs (`mcp__aws-mcp__*`); you configure the server yourself | | Tool naming | `fs_read`, `fs_write`, `shell` | `Read`, `Write`, `Edit`, `Bash` | | Permission model | two lists: `tools` grants with a prompt, `allowedTools` pre-approves | one list: `clientConfig.claudeCli.tools` is the ceiling, and your `settings.json` decides what prompts | @@ -383,9 +387,7 @@ images, and diagrams — not a skill library. Point it at a file and ask a quest | Skills resolution | through the bundled `skill-lookup` MCP server. Skills install to `.konductor/skills/`, deliberately outside the `.kiro/skills/` tree Kiro CLI scans, and the agent reaches them through that server | native Claude Code skills: `clientConfig.claudeCli.skills` names the set, and each body installs to `.claude/skills//SKILL.md` | | Extra setup | none | agent-teams env var **and** a permissions allowlist | -The delegation row is the one that changes behaviour most: a workflow that relies on one -specialist handing off to another may work in Kiro CLI and not in Claude Code — `k-developer` -and `k-tpm` have Kiro targets but no `Agent(...)` tool at all. +In both runtimes a specialist hands off by returning to the orchestrator, never by spawning. --- diff --git a/docs/user-guide/concepts.md b/docs/user-guide/concepts.md index ca41b9a1..23cd1677 100644 --- a/docs/user-guide/concepts.md +++ b/docs/user-guide/concepts.md @@ -166,14 +166,11 @@ than doing them itself. ## Orchestrator and delegation -**`konductor`**, as the orchestrator, is a hub — though not an exclusive one. In Kiro CLI four -specialists can also spawn each other directly (`k-architect` → developer and QA; -`k-developer` → QA; `k-quality-assurance` → developer and browser; `k-tpm` → developer and -researcher), and in Claude Code `k-architect` and `k-quality-assurance` carry `Agent(...)` tools -of their own — see [Who can delegate to whom](agents.md#who-can-delegate-to-whom). In the common -case, though, every piece of work routes through `konductor`, which decides who owns it, hands it -over, and checks the result. The core rules tell specialists not to use those grants: a -specialist does its own part, then returns a note naming the next role, or a proposed parallel +**`konductor`**, as the orchestrator, is the hub. Specialists hold no spawn tool in Kiro CLI +v3 or Claude Code, and the core rules forbid spawning everywhere (see +[Who can delegate to whom](agents.md#who-can-delegate-to-whom)), so every piece of +work routes through `konductor`, which decides who owns it, hands it over, and checks the result. +A specialist does its own part, then returns a note naming the next role, or a proposed parallel split that gives each part's agent, inputs and files, and `konductor` routes it. A **delegation** is not a casual instruction — it is a fixed 7-section format defined by the diff --git a/docs/user-guide/faq.md b/docs/user-guide/faq.md index e33010f7..b7585e6c 100644 --- a/docs/user-guide/faq.md +++ b/docs/user-guide/faq.md @@ -210,17 +210,20 @@ transmitted. Delete it whenever you like. ## Why did my code review skip the security pass? -Because it always does. `k-code-review-workflow` step 6 tries to spawn `k-architect` for an -adversarial pass, and **the only agent that owns the SOP cannot spawn the architect** — in Kiro CLI -`k-developer`'s `availableAgents` list does not include it, and in Claude Code it has no -`Agent(...)` tool. -The SOP is written to skip the step quietly when the capability is missing, so you get standard -review with no error. - -Routing through `konductor` does not fix it either: the orchestrator does not declare -`k-code-review-workflow`, so it hands the SOP to the developer, which hits the same wall. - -Run the adversarial pass yourself against the agent that owns it: +`k-code-review-workflow` step 6 asks for an adversarial pass by `k-architect`, and **the only +agent that owns the SOP cannot spawn the architect**: no specialist holds a spawn tool in Kiro CLI +v3 or Claude Code, and the core rules forbid it in Kiro CLI v2. + +Step 6 tells the developer to return it as a handoff for `k-architect` when it cannot spawn. +Under `konductor`, `konductor` routes the handoff and sends the findings back for the report. If +you started `k-developer` directly, nothing routes the handoff, and it comes back to you. To +finish the report, ask `konductor` to run `k-adversarial-pull-request-review`, then pass its +findings and the handoff to `k-developer` and ask it to resume `k-code-review-workflow` at step 6. +Started directly, `k-architect` returns the review's passes as a split, or runs them itself in one +session and says so. + +To run the adversarial pass on its own, ask `konductor`, which routes it to `k-architect`, the +agent that owns it: ```text Run k-adversarial-pull-request-review against my current diff. diff --git a/docs/user-guide/glossary.md b/docs/user-guide/glossary.md index d5690591..e2a43772 100644 --- a/docs/user-guide/glossary.md +++ b/docs/user-guide/glossary.md @@ -33,8 +33,8 @@ Another name for a **SOP**. ### `availableAgents` The list in a Kiro CLI agent spec (`clientConfig.kiroCli.toolsSettings.subagent.availableAgents`) -naming exactly which agents that agent may spawn. An empty list means the agent holds the -`subagent` tool but can spawn nothing. Only `konductor` is unrestricted. See +naming exactly which agents that agent may spawn. Only `konductor` declares one; specialists +hold no `subagent` tool at all. See [Who can delegate to whom](agents.md#who-can-delegate-to-whom). ### Canonical model diff --git a/docs/user-guide/sop-workflows/code-review.md b/docs/user-guide/sop-workflows/code-review.md index 65411e09..1e46fcd6 100644 --- a/docs/user-guide/sop-workflows/code-review.md +++ b/docs/user-guide/sop-workflows/code-review.md @@ -132,15 +132,15 @@ references, and a verdict of **READY FOR CR** or **NEEDS FIXES**. - **The diff, not just the file list, is the input.** Step 1 captures full hunks; the changed-file list is derived from them. -- **The adversarial step never actually fires.** Step 6 tells the agent to spawn - `k-architect`, but its one owning agent cannot: in Kiro CLI `k-developer`'s - `subagent.availableAgents` is `["k-quality-assurance"]`, which does not include the architect, - and in Claude Code it has no `Agent(...)` tool at all. - The SOP degrades quietly by design (*"You MUST skip this step if no adversarial CR-review - capability is available"*), so you get standard review only. Routing through the orchestrator does - not help — it does not declare this SOP. Run - [`k-adversarial-pull-request-review`](#k-adversarial-pull-request-review) against `k-architect` - separately instead. See +- **The adversarial step runs only through `konductor`.** Step 6 asks for `k-architect`, but its + one owning agent cannot spawn: no specialist holds a spawn tool in Kiro CLI v3 or Claude Code, + and the core rules forbid it in Kiro CLI v2. Step 6 has `k-developer` return it as a handoff + instead. Under `konductor`, `konductor` routes it to `k-architect`, and the findings come back + for the report. When `k-developer` is started directly, the handoff comes back to you: ask + `konductor` to run [`k-adversarial-pull-request-review`](#k-adversarial-pull-request-review), + then pass its findings and the handoff to `k-developer` and ask it to resume + `k-code-review-workflow` at step 6 to finish the report. Started directly, `k-architect` returns + the review's passes as a split, or runs them itself in one session and says so. See [Agents reference](../agents.md#why-this-matters-a-real-consequence). - **The adversarial step is also skipped for frontend-only reviews** by design, because it targets backend and infra gaps. diff --git a/docs/user-guide/use-cases/plan-and-verify-work.md b/docs/user-guide/use-cases/plan-and-verify-work.md index 5f4049bb..dd3295c4 100644 --- a/docs/user-guide/use-cases/plan-and-verify-work.md +++ b/docs/user-guide/use-cases/plan-and-verify-work.md @@ -182,10 +182,8 @@ The chain itself is runtime-agnostic. What differs is dispatch mechanics: Functionally the same three SOPs run either way. -One caveat worth knowing: **specialist-to-specialist handoff is narrower in Claude Code**. There, -only `k-architect` (→ developer, QA) and `k-quality-assurance` (→ developer, browser) carry -`Agent(...)` tools of their own; `k-developer` and `k-tpm` have Kiro CLI targets but no `Agent(...)` -tool at all. The multiplexer variants shell out instead, so they work either way. +Specialists never spawn each other. Kiro CLI v3 and Claude Code enforce this in config; Kiro CLI +v2 relies on the core rules. A specialist returns to the orchestrator, which routes the next step. --- diff --git a/docs/user-guide/use-cases/review-code-and-tests.md b/docs/user-guide/use-cases/review-code-and-tests.md index 763930cf..621083a7 100644 --- a/docs/user-guide/use-cases/review-code-and-tests.md +++ b/docs/user-guide/use-cases/review-code-and-tests.md @@ -29,22 +29,24 @@ it generates. **`k-developer`, the SOP's only owner, cannot do that spawn**, verified against the agent specs: -- In **Claude Code**, `k-developer` has no `Agent(...)` tool at all. (`k-architect` and - `k-quality-assurance` do hold `Agent(...)` tools there — delegation in Claude Code is not - orchestrator-only — but the agent that owns this SOP is not one of them.) -- In **Kiro CLI**, delegation goes through `toolsSettings.subagent.availableAgents`, and - `k-developer`'s is `["k-quality-assurance"]` — no architect. +- In **Claude Code**, no specialist has an `Agent(...)` tool; only `konductor` does. +- In **Kiro CLI v3**, no specialist has the `subagent` tool; only `konductor` does. In Kiro CLI + v2, `@builtin` still carries it, and only the core rules stop the spawn. -Step 6's own constraints anticipate this — *"You MUST skip this step if no adversarial CR-review -capability is available"* — so the workflow degrades gracefully rather than erroring. In practice -**step 6 silently no-ops and you get standard review only.** +**Through `konductor`, step 6 still runs.** Step 6 tells `k-developer` to return it as a +handoff for `k-architect`, naming the SOP and the step to resume at, when it cannot spawn the +architect. `konductor` routes it to the architect and sends the findings back to the developer +to finish the report. -**Routing through `konductor` does not fix it.** `k-code-review-workflow` is not among the ten -SOPs the orchestrator declares, so it delegates the whole SOP to the developer, which hits the -same wall. +**Started directly, the handoff comes back to you.** Nothing routes it, so the report waits on +step 6 until the adversarial pass runs. To finish the report, ask `konductor` to run +`k-adversarial-pull-request-review`, then pass its findings and the handoff to `k-developer` and +ask it to resume `k-code-review-workflow` at step 6. Started directly, `k-architect` returns the +review's passes as a split, or runs them itself in one session and says so. -**The workaround that does work:** run `k-adversarial-pull-request-review` yourself against -`k-architect`, which owns it. That is step 4 below. +**To run the adversarial pass on its own:** ask `konductor` to run +`k-adversarial-pull-request-review` against your diff; `konductor` routes it to `k-architect`, +which owns it. That is step 4 below. --- @@ -168,7 +170,7 @@ afterwards — this is the one SOP here where "review" means "changed your code" ### 4. `k-adversarial-pull-request-review` — the security second opinion -Ask **`k-architect`** directly. This is the workaround for the step 6 gap. +Ask **`konductor`**, which routes it to `k-architect`. This is the workaround for the step 6 gap. ```text Run k-adversarial-pull-request-review against my current diff. diff --git a/tests/test_agent_tool_subagent_references.py b/tests/test_agent_tool_subagent_references.py index 05b61f50..a156665b 100644 --- a/tests/test_agent_tool_subagent_references.py +++ b/tests/test_agent_tool_subagent_references.py @@ -52,3 +52,71 @@ def test_agent_tool_entries_splits_comma_separated_names(): def test_agent_tool_entries_ignores_specs_without_an_agent_grant(): spec = {"clientConfig": {"claudeCli": {"tools": ["Read", "Write"]}}} assert _agent_tool_entries(spec) == [] + + +# `*` is rejected here, at spec level, because a wildcard on v2 includes `subagent`. +KIRO_SPAWN_TOOLS = {"subagent", "use_subagent", "agent_crew", "*"} +CLAUDE_SPAWN_TOOLS = {"Agent", "Task", "Workflow"} + + +def _spawn_grants(spec): + client = spec.get("clientConfig", {}) + kiro = client.get("kiroCli", {}) + grants = [] + for tool in kiro.get("tools", []): + if tool in KIRO_SPAWN_TOOLS or tool.split("/")[0] in KIRO_SPAWN_TOOLS: + grants.append(f"kiroCli.tools has {tool}") + for tool in kiro.get("allowedTools", []): + if tool in KIRO_SPAWN_TOOLS or tool.split("/")[0] in KIRO_SPAWN_TOOLS: + grants.append(f"kiroCli.allowedTools has {tool}") + for key in kiro.get("toolsSettings", {}): + if key in KIRO_SPAWN_TOOLS: + grants.append(f"kiroCli.toolsSettings has {key}") + for tool in client.get("claudeCli", {}).get("tools") or []: + if tool.split("(")[0].strip() in CLAUDE_SPAWN_TOOLS: + grants.append(f"claudeCli.tools has {tool}") + return grants + + +def test_specialists_hold_no_spawn_grant(): + failures = [] + for spec_path in sorted(AGENTS_DIR.glob("k-*.agent-spec.json")): + spec = json.loads(spec_path.read_text()) + failures.extend(f"{spec_path.name}: {grant}" for grant in _spawn_grants(spec)) + + assert not failures, "specialists must not spawn agents:\n" + "\n".join(failures) + + +def test_spawn_grants_catches_every_alias(): + spec = { + "clientConfig": { + "kiroCli": { + "tools": ["@builtin", "subagent", "use_subagent", "agent_crew", "*"], + "allowedTools": ["fs_read", "use_subagent"], + "toolsSettings": {"subagent": {}}, + }, + "claudeCli": {"tools": ["Read", "Agent", "Task", "Workflow", "Agent(k-developer)"]}, + } + } + assert _spawn_grants(spec) == [ + "kiroCli.tools has subagent", + "kiroCli.tools has use_subagent", + "kiroCli.tools has agent_crew", + "kiroCli.tools has *", + "kiroCli.allowedTools has use_subagent", + "kiroCli.toolsSettings has subagent", + "claudeCli.tools has Agent", + "claudeCli.tools has Task", + "claudeCli.tools has Workflow", + "claudeCli.tools has Agent(k-developer)", + ] + + +def test_spawn_grants_ignores_ordinary_tools(): + spec = { + "clientConfig": { + "kiroCli": {"tools": ["@builtin", "@aws-mcp"], "toolsSettings": {"shell": {}}}, + "claudeCli": {"tools": ["Read", "Bash", "mcp__aws-mcp__*"]}, + } + } + assert _spawn_grants(spec) == []