diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 050acdf0..7e62ea87 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -29,6 +29,7 @@ mod uninstall; mod update; mod vault; mod vent; +mod work_missing_worktree; pub(crate) mod work; mod workflow; diff --git a/src/cli/work.rs b/src/cli/work.rs index ecd9f6a7..b6b913b9 100644 --- a/src/cli/work.rs +++ b/src/cli/work.rs @@ -747,9 +747,9 @@ fn execute_work_impl( .as_str() .map(std::path::PathBuf::from); let Some(ref wpath) = worktree_path else { - let msg = "claim succeeded but no worktree was created (bad git state?)"; - release_and_comment(&mcp, item_id, msg, args.notify.as_deref()); - crate::ui::error(msg); + let msg = crate::cli::work_missing_worktree::missing_worktree_message(&claim); + release_and_comment(&mcp, item_id, &msg, args.notify.as_deref()); + crate::ui::error(&msg); // Structural: whatever broke the git worktree state (e.g. a stale // "prunable" registration, confirmed live for items #465/#466) won't // heal itself between attempts, so fail straight to terminal instead @@ -993,6 +993,7 @@ impl agentflare_jobs::InProcessExecutor for WorkItemExecutor { mod tests { use super::*; + include!("work_worktree_error_tests.rs"); #[test] fn job_failure_for_structural_setup_failure_is_fatal() { // Mirrors the "claim succeeded but no worktree was created" and diff --git a/src/cli/work_missing_worktree.rs b/src/cli/work_missing_worktree.rs new file mode 100644 index 00000000..9bb339b0 --- /dev/null +++ b/src/cli/work_missing_worktree.rs @@ -0,0 +1,41 @@ +//! Missing-worktree diagnostics when claim succeeds but provisioning fails. +//! Split out of `work.rs` for the LOC gate. + +/// Builds the user-facing message when `item::claim` returned `acquired` but +/// no `worktree_path`. The server already diagnoses most failures as +/// `worktree_error`; only fall back to the generic guess when it didn't. +pub(crate) fn missing_worktree_message(claim: &serde_json::Value) -> String { + if let Some(error) = claim["worktree_error"].as_str() { + if error.is_empty() { + "claim succeeded but no worktree was created (bad git state?)".to_string() + } else { + format!("claim succeeded but no worktree was created: {error}") + } + } else { + "claim succeeded but no worktree was created (bad git state?)".to_string() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn missing_worktree_message_surfaces_server_worktree_error() { + let claim = serde_json::json!({ + "worktree_error": "fatal: not a git repository (or any of the parent directories): .git" + }); + let msg = missing_worktree_message(&claim); + assert!(msg.contains("not a git repository")); + assert!(!msg.ends_with("(bad git state?)")); + } + + #[test] + fn missing_worktree_message_falls_back_to_generic_guess_without_worktree_error() { + let claim = serde_json::json!({ "status": "acquired" }); + assert_eq!( + missing_worktree_message(&claim), + "claim succeeded but no worktree was created (bad git state?)" + ); + } +} diff --git a/src/cli/work_worktree_error_tests.rs b/src/cli/work_worktree_error_tests.rs new file mode 100644 index 00000000..851100bd --- /dev/null +++ b/src/cli/work_worktree_error_tests.rs @@ -0,0 +1,58 @@ +// End-to-end coverage for worktree provisioning failures during `execute_work`. +// Split out of `work.rs` to keep that file under the LOC gate. +// Included from `work::tests` via `include!`. + +/// Drives `execute_work_impl` end-to-end through a real claim against a +/// `repo_root` that is deliberately NOT a git repo, so `item::claim`'s +/// `git worktree add` fails server-side and the response carries +/// `worktree_error` but no `worktree_path` -- the same server-side shape +/// `item_claim_response_includes_worktree_error_instead_of_silently_omitting_it` +/// (`mcp_server::tests::action_tests`) covers. Guards that +/// `missing_worktree_message` (unit-tested in `work_missing_worktree`) is +/// actually wired into this call site's `release_and_comment`, not just +/// defined and unused. +#[test] +fn execute_work_impl_posts_server_worktree_error_when_provisioning_fails() { + let tmp = tempfile::tempdir().unwrap(); + let repo_root = tmp.path().join("repo"); + std::fs::create_dir_all(&repo_root).unwrap(); + + crate::paths::test_support::with_temp_home(|| { + let seed_mcp = AgentflareMcp::for_project_dir(repo_root.clone()); + let item = seed_mcp + .with_backend_db(|conn| seeded_item(&seed_mcp, conn)) + .unwrap(); + let work_args = WorkArgs { + target: item.id.clone(), + agent: Some(agent_registry::Agent::ClaudeCode.as_str().to_string()), + timeout: DEFAULT_TIMEOUT_SECS, + idle_timeout: DEFAULT_IDLE_TIMEOUT_SECS, + max_turns: None, + max_cost_usd: None, + model: None, + notify: None, + repo_root: Some(repo_root.clone()), + }; + let mut log = Vec::new(); + let outcome = execute_work_impl(work_args, &mut log, |_, _, _, _, _, _, _, _, _, _| { + panic!("pipeline must not run when worktree provisioning failed"); + }); + + assert_eq!(outcome.exit_code, 1); + assert!( + outcome.fatal, + "worktree provisioning failure is structural, not retryable" + ); + + let comments = seed_mcp + .with_backend_db(|conn| agentflare_backend::comment::list_by_item(conn, &item.id)) + .unwrap() + .unwrap(); + assert_eq!(comments.len(), 1); + assert!( + !comments[0].body.ends_with("(bad git state?)"), + "comment must include the server's worktree_error, got: {}", + comments[0].body + ); + }); +} diff --git a/src/mcp_server/item.rs b/src/mcp_server/item.rs index 097005bf..ffd890d9 100644 --- a/src/mcp_server/item.rs +++ b/src/mcp_server/item.rs @@ -624,7 +624,18 @@ impl AgentflareMcp { Some(Err(e)) => { resp["worktree_error"] = serde_json::Value::String(e); } - None => {} + None => { + let reason = match (&item, &target_branch) { + (None, _) => { + "item record could not be read after claim".to_string() + } + (Some(_), None) => { + "target branch could not be resolved for worktree".to_string() + } + _ => "worktree creation was skipped (unknown reason)".to_string(), + }; + resp["worktree_error"] = serde_json::Value::String(reason); + } } resp.to_string() }