diff --git a/server/src/chat/commands/show.rs b/server/src/chat/commands/show.rs index bd8f681b..c3b7f559 100644 --- a/server/src/chat/commands/show.rs +++ b/server/src/chat/commands/show.rs @@ -102,10 +102,14 @@ pub(super) fn handle_show(ctx: &CommandContext) -> Option { } }; - // `content` comes from the CRDT / content store. If unavailable, report - // it rather than silently reading a stale on-disk copy. + // `content` must be present for any story the shared lookup returns — + // story 1222 made the content store retain a story's body indefinitely, + // so a missing body here is a content-store bug, not an expected state. + // Fail loudly instead of masking it with a placeholder message. let text = content.unwrap_or_else(|| { - format!("Story {story_id} found in pipeline but its content is unavailable.") + panic!( + "story {story_id} found by find_story_by_number but has no content in the content store" + ) }); // Strip front matter block from the displayed body; source the metadata diff --git a/server/src/chat/commands/triage.rs b/server/src/chat/commands/triage.rs index 1af936ed..cd36b654 100644 --- a/server/src/chat/commands/triage.rs +++ b/server/src/chat/commands/triage.rs @@ -19,46 +19,34 @@ use std::process::Command; const MAX_DIRTY_FILES_SHOWN: usize = 20; /// Handle `{bot_name} status {number}`. +/// +/// Resolves the numeric prefix via the shared [`crate::chat::lookup::find_story_by_number`] +/// lookup against the room's active project — the same lookup `show` uses — +/// rather than a separate pipeline-item scan, so `status ` and `show ` +/// always agree on which story a bare number refers to (story 1237). pub(super) fn handle_triage(ctx: &CommandContext) -> Option { let num_str = ctx.args.trim(); if num_str.is_empty() || !num_str.chars().all(|c| c.is_ascii_digit()) { return None; } - match find_story_by_number(num_str) { - Some((story_id, item)) => Some(build_triage_dump(ctx, &story_id, &item, num_str)), + match crate::chat::lookup::find_story_by_number(ctx.effective_root(), num_str) { + Some((story_id, stage_dir, _path, content)) => Some(build_triage_dump( + ctx, &story_id, &stage_dir, content, num_str, + )), None => Some(format!("Story **{num_str}** not found in the pipeline.")), } } -/// Find a pipeline item whose numeric prefix matches `num_str` by querying the -/// CRDT state. Returns `(story_id, PipelineItem)` for the first match. -fn find_story_by_number(num_str: &str) -> Option<(String, crate::pipeline_state::PipelineItem)> { - let items = crate::pipeline_state::read_all_typed(); - for item in items { - let file_num = item - .story_id - .0 - .split('_') - .next() - .filter(|s| !s.is_empty() && s.chars().all(|c| c.is_ascii_digit())) - .unwrap_or(""); - if file_num == num_str { - let story_id = item.story_id.0.clone(); - return Some((story_id, item)); - } - } - None -} - /// Build the full triage dump for a story. fn build_triage_dump( ctx: &CommandContext, story_id: &str, - item: &crate::pipeline_state::PipelineItem, + fallback_stage_dir: &str, + content: Option, num_str: &str, ) -> String { - let contents = match crate::db::read_content(crate::db::ContentKey::Story(story_id)) { + let contents = match content { Some(c) => c, None => return format!("Story {num_str}: content not found in content store."), }; @@ -72,8 +60,18 @@ fn build_triage_dump( // ---- Header ---- out.push_str(&format!("## Story {num_str} — {name}\n")); - let stage_name = crate::pipeline_state::stage_label(&item.stage); - let dir_name = crate::pipeline_state::stage_dir_name(&item.stage); + // `fallback_stage_dir` covers the rare case where the shared lookup found + // the story via the content store but it hasn't synced into the CRDT yet. + let (stage_name, dir_name) = match crdt_item.as_ref() { + Some(w) => ( + crate::pipeline_state::stage_label(w.stage()).to_string(), + crate::pipeline_state::stage_dir_name(w.stage()).to_string(), + ), + None => ( + fallback_stage_dir.to_string(), + fallback_stage_dir.to_string(), + ), + }; out.push_str(&format!("**Stage:** {stage_name} (`{dir_name}`)\n\n")); // ---- CRDT metadata ---- @@ -356,6 +354,44 @@ mod tests { ); } + /// Story 1237, AC1 + AC3: `status ` and `show ` must resolve a bare + /// story number to the same story in the same room — both now go through + /// the shared `chat::lookup::find_story_by_number` lookup instead of + /// `status`/`triage` running its own separate pipeline-item scan. + #[test] + fn status_and_show_resolve_to_same_story_in_same_room() { + let tmp = tempfile::TempDir::new().unwrap(); + write_story_file( + tmp.path(), + "2_current", + "9910_story_shared_lookup.md", + "---\nname: Shared Lookup Test\n---\n\n# Story\n\nBody text for consistency check.", + Some("Shared Lookup Test"), + ); + + let status_output = status_triage_cmd(tmp.path(), "9910").unwrap(); + + let services = + crate::services::Services::new_test(tmp.path().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, + }; + let show_output = try_handle_command(&dispatch, "@timmy show 9910").unwrap(); + + assert!( + status_output.contains("Shared Lookup Test"), + "status should resolve story 9910: {status_output}" + ); + assert!( + show_output.contains("Body text for consistency check."), + "show should resolve story 9910: {show_output}" + ); + } + #[test] fn whatsup_works_for_story_in_backlog() { let tmp = tempfile::TempDir::new().unwrap();