wip(929): stage 10 sweep — production callsites move to CRDT, yaml_legacy shrinks

After 932 (review_hold register) and 933 (item_type + epic registers), the
remaining production yaml_legacy callers all had typed CRDT equivalents.
Migrated:

- agents/lifecycle.rs:
  - transition_to_merge_failure writes to MergeJob.error CRDT entry instead
    of YAML body. The legacy `merge_failure: "..."` front-matter write is gone.
  - reject_story_from_qa inlines the QA-rejection notes append; no longer
    needs yaml_legacy::write_rejection_notes_to_content.
  - fields_to_clear_transform helper deleted along with all five callers —
    blocked/retry_count/merge_failure are typed CRDT fields now, so clearing
    the equivalent YAML keys is redundant.

- http/workflow/pipeline.rs:
  - load_pipeline_state reads merge_failure from MergeJob.error (mirrors
    status_tools.rs).
  - validate_story_dirs checks the typed CRDT `name` register instead of
    parsing YAML front matter.

- http/mcp/status_tools.rs: review_hold reads the typed CRDT register
  (yaml_residue wrap was the last one in this file).
- http/mcp/story_tools/criteria.rs: story_name reads from CRDT.
- service/agents/mod.rs::get_work_item_content: name/agent come from CRDT.
- service/notifications/io/mod.rs::read_story_name: same.
- http/workflow/bug_ops/{bug,refactor}.rs: name-fallback paths drop YAML
  parsing in favour of the CRDT-derived item.name.

Dead helpers removed from db/yaml_legacy.rs:
  yaml_residue, write_merge_failure_in_content, write_rejection_notes_to_content,
  clear_front_matter_field_in_content, write_review_hold_in_content,
  clear_front_matter_field, write_review_hold (the last four shipped in 932).
Remaining surface: FrontMatter / StoryMetadata structs, parse_front_matter,
set_front_matter_field — kept for `coverage_baseline` writes via
test_results.rs and the generic update_story front_matter escape hatch.

Test fixtures rewritten to seed the CRDT register instead of relying on
YAML parsing during write_item_with_content:
- has_review_hold_returns_* tests
- item_type_from_id_uses_crdt_register_for_numeric_ids
- tool_list_epics_shows_member_rollup
- get_work_item_content (both copies — http/agents + service/agents)
- validate_story_dirs_missing_name_in_crdt
- server_side_merge_*_sets_merge_failure (assert MergeJob.error, not YAML)

