diff --git a/Cargo.lock b/Cargo.lock index 319a5eab9..160852fb2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1924,6 +1924,7 @@ version = "0.218.0-nightly.0" dependencies = [ "async-trait", "dialoguer", + "fabro-types", "fabro-util", "serde", "serde_json", @@ -2198,6 +2199,7 @@ dependencies = [ "fabro-http", "fabro-interview", "fabro-static", + "fabro-types", "fabro-workflow", "futures-util", "rustls", diff --git a/lib/crates/fabro-api/build.rs b/lib/crates/fabro-api/build.rs index b4ce4c378..be897f31a 100644 --- a/lib/crates/fabro-api/build.rs +++ b/lib/crates/fabro-api/build.rs @@ -309,6 +309,11 @@ fn main() { "fabro_types::settings::server::WebhookStrategy", &[], ), + ("ActorKind", "fabro_types::ActorKind", &[]), + ("ActorRef", "fabro_types::ActorRef", &[]), + ("QuestionType", "fabro_types::QuestionType", &[]), + ("NodeStatusRecord", "fabro_types::NodeStatusRecord", &[]), + ("InternalStageStatus", "fabro_types::StageStatus", &[]), ("PullRequestRecord", "fabro_types::PullRequestRecord", &[]), ("PullRequestDetail", "fabro_types::PullRequestDetail", &[]), ("PullRequestUser", "fabro_types::PullRequestUser", &[]), diff --git a/lib/crates/fabro-api/src/lib.rs b/lib/crates/fabro-api/src/lib.rs index f939bbe84..7218556b3 100644 --- a/lib/crates/fabro-api/src/lib.rs +++ b/lib/crates/fabro-api/src/lib.rs @@ -28,8 +28,9 @@ pub mod types { BlockedReason, FailureReason, RunControlAction, RunStatus, SuccessReason, TerminalStatus, }; pub use fabro_types::{ - DiffStats, DirtyStatus, GitContext, PreRunPushOutcome, RepositoryReference, RunSummary, - SecretType, ServerSettings, WorkflowSettings, + ActorKind, ActorRef, DiffStats, DirtyStatus, GitContext, NodeStatusRecord, + PreRunPushOutcome, QuestionType, RepositoryReference, RunSummary, SecretType, + ServerSettings, StageStatus as InternalStageStatus, WorkflowSettings, }; pub use crate::generated::types::*; diff --git a/lib/crates/fabro-api/tests/actor_kind_round_trip.rs b/lib/crates/fabro-api/tests/actor_kind_round_trip.rs new file mode 100644 index 000000000..9a500ae2a --- /dev/null +++ b/lib/crates/fabro-api/tests/actor_kind_round_trip.rs @@ -0,0 +1,52 @@ +use std::any::{TypeId, type_name}; + +use fabro_api::types::ActorKind as ApiActorKind; +use fabro_types::ActorKind; +use serde_json::json; + +#[test] +fn actor_kind_reuses_canonical_type() { + assert_same_type::(); +} + +#[test] +fn actor_kind_serializes_as_snake_case_strings() { + assert_eq!( + serde_json::to_value(ActorKind::User).unwrap(), + json!("user") + ); + assert_eq!( + serde_json::to_value(ActorKind::Agent).unwrap(), + json!("agent") + ); + assert_eq!( + serde_json::to_value(ActorKind::System).unwrap(), + json!("system") + ); +} + +#[test] +fn actor_kind_deserializes_each_variant() { + assert_eq!( + serde_json::from_value::(json!("user")).unwrap(), + ActorKind::User + ); + assert_eq!( + serde_json::from_value::(json!("agent")).unwrap(), + ActorKind::Agent + ); + assert_eq!( + serde_json::from_value::(json!("system")).unwrap(), + ActorKind::System + ); +} + +fn assert_same_type() { + assert_eq!( + TypeId::of::(), + TypeId::of::(), + "{} should be the same type as {}", + type_name::(), + type_name::() + ); +} diff --git a/lib/crates/fabro-api/tests/actor_ref_round_trip.rs b/lib/crates/fabro-api/tests/actor_ref_round_trip.rs new file mode 100644 index 000000000..ff381bac5 --- /dev/null +++ b/lib/crates/fabro-api/tests/actor_ref_round_trip.rs @@ -0,0 +1,51 @@ +use std::any::{TypeId, type_name}; + +use fabro_api::types::ActorRef as ApiActorRef; +use fabro_types::{ActorKind, ActorRef}; +use serde_json::json; + +#[test] +fn actor_ref_reuses_canonical_type() { + assert_same_type::(); +} + +#[test] +fn actor_ref_round_trips_representative_json() { + let value = json!({ + "kind": "agent", + "id": "agent-1", + "display": "Agent 1" + }); + + let actor: ActorRef = serde_json::from_value(value.clone()).unwrap(); + assert_eq!(actor, ActorRef { + kind: ActorKind::Agent, + id: Some("agent-1".to_string()), + display: Some("Agent 1".to_string()), + }); + assert_eq!(serde_json::to_value(actor).unwrap(), value); +} + +#[test] +fn actor_ref_omits_absent_optional_fields() { + let actor = ActorRef { + kind: ActorKind::System, + id: None, + display: None, + }; + + assert_eq!( + serde_json::to_value(actor).unwrap(), + json!({"kind": "system"}) + ); +} + +fn assert_same_type() { + assert_eq!( + TypeId::of::(), + TypeId::of::(), + "{} should be the same type as {}", + type_name::(), + type_name::() + ); +} diff --git a/lib/crates/fabro-api/tests/internal_stage_status_round_trip.rs b/lib/crates/fabro-api/tests/internal_stage_status_round_trip.rs new file mode 100644 index 000000000..41667fb1d --- /dev/null +++ b/lib/crates/fabro-api/tests/internal_stage_status_round_trip.rs @@ -0,0 +1,68 @@ +use std::any::{TypeId, type_name}; + +use fabro_api::types::InternalStageStatus as ApiInternalStageStatus; +use fabro_types::StageStatus; +use serde_json::json; + +#[test] +fn internal_stage_status_reuses_canonical_type() { + assert_same_type::(); +} + +#[test] +fn internal_stage_status_serializes_as_snake_case_strings() { + assert_eq!( + serde_json::to_value(StageStatus::Success).unwrap(), + json!("success") + ); + assert_eq!( + serde_json::to_value(StageStatus::Fail).unwrap(), + json!("fail") + ); + assert_eq!( + serde_json::to_value(StageStatus::Skipped).unwrap(), + json!("skipped") + ); + assert_eq!( + serde_json::to_value(StageStatus::PartialSuccess).unwrap(), + json!("partial_success") + ); + assert_eq!( + serde_json::to_value(StageStatus::Retry).unwrap(), + json!("retry") + ); +} + +#[test] +fn internal_stage_status_deserializes_each_variant() { + assert_eq!( + serde_json::from_value::(json!("success")).unwrap(), + StageStatus::Success + ); + assert_eq!( + serde_json::from_value::(json!("fail")).unwrap(), + StageStatus::Fail + ); + assert_eq!( + serde_json::from_value::(json!("skipped")).unwrap(), + StageStatus::Skipped + ); + assert_eq!( + serde_json::from_value::(json!("partial_success")).unwrap(), + StageStatus::PartialSuccess + ); + assert_eq!( + serde_json::from_value::(json!("retry")).unwrap(), + StageStatus::Retry + ); +} + +fn assert_same_type() { + assert_eq!( + TypeId::of::(), + TypeId::of::(), + "{} should be the same type as {}", + type_name::(), + type_name::() + ); +} diff --git a/lib/crates/fabro-api/tests/node_status_record_round_trip.rs b/lib/crates/fabro-api/tests/node_status_record_round_trip.rs new file mode 100644 index 000000000..1d6131577 --- /dev/null +++ b/lib/crates/fabro-api/tests/node_status_record_round_trip.rs @@ -0,0 +1,33 @@ +use std::any::{TypeId, type_name}; + +use fabro_api::types::NodeStatusRecord as ApiNodeStatusRecord; +use fabro_types::NodeStatusRecord; +use serde_json::json; + +#[test] +fn node_status_record_reuses_canonical_type() { + assert_same_type::(); +} + +#[test] +fn node_status_record_round_trips_representative_json() { + let value = json!({ + "status": "partial_success", + "notes": "continued with warnings", + "failure_reason": null, + "timestamp": "2026-04-29T12:34:56Z" + }); + + let record: NodeStatusRecord = serde_json::from_value(value.clone()).unwrap(); + assert_eq!(serde_json::to_value(record).unwrap(), value); +} + +fn assert_same_type() { + assert_eq!( + TypeId::of::(), + TypeId::of::(), + "{} should be the same type as {}", + type_name::(), + type_name::() + ); +} diff --git a/lib/crates/fabro-api/tests/question_type_round_trip.rs b/lib/crates/fabro-api/tests/question_type_round_trip.rs new file mode 100644 index 000000000..569349202 --- /dev/null +++ b/lib/crates/fabro-api/tests/question_type_round_trip.rs @@ -0,0 +1,68 @@ +use std::any::{TypeId, type_name}; + +use fabro_api::types::QuestionType as ApiQuestionType; +use fabro_types::QuestionType; +use serde_json::json; + +#[test] +fn question_type_reuses_canonical_type() { + assert_same_type::(); +} + +#[test] +fn question_type_serializes_as_snake_case_strings() { + assert_eq!( + serde_json::to_value(QuestionType::YesNo).unwrap(), + json!("yes_no") + ); + assert_eq!( + serde_json::to_value(QuestionType::MultipleChoice).unwrap(), + json!("multiple_choice") + ); + assert_eq!( + serde_json::to_value(QuestionType::MultiSelect).unwrap(), + json!("multi_select") + ); + assert_eq!( + serde_json::to_value(QuestionType::Freeform).unwrap(), + json!("freeform") + ); + assert_eq!( + serde_json::to_value(QuestionType::Confirmation).unwrap(), + json!("confirmation") + ); +} + +#[test] +fn question_type_deserializes_each_variant() { + assert_eq!( + serde_json::from_value::(json!("yes_no")).unwrap(), + QuestionType::YesNo + ); + assert_eq!( + serde_json::from_value::(json!("multiple_choice")).unwrap(), + QuestionType::MultipleChoice + ); + assert_eq!( + serde_json::from_value::(json!("multi_select")).unwrap(), + QuestionType::MultiSelect + ); + assert_eq!( + serde_json::from_value::(json!("freeform")).unwrap(), + QuestionType::Freeform + ); + assert_eq!( + serde_json::from_value::(json!("confirmation")).unwrap(), + QuestionType::Confirmation + ); +} + +fn assert_same_type() { + assert_eq!( + TypeId::of::(), + TypeId::of::(), + "{} should be the same type as {}", + type_name::(), + type_name::() + ); +} diff --git a/lib/crates/fabro-cli/src/commands/run/attach.rs b/lib/crates/fabro-cli/src/commands/run/attach.rs index c0d302b77..ecca558cc 100644 --- a/lib/crates/fabro-cli/src/commands/run/attach.rs +++ b/lib/crates/fabro-cli/src/commands/run/attach.rs @@ -17,10 +17,10 @@ use std::time::Duration; use anyhow::Result; use fabro_api::types; -use fabro_interview::{AnswerValue, ConsoleInterviewer, Question, QuestionOption, QuestionType}; +use fabro_interview::{AnswerValue, ConsoleInterviewer, Question}; use fabro_store::EventEnvelope; use fabro_types::settings::run::ApprovalMode; -use fabro_types::{EventBody, RunId}; +use fabro_types::{EventBody, InterviewOption, RunId}; use fabro_util::json::normalize_json_value; use fabro_util::printer::Printer; use fabro_util::terminal::Styles; @@ -287,19 +287,12 @@ async fn handle_detach_signal( } fn api_question_to_question(question: &types::ApiQuestion) -> Question { - let question_type = match question.question_type { - types::QuestionType::YesNo => QuestionType::YesNo, - types::QuestionType::MultipleChoice => QuestionType::MultipleChoice, - types::QuestionType::MultiSelect => QuestionType::MultiSelect, - types::QuestionType::Freeform => QuestionType::Freeform, - types::QuestionType::Confirmation => QuestionType::Confirmation, - }; - let mut converted = Question::new(question.text.clone(), question_type); + let mut converted = Question::new(question.text.clone(), question.question_type); converted.id.clone_from(&question.id); converted.options = question .options .iter() - .map(|option| QuestionOption { + .map(|option| InterviewOption { key: option.key.clone(), label: option.label.clone(), }) diff --git a/lib/crates/fabro-cli/src/commands/run/runner.rs b/lib/crates/fabro-cli/src/commands/run/runner.rs index 24c423f12..e8be3ccba 100644 --- a/lib/crates/fabro-cli/src/commands/run/runner.rs +++ b/lib/crates/fabro-cli/src/commands/run/runner.rs @@ -594,13 +594,13 @@ mod tests { use chrono::Utc; use fabro_auth::{AuthCredential, AuthDetails}; use fabro_config::Storage; - use fabro_interview::{AnswerValue, ControlInterviewer, Interviewer, Question, QuestionType}; + use fabro_interview::{AnswerValue, ControlInterviewer, Interviewer, Question}; use fabro_model::Provider; use fabro_types::run_event::{ InterviewCompletedProps, InterviewStartedProps, RunCompletedProps, RunControlEffectProps, RunFailedProps, RunStatusTransitionProps, }; - use fabro_types::{ActorRef, EventBody, FailureReason, SuccessReason, fixtures}; + use fabro_types::{ActorRef, EventBody, FailureReason, QuestionType, SuccessReason, fixtures}; use fabro_vault::{SecretType, Vault}; use fabro_workflow::event::RunEventSink; diff --git a/lib/crates/fabro-interview/Cargo.toml b/lib/crates/fabro-interview/Cargo.toml index f583cfc3d..ed18ab877 100644 --- a/lib/crates/fabro-interview/Cargo.toml +++ b/lib/crates/fabro-interview/Cargo.toml @@ -20,6 +20,7 @@ tokio.workspace = true tracing.workspace = true dialoguer.workspace = true fabro-util = { path = "../fabro-util" } +fabro-types = { path = "../fabro-types" } [dev-dependencies] tokio = { workspace = true, features = ["test-util", "macros"] } diff --git a/lib/crates/fabro-interview/src/auto_approve.rs b/lib/crates/fabro-interview/src/auto_approve.rs index 8efb620ff..8c8584e70 100644 --- a/lib/crates/fabro-interview/src/auto_approve.rs +++ b/lib/crates/fabro-interview/src/auto_approve.rs @@ -1,6 +1,7 @@ use async_trait::async_trait; +use fabro_types::QuestionType; -use crate::{Answer, AnswerValue, Interviewer, Question, QuestionType}; +use crate::{Answer, AnswerValue, Interviewer, Question}; /// Always approves: YES for yes/no, first option for multiple choice, /// "auto-approved" for freeform. @@ -28,8 +29,9 @@ impl Interviewer for AutoApproveInterviewer { #[cfg(test)] mod tests { + use fabro_types::InterviewOption; + use super::*; - use crate::QuestionOption; #[tokio::test] async fn yes_no_returns_yes() { @@ -52,11 +54,11 @@ mod tests { let interviewer = AutoApproveInterviewer; let mut q = Question::new("Choose:", QuestionType::MultipleChoice); q.options = vec![ - QuestionOption { + InterviewOption { key: "A".to_string(), label: "Alpha".to_string(), }, - QuestionOption { + InterviewOption { key: "B".to_string(), label: "Beta".to_string(), }, @@ -65,7 +67,7 @@ mod tests { assert_eq!(answer.value, AnswerValue::Selected("A".to_string())); assert_eq!( answer.selected_option, - Some(QuestionOption { + Some(InterviewOption { key: "A".to_string(), label: "Alpha".to_string(), }) diff --git a/lib/crates/fabro-interview/src/callback.rs b/lib/crates/fabro-interview/src/callback.rs index fd0424437..990911318 100644 --- a/lib/crates/fabro-interview/src/callback.rs +++ b/lib/crates/fabro-interview/src/callback.rs @@ -24,8 +24,10 @@ impl Interviewer for CallbackInterviewer { #[cfg(test)] mod tests { + use fabro_types::QuestionType; + use super::*; - use crate::{AnswerValue, QuestionType}; + use crate::AnswerValue; #[tokio::test] async fn calls_callback_with_question() { diff --git a/lib/crates/fabro-interview/src/console.rs b/lib/crates/fabro-interview/src/console.rs index 98467899c..e30b72d88 100644 --- a/lib/crates/fabro-interview/src/console.rs +++ b/lib/crates/fabro-interview/src/console.rs @@ -3,11 +3,12 @@ use std::io::IsTerminal; use async_trait::async_trait; use dialoguer::console::Term; use dialoguer::theme::ColorfulTheme; +use fabro_types::{InterviewOption, QuestionType}; use fabro_util::terminal::Styles; use tokio::io::{self, AsyncBufReadExt, BufReader}; use tokio::task; -use crate::{Answer, AnswerValue, Interviewer, Question, QuestionOption, QuestionType}; +use crate::{Answer, AnswerValue, Interviewer, Question}; enum PromptRead { Line(String), @@ -28,7 +29,7 @@ impl ConsoleInterviewer { } } -fn find_matching_option(response: &str, options: &[QuestionOption]) -> Option { +fn find_matching_option(response: &str, options: &[InterviewOption]) -> Option { let trimmed = response.trim(); // Try matching by key (case-insensitive) for opt in options { @@ -291,11 +292,11 @@ mod tests { #[test] fn find_matching_option_by_key() { let options = vec![ - crate::QuestionOption { + InterviewOption { key: "A".to_string(), label: "Approve".to_string(), }, - crate::QuestionOption { + InterviewOption { key: "R".to_string(), label: "Reject".to_string(), }, @@ -308,7 +309,7 @@ mod tests { #[test] fn find_matching_option_by_key_case_insensitive() { - let options = vec![crate::QuestionOption { + let options = vec![InterviewOption { key: "Y".to_string(), label: "Yes".to_string(), }]; @@ -319,11 +320,11 @@ mod tests { #[test] fn find_matching_option_by_index() { let options = vec![ - crate::QuestionOption { + InterviewOption { key: "A".to_string(), label: "Alpha".to_string(), }, - crate::QuestionOption { + InterviewOption { key: "B".to_string(), label: "Beta".to_string(), }, @@ -336,7 +337,7 @@ mod tests { #[test] fn find_matching_option_no_match() { - let options = vec![crate::QuestionOption { + let options = vec![InterviewOption { key: "A".to_string(), label: "Alpha".to_string(), }]; @@ -346,7 +347,7 @@ mod tests { #[test] fn find_matching_option_index_out_of_range() { - let options = vec![crate::QuestionOption { + let options = vec![InterviewOption { key: "A".to_string(), label: "Alpha".to_string(), }]; @@ -357,7 +358,7 @@ mod tests { #[test] fn non_tty_multiple_choice_eof_returns_interrupted() { let mut question = Question::new("Approve?", QuestionType::MultipleChoice); - question.options = vec![crate::QuestionOption { + question.options = vec![InterviewOption { key: "A".to_string(), label: "Approve".to_string(), }]; diff --git a/lib/crates/fabro-interview/src/control.rs b/lib/crates/fabro-interview/src/control.rs index 602c17d2b..46d307e07 100644 --- a/lib/crates/fabro-interview/src/control.rs +++ b/lib/crates/fabro-interview/src/control.rs @@ -124,10 +124,11 @@ impl Interviewer for ControlInterviewer { mod tests { use std::sync::Arc; + use fabro_types::QuestionType; use tokio::task; use super::*; - use crate::{AnswerValue, QuestionType}; + use crate::AnswerValue; #[tokio::test] async fn submit_before_ask_buffers_answer() { diff --git a/lib/crates/fabro-interview/src/lib.rs b/lib/crates/fabro-interview/src/lib.rs index c311a5218..b61fbaabc 100644 --- a/lib/crates/fabro-interview/src/lib.rs +++ b/lib/crates/fabro-interview/src/lib.rs @@ -10,38 +10,10 @@ mod replay; use std::collections::HashMap; use async_trait::async_trait; +use fabro_types::{InterviewOption, QuestionType}; use serde::{Deserialize, Serialize}; use tokio::time; -/// The type of question being asked. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub enum QuestionType { - YesNo, - MultipleChoice, - MultiSelect, - Freeform, - Confirmation, -} - -impl std::fmt::Display for QuestionType { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::YesNo => write!(f, "yes_no"), - Self::MultipleChoice => write!(f, "multiple_choice"), - Self::MultiSelect => write!(f, "multi_select"), - Self::Freeform => write!(f, "freeform"), - Self::Confirmation => write!(f, "confirmation"), - } - } -} - -/// An option presented to the user for multiple-choice questions. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct QuestionOption { - pub key: String, - pub label: String, -} - /// A question presented to the user. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Question { @@ -49,7 +21,7 @@ pub struct Question { pub id: String, pub text: String, pub question_type: QuestionType, - pub options: Vec, + pub options: Vec, pub allow_freeform: bool, pub default: Option, pub timeout_seconds: Option, @@ -94,7 +66,7 @@ pub enum AnswerValue { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Answer { pub value: AnswerValue, - pub selected_option: Option, + pub selected_option: Option, pub text: Option, } @@ -153,7 +125,7 @@ impl Answer { } } - pub fn selected(key: impl Into, option: QuestionOption) -> Self { + pub fn selected(key: impl Into, option: InterviewOption) -> Self { let key = key.into(); Self { value: AnswerValue::Selected(key), @@ -300,7 +272,7 @@ mod tests { #[test] fn answer_selected() { - let opt = QuestionOption { + let opt = InterviewOption { key: "A".to_string(), label: "Approve".to_string(), }; @@ -318,11 +290,11 @@ mod tests { #[test] fn question_option_eq() { - let a = QuestionOption { + let a = InterviewOption { key: "Y".to_string(), label: "Yes".to_string(), }; - let b = QuestionOption { + let b = InterviewOption { key: "Y".to_string(), label: "Yes".to_string(), }; diff --git a/lib/crates/fabro-interview/src/queue.rs b/lib/crates/fabro-interview/src/queue.rs index ba79f0b33..431451cad 100644 --- a/lib/crates/fabro-interview/src/queue.rs +++ b/lib/crates/fabro-interview/src/queue.rs @@ -29,8 +29,10 @@ impl Interviewer for QueueInterviewer { #[cfg(test)] mod tests { + use fabro_types::QuestionType; + use super::*; - use crate::{AnswerValue, QuestionType}; + use crate::AnswerValue; #[tokio::test] async fn returns_queued_answers_in_order() { diff --git a/lib/crates/fabro-interview/src/recording.rs b/lib/crates/fabro-interview/src/recording.rs index ace98f179..87ebfc8e2 100644 --- a/lib/crates/fabro-interview/src/recording.rs +++ b/lib/crates/fabro-interview/src/recording.rs @@ -99,8 +99,10 @@ impl Interviewer for RecordingInterviewer { #[cfg(test)] mod tests { + use fabro_types::QuestionType; + use super::*; - use crate::{AnswerValue, AutoApproveInterviewer, QuestionType}; + use crate::{AnswerValue, AutoApproveInterviewer}; #[tokio::test] async fn records_question_answer_pairs() { @@ -149,14 +151,14 @@ mod tests { let json = recorder.to_json().unwrap(); assert!(json.contains("approve?")); - assert!(json.contains("YesNo")); + assert!(json.contains("yes_no")); } #[test] fn from_json_deserializes_recordings() { let json = r#"[ [ - {"text":"approve?","question_type":"YesNo","options":[],"allow_freeform":false,"default":null,"timeout_seconds":null,"stage":"","metadata":{}}, + {"text":"approve?","question_type":"yes_no","options":[],"allow_freeform":false,"default":null,"timeout_seconds":null,"stage":"","metadata":{}}, {"value":"Yes","selected_option":null,"text":null} ] ]"#; diff --git a/lib/crates/fabro-interview/src/replay.rs b/lib/crates/fabro-interview/src/replay.rs index c4f9e335c..50a70f917 100644 --- a/lib/crates/fabro-interview/src/replay.rs +++ b/lib/crates/fabro-interview/src/replay.rs @@ -36,8 +36,10 @@ impl Interviewer for ReplayInterviewer { #[cfg(test)] mod tests { + use fabro_types::QuestionType; + use super::*; - use crate::{AnswerValue, QuestionType}; + use crate::AnswerValue; #[tokio::test] async fn replays_recorded_answers() { diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 421fbeda4..fbaae76fd 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -32,21 +32,19 @@ pub use fabro_api::types::{ ForkRequest, ForkResponse, MergeRunPullRequestRequest, MergeRunPullRequestResponse, ModelReference, PaginatedEventList, PaginatedRunList, PaginationMeta, PreflightResponse, PreviewUrlRequest, PreviewUrlResponse, PruneRunEntry, PruneRunsRequest, PruneRunsResponse, - QuestionType as ApiQuestionType, RenderWorkflowGraphDirection, RenderWorkflowGraphRequest, - RewindRequest, RewindResponse, RunArtifactEntry, RunArtifactListResponse, RunBilling, - RunBillingStage, RunBillingTotals, RunError, RunManifest, RunStage, RunStatusResponse, - SandboxFileEntry, SandboxFileListResponse, SshAccessRequest, SshAccessResponse, - StageStatus as ApiStageStatus, StartRunRequest, SubmitAnswerRequest, SystemFeatures, - SystemInfoResponse, SystemRunCounts, TimelineEntryResponse, WriteBlobResponse, + RenderWorkflowGraphDirection, RenderWorkflowGraphRequest, RewindRequest, RewindResponse, + RunArtifactEntry, RunArtifactListResponse, RunBilling, RunBillingStage, RunBillingTotals, + RunError, RunManifest, RunStage, RunStatusResponse, SandboxFileEntry, SandboxFileListResponse, + SshAccessRequest, SshAccessResponse, StageStatus as ApiStageStatus, StartRunRequest, + SubmitAnswerRequest, SystemFeatures, SystemInfoResponse, SystemRunCounts, + TimelineEntryResponse, WriteBlobResponse, }; use fabro_auth::{ CredentialSource, VaultCredentialSource, auth_issue_message, parse_credential_secret, }; use fabro_config::daemon::ServerDaemon; use fabro_config::{RunLayer, RunSettingsBuilder, ServerSettingsBuilder, Storage, envfile}; -use fabro_interview::{ - Answer, ControlInterviewer, Interviewer, Question, QuestionType, WorkerControlEnvelope, -}; +use fabro_interview::{Answer, ControlInterviewer, Interviewer, Question, WorkerControlEnvelope}; use fabro_llm::client::Client as LlmClient; use fabro_llm::generate::{GenerateParams, generate_object}; use fabro_llm::model_test::{ModelTestMode, run_model_test}; @@ -76,9 +74,9 @@ use fabro_types::settings::server::{ }; use fabro_types::settings::{InterpString, RunNamespace}; use fabro_types::{ - ActorRef, EventBody, InterviewQuestionRecord, InterviewQuestionType, PullRequestRecord, - RunBlobId, RunClientProvenance, RunControlAction, RunEvent, RunId, RunProvenance, - RunServerProvenance, RunSubjectProvenance, ServerSettings, + ActorRef, EventBody, InterviewQuestionRecord, PullRequestRecord, QuestionType, RunBlobId, + RunClientProvenance, RunControlAction, RunEvent, RunId, RunProvenance, RunServerProvenance, + RunSubjectProvenance, ServerSettings, }; use fabro_util::error::{collect_causes, render_with_causes}; use fabro_util::version::FABRO_VERSION; @@ -3988,39 +3986,12 @@ fn resolved_log_destination(state: &AppState) -> anyhow::Result ) } -fn api_question_type(question_type: InterviewQuestionType) -> ApiQuestionType { - match question_type { - InterviewQuestionType::YesNo => ApiQuestionType::YesNo, - InterviewQuestionType::MultipleChoice => ApiQuestionType::MultipleChoice, - InterviewQuestionType::MultiSelect => ApiQuestionType::MultiSelect, - InterviewQuestionType::Freeform => ApiQuestionType::Freeform, - InterviewQuestionType::Confirmation => ApiQuestionType::Confirmation, - } -} - -fn runtime_question_type(question_type: InterviewQuestionType) -> QuestionType { - match question_type { - InterviewQuestionType::YesNo => QuestionType::YesNo, - InterviewQuestionType::MultipleChoice => QuestionType::MultipleChoice, - InterviewQuestionType::MultiSelect => QuestionType::MultiSelect, - InterviewQuestionType::Freeform => QuestionType::Freeform, - InterviewQuestionType::Confirmation => QuestionType::Confirmation, - } -} - fn runtime_question_from_interview_record(question: &InterviewQuestionRecord) -> Question { Question { id: question.id.clone(), text: question.text.clone(), - question_type: runtime_question_type(question.question_type), - options: question - .options - .iter() - .map(|option| fabro_interview::QuestionOption { - key: option.key.clone(), - label: option.label.clone(), - }) - .collect(), + question_type: question.question_type, + options: question.options.clone(), allow_freeform: question.allow_freeform, default: None, timeout_seconds: question.timeout_seconds, @@ -4035,7 +4006,7 @@ fn api_question_from_interview_record(question: &InterviewQuestionRecord) -> Api id: question.id.clone(), text: question.text.clone(), stage: question.stage.clone(), - question_type: api_question_type(question.question_type), + question_type: question.question_type, options: question .options .iter() @@ -4107,7 +4078,7 @@ fn validate_answer_for_question( ) -> Result<(), Response> { match (&question.question_type, &answer.value) { ( - InterviewQuestionType::YesNo | InterviewQuestionType::Confirmation, + QuestionType::YesNo | QuestionType::Confirmation, fabro_interview::AnswerValue::Yes | fabro_interview::AnswerValue::No, ) | ( @@ -4116,14 +4087,14 @@ fn validate_answer_for_question( | fabro_interview::AnswerValue::Skipped | fabro_interview::AnswerValue::Timeout, ) => Ok(()), - (InterviewQuestionType::MultipleChoice, fabro_interview::AnswerValue::Selected(key)) => { + (QuestionType::MultipleChoice, fabro_interview::AnswerValue::Selected(key)) => { if question.options.iter().any(|option| option.key == *key) { Ok(()) } else { Err(ApiError::bad_request("Invalid option key.").into_response()) } } - (InterviewQuestionType::MultiSelect, fabro_interview::AnswerValue::MultiSelected(keys)) => { + (QuestionType::MultiSelect, fabro_interview::AnswerValue::MultiSelected(keys)) => { if keys .iter() .all(|key| question.options.iter().any(|option| option.key == *key)) @@ -4133,7 +4104,7 @@ fn validate_answer_for_question( Err(ApiError::bad_request("Invalid option key.").into_response()) } } - (InterviewQuestionType::Freeform, fabro_interview::AnswerValue::Text(text)) + (QuestionType::Freeform, fabro_interview::AnswerValue::Text(text)) if !text.trim().is_empty() => { Ok(()) @@ -4216,10 +4187,7 @@ fn answer_from_request( .find(|option| option.key == key) .cloned(); match option { - Some(option) => Ok(Answer::selected(key, fabro_interview::QuestionOption { - key: option.key, - label: option.label, - })), + Some(option) => Ok(Answer::selected(key, option)), None => Err(ApiError::bad_request("Invalid option key.").into_response()), } } else if !req.selected_option_keys.is_empty() { @@ -8119,13 +8087,13 @@ mod tests { use fabro_auth::{AuthCredential, AuthDetails}; use fabro_config::ServerSettingsBuilder; use fabro_config::bind::Bind; - use fabro_interview::{AnswerValue, ControlInterviewer, Interviewer, Question, QuestionType}; + use fabro_interview::{AnswerValue, ControlInterviewer, Interviewer, Question}; use fabro_llm::types::{Message as LlmMessage, Request as LlmRequest}; use fabro_model::Provider; use fabro_types::settings::ServerAuthMethod; use fabro_types::{ - AttrValue, Graph, InterviewQuestionRecord, InterviewQuestionType, RunAuthMethod, RunBlobId, - RunId, RunSpec, fixtures, + AttrValue, Graph, InterviewQuestionRecord, QuestionType, RunAuthMethod, RunBlobId, RunId, + RunSpec, fixtures, }; use httpmock::Method::POST; use httpmock::MockServer; @@ -10488,7 +10456,7 @@ slug = "fabro" id: "q-1".to_string(), text: "Approve deploy?".to_string(), stage: "gate".to_string(), - question_type: InterviewQuestionType::MultipleChoice, + question_type: QuestionType::MultipleChoice, options: vec![fabro_types::run_event::InterviewOption { key: "approve".to_string(), label: "Approve".to_string(), diff --git a/lib/crates/fabro-slack/Cargo.toml b/lib/crates/fabro-slack/Cargo.toml index 336368656..b1a535d97 100644 --- a/lib/crates/fabro-slack/Cargo.toml +++ b/lib/crates/fabro-slack/Cargo.toml @@ -14,6 +14,7 @@ workspace = true [dependencies] fabro-interview = { path = "../fabro-interview" } +fabro-types = { path = "../fabro-types" } fabro-workflow = { path = "../fabro-workflow" } fabro-http.workspace = true fabro-static.workspace = true diff --git a/lib/crates/fabro-slack/src/blocks.rs b/lib/crates/fabro-slack/src/blocks.rs index 7211b2322..7ebc92be9 100644 --- a/lib/crates/fabro-slack/src/blocks.rs +++ b/lib/crates/fabro-slack/src/blocks.rs @@ -1,4 +1,5 @@ -use fabro_interview::{Question, QuestionType}; +use fabro_interview::Question; +use fabro_types::QuestionType; use serde_json::{Value, json}; use crate::payload::{SlackActionPayload, encode_action_value}; @@ -120,7 +121,7 @@ pub fn question_to_blocks(run_id: &str, question_id: &str, question: &Question) #[cfg(test)] mod tests { - use fabro_interview::QuestionOption; + use fabro_types::InterviewOption; use super::*; @@ -164,15 +165,15 @@ mod tests { fn multiple_choice_produces_button_per_option() { let mut q = Question::new("Pick a language:", QuestionType::MultipleChoice); q.options = vec![ - QuestionOption { + InterviewOption { key: "rs".to_string(), label: "Rust".to_string(), }, - QuestionOption { + InterviewOption { key: "ts".to_string(), label: "TypeScript".to_string(), }, - QuestionOption { + InterviewOption { key: "py".to_string(), label: "Python".to_string(), }, @@ -250,11 +251,11 @@ mod tests { fn multi_select_produces_checkboxes_and_submit_button() { let mut q = Question::new("Select features:", QuestionType::MultiSelect); q.options = vec![ - QuestionOption { + InterviewOption { key: "a".to_string(), label: "Auth".to_string(), }, - QuestionOption { + InterviewOption { key: "b".to_string(), label: "Billing".to_string(), }, diff --git a/lib/crates/fabro-store/src/run_state.rs b/lib/crates/fabro-store/src/run_state.rs index f32f72685..77100a86e 100644 --- a/lib/crates/fabro-store/src/run_state.rs +++ b/lib/crates/fabro-store/src/run_state.rs @@ -566,9 +566,9 @@ mod tests { InterviewCompletedProps, InterviewOption, InterviewStartedProps, RunControlEffectProps, }; use fabro_types::{ - BlockedReason, Checkpoint, EventBody, FailureReason, InterviewQuestionType, NodeState, - RunBlobId, RunControlAction, RunEvent, RunStatus, SuccessReason, TerminalStatus, - WorkflowSettings, fixtures, + BlockedReason, Checkpoint, EventBody, FailureReason, NodeState, QuestionType, RunBlobId, + RunControlAction, RunEvent, RunStatus, SuccessReason, TerminalStatus, WorkflowSettings, + fixtures, }; use serde_json::json; @@ -783,10 +783,7 @@ mod tests { .expect("pending interview should be present"); assert_eq!(pending.question.id, "q-1"); assert_eq!(pending.question.stage, "gate"); - assert_eq!( - pending.question.question_type, - InterviewQuestionType::MultipleChoice - ); + assert_eq!(pending.question.question_type, QuestionType::MultipleChoice); assert_eq!(pending.question.options.len(), 2); assert!(pending.question.allow_freeform); assert_eq!(pending.question.timeout_seconds, Some(30.0)); diff --git a/lib/crates/fabro-types/src/interview.rs b/lib/crates/fabro-types/src/interview.rs index 332b098c2..4ad2dc295 100644 --- a/lib/crates/fabro-types/src/interview.rs +++ b/lib/crates/fabro-types/src/interview.rs @@ -16,7 +16,7 @@ use crate::run_event::InterviewOption; )] #[serde(rename_all = "snake_case")] #[strum(serialize_all = "snake_case")] -pub enum InterviewQuestionType { +pub enum QuestionType { YesNo, MultipleChoice, MultiSelect, @@ -34,7 +34,7 @@ pub struct InterviewQuestionRecord { #[serde(default)] pub stage: String, #[serde(default)] - pub question_type: InterviewQuestionType, + pub question_type: QuestionType, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub options: Vec, #[serde(default)] @@ -52,18 +52,15 @@ mod tests { #[test] fn question_type_wire_names_roundtrip() { let cases = [ - ("yes_no", InterviewQuestionType::YesNo), - ("multiple_choice", InterviewQuestionType::MultipleChoice), - ("multi_select", InterviewQuestionType::MultiSelect), - ("freeform", InterviewQuestionType::Freeform), - ("confirmation", InterviewQuestionType::Confirmation), + ("yes_no", QuestionType::YesNo), + ("multiple_choice", QuestionType::MultipleChoice), + ("multi_select", QuestionType::MultiSelect), + ("freeform", QuestionType::Freeform), + ("confirmation", QuestionType::Confirmation), ]; for (wire, question_type) in cases { - assert_eq!( - wire.parse::().unwrap(), - question_type - ); + assert_eq!(wire.parse::().unwrap(), question_type); assert_eq!(question_type.to_string(), wire); } } diff --git a/lib/crates/fabro-types/src/lib.rs b/lib/crates/fabro-types/src/lib.rs index 6075e8e96..2010a287f 100644 --- a/lib/crates/fabro-types/src/lib.rs +++ b/lib/crates/fabro-types/src/lib.rs @@ -48,7 +48,7 @@ pub use diff::DiffStats; pub use event_envelope::EventEnvelope; pub use failure_signature::FailureSignature; pub use graph::{AttrValue, Edge, Graph, Node, is_llm_handler_type, shape_to_handler_type}; -pub use interview::{InterviewQuestionRecord, InterviewQuestionType}; +pub use interview::{InterviewQuestionRecord, QuestionType}; pub use node_status::NodeStatusRecord; pub use outcome::{FailureCategory, FailureDetail, NodeResult, Outcome, OutcomeMeta, StageStatus}; pub use pull_request::{ @@ -64,7 +64,7 @@ pub use run::{ RunProvenance, RunServerProvenance, RunSpec, RunSubjectProvenance, }; pub use run_blob_id::RunBlobId; -pub use run_event::{ActorKind, ActorRef, EventBody, RunEvent, RunNoticeLevel}; +pub use run_event::{ActorKind, ActorRef, EventBody, InterviewOption, RunEvent, RunNoticeLevel}; pub use run_id::{RunId, fixtures}; pub use run_projection::{NodeState, PendingInterviewRecord, RunProjection}; pub use run_summary::RunSummary; diff --git a/lib/crates/fabro-workflow/src/handler/human.rs b/lib/crates/fabro-workflow/src/handler/human.rs index a56252ec7..8f6d1b50b 100644 --- a/lib/crates/fabro-workflow/src/handler/human.rs +++ b/lib/crates/fabro-workflow/src/handler/human.rs @@ -5,9 +5,8 @@ use std::time::Instant; use async_trait::async_trait; use fabro_graphviz::graph::{Graph, Node}; -use fabro_interview::{Answer, AnswerValue, Interviewer, Question, QuestionOption, QuestionType}; -use fabro_types::BlockedReason; -use fabro_types::run_event::InterviewOption; +use fabro_interview::{Answer, AnswerValue, Interviewer, Question}; +use fabro_types::{BlockedReason, InterviewOption, QuestionType}; use ulid::Ulid; use super::{EngineServices, Handler}; @@ -226,9 +225,9 @@ impl Handler for HumanHandler { } // 2. Build question - let options: Vec = choices + let options: Vec = choices .iter() - .map(|c| QuestionOption { + .map(|c| InterviewOption { key: c.key.clone(), label: c.label.clone(), }) @@ -718,7 +717,7 @@ mod tests { #[tokio::test] async fn wait_human_emits_blocked_then_unblocked_around_interview() { let interviewer = Arc::new(CallbackInterviewer::new(|_| { - Answer::selected("A", QuestionOption { + Answer::selected("A", InterviewOption { key: "A".to_string(), label: "Approve".to_string(), })