Story 44: Agent Completion Report via MCP
- report_completion MCP tool for agents to signal done - Rejects if worktree has uncommitted changes - Runs acceptance gates (clippy, tests) automatically - Stores completion status on agent record - 10 new tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -5,6 +5,7 @@ use serde::Serialize;
|
||||
use std::collections::HashMap;
|
||||
use std::io::{BufRead, BufReader};
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::process::Command;
|
||||
use std::sync::{Arc, Mutex};
|
||||
use tokio::sync::broadcast;
|
||||
|
||||
@@ -69,6 +70,14 @@ impl std::fmt::Display for AgentStatus {
|
||||
}
|
||||
}
|
||||
|
||||
/// Report produced by an agent calling `report_completion`.
|
||||
#[derive(Debug, Serialize, Clone)]
|
||||
pub struct CompletionReport {
|
||||
pub summary: String,
|
||||
pub gates_passed: bool,
|
||||
pub gate_output: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize, Clone)]
|
||||
pub struct AgentInfo {
|
||||
pub story_id: String,
|
||||
@@ -77,6 +86,7 @@ pub struct AgentInfo {
|
||||
pub session_id: Option<String>,
|
||||
pub worktree_path: Option<String>,
|
||||
pub base_branch: Option<String>,
|
||||
pub completion: Option<CompletionReport>,
|
||||
}
|
||||
|
||||
struct StoryAgent {
|
||||
@@ -88,6 +98,8 @@ struct StoryAgent {
|
||||
task_handle: Option<tokio::task::JoinHandle<()>>,
|
||||
/// Accumulated events for polling via get_agent_output.
|
||||
event_log: Arc<Mutex<Vec<AgentEvent>>>,
|
||||
/// Set when the agent calls report_completion.
|
||||
completion: Option<CompletionReport>,
|
||||
}
|
||||
|
||||
/// Build an `AgentInfo` snapshot from a `StoryAgent` map entry.
|
||||
@@ -105,6 +117,7 @@ fn agent_info_from_entry(story_id: &str, agent: &StoryAgent) -> AgentInfo {
|
||||
.worktree_info
|
||||
.as_ref()
|
||||
.map(|wt| wt.base_branch.clone()),
|
||||
completion: agent.completion.clone(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -179,6 +192,7 @@ impl AgentPool {
|
||||
tx: tx.clone(),
|
||||
task_handle: None,
|
||||
event_log: event_log.clone(),
|
||||
completion: None,
|
||||
},
|
||||
);
|
||||
}
|
||||
@@ -269,6 +283,7 @@ impl AgentPool {
|
||||
session_id: None,
|
||||
worktree_path: Some(wt_path_str),
|
||||
base_branch: Some(wt_info.base_branch.clone()),
|
||||
completion: None,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -431,6 +446,7 @@ impl AgentPool {
|
||||
session_id,
|
||||
worktree_path: None,
|
||||
base_branch: None,
|
||||
completion: None,
|
||||
}
|
||||
});
|
||||
}
|
||||
@@ -476,6 +492,91 @@ impl AgentPool {
|
||||
worktree::create_worktree(project_root, story_id, &config, self.port).await
|
||||
}
|
||||
|
||||
/// Report that an agent has finished work on a story.
|
||||
///
|
||||
/// - Rejects with an error if the worktree has uncommitted changes.
|
||||
/// - Runs acceptance gates (cargo clippy + cargo nextest run / cargo test).
|
||||
/// - Stores the `CompletionReport` on the agent record.
|
||||
/// - Transitions status to `Completed` (gates passed) or `Failed` (gates failed).
|
||||
/// - Emits a `Done` event so `wait_for_agent` unblocks.
|
||||
pub async fn report_completion(
|
||||
&self,
|
||||
story_id: &str,
|
||||
agent_name: &str,
|
||||
summary: &str,
|
||||
) -> Result<CompletionReport, String> {
|
||||
let key = composite_key(story_id, agent_name);
|
||||
|
||||
// Verify agent exists, is Running, and grab its worktree path.
|
||||
let worktree_path = {
|
||||
let agents = self.agents.lock().map_err(|e| e.to_string())?;
|
||||
let agent = agents
|
||||
.get(&key)
|
||||
.ok_or_else(|| format!("No agent '{agent_name}' for story '{story_id}'"))?;
|
||||
|
||||
if agent.status != AgentStatus::Running {
|
||||
return Err(format!(
|
||||
"Agent '{agent_name}' for story '{story_id}' is not running (status: {}). \
|
||||
report_completion can only be called by a running agent.",
|
||||
agent.status
|
||||
));
|
||||
}
|
||||
|
||||
agent
|
||||
.worktree_info
|
||||
.as_ref()
|
||||
.map(|wt| wt.path.clone())
|
||||
.ok_or_else(|| {
|
||||
format!(
|
||||
"Agent '{agent_name}' for story '{story_id}' has no worktree. \
|
||||
Cannot run acceptance gates."
|
||||
)
|
||||
})?
|
||||
};
|
||||
|
||||
let path = worktree_path.clone();
|
||||
|
||||
// Run gate checks in a blocking thread to avoid stalling the async runtime.
|
||||
let (gates_passed, gate_output) = tokio::task::spawn_blocking(move || {
|
||||
// Step 1: Reject if worktree is dirty.
|
||||
check_uncommitted_changes(&path)?;
|
||||
// Step 2: Run clippy + tests and return (passed, output).
|
||||
run_acceptance_gates(&path)
|
||||
})
|
||||
.await
|
||||
.map_err(|e| format!("Gate check task panicked: {e}"))??;
|
||||
|
||||
let report = CompletionReport {
|
||||
summary: summary.to_string(),
|
||||
gates_passed,
|
||||
gate_output,
|
||||
};
|
||||
|
||||
// Store the completion report and advance status.
|
||||
let (tx, session_id) = {
|
||||
let mut agents = self.agents.lock().map_err(|e| e.to_string())?;
|
||||
let agent = agents.get_mut(&key).ok_or_else(|| {
|
||||
format!("Agent '{agent_name}' for story '{story_id}' disappeared during gate check")
|
||||
})?;
|
||||
agent.completion = Some(report.clone());
|
||||
agent.status = if gates_passed {
|
||||
AgentStatus::Completed
|
||||
} else {
|
||||
AgentStatus::Failed
|
||||
};
|
||||
(agent.tx.clone(), agent.session_id.clone())
|
||||
};
|
||||
|
||||
// Emit Done so wait_for_agent unblocks.
|
||||
let _ = tx.send(AgentEvent::Done {
|
||||
story_id: story_id.to_string(),
|
||||
agent_name: agent_name.to_string(),
|
||||
session_id,
|
||||
});
|
||||
|
||||
Ok(report)
|
||||
}
|
||||
|
||||
/// Return the port this server is running on.
|
||||
pub fn port(&self) -> u16 {
|
||||
self.port
|
||||
@@ -511,10 +612,135 @@ impl AgentPool {
|
||||
tx: tx.clone(),
|
||||
task_handle: None,
|
||||
event_log: Arc::new(Mutex::new(Vec::new())),
|
||||
completion: None,
|
||||
},
|
||||
);
|
||||
tx
|
||||
}
|
||||
|
||||
/// Test helper: inject an agent with a specific worktree path for testing
|
||||
/// gate-related logic.
|
||||
#[cfg(test)]
|
||||
pub fn inject_test_agent_with_path(
|
||||
&self,
|
||||
story_id: &str,
|
||||
agent_name: &str,
|
||||
status: AgentStatus,
|
||||
worktree_path: PathBuf,
|
||||
) -> broadcast::Sender<AgentEvent> {
|
||||
let (tx, _) = broadcast::channel::<AgentEvent>(64);
|
||||
let key = composite_key(story_id, agent_name);
|
||||
let mut agents = self.agents.lock().unwrap();
|
||||
agents.insert(
|
||||
key,
|
||||
StoryAgent {
|
||||
agent_name: agent_name.to_string(),
|
||||
status,
|
||||
worktree_info: Some(WorktreeInfo {
|
||||
path: worktree_path,
|
||||
branch: format!("feature/story-{story_id}"),
|
||||
base_branch: "master".to_string(),
|
||||
}),
|
||||
session_id: None,
|
||||
tx: tx.clone(),
|
||||
task_handle: None,
|
||||
event_log: Arc::new(Mutex::new(Vec::new())),
|
||||
completion: None,
|
||||
},
|
||||
);
|
||||
tx
|
||||
}
|
||||
}
|
||||
|
||||
// ── Acceptance-gate helpers ───────────────────────────────────────────────────
|
||||
|
||||
/// Check whether the given directory has any uncommitted git changes.
|
||||
/// Returns `Err` with a descriptive message if there are any.
|
||||
fn check_uncommitted_changes(path: &Path) -> Result<(), String> {
|
||||
let output = Command::new("git")
|
||||
.args(["status", "--porcelain"])
|
||||
.current_dir(path)
|
||||
.output()
|
||||
.map_err(|e| format!("Failed to run git status: {e}"))?;
|
||||
|
||||
let stdout = String::from_utf8_lossy(&output.stdout);
|
||||
if !stdout.trim().is_empty() {
|
||||
return Err(format!(
|
||||
"Worktree has uncommitted changes. Commit your work before calling \
|
||||
report_completion:\n{stdout}"
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Run `cargo clippy` and `cargo nextest run` (falling back to `cargo test`) in
|
||||
/// the given directory. Returns `(gates_passed, combined_output)`.
|
||||
fn run_acceptance_gates(path: &Path) -> Result<(bool, String), String> {
|
||||
let mut all_output = String::new();
|
||||
let mut all_passed = true;
|
||||
|
||||
// ── cargo clippy ──────────────────────────────────────────────
|
||||
let clippy = Command::new("cargo")
|
||||
.args(["clippy", "--all-targets", "--all-features"])
|
||||
.current_dir(path)
|
||||
.output()
|
||||
.map_err(|e| format!("Failed to run cargo clippy: {e}"))?;
|
||||
|
||||
all_output.push_str("=== cargo clippy ===\n");
|
||||
let clippy_stdout = String::from_utf8_lossy(&clippy.stdout);
|
||||
let clippy_stderr = String::from_utf8_lossy(&clippy.stderr);
|
||||
if !clippy_stdout.is_empty() {
|
||||
all_output.push_str(&clippy_stdout);
|
||||
}
|
||||
if !clippy_stderr.is_empty() {
|
||||
all_output.push_str(&clippy_stderr);
|
||||
}
|
||||
all_output.push('\n');
|
||||
|
||||
if !clippy.status.success() {
|
||||
all_passed = false;
|
||||
}
|
||||
|
||||
// ── cargo nextest run (fallback: cargo test) ──────────────────
|
||||
all_output.push_str("=== tests ===\n");
|
||||
|
||||
let (test_success, test_out) = match Command::new("cargo")
|
||||
.args(["nextest", "run"])
|
||||
.current_dir(path)
|
||||
.output()
|
||||
{
|
||||
Ok(o) => {
|
||||
let combined = format!(
|
||||
"{}{}",
|
||||
String::from_utf8_lossy(&o.stdout),
|
||||
String::from_utf8_lossy(&o.stderr)
|
||||
);
|
||||
(o.status.success(), combined)
|
||||
}
|
||||
Err(_) => {
|
||||
// nextest not available — fall back to cargo test
|
||||
let o = Command::new("cargo")
|
||||
.args(["test"])
|
||||
.current_dir(path)
|
||||
.output()
|
||||
.map_err(|e| format!("Failed to run cargo test: {e}"))?;
|
||||
let combined = format!(
|
||||
"{}{}",
|
||||
String::from_utf8_lossy(&o.stdout),
|
||||
String::from_utf8_lossy(&o.stderr)
|
||||
);
|
||||
(o.status.success(), combined)
|
||||
}
|
||||
};
|
||||
|
||||
all_output.push_str(&test_out);
|
||||
all_output.push('\n');
|
||||
|
||||
if !test_success {
|
||||
all_passed = false;
|
||||
}
|
||||
|
||||
Ok((all_passed, all_output))
|
||||
}
|
||||
|
||||
/// Spawn claude agent in a PTY and stream events through the broadcast channel.
|
||||
@@ -792,4 +1018,66 @@ mod tests {
|
||||
let info = pool.wait_for_agent("s5", "bot", 2000).await.unwrap();
|
||||
assert_eq!(info.story_id, "s5");
|
||||
}
|
||||
|
||||
// ── report_completion tests ────────────────────────────────────
|
||||
|
||||
#[tokio::test]
|
||||
async fn report_completion_rejects_nonexistent_agent() {
|
||||
let pool = AgentPool::new(3001);
|
||||
let result = pool
|
||||
.report_completion("no_story", "no_bot", "done")
|
||||
.await;
|
||||
assert!(result.is_err());
|
||||
let msg = result.unwrap_err();
|
||||
assert!(msg.contains("No agent"), "unexpected: {msg}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn report_completion_rejects_non_running_agent() {
|
||||
let pool = AgentPool::new(3001);
|
||||
pool.inject_test_agent("s6", "bot", AgentStatus::Completed);
|
||||
|
||||
let result = pool.report_completion("s6", "bot", "done").await;
|
||||
assert!(result.is_err());
|
||||
let msg = result.unwrap_err();
|
||||
assert!(
|
||||
msg.contains("not running"),
|
||||
"expected 'not running' in: {msg}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn report_completion_rejects_dirty_worktree() {
|
||||
use std::fs;
|
||||
use tempfile::tempdir;
|
||||
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo = tmp.path();
|
||||
|
||||
// Init a real git repo and make an initial commit
|
||||
Command::new("git")
|
||||
.args(["init"])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["commit", "--allow-empty", "-m", "init"])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
// Write an uncommitted file
|
||||
fs::write(repo.join("dirty.txt"), "not committed").unwrap();
|
||||
|
||||
let pool = AgentPool::new(3001);
|
||||
pool.inject_test_agent_with_path("s7", "bot", AgentStatus::Running, repo.to_path_buf());
|
||||
|
||||
let result = pool.report_completion("s7", "bot", "done").await;
|
||||
assert!(result.is_err());
|
||||
let msg = result.unwrap_err();
|
||||
assert!(
|
||||
msg.contains("uncommitted"),
|
||||
"expected 'uncommitted' in: {msg}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user