- {
+ question
+ .review_target
+ .as_ref()
+ .map(|target| format!("Review link: {}", target.url()))
+}
+
/// Ask a multiple-choice question using dialoguer's `Select` widget on a TTY.
fn ask_select_interactive(question: &Question) -> Answer {
let items: Vec = question
@@ -237,6 +249,9 @@ impl Interviewer for ConsoleInterviewer {
let rendered = self.styles.render_markdown(context_text);
eprint!("{rendered}");
}
+ if let Some(line) = review_target_line(&question) {
+ eprintln!("{line}");
+ }
let q = question;
let answer = task::spawn_blocking(move || match q.question_type {
QuestionType::MultipleChoice => ask_select_interactive(&q),
@@ -251,6 +266,9 @@ impl Interviewer for ConsoleInterviewer {
// Non-TTY fallback: line-based stdin reading
let s = self.styles;
+ if let Some(line) = review_target_line(&question) {
+ eprintln!("{line}");
+ }
eprintln!("{} {}", s.bold_cyan.apply_to("?"), question.text);
let answer = match question.question_type {
@@ -314,6 +332,33 @@ mod tests {
assert_eq!(answer.value, AnswerValue::Selected("A".to_string()));
}
+ #[test]
+ fn review_target_line_shows_only_the_url() {
+ let target = fabro_types::ReviewTarget::new(
+ "Quarry review exercise",
+ "https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef",
+ fabro_types::ReviewTargetKind::Document,
+ )
+ .unwrap();
+ let mut question = Question::new(target.question_text(), QuestionType::MultipleChoice);
+ question.review_target = Some(target);
+
+ assert_eq!(
+ review_target_line(&question).as_deref(),
+ Some(
+ "Review link: \
+ https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef"
+ )
+ );
+ }
+
+ #[test]
+ fn review_target_line_is_absent_without_a_target() {
+ let question = Question::new("Approve?", QuestionType::YesNo);
+
+ assert_eq!(review_target_line(&question), None);
+ }
+
#[test]
fn find_matching_option_by_key_case_insensitive() {
let options = vec![InterviewOption {
diff --git a/lib/components/fabro-interview/src/lib.rs b/lib/components/fabro-interview/src/lib.rs
index 6d8c414a1..4493f98ca 100644
--- a/lib/components/fabro-interview/src/lib.rs
+++ b/lib/components/fabro-interview/src/lib.rs
@@ -10,7 +10,7 @@ mod replay;
use std::collections::HashMap;
use async_trait::async_trait;
-use fabro_types::{InterviewOption, Principal, QuestionType, SystemActorKind};
+use fabro_types::{InterviewOption, Principal, QuestionType, ReviewTarget, SystemActorKind};
use serde::{Deserialize, Serialize};
use tokio::time;
@@ -29,6 +29,8 @@ pub struct Question {
pub metadata: HashMap,
#[serde(default)]
pub context_display: Option,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub review_target: Option,
}
impl Question {
@@ -44,6 +46,7 @@ impl Question {
stage: String::new(),
metadata: HashMap::new(),
context_display: None,
+ review_target: None,
}
}
}
@@ -219,7 +222,7 @@ pub trait Interviewer: Send + Sync {
// Re-export all implementors at the crate root
pub use auto_approve::AutoApproveInterviewer;
pub use callback::CallbackInterviewer;
-pub use console::ConsoleInterviewer;
+pub use console::{ConsoleInterviewer, review_target_line};
pub use control::{ControlInterviewer, SubmitError};
pub use control_protocol::{
WORKER_CONTROL_INVALID_CURSOR_REASON, WORKER_CONTROL_PONG_TIMEOUT_REASON,
@@ -260,6 +263,7 @@ mod tests {
assert!(q.timeout_seconds.is_none());
assert!(q.stage.is_empty());
assert!(q.metadata.is_empty());
+ assert!(q.review_target.is_none());
}
#[test]
diff --git a/lib/components/fabro-slack/src/blocks.rs b/lib/components/fabro-slack/src/blocks.rs
index da2626167..f9484b07f 100644
--- a/lib/components/fabro-slack/src/blocks.rs
+++ b/lib/components/fabro-slack/src/blocks.rs
@@ -99,7 +99,13 @@ fn truncate_to_limit(text: &str, limit: usize, suffix: &str) -> String {
/// final text is bounded by Slack's section-text limit so even pathological
/// inputs cannot produce `invalid_blocks`.
fn header_section(question: &Question, run_web_url: Option<&str>) -> Value {
- let mut text = format!("*{}*", escape_slack_controls(&question.text));
+ let mut text = question.review_target.as_ref().map_or_else(
+ || format!("*{}*", escape_slack_controls(&question.text)),
+ |target| {
+ let link = slack_link(target.url(), target.label());
+ format!("*{}*", target.question_text_with_link(&link))
+ },
+ );
if !question.stage.is_empty() {
let _ = write!(
text,
@@ -420,9 +426,18 @@ fn lifecycle_pull_request_text(pull_request: &RunLifecyclePullRequest<'_>) -> St
text
}
+/// Render `label` as a link to `url` in Slack's `` syntax, falling
+/// back to plain text when the URL cannot be embedded safely.
+///
+/// `escape_slack_controls` covers Slack's documented escapes (`&`, `<`, `>`)
+/// but Slack has no escape for `|`, which separates the URL from the label.
+/// A `|` in the label would split the markup and can make Slack reject the
+/// whole block, so it is replaced inside link labels only. Labels can be
+/// model-authored (a review target label, for example), so this is reachable.
fn slack_link(url: &str, label: &str) -> String {
if is_safe_slack_link_url(url) {
- format!("<{}|{}>", url, escape_slack_controls(label))
+ let label = escape_slack_controls(label).replace('|', "/");
+ format!("<{url}|{label}>")
} else {
escape_slack_controls(label)
}
@@ -634,6 +649,58 @@ mod tests {
assert!(header.contains(""));
}
+ #[test]
+ fn header_renders_review_target_as_the_question_link() {
+ let mut q = Question::new(
+ "Review the Quarry review exercise document, then choose the next action.",
+ QuestionType::MultipleChoice,
+ );
+ q.review_target = Some(
+ fabro_types::ReviewTarget::new(
+ "Quarry review exercise",
+ "https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef",
+ fabro_types::ReviewTargetKind::Document,
+ )
+ .unwrap(),
+ );
+
+ let blocks = question_to_blocks("run-1", "q-1", &q, None);
+ let header = serde_json::to_value(&blocks).unwrap()[0]["text"]["text"]
+ .as_str()
+ .unwrap()
+ .to_string();
+
+ assert_eq!(
+ header,
+ "*Review the \
+ document, then choose the next action.*"
+ );
+ }
+
+ #[test]
+ fn review_target_label_pipe_cannot_split_the_slack_link() {
+ let mut q = Question::new("Review", QuestionType::MultipleChoice);
+ q.review_target = Some(
+ fabro_types::ReviewTarget::new(
+ "Draft | v2",
+ "https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef",
+ fabro_types::ReviewTargetKind::Document,
+ )
+ .unwrap(),
+ );
+
+ let blocks = question_to_blocks("run-1", "q-1", &q, None);
+ let header = serde_json::to_value(&blocks).unwrap()[0]["text"]["text"]
+ .as_str()
+ .unwrap()
+ .to_string();
+
+ // Exactly one `|`: the one separating the URL from the label.
+ assert_eq!(header.matches('|').count(), 1);
+ assert!(header.contains("|Draft / v2>"));
+ }
+
#[test]
fn header_omits_link_when_url_missing() {
let q = Question::new("Approve Plan", QuestionType::YesNo);
diff --git a/lib/components/fabro-store/src/run_state.rs b/lib/components/fabro-store/src/run_state.rs
index 644048c90..99e8d121e 100644
--- a/lib/components/fabro-store/src/run_state.rs
+++ b/lib/components/fabro-store/src/run_state.rs
@@ -335,6 +335,7 @@ impl RunProjectionReducer for RunProjection {
allow_freeform: props.allow_freeform,
timeout_seconds: props.timeout_seconds,
context_display: props.context_display.clone(),
+ review_target: props.review_target.clone(),
},
started_at: ts,
});
@@ -3531,6 +3532,14 @@ mod tests {
allow_freeform: true,
timeout_seconds: Some(30.0),
context_display: Some("Latest draft".to_string()),
+ review_target: Some(
+ fabro_types::ReviewTarget::new(
+ "Quarry review exercise",
+ "https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef",
+ fabro_types::ReviewTargetKind::Document,
+ )
+ .unwrap(),
+ ),
}),
Some("gate"),
))
@@ -3558,6 +3567,14 @@ mod tests {
pending.question.context_display.as_deref(),
Some("Latest draft")
);
+ assert_eq!(
+ pending
+ .question
+ .review_target
+ .as_ref()
+ .map(fabro_types::ReviewTarget::url),
+ Some("https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef")
+ );
state
.apply_event(&test_event(
diff --git a/lib/components/fabro-store/tests/serializable_projection.rs b/lib/components/fabro-store/tests/serializable_projection.rs
index 66600c154..140542a70 100644
--- a/lib/components/fabro-store/tests/serializable_projection.rs
+++ b/lib/components/fabro-store/tests/serializable_projection.rs
@@ -235,6 +235,7 @@ fn projection_query_methods_expose_common_state() {
allow_freeform: true,
timeout_seconds: None,
context_display: None,
+ review_target: None,
},
started_at: Utc::now(),
})]);
diff --git a/lib/components/fabro-validate/src/rules/inert_attribute.rs b/lib/components/fabro-validate/src/rules/inert_attribute.rs
index 2ae6d0e04..ac2629422 100644
--- a/lib/components/fabro-validate/src/rules/inert_attribute.rs
+++ b/lib/components/fabro-validate/src/rules/inert_attribute.rs
@@ -22,6 +22,7 @@ const HANDLER_SPECIFIC_ATTRS: &[(&str, &[&str])] = &[
("output_retries", &["agent", "prompt"]),
("output_schema", &["agent", "prompt", "command"]),
("prompt", &["agent", "prompt", "parallel.fan_in"]),
+ ("review_target", &["human"]),
];
struct Rule;
@@ -197,9 +198,28 @@ mod tests {
"prompt".to_string(),
node_with_attr("prompt", "tab", "output_retries", "2"),
);
+ g.nodes.insert(
+ "human".to_string(),
+ node_with_attr("human", "hexagon", "review_target", "true"),
+ );
assert!(Rule.apply(&g).is_empty());
}
+ #[test]
+ fn warns_on_review_target_on_non_human_node() {
+ let mut g = minimal_graph();
+ g.nodes.insert(
+ "work".to_string(),
+ node_with_attr("work", "box", "review_target", "true"),
+ );
+
+ let diagnostics = Rule.apply(&g);
+
+ assert_eq!(diagnostics.len(), 1);
+ assert!(diagnostics[0].message.contains("'review_target'"));
+ assert!(diagnostics[0].message.contains("human"));
+ }
+
#[test]
fn accepts_prompt_on_shapeless_node_defaulting_to_agent() {
let mut g = minimal_graph();
diff --git a/lib/components/fabro-workflow/src/context.rs b/lib/components/fabro-workflow/src/context.rs
index 6e94368b2..46aefccda 100644
--- a/lib/components/fabro-workflow/src/context.rs
+++ b/lib/components/fabro-workflow/src/context.rs
@@ -12,6 +12,7 @@ pub mod keys {
pub const PREFERRED_LABEL: &str = "preferred_label";
pub const LAST_STAGE: &str = "last_stage";
pub const LAST_RESPONSE: &str = "last_response";
+ pub const REVIEW_TARGET: &str = "review_target";
// --- graph.* keys ---
pub const GRAPH_GOAL: &str = "graph.goal";
@@ -142,6 +143,7 @@ pub mod keys {
assert!(!is_engine_internal_key("outcome"));
assert!(!is_engine_internal_key("last_stage"));
assert!(!is_engine_internal_key("review.result"));
+ assert!(!is_engine_internal_key(REVIEW_TARGET));
assert!(!is_engine_internal_key("user.name"));
}
}
diff --git a/lib/components/fabro-workflow/src/event/convert.rs b/lib/components/fabro-workflow/src/event/convert.rs
index 5cef4e581..e2d236533 100644
--- a/lib/components/fabro-workflow/src/event/convert.rs
+++ b/lib/components/fabro-workflow/src/event/convert.rs
@@ -423,6 +423,7 @@ fn event_body_from_event(event: &Event) -> EventBody {
allow_freeform,
timeout_seconds,
context_display,
+ review_target,
} => EventBody::InterviewStarted(fabro_types::InterviewStartedProps {
question_id: question_id.clone(),
question: question.clone(),
@@ -432,6 +433,7 @@ fn event_body_from_event(event: &Event) -> EventBody {
allow_freeform: *allow_freeform,
timeout_seconds: *timeout_seconds,
context_display: context_display.clone(),
+ review_target: review_target.clone(),
}),
Event::InterviewCompleted {
actor: _,
diff --git a/lib/components/fabro-workflow/src/event/events.rs b/lib/components/fabro-workflow/src/event/events.rs
index 20803b1fa..9870de6ae 100644
--- a/lib/components/fabro-workflow/src/event/events.rs
+++ b/lib/components/fabro-workflow/src/event/events.rs
@@ -4,9 +4,10 @@ use ::fabro_types::{
AutomationRef, BilledTokenCounts, BlockedReason, CommandTermination, DiffSummary,
FailureReason, ForkSourceRef, GitContext, PairId, PairMessageId, PairSystemMessageKind,
PairTarget, ParallelBranchId, ParallelBranchResult, PendingReason, PermissionLevel, Principal,
- PullRequestLink, RunBlobId, RunFailure, RunId, RunNoticeLevel, RunPairEndedReason,
- RunPairFailedReason, RunProvenance, RunRunnableSource, RunTiming, SandboxProviderKind, StageId,
- StageOutcome, StageTiming, SuccessReason, run_event as fabro_types,
+ PullRequestLink, ReviewTarget, RunBlobId, RunFailure, RunId, RunNoticeLevel,
+ RunPairEndedReason, RunPairFailedReason, RunProvenance, RunRunnableSource, RunTiming,
+ SandboxProviderKind, StageId, StageOutcome, StageTiming, SuccessReason,
+ run_event as fabro_types,
};
use fabro_agent::{AgentEvent, SandboxEvent};
use fabro_model::{ReasoningEffort, Speed};
@@ -355,6 +356,8 @@ pub enum Event {
timeout_seconds: Option,
#[serde(default, skip_serializing_if = "Option::is_none")]
context_display: Option,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ review_target: Option,
},
InterviewCompleted {
#[serde(default, skip_serializing_if = "Option::is_none")]
diff --git a/lib/components/fabro-workflow/src/handler/human.rs b/lib/components/fabro-workflow/src/handler/human.rs
index 77b83454a..a961ecb02 100644
--- a/lib/components/fabro-workflow/src/handler/human.rs
+++ b/lib/components/fabro-workflow/src/handler/human.rs
@@ -6,7 +6,7 @@ use std::time::Instant;
use async_trait::async_trait;
use fabro_graphviz::graph::{Graph, Node};
use fabro_interview::{Answer, AnswerValue, Interviewer, Question, ask_with_timeout};
-use fabro_types::{InterviewOption, Principal, QuestionType, SystemActorKind};
+use fabro_types::{InterviewOption, Principal, QuestionType, ReviewTarget, SystemActorKind};
use ulid::Ulid;
use super::{EngineServices, Handler, NodeTimeoutPolicy};
@@ -121,6 +121,23 @@ fn build_human_gate_question(
question.stage.clone_from(&node.id);
question.timeout_seconds = node.timeout().map(|duration| duration.as_secs_f64());
+ if node.review_target() {
+ let value = context.get(keys::REVIEW_TARGET).ok_or_else(|| {
+ format!(
+ "Human gate \"{}\" has review_target=true but context.review_target is missing",
+ node.id
+ )
+ })?;
+ let review_target = serde_json::from_value::(value).map_err(|error| {
+ format!(
+ "Human gate \"{}\" has invalid context.review_target: {error}",
+ node.id
+ )
+ })?;
+ question.text = review_target.question_text();
+ question.review_target = Some(review_target);
+ }
+
if let Some(serde_json::Value::String(last_node)) = context.get(keys::LAST_STAGE) {
if let Some(serde_json::Value::String(response)) =
context.get(&keys::response_key(&last_node))
@@ -253,6 +270,7 @@ impl Handler for HumanHandler {
allow_freeform: question.allow_freeform,
timeout_seconds: question.timeout_seconds,
context_display: question.context_display.clone(),
+ review_target: question.review_target.clone(),
},
&stage_scope,
);
@@ -639,6 +657,15 @@ mod tests {
graph
}
+ fn enable_review_target(graph: &mut Graph) {
+ graph
+ .nodes
+ .get_mut("gate")
+ .unwrap()
+ .attrs
+ .insert("review_target".to_string(), AttrValue::Boolean(true));
+ }
+
#[test]
fn parse_accelerator_key_bracket() {
assert_eq!(parse_accelerator_key("[A] Approve"), "A");
@@ -665,6 +692,160 @@ mod tests {
assert_eq!(parse_accelerator_key(""), "");
}
+ #[tokio::test]
+ async fn review_target_gate_snapshots_validated_context_into_question_and_event() {
+ let inner = Box::new(AutoApproveInterviewer::engine());
+ let recorder = Arc::new(RecordingInterviewer::new(inner));
+ let handler = HumanHandler::new(recorder.clone());
+ let mut graph = build_graph_with_human_gate();
+ enable_review_target(&mut graph);
+ let node = graph.nodes.get("gate").unwrap();
+ let context = Context::new();
+ let target_value = serde_json::json!({
+ "label": "Quarry review exercise",
+ "url": "https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef",
+ "kind": "document",
+ });
+ context.set(keys::REVIEW_TARGET, target_value.clone());
+ let events = Arc::new(Mutex::new(Vec::new()));
+
+ let outcome = handler
+ .execute(
+ node,
+ &context,
+ &graph,
+ Path::new("/tmp/test"),
+ &make_services_with_events(Arc::clone(&events)),
+ )
+ .await
+ .unwrap();
+
+ let recordings = recorder.recordings();
+ assert_eq!(recordings.len(), 1);
+ let question = &recordings[0].0;
+ assert_eq!(
+ question.text,
+ "Review the Quarry review exercise document, then choose the next action."
+ );
+ assert_eq!(
+ question.review_target.as_ref().map(ReviewTarget::url),
+ Some("https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef")
+ );
+
+ let event_target = events
+ .lock()
+ .expect("event log lock poisoned")
+ .iter()
+ .find_map(|event| match &event.body {
+ EventBody::InterviewStarted(props) => props.review_target.clone(),
+ _ => None,
+ })
+ .expect("interview.started should carry the review target");
+ assert_eq!(event_target.label(), "Quarry review exercise");
+
+ context.apply_updates(&outcome.context_updates);
+ assert_eq!(context.get(keys::REVIEW_TARGET), Some(target_value));
+ }
+
+ #[tokio::test]
+ async fn review_target_gate_fails_before_interview_when_context_is_missing() {
+ let inner = Box::new(AutoApproveInterviewer::engine());
+ let recorder = Arc::new(RecordingInterviewer::new(inner));
+ let handler = HumanHandler::new(recorder.clone());
+ let mut graph = build_graph_with_human_gate();
+ enable_review_target(&mut graph);
+ let node = graph.nodes.get("gate").unwrap();
+
+ let outcome = handler
+ .execute(
+ node,
+ &Context::new(),
+ &graph,
+ Path::new("/tmp/test"),
+ &make_services(),
+ )
+ .await
+ .unwrap();
+
+ assert!(outcome.status.is_failure());
+ assert_eq!(
+ outcome.failure_reason(),
+ Some("Human gate \"gate\" has review_target=true but context.review_target is missing")
+ );
+ assert!(recorder.recordings().is_empty());
+ }
+
+ #[tokio::test]
+ async fn review_target_gate_rejects_unsafe_url_without_echoing_it() {
+ let inner = Box::new(AutoApproveInterviewer::engine());
+ let recorder = Arc::new(RecordingInterviewer::new(inner));
+ let handler = HumanHandler::new(recorder.clone());
+ let mut graph = build_graph_with_human_gate();
+ enable_review_target(&mut graph);
+ let node = graph.nodes.get("gate").unwrap();
+ let context = Context::new();
+ context.set(
+ keys::REVIEW_TARGET,
+ serde_json::json!({
+ "label": "Unsafe review",
+ "url": "javascript:alert(1)",
+ "kind": "document",
+ }),
+ );
+
+ let outcome = handler
+ .execute(
+ node,
+ &context,
+ &graph,
+ Path::new("/tmp/test"),
+ &make_services(),
+ )
+ .await
+ .unwrap();
+
+ let failure = outcome
+ .failure_reason()
+ .expect("invalid review target should fail");
+ assert!(failure.contains("review target URL must use http or https"));
+ assert!(!failure.contains("javascript:alert"));
+ assert!(recorder.recordings().is_empty());
+ }
+
+ #[tokio::test]
+ async fn human_gate_ignores_review_target_context_without_opt_in() {
+ let inner = Box::new(AutoApproveInterviewer::engine());
+ let recorder = Arc::new(RecordingInterviewer::new(inner));
+ let handler = HumanHandler::new(recorder.clone());
+ let graph = build_graph_with_human_gate();
+ let node = graph.nodes.get("gate").unwrap();
+ let context = Context::new();
+ context.set(
+ keys::REVIEW_TARGET,
+ serde_json::json!({
+ "label": "Unsafe review",
+ "url": "javascript:alert(1)",
+ "kind": "document",
+ }),
+ );
+
+ let outcome = handler
+ .execute(
+ node,
+ &context,
+ &graph,
+ Path::new("/tmp/test"),
+ &make_services(),
+ )
+ .await
+ .unwrap();
+
+ assert_eq!(outcome.status, crate::outcome::StageOutcome::Succeeded);
+ let recordings = recorder.recordings();
+ assert_eq!(recordings[0].0.text, "Review Changes");
+ assert!(recordings[0].0.review_target.is_none());
+ }
+
#[tokio::test]
async fn wait_human_auto_approve_selects_first() {
let interviewer = Arc::new(AutoApproveInterviewer::engine());
diff --git a/lib/components/fabro-workflow/src/interview_runtime.rs b/lib/components/fabro-workflow/src/interview_runtime.rs
index f0370ae0e..8eb58d889 100644
--- a/lib/components/fabro-workflow/src/interview_runtime.rs
+++ b/lib/components/fabro-workflow/src/interview_runtime.rs
@@ -224,6 +224,7 @@ impl AgentQuestionRuntime for WorkflowAgentQuestionRuntime {
allow_freeform: question.allow_freeform,
timeout_seconds: None,
context_display: question.context_display.clone(),
+ review_target: question.review_target.clone(),
},
&self.stage_scope,
);
diff --git a/lib/foundation/fabro-api/build.rs b/lib/foundation/fabro-api/build.rs
index 740585545..7dcf280c9 100644
--- a/lib/foundation/fabro-api/build.rs
+++ b/lib/foundation/fabro-api/build.rs
@@ -468,6 +468,8 @@ fn main() {
&[],
),
("InterviewOption", "fabro_types::InterviewOption", &[]),
+ ("ReviewTarget", "fabro_types::ReviewTarget", &[]),
+ ("ReviewTargetKind", "fabro_types::ReviewTargetKind", &[]),
(
"InterviewQuestionRecord",
"fabro_types::InterviewQuestionRecord",
diff --git a/lib/foundation/fabro-api/src/lib.rs b/lib/foundation/fabro-api/src/lib.rs
index b9a6a7185..27a4426ad 100644
--- a/lib/foundation/fabro-api/src/lib.rs
+++ b/lib/foundation/fabro-api/src/lib.rs
@@ -55,12 +55,12 @@ pub mod types {
PairTranscriptResponse, ParallelBranchResult, PendingInterviewRecord, PermissionLevel,
PreRunPushOutcome, Principal, PullRequest, PullRequestDetails, PullRequestDetailsStatus,
PullRequestDetailsUnavailableReason, PullRequestLink, PullRequestMeta, PullRequestResponse,
- QuestionType, ReasoningOutput, RepositoryRef, Role, Run, RunApproval, RunApprovalState,
- RunClientProvenance, RunEvent, RunEventDetailContentKind, RunEventDetailResponse,
- RunFailure, RunPairStatusResponse, RunProjection, RunProvenance, RunRunnableSource,
- RunSandbox, RunSandboxFailure, RunSandboxInstance, RunSandboxKind, RunSandboxPlan,
- RunSandboxRuntime, RunServerProvenance, RunSize, SandboxDetails, SandboxInfo,
- SandboxListMeta, SandboxListResponse, SandboxNetwork, SandboxNetworkPolicy,
+ QuestionType, ReasoningOutput, RepositoryRef, ReviewTarget, ReviewTargetKind, Role, Run,
+ RunApproval, RunApprovalState, RunClientProvenance, RunEvent, RunEventDetailContentKind,
+ RunEventDetailResponse, RunFailure, RunPairStatusResponse, RunProjection, RunProvenance,
+ RunRunnableSource, RunSandbox, RunSandboxFailure, RunSandboxInstance, RunSandboxKind,
+ RunSandboxPlan, RunSandboxRuntime, RunServerProvenance, RunSize, SandboxDetails,
+ SandboxInfo, SandboxListMeta, SandboxListResponse, SandboxNetwork, SandboxNetworkPolicy,
SandboxNetworkPolicyMode, SandboxProviderKind, SandboxProviderLookupError,
SandboxResources, SandboxService, SandboxServiceListResponse, SandboxState,
SandboxTimestamps, SecretMetadata, SecretType, ServerSettings, SessionDetail, SessionId,
diff --git a/lib/foundation/fabro-api/tests/interview_question_record_round_trip.rs b/lib/foundation/fabro-api/tests/interview_question_record_round_trip.rs
index 14338c4b4..e6c828077 100644
--- a/lib/foundation/fabro-api/tests/interview_question_record_round_trip.rs
+++ b/lib/foundation/fabro-api/tests/interview_question_record_round_trip.rs
@@ -1,12 +1,17 @@
use std::any::{TypeId, type_name};
-use fabro_api::types::InterviewQuestionRecord as ApiInterviewQuestionRecord;
-use fabro_types::InterviewQuestionRecord;
+use fabro_api::types::{
+ InterviewQuestionRecord as ApiInterviewQuestionRecord, ReviewTarget as ApiReviewTarget,
+ ReviewTargetKind as ApiReviewTargetKind,
+};
+use fabro_types::{InterviewQuestionRecord, ReviewTarget, ReviewTargetKind};
use serde_json::json;
#[test]
fn interview_question_record_reuses_canonical_type() {
assert_same_type::();
+ assert_same_type::();
+ assert_same_type::();
}
#[test]
@@ -27,7 +32,12 @@ fn interview_question_record_round_trips_representative_json() {
],
"allow_freeform": true,
"timeout_seconds": 30.0,
- "context_display": "Diff summary"
+ "context_display": "Diff summary",
+ "review_target": {
+ "label": "Quarry review exercise",
+ "url": "https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef",
+ "kind": "document"
+ }
});
let question: InterviewQuestionRecord = serde_json::from_value(value.clone()).unwrap();
diff --git a/lib/foundation/fabro-api/tests/pending_interview_record_round_trip.rs b/lib/foundation/fabro-api/tests/pending_interview_record_round_trip.rs
index dae81fdc3..c1ac64a8a 100644
--- a/lib/foundation/fabro-api/tests/pending_interview_record_round_trip.rs
+++ b/lib/foundation/fabro-api/tests/pending_interview_record_round_trip.rs
@@ -28,7 +28,12 @@ fn pending_interview_record_round_trips_populated_question() {
],
"allow_freeform": true,
"timeout_seconds": 30.0,
- "context_display": "Diff summary"
+ "context_display": "Diff summary",
+ "review_target": {
+ "label": "Quarry review exercise",
+ "url": "https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef",
+ "kind": "document"
+ }
},
"started_at": "2026-04-29T12:34:56Z"
});
diff --git a/lib/foundation/fabro-types/Cargo.toml b/lib/foundation/fabro-types/Cargo.toml
index 483e7d697..273309c73 100644
--- a/lib/foundation/fabro-types/Cargo.toml
+++ b/lib/foundation/fabro-types/Cargo.toml
@@ -28,6 +28,7 @@ serde.workspace = true
serde_json.workspace = true
sha2.workspace = true
strum.workspace = true
+thiserror.workspace = true
toml.workspace = true
ulid.workspace = true
url.workspace = true
diff --git a/lib/foundation/fabro-types/src/graph.rs b/lib/foundation/fabro-types/src/graph.rs
index 7ef9bad99..e593d55e4 100644
--- a/lib/foundation/fabro-types/src/graph.rs
+++ b/lib/foundation/fabro-types/src/graph.rs
@@ -193,6 +193,11 @@ impl Node {
self.bool_attr("goal_gate").unwrap_or(false)
}
+ #[must_use]
+ pub fn review_target(&self) -> bool {
+ self.bool_attr("review_target").unwrap_or(false)
+ }
+
#[must_use]
pub fn retry_target(&self) -> Option<&str> {
self.str_attr("retry_target")
@@ -590,6 +595,7 @@ mod tests {
assert_eq!(node.output_retries(), 2);
assert_eq!(node.max_retries(), None);
assert!(!node.goal_gate());
+ assert!(!node.review_target());
assert_eq!(node.retry_target(), None);
assert_eq!(node.fallback_retry_target(), None);
assert_eq!(node.fidelity(), None);
@@ -652,12 +658,15 @@ mod tests {
);
node.attrs
.insert("goal_gate".to_string(), AttrValue::Boolean(true));
+ node.attrs
+ .insert("review_target".to_string(), AttrValue::Boolean(true));
node.attrs
.insert("max_retries".to_string(), AttrValue::Integer(3));
assert_eq!(node.label(), "Plan step");
assert_eq!(node.shape(), "diamond");
assert!(node.goal_gate());
+ assert!(node.review_target());
assert_eq!(node.max_retries(), Some(3));
}
diff --git a/lib/foundation/fabro-types/src/interview.rs b/lib/foundation/fabro-types/src/interview.rs
index 4ad2dc295..729ad6250 100644
--- a/lib/foundation/fabro-types/src/interview.rs
+++ b/lib/foundation/fabro-types/src/interview.rs
@@ -1,7 +1,173 @@
+use serde::de::Error as _;
use serde::{Deserialize, Serialize};
+use thiserror::Error;
use crate::run_event::InterviewOption;
+const REVIEW_TARGET_LABEL_MAX_CHARS: usize = 200;
+const REVIEW_TARGET_URL_MAX_CHARS: usize = 2048;
+
+/// The type of resource a human should review. `Display` renders the noun used
+/// in review question text ("document").
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, strum::Display)]
+#[serde(rename_all = "snake_case")]
+#[strum(serialize_all = "snake_case")]
+pub enum ReviewTargetKind {
+ Document,
+}
+
+/// A validated external resource presented as the primary subject of a human
+/// review question.
+#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
+pub struct ReviewTarget {
+ label: String,
+ url: String,
+ kind: ReviewTargetKind,
+}
+
+impl ReviewTarget {
+ pub fn new(
+ label: impl Into,
+ url: impl Into,
+ kind: ReviewTargetKind,
+ ) -> Result {
+ let label = label.into();
+ let url = url.into();
+ let label = label.trim();
+ let url = url.trim();
+
+ validate_review_target_label(label)?;
+ validate_review_target_url(url)?;
+
+ Ok(Self {
+ label: label.to_string(),
+ url: url.to_string(),
+ kind,
+ })
+ }
+
+ #[must_use]
+ pub fn label(&self) -> &str {
+ &self.label
+ }
+
+ #[must_use]
+ pub fn url(&self) -> &str {
+ &self.url
+ }
+
+ #[must_use]
+ pub const fn kind(&self) -> ReviewTargetKind {
+ self.kind
+ }
+
+ /// The question text shown to a human, with the label rendered as plain
+ /// text.
+ #[must_use]
+ pub fn question_text(&self) -> String {
+ self.question_text_with_link(&self.label)
+ }
+
+ /// The same sentence as [`Self::question_text`], with the label replaced by
+ /// a client-specific rendering of the link (Slack `` syntax, for
+ /// example). This is the single definition of the review question wording.
+ #[must_use]
+ pub fn question_text_with_link(&self, rendered_link: &str) -> String {
+ format!(
+ "Review the {rendered_link} {}, then choose the next action.",
+ self.kind
+ )
+ }
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, Error)]
+pub enum ReviewTargetError {
+ #[error("review target label must not be empty")]
+ EmptyLabel,
+ #[error("review target label must be at most {REVIEW_TARGET_LABEL_MAX_CHARS} characters")]
+ LabelTooLong,
+ #[error("review target label must not contain control characters")]
+ LabelContainsControl,
+ #[error("review target URL must not be empty")]
+ EmptyUrl,
+ #[error("review target URL must be at most {REVIEW_TARGET_URL_MAX_CHARS} characters")]
+ UrlTooLong,
+ #[error("review target URL must not contain control characters or link delimiters")]
+ UrlContainsUnsafeCharacters,
+ #[error("review target URL must be a valid absolute URL")]
+ InvalidUrl,
+ #[error("review target URL must use http or https")]
+ UnsupportedUrlScheme,
+ #[error("review target URL must include a host")]
+ MissingUrlHost,
+ #[error("review target URL must not include username or password credentials")]
+ UrlContainsCredentials,
+}
+
+fn validate_review_target_label(label: &str) -> Result<(), ReviewTargetError> {
+ if label.is_empty() {
+ return Err(ReviewTargetError::EmptyLabel);
+ }
+ if label.chars().count() > REVIEW_TARGET_LABEL_MAX_CHARS {
+ return Err(ReviewTargetError::LabelTooLong);
+ }
+ if label.chars().any(char::is_control) {
+ return Err(ReviewTargetError::LabelContainsControl);
+ }
+ Ok(())
+}
+
+#[expect(
+ clippy::disallowed_types,
+ reason = "Review target validation parses an untrusted URL only to enforce safe display schemes and syntax; Fabro never fetches the URL."
+)]
+fn validate_review_target_url(url: &str) -> Result<(), ReviewTargetError> {
+ if url.is_empty() {
+ return Err(ReviewTargetError::EmptyUrl);
+ }
+ if url.chars().count() > REVIEW_TARGET_URL_MAX_CHARS {
+ return Err(ReviewTargetError::UrlTooLong);
+ }
+ if url
+ .chars()
+ .any(|character| character.is_control() || matches!(character, '<' | '>' | '|'))
+ {
+ return Err(ReviewTargetError::UrlContainsUnsafeCharacters);
+ }
+
+ let parsed = url::Url::parse(url).map_err(|_| ReviewTargetError::InvalidUrl)?;
+ if !matches!(parsed.scheme(), "http" | "https") {
+ return Err(ReviewTargetError::UnsupportedUrlScheme);
+ }
+ if parsed.host_str().is_none() {
+ return Err(ReviewTargetError::MissingUrlHost);
+ }
+ if !parsed.username().is_empty() || parsed.password().is_some() {
+ return Err(ReviewTargetError::UrlContainsCredentials);
+ }
+ Ok(())
+}
+
+impl<'de> Deserialize<'de> for ReviewTarget {
+ fn deserialize(deserializer: D) -> Result
+ where
+ D: serde::Deserializer<'de>,
+ {
+ // Unknown fields are ignored so the OpenAPI contract (which leaves
+ // `additionalProperties` permissive) and this deserializer agree, and
+ // so an unknown key in a persisted event cannot fail the whole event.
+ #[derive(Deserialize)]
+ struct WireReviewTarget {
+ label: String,
+ url: String,
+ kind: ReviewTargetKind,
+ }
+
+ let wire = WireReviewTarget::deserialize(deserializer)?;
+ Self::new(wire.label, wire.url, wire.kind).map_err(D::Error::custom)
+ }
+}
+
#[derive(
Debug,
Clone,
@@ -43,6 +209,8 @@ pub struct InterviewQuestionRecord {
pub timeout_seconds: Option,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub context_display: Option,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub review_target: Option,
}
#[cfg(test)]
@@ -64,4 +232,52 @@ mod tests {
assert_eq!(question_type.to_string(), wire);
}
}
+
+ #[test]
+ fn review_target_roundtrips_and_builds_question_text() {
+ let value = serde_json::json!({
+ "label": "Quarry review exercise",
+ "url": "https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef",
+ "kind": "document",
+ });
+
+ let target: ReviewTarget = serde_json::from_value(value.clone()).unwrap();
+
+ assert_eq!(target.label(), "Quarry review exercise");
+ assert_eq!(
+ target.question_text(),
+ "Review the Quarry review exercise document, then choose the next action."
+ );
+ assert_eq!(serde_json::to_value(target).unwrap(), value);
+ }
+
+ #[test]
+ fn review_target_rejects_non_http_urls_and_credentials() {
+ for url in [
+ "javascript:alert(1)",
+ "file:///tmp/review.md",
+ "https://user:secret@example.com/review",
+ ] {
+ assert!(
+ ReviewTarget::new("Review", url, ReviewTargetKind::Document).is_err(),
+ "URL should be rejected: {url}"
+ );
+ }
+ }
+
+ #[test]
+ fn review_target_rejects_blank_or_unsafe_display_values() {
+ assert_eq!(
+ ReviewTarget::new(" ", "https://example.com", ReviewTargetKind::Document),
+ Err(ReviewTargetError::EmptyLabel)
+ );
+ assert_eq!(
+ ReviewTarget::new(
+ "Review",
+ "https://example.com/a|b",
+ ReviewTargetKind::Document
+ ),
+ Err(ReviewTargetError::UrlContainsUnsafeCharacters)
+ );
+ }
}
diff --git a/lib/foundation/fabro-types/src/lib.rs b/lib/foundation/fabro-types/src/lib.rs
index b7f3b0474..8b5288c7b 100644
--- a/lib/foundation/fabro-types/src/lib.rs
+++ b/lib/foundation/fabro-types/src/lib.rs
@@ -75,7 +75,9 @@ pub use graph::{
AttrValue, Edge, Graph, KNOWN_HANDLER_TYPES, Node, is_known_handler_type, is_llm_handler_type,
shape_to_handler_type,
};
-pub use interview::{InterviewQuestionRecord, QuestionType};
+pub use interview::{
+ InterviewQuestionRecord, QuestionType, ReviewTarget, ReviewTargetError, ReviewTargetKind,
+};
pub use llm_backend::AgentBackend;
pub use manifest_path::{ManifestPath, ManifestPathParseError};
pub use mcp_store::{
diff --git a/lib/foundation/fabro-types/src/run_event/misc.rs b/lib/foundation/fabro-types/src/run_event/misc.rs
index d3bf9c1c0..13612c677 100644
--- a/lib/foundation/fabro-types/src/run_event/misc.rs
+++ b/lib/foundation/fabro-types/src/run_event/misc.rs
@@ -1,7 +1,9 @@
use serde::{Deserialize, Serialize};
use super::ExecOutputTail;
-use crate::{CommandTermination, ParallelBranchResult, PullRequestLink, StageId, StageOutcome};
+use crate::{
+ CommandTermination, ParallelBranchResult, PullRequestLink, ReviewTarget, StageId, StageOutcome,
+};
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)]
pub struct InterviewOption {
@@ -65,6 +67,8 @@ pub struct InterviewStartedProps {
pub timeout_seconds: Option,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub context_display: Option,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub review_target: Option,
}
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
diff --git a/lib/packages/fabro-api-client/src/.openapi-generator/FILES b/lib/packages/fabro-api-client/src/.openapi-generator/FILES
index 4ae265b76..d0ea6bd28 100644
--- a/lib/packages/fabro-api-client/src/.openapi-generator/FILES
+++ b/lib/packages/fabro-api-client/src/.openapi-generator/FILES
@@ -335,6 +335,8 @@ models/replace-mcp-server-request.ts
models/repo-check-response-permissions.ts
models/repo-check-response.ts
models/repository-ref.ts
+models/review-target-kind.ts
+models/review-target.ts
models/rewind-request.ts
models/rewind-response.ts
models/root-response-urls.ts
diff --git a/lib/packages/fabro-api-client/src/models/api-question.ts b/lib/packages/fabro-api-client/src/models/api-question.ts
index 74908ff63..8b0330475 100644
--- a/lib/packages/fabro-api-client/src/models/api-question.ts
+++ b/lib/packages/fabro-api-client/src/models/api-question.ts
@@ -19,6 +19,9 @@ import type { InterviewOption } from './interview-option';
// May contain unused imports in some cases
// @ts-ignore
import type { QuestionType } from './question-type';
+// May contain unused imports in some cases
+// @ts-ignore
+import type { ReviewTarget } from './review-target';
/**
* A pending human-in-the-loop question generated by a workflow stage.
@@ -53,4 +56,5 @@ export interface ApiQuestion {
* Optional contextual text shown alongside the question.
*/
'context_display'?: string | null;
+ 'review_target'?: ReviewTarget | null;
}
diff --git a/lib/packages/fabro-api-client/src/models/index.ts b/lib/packages/fabro-api-client/src/models/index.ts
index 029a2d3ab..ac6fc24a4 100644
--- a/lib/packages/fabro-api-client/src/models/index.ts
+++ b/lib/packages/fabro-api-client/src/models/index.ts
@@ -305,6 +305,8 @@ export * from './replace-mcp-server-request';
export * from './repo-check-response';
export * from './repo-check-response-permissions';
export * from './repository-ref';
+export * from './review-target';
+export * from './review-target-kind';
export * from './rewind-request';
export * from './rewind-response';
export * from './root-response';
diff --git a/lib/packages/fabro-api-client/src/models/interview-question-record.ts b/lib/packages/fabro-api-client/src/models/interview-question-record.ts
index 6d940e919..2f2ecd2b8 100644
--- a/lib/packages/fabro-api-client/src/models/interview-question-record.ts
+++ b/lib/packages/fabro-api-client/src/models/interview-question-record.ts
@@ -19,6 +19,9 @@ import type { InterviewOption } from './interview-option';
// May contain unused imports in some cases
// @ts-ignore
import type { QuestionType } from './question-type';
+// May contain unused imports in some cases
+// @ts-ignore
+import type { ReviewTarget } from './review-target';
/**
* Storage shape of an interview question recorded in the event log.
@@ -32,4 +35,5 @@ export interface InterviewQuestionRecord {
'allow_freeform': boolean;
'timeout_seconds'?: number | null;
'context_display'?: string | null;
+ 'review_target'?: ReviewTarget | null;
}
diff --git a/lib/packages/fabro-api-client/src/models/review-target-kind.ts b/lib/packages/fabro-api-client/src/models/review-target-kind.ts
new file mode 100644
index 000000000..e50c78a38
--- /dev/null
+++ b/lib/packages/fabro-api-client/src/models/review-target-kind.ts
@@ -0,0 +1,25 @@
+/* tslint:disable */
+/* eslint-disable */
+/**
+ * Fabro Run API
+ * HTTP API for managing Fabro workflow run executions.
+ *
+ * The version of the OpenAPI document: 0.1.0
+ *
+ *
+ * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
+ * https://openapi-generator.tech
+ * Do not edit the class manually.
+ */
+
+
+
+/**
+ * The type of resource presented for human review.
+ */
+
+export const ReviewTargetKind = {
+ DOCUMENT: 'document'
+} as const;
+
+export type ReviewTargetKind = typeof ReviewTargetKind[keyof typeof ReviewTargetKind];
diff --git a/lib/packages/fabro-api-client/src/models/review-target.ts b/lib/packages/fabro-api-client/src/models/review-target.ts
new file mode 100644
index 000000000..61b476d21
--- /dev/null
+++ b/lib/packages/fabro-api-client/src/models/review-target.ts
@@ -0,0 +1,33 @@
+/* tslint:disable */
+/* eslint-disable */
+/**
+ * Fabro Run API
+ * HTTP API for managing Fabro workflow run executions.
+ *
+ * The version of the OpenAPI document: 0.1.0
+ *
+ *
+ * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
+ * https://openapi-generator.tech
+ * Do not edit the class manually.
+ */
+
+
+// May contain unused imports in some cases
+// @ts-ignore
+import type { ReviewTargetKind } from './review-target-kind';
+
+/**
+ * A validated external resource presented as the primary subject of a human review question.
+ */
+export interface ReviewTarget {
+ /**
+ * Human-readable link label.
+ */
+ 'label': string;
+ /**
+ * Absolute HTTP or HTTPS URL opened by the reviewer.
+ */
+ 'url': string;
+ 'kind': ReviewTargetKind;
+}