huskies: merge 869

This commit is contained in:
dave
2026-04-29 14:51:00 +00:00
parent b9bb1ff804
commit f3e4d5d072
8 changed files with 148 additions and 8 deletions
@@ -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( pub(super) fn resolve_qa_mode_from_store(
_project_root: &Path, _project_root: &Path,
story_id: &str, story_id: &str,
default: crate::io::story_metadata::QaMode, default: crate::io::story_metadata::QaMode,
) -> 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) { 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 default
} }
+2 -1
View File
@@ -52,7 +52,8 @@ pub use types::{
TestJobCrdt, TestJobView, TokenUsageCrdt, TokenUsageView, subscribe, TestJobCrdt, TestJobView, TokenUsageCrdt, TokenUsageView, subscribe,
}; };
pub use write::{ 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)] #[cfg(test)]
+6
View File
@@ -315,6 +315,11 @@ pub(super) fn extract_item_view(item: &PipelineItemCrdt) -> Option<PipelineItemV
_ => None, _ => None,
}; };
let qa_mode = match item.qa_mode.view() {
JsonValue::String(s) if !s.is_empty() => Some(s),
_ => None,
};
Some(PipelineItemView { Some(PipelineItemView {
story_id, story_id,
stage, stage,
@@ -326,6 +331,7 @@ pub(super) fn extract_item_view(item: &PipelineItemCrdt) -> Option<PipelineItemV
claimed_by, claimed_by,
claimed_at, claimed_at,
merged_at, merged_at,
qa_mode,
}) })
} }
+6
View File
@@ -81,6 +81,9 @@ pub struct PipelineItemCrdt {
/// Written once when the item transitions to `5_done`. Used by the /// Written once when the item transitions to `5_done`. Used by the
/// sweep loop to determine when to promote to `6_archived`. /// sweep loop to determine when to promote to `6_archived`.
pub merged_at: LwwRegisterCrdt<f64>, pub merged_at: LwwRegisterCrdt<f64>,
/// QA mode override for this item: `"server"`, `"agent"`, or `"human"`.
/// Empty string means "use the project default".
pub qa_mode: LwwRegisterCrdt<String>,
} }
/// CRDT node that holds a single peer's presence entry. /// 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. /// Unix timestamp (seconds) when the item was merged to master.
/// `None` for items that were never in `5_done` or for legacy items. /// `None` for items that were never in `5_done` or for legacy items.
pub merged_at: Option<f64>, pub merged_at: Option<f64>,
/// QA mode override from the CRDT register: `"server"`, `"agent"`, or `"human"`.
/// `None` means the register is unset (use project default).
pub qa_mode: Option<String>,
} }
/// A snapshot of a single node presence entry derived from the CRDT document. /// A snapshot of a single node presence entry derived from the CRDT document.
+84
View File
@@ -8,6 +8,7 @@ use serde_json::json;
use super::state::{apply_and_persist, emit_event, get_crdt, rebuild_index}; use super::state::{apply_and_persist, emit_event, get_crdt, rebuild_index};
use super::types::{CrdtEvent, PipelineDoc, PipelineItemCrdt}; use super::types::{CrdtEvent, PipelineDoc, PipelineItemCrdt};
use crate::io::story_metadata::QaMode;
use crate::slog; use crate::slog;
// ── Name migration helpers ──────────────────────────────────────────── // ── Name migration helpers ────────────────────────────────────────────
@@ -185,6 +186,29 @@ pub fn migrate_names_from_slugs() {
slog!("[crdt] Migrated names for {count} items from story ID 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<QaMode>) -> 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. /// Write a pipeline item state through CRDT operations.
/// ///
/// If the item exists, updates its registers. If not, inserts a new item /// 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_by": claimed_by.unwrap_or(""),
"claimed_at": claimed_at.unwrap_or(0.0), "claimed_at": claimed_at.unwrap_or(0.0),
"merged_at": merged_at.unwrap_or(0.0), "merged_at": merged_at.unwrap_or(0.0),
"qa_mode": "",
}) })
.into(); .into();
@@ -312,6 +337,7 @@ pub fn write_item(
item.claimed_by.advance_seq(floor); item.claimed_by.advance_seq(floor);
item.claimed_at.advance_seq(floor); item.claimed_at.advance_seq(floor);
item.merged_at.advance_seq(floor); item.merged_at.advance_seq(floor);
item.qa_mode.advance_seq(floor);
} }
// Broadcast a CrdtEvent for the new item. // Broadcast a CrdtEvent for the new item.
@@ -613,6 +639,64 @@ mod tests {
// We call it here just to confirm no panic. // We call it here just to confirm no panic.
migrate_names_from_slugs(); 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::json_crdt::OpState;
use bft_json_crdt::keypair::make_keypair; use bft_json_crdt::keypair::make_keypair;
use bft_json_crdt::op::ROOT_ID; use bft_json_crdt::op::ROOT_ID;
@@ -30,6 +30,14 @@ pub(crate) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result<String
front_matter.insert(k.clone(), v.clone()); front_matter.insert(k.clone(), v.clone());
} }
} }
// Intercept `qa` field — route through the typed CRDT register instead of YAML.
if let Some(qa_val) = front_matter.remove("qa") {
let mode = qa_val
.as_str()
.and_then(crate::io::story_metadata::QaMode::from_str);
crate::crdt_state::set_qa_mode(story_id, mode);
}
let front_matter_opt = if front_matter.is_empty() { let front_matter_opt = if front_matter.is_empty() {
None None
} else { } else {
@@ -37,7 +45,11 @@ pub(crate) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result<String
}; };
let root = ctx.state.get_project_root()?; let root = ctx.state.get_project_root()?;
update_story_in_file(&root, story_id, user_story, description, front_matter_opt)?;
// Only call update_story_in_file when there is something left to write.
if user_story.is_some() || description.is_some() || front_matter_opt.is_some() {
update_story_in_file(&root, story_id, user_story, description, front_matter_opt)?;
}
// Bug 503: warn if any depends_on in the (now updated) story points at an archived story. // Bug 503: warn if any depends_on in the (now updated) story points at an archived story.
let stage = crate::pipeline_state::read_typed(story_id) let stage = crate::pipeline_state::read_typed(story_id)
+14 -4
View File
@@ -108,11 +108,21 @@ pub fn resolve_qa_mode(path: &Path, default: QaMode) -> 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, /// Checks the typed `qa_mode` CRDT register first. If the register holds a
/// otherwise returns `default`. /// recognised value (`"server"`, `"agent"`, or `"human"`), returns it.
pub fn resolve_qa_mode_from_content(contents: &str, default: QaMode) -> QaMode { /// 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) { match parse_front_matter(contents) {
Ok(meta) => meta.qa.unwrap_or(default), Ok(meta) => meta.qa.unwrap_or(default),
Err(_) => default, Err(_) => default,
+6
View File
@@ -205,6 +205,7 @@ mod tests {
claimed_by: None, claimed_by: None,
claimed_at: None, claimed_at: None,
merged_at: None, merged_at: None,
qa_mode: None,
}; };
let item = PipelineItem::try_from(&view).unwrap(); let item = PipelineItem::try_from(&view).unwrap();
assert_eq!(item.story_id, StoryId("42_story_test".to_string())); assert_eq!(item.story_id, StoryId("42_story_test".to_string()));
@@ -227,6 +228,7 @@ mod tests {
claimed_by: None, claimed_by: None,
claimed_at: None, claimed_at: None,
merged_at: None, merged_at: None,
qa_mode: None,
}; };
let item = PipelineItem::try_from(&view).unwrap(); let item = PipelineItem::try_from(&view).unwrap();
assert!(matches!(item.stage, Stage::Coding)); assert!(matches!(item.stage, Stage::Coding));
@@ -246,6 +248,7 @@ mod tests {
claimed_by: None, claimed_by: None,
claimed_at: None, claimed_at: None,
merged_at: None, merged_at: None,
qa_mode: None,
}; };
let item = PipelineItem::try_from(&view).unwrap(); let item = PipelineItem::try_from(&view).unwrap();
assert!(matches!(item.stage, Stage::Merge { .. })); assert!(matches!(item.stage, Stage::Merge { .. }));
@@ -272,6 +275,7 @@ mod tests {
claimed_by: None, claimed_by: None,
claimed_at: None, claimed_at: None,
merged_at: None, merged_at: None,
qa_mode: None,
}; };
let item = PipelineItem::try_from(&view).unwrap(); let item = PipelineItem::try_from(&view).unwrap();
assert!(matches!( assert!(matches!(
@@ -296,6 +300,7 @@ mod tests {
claimed_by: None, claimed_by: None,
claimed_at: None, claimed_at: None,
merged_at: None, merged_at: None,
qa_mode: None,
}; };
let item = PipelineItem::try_from(&view).unwrap(); let item = PipelineItem::try_from(&view).unwrap();
assert!(matches!( assert!(matches!(
@@ -320,6 +325,7 @@ mod tests {
claimed_by: None, claimed_by: None,
claimed_at: None, claimed_at: None,
merged_at: None, merged_at: None,
qa_mode: None,
}; };
let result = PipelineItem::try_from(&view); let result = PipelineItem::try_from(&view);
assert!(matches!( assert!(matches!(