huskies: merge 1195 bug Chat show renders metadata from stale content-text YAML instead of CRDT registers
This commit is contained in:
@@ -3,66 +3,84 @@
|
|||||||
use super::CommandContext;
|
use super::CommandContext;
|
||||||
use crate::io::story_metadata::QaMode;
|
use crate::io::story_metadata::QaMode;
|
||||||
|
|
||||||
/// Strip YAML front matter and return a summary of useful fields + the remaining body.
|
/// Strip a leading YAML front-matter block (`---\n...\n---`) and return the
|
||||||
|
/// remaining body text.
|
||||||
|
///
|
||||||
|
/// The front-matter block itself is no longer parsed for display — story
|
||||||
|
/// 1195 found that field extracted here (agent, depends_on, blocked,
|
||||||
|
/// retry_count, qa) can go stale relative to the CRDT registers that are the
|
||||||
|
/// actual source of truth. Metadata display now comes exclusively from
|
||||||
|
/// [`crdt_metadata_summary`].
|
||||||
#[allow(clippy::string_slice)] // indices from find("\n---") on ASCII delimiter; "---" and "\n---" are ASCII-only
|
#[allow(clippy::string_slice)] // indices from find("\n---") on ASCII delimiter; "---" and "\n---" are ASCII-only
|
||||||
fn strip_front_matter(text: &str) -> (String, String) {
|
fn strip_front_matter(text: &str) -> String {
|
||||||
let trimmed = text.trim_start();
|
let trimmed = text.trim_start();
|
||||||
if !trimmed.starts_with("---") {
|
if !trimmed.starts_with("---") {
|
||||||
return (String::new(), text.to_string());
|
return text.to_string();
|
||||||
}
|
}
|
||||||
|
|
||||||
// Find the closing ---
|
// Find the closing ---
|
||||||
if let Some(end) = trimmed[3..].find("\n---") {
|
if let Some(end) = trimmed[3..].find("\n---") {
|
||||||
let yaml_block = &trimmed[3..3 + end].trim();
|
trimmed[3 + end + 4..].to_string() // skip past closing ---
|
||||||
let body = &trimmed[3 + end + 4..]; // skip past closing ---
|
|
||||||
|
|
||||||
// Extract useful fields from YAML (simple line-based parsing)
|
|
||||||
let mut parts = Vec::new();
|
|
||||||
for line in yaml_block.lines() {
|
|
||||||
let line = line.trim();
|
|
||||||
if line.starts_with("depends_on:") {
|
|
||||||
let val = line.trim_start_matches("depends_on:").trim();
|
|
||||||
if !val.is_empty() && val != "[]" {
|
|
||||||
parts.push(format!("**Depends on:** {val}"));
|
|
||||||
}
|
|
||||||
} else if line.starts_with("agent:") {
|
|
||||||
let val = line.trim_start_matches("agent:").trim().trim_matches('"');
|
|
||||||
if !val.is_empty() {
|
|
||||||
parts.push(format!("**Agent:** {val}"));
|
|
||||||
}
|
|
||||||
} else if line.starts_with("blocked:") {
|
|
||||||
let val = line.trim_start_matches("blocked:").trim();
|
|
||||||
if val == "true" {
|
|
||||||
parts.push("**Blocked:** yes".to_string());
|
|
||||||
}
|
|
||||||
} else if line.starts_with("retry_count:") {
|
|
||||||
let val = line.trim_start_matches("retry_count:").trim();
|
|
||||||
if val != "0" && !val.is_empty() {
|
|
||||||
parts.push(format!("**Retries:** {val}"));
|
|
||||||
}
|
|
||||||
} else if line.starts_with("qa:") {
|
|
||||||
let val = line.trim_start_matches("qa:").trim().trim_matches('"');
|
|
||||||
if let Some(QaMode::Human) = QaMode::from_str(val) {
|
|
||||||
parts.push("**QA:** human review required".to_string());
|
|
||||||
}
|
|
||||||
} else if line.starts_with("merge_failure:") {
|
|
||||||
let val = line
|
|
||||||
.trim_start_matches("merge_failure:")
|
|
||||||
.trim()
|
|
||||||
.trim_matches('"');
|
|
||||||
if !val.is_empty() {
|
|
||||||
parts.push(format!("**Merge failure:** {val}"));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
(parts.join(" · "), body.to_string())
|
|
||||||
} else {
|
} else {
|
||||||
// No closing ---, return as-is
|
// No closing ---, return as-is
|
||||||
(String::new(), text.to_string())
|
text.to_string()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Build the metadata summary line from CRDT registers: agent, depends_on,
|
||||||
|
/// blocked, retry count, QA mode, and merge failure detail.
|
||||||
|
///
|
||||||
|
/// Register values are the sole source here — no fallback to content-text
|
||||||
|
/// YAML, which can go stale relative to the CRDT (story 1195). Mirrors the
|
||||||
|
/// CRDT-first pattern already used by `status <number>` (`triage.rs`).
|
||||||
|
fn crdt_metadata_summary(story_id: &str) -> String {
|
||||||
|
let Some(item) = crate::crdt_state::read_item(story_id) else {
|
||||||
|
return String::new();
|
||||||
|
};
|
||||||
|
|
||||||
|
let mut parts = Vec::new();
|
||||||
|
|
||||||
|
if matches!(
|
||||||
|
item.stage(),
|
||||||
|
crate::pipeline_state::Stage::Blocked { .. }
|
||||||
|
| crate::pipeline_state::Stage::MergeFailure { .. }
|
||||||
|
| crate::pipeline_state::Stage::MergeFailureFinal { .. }
|
||||||
|
| crate::pipeline_state::Stage::Archived {
|
||||||
|
reason: crate::pipeline_state::ArchiveReason::Blocked { .. },
|
||||||
|
..
|
||||||
|
}
|
||||||
|
) {
|
||||||
|
parts.push("**Blocked:** yes".to_string());
|
||||||
|
}
|
||||||
|
|
||||||
|
if let Some(agent) = item.agent() {
|
||||||
|
parts.push(format!("**Agent:** {agent}"));
|
||||||
|
}
|
||||||
|
|
||||||
|
let deps = item.depends_on();
|
||||||
|
if !deps.is_empty() {
|
||||||
|
let nums: Vec<String> = deps.iter().map(|n| format!("#{n}")).collect();
|
||||||
|
parts.push(format!("**Depends on:** {}", nums.join(", ")));
|
||||||
|
}
|
||||||
|
|
||||||
|
let rc = item.retry_count();
|
||||||
|
if rc > 0 {
|
||||||
|
parts.push(format!("**Retries:** {rc}"));
|
||||||
|
}
|
||||||
|
|
||||||
|
if let Some(QaMode::Human) = item.qa_mode() {
|
||||||
|
parts.push("**QA:** human review required".to_string());
|
||||||
|
}
|
||||||
|
|
||||||
|
if let Some(job) = crate::crdt_state::read_merge_job(story_id)
|
||||||
|
&& let Some(err) = job.error
|
||||||
|
{
|
||||||
|
parts.push(format!("**Merge failure:** {err}"));
|
||||||
|
}
|
||||||
|
|
||||||
|
parts.join(" · ")
|
||||||
|
}
|
||||||
|
|
||||||
/// Display the full markdown text of a work item identified by its numeric ID.
|
/// Display the full markdown text of a work item identified by its numeric ID.
|
||||||
///
|
///
|
||||||
/// Lookup priority: CRDT → content store → filesystem (Story 512).
|
/// Lookup priority: CRDT → content store → filesystem (Story 512).
|
||||||
@@ -90,8 +108,11 @@ pub(super) fn handle_show(ctx: &CommandContext) -> Option<String> {
|
|||||||
format!("Story {story_id} found in pipeline but its content is unavailable.")
|
format!("Story {story_id} found in pipeline but its content is unavailable.")
|
||||||
});
|
});
|
||||||
|
|
||||||
// Strip front matter block and extract useful metadata to show inline.
|
// Strip front matter block from the displayed body; source the metadata
|
||||||
let (front_matter_summary, body) = strip_front_matter(&text);
|
// summary separately from CRDT registers, which win over any stale
|
||||||
|
// content-text YAML (story 1195).
|
||||||
|
let body = strip_front_matter(&text);
|
||||||
|
let front_matter_summary = crdt_metadata_summary(&story_id);
|
||||||
|
|
||||||
// Convert markdown headings to bold text for consistent rendering across
|
// Convert markdown headings to bold text for consistent rendering across
|
||||||
// Matrix clients. Element X doesn't style <h2> tags distinctly, but bold
|
// Matrix clients. Element X doesn't style <h2> tags distinctly, but bold
|
||||||
@@ -254,6 +275,77 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Story 1195, AC 1 + 2: `show` must source metadata from the CRDT
|
||||||
|
/// registers, and a CRDT-only write (e.g. an agent pin via `update_story`)
|
||||||
|
/// must be visible immediately — no content-text rewrite required.
|
||||||
|
#[test]
|
||||||
|
fn show_command_reflects_crdt_only_agent_pin_immediately() {
|
||||||
|
crate::crdt_state::init_for_test();
|
||||||
|
let tmp = tempfile::TempDir::new().unwrap();
|
||||||
|
// Use a high story number to avoid collisions with other tests in the
|
||||||
|
// global content store.
|
||||||
|
write_story_file(
|
||||||
|
tmp.path(),
|
||||||
|
"2_current",
|
||||||
|
"9904_story_pin_test.md",
|
||||||
|
"---\nname: Pin Test\n---\n\n# Story\n\nBody text.",
|
||||||
|
None,
|
||||||
|
);
|
||||||
|
crate::crdt_state::write_item_str(
|
||||||
|
"9904_story_pin_test",
|
||||||
|
"2_current",
|
||||||
|
Some("Pin Test"),
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
);
|
||||||
|
|
||||||
|
// Set the pin via the same CRDT-only register write `update_story`
|
||||||
|
// uses (crdt_state::set_agent) — the content body is never touched.
|
||||||
|
crate::crdt_state::set_agent("9904_story_pin_test", "coder-1".parse().ok());
|
||||||
|
|
||||||
|
let output = show_cmd_with_root(tmp.path(), "9904").unwrap();
|
||||||
|
assert!(
|
||||||
|
output.contains("**Agent:** coder-1"),
|
||||||
|
"show should reflect the CRDT-only agent pin immediately: {output}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Story 1195, AC 1: register values must win over stale content-text
|
||||||
|
/// YAML — a front-matter `agent:` value that no longer matches the CRDT
|
||||||
|
/// register must not leak into the displayed summary.
|
||||||
|
#[test]
|
||||||
|
fn show_command_prefers_crdt_agent_over_stale_yaml_front_matter() {
|
||||||
|
crate::crdt_state::init_for_test();
|
||||||
|
let tmp = tempfile::TempDir::new().unwrap();
|
||||||
|
write_story_file(
|
||||||
|
tmp.path(),
|
||||||
|
"2_current",
|
||||||
|
"9905_story_stale_yaml.md",
|
||||||
|
"---\nname: Stale YAML\nagent: coder-1\n---\n\n# Story\n\nBody text.",
|
||||||
|
None,
|
||||||
|
);
|
||||||
|
crate::crdt_state::write_item_str(
|
||||||
|
"9905_story_stale_yaml",
|
||||||
|
"2_current",
|
||||||
|
Some("Stale YAML"),
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
);
|
||||||
|
crate::crdt_state::set_agent("9905_story_stale_yaml", "coder-2".parse().ok());
|
||||||
|
|
||||||
|
let output = show_cmd_with_root(tmp.path(), "9905").unwrap();
|
||||||
|
assert!(
|
||||||
|
output.contains("**Agent:** coder-2"),
|
||||||
|
"show should display the live CRDT agent: {output}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
!output.contains("**Agent:** coder-1"),
|
||||||
|
"show must not surface the stale YAML agent value: {output}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn show_command_case_insensitive() {
|
fn show_command_case_insensitive() {
|
||||||
let result = super::super::tests::try_cmd_addressed(
|
let result = super::super::tests::try_cmd_addressed(
|
||||||
|
|||||||
Reference in New Issue
Block a user