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

huskies: merge 866

Browse Source
This commit is contained in:
dave
2026-04-29 22:42:59 +00:00
parent a49f668b5a
commit 9a3f60d5d3
19 changed files with 289 additions and 144 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/agents/lifecycle.rs
+103
View File
@@ -230,6 +230,45 @@ pub fn reject_story_from_qa(story_id: &str, notes: &str) -> Result<(), String> {
.map_err(|e| e.to_string())
}
/// Transition a story to the `Blocked` stage via the state machine.
///
/// Builds a `PipelineEvent::Block { reason }`, validates the transition, and
/// writes the resulting `Stage::Blocked` to the CRDT. Returns `Err` on
/// `TransitionError` — callers must NOT fall back to direct register writes.
pub fn transition_to_blocked(story_id: &str, reason: &str) -> Result<(), String> {
let transform = fields_to_clear_transform(&["blocked"]);
apply_transition(
story_id,
PipelineEvent::Block {
reason: reason.to_string(),
},
transform.as_ref().map(|f| f.as_ref()),
)
.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
/// resulting `Stage::Coding` to the CRDT, and resets `retry_count` to 0.
/// Returns `Err` on `TransitionError` — callers must NOT fall back to direct
/// register writes.
pub fn transition_to_unblocked(story_id: &str) -> Result<(), String> {
let transform = fields_to_clear_transform(&["blocked", "merge_failure", "retry_count"]);
apply_transition(
story_id,
PipelineEvent::Unblock,
transform.as_ref().map(|f| f.as_ref()),
)
.map(|_| ())
.map_err(|e| e.to_string())?;
// Reset the retry counter in the CRDT so the story gets a fresh budget.
crate::crdt_state::set_retry_count(story_id, 0);
Ok(())
}
/// Map a (current stage, target stage name) pair to the appropriate PipelineEvent.
fn map_stage_move_to_event(
from: &Stage,
@@ -261,6 +300,7 @@ fn map_stage_move_to_event(
merge_commit: GitSha("manual".to_string()),
}),
(Stage::Coding | Stage::Qa | Stage::Backlog, "done") => Ok(PipelineEvent::Close),
(Stage::Blocked { .. }, "current") => Ok(PipelineEvent::Unblock),
(
Stage::Archived {
reason: ArchiveReason::Blocked { .. },
@@ -345,6 +385,7 @@ fn stage_to_name(s: &Stage) -> &'static str {
Stage::Upcoming => "upcoming",
Stage::Backlog => "backlog",
Stage::Coding => "current",
Stage::Blocked { .. } => "blocked",
Stage::Qa => "qa",
Stage::Merge { .. } => "merge",
Stage::Done { .. } => "done",
@@ -452,6 +493,68 @@ mod tests {
assert_eq!(item_type_from_id("99999"), "story");
}
// ── Story 866: block/unblock round-trip regression test ──────────────────
/// Regression test (story 866): block a story via the new state-machine path,
/// verify it lands in `Stage::Blocked`, then unblock and verify it returns
/// to `Stage::Coding`.
#[test]
fn block_unblock_round_trip_via_state_machine() {
crate::db::ensure_content_store();
crate::db::write_item_with_content(
"99866_story_block_test",
"2_current",
"---\nname: Block Round Trip\n---\n# Story\n",
);
// Verify starting state is Coding.
let item = crate::pipeline_state::read_typed("99866_story_block_test")
.expect("read should succeed")
.expect("item should exist");
assert_eq!(
item.stage.dir_name(),
"2_current",
"should start in 2_current"
);
// Block via the state machine.
transition_to_blocked("99866_story_block_test", "retry limit exceeded")
.expect("transition_to_blocked should succeed");
// Verify the CRDT now shows Stage::Blocked.
let item = crate::pipeline_state::read_typed("99866_story_block_test")
.expect("read should succeed")
.expect("item should exist after block");
assert_eq!(
item.stage.dir_name(),
"2_blocked",
"should be in 2_blocked after transition_to_blocked"
);
assert!(item.stage.is_blocked(), "is_blocked() should return true");
assert!(
matches!(item.stage, Stage::Blocked { .. }),
"stage should be Stage::Blocked variant"
);
// Unblock via the state machine.
transition_to_unblocked("99866_story_block_test")
.expect("transition_to_unblocked should succeed");
// Verify the story returned to Coding.
let item = crate::pipeline_state::read_typed("99866_story_block_test")
.expect("read should succeed")
.expect("item should exist after unblock");
assert_eq!(
item.stage.dir_name(),
"2_current",
"should return to 2_current after unblock"
);
assert!(
matches!(item.stage, Stage::Coding),
"stage should be Stage::Coding after unblock"
);
}
// ── feature_branch_has_unmerged_changes tests ────────────────────────────
fn init_git_repo(repo: &std::path::Path) {
server/src/agents/pool/auto_assign/merge.rs
+10 -23
View File
@@ -119,34 +119,21 @@ impl AgentPool {
}
// AC6: Detect empty-diff stories before starting the merge pipeline.
// If the worktree has no commits on the feature branch, write a
// merge_failure and block the story immediately — no merge job needed.
// If the worktree has no commits on the feature branch, block the
// story immediately via the state machine — no merge job needed.
if let Some(wt_path) = worktree::find_worktree_path(project_root, story_id)
&& !crate::agents::gates::worktree_has_committed_work(&wt_path)
{
slog_warn!(
"[auto-assign] Story '{story_id}' in 4_merge/ has no commits \
on feature branch. Writing merge_failure and blocking."
);
let empty_diff_reason = "Feature branch has no code changes — the coder agent \
did not produce any commits.";
if let Some(contents) = crate::db::read_content(story_id) {
let updated = crate::io::story_metadata::write_merge_failure_in_content(
&contents,
empty_diff_reason,
);
let blocked = crate::io::story_metadata::write_blocked_in_content(&updated);
crate::db::write_content(story_id, &blocked);
crate::db::write_item_with_content(story_id, "4_merge", &blocked);
} else {
let story_path = project_root
.join(".huskies/work/4_merge")
.join(format!("{story_id}.md"));
let _ = crate::io::story_metadata::write_merge_failure(
&story_path,
empty_diff_reason,
);
let _ = crate::io::story_metadata::write_blocked(&story_path);
slog_warn!(
"[auto-assign] Story '{story_id}' in 4_merge/ has no commits \
on feature branch. Blocking via state machine."
);
if let Err(e) =
crate::agents::lifecycle::transition_to_blocked(story_id, empty_diff_reason)
{
slog_error!("[auto-assign] Failed to transition '{story_id}' to Blocked: {e}");
}
let _ = self
.watcher_tx
server/src/agents/pool/auto_assign/story_checks.rs
+9 -1
View File
@@ -34,8 +34,16 @@ pub(super) fn has_review_hold(project_root: &Path, _stage_dir: &str, story_id: &
.unwrap_or(false)
}
/// Return `true` if the story file has `blocked: true` in its front matter.
/// Return `true` if the story is blocked — either via the typed `Stage::Blocked`
/// variant or the legacy `blocked: true` front-matter field.
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).
if let Ok(Some(item)) = crate::pipeline_state::read_typed(story_id)
&& item.stage.is_blocked()
{
return true;
}
// Legacy fallback: check front-matter field for backward compatibility.
use crate::io::story_metadata::parse_front_matter;
let contents = match read_story_contents(project_root, story_id) {
Some(c) => c,
server/src/agents/pool/auto_assign/watchdog/tests/limits_tests.rs
+28 -24
View File
@@ -267,12 +267,13 @@ max_turns = 10
let found = pool.run_watchdog_pass(Some(root));
assert!(found >= 1, "watchdog should detect the over-limit agent");
// With max_retries=1, the first violation blocks immediately.
let updated = crate::db::read_content(story_id)
.expect("story content must still exist after watchdog termination");
assert!(
updated.contains("blocked: true"),
"story must be marked `blocked: true` after limit termination with max_retries=1 — got:\n{updated}"
// With max_retries=1, the first violation blocks immediately via the state machine.
let item = crate::crdt_state::read_item(story_id)
.expect("story must be in CRDT after watchdog termination");
assert_eq!(
item.stage, "2_blocked",
"story stage must be 2_blocked after limit termination with max_retries=1 — got: {}",
item.stage
);
// Sanity: the agent itself is also Failed with the right reason.
@@ -407,11 +408,12 @@ max_turns = 10
let event = rx.try_recv().expect("watchdog must emit an Error event");
assert!(matches!(event, AgentEvent::Error { .. }));
// With max_retries=1, the story is blocked.
let updated = crate::db::read_content(story_id).unwrap();
assert!(
updated.contains("blocked: true"),
"story must be blocked after per-session overrun with max_retries=1"
// With max_retries=1, the story is blocked via the state machine.
let item = crate::crdt_state::read_item(story_id)
.expect("story must be in CRDT after per-session overrun");
assert_eq!(
item.stage, "2_blocked",
"story stage must be 2_blocked after per-session overrun with max_retries=1"
);
}
@@ -470,9 +472,9 @@ max_turns = 10
Some(1),
"after session 1, retry_count should be 1 in CRDT"
);
let content = crate::db::read_content(story_id).unwrap();
assert!(
!content.contains("blocked: true"),
assert_ne!(
item.stage.as_str(),
"2_blocked",
"story should NOT be blocked after session 1"
);
}
@@ -490,9 +492,9 @@ max_turns = 10
Some(2),
"after session 2, retry_count should be 2 in CRDT"
);
let content = crate::db::read_content(story_id).unwrap();
assert!(
!content.contains("blocked: true"),
assert_ne!(
item.stage.as_str(),
"2_blocked",
"story should NOT be blocked after session 2"
);
}
@@ -504,16 +506,18 @@ max_turns = 10
pool.inject_test_agent_with_session(story_id, "coder-1", AgentStatus::Running, "session-3");
pool.run_watchdog_pass(Some(root));
let content = crate::db::read_content(story_id).unwrap();
assert!(
content.contains("blocked: true"),
"story must be blocked after session 3 (retry_count=3 >= max_retries=3) — got:\n{content}"
);
let item = crate::crdt_state::read_item(story_id).expect("story must be in CRDT");
assert_eq!(
item.stage, "2_blocked",
"story must be blocked after session 3 (retry_count=3 >= max_retries=3) — got: {}",
item.stage
);
// retry_count resets to 0 on stage transition (Bug 780) — the fact
// that the story reached 2_blocked proves the retry limit was hit.
assert_eq!(
item.retry_count,
Some(3),
"retry_count should be 3 in CRDT after session 3"
Some(0),
"retry_count should reset to 0 after stage transition to blocked"
);
}
}
server/src/agents/pool/pipeline/advance/helpers.rs
+5 -14
View File
@@ -104,8 +104,6 @@ pub(crate) fn should_block_story(
max_retries: u32,
stage_label: &str,
) -> Option<String> {
use crate::io::story_metadata::write_blocked_in_content;
if max_retries == 0 {
return None;
}
@@ -119,23 +117,16 @@ pub(crate) fn should_block_story(
}
if new_count >= max_retries {
let reason =
format!("Retry limit exceeded ({new_count}/{max_retries}) at {stage_label} stage");
slog_warn!(
"[pipeline] Story '{story_id}' reached retry limit ({new_count}/{max_retries}) \
at {stage_label} stage. Marking as blocked."
);
if let Some(contents) = crate::db::read_content(story_id) {
let blocked = write_blocked_in_content(&contents);
crate::db::write_content(story_id, &blocked);
let stage = crate::pipeline_state::read_typed(story_id)
.ok()
.flatten()
.map(|i| i.stage.dir_name().to_string())
.unwrap_or_else(|| "2_current".to_string());
crate::db::write_item_with_content(story_id, &stage, &blocked);
if let Err(e) = crate::agents::lifecycle::transition_to_blocked(story_id, &reason) {
slog_error!("[pipeline] Failed to transition '{story_id}' to Blocked: {e}");
}
Some(format!(
"Retry limit exceeded ({new_count}/{max_retries}) at {stage_label} stage"
))
Some(reason)
} else {
slog!(
"[pipeline] Story '{story_id}' retry {new_count}/{max_retries} at {stage_label} stage."
server/src/agents/pool/pipeline/merge/runner.rs
+12 -11
View File
@@ -114,19 +114,20 @@ impl AgentPool {
Err(e) => e.clone(),
};
let is_no_commits = reason.contains("no commits to merge");
if let Some(contents) = crate::db::read_content(&sid) {
let with_failure = crate::io::story_metadata::write_merge_failure_in_content(
&contents, &reason,
);
let updated = if is_no_commits {
crate::io::story_metadata::write_blocked_in_content(&with_failure)
} else {
with_failure
};
crate::db::write_content(&sid, &updated);
crate::db::write_item_with_content(&sid, "4_merge", &updated);
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}");
}
let _ = pool
.watcher_tx
.send(crate::io::watcher::WatcherEvent::StoryBlocked {