Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ mod uninstall;
mod update;
mod vault;
mod vent;
mod work_missing_worktree;
pub(crate) mod work;
mod workflow;

Expand Down
7 changes: 4 additions & 3 deletions src/cli/work.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
41 changes: 41 additions & 0 deletions src/cli/work_missing_worktree.rs
Original file line number Diff line number Diff line change
@@ -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?)"
);
}
}
58 changes: 58 additions & 0 deletions src/cli/work_worktree_error_tests.rs
Original file line number Diff line number Diff line change
@@ -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
);
});
}
13 changes: 12 additions & 1 deletion src/mcp_server/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down