fix(mcp): reject done when push succeeds but PR creation silently fails

scripts/loc-gate.sh

@@ -21,6 +21,12 @@ ALLOWLIST=(
   # small, unrelated fix shouldn't be blocked on splitting. Frozen at
   # <= FROZEN_LIMIT; a real split is separate work.
   src/cli/work.rs
+  # At 1493 lines on master, item #109's push/PR-failure completion gate
+  # (mirroring the existing auto-commit-failure gate right above it) pushed
+  # it over LIMIT. Splitting item.rs's per-action handlers into submodules
+  # is worth doing but is a separate, larger refactor than a completion-
+  # correctness fix should carry. Frozen at <= FROZEN_LIMIT.
+  src/mcp_server/item.rs
 )
 
 cd "$(dirname "$0")/.."

src/mcp_server/item.rs

@@ -834,6 +834,10 @@ impl AgentflareMcp {
         // no-ops (logging) rather than trusting push/PR success alone as
         // proof nothing would be lost.
         let in_review = owns_claim && pr_url.is_some();
+        let diverged = item
+            .as_ref()
+            .zip(target_branch.as_ref())
+            .is_some_and(|(i, t)| crate::worktree::branch_diverged(i, &repo_root, t));
         // No PR resulted — either nothing was ever committed on the claimed
         // branch, or a real commit's push/PR failed for some other reason.
         // Only the former should block completion: marking an item
@@ -842,14 +846,24 @@ impl AgentflareMcp {
         // merely replies with text, with no tool use, previously exited 0
         // and sailed straight through to `mark_completed` below with zero
         // code changed.
-        let nothing_was_ever_committed = !in_review
-            && owns_claim
-            && item
-                .as_ref()
-                .zip(target_branch.as_ref())
-                .is_none_or(|(item, target)| {
-                    !crate::worktree::branch_diverged(item, &repo_root, target)
-                });
+        let nothing_was_ever_committed = !in_review && owns_claim && !diverged;
+        // A real commit exists but push/PR creation soft-failed and never
+        // produced a PR (item #109) -- hard-error instead of completing.
+        let push_or_pr_failed = !in_review && owns_claim && should_push && diverged;
+        if push_or_pr_failed {
+            let _ = self.comment_impl(CommentRequest {
+                action: "create".into(),
+                item_id: Some(item_id.clone()),
+                body: Some(format!(
+                    "## agentflare work — PR creation failed\n\nThe branch has real commits but no pull request resulted; check server logs for item {item_id}. Left in place rather than completed."
+                )),
+                ..Default::default()
+            });
+            return Err(ErrorData::internal_error(
+                format!("item {item_id}: real commits but no PR resulted -- not marking completed"),
+                None,
+            ));
+        }
         // Shared by both "nothing was ever committed" (worktree is clean by
         // definition, safe to remove) and a real completion: release the
         // lease so the item is genuinely available again — either for a

src/mcp_server/tests/item_pr_failure_tests.rs

@@ -0,0 +1,115 @@
+#[allow(unused_imports)]
+use super::*;
+
+// Item #109: a real commit landed on the claimed branch and `git push`
+// itself succeeded, but no PR resulted (here: `origin` is a local bare
+// repo, so `RepoId::resolve_from_remote` can't recognize it as a GitHub
+// remote and `push_and_open_pr` skips PR creation -- the same soft-fail
+// path a missing `gh` credential or a `gh pr create` error would take).
+// `done` must not fall through to `mark_completed`: a pushed branch with
+// no PR anywhere is not "in_review", and it's certainly not "completed".
+#[test]
+fn item_done_reports_a_hard_error_when_push_succeeds_but_no_pr_results() {
+    let tmp = tempfile::tempdir().unwrap();
+    let repo_dir = tempfile::tempdir().unwrap();
+    let repo_root = repo_dir.path().to_path_buf();
+    let origin_dir = tempfile::tempdir().unwrap();
+    let run_git = |dir: &std::path::Path, args: &[&str]| {
+        let out = std::process::Command::new("git")
+            .args(args)
+            .current_dir(dir)
+            .output()
+            .unwrap();
+        assert!(
+            out.status.success(),
+            "git {args:?} failed: {}",
+            String::from_utf8_lossy(&out.stderr)
+        );
+        out
+    };
+    run_git(origin_dir.path(), &["init", "--bare", "-b", "master"]);
+    run_git(&repo_root, &["init", "-b", "master"]);
+    run_git(&repo_root, &["config", "user.email", "test@test.com"]);
+    run_git(&repo_root, &["config", "user.name", "Test"]);
+    run_git(&repo_root, &["commit", "--allow-empty", "-m", "initial"]);
+    run_git(
+        &repo_root,
+        &[
+            "remote",
+            "add",
+            "origin",
+            origin_dir.path().to_str().unwrap(),
+        ],
+    );
+    run_git(&repo_root, &["push", "origin", "master"]);
+
+    let s = AgentflareMcp {
+        backend_db_override: Some(tmp.path().join("backend.db")),
+        backend_project_link_override: Some(tmp.path().join("project.json")),
+        worktree_repo_root_override: Some(repo_root),
+        ..Default::default()
+    };
+
+    let created: serde_json::Value =
+        serde_json::from_str(&s.item(Parameters(empty_item_create("Test"))).unwrap()).unwrap();
+    let item_id = created["id"].as_str().unwrap().to_string();
+
+    let claimed: serde_json::Value = serde_json::from_str(
+        &s.item(Parameters(ItemRequest {
+            action: "claim".into(),
+            id: Some(item_id.clone()),
+            ..Default::default()
+        }))
+        .unwrap(),
+    )
+    .unwrap();
+    let worktree_path = std::path::PathBuf::from(claimed["worktree_path"].as_str().unwrap());
+    std::fs::write(worktree_path.join("real_work.txt"), "real work").unwrap();
+
+    let err = s
+        .item(Parameters(ItemRequest {
+            action: "done".into(),
+            id: Some(item_id.clone()),
+            summary: Some("Did the real work.".into()),
+            ..Default::default()
+        }))
+        .unwrap_err();
+
+    assert!(
+        err.message.contains("no PR resulted") || err.message.contains("not marking completed"),
+        "error must call out the missing PR: {}",
+        err.message
+    );
+
+    let comments: serde_json::Value = serde_json::from_str(
+        &s.comment(Parameters(CommentRequest {
+            action: "list".into(),
+            item_id: Some(item_id.clone()),
+            ..Default::default()
+        }))
+        .unwrap(),
+    )
+    .unwrap();
+    let arr = comments.as_array().unwrap();
+    assert!(
+        arr.iter().any(|c| c["body"]
+            .as_str()
+            .unwrap_or_default()
+            .contains("PR creation failed")),
+        "a PR-failure comment must be posted on the item: {arr:?}"
+    );
+
+    let item_state: serde_json::Value = serde_json::from_str(
+        &s.item(Parameters(ItemRequest {
+            action: "get".into(),
+            id: Some(item_id),
+            ..Default::default()
+        }))
+        .unwrap(),
+    )
+    .unwrap();
+    assert!(
+        item_state["completed_at"].is_null(),
+        "a push with no resulting PR must not be treated as completion: {item_state:?}"
+    );
+}

src/mcp_server/tests/mod.rs

@@ -351,6 +351,7 @@ mod action_tests;
 mod artifact_tests;
 mod asset_tests;
 mod item_commit_failure_tests;
+mod item_pr_failure_tests;
 mod item_tests;
 mod item_tests_reporting;
 mod project_resolution_tests;