huskies: merge 560_story_make_merge_agent_work_return_results_like_run_tests_instead_of_polling
This commit is contained in:
@@ -24,16 +24,23 @@ impl AgentPool {
|
||||
project_root: &Path,
|
||||
story_id: &str,
|
||||
) -> Result<(), String> {
|
||||
// Guard against double-starts.
|
||||
// Guard against double-starts; clear any completed/failed entry so the
|
||||
// caller can retry without needing to call a separate cleanup step.
|
||||
{
|
||||
let jobs = self.merge_jobs.lock().map_err(|e| e.to_string())?;
|
||||
if let Some(job) = jobs.get(story_id)
|
||||
&& matches!(job.status, crate::agents::merge::MergeJobStatus::Running)
|
||||
{
|
||||
return Err(format!(
|
||||
"Merge already in progress for '{story_id}'. \
|
||||
Use get_merge_status to poll for completion."
|
||||
));
|
||||
let mut jobs = self.merge_jobs.lock().map_err(|e| e.to_string())?;
|
||||
if let Some(job) = jobs.get(story_id) {
|
||||
match &job.status {
|
||||
crate::agents::merge::MergeJobStatus::Running => {
|
||||
return Err(format!(
|
||||
"Merge already in progress for '{story_id}'. \
|
||||
Use get_merge_status to poll for completion."
|
||||
));
|
||||
}
|
||||
// Completed or Failed: clear stale entry so we can start fresh.
|
||||
_ => {
|
||||
jobs.remove(story_id);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -6,7 +6,10 @@ use crate::slog;
|
||||
use crate::slog_warn;
|
||||
use serde_json::{Value, json};
|
||||
|
||||
pub(super) fn tool_merge_agent_work(args: &Value, ctx: &AppContext) -> Result<String, String> {
|
||||
pub(super) async fn tool_merge_agent_work(
|
||||
args: &Value,
|
||||
ctx: &AppContext,
|
||||
) -> Result<String, String> {
|
||||
let story_id = args
|
||||
.get("story_id")
|
||||
.and_then(|v| v.as_str())
|
||||
@@ -16,53 +19,26 @@ pub(super) fn tool_merge_agent_work(args: &Value, ctx: &AppContext) -> Result<St
|
||||
ctx.agents.start_merge_agent_work(&project_root, story_id)?;
|
||||
|
||||
// Block until the merge completes instead of returning immediately.
|
||||
// Uses tokio::time::sleep so the async executor is not blocked.
|
||||
// This prevents the mergemaster from burning all its turns polling
|
||||
// get_merge_status in a tight loop.
|
||||
let sid = story_id.to_string();
|
||||
let agents = ctx.agents.clone();
|
||||
loop {
|
||||
std::thread::sleep(std::time::Duration::from_secs(10));
|
||||
tokio::time::sleep(std::time::Duration::from_secs(10)).await;
|
||||
if let Some(job) = agents.get_merge_status(&sid) {
|
||||
match &job.status {
|
||||
crate::agents::merge::MergeJobStatus::Running => continue,
|
||||
_ => return tool_get_merge_status_inner(&sid, &job),
|
||||
_ => break,
|
||||
}
|
||||
} else {
|
||||
return Err(format!("Merge job disappeared for '{sid}'."));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn tool_get_merge_status_inner(
|
||||
story_id: &str,
|
||||
job: &crate::agents::merge::MergeJob,
|
||||
) -> Result<String, String> {
|
||||
match &job.status {
|
||||
crate::agents::merge::MergeJobStatus::Running => serde_json::to_string_pretty(&json!({
|
||||
"story_id": story_id,
|
||||
"status": "running",
|
||||
"message": "Merge pipeline is still running."
|
||||
}))
|
||||
.map_err(|e| format!("Serialization error: {e}")),
|
||||
crate::agents::merge::MergeJobStatus::Completed(report) => {
|
||||
serde_json::to_string_pretty(&json!({
|
||||
"story_id": story_id,
|
||||
"status": "completed",
|
||||
"success": report.success,
|
||||
"had_conflicts": report.had_conflicts,
|
||||
"conflicts_resolved": report.conflicts_resolved,
|
||||
"gates_passed": report.gates_passed,
|
||||
"gate_output": report.gate_output,
|
||||
}))
|
||||
.map_err(|e| format!("Serialization error: {e}"))
|
||||
}
|
||||
crate::agents::merge::MergeJobStatus::Failed(err) => serde_json::to_string_pretty(&json!({
|
||||
"story_id": story_id,
|
||||
"status": "failed",
|
||||
"error": err,
|
||||
}))
|
||||
.map_err(|e| format!("Serialization error: {e}")),
|
||||
}
|
||||
// Return the full result (same fields as get_merge_status) so the caller
|
||||
// has everything it needs without a second round-trip.
|
||||
tool_get_merge_status(args, ctx)
|
||||
}
|
||||
|
||||
pub(super) fn tool_get_merge_status(args: &Value, ctx: &AppContext) -> Result<String, String> {
|
||||
@@ -80,7 +56,7 @@ pub(super) fn tool_get_merge_status(args: &Value, ctx: &AppContext) -> Result<St
|
||||
serde_json::to_string_pretty(&json!({
|
||||
"story_id": story_id,
|
||||
"status": "running",
|
||||
"message": "Merge pipeline is still running. Poll again in 10-15 seconds."
|
||||
"message": "Merge pipeline is still running."
|
||||
}))
|
||||
.map_err(|e| format!("Serialization error: {e}"))
|
||||
}
|
||||
@@ -273,11 +249,11 @@ mod tests {
|
||||
assert!(!req_names.contains(&"agent_name"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_merge_agent_work_missing_story_id() {
|
||||
#[tokio::test]
|
||||
async fn tool_merge_agent_work_missing_story_id() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let ctx = test_ctx(tmp.path());
|
||||
let result = tool_merge_agent_work(&json!({}), &ctx);
|
||||
let result = tool_merge_agent_work(&json!({}), &ctx).await;
|
||||
assert!(result.is_err());
|
||||
assert!(result.unwrap_err().contains("story_id"));
|
||||
}
|
||||
|
||||
@@ -678,7 +678,7 @@ fn handle_tools_list(id: Option<Value>) -> JsonRpcResponse {
|
||||
},
|
||||
{
|
||||
"name": "merge_agent_work",
|
||||
"description": "Start the mergemaster pipeline for a completed story as a background job. Returns immediately — poll get_merge_status(story_id) until the merge completes or fails. The pipeline squash-merges the feature branch into master, runs quality gates, moves the story to done, and cleans up.",
|
||||
"description": "Run the mergemaster pipeline for a completed story. Blocks until the merge completes or fails, then returns the full result — no polling needed. The pipeline squash-merges the feature branch into master, runs quality gates, moves the story to done, and cleans up.",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
@@ -696,7 +696,7 @@ fn handle_tools_list(id: Option<Value>) -> JsonRpcResponse {
|
||||
},
|
||||
{
|
||||
"name": "get_merge_status",
|
||||
"description": "Check the status of a merge_agent_work background job. Returns running/completed/failed. When completed, includes the full merge report with conflict details, gate output, and whether the story was archived.",
|
||||
"description": "Check the cached result of a merge_agent_work job. Returns the full merge report immediately — no polling needed. Useful if merge_agent_work already returned but you need the result again.",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
@@ -1254,7 +1254,7 @@ async fn handle_tools_call(id: Option<Value>, params: &Value, ctx: &AppContext)
|
||||
"create_refactor" => story_tools::tool_create_refactor(&args, ctx),
|
||||
"list_refactors" => story_tools::tool_list_refactors(ctx),
|
||||
// Mergemaster tools
|
||||
"merge_agent_work" => merge_tools::tool_merge_agent_work(&args, ctx),
|
||||
"merge_agent_work" => merge_tools::tool_merge_agent_work(&args, ctx).await,
|
||||
"get_merge_status" => merge_tools::tool_get_merge_status(&args, ctx),
|
||||
"move_story_to_merge" => merge_tools::tool_move_story_to_merge(&args, ctx).await,
|
||||
"report_merge_failure" => merge_tools::tool_report_merge_failure(&args, ctx),
|
||||
|
||||
Reference in New Issue
Block a user