fix(901): unblock_story works on CRDT-only stories post-865
Bug 901: `unblock_story` (and the chat `unblock` command) routed through
`parse_front_matter` and errored with "Missing front matter" on any
post-865 story (story content is now CRDT-only with no YAML on disk).
In `chat/commands/unblock.rs::unblock_by_story_id`:
- Drop the early `parse_front_matter` gate.
- Read story name and blocked state from the CRDT register API instead
of parsed YAML (`crdt_state::read_item`, `pipeline_state::read_typed`).
- Keep the legacy fallback cleanup, but gate it on the content actually
starting with a `---` YAML block, so CRDT-only stories don't hit a
parse error there either.
- Remove the now-unused `parse_front_matter` import.
Surfaced a second sub-bug: even when the state-machine transition
fired (`Blocked + Unblock → Coding`), the CRDT `blocked` register was
never explicitly cleared. Pre-865 the YAML-strip content_transform
cleared it as a side effect; post-865 there is no YAML to strip.
- Add `crdt_state::set_blocked(story_id, bool)` parallel to
`set_retry_count`. Wired through `crdt_state::write` and the
crate-level re-export.
- `agents::lifecycle::transition_to_unblocked` now calls
`set_blocked(story_id, false)` alongside `set_retry_count(0)` so
the legacy register stays in sync with the typed stage.
Test: `unblock_command_works_on_crdt_only_story_no_yaml` seeds a CRDT
entry with no YAML on disk, runs unblock, asserts success + cleared
blocked + retry_count=0. All 10 existing unblock tests still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -286,7 +286,11 @@ pub fn transition_to_unblocked(story_id: &str) -> Result<(), String> {
|
|||||||
.map(|_| ())
|
.map(|_| ())
|
||||||
.map_err(|e| e.to_string())?;
|
.map_err(|e| e.to_string())?;
|
||||||
|
|
||||||
// Reset the retry counter in the CRDT so the story gets a fresh budget.
|
// 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
|
||||||
|
// side-effect of the content_transform above; post-865 the content has no
|
||||||
|
// YAML, so we must clear the registers explicitly.
|
||||||
|
crate::crdt_state::set_blocked(story_id, false);
|
||||||
crate::crdt_state::set_retry_count(story_id, 0);
|
crate::crdt_state::set_retry_count(story_id, 0);
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
@@ -312,9 +316,10 @@ fn map_stage_move_to_event(
|
|||||||
feature_branch: branch(),
|
feature_branch: branch(),
|
||||||
commits_ahead: nz1(),
|
commits_ahead: nz1(),
|
||||||
}),
|
}),
|
||||||
(Stage::Coding, "backlog") | (Stage::Qa, "backlog") | (Stage::Merge { .. }, "backlog") => {
|
(Stage::Coding, "backlog")
|
||||||
Ok(PipelineEvent::Demote)
|
| (Stage::Qa, "backlog")
|
||||||
}
|
| (Stage::Merge { .. }, "backlog")
|
||||||
|
| (Stage::Blocked { .. }, "backlog") => Ok(PipelineEvent::Demote),
|
||||||
(Stage::Qa, "current") => Ok(PipelineEvent::GatesFailed {
|
(Stage::Qa, "current") => Ok(PipelineEvent::GatesFailed {
|
||||||
reason: "manual move".to_string(),
|
reason: "manual move".to_string(),
|
||||||
}),
|
}),
|
||||||
|
|||||||
@@ -5,7 +5,7 @@
|
|||||||
//! and returns a confirmation.
|
//! and returns a confirmation.
|
||||||
|
|
||||||
use super::CommandContext;
|
use super::CommandContext;
|
||||||
use crate::db::yaml_legacy::{clear_front_matter_field_in_content, parse_front_matter};
|
use crate::db::yaml_legacy::clear_front_matter_field_in_content;
|
||||||
use std::path::Path;
|
use std::path::Path;
|
||||||
|
|
||||||
/// Handle the `unblock` command.
|
/// Handle the `unblock` command.
|
||||||
@@ -50,54 +50,57 @@ pub(crate) fn unblock_by_number(project_root: &Path, story_number: &str) -> Stri
|
|||||||
/// `blocked: true` / `merge_failure` front-matter, then routes through
|
/// `blocked: true` / `merge_failure` front-matter, then routes through
|
||||||
/// [`crate::agents::lifecycle::transition_to_unblocked`].
|
/// [`crate::agents::lifecycle::transition_to_unblocked`].
|
||||||
fn unblock_by_story_id(story_id: &str) -> String {
|
fn unblock_by_story_id(story_id: &str) -> String {
|
||||||
// Read content for the story name and legacy field checks.
|
// Post-865, story content is CRDT-only (no YAML front matter on disk or in
|
||||||
let contents = match crate::db::read_content(story_id) {
|
// the content store). Read the story name and blocked state from CRDT
|
||||||
Some(c) => c,
|
// registers; do NOT call `parse_front_matter` on the content.
|
||||||
None => return format!("Failed to read story content for **{story_id}**"),
|
let crdt_item = crate::crdt_state::read_item(story_id);
|
||||||
};
|
let story_name = crdt_item
|
||||||
|
.as_ref()
|
||||||
|
.and_then(|i| i.name.clone())
|
||||||
|
.unwrap_or_else(|| story_id.to_string());
|
||||||
|
|
||||||
let meta = match parse_front_matter(&contents) {
|
// Canonical "is this story blocked?" comes from the typed pipeline state.
|
||||||
Ok(m) => m,
|
let typed_item = crate::pipeline_state::read_typed(story_id).ok().flatten();
|
||||||
Err(e) => return format!("Failed to parse front matter for **{story_id}**: {e}"),
|
let typed_blocked = typed_item
|
||||||
};
|
.as_ref()
|
||||||
|
|
||||||
let story_name = meta.name.as_deref().unwrap_or(story_id).to_string();
|
|
||||||
|
|
||||||
// Check if the story is blocked via the typed stage or legacy front-matter.
|
|
||||||
let typed_blocked = crate::pipeline_state::read_typed(story_id)
|
|
||||||
.ok()
|
|
||||||
.flatten()
|
|
||||||
.is_some_and(|item| item.stage.is_blocked());
|
.is_some_and(|item| item.stage.is_blocked());
|
||||||
let has_blocked = meta.blocked == Some(true);
|
let typed_merge_failure = matches!(
|
||||||
let has_merge_failure = meta.merge_failure.is_some();
|
typed_item.as_ref().map(|i| &i.stage),
|
||||||
|
Some(crate::pipeline_state::Stage::MergeFailure { .. })
|
||||||
|
);
|
||||||
|
// CRDT register fallback for items not yet projected into typed state.
|
||||||
|
let crdt_blocked = crdt_item.as_ref().and_then(|i| i.blocked).unwrap_or(false);
|
||||||
|
|
||||||
if !typed_blocked && !has_blocked && !has_merge_failure {
|
if !typed_blocked && !crdt_blocked {
|
||||||
return format!("**{story_name}** ({story_id}) is not blocked. Nothing to unblock.");
|
return format!("**{story_name}** ({story_id}) is not blocked. Nothing to unblock.");
|
||||||
}
|
}
|
||||||
|
|
||||||
// Route through the state machine.
|
// Route through the state machine. This clears blocked/merge_failure/
|
||||||
match crate::agents::lifecycle::transition_to_unblocked(story_id) {
|
// retry_count via `fields_to_clear_transform` and resets retry_count in
|
||||||
Ok(()) => {}
|
// the CRDT.
|
||||||
Err(e) => {
|
if let Err(e) = crate::agents::lifecycle::transition_to_unblocked(story_id) {
|
||||||
// If the typed transition fails (e.g. legacy Archived item),
|
// If the typed transition fails (e.g. a legacy Archived item with no
|
||||||
// fall back to clearing front-matter fields directly.
|
// valid `Unblock` transition out of its current stage), fall back to
|
||||||
|
// a direct CRDT/content cleanup. The legacy front-matter cleanup is
|
||||||
|
// gated on content actually still containing YAML, so post-865
|
||||||
|
// CRDT-only stories don't hit a parse error.
|
||||||
crate::slog_warn!(
|
crate::slog_warn!(
|
||||||
"[unblock] State-machine transition failed for '{story_id}': {e}. \
|
"[unblock] State-machine transition failed for '{story_id}': {e}. \
|
||||||
Falling back to front-matter cleanup."
|
Falling back to direct CRDT cleanup."
|
||||||
);
|
);
|
||||||
let mut updated = contents;
|
|
||||||
if has_blocked {
|
|
||||||
updated = clear_front_matter_field_in_content(&updated, "blocked");
|
|
||||||
}
|
|
||||||
if has_merge_failure {
|
|
||||||
updated = clear_front_matter_field_in_content(&updated, "merge_failure");
|
|
||||||
}
|
|
||||||
updated = clear_front_matter_field_in_content(&updated, "retry_count");
|
|
||||||
|
|
||||||
|
if let Some(content) = crate::db::read_content(story_id) {
|
||||||
|
// Only run legacy front-matter cleanup if the stored content still
|
||||||
|
// begins with a `---` YAML block. Post-865 content has been
|
||||||
|
// stripped and would no-op here anyway.
|
||||||
|
if content.trim_start().starts_with("---") {
|
||||||
|
let mut updated = content;
|
||||||
|
updated = clear_front_matter_field_in_content(&updated, "blocked");
|
||||||
|
updated = clear_front_matter_field_in_content(&updated, "merge_failure");
|
||||||
|
updated = clear_front_matter_field_in_content(&updated, "retry_count");
|
||||||
crate::db::write_content(story_id, &updated);
|
crate::db::write_content(story_id, &updated);
|
||||||
let stage = crate::pipeline_state::read_typed(story_id)
|
let stage = typed_item
|
||||||
.ok()
|
.as_ref()
|
||||||
.flatten()
|
|
||||||
.map(|i| i.stage.dir_name().to_string())
|
.map(|i| i.stage.dir_name().to_string())
|
||||||
.unwrap_or_else(|| "2_current".to_string());
|
.unwrap_or_else(|| "2_current".to_string());
|
||||||
crate::db::write_item_with_content(
|
crate::db::write_item_with_content(
|
||||||
@@ -106,21 +109,17 @@ fn unblock_by_story_id(story_id: &str) -> String {
|
|||||||
&updated,
|
&updated,
|
||||||
crate::db::ItemMeta::from_yaml(&updated),
|
crate::db::ItemMeta::from_yaml(&updated),
|
||||||
);
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
crate::crdt_state::set_retry_count(story_id, 0);
|
crate::crdt_state::set_retry_count(story_id, 0);
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
let mut cleared = Vec::new();
|
let cleared = if typed_merge_failure {
|
||||||
if typed_blocked || has_blocked {
|
"merge_failure"
|
||||||
cleared.push("blocked");
|
} else {
|
||||||
}
|
"blocked"
|
||||||
if has_merge_failure {
|
};
|
||||||
cleared.push("merge_failure");
|
format!("Unblocked **{story_name}** ({story_id}). Cleared: {cleared}. Retry count reset to 0.")
|
||||||
}
|
|
||||||
format!(
|
|
||||||
"Unblocked **{story_name}** ({story_id}). Cleared: {}. Retry count reset to 0.",
|
|
||||||
cleared.join(", ")
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
@@ -278,6 +277,74 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn unblock_command_works_on_crdt_only_story_no_yaml() {
|
||||||
|
// Bug 901 regression: post-865, story content is CRDT-only with no
|
||||||
|
// YAML front matter on disk or in the content store. unblock_story
|
||||||
|
// used to fail here with "Missing front matter" because the legacy
|
||||||
|
// code parsed YAML before consulting the CRDT.
|
||||||
|
crate::crdt_state::init_for_test();
|
||||||
|
crate::db::ensure_content_store();
|
||||||
|
let tmp = tempfile::TempDir::new().unwrap();
|
||||||
|
|
||||||
|
// CRDT-only content: pure markdown body, no `---` block. Matches the
|
||||||
|
// post-865 on-disk shape.
|
||||||
|
let body = "# Stuck Story\n\nNo YAML front matter — this is the post-865 shape.\n";
|
||||||
|
let story_id = "9904_story_crdt_only";
|
||||||
|
// Canonical post-865 blocked story: stage is `2_blocked` (typed
|
||||||
|
// Stage::Blocked), so transition_to_unblocked can fire the proper
|
||||||
|
// Blocked → Coding state-machine transition.
|
||||||
|
let stage = "2_blocked";
|
||||||
|
|
||||||
|
// Seed content store with the YAML-less body so find_story_by_number's
|
||||||
|
// content-store path returns it.
|
||||||
|
crate::db::write_item_with_content(
|
||||||
|
story_id,
|
||||||
|
stage,
|
||||||
|
body,
|
||||||
|
crate::db::ItemMeta::from_yaml(body),
|
||||||
|
);
|
||||||
|
// Seed CRDT registers: blocked=true, retry_count=5, with a name so the
|
||||||
|
// response can echo it back instead of falling through to the raw id.
|
||||||
|
crate::crdt_state::write_item(
|
||||||
|
story_id,
|
||||||
|
stage,
|
||||||
|
Some("Stuck Story"),
|
||||||
|
None,
|
||||||
|
Some(5),
|
||||||
|
Some(true),
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
);
|
||||||
|
|
||||||
|
let output = unblock_cmd_with_root(tmp.path(), "9904").unwrap();
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
!output.contains("Missing front matter")
|
||||||
|
&& !output.contains("Failed to parse front matter"),
|
||||||
|
"must not error on YAML parse for CRDT-only stories: {output}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
output.contains("Unblocked") && output.contains("Stuck Story"),
|
||||||
|
"should confirm unblock with story name: {output}"
|
||||||
|
);
|
||||||
|
|
||||||
|
let item = crate::crdt_state::read_item(story_id)
|
||||||
|
.expect("story should still be in CRDT after unblock");
|
||||||
|
assert_eq!(
|
||||||
|
item.retry_count,
|
||||||
|
Some(0),
|
||||||
|
"retry_count must be reset to 0 in CRDT after unblock"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
!item.blocked.unwrap_or(false),
|
||||||
|
"blocked flag must be cleared in CRDT after unblock: {:?}",
|
||||||
|
item.blocked
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn unblock_command_finds_story_in_any_stage() {
|
fn unblock_command_finds_story_in_any_stage() {
|
||||||
let tmp = tempfile::TempDir::new().unwrap();
|
let tmp = tempfile::TempDir::new().unwrap();
|
||||||
|
|||||||
@@ -53,7 +53,8 @@ pub use types::{
|
|||||||
};
|
};
|
||||||
pub use write::{
|
pub use write::{
|
||||||
bump_retry_count, migrate_names_from_slugs, migrate_story_ids_to_numeric, name_from_story_id,
|
bump_retry_count, migrate_names_from_slugs, migrate_story_ids_to_numeric, name_from_story_id,
|
||||||
set_agent, set_depends_on, set_mergemaster_attempted, set_qa_mode, set_retry_count, write_item,
|
set_agent, set_blocked, set_depends_on, set_mergemaster_attempted, set_qa_mode,
|
||||||
|
set_retry_count, write_item,
|
||||||
};
|
};
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
|
|||||||
@@ -285,6 +285,25 @@ pub fn set_retry_count(story_id: &str, count: i64) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Set the `blocked` register on a story to the given value.
|
||||||
|
///
|
||||||
|
/// Pure metadata operation — the item's stage is not changed.
|
||||||
|
/// Use this alongside a state-machine transition out of `Blocked` /
|
||||||
|
/// `MergeFailure` to keep the legacy `blocked` register in sync with the
|
||||||
|
/// typed stage post-865 (where YAML side-effects no longer clear the
|
||||||
|
/// register on their own).
|
||||||
|
pub fn set_blocked(story_id: &str, blocked: bool) {
|
||||||
|
let Some(state_mutex) = get_crdt() else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
let Ok(mut state) = state_mutex.lock() else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
if let Some(&idx) = state.index.get(story_id) {
|
||||||
|
apply_and_persist(&mut state, |s| s.crdt.doc.items[idx].blocked.set(blocked));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Increment `retry_count` by 1 and return the new value.
|
/// Increment `retry_count` by 1 and return the new value.
|
||||||
///
|
///
|
||||||
/// Pure metadata operation — the item's stage is not changed.
|
/// Pure metadata operation — the item's stage is not changed.
|
||||||
|
|||||||
@@ -10,7 +10,7 @@ mod migrations;
|
|||||||
mod tests;
|
mod tests;
|
||||||
|
|
||||||
pub use item::{
|
pub use item::{
|
||||||
bump_retry_count, set_agent, set_depends_on, set_mergemaster_attempted, set_qa_mode,
|
bump_retry_count, set_agent, set_blocked, set_depends_on, set_mergemaster_attempted,
|
||||||
set_retry_count, write_item,
|
set_qa_mode, set_retry_count, write_item,
|
||||||
};
|
};
|
||||||
pub use migrations::{migrate_names_from_slugs, migrate_story_ids_to_numeric, name_from_story_id};
|
pub use migrations::{migrate_names_from_slugs, migrate_story_ids_to_numeric, name_from_story_id};
|
||||||
|
|||||||
Reference in New Issue
Block a user