story-kit: merge 226_bug_mergemaster_accepts_stories_without_squash_merging_code

server/src/agents.rs

@@ -2270,6 +2270,37 @@ pub fn move_story_to_current(project_root: &Path, story_id: &str) -> Result<(),
     Ok(())
 }
 
+/// Check whether a feature branch `feature/story-{story_id}` exists and has
+/// commits that are not yet on master.  Returns `true` when there is unmerged
+/// work, `false` when there is no branch or all its commits are already
+/// reachable from master.
+pub fn feature_branch_has_unmerged_changes(project_root: &Path, story_id: &str) -> bool {
+    let branch = format!("feature/story-{story_id}");
+
+    // Check if the branch exists.
+    let branch_check = Command::new("git")
+        .args(["rev-parse", "--verify", &branch])
+        .current_dir(project_root)
+        .output();
+    match branch_check {
+        Ok(out) if out.status.success() => {}
+        _ => return false, // No feature branch → nothing to merge.
+    }
+
+    // Check if the branch has commits not reachable from master.
+    let log = Command::new("git")
+        .args(["log", &format!("master..{branch}"), "--oneline"])
+        .current_dir(project_root)
+        .output();
+    match log {
+        Ok(out) => {
+            let stdout = String::from_utf8_lossy(&out.stdout);
+            !stdout.trim().is_empty()
+        }
+        Err(_) => false,
+    }
+}
+
 /// Move a story from `work/2_current/` to `work/5_done/` and auto-commit.
 ///
 /// * If the story is in `2_current/`, it is moved to `5_done/` and committed.
@@ -2778,18 +2809,27 @@ fn run_squash_merge(
     all_output.push('\n');
 
     if !commit.status.success() {
-        // Nothing to commit (e.g. empty diff) — treat as success.
+        // Bug 226: "nothing to commit" means the feature branch has no changes
+        // beyond what's already on master.  This must NOT be treated as success
+        // — it means the code was never actually merged.
         if commit_stderr.contains("nothing to commit")
             || commit_stdout.contains("nothing to commit")
         {
+            all_output.push_str(
+                "=== Nothing to commit — feature branch has no changes beyond master ===\n",
+            );
             cleanup_merge_workspace(project_root, &merge_wt_path, &merge_branch);
             return Ok(SquashMergeResult {
-                success: true,
+                success: false,
                 had_conflicts,
                 conflicts_resolved,
-                conflict_details,
+                conflict_details: Some(
+                    "Squash-merge resulted in an empty diff — the feature branch has no \
+                     code changes to merge into master."
+                        .to_string(),
+                ),
                 output: all_output,
-                gates_passed: true,
+                gates_passed: false,
             });
         }
         cleanup_merge_workspace(project_root, &merge_wt_path, &merge_branch);
@@ -2803,6 +2843,39 @@ fn run_squash_merge(
         });
     }
 
+    // ── Bug 226: Verify the commit contains real code changes ─────
+    // If the merge only brought in .story_kit/ files (pipeline file moves),
+    // there are no actual code changes to land on master.  Abort.
+    {
+        let diff_check = Command::new("git")
+            .args(["diff", "--name-only", "HEAD~1..HEAD"])
+            .current_dir(&merge_wt_path)
+            .output()
+            .map_err(|e| format!("Failed to check merge diff: {e}"))?;
+        let changed_files = String::from_utf8_lossy(&diff_check.stdout);
+        let has_code_changes = changed_files
+            .lines()
+            .any(|f| !f.starts_with(".story_kit/"));
+        if !has_code_changes {
+            all_output.push_str(
+                "=== Merge commit contains only .story_kit/ file moves, no code changes ===\n",
+            );
+            cleanup_merge_workspace(project_root, &merge_wt_path, &merge_branch);
+            return Ok(SquashMergeResult {
+                success: false,
+                had_conflicts,
+                conflicts_resolved,
+                conflict_details: Some(
+                    "Feature branch has no code changes outside .story_kit/ — only \
+                     pipeline file moves were found."
+                        .to_string(),
+                ),
+                output: all_output,
+                gates_passed: false,
+            });
+        }
+    }
+
     // ── Run component setup from project.toml (same as worktree creation) ──────────
     {
         let config = ProjectConfig::load(&merge_wt_path).unwrap_or_default();
@@ -5909,10 +5982,10 @@ theirs
         );
     }
 
-    /// Verifies that `run_squash_merge` returns `success: true` with an empty
+    /// Bug 226: Verifies that `run_squash_merge` returns `success: false` when
-    /// diff when the feature branch code is already on master.
+    /// the feature branch has no changes beyond what's already on master (empty diff).
     #[tokio::test]
-    async fn squash_merge_empty_diff_succeeds() {
+    async fn squash_merge_empty_diff_fails() {
         use std::fs;
         use tempfile::tempdir;
 
@@ -5939,8 +6012,6 @@ theirs
             .current_dir(repo)
             .output()
             .unwrap();
-        // Make a commit that doesn't actually change anything meaningful
-        // (e.g. the code was already cherry-picked to master).
         Command::new("git")
             .args(["checkout", "master"])
             .current_dir(repo)
@@ -5950,10 +6021,10 @@ theirs
         let result =
             run_squash_merge(repo, "feature/story-empty_test", "empty_test").unwrap();
 
-        // Empty diff should be treated as success (nothing to merge).
+        // Bug 226: empty diff must NOT be treated as success.
         assert!(
-            result.success,
+            !result.success,
-            "empty diff merge should succeed: {}",
+            "empty diff merge must fail, not silently succeed: {}",
             result.output
         );
 
@@ -5964,6 +6035,120 @@ theirs
         );
     }
 
+    /// Bug 226: Verifies that `run_squash_merge` fails when the feature branch
+    /// only contains .story_kit/ file moves with no real code changes.
+    #[tokio::test]
+    async fn squash_merge_md_only_changes_fails() {
+        use std::fs;
+        use tempfile::tempdir;
+
+        let tmp = tempdir().unwrap();
+        let repo = tmp.path();
+        init_git_repo(repo);
+
+        // Create a feature branch that only moves a .story_kit/ file.
+        Command::new("git")
+            .args(["checkout", "-b", "feature/story-md_only_test"])
+            .current_dir(repo)
+            .output()
+            .unwrap();
+        let sk_dir = repo.join(".story_kit/work/2_current");
+        fs::create_dir_all(&sk_dir).unwrap();
+        fs::write(
+            sk_dir.join("md_only_test.md"),
+            "---\nname: Test\n---\n",
+        )
+        .unwrap();
+        Command::new("git")
+            .args(["add", "."])
+            .current_dir(repo)
+            .output()
+            .unwrap();
+        Command::new("git")
+            .args(["commit", "-m", "move story file"])
+            .current_dir(repo)
+            .output()
+            .unwrap();
+        Command::new("git")
+            .args(["checkout", "master"])
+            .current_dir(repo)
+            .output()
+            .unwrap();
+
+        let result =
+            run_squash_merge(repo, "feature/story-md_only_test", "md_only_test").unwrap();
+
+        // The squash merge will commit the .story_kit/ file, but should fail because
+        // there are no code changes outside .story_kit/.
+        assert!(
+            !result.success,
+            "merge with only .story_kit/ changes must fail: {}",
+            result.output
+        );
+
+        // Cleanup should still happen.
+        assert!(
+            !repo.join(".story_kit/merge_workspace").exists(),
+            "merge workspace should be cleaned up"
+        );
+    }
+
+    /// Bug 226: feature_branch_has_unmerged_changes returns true when the
+    /// feature branch has commits not on master.
+    #[test]
+    fn feature_branch_has_unmerged_changes_detects_unmerged_code() {
+        use std::fs;
+        use tempfile::tempdir;
+
+        let tmp = tempdir().unwrap();
+        let repo = tmp.path();
+        init_git_repo(repo);
+
+        // Create a feature branch with a code commit.
+        Command::new("git")
+            .args(["checkout", "-b", "feature/story-50_story_test"])
+            .current_dir(repo)
+            .output()
+            .unwrap();
+        fs::write(repo.join("feature.rs"), "fn main() {}").unwrap();
+        Command::new("git")
+            .args(["add", "."])
+            .current_dir(repo)
+            .output()
+            .unwrap();
+        Command::new("git")
+            .args(["commit", "-m", "add feature"])
+            .current_dir(repo)
+            .output()
+            .unwrap();
+        Command::new("git")
+            .args(["checkout", "master"])
+            .current_dir(repo)
+            .output()
+            .unwrap();
+
+        assert!(
+            feature_branch_has_unmerged_changes(repo, "50_story_test"),
+            "should detect unmerged changes on feature branch"
+        );
+    }
+
+    /// Bug 226: feature_branch_has_unmerged_changes returns false when no
+    /// feature branch exists.
+    #[test]
+    fn feature_branch_has_unmerged_changes_false_when_no_branch() {
+        use tempfile::tempdir;
+
+        let tmp = tempdir().unwrap();
+        let repo = tmp.path();
+        init_git_repo(repo);
+
+        assert!(
+            !feature_branch_has_unmerged_changes(repo, "99_nonexistent"),
+            "should return false when no feature branch"
+        );
+    }
+
     /// Verifies that stale merge_workspace directories from previous failed
     /// merges are cleaned up before a new merge attempt.
     #[tokio::test]

server/src/http/mcp.rs

@@ -1,4 +1,4 @@
-use crate::agents::{close_bug_to_archive, move_story_to_archived, move_story_to_merge, move_story_to_qa, PipelineStage};
+use crate::agents::{close_bug_to_archive, feature_branch_has_unmerged_changes, move_story_to_archived, move_story_to_merge, move_story_to_qa, PipelineStage};
 use crate::config::ProjectConfig;
 use crate::log_buffer;
 use crate::slog;
@@ -1432,6 +1432,17 @@ fn tool_accept_story(args: &Value, ctx: &AppContext) -> Result<String, String> {
         .ok_or("Missing required argument: story_id")?;
 
     let project_root = ctx.agents.get_project_root(&ctx.state)?;
+
+    // Bug 226: Refuse to accept if the feature branch has unmerged code.
+    // The code must be squash-merged via merge_agent_work first.
+    if feature_branch_has_unmerged_changes(&project_root, story_id) {
+        return Err(format!(
+            "Cannot accept story '{story_id}': feature branch 'feature/story-{story_id}' \
+             has unmerged changes. Use merge_agent_work to squash-merge the code into \
+             master first."
+        ));
+    }
+
     move_story_to_archived(&project_root, story_id)?;
     ctx.agents.remove_agents_for_story(story_id);
 
@@ -3113,6 +3124,76 @@ mod tests {
         assert!(result.is_err());
     }
 
+    /// Bug 226: accept_story must refuse when the feature branch has unmerged code.
+    #[test]
+    fn tool_accept_story_refuses_when_feature_branch_has_unmerged_code() {
+        let tmp = tempfile::tempdir().unwrap();
+        setup_git_repo_in(tmp.path());
+
+        // Create a feature branch with code changes.
+        std::process::Command::new("git")
+            .args(["checkout", "-b", "feature/story-50_story_test"])
+            .current_dir(tmp.path())
+            .output()
+            .unwrap();
+        std::fs::write(tmp.path().join("feature.rs"), "fn main() {}").unwrap();
+        std::process::Command::new("git")
+            .args(["add", "."])
+            .current_dir(tmp.path())
+            .output()
+            .unwrap();
+        std::process::Command::new("git")
+            .args(["commit", "-m", "add feature"])
+            .current_dir(tmp.path())
+            .output()
+            .unwrap();
+        std::process::Command::new("git")
+            .args(["checkout", "master"])
+            .current_dir(tmp.path())
+            .output()
+            .unwrap();
+
+        // Create story file in current/ so move_story_to_archived would work.
+        let current_dir = tmp.path().join(".story_kit/work/2_current");
+        std::fs::create_dir_all(&current_dir).unwrap();
+        std::fs::write(
+            current_dir.join("50_story_test.md"),
+            "---\nname: Test\n---\n",
+        )
+        .unwrap();
+
+        let ctx = test_ctx(tmp.path());
+        let result =
+            tool_accept_story(&json!({"story_id": "50_story_test"}), &ctx);
+        assert!(result.is_err(), "should refuse when feature branch has unmerged code");
+        let err = result.unwrap_err();
+        assert!(
+            err.contains("unmerged"),
+            "error should mention unmerged changes: {err}"
+        );
+    }
+
+    /// Bug 226: accept_story succeeds when no feature branch exists (e.g. manual stories).
+    #[test]
+    fn tool_accept_story_succeeds_when_no_feature_branch() {
+        let tmp = tempfile::tempdir().unwrap();
+        setup_git_repo_in(tmp.path());
+
+        // Create story file in current/ (no feature branch).
+        let current_dir = tmp.path().join(".story_kit/work/2_current");
+        std::fs::create_dir_all(&current_dir).unwrap();
+        std::fs::write(
+            current_dir.join("51_story_no_branch.md"),
+            "---\nname: No Branch\n---\n",
+        )
+        .unwrap();
+
+        let ctx = test_ctx(tmp.path());
+        let result =
+            tool_accept_story(&json!({"story_id": "51_story_no_branch"}), &ctx);
+        assert!(result.is_ok(), "should succeed when no feature branch: {result:?}");
+    }
+
     // ── tool_check_criterion tests ────────────────────────────────
 
     #[test]