crashlabs/huskies
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 26 Wiki Activity

huskies: merge 868

Browse Source
This commit is contained in:
dave
2026-04-29 23:28:57 +00:00
parent e02e566648
commit 1d86202abb
15 changed files with 135 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/agents/lifecycle.rs
+24
View File
@@ -248,6 +248,28 @@ pub fn transition_to_blocked(story_id: &str, reason: &str) -> Result<(), String>
.map_err(|e| e.to_string())
}
/// Transition a story from `Stage::Merge` to `Stage::MergeFailure` via the state machine.
///
/// Builds a `PipelineEvent::MergeFailed { reason }`, validates the transition, writes
/// the resulting `Stage::MergeFailure` to the CRDT, and persists the reason to front
/// matter so it survives server restarts.
/// Returns `Err` on `TransitionError` — callers must NOT fall back to direct register writes.
pub fn transition_to_merge_failure(story_id: &str, reason: &str) -> Result<(), String> {
let reason_owned = reason.to_string();
let transform: Box<dyn Fn(&str) -> String> = Box::new(move |content: &str| {
crate::io::story_metadata::write_merge_failure_in_content(content, &reason_owned)
});
apply_transition(
story_id,
PipelineEvent::MergeFailed {
reason: reason.to_string(),
},
Some(&*transform),
)
.map(|_| ())
.map_err(|e| e.to_string())
}
/// Transition a story out of `Blocked` back to `Coding` via the state machine.
///
/// Builds a `PipelineEvent::Unblock`, validates the transition, writes the
@@ -301,6 +323,7 @@ fn map_stage_move_to_event(
}),
(Stage::Coding | Stage::Qa | Stage::Backlog, "done") => Ok(PipelineEvent::Close),
(Stage::Blocked { .. }, "current") => Ok(PipelineEvent::Unblock),
(Stage::MergeFailure { .. }, "backlog") => Ok(PipelineEvent::Unblock),
(
Stage::Archived {
reason: ArchiveReason::Blocked { .. },
@@ -388,6 +411,7 @@ fn stage_to_name(s: &Stage) -> &'static str {
Stage::Blocked { .. } => "blocked",
Stage::Qa => "qa",
Stage::Merge { .. } => "merge",
Stage::MergeFailure { .. } => "merge_failure",
Stage::Done { .. } => "done",
Stage::Archived { .. } => "archived",
Stage::Frozen { .. } => "frozen",
server/src/agents/pool/pipeline/merge/runner.rs
+8 -10
View File
@@ -114,16 +114,6 @@ impl AgentPool {
Err(e) => e.clone(),
};
let is_no_commits = reason.contains("no commits to merge");
if !is_no_commits {
// Write merge_failure to content for non-blocking failures.
if let Some(contents) = crate::db::read_content(&sid) {
let updated = crate::io::story_metadata::write_merge_failure_in_content(
&contents, &reason,
);
crate::db::write_content(&sid, &updated);
crate::db::write_item_with_content(&sid, "4_merge", &updated);
}
}
if is_no_commits {
if let Err(e) = crate::agents::lifecycle::transition_to_blocked(&sid, &reason) {
crate::slog_error!("[merge] Failed to transition '{sid}' to Blocked: {e}");
@@ -135,6 +125,14 @@ impl AgentPool {
reason,
});
} else {
// Transition through the state machine (Merge → MergeFailure).
if let Err(e) =
crate::agents::lifecycle::transition_to_merge_failure(&sid, &reason)
{
crate::slog_error!(
"[merge] Failed to transition '{sid}' to MergeFailure: {e}"
);
}
let _ = pool
.watcher_tx
.send(crate::io::watcher::WatcherEvent::MergeFailure {
server/src/chat/transport/matrix/delete.rs
+1
View File
@@ -114,6 +114,7 @@ fn stage_display_name(stage: &str) -> &str {
Some(Stage::Merge { .. }) => "merge",
Some(Stage::Done { .. }) => "done",
Some(Stage::Archived { .. }) => "archived",
Some(Stage::MergeFailure { .. }) => "merge-failure",
Some(Stage::Frozen { .. }) => "frozen",
None => stage,
}
server/src/http/mcp/merge_tools.rs
+5 -23
View File
@@ -1,7 +1,6 @@
//! MCP merge tools — merge agent work to master and report merge failures.
use crate::agents::move_story_to_merge;
use crate::http::context::AppContext;
use crate::io::story_metadata::write_merge_failure;
use crate::slog;
use crate::slog_warn;
use serde_json::{Value, json};
@@ -189,30 +188,14 @@ pub(super) fn tool_report_merge_failure(args: &Value, ctx: &AppContext) -> Resul
reason: reason.to_string(),
});
// Persist the failure reason to the story file's front matter so it
// survives server restarts and is visible in the web UI.
if let Ok(project_root) = ctx.state.get_project_root() {
let story_file = project_root
.join(".huskies")
.join("work")
.join("4_merge")
.join(format!("{story_id}.md"));
if story_file.exists() {
if let Err(e) = write_merge_failure(&story_file, reason) {
slog_warn!(
"[mergemaster] Failed to persist merge_failure to story file for '{story_id}': {e}"
);
}
} else {
slog_warn!(
"[mergemaster] Story file not found in 4_merge/ for '{story_id}'; \
merge_failure not persisted to front matter"
);
}
// Route the failure through the typed state machine (Merge → MergeFailure).
// This persists the reason in front matter and updates the CRDT stage.
if let Err(e) = crate::agents::lifecycle::transition_to_merge_failure(story_id, reason) {
slog_warn!("[mergemaster] Failed to transition '{story_id}' to MergeFailure: {e}");
}
Ok(format!(
"Merge failure for '{story_id}' recorded. Story remains in work/4_merge/. Reason: {reason}"
"Merge failure for '{story_id}' recorded. Reason: {reason}"
))
}
@@ -451,7 +434,6 @@ mod tests {
assert!(result.is_ok());
let msg = result.unwrap();
assert!(msg.contains("42_story_foo"));
assert!(msg.contains("work/4_merge/"));
assert!(msg.contains("Unresolvable merge conflicts"));
}
}
server/src/http/mcp/story_tools/epic.rs
+1
View File
@@ -144,6 +144,7 @@ pub(crate) fn tool_show_epic(args: &Value, _ctx: &AppContext) -> Result<String,
Stage::Merge { .. } => "merge",
Stage::Done { .. } => "done",
Stage::Archived { .. } => "archived",
Stage::MergeFailure { .. } => "merge_failure",
Stage::Frozen { .. } => "frozen",
Stage::Blocked { .. } => "blocked",
};
server/src/http/workflow/pipeline.rs
+1
View File
@@ -149,6 +149,7 @@ pub fn load_pipeline_state(ctx: &AppContext) -> Result<PipelineState, String> {
Stage::Blocked { .. } => state.current.push(story), // blocked shown with current
Stage::Qa => state.qa.push(story),
Stage::Merge { .. } => state.merge.push(story),
Stage::MergeFailure { .. } => state.merge.push(story), // show merge failures with merge
Stage::Done { .. } => state.done.push(story),
Stage::Archived { .. } => {} // skip archived
Stage::Frozen { .. } => state.backlog.push(story), // show frozen with backlog
server/src/io/story_metadata/fields.rs
-21
View File
@@ -83,27 +83,6 @@ pub fn clear_front_matter_field(path: &Path, key: &str) -> Result<(), String> {
Ok(())
}
/// Write or update a `merge_failure:` field in the YAML front matter of a story file.
///
/// The reason is stored as a quoted YAML string so that colons, hashes, and newlines
/// in the failure message do not break front-matter parsing.
/// If no front matter is present, this is a no-op (returns Ok).
pub fn write_merge_failure(path: &Path, reason: &str) -> Result<(), String> {
let contents =
fs::read_to_string(path).map_err(|e| format!("Failed to read story file: {e}"))?;
// Produce a YAML-safe inline quoted string: collapse newlines, escape inner quotes.
let escaped = reason
.replace('"', "\\\"")
.replace('\n', " ")
.replace('\r', "");
let yaml_value = format!("\"{escaped}\"");
let updated = set_front_matter_field(&contents, "merge_failure", &yaml_value);
fs::write(path, &updated).map_err(|e| format!("Failed to write story file: {e}"))?;
Ok(())
}
/// Write `review_hold: true` to the YAML front matter of a story file.
///
/// Used to mark spikes that have passed QA and are waiting for human review.
server/src/io/story_metadata/mod.rs
+3 -3
View File
@@ -14,9 +14,9 @@ mod types;
pub use deps::{check_archived_deps, check_archived_deps_from_list, check_unmet_deps};
pub use fields::{
clear_front_matter_field, clear_front_matter_field_in_content, set_front_matter_field,
write_depends_on, write_depends_on_in_content, write_merge_failure,
write_merge_failure_in_content, write_mergemaster_attempted_in_content,
write_rejection_notes_to_content, write_review_hold, write_review_hold_in_content,
write_depends_on, write_depends_on_in_content, write_merge_failure_in_content,
write_mergemaster_attempted_in_content, write_rejection_notes_to_content, write_review_hold,
write_review_hold_in_content,
};
pub use parser::{
is_story_frozen_in_store, parse_front_matter, parse_unchecked_todos, resolve_qa_mode,
server/src/io/watcher/mod.rs
+3
View File
@@ -55,6 +55,9 @@ pub fn stage_metadata(stage: &str, item_id: &str) -> Option<(&'static str, Strin
Stage::Blocked { .. } => ("block", format!("huskies: block {item_id}")),
Stage::Qa => ("qa", format!("huskies: queue {item_id} for QA")),
Stage::Merge { .. } => ("merge", format!("huskies: queue {item_id} for merge")),
Stage::MergeFailure { .. } => {
("merge_failure", format!("huskies: merge_failure {item_id}"))
}
Stage::Done { .. } => ("done", format!("huskies: done {item_id}")),
Stage::Archived { .. } => ("accept", format!("huskies: accept {item_id}")),
Stage::Frozen { .. } => ("freeze", format!("huskies: freeze {item_id}")),
server/src/pipeline_state/projection.rs
+8
View File
@@ -91,6 +91,14 @@ pub fn project_stage(view: &PipelineItemView) -> Result<Stage, ProjectionError>
commits_ahead: NonZeroU32::new(1).expect("1 is non-zero"),
})
}
"4_merge_failure" => {
// The reason is persisted in front-matter (merge_failure: "...") but
// is not part of the raw CRDT view; the projection uses an empty
// string here. Consumers that need the reason should read content.
Ok(Stage::MergeFailure {
reason: String::new(),
})
}
"5_done" => {
// Use the stored merged_at timestamp if present. Legacy items
// that pre-date this field have merged_at = None, so we fall back
server/src/pipeline_state/tests.rs
+53
View File
@@ -655,4 +655,57 @@ fn regression_freeze_unfreeze_restores_crdt_stage() {
);
}
// ── Story 868: MergeFailure regression ─────────────────────────────
/// Regression test (story 868): applying `PipelineEvent::MergeFailed` to a story
/// in `Stage::Merge` transitions it to `Stage::MergeFailure` and the emitted
/// `TransitionFired` event carries the full reason string in its payload.
#[test]
fn merge_failure_transition_emits_event_with_full_reason() {
crate::crdt_state::init_for_test();
crate::db::ensure_content_store();
let story_id = "99868_story_merge_failure_event";
crate::db::write_item_with_content(
story_id,
"4_merge",
"---\nname: Merge Failure Event Test\n---\n# Story\n",
);
let reason = "Conflict in server/src/main.rs: both modified";
let fired = super::apply::apply_transition(
story_id,
PipelineEvent::MergeFailed {
reason: reason.to_string(),
},
None,
)
.expect("MergeFailed transition should succeed");
// The emitted event payload carries the full reason string.
match &fired.event {
PipelineEvent::MergeFailed { reason: r } => {
assert_eq!(r, reason, "emitted event should carry the full reason");
}
other => panic!("expected MergeFailed event, got: {other:?}"),
}
// The story transitioned to MergeFailure.
assert!(
matches!(fired.after, Stage::MergeFailure { .. }),
"after-stage should be MergeFailure: {:?}",
fired.after
);
// Verify CRDT reflects the new stage.
let item = read_typed(story_id)
.expect("CRDT read should succeed")
.expect("item should exist");
assert_eq!(
item.stage.dir_name(),
"4_merge_failure",
"CRDT stage should be 4_merge_failure"
);
}
// ── ProjectionError Display ─────────────────────────────────────────
server/src/pipeline_state/transition.rs
+10
View File
@@ -33,6 +33,9 @@ pub enum PipelineEvent {
},
/// Mergemaster squash succeeded.
MergeSucceeded { merge_commit: GitSha },
/// Merge pipeline failed (conflicts or gate failures); story moves to
/// `Stage::MergeFailure` awaiting human intervention or retry.
MergeFailed { reason: String },
/// Mergemaster gave up after retry budget.
MergeFailedFinal { reason: String },
/// Story accepted (Done → Archived).
@@ -87,6 +90,7 @@ pub fn event_label(e: &PipelineEvent) -> &'static str {
PipelineEvent::GatesFailed { .. } => "GatesFailed",
PipelineEvent::QaSkipped { .. } => "QaSkipped",
PipelineEvent::MergeSucceeded { .. } => "MergeSucceeded",
PipelineEvent::MergeFailed { .. } => "MergeFailed",
PipelineEvent::MergeFailedFinal { .. } => "MergeFailedFinal",
PipelineEvent::Accepted => "Accepted",
PipelineEvent::Block { .. } => "Block",
@@ -174,6 +178,9 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result<Stage, Transitio
reason: ArchiveReason::ReviewHeld { reason },
}),
// ── MergeFailed: Merge → MergeFailure (recoverable intermediate) ──
(Merge { .. }, MergeFailed { reason }) => Ok(MergeFailure { reason }),
(Merge { .. }, MergeFailedFinal { reason }) => Ok(Archived {
archived_at: now,
reason: ArchiveReason::MergeFailed { reason },
@@ -221,6 +228,9 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result<Stage, Transitio
// ── Unblock: Blocked → Coding ─────────────────────────────────
(Blocked { .. }, Unblock) => Ok(Coding),
// ── Unblock MergeFailure → Backlog ───────────────────────────────
(MergeFailure { .. }, Unblock) => Ok(Backlog),
// ── Legacy unblock: Archived(Blocked|MergeFailed) → Backlog ──
(
Archived {
server/src/pipeline_state/types.rs
+16 -3
View File
@@ -60,9 +60,9 @@ impl fmt::Display for AgentName {
/// | current | `Coding` |
/// | qa_pending | `Qa` |
/// | merge_pending | `Merge { .. }` |
/// | merge_failure | `MergeFailure { .. }` |
/// | done | `Done { .. }` |
/// | blocked | `Blocked { .. }` |
/// | merge_failure | `Archived { MergeFailed { .. } }` |
/// | archived | `Archived { Completed }` |
/// | superseded | `Archived { Superseded { .. } }` |
/// | rejected | `Archived { Rejected { .. } }` |
@@ -106,6 +106,11 @@ pub enum Stage {
reason: ArchiveReason,
},
/// Merge pipeline failed (conflicts or gate failures). Story is held here
/// awaiting human intervention or retry. Unlike `Archived(MergeFailed)`,
/// this is a recoverable intermediate state — `Unblock` returns to `Backlog`.
MergeFailure { reason: String },
/// Pipeline advancement and auto-assign are suspended. Resumes to
/// `resume_to` when unfrozen.
Frozen { resume_to: Box<Stage> },
@@ -154,12 +159,13 @@ impl Stage {
stage_dir_name(self)
}
/// Returns true if this is the `Blocked` variant (or the legacy
/// `Archived(Blocked)` for backward-compatible reads).
/// Returns true if this is the `Blocked` or `MergeFailure` variant (or the
/// legacy `Archived(Blocked)` for backward-compatible reads).
pub fn is_blocked(&self) -> bool {
matches!(
self,
Stage::Blocked { .. }
| Stage::MergeFailure { .. }
| Stage::Archived {
reason: ArchiveReason::Blocked { .. },
..
@@ -198,6 +204,11 @@ impl Stage {
}),
// Frozen: stub with Coding as resume_to — rich resume_to is loaded
// from front matter by the projection layer.
"4_merge_failure" => Some(Stage::MergeFailure {
reason: String::new(),
}),
// Frozen: stub with Coding as resume_to — rich resume_to is loaded
// from front matter by the projection layer.
"7_frozen" => Some(Stage::Frozen {
resume_to: Box::new(Stage::Coding),
}),
@@ -278,6 +289,7 @@ pub fn stage_label(s: &Stage) -> &'static str {
Stage::Coding => "Coding",
Stage::Qa => "Qa",
Stage::Merge { .. } => "Merge",
Stage::MergeFailure { .. } => "MergeFailure",
Stage::Done { .. } => "Done",
Stage::Blocked { .. } => "Blocked",
Stage::Archived { .. } => "Archived",
@@ -294,6 +306,7 @@ pub fn stage_dir_name(s: &Stage) -> &'static str {
Stage::Blocked { .. } => "2_blocked",
Stage::Qa => "3_qa",
Stage::Merge { .. } => "4_merge",
Stage::MergeFailure { .. } => "4_merge_failure",
Stage::Done { .. } => "5_done",
Stage::Archived { .. } => "6_archived",
Stage::Frozen { .. } => "7_frozen",
server/src/service/agents/mod.rs
+1
View File
@@ -208,6 +208,7 @@ pub fn get_work_item_content(
crate::pipeline_state::Stage::Blocked { .. } => "blocked",
crate::pipeline_state::Stage::Qa => "qa",
crate::pipeline_state::Stage::Merge { .. } => "merge",
crate::pipeline_state::Stage::MergeFailure { .. } => "merge_failure",
crate::pipeline_state::Stage::Done { .. } => "done",
crate::pipeline_state::Stage::Archived { .. } => "archived",
crate::pipeline_state::Stage::Frozen { .. } => "frozen",
server/src/service/notifications/format.rs
+1
View File
@@ -18,6 +18,7 @@ pub fn stage_display_name(stage: &str) -> &'static str {
Some(Stage::Merge { .. }) => "Merge",
Some(Stage::Done { .. }) => "Done",
Some(Stage::Archived { .. }) => "Archived",
Some(Stage::MergeFailure { .. }) => "MergeFailure",
Some(Stage::Frozen { .. }) => "Frozen",
None => "Unknown",
}