From 48263689ed2862acdb69113d693dae4704ed0a25 Mon Sep 17 00:00:00 2001 From: proboscis Date: Mon, 26 Jan 2026 21:58:48 +0900 Subject: [PATCH] fix: unify playlist item short ID hash algorithm to SHA256 Replace unstable DefaultHasher with stable SHA256-based hashing in PlaylistItem::short_id() to ensure consistency with Runnable::PlaylistItem.short_id(). This fixes the hash mismatch where 'runbox playlist show' displayed different short IDs than 'runbox run' could resolve, making playlist item IDs unusable for running commands directly. Changes: - Export stable_short_id() from runnable.rs - Update PlaylistItem::short_id() to use the same SHA256 algorithm - All playlist item IDs are now stable across Rust versions Closes ISSUE-022 --- crates/runbox-core/src/lib.rs | 2 +- crates/runbox-core/src/playlist.rs | 23 +++++++------ crates/runbox-core/src/runnable.rs | 52 +++++++++++++++++++----------- 3 files changed, 47 insertions(+), 30 deletions(-) diff --git a/crates/runbox-core/src/lib.rs b/crates/runbox-core/src/lib.rs index be23879..3e5bec8 100644 --- a/crates/runbox-core/src/lib.rs +++ b/crates/runbox-core/src/lib.rs @@ -29,7 +29,7 @@ pub use record::{Record, RecordCommand, RecordGitState, RecordValidationError}; pub use result::{Artifact, Execution, Output, RunResult}; pub use run::{CodeState, Exec, LogRef, Patch, Run, RunStatus, RuntimeHandle, Timeline}; pub use runnable::{ - format_ambiguous_matches, ResolveResult, Runnable, RunnableMatch, RunnableType, + format_ambiguous_matches, stable_short_id, ResolveResult, Runnable, RunnableMatch, RunnableType, }; pub use runtime::{BackgroundAdapter, RuntimeAdapter, RuntimeRegistry, TmuxAdapter}; pub use skill::{ diff --git a/crates/runbox-core/src/playlist.rs b/crates/runbox-core/src/playlist.rs index 5139768..fecc4f7 100644 --- a/crates/runbox-core/src/playlist.rs +++ b/crates/runbox-core/src/playlist.rs @@ -18,17 +18,20 @@ pub struct PlaylistItem { impl PlaylistItem { /// Generate a short ID for this playlist item based on hash of playlist_id, index, and template_id. - /// The short ID is deterministic and unique within a playlist. + /// The short ID is deterministic, unique within a playlist, and stable across Rust versions (uses SHA256). + /// + /// This uses the same algorithm as `Runnable::PlaylistItem.short_id()` to ensure consistency + /// between `runbox playlist show` and `runbox run `. pub fn short_id(&self, playlist_id: &str, index: usize) -> String { - use std::collections::hash_map::DefaultHasher; - use std::hash::{Hash, Hasher}; - - let mut hasher = DefaultHasher::new(); - playlist_id.hash(&mut hasher); - index.hash(&mut hasher); - self.template_id.hash(&mut hasher); - - format!("{:08x}", hasher.finish() as u32) // First 8 hex chars + use crate::runnable::stable_short_id; + + let mut data = b"playlist_item\0".to_vec(); + data.extend_from_slice(playlist_id.as_bytes()); + data.push(0); + data.extend_from_slice(index.to_string().as_bytes()); + data.push(0); + data.extend_from_slice(self.template_id.as_bytes()); + stable_short_id(&data) } } diff --git a/crates/runbox-core/src/runnable.rs b/crates/runbox-core/src/runnable.rs index f6de6be..217a6a4 100644 --- a/crates/runbox-core/src/runnable.rs +++ b/crates/runbox-core/src/runnable.rs @@ -35,7 +35,10 @@ impl std::str::FromStr for RunnableType { "template" => Ok(RunnableType::Template), "replay" => Ok(RunnableType::Replay), "playlist" => Ok(RunnableType::Playlist), - _ => Err(format!("Invalid runnable type: {}. Valid types: template, replay, playlist", s)), + _ => Err(format!( + "Invalid runnable type: {}. Valid types: template, replay, playlist", + s + )), } } } @@ -61,10 +64,13 @@ pub enum Runnable { /// Generate a stable 8-character hex short ID from input bytes using SHA256. /// This is stable across Rust versions unlike DefaultHasher. -fn stable_short_id(data: &[u8]) -> String { +pub fn stable_short_id(data: &[u8]) -> String { let hash = Sha256::digest(data); // Take first 4 bytes (8 hex chars) - format!("{:02x}{:02x}{:02x}{:02x}", hash[0], hash[1], hash[2], hash[3]) + format!( + "{:02x}{:02x}{:02x}{:02x}", + hash[0], hash[1], hash[2], hash[3] + ) } /// Check if a string looks like a valid UUID hex portion (for replay short ID extraction) @@ -91,10 +97,8 @@ impl Runnable { Runnable::Replay(run_id) => { // run_id format: "run_{uuid}" // Extract hex chars from UUID, removing "run_" prefix and dashes - let uuid_part = run_id - .trim_start_matches("run_") - .replace('-', ""); - + let uuid_part = run_id.trim_start_matches("run_").replace('-', ""); + // If it looks like valid UUID hex, extract first 8 chars (lowercase) if is_valid_uuid_hex(&uuid_part) { uuid_part.chars().take(8).collect::().to_lowercase() @@ -168,7 +172,7 @@ impl Runnable { /// Returns the source label for display in list view. /// /// - Template: "-" (no source, it's a root definition) - /// - Replay: shortened run_id + /// - Replay: shortened run_id /// - PlaylistItem: "name[index]" format pub fn source_label(&self) -> String { match self { @@ -178,9 +182,7 @@ impl Runnable { run_id.trim_start_matches("run_").chars().take(10).collect() } Runnable::PlaylistItem { - playlist_id, - index, - .. + playlist_id, index, .. } => { // Format as "name[idx]" - strip the "pl_" prefix for brevity let name = playlist_id.trim_start_matches("pl_"); @@ -298,7 +300,7 @@ mod tests { // If this test fails after a code change, it means short IDs have changed let runnable = Runnable::Template("tpl_echo".to_string()); let short_id = runnable.short_id(); - + // SHA256("template\0tpl_echo") first 4 bytes as hex // This is a known value that should not change assert_eq!(short_id.len(), 8); @@ -331,11 +333,11 @@ mod tests { // Non-UUID run ID should fallback to hash let runnable = Runnable::Replay("run_custom_id_123".to_string()); let short_id = runnable.short_id(); - + // Should be 8 hex chars (from hash) assert_eq!(short_id.len(), 8); assert!(short_id.chars().all(|c| c.is_ascii_hexdigit())); - + // Should be deterministic let short_id2 = Runnable::Replay("run_custom_id_123".to_string()).short_id(); assert_eq!(short_id, short_id2); @@ -384,7 +386,7 @@ mod tests { label: None, }; let short_id = runnable.short_id(); - + assert_eq!(short_id.len(), 8); assert!(short_id.chars().all(|c| c.is_ascii_hexdigit())); } @@ -578,10 +580,22 @@ mod tests { #[test] fn test_runnable_type_from_str() { - assert_eq!("template".parse::().unwrap(), RunnableType::Template); - assert_eq!("replay".parse::().unwrap(), RunnableType::Replay); - assert_eq!("playlist".parse::().unwrap(), RunnableType::Playlist); - assert_eq!("TEMPLATE".parse::().unwrap(), RunnableType::Template); + assert_eq!( + "template".parse::().unwrap(), + RunnableType::Template + ); + assert_eq!( + "replay".parse::().unwrap(), + RunnableType::Replay + ); + assert_eq!( + "playlist".parse::().unwrap(), + RunnableType::Playlist + ); + assert_eq!( + "TEMPLATE".parse::().unwrap(), + RunnableType::Template + ); assert!("invalid".parse::().is_err()); }