cargo fmt --check, clippy --all-targets -- -D warnings, and the
2856-test suite all pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Timmy
2026-05-12 20:13:17 +01:00
parent 7d7ab85994
commit 4888f051c3
14 changed files with 132 additions and 206 deletions
+30 -51
View File
@@ -10,7 +10,6 @@ use std::num::NonZeroU32;
use std::path::Path;
use std::process::Command;
use crate::db::yaml_legacy::clear_front_matter_field_in_content;
use crate::pipeline_state::{
ApplyError, ArchiveReason, BranchName, GitSha, PipelineEvent, Stage, TransitionFired,
apply_transition, stage_label,
@@ -46,23 +45,6 @@ pub(crate) fn item_type_from_id(item_id: &str) -> &'static str {
"story"
}
type ContentTransform = Box<dyn Fn(&str) -> String>;
/// Build a content-transform closure that clears the given front-matter fields.
fn fields_to_clear_transform(fields: &[&str]) -> Option<ContentTransform> {
if fields.is_empty() {
return None;
}
let fields: Vec<String> = fields.iter().map(|s| s.to_string()).collect();
Some(Box::new(move |content: &str| {
let mut result = content.to_string();
for field in &fields {
result = clear_front_matter_field_in_content(&result, field);
}
result
}))
}
/// Move a work item (story, bug, or spike) from `1_backlog` to `work/2_current/`.
///
/// Only promotes from `1_backlog` — stories already in later stages (3_qa, 4_merge,
@@ -142,8 +124,7 @@ pub fn move_story_to_done(story_id: &str) -> Result<(), String> {
}
};
let transform = fields_to_clear_transform(&["merge_failure", "blocked"]);
apply_transition(story_id, event, transform.as_ref().map(|f| f.as_ref()))
apply_transition(story_id, event, None)
.map(|_| ())
.map_err(|e| e.to_string())
}
@@ -179,8 +160,7 @@ pub fn move_story_to_merge(story_id: &str) -> Result<(), String> {
}
};
let transform = fields_to_clear_transform(&["blocked"]);
apply_transition(story_id, event, transform.as_ref().map(|f| f.as_ref()))
apply_transition(story_id, event, None)
.map(|_| ())
.map_err(|e| e.to_string())
}
@@ -197,14 +177,9 @@ pub fn move_story_to_qa(story_id: &str) -> Result<(), String> {
return Ok(());
}
let transform = fields_to_clear_transform(&["blocked"]);
apply_transition(
story_id,
PipelineEvent::GatesStarted,
transform.as_ref().map(|f| f.as_ref()),
)
.map(|_| ())
.map_err(|e| e.to_string())
apply_transition(story_id, PipelineEvent::GatesStarted, None)
.map(|_| ())
.map_err(|e| e.to_string())
}
/// Move a story from `work/3_qa/` back to `work/2_current/`, clearing
@@ -225,7 +200,7 @@ pub fn reject_story_from_qa(story_id: &str, notes: &str) -> Result<(), String> {
} else {
let notes_owned = notes.to_string();
let transform = move |content: &str| -> String {
crate::db::yaml_legacy::write_rejection_notes_to_content(content, &notes_owned)
format!("{content}\n\n## QA Rejection Notes\n\n{notes_owned}\n")
};
apply_transition(
story_id,
@@ -245,13 +220,12 @@ pub fn reject_story_from_qa(story_id: &str, notes: &str) -> Result<(), String> {
/// 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()),
None,
)
.map(|_| ())
.map_err(|e| e.to_string())
@@ -260,9 +234,10 @@ pub fn transition_to_blocked(story_id: &str, reason: &str) -> Result<(), String>
/// Transition a story from `Stage::Merge` (or `Stage::MergeFailure`) 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.
/// Builds a `PipelineEvent::MergeFailed { reason }`, validates the transition,
/// writes the resulting `Stage::MergeFailure` to the CRDT, and persists the
/// reason to the typed `MergeJob.error` CRDT register so it survives server
/// restarts (story 929: the legacy YAML write of `merge_failure: "..."` is gone).
///
/// When the story is already in `MergeFailure`, this is a silent self-loop: the
/// returned `TransitionFired::before` will be `Stage::MergeFailure`. Callers
@@ -273,18 +248,27 @@ pub fn transition_to_merge_failure(
story_id: &str,
reason: &str,
) -> Result<TransitionFired, String> {
let reason_owned = reason.to_string();
let transform: Box<dyn Fn(&str) -> String> = Box::new(move |content: &str| {
crate::db::yaml_legacy::write_merge_failure_in_content(content, &reason_owned)
});
apply_transition(
let fired = apply_transition(
story_id,
PipelineEvent::MergeFailed {
reason: reason.to_string(),
},
Some(&*transform),
None,
)
.map_err(|e| e.to_string())
.map_err(|e| e.to_string())?;
// Persist the failure reason on the MergeJob CRDT entry so display tools
// (status_tools, chat status renderer, pipeline.rs::load_pipeline_state)
// can surface it without re-parsing YAML.
crate::crdt_state::write_merge_job(
story_id,
"failed",
chrono::Utc::now().timestamp() as f64,
None,
Some(reason),
);
Ok(fired)
}
/// Transition a story out of `Blocked` back to `Coding` via the state machine.
@@ -294,14 +278,9 @@ pub fn transition_to_merge_failure(
/// 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())?;
apply_transition(story_id, PipelineEvent::Unblock, None)
.map(|_| ())
.map_err(|e| e.to_string())?;
// Reset CRDT registers so the legacy `blocked`/`retry_count` fields match
// the new typed stage. Pre-865, YAML stripping kept these in sync as a