huskies: merge 877

server/src/chat/transport/matrix/assign.rs

@@ -1,16 +1,17 @@
 //! Assign command: pre-assign or re-assign a coder model to a story.
 //!
-//! `{bot_name} assign {number} {model}` finds the story by number, updates the
+//! `{bot_name} assign {number} {model}` finds the story by number, writes the
-//! `agent` field in its front matter, and — when a coder is already running on
+//! agent name into the typed CRDT `agent` register, and — when a coder is
-//! the story — stops the current coder and starts the newly-assigned one.
+//! already running on the story — stops it and starts the newly-assigned one
+//! via [`crate::service::work_item::assign_and_start`].
 //!
 //! When no coder is running (the story has not been started yet), the command
-//! behaves as before: it simply persists the assignment in the front matter so
+//! persists the assignment in the CRDT register so the next `start` invocation
-//! that the next `start` invocation picks it up automatically.
+//! picks it up automatically.
 
 use crate::agents::{AgentPool, AgentStatus};
 use crate::chat::util::strip_bot_mention;
-use crate::io::story_metadata::{parse_front_matter, set_front_matter_field};
+use crate::io::story_metadata::parse_front_matter;
 use std::path::Path;
 
 /// A parsed assign command from a Matrix message body.
@@ -77,10 +78,14 @@ pub fn resolve_agent_name(model: &str) -> String {
 
 /// Handle an assign command asynchronously.
 ///
-/// Finds the work item by `story_number` across all pipeline stages, updates
+/// Finds the work item by `story_number` across all pipeline stages, writes
-/// the `agent` field in its front matter, and — if a coder is currently
+/// the agent pin to the CRDT register, and — if a coder is currently running
-/// running on the story — stops it and starts the newly-assigned agent.
+/// on the story — stops it and starts the newly-assigned agent via
-/// Returns a markdown-formatted response string.
+/// [`crate::service::work_item::assign_and_start`].
+///
+/// When no coder is running the assignment is persisted in the CRDT so the
+/// next `start` invocation picks it up automatically.  Returns a
+/// markdown-formatted response string.
 pub async fn handle_assign(
     bot_name: &str,
     story_number: &str,
@@ -88,7 +93,7 @@ pub async fn handle_assign(
     project_root: &Path,
     agents: &AgentPool,
 ) -> String {
-    // Find the story by numeric prefix: CRDT → content store → filesystem.
+    // Parse: find the story by numeric prefix (CRDT → content store → filesystem).
     let (story_id, _stage_dir, _path, content) =
         match crate::chat::lookup::find_story_by_number(project_root, story_number) {
             Some(found) => found,
@@ -97,33 +102,13 @@ pub async fn handle_assign(
             }
         };
 
-    let current_content = content.or_else(|| crate::db::read_content(&story_id));
+    let story_name = content
-
+        .or_else(|| crate::db::read_content(&story_id))
-    let story_name = current_content
+        .and_then(|c| parse_front_matter(&c).ok().and_then(|m| m.name))
-        .as_ref()
-        .and_then(|c| parse_front_matter(c).ok().and_then(|m| m.name))
         .unwrap_or_else(|| story_id.clone());
 
     let agent_name = resolve_agent_name(model_str);
 
-    // Write `agent: <agent_name>` into the story's front matter via content store.
-    let write_result = match current_content {
-        Some(contents) => {
-            let updated = set_front_matter_field(&contents, "agent", &agent_name);
-            crate::db::write_item_with_content(&story_id, &_stage_dir, &updated);
-            Ok(())
-        }
-        None => Err(format!("Story content not found for {story_id}")),
-    };
-
-    if let Err(e) = write_result {
-        return format!("Failed to assign model to **{story_name}**: {e}");
-    }
-
-    // Mirror the assignment into the CRDT register so the in-memory pipeline
-    // state stays consistent with the front-matter.
-    crate::crdt_state::set_agent(&story_id, Some(&agent_name));
-
     // Check whether a coder is already running on this story.
     let running_coders: Vec<_> = agents
         .list_agents()
@@ -137,14 +122,15 @@ pub async fn handle_assign(
         .collect();
 
     if running_coders.is_empty() {
-        // No coder running — just persist the assignment.
+        // No coder running — persist the CRDT agent pin for the future start.
+        crate::crdt_state::set_agent(&story_id, Some(&agent_name));
         return format!(
             "Assigned **{agent_name}** to **{story_name}** (story {story_number}). \
              The model will be used when the story starts."
         );
     }
 
-    // Stop each running coder, then start the newly assigned one.
+    // Stop each running coder, then assign+start the newly-assigned one.
     let stopped: Vec<String> = running_coders
         .iter()
         .map(|a| a.agent_name.clone())
@@ -169,8 +155,8 @@ pub async fn handle_assign(
         story_id
     );
 
-    match agents
+    // Service call: persist CRDT agent pin and start the new agent.
-        .start_agent(project_root, &story_id, Some(&agent_name), None, None)
+    match crate::service::work_item::assign_and_start(&story_id, &agent_name, project_root, agents)
         .await
     {
         Ok(info) => {
@@ -317,7 +303,8 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn handle_assign_writes_front_matter_when_no_coder_running() {
+    async fn handle_assign_sets_crdt_agent_when_no_coder_running() {
+        crate::crdt_state::init_for_test();
         let tmp = tempfile::tempdir().unwrap();
         write_story_file(
             tmp.path(),
@@ -325,6 +312,19 @@ mod tests {
             "9972_story_test.md",
             "---\nname: Test Feature\n---\n\n# Story 9972\n",
         );
+        // Seed CRDT so set_agent can write to the item.
+        crate::crdt_state::write_item(
+            "9972_story_test",
+            "1_backlog",
+            Some("Test Feature"),
+            None,
+            None,
+            None,
+            None,
+            None,
+            None,
+            None,
+        );
 
         let agents = std::sync::Arc::new(AgentPool::new_test(3000));
         let response = handle_assign("Timmy", "9972", "opus", tmp.path(), &agents).await;
@@ -343,16 +343,24 @@ mod tests {
             "response should indicate assignment for future start: {response}"
         );
 
-        let contents = crate::db::read_content("9972_story_test")
+        // CRDT register should be set (no longer checks YAML front matter).
-            .expect("content store should have updated content");
+        let dump = crate::crdt_state::dump_crdt_state(Some("9972_story_test"));
-        assert!(
+        let item = dump
-            contents.contains("agent: coder-opus"),
+            .items
-            "front matter should contain agent field: {contents}"
+            .iter()
+            .find(|i| i.story_id.as_deref() == Some("9972_story_test"))
+            .expect("item must be in CRDT");
+        assert_eq!(
+            item.agent.as_deref(),
+            Some("coder-opus"),
+            "CRDT agent register should be set: {:?}",
+            item.agent
         );
     }
 
     #[tokio::test]
     async fn handle_assign_with_already_prefixed_name_does_not_double_prefix() {
+        crate::crdt_state::init_for_test();
         let tmp = tempfile::tempdir().unwrap();
         write_story_file(
             tmp.path(),
@@ -360,6 +368,18 @@ mod tests {
             "9973_story_small.md",
             "---\nname: Small Story\n---\n",
         );
+        crate::crdt_state::write_item(
+            "9973_story_small",
+            "1_backlog",
+            Some("Small Story"),
+            None,
+            None,
+            None,
+            None,
+            None,
+            None,
+            None,
+        );
 
         let agents = std::sync::Arc::new(AgentPool::new_test(3000));
         let response = handle_assign("Timmy", "9973", "coder-opus", tmp.path(), &agents).await;
@@ -373,16 +393,24 @@ mod tests {
             "must not double-prefix: {response}"
         );
 
-        let contents = crate::db::read_content("9973_story_small")
+        // CRDT must have coder-opus, not coder-coder-opus.
-            .expect("content store should have updated content");
+        let dump = crate::crdt_state::dump_crdt_state(Some("9973_story_small"));
-        assert!(
+        let item = dump
-            contents.contains("agent: coder-opus"),
+            .items
-            "must write coder-opus, not coder-coder-opus: {contents}"
+            .iter()
+            .find(|i| i.story_id.as_deref() == Some("9973_story_small"))
+            .expect("item must be in CRDT");
+        assert_eq!(
+            item.agent.as_deref(),
+            Some("coder-opus"),
+            "must write coder-opus, not coder-coder-opus: {:?}",
+            item.agent
         );
     }
 
     #[tokio::test]
     async fn handle_assign_overwrites_existing_agent_field() {
+        crate::crdt_state::init_for_test();
         let tmp = tempfile::tempdir().unwrap();
         write_story_file(
             tmp.path(),
@@ -390,19 +418,34 @@ mod tests {
             "9974_story_existing.md",
             "---\nname: Existing\nagent: coder-sonnet\n---\n",
         );
+        crate::crdt_state::write_item(
+            "9974_story_existing",
+            "1_backlog",
+            Some("Existing"),
+            Some("coder-sonnet"),
+            None,
+            None,
+            None,
+            None,
+            None,
+            None,
+        );
 
         let agents = std::sync::Arc::new(AgentPool::new_test(3000));
         handle_assign("Timmy", "9974", "opus", tmp.path(), &agents).await;
 
-        let contents = crate::db::read_content("9974_story_existing")
+        // CRDT agent register must be updated to the new value.
-            .expect("content store should have updated content");
+        let dump = crate::crdt_state::dump_crdt_state(Some("9974_story_existing"));
-        assert!(
+        let item = dump
-            contents.contains("agent: coder-opus"),
+            .items
-            "should overwrite old agent: {contents}"
+            .iter()
-        );
+            .find(|i| i.story_id.as_deref() == Some("9974_story_existing"))
-        assert!(
+            .expect("item must be in CRDT");
-            !contents.contains("coder-sonnet"),
+        assert_eq!(
-            "old agent should no longer appear: {contents}"
+            item.agent.as_deref(),
+            Some("coder-opus"),
+            "CRDT agent must be updated to coder-opus: {:?}",
+            item.agent
         );
     }
 

server/src/http/mcp/agent_tools/lifecycle.rs

@@ -15,11 +15,22 @@ pub(crate) async fn tool_start_agent(args: &Value, ctx: &AppContext) -> Result<S
     let agent_name = args.get("agent_name").and_then(|v| v.as_str());
 
     let project_root = ctx.services.agents.get_project_root(&ctx.state)?;
-    let info = ctx
+    // When an explicit agent_name is provided, persist the agent pin in the
-        .services
+    // CRDT register before starting — same path as the Matrix `assign` command.
-        .agents
+    let info = if let Some(name) = agent_name {
-        .start_agent(&project_root, story_id, agent_name, None, None)
+        crate::service::work_item::assign_and_start(
-        .await?;
+            story_id,
+            name,
+            &project_root,
+            &ctx.services.agents,
+        )
+        .await?
+    } else {
+        ctx.services
+            .agents
+            .start_agent(&project_root, story_id, None, None, None)
+            .await?
+    };
 
     // Snapshot coverage baseline from the most recent coverage report (best-effort).
     if let Some(pct) = read_coverage_percent_from_json(&project_root)

server/src/service/work_item/assign.rs

@@ -0,0 +1,139 @@
+//! Assign-and-start: unified service for pinning an agent to a story and starting it.
+//!
+//! Both the Matrix `assign` command and the MCP `start_agent` tool delegate
+//! here when an explicit `agent_name` is provided, ensuring that the CRDT
+//! `agent` register is always written before the agent is spawned.  This
+//! eliminates the asymmetry where the chat path wrote YAML + CRDT while the
+//! MCP path only called `start_agent`.
+
+use crate::agents::{AgentInfo, AgentPool};
+use std::path::Path;
+
+/// Pin an agent to a story in the CRDT and immediately start it.
+///
+/// Writes `agent_name` into the typed CRDT `agent` register for `story_id`,
+/// then delegates to [`AgentPool::start_agent`] with the explicit name.
+///
+/// Callers that need to stop an already-running agent first (e.g. the Matrix
+/// `assign` command replacing a running coder) must do so before calling this
+/// function; the pool's concurrency guard will reject a duplicate start.
+///
+/// Returns the [`AgentInfo`] for the newly started agent, or an error string.
+pub async fn assign_and_start(
+    story_id: &str,
+    agent_name: &str,
+    project_root: &Path,
+    agents: &AgentPool,
+) -> Result<AgentInfo, String> {
+    crate::crdt_state::set_agent(story_id, Some(agent_name));
+    agents
+        .start_agent(project_root, story_id, Some(agent_name), None, None)
+        .await
+}
+
+// ---------------------------------------------------------------------------
+// Tests
+// ---------------------------------------------------------------------------
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::agents::AgentPool;
+    use std::sync::Arc;
+
+    fn make_agents() -> Arc<AgentPool> {
+        Arc::new(AgentPool::new_test(3000))
+    }
+
+    /// Regression test (story 877, AC4): assigning via the Matrix chat path
+    /// and via the MCP `start_agent` path both call `assign_and_start`, which
+    /// writes the typed CRDT `agent` register before spawning.  This test
+    /// verifies the CRDT write is unconditional even when the pool call fails
+    /// (no git repo in the test environment).
+    #[tokio::test]
+    async fn assign_and_start_writes_crdt_agent_register_before_pool_call() {
+        crate::crdt_state::init_for_test();
+        let story_id = "8770_story_assign_regression_crdt";
+
+        // Seed the CRDT so set_agent can find the item.
+        crate::crdt_state::write_item(
+            story_id,
+            "2_current",
+            Some("Assign Regression"),
+            None,
+            None,
+            None,
+            None,
+            None,
+            None,
+            None,
+        );
+
+        let tmp = tempfile::tempdir().unwrap();
+        let agents = make_agents();
+
+        // The pool call will fail (no git repo / project config), but the CRDT
+        // write must succeed before it is attempted.
+        let _ = assign_and_start(story_id, "coder-opus", tmp.path(), &agents).await;
+
+        let dump = crate::crdt_state::dump_crdt_state(Some(story_id));
+        let item = dump
+            .items
+            .iter()
+            .find(|i| i.story_id.as_deref() == Some(story_id))
+            .expect("item must be in CRDT");
+        assert_eq!(
+            item.agent.as_deref(),
+            Some("coder-opus"),
+            "CRDT agent register must be set to coder-opus after assign_and_start"
+        );
+    }
+
+    /// Both paths (Matrix and MCP) that call `assign_and_start` leave the same
+    /// CRDT register value, regardless of which path is used.
+    #[tokio::test]
+    async fn assign_and_start_same_crdt_state_from_both_paths() {
+        crate::crdt_state::init_for_test();
+        let story_id_a = "8771_story_assign_path_a";
+        let story_id_b = "8772_story_assign_path_b";
+
+        for sid in &[story_id_a, story_id_b] {
+            crate::crdt_state::write_item(
+                sid,
+                "2_current",
+                Some("Path Test"),
+                None,
+                None,
+                None,
+                None,
+                None,
+                None,
+                None,
+            );
+        }
+
+        let tmp = tempfile::tempdir().unwrap();
+        let agents = make_agents();
+
+        // Simulate Matrix path: call assign_and_start directly (as handle_assign does).
+        let _ = assign_and_start(story_id_a, "coder-sonnet", tmp.path(), &agents).await;
+
+        // Simulate MCP path: call assign_and_start directly (as tool_start_agent does).
+        let _ = assign_and_start(story_id_b, "coder-sonnet", tmp.path(), &agents).await;
+
+        // Both must leave the same CRDT agent register value.
+        for sid in &[story_id_a, story_id_b] {
+            let dump = crate::crdt_state::dump_crdt_state(Some(sid));
+            let item = dump
+                .items
+                .iter()
+                .find(|i| i.story_id.as_deref() == Some(*sid))
+                .expect("item must be in CRDT");
+            assert_eq!(
+                item.agent.as_deref(),
+                Some("coder-sonnet"),
+                "CRDT agent register must match for story {sid}"
+            );
+        }
+    }
+}

server/src/service/work_item/mod.rs

@@ -1,5 +1,9 @@
 //! Work-item service — cross-cutting domain logic that applies to all pipeline
 //! work-item types (stories, bugs, spikes, refactors).
 
+/// Assign-and-start: unified assign + spawn for both chat and MCP paths.
+pub mod assign;
 /// Canonical delete sequence for any work item type.
 pub mod delete;
+
+pub use assign::assign_and_start;