From f3e4d5d07233afd2f4904db380b3821a06158dab Mon Sep 17 00:00:00 2001 From: dave Date: Wed, 29 Apr 2026 14:51:00 +0000 Subject: [PATCH] huskies: merge 869 --- .../agents/pool/pipeline/advance/helpers.rs | 19 ++++- server/src/crdt_state/mod.rs | 3 +- server/src/crdt_state/read.rs | 6 ++ server/src/crdt_state/types.rs | 6 ++ server/src/crdt_state/write.rs | 84 +++++++++++++++++++ .../src/http/mcp/story_tools/story/update.rs | 14 +++- server/src/io/story_metadata/parser.rs | 18 +++- server/src/pipeline_state/projection.rs | 6 ++ 8 files changed, 148 insertions(+), 8 deletions(-) diff --git a/server/src/agents/pool/pipeline/advance/helpers.rs b/server/src/agents/pool/pipeline/advance/helpers.rs index 9a6c8db6..876e14ed 100644 --- a/server/src/agents/pool/pipeline/advance/helpers.rs +++ b/server/src/agents/pool/pipeline/advance/helpers.rs @@ -50,14 +50,29 @@ pub(crate) fn spawn_pipeline_advance( }); } -/// Resolve QA mode from the content store. +/// Resolve QA mode for a story. +/// +/// Checks the typed `qa_mode` CRDT register first. If the register holds a +/// recognised value, returns it immediately without touching the content store. +/// Otherwise reads the story content and falls back to YAML front-matter +/// parsing via [`crate::io::story_metadata::resolve_qa_mode_from_content`]. pub(super) fn resolve_qa_mode_from_store( _project_root: &Path, story_id: &str, default: crate::io::story_metadata::QaMode, ) -> crate::io::story_metadata::QaMode { + // CRDT register is the authoritative source; check it before the content store. + if let Some(view) = crate::crdt_state::read_item(story_id) + && let Some(ref s) = view.qa_mode + && let Some(mode) = crate::io::story_metadata::QaMode::from_str(s) + { + return mode; + } + // Fall back to YAML front matter for backward compatibility. if let Some(contents) = crate::db::read_content(story_id) { - return crate::io::story_metadata::resolve_qa_mode_from_content(&contents, default); + return crate::io::story_metadata::resolve_qa_mode_from_content( + story_id, &contents, default, + ); } default } diff --git a/server/src/crdt_state/mod.rs b/server/src/crdt_state/mod.rs index e7276fc6..0e4c2565 100644 --- a/server/src/crdt_state/mod.rs +++ b/server/src/crdt_state/mod.rs @@ -52,7 +52,8 @@ pub use types::{ TestJobCrdt, TestJobView, TokenUsageCrdt, TokenUsageView, subscribe, }; pub use write::{ - migrate_names_from_slugs, migrate_story_ids_to_numeric, name_from_story_id, write_item, + migrate_names_from_slugs, migrate_story_ids_to_numeric, name_from_story_id, set_qa_mode, + write_item, }; #[cfg(test)] diff --git a/server/src/crdt_state/read.rs b/server/src/crdt_state/read.rs index 5b3eb55b..2fa9bc6a 100644 --- a/server/src/crdt_state/read.rs +++ b/server/src/crdt_state/read.rs @@ -315,6 +315,11 @@ pub(super) fn extract_item_view(item: &PipelineItemCrdt) -> Option None, }; + let qa_mode = match item.qa_mode.view() { + JsonValue::String(s) if !s.is_empty() => Some(s), + _ => None, + }; + Some(PipelineItemView { story_id, stage, @@ -326,6 +331,7 @@ pub(super) fn extract_item_view(item: &PipelineItemCrdt) -> Option, + /// QA mode override for this item: `"server"`, `"agent"`, or `"human"`. + /// Empty string means "use the project default". + pub qa_mode: LwwRegisterCrdt, } /// CRDT node that holds a single peer's presence entry. @@ -122,6 +125,9 @@ pub struct PipelineItemView { /// Unix timestamp (seconds) when the item was merged to master. /// `None` for items that were never in `5_done` or for legacy items. pub merged_at: Option, + /// QA mode override from the CRDT register: `"server"`, `"agent"`, or `"human"`. + /// `None` means the register is unset (use project default). + pub qa_mode: Option, } /// A snapshot of a single node presence entry derived from the CRDT document. diff --git a/server/src/crdt_state/write.rs b/server/src/crdt_state/write.rs index 483de3be..4b32c2f7 100644 --- a/server/src/crdt_state/write.rs +++ b/server/src/crdt_state/write.rs @@ -8,6 +8,7 @@ use serde_json::json; use super::state::{apply_and_persist, emit_event, get_crdt, rebuild_index}; use super::types::{CrdtEvent, PipelineDoc, PipelineItemCrdt}; +use crate::io::story_metadata::QaMode; use crate::slog; // ── Name migration helpers ──────────────────────────────────────────── @@ -185,6 +186,29 @@ pub fn migrate_names_from_slugs() { slog!("[crdt] Migrated names for {count} items from story ID slugs"); } +/// Set the typed `qa_mode` CRDT register for a pipeline item. +/// +/// Passing `Some(mode)` writes the mode string (e.g. `"server"`, `"agent"`, `"human"`) +/// to the item's `qa_mode` register and persists a signed op. +/// Passing `None` clears the register to an empty string, which means +/// "use the project default" (same as if the field was never set). +/// +/// Returns `true` if the item was found and the op was applied, `false` otherwise. +pub fn set_qa_mode(story_id: &str, mode: Option) -> bool { + let Some(state_mutex) = get_crdt() else { + return false; + }; + let Ok(mut state) = state_mutex.lock() else { + return false; + }; + let Some(&idx) = state.index.get(story_id) else { + return false; + }; + let value = mode.map(|m| m.as_str().to_string()).unwrap_or_default(); + apply_and_persist(&mut state, |s| s.crdt.doc.items[idx].qa_mode.set(value)); + true +} + /// Write a pipeline item state through CRDT operations. /// /// If the item exists, updates its registers. If not, inserts a new item @@ -287,6 +311,7 @@ pub fn write_item( "claimed_by": claimed_by.unwrap_or(""), "claimed_at": claimed_at.unwrap_or(0.0), "merged_at": merged_at.unwrap_or(0.0), + "qa_mode": "", }) .into(); @@ -312,6 +337,7 @@ pub fn write_item( item.claimed_by.advance_seq(floor); item.claimed_at.advance_seq(floor); item.merged_at.advance_seq(floor); + item.qa_mode.advance_seq(floor); } // Broadcast a CrdtEvent for the new item. @@ -613,6 +639,64 @@ mod tests { // We call it here just to confirm no panic. migrate_names_from_slugs(); } + + // ── set_qa_mode regression tests ───────────────────────────────────────── + + #[test] + fn set_qa_mode_round_trip_server_then_human() { + use crate::io::story_metadata::QaMode; + init_for_test(); + + write_item( + "869_story_qa_roundtrip", + "1_backlog", + None, + None, + None, + None, + None, + None, + None, + None, + ); + + // Set qa=server via typed path and assert CRDT register reflects it. + let ok = set_qa_mode("869_story_qa_roundtrip", Some(QaMode::Server)); + assert!(ok, "set_qa_mode should return true for known item"); + let view = read_item("869_story_qa_roundtrip").unwrap(); + assert_eq!( + view.qa_mode.as_deref(), + Some("server"), + "CRDT register should hold \"server\"" + ); + + // Set qa=human via typed path and assert CRDT register is updated. + let ok = set_qa_mode("869_story_qa_roundtrip", Some(QaMode::Human)); + assert!(ok, "set_qa_mode should return true for known item"); + let view = read_item("869_story_qa_roundtrip").unwrap(); + assert_eq!( + view.qa_mode.as_deref(), + Some("human"), + "CRDT register should hold \"human\"" + ); + + // Clear via None — register goes back to unset. + let ok = set_qa_mode("869_story_qa_roundtrip", None); + assert!(ok, "set_qa_mode(None) should return true"); + let view = read_item("869_story_qa_roundtrip").unwrap(); + assert_eq!( + view.qa_mode, None, + "clearing qa_mode should leave register unset" + ); + } + + #[test] + fn set_qa_mode_returns_false_for_unknown_story() { + init_for_test(); + use crate::io::story_metadata::QaMode; + let ok = set_qa_mode("nonexistent_story_qa", Some(QaMode::Server)); + assert!(!ok, "set_qa_mode should return false for unknown story_id"); + } use bft_json_crdt::json_crdt::OpState; use bft_json_crdt::keypair::make_keypair; use bft_json_crdt::op::ROOT_ID; diff --git a/server/src/http/mcp/story_tools/story/update.rs b/server/src/http/mcp/story_tools/story/update.rs index 32d14f4c..20f8abc0 100644 --- a/server/src/http/mcp/story_tools/story/update.rs +++ b/server/src/http/mcp/story_tools/story/update.rs @@ -30,6 +30,14 @@ pub(crate) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result Result QaMode { } } -/// Resolve the effective QA mode from story content (no filesystem access). +/// Resolve the effective QA mode for a story by story ID. /// -/// Parses front matter from `contents` and returns the `qa` field if present, -/// otherwise returns `default`. -pub fn resolve_qa_mode_from_content(contents: &str, default: QaMode) -> QaMode { +/// Checks the typed `qa_mode` CRDT register first. If the register holds a +/// recognised value (`"server"`, `"agent"`, or `"human"`), returns it. +/// Otherwise falls back to parsing the `qa` YAML front-matter field from +/// `contents`. If neither source provides a value, returns `default`. +pub fn resolve_qa_mode_from_content(story_id: &str, contents: &str, default: QaMode) -> QaMode { + // CRDT register takes precedence over YAML front matter. + if let Some(view) = crate::crdt_state::read_item(story_id) + && let Some(ref s) = view.qa_mode + && let Some(mode) = QaMode::from_str(s) + { + return mode; + } + // Fall back to YAML front matter for backward compatibility. match parse_front_matter(contents) { Ok(meta) => meta.qa.unwrap_or(default), Err(_) => default, diff --git a/server/src/pipeline_state/projection.rs b/server/src/pipeline_state/projection.rs index 5a523d26..938262fe 100644 --- a/server/src/pipeline_state/projection.rs +++ b/server/src/pipeline_state/projection.rs @@ -205,6 +205,7 @@ mod tests { claimed_by: None, claimed_at: None, merged_at: None, + qa_mode: None, }; let item = PipelineItem::try_from(&view).unwrap(); assert_eq!(item.story_id, StoryId("42_story_test".to_string())); @@ -227,6 +228,7 @@ mod tests { claimed_by: None, claimed_at: None, merged_at: None, + qa_mode: None, }; let item = PipelineItem::try_from(&view).unwrap(); assert!(matches!(item.stage, Stage::Coding)); @@ -246,6 +248,7 @@ mod tests { claimed_by: None, claimed_at: None, merged_at: None, + qa_mode: None, }; let item = PipelineItem::try_from(&view).unwrap(); assert!(matches!(item.stage, Stage::Merge { .. })); @@ -272,6 +275,7 @@ mod tests { claimed_by: None, claimed_at: None, merged_at: None, + qa_mode: None, }; let item = PipelineItem::try_from(&view).unwrap(); assert!(matches!( @@ -296,6 +300,7 @@ mod tests { claimed_by: None, claimed_at: None, merged_at: None, + qa_mode: None, }; let item = PipelineItem::try_from(&view).unwrap(); assert!(matches!( @@ -320,6 +325,7 @@ mod tests { claimed_by: None, claimed_at: None, merged_at: None, + qa_mode: None, }; let result = PipelineItem::try_from(&view); assert!(matches!(