huskies: merge 890

This commit is contained in:
dave
2026-05-12 14:43:27 +00:00
parent bb845d17cf
commit 2c5326f339
5 changed files with 227 additions and 182 deletions
@@ -518,10 +518,11 @@ mod tests {
"[[agent]]\nname = \"mergemaster\"\nstage = \"mergemaster\"\n", "[[agent]]\nname = \"mergemaster\"\nstage = \"mergemaster\"\n",
) )
.unwrap(); .unwrap();
crate::crdt_state::init_for_test();
crate::db::ensure_content_store(); crate::db::ensure_content_store();
crate::db::write_item_with_content( crate::db::write_item_with_content(
"9860_story_conflict", "9860_story_conflict",
"4_merge", "4_merge_failure",
"---\nname: Conflict\nmerge_failure: \"CONFLICT (content): server/src/lib.rs\"\n---\n", "---\nname: Conflict\nmerge_failure: \"CONFLICT (content): server/src/lib.rs\"\n---\n",
crate::db::ItemMeta::from_yaml( crate::db::ItemMeta::from_yaml(
"---\nname: Conflict\nmerge_failure: \"CONFLICT (content): server/src/lib.rs\"\n---\n", "---\nname: Conflict\nmerge_failure: \"CONFLICT (content): server/src/lib.rs\"\n---\n",
@@ -555,10 +556,11 @@ mod tests {
"[[agent]]\nname = \"mergemaster\"\nstage = \"mergemaster\"\n", "[[agent]]\nname = \"mergemaster\"\nstage = \"mergemaster\"\n",
) )
.unwrap(); .unwrap();
crate::crdt_state::init_for_test();
crate::db::ensure_content_store(); crate::db::ensure_content_store();
crate::db::write_item_with_content( crate::db::write_item_with_content(
"9861_story_nothing", "9861_story_nothing",
"4_merge", "4_merge_failure",
"---\nname: Nothing\nmerge_failure: \"nothing to commit, working tree clean\"\n---\n", "---\nname: Nothing\nmerge_failure: \"nothing to commit, working tree clean\"\n---\n",
crate::db::ItemMeta::from_yaml( crate::db::ItemMeta::from_yaml(
"---\nname: Nothing\nmerge_failure: \"nothing to commit, working tree clean\"\n---\n", "---\nname: Nothing\nmerge_failure: \"nothing to commit, working tree clean\"\n---\n",
+53 -70
View File
@@ -12,7 +12,7 @@ use super::super::super::PipelineStage;
use super::super::AgentPool; use super::super::AgentPool;
use super::scan::{find_free_agent_for_stage, is_story_assigned_for_stage, scan_stage_items}; use super::scan::{find_free_agent_for_stage, is_story_assigned_for_stage, scan_stage_items};
use super::story_checks::{ use super::story_checks::{
has_content_conflict_failure, has_merge_failure, has_mergemaster_attempted, has_review_hold, has_content_conflict_failure, has_mergemaster_attempted, has_review_hold,
has_unmet_dependencies, is_story_blocked, is_story_frozen, has_unmet_dependencies, is_story_blocked, is_story_frozen,
}; };
@@ -36,75 +36,6 @@ impl AgentPool {
// call invokes the LLM-driven recovery path. // call invokes the LLM-driven recovery path.
let merge_items = scan_stage_items(project_root, "4_merge"); let merge_items = scan_stage_items(project_root, "4_merge");
for story_id in &merge_items { for story_id in &merge_items {
// Stories with a recorded merge failure may be eligible for
// automatic mergemaster dispatch when the failure is a content
// conflict — otherwise they need human intervention.
if has_merge_failure(project_root, "4_merge", story_id) {
// Auto-spawn mergemaster for content conflicts, but only once.
if has_content_conflict_failure(project_root, "4_merge", story_id)
&& !has_mergemaster_attempted(project_root, "4_merge", story_id)
&& !is_story_blocked(project_root, "4_merge", story_id)
{
// Find the mergemaster agent.
let mergemaster_agent = {
let agents = match self.agents.lock() {
Ok(a) => a,
Err(e) => {
slog_error!(
"[auto-assign] Failed to lock agents for mergemaster check: {e}"
);
continue;
}
};
if is_story_assigned_for_stage(
config,
&agents,
story_id,
&PipelineStage::Mergemaster,
) {
// Already running — don't spawn again.
None
} else {
find_free_agent_for_stage(config, &agents, &PipelineStage::Mergemaster)
.map(str::to_string)
}
};
if let Some(agent_name) = mergemaster_agent {
slog!(
"[auto-assign] Content conflict on '{story_id}'; \
auto-spawning mergemaster '{agent_name}'."
);
// Record mergemaster_attempted before spawning so a
// crash/restart doesn't re-trigger an infinite loop.
if let Some(contents) = crate::db::read_content(story_id) {
let updated =
crate::db::yaml_legacy::write_mergemaster_attempted_in_content(
&contents,
);
crate::db::write_content(story_id, &updated);
crate::db::write_item_with_content(
story_id,
"4_merge",
&updated,
crate::db::ItemMeta::from_yaml(&updated),
);
}
crate::crdt_state::set_mergemaster_attempted(story_id, true);
if let Err(e) = self
.start_agent(project_root, story_id, Some(&agent_name), None, None)
.await
{
slog!(
"[auto-assign] Failed to start mergemaster '{agent_name}' \
for '{story_id}': {e}"
);
}
}
}
continue;
}
if has_review_hold(project_root, "4_merge", story_id) { if has_review_hold(project_root, "4_merge", story_id) {
continue; continue;
} }
@@ -176,5 +107,57 @@ impl AgentPool {
slog!("[auto-assign] Triggering server-side merge for '{story_id}' in 4_merge/"); slog!("[auto-assign] Triggering server-side merge for '{story_id}' in 4_merge/");
self.trigger_server_side_merge(project_root, story_id); self.trigger_server_side_merge(project_root, story_id);
} }
// ── 4_merge_failure: auto-spawn mergemaster on content conflict ───────
//
// Stories transition to 4_merge_failure when the server-side merge fails.
// Content conflicts get one automatic mergemaster attempt; other failures
// require human intervention.
let merge_failure_items = scan_stage_items(project_root, "4_merge_failure");
for story_id in &merge_failure_items {
if has_content_conflict_failure(project_root, "4_merge_failure", story_id)
&& !has_mergemaster_attempted(project_root, "4_merge_failure", story_id)
{
let mergemaster_agent = {
let agents = match self.agents.lock() {
Ok(a) => a,
Err(e) => {
slog_error!(
"[auto-assign] Failed to lock agents for mergemaster check: {e}"
);
continue;
}
};
if is_story_assigned_for_stage(
config,
&agents,
story_id,
&PipelineStage::Mergemaster,
) {
None
} else {
find_free_agent_for_stage(config, &agents, &PipelineStage::Mergemaster)
.map(str::to_string)
}
};
if let Some(agent_name) = mergemaster_agent {
slog!(
"[auto-assign] Content conflict on '{story_id}'; \
auto-spawning mergemaster '{agent_name}'."
);
crate::crdt_state::set_mergemaster_attempted(story_id, true);
if let Err(e) = self
.start_agent(project_root, story_id, Some(&agent_name), None, None)
.await
{
slog!(
"[auto-assign] Failed to start mergemaster '{agent_name}' \
for '{story_id}': {e}"
);
}
}
}
}
} }
} }
+161 -101
View File
@@ -29,37 +29,79 @@ pub(super) fn read_story_front_matter_agent(
parse_front_matter(&contents).ok()?.agent parse_front_matter(&contents).ok()?.agent
} }
/// Return `true` if the story file in the given stage has `review_hold: true` in its front matter. /// Return `true` if the story is in the `Frozen` pipeline stage.
pub(super) fn has_review_hold(project_root: &Path, _stage_dir: &str, story_id: &str) -> bool { ///
use crate::db::yaml_legacy::parse_front_matter; /// In the typed CRDT model, `Frozen` is the authoritative representation of
let contents = match read_story_contents(project_root, story_id) { /// stories that are held for human review (replacing the legacy
Some(c) => c, /// `review_hold: true` YAML front-matter field). The typed stage register is
None => return false, /// the only source consulted — stale YAML is ignored.
}; pub(super) fn has_review_hold(_project_root: &Path, _stage_dir: &str, story_id: &str) -> bool {
parse_front_matter(&contents) crate::pipeline_state::read_typed(story_id)
.ok() .ok()
.and_then(|m| m.review_hold) .flatten()
.map(|item| item.stage.is_frozen())
.unwrap_or(false) .unwrap_or(false)
} }
/// Return `true` if the story is blocked — either via the typed `Stage::Blocked` /// Return `true` if the story is blocked via the typed `Stage::Blocked` or
/// variant or the legacy `blocked: true` front-matter field. /// `Stage::MergeFailure` variant (or the legacy `Archived(Blocked)` state).
pub(super) fn is_story_blocked(project_root: &Path, _stage_dir: &str, story_id: &str) -> bool { ///
// Check the typed stage first (authoritative after story 866). /// The typed pipeline stage register is the only source consulted — the legacy
if let Ok(Some(item)) = crate::pipeline_state::read_typed(story_id) /// `blocked: true` YAML front-matter field is no longer checked.
&& item.stage.is_blocked() pub(super) fn is_story_blocked(_project_root: &Path, _stage_dir: &str, story_id: &str) -> bool {
{ crate::pipeline_state::read_typed(story_id)
return true;
}
// Legacy fallback: check front-matter field for backward compatibility.
use crate::db::yaml_legacy::parse_front_matter;
let contents = match read_story_contents(project_root, story_id) {
Some(c) => c,
None => return false,
};
parse_front_matter(&contents)
.ok() .ok()
.and_then(|m| m.blocked) .flatten()
.map(|item| item.stage.is_blocked())
.unwrap_or(false)
}
/// Return `true` if the story's merge failure contains a git content-conflict
/// marker (`"Merge conflict"` or `"CONFLICT (content):"`).
///
/// Used by the auto-assigner to decide whether to spawn mergemaster automatically.
/// The typed stage register is consulted first; the CRDT content store is then
/// scanned for conflict markers (the projection layer does not carry the reason
/// string). No YAML front-matter parsing is performed.
pub(super) fn has_content_conflict_failure(
_project_root: &Path,
_stage_dir: &str,
story_id: &str,
) -> bool {
let is_merge_failure = crate::pipeline_state::read_typed(story_id)
.ok()
.flatten()
.map(|item| {
matches!(
item.stage,
crate::pipeline_state::Stage::MergeFailure { .. }
)
})
.unwrap_or(false);
if !is_merge_failure {
return false;
}
// The projection does not carry the reason string; read the raw content
// from the CRDT content store and scan for conflict markers.
crate::db::read_content(story_id)
.map(|content| {
content.contains("Merge conflict") || content.contains("CONFLICT (content):")
})
.unwrap_or(false)
}
/// Return `true` if the CRDT `mergemaster_attempted` register is set for this story.
///
/// Used to prevent the auto-assigner from repeatedly spawning mergemaster for
/// the same story after a failed mergemaster session. The CRDT register is the
/// only source consulted — the legacy YAML field is no longer checked.
pub(super) fn has_mergemaster_attempted(
_project_root: &Path,
_stage_dir: &str,
story_id: &str,
) -> bool {
crate::crdt_state::read_item(story_id)
.and_then(|view| view.mergemaster_attempted)
.unwrap_or(false) .unwrap_or(false)
} }
@@ -120,97 +162,115 @@ pub(super) fn is_story_frozen(_project_root: &Path, _stage_dir: &str, story_id:
.unwrap_or(false) .unwrap_or(false)
} }
/// Return `true` if the story file has a `merge_failure` field in its front matter.
pub(super) fn has_merge_failure(project_root: &Path, _stage_dir: &str, story_id: &str) -> bool {
use crate::db::yaml_legacy::parse_front_matter;
let contents = match read_story_contents(project_root, story_id) {
Some(c) => c,
None => return false,
};
parse_front_matter(&contents)
.ok()
.and_then(|m| m.merge_failure)
.is_some()
}
/// Return `true` if the story's `merge_failure` contains a git content-conflict
/// marker (`"Merge conflict"` or `"CONFLICT (content):"`).
///
/// Used by the auto-assigner to decide whether to spawn mergemaster automatically.
pub(super) fn has_content_conflict_failure(
project_root: &Path,
_stage_dir: &str,
story_id: &str,
) -> bool {
use crate::db::yaml_legacy::parse_front_matter;
let contents = match read_story_contents(project_root, story_id) {
Some(c) => c,
None => return false,
};
parse_front_matter(&contents)
.ok()
.and_then(|m| m.merge_failure)
.map(|reason| reason.contains("Merge conflict") || reason.contains("CONFLICT (content):"))
.unwrap_or(false)
}
/// Return `true` if the story has `mergemaster_attempted: true` in its front matter.
///
/// Used to prevent the auto-assigner from repeatedly spawning mergemaster for
/// the same story after a failed mergemaster session.
pub(super) fn has_mergemaster_attempted(
project_root: &Path,
_stage_dir: &str,
story_id: &str,
) -> bool {
use crate::db::yaml_legacy::parse_front_matter;
let contents = match read_story_contents(project_root, story_id) {
Some(c) => c,
None => return false,
};
parse_front_matter(&contents)
.ok()
.and_then(|m| m.mergemaster_attempted)
.unwrap_or(false)
}
// ── Tests ────────────────────────────────────────────────────────────────── // ── Tests ──────────────────────────────────────────────────────────────────
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
// ── has_review_hold ───────────────────────────────────────────────────────
#[test] #[test]
fn has_review_hold_returns_true_when_set() { fn has_review_hold_returns_true_when_frozen() {
let tmp = tempfile::tempdir().unwrap(); crate::crdt_state::init_for_test();
crate::db::ensure_content_store(); crate::db::ensure_content_store();
crate::db::write_item_with_content(
"10_spike_research",
"3_qa",
"---\nname: Research spike\nreview_hold: true\n---\n# Spike\n",
crate::db::ItemMeta::from_yaml(
"---\nname: Research spike\nreview_hold: true\n---\n# Spike\n",
),
);
assert!(has_review_hold(tmp.path(), "3_qa", "10_spike_research"));
}
#[test]
fn has_review_hold_returns_false_when_not_set() {
let tmp = tempfile::tempdir().unwrap(); let tmp = tempfile::tempdir().unwrap();
let qa_dir = tmp.path().join(".huskies/work/3_qa"); crate::db::write_item_with_content(
std::fs::create_dir_all(&qa_dir).unwrap(); "890_spike_frozen",
let spike_path = qa_dir.join("10_spike_research.md"); "7_frozen",
std::fs::write(&spike_path, "---\nname: Research spike\n---\n# Spike\n").unwrap(); "---\nname: Frozen Spike\n---\n# Spike\n",
assert!(!has_review_hold(tmp.path(), "3_qa", "10_spike_research")); crate::db::ItemMeta::named("Frozen Spike"),
);
assert!(has_review_hold(tmp.path(), "3_qa", "890_spike_frozen"));
} }
#[test] #[test]
fn has_review_hold_returns_false_when_file_missing() { fn has_review_hold_returns_false_for_qa_stage() {
crate::crdt_state::init_for_test();
crate::db::ensure_content_store();
let tmp = tempfile::tempdir().unwrap();
crate::db::write_item_with_content(
"890_spike_active_qa",
"3_qa",
"---\nname: Active QA Spike\n---\n# Spike\n",
crate::db::ItemMeta::named("Active QA Spike"),
);
assert!(!has_review_hold(tmp.path(), "3_qa", "890_spike_active_qa"));
}
#[test]
fn has_review_hold_returns_false_when_story_unknown() {
let tmp = tempfile::tempdir().unwrap(); let tmp = tempfile::tempdir().unwrap();
assert!(!has_review_hold(tmp.path(), "3_qa", "99_spike_missing")); assert!(!has_review_hold(tmp.path(), "3_qa", "99_spike_missing"));
} }
// ── is_story_blocked — regression: typed stage is sole authority ──────────
#[test]
fn is_story_blocked_set_via_typed_stage_returns_true() {
crate::crdt_state::init_for_test();
crate::db::ensure_content_store();
let tmp = tempfile::tempdir().unwrap();
crate::db::write_item_with_content(
"890_story_blocked_set",
"2_blocked",
"---\nname: Blocked Story\n---\n",
crate::db::ItemMeta::named("Blocked Story"),
);
assert!(is_story_blocked(
tmp.path(),
"2_blocked",
"890_story_blocked_set"
));
}
#[test]
fn is_story_blocked_cleared_via_typed_stage_returns_false() {
crate::crdt_state::init_for_test();
crate::db::ensure_content_store();
let tmp = tempfile::tempdir().unwrap();
// First set to blocked.
crate::db::write_item_with_content(
"890_story_blocked_clear",
"2_blocked",
"---\nname: Clearable Story\n---\n",
crate::db::ItemMeta::named("Clearable Story"),
);
// Then clear by transitioning to an active stage.
crate::db::write_item_with_content(
"890_story_blocked_clear",
"2_current",
"---\nname: Clearable Story\n---\n",
crate::db::ItemMeta::named("Clearable Story"),
);
assert!(!is_story_blocked(
tmp.path(),
"2_current",
"890_story_blocked_clear"
));
}
#[test]
fn is_story_blocked_stale_yaml_is_ignored() {
crate::crdt_state::init_for_test();
crate::db::ensure_content_store();
let tmp = tempfile::tempdir().unwrap();
// YAML front matter says `blocked: true`, but the typed CRDT stage is backlog.
// After removing the YAML fallback, the function must return false.
crate::db::write_item_with_content(
"890_story_stale_yaml",
"1_backlog",
"---\nname: Stale\nblocked: true\n---\n",
crate::db::ItemMeta::named("Stale"),
);
assert!(
!is_story_blocked(tmp.path(), "1_backlog", "890_story_stale_yaml"),
"stale YAML `blocked: true` must not be reported as blocked when typed stage is Backlog"
);
}
// ── has_unmet_dependencies ────────────────────────────────────────────────
#[test] #[test]
fn has_unmet_dependencies_returns_true_when_dep_not_done() { fn has_unmet_dependencies_returns_true_when_dep_not_done() {
let tmp = tempfile::tempdir().unwrap(); let tmp = tempfile::tempdir().unwrap();
+9 -4
View File
@@ -113,9 +113,10 @@ pub(super) fn get_crdt() -> Option<&'static Mutex<CrdtState>> {
/// Initialise a minimal in-memory CRDT state for unit tests. /// Initialise a minimal in-memory CRDT state for unit tests.
/// ///
/// This avoids the async SQLite setup from `init()`. Ops are accepted via a /// This avoids the async SQLite setup from `init()`. Ops are sent to a
/// channel whose receiver is immediately dropped, so nothing is persisted. /// channel whose receiver is leaked (so nothing is persisted, but the channel
/// Safe to call multiple times — subsequent calls are no-ops (OnceLock). /// stays open and `apply_and_persist` succeeds silently).
/// Safe to call multiple times — subsequent calls are no-ops (thread-local).
#[cfg(test)] #[cfg(test)]
pub fn init_for_test() { pub fn init_for_test() {
// Initialise thread-local CRDT for test isolation. // Initialise thread-local CRDT for test isolation.
@@ -126,7 +127,11 @@ pub fn init_for_test() {
if lock.get().is_none() { if lock.get().is_none() {
let keypair = make_keypair(); let keypair = make_keypair();
let crdt = BaseCrdt::<PipelineDoc>::new(&keypair); let crdt = BaseCrdt::<PipelineDoc>::new(&keypair);
let (persist_tx, _rx) = mpsc::unbounded_channel(); let (persist_tx, rx) = mpsc::unbounded_channel();
// Leak the receiver so the channel stays open: apply_and_persist
// can then send without error, preventing [crdt_persist] WARNs
// from racing with other tests that watch the global log buffer.
std::mem::forget(rx);
let state = CrdtState { let state = CrdtState {
crdt, crdt,
keypair, keypair,
-5
View File
@@ -188,11 +188,6 @@ pub(crate) fn write_review_hold_in_content(contents: &str) -> String {
set_front_matter_field(contents, "review_hold", "true") set_front_matter_field(contents, "review_hold", "true")
} }
/// Write `mergemaster_attempted: true` to story content.
pub(crate) fn write_mergemaster_attempted_in_content(contents: &str) -> String {
set_front_matter_field(contents, "mergemaster_attempted", "true")
}
/// Remove a key from the YAML front matter of a story file on disk. /// Remove a key from the YAML front matter of a story file on disk.
/// ///
/// Legacy filesystem-backed wrapper around /// Legacy filesystem-backed wrapper around