a49a1cf7cb
Each chat command that previously read parse_front_matter for story metadata (name, agent, depends_on, blocked, retry_count, merge_failure, qa_mode) now reads from the typed CRDT API: - WorkItem (via crdt_state::read_item) for pipeline-item registers. - MergeJobView (via crdt_state::read_merge_job) for the merge failure detail text, which has its own LWW-map CRDT entry. Files migrated: depends.rs, freeze.rs, move_story.rs, overview.rs, status/render.rs, triage.rs, unblock.rs, unreleased.rs. unblock.rs: also removes the legacy front-matter cleanup branch that fired when the typed Blocked→Coding transition failed. Post-929 there is no YAML on disk to clean; the fallback now just resets retry_count in the CRDT. triage.rs: drops the YAML-only `review_hold` and `coverage_baseline` fields from the dump. These have no CRDT register and were never load-bearing on the triage output; if needed later, add a CRDT register and surface it back. Tests: - The three status/render merge-failure rendering tests now seed a MergeJob CRDT entry via write_merge_job instead of writing YAML. - The unblock test that asserted YAML cleanup on disk is now an assertion on the CRDT registers (blocked=false, retry_count=0). Also re-seeded in `2_blocked` stage so the typed Blocked → Coding transition actually fires (not the fallback path). All 2855 tests pass; fmt clean; clippy clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
350 lines
12 KiB
Rust
350 lines
12 KiB
Rust
//! Handler for the `unblock` command.
|
|
//!
|
|
//! `{bot_name} unblock {number}` finds the blocked work item by number across
|
|
//! all pipeline stages, clears the `blocked` flag, resets `retry_count` to 0,
|
|
//! and returns a confirmation.
|
|
|
|
use super::CommandContext;
|
|
use std::path::Path;
|
|
|
|
/// Handle the `unblock` command.
|
|
///
|
|
/// Parses `<number>` from `ctx.args`, locates the work item, checks that it is
|
|
/// blocked, clears the `blocked` flag, resets `retry_count` to 0, and returns
|
|
/// a Markdown confirmation string.
|
|
pub(super) fn handle_unblock(ctx: &CommandContext) -> Option<String> {
|
|
let num_str = ctx.args.trim();
|
|
|
|
if num_str.is_empty() || !num_str.chars().all(|c| c.is_ascii_digit()) {
|
|
return Some(format!(
|
|
"Usage: `{} unblock <number>` (e.g. `unblock 42`)",
|
|
ctx.services.bot_name
|
|
));
|
|
}
|
|
|
|
Some(unblock_by_number(ctx.effective_root(), num_str))
|
|
}
|
|
|
|
/// Core unblock logic: find story by numeric prefix and reset its blocked state.
|
|
///
|
|
/// Returns a Markdown-formatted response string suitable for all transports.
|
|
/// Also used by the MCP `unblock` tool.
|
|
///
|
|
/// Lookup priority: CRDT → content store.
|
|
pub(crate) fn unblock_by_number(project_root: &Path, story_number: &str) -> String {
|
|
let (story_id, _, _, _) =
|
|
match crate::chat::lookup::find_story_by_number(project_root, story_number) {
|
|
Some(found) => found,
|
|
None => {
|
|
return format!("No story, bug, or spike with number **{story_number}** found.");
|
|
}
|
|
};
|
|
|
|
unblock_by_story_id(&story_id)
|
|
}
|
|
|
|
/// Unblock a story via the typed state machine.
|
|
///
|
|
/// Checks whether the story is in `Stage::Blocked` (new path) or has legacy
|
|
/// `blocked: true` / `merge_failure` front-matter, then routes through
|
|
/// [`crate::agents::lifecycle::transition_to_unblocked`].
|
|
fn unblock_by_story_id(story_id: &str) -> String {
|
|
// Post-865, story content is CRDT-only (no YAML front matter on disk or in
|
|
// the content store). Read the story name and blocked state from CRDT
|
|
// registers; do NOT call `parse_front_matter` on the content.
|
|
let crdt_item = crate::crdt_state::read_item(story_id);
|
|
let story_name = crdt_item
|
|
.as_ref()
|
|
.and_then(|i| i.name().map(str::to_string))
|
|
.unwrap_or_else(|| story_id.to_string());
|
|
|
|
// Canonical "is this story blocked?" comes from the typed pipeline state.
|
|
let typed_item = crate::pipeline_state::read_typed(story_id).ok().flatten();
|
|
let typed_blocked = typed_item
|
|
.as_ref()
|
|
.is_some_and(|item| item.stage.is_blocked());
|
|
let typed_merge_failure = matches!(
|
|
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().is_some_and(|i| i.blocked());
|
|
|
|
if !typed_blocked && !crdt_blocked {
|
|
return format!("**{story_name}** ({story_id}) is not blocked. Nothing to unblock.");
|
|
}
|
|
|
|
// Route through the state machine. This clears blocked/merge_failure/
|
|
// retry_count via `fields_to_clear_transform` and resets retry_count in
|
|
// the CRDT.
|
|
if let Err(e) = crate::agents::lifecycle::transition_to_unblocked(story_id) {
|
|
// If the typed transition fails (e.g. a legacy Archived item with no
|
|
// valid `Unblock` transition out of its current stage), at least
|
|
// reset retry_count directly in the CRDT so the agent doesn't stay
|
|
// tagged with a stale fail counter. Post-929 there's no FS shadow
|
|
// to clean up alongside.
|
|
crate::slog_warn!(
|
|
"[unblock] State-machine transition failed for '{story_id}': {e}. \
|
|
Falling back to direct CRDT retry_count reset."
|
|
);
|
|
crate::crdt_state::set_retry_count(story_id, 0);
|
|
}
|
|
|
|
let cleared = if typed_merge_failure {
|
|
"merge_failure"
|
|
} else {
|
|
"blocked"
|
|
};
|
|
format!("Unblocked **{story_name}** ({story_id}). Cleared: {cleared}. Retry count reset to 0.")
|
|
}
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Tests
|
|
// ---------------------------------------------------------------------------
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::super::{CommandDispatch, try_handle_command};
|
|
|
|
fn unblock_cmd_with_root(root: &std::path::Path, args: &str) -> Option<String> {
|
|
let services = crate::services::Services::new_test(root.to_path_buf(), "Timmy".to_string());
|
|
let room_id = "!test:example.com".to_string();
|
|
let dispatch = CommandDispatch {
|
|
services: &services,
|
|
project_root: &services.project_root,
|
|
bot_user_id: "@timmy:homeserver.local",
|
|
room_id: &room_id,
|
|
};
|
|
try_handle_command(&dispatch, &format!("@timmy unblock {args}"))
|
|
}
|
|
|
|
use crate::chat::test_helpers::write_story_file;
|
|
|
|
#[test]
|
|
fn unblock_command_is_registered() {
|
|
use super::super::commands;
|
|
assert!(
|
|
commands().iter().any(|c| c.name == "unblock"),
|
|
"unblock command must be in the registry"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn unblock_command_appears_in_help() {
|
|
let result = super::super::tests::try_cmd_addressed(
|
|
"Timmy",
|
|
"@timmy:homeserver.local",
|
|
"@timmy help",
|
|
);
|
|
let output = result.unwrap();
|
|
assert!(
|
|
output.contains("unblock"),
|
|
"help should list unblock command: {output}"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn unblock_command_no_args_returns_usage() {
|
|
let tmp = tempfile::TempDir::new().unwrap();
|
|
let output = unblock_cmd_with_root(tmp.path(), "").unwrap();
|
|
assert!(
|
|
output.contains("Usage"),
|
|
"no args should show usage hint: {output}"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn unblock_command_non_numeric_returns_usage() {
|
|
let tmp = tempfile::TempDir::new().unwrap();
|
|
let output = unblock_cmd_with_root(tmp.path(), "abc").unwrap();
|
|
assert!(
|
|
output.contains("Usage"),
|
|
"non-numeric arg should show usage hint: {output}"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn unblock_command_not_found_returns_error() {
|
|
let tmp = tempfile::TempDir::new().unwrap();
|
|
let output = unblock_cmd_with_root(tmp.path(), "999").unwrap();
|
|
assert!(
|
|
output.contains("999") && output.contains("found"),
|
|
"not-found message should include number and 'found': {output}"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn unblock_command_not_blocked_returns_error() {
|
|
let tmp = tempfile::TempDir::new().unwrap();
|
|
write_story_file(
|
|
tmp.path(),
|
|
"2_current",
|
|
"42_story_test.md",
|
|
"---\nname: Test Story\nretry_count: 2\n---\n# Story\n",
|
|
);
|
|
let output = unblock_cmd_with_root(tmp.path(), "42").unwrap();
|
|
assert!(
|
|
output.contains("not blocked"),
|
|
"non-blocked story should return not-blocked error: {output}"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn unblock_command_clears_blocked_and_resets_retry_count() {
|
|
crate::crdt_state::init_for_test();
|
|
let tmp = tempfile::TempDir::new().unwrap();
|
|
// Use a high story number (9903) to avoid collisions with other tests in the
|
|
// global content store.
|
|
write_story_file(
|
|
tmp.path(),
|
|
"2_blocked",
|
|
"9903_story_stuck.md",
|
|
"---\nname: Stuck Story\nblocked: true\nretry_count: 5\n---\n# Story\n",
|
|
);
|
|
// Seed the story in the CRDT in 2_blocked stage so the typed
|
|
// Blocked → Coding transition fires and clears `blocked` properly.
|
|
crate::crdt_state::write_item(
|
|
"9903_story_stuck",
|
|
"2_blocked",
|
|
Some("Stuck Story"),
|
|
None,
|
|
Some(5),
|
|
Some(true),
|
|
None,
|
|
None,
|
|
None,
|
|
None,
|
|
);
|
|
|
|
let output = unblock_cmd_with_root(tmp.path(), "9903").unwrap();
|
|
assert!(
|
|
output.contains("Unblocked") && output.contains("Stuck Story"),
|
|
"should confirm unblock with story name: {output}"
|
|
);
|
|
assert!(
|
|
output.contains("9903_story_stuck"),
|
|
"should include story_id in response: {output}"
|
|
);
|
|
|
|
// Post-929: the CRDT is the sole source of truth; we no longer clean
|
|
// YAML front matter from on-disk content. Verify the CRDT registers
|
|
// were updated correctly.
|
|
let item = crate::crdt_state::read_item("9903_story_stuck")
|
|
.expect("story should be in CRDT after unblock");
|
|
assert_eq!(
|
|
item.retry_count(),
|
|
0,
|
|
"retry_count should be reset to 0 in CRDT after unblock"
|
|
);
|
|
assert!(
|
|
!item.blocked(),
|
|
"blocked flag should be cleared in CRDT after unblock"
|
|
);
|
|
}
|
|
|
|
#[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(),
|
|
0,
|
|
"retry_count must be reset to 0 in CRDT after unblock"
|
|
);
|
|
assert!(
|
|
!item.blocked(),
|
|
"blocked flag must be cleared in CRDT after unblock"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn unblock_command_finds_story_in_any_stage() {
|
|
let tmp = tempfile::TempDir::new().unwrap();
|
|
// Use a high story number (9901) to avoid collisions with other tests in the
|
|
// global content store.
|
|
write_story_file(
|
|
tmp.path(),
|
|
"3_qa",
|
|
"9901_story_in_qa.md",
|
|
"---\nname: In QA\nblocked: true\nretry_count: 3\n---\n# Story\n",
|
|
);
|
|
|
|
let output = unblock_cmd_with_root(tmp.path(), "9901").unwrap();
|
|
assert!(
|
|
output.contains("Unblocked"),
|
|
"should unblock story in qa stage: {output}"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn unblock_command_includes_story_id_in_response() {
|
|
let tmp = tempfile::TempDir::new().unwrap();
|
|
// Use a high story number (9902) to avoid collisions with other tests in the
|
|
// global content store.
|
|
write_story_file(
|
|
tmp.path(),
|
|
"1_backlog",
|
|
"9902_story_blocked_one.md",
|
|
"---\nname: Blocked One\nblocked: true\nretry_count: 2\n---\n",
|
|
);
|
|
|
|
let output = unblock_cmd_with_root(tmp.path(), "9902").unwrap();
|
|
assert!(
|
|
output.contains("9902_story_blocked_one"),
|
|
"response should include story_id: {output}"
|
|
);
|
|
}
|
|
}
|