feat(agents): add Implementer and Verifier sub-agent roles (#404)
The existing taxonomy (General / Explore / Plan / Review / Custom)
covered "do something" / "go look" / "think first" / "grade work" /
"explicit allowlist" but had no distinct posture for two of the most
common patterns:
- **Implementer** — "land this change with the minimum surrounding
edit". Distinct from General's flexible posture and Plan's
no-execution stance.
- **Verifier** — "run the test suite and report pass/fail with
evidence". Distinct from Review's read-and-grade stance.
Per the issue body's guidance ("avoid a large undifferentiated role
list") this PR adds **only those two**. Researcher and ReleaseManager
stay open as v0.8.9 candidates if user demand surfaces.
### What's wired
- Two new \`SubAgentType\` variants: \`Implementer\`, \`Verifier\`.
- New prompt constants \`IMPLEMENTER_AGENT_PROMPT\` and
\`VERIFIER_AGENT_PROMPT\` with role-specific posture (write-the-
minimum-edit / run-the-tests-don't-fix-them).
- \`from_str\` accepts the obvious aliases:
\`implementer\` / \`implement\` / \`implementation\` / \`builder\`
for Implementer; \`verifier\` / \`verify\` / \`verification\` /
\`validator\` / \`tester\` for Verifier. Case-insensitive like the
existing aliases.
- \`as_str\` round-trips: every variant's label parses back to the
same variant via \`from_str\`. Test pins this so a future role
addition can't accidentally drop the round-trip.
- Deprecated \`allowed_tools()\` advisory list updated:
Implementer carries write/edit/patch + shell + checklist tools;
Verifier carries read + shell + run_tests + diagnostics but
**no** write tools.
- \`crates/tui/src/tui/views/mod.rs\` agent-type sort order extended
to include the new variants; \`format_agent_type\` now delegates to
\`as_str\` so future additions land in one place.
### Tests
- 6 new tests in \`tools::subagent::tests\`:
- alias coverage for Implementer (4 aliases) and Verifier (5)
- round-trip via \`as_str\` for **all** variants — guards against
forgetting to register a new variant in either direction
- distinct-prompts guard: catches the copy-paste bug where two new
variants would inherit the same prompt as General
- Implementer's advisory list contains write tools
- Verifier's advisory list contains test-runner tools but NO writes
### Verification
cargo fmt --all -- --check ✓
cargo clippy --workspace --all-targets --all-features --locked -- -D warnings ✓
cargo test --workspace --all-features --locked ✓ 1856 + supporting
Closes #404 (minimal-taxonomy interpretation per the issue body).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Hunter Bown
@@ -177,6 +177,16 @@ pub enum SubAgentType {
|
||||
Plan,
|
||||
/// Code review - read + analysis tools.
|
||||
Review,
|
||||
/// Implementation — focused on writing / patching code to satisfy
|
||||
/// a specific change. Distinct from `General` in that the prompt
|
||||
/// posture pushes hard on landing the change cleanly with the
|
||||
/// minimum surrounding edit (#404).
|
||||
Implementer,
|
||||
/// Verification — focused on running the test suite or other
|
||||
/// validation gates and reporting pass/fail with evidence.
|
||||
/// Distinct from `Review` in that Review reads code and grades it;
|
||||
/// Verifier *runs* tests and reports the outcome (#404).
|
||||
Verifier,
|
||||
/// Custom tool access defined at spawn time.
|
||||
Custom,
|
||||
}
|
||||
@@ -192,6 +202,8 @@ impl SubAgentType {
|
||||
"explore" | "exploration" | "explorer" => Some(Self::Explore),
|
||||
"plan" | "planning" | "awaiter" => Some(Self::Plan),
|
||||
"review" | "code-review" | "code_review" | "reviewer" => Some(Self::Review),
|
||||
"implementer" | "implement" | "implementation" | "builder" => Some(Self::Implementer),
|
||||
"verifier" | "verify" | "verification" | "validator" | "tester" => Some(Self::Verifier),
|
||||
"custom" => Some(Self::Custom),
|
||||
_ => None,
|
||||
}
|
||||
@@ -204,6 +216,8 @@ impl SubAgentType {
|
||||
Self::Explore => "explore",
|
||||
Self::Plan => "plan",
|
||||
Self::Review => "review",
|
||||
Self::Implementer => "implementer",
|
||||
Self::Verifier => "verifier",
|
||||
Self::Custom => "custom",
|
||||
}
|
||||
}
|
||||
@@ -216,6 +230,8 @@ impl SubAgentType {
|
||||
Self::Explore => EXPLORE_AGENT_PROMPT.to_string(),
|
||||
Self::Plan => PLAN_AGENT_PROMPT.to_string(),
|
||||
Self::Review => REVIEW_AGENT_PROMPT.to_string(),
|
||||
Self::Implementer => IMPLEMENTER_AGENT_PROMPT.to_string(),
|
||||
Self::Verifier => VERIFIER_AGENT_PROMPT.to_string(),
|
||||
Self::Custom => CUSTOM_AGENT_PROMPT.to_string(),
|
||||
}
|
||||
}
|
||||
@@ -289,6 +305,44 @@ impl SubAgentType {
|
||||
"todo_list",
|
||||
],
|
||||
Self::Review => vec!["list_dir", "read_file", "grep_files", "file_search", "note"],
|
||||
Self::Implementer => vec![
|
||||
"list_dir",
|
||||
"read_file",
|
||||
"write_file",
|
||||
"edit_file",
|
||||
"apply_patch",
|
||||
"grep_files",
|
||||
"file_search",
|
||||
"exec_shell",
|
||||
"exec_shell_wait",
|
||||
"exec_shell_interact",
|
||||
"exec_wait",
|
||||
"exec_interact",
|
||||
"note",
|
||||
"checklist_write",
|
||||
"checklist_add",
|
||||
"checklist_update",
|
||||
"checklist_list",
|
||||
"todo_write",
|
||||
"todo_add",
|
||||
"todo_update",
|
||||
"todo_list",
|
||||
"update_plan",
|
||||
],
|
||||
Self::Verifier => vec![
|
||||
"list_dir",
|
||||
"read_file",
|
||||
"grep_files",
|
||||
"file_search",
|
||||
"exec_shell",
|
||||
"exec_shell_wait",
|
||||
"exec_shell_interact",
|
||||
"exec_wait",
|
||||
"exec_interact",
|
||||
"run_tests",
|
||||
"diagnostics",
|
||||
"note",
|
||||
],
|
||||
Self::Custom => vec![], // Must be provided by caller.
|
||||
}
|
||||
}
|
||||
@@ -3489,6 +3543,61 @@ const CUSTOM_AGENT_PROMPT: &str = concat!(
|
||||
include_str!("../../prompts/subagent_output_format.md"),
|
||||
);
|
||||
|
||||
const IMPLEMENTER_AGENT_PROMPT: &str = concat!(
|
||||
"You are an implementation sub-agent. Your job is to land the change\n",
|
||||
"the parent assigned to you — write the code, modify the files, satisfy\n",
|
||||
"the contract — with the *minimum* surrounding edit. You do not refactor\n",
|
||||
"adjacent code. You do not rename unused variables. You do not 'tidy up'\n",
|
||||
"while you're in the file. If you see related work that should happen,\n",
|
||||
"surface it under RISKS or BLOCKERS rather than starting it.\n",
|
||||
"\n",
|
||||
"Method:\n",
|
||||
"- Read the target file(s) end-to-end before editing. Edits made without\n",
|
||||
" reading the file produce structurally wrong patches.\n",
|
||||
"- Prefer `edit_file` (single search/replace) for narrow changes.\n",
|
||||
" Reach for `apply_patch` only when the change spans multiple hunks\n",
|
||||
" or is structurally tricky.\n",
|
||||
"- After every batch of edits, run a quick verification: a relevant\n",
|
||||
" `cargo check` / `npm run lint` / `pytest -k <test>` so you don't\n",
|
||||
" hand the parent a half-baked implementation.\n",
|
||||
"- If the change requires writing tests, write them first or alongside\n",
|
||||
" the implementation — never as a follow-up the parent has to ask for.\n",
|
||||
"\n",
|
||||
"CHANGES is the load-bearing section for implementers. List every file\n",
|
||||
"you modified with a one-line summary of what changed and why. The parent\n",
|
||||
"uses CHANGES to decide what to inspect next.\n",
|
||||
"\n",
|
||||
include_str!("../../prompts/subagent_output_format.md"),
|
||||
);
|
||||
|
||||
const VERIFIER_AGENT_PROMPT: &str = concat!(
|
||||
"You are a verification sub-agent. Your job is to *run* the project's\n",
|
||||
"test suite (or other validation gates) and report pass/fail with the\n",
|
||||
"evidence the parent needs to act. You are read-only by convention —\n",
|
||||
"do not patch failing tests, do not 'fix' lints, do not modify code.\n",
|
||||
"If a fix seems obvious, describe it under RISKS so the parent can\n",
|
||||
"spawn an Implementer.\n",
|
||||
"\n",
|
||||
"Method:\n",
|
||||
"- Run the right gate for the language: `cargo test --workspace`,\n",
|
||||
" `npm test`, `pytest`, `go test ./...`. Use `run_tests` when it's\n",
|
||||
" available; fall back to `exec_shell` when the project has a custom\n",
|
||||
" invocation.\n",
|
||||
"- Run lints if requested: `cargo clippy -- -D warnings`,\n",
|
||||
" `npm run lint`, `ruff check .`. Don't run lints the parent didn't\n",
|
||||
" ask for; lint noise drowns the signal you were spawned to surface.\n",
|
||||
"- Capture the exact failing assertion plus the stack trace / file:line\n",
|
||||
" in EVIDENCE. A failure summarised as 'cargo test failed' is useless;\n",
|
||||
" the parent needs the actual panic.\n",
|
||||
"\n",
|
||||
"OUTCOME goes at the top of SUMMARY: PASS / FAIL / FLAKY. If FLAKY,\n",
|
||||
"say which test and how many runs you tried.\n",
|
||||
"\n",
|
||||
"CHANGES will almost always be \"None.\" for a verifier.\n",
|
||||
"\n",
|
||||
include_str!("../../prompts/subagent_output_format.md"),
|
||||
);
|
||||
|
||||
// === Tests ===
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -51,6 +51,123 @@ fn test_agent_type_from_str() {
|
||||
assert_eq!(SubAgentType::from_str("invalid"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_agent_type_implementer_aliases() {
|
||||
// #404 — Implementer accepts the obvious aliases the model is
|
||||
// likely to reach for when the user says "build this".
|
||||
for alias in ["implementer", "implement", "implementation", "builder"] {
|
||||
assert_eq!(
|
||||
SubAgentType::from_str(alias),
|
||||
Some(SubAgentType::Implementer),
|
||||
"alias {alias} should resolve to Implementer"
|
||||
);
|
||||
}
|
||||
// Case-insensitive.
|
||||
assert_eq!(
|
||||
SubAgentType::from_str("IMPLEMENTER"),
|
||||
Some(SubAgentType::Implementer)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_agent_type_verifier_aliases() {
|
||||
// #404 — Verifier accepts test/validate aliases distinct from
|
||||
// Reviewer, which is for *grading* code rather than *running* it.
|
||||
for alias in ["verifier", "verify", "verification", "validator", "tester"] {
|
||||
assert_eq!(
|
||||
SubAgentType::from_str(alias),
|
||||
Some(SubAgentType::Verifier),
|
||||
"alias {alias} should resolve to Verifier"
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
SubAgentType::from_str("VERIFY"),
|
||||
Some(SubAgentType::Verifier)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_agent_type_round_trips_via_as_str() {
|
||||
// Every type should serialize to a string that round-trips back
|
||||
// through `from_str`. Catches missed variants when adding a new
|
||||
// role.
|
||||
for t in [
|
||||
SubAgentType::General,
|
||||
SubAgentType::Explore,
|
||||
SubAgentType::Plan,
|
||||
SubAgentType::Review,
|
||||
SubAgentType::Implementer,
|
||||
SubAgentType::Verifier,
|
||||
SubAgentType::Custom,
|
||||
] {
|
||||
let label = t.as_str();
|
||||
let back = SubAgentType::from_str(label)
|
||||
.unwrap_or_else(|| panic!("as_str label {label:?} doesn't round-trip via from_str"));
|
||||
assert_eq!(back, t, "round-trip failed for {t:?} via {label:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_implementer_and_verifier_have_distinct_prompts() {
|
||||
// The whole point of adding the types is that they carry distinct
|
||||
// posture. Defensive guard: catch the easy bug where copy-paste
|
||||
// leaves two new variants with the same prompt as `General`.
|
||||
let implementer = SubAgentType::Implementer.system_prompt();
|
||||
let verifier = SubAgentType::Verifier.system_prompt();
|
||||
let general = SubAgentType::General.system_prompt();
|
||||
assert_ne!(
|
||||
implementer, general,
|
||||
"Implementer prompt must differ from General"
|
||||
);
|
||||
assert_ne!(
|
||||
verifier, general,
|
||||
"Verifier prompt must differ from General"
|
||||
);
|
||||
assert_ne!(
|
||||
implementer, verifier,
|
||||
"Implementer and Verifier must differ"
|
||||
);
|
||||
// Sanity: each prompt mentions the role's defining verb so the
|
||||
// model has clear direction.
|
||||
assert!(
|
||||
implementer.to_lowercase().contains("implement")
|
||||
|| implementer.to_lowercase().contains("write the code"),
|
||||
"Implementer prompt should reference its role: {implementer}"
|
||||
);
|
||||
assert!(
|
||||
verifier.to_lowercase().contains("verif")
|
||||
|| verifier.to_lowercase().contains("test suite")
|
||||
|| verifier.to_lowercase().contains("validation"),
|
||||
"Verifier prompt should reference its role: {verifier}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_implementer_allowed_tools_include_writes() {
|
||||
// Implementer is the write-heavy role; the deprecated
|
||||
// `allowed_tools()` advisory list should reflect that the role
|
||||
// can write/edit/patch even if today's runtime grants full
|
||||
// inheritance.
|
||||
#[allow(deprecated)]
|
||||
let tools = SubAgentType::Implementer.allowed_tools();
|
||||
assert!(tools.contains(&"write_file"));
|
||||
assert!(tools.contains(&"edit_file"));
|
||||
assert!(tools.contains(&"apply_patch"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_verifier_allowed_tools_include_test_runner_but_no_writes() {
|
||||
// Verifier runs validation; it should not have write tools in
|
||||
// its advisory list. The runtime will still gate writes through
|
||||
// approval, but the advisory list signals intent.
|
||||
#[allow(deprecated)]
|
||||
let tools = SubAgentType::Verifier.allowed_tools();
|
||||
assert!(tools.contains(&"run_tests"));
|
||||
assert!(tools.contains(&"diagnostics"));
|
||||
assert!(!tools.contains(&"write_file"));
|
||||
assert!(!tools.contains(&"apply_patch"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_spawn_request_accepts_message_and_agent_type_aliases() {
|
||||
let input = json!({
|
||||
|
||||
@@ -1644,19 +1644,17 @@ fn agent_type_order(agent_type: &SubAgentType) -> u8 {
|
||||
SubAgentType::General => 0,
|
||||
SubAgentType::Explore => 1,
|
||||
SubAgentType::Plan => 2,
|
||||
SubAgentType::Review => 3,
|
||||
SubAgentType::Custom => 4,
|
||||
SubAgentType::Implementer => 3,
|
||||
SubAgentType::Verifier => 4,
|
||||
SubAgentType::Review => 5,
|
||||
SubAgentType::Custom => 6,
|
||||
}
|
||||
}
|
||||
|
||||
fn format_agent_type(agent_type: &SubAgentType) -> &'static str {
|
||||
match agent_type {
|
||||
SubAgentType::General => "general",
|
||||
SubAgentType::Explore => "explore",
|
||||
SubAgentType::Plan => "plan",
|
||||
SubAgentType::Review => "review",
|
||||
SubAgentType::Custom => "custom",
|
||||
}
|
||||
// Source of truth lives on the enum so any new role lands in both
|
||||
// the user-visible label and the sort order via the as_str() helper.
|
||||
agent_type.as_str()
|
||||
}
|
||||
|
||||
fn format_agent_status(
|
||||
|
||||
Reference in New Issue
Block a user