scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-08-28 05:27:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Merge pull request #671 from fabro-sh/feat/human-review-target

Browse source 
Add structured review targets to human gates
This commit is contained in:
Bryan Helmkamp 2026-07-28 16:54:59 -04:00 • committed by GitHub
commit eb2d612906
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
42 changed files with 1015 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
Cargo.lock generated
View file

@ -3258,6 +3258,7 @@ dependencies = [
"shlex",
"strum 0.28.0",
"tempfile",
"thiserror 2.0.18",
"toml 0.8.23",
"ulid",
"url",

53
apps/fabro-web/app/components/interview-dock.test.tsx
View file

@ -4,6 +4,7 @@ import { SWRConfig } from "swr";
import {
type ApiQuestion,
QuestionType,
ReviewTargetKind,
} from "@qltysh/fabro-api-client";
import {
@ -78,6 +79,58 @@ describe("InterviewDock", () => {
expect(text).toContain("Awaiting input");
});
test("renders the review target as the link in the question", () => {
const url =
"https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef";
const tree = render(
<InterviewDock
runId="run-1"
questions={[
makeQuestion({
text: "Review the Quarry review exercise document, then choose the next action.",
review_target: {
label: "Quarry review exercise",
url,
kind: ReviewTargetKind.DOCUMENT,
},
}),
]}
/>,
);
expect(textContent(tree.toJSON())).toContain(
"Review the Quarry review exercise document, then choose the next action.",
);
const links = tree.root.findAllByType("a");
expect(links).toHaveLength(1);
expect(links[0].props.href).toBe(url);
expect(links[0].props.target).toBe("_blank");
expect(links[0].props.rel).toBe("noopener noreferrer");
expect(links[0].props.referrerPolicy).toBe("no-referrer");
});
test("does not link an unsafe review target received from the API", () => {
const fallback = "Review the document, then choose the next action.";
const tree = render(
<InterviewDock
runId="run-1"
questions={[
makeQuestion({
text: fallback,
review_target: {
label: "Unsafe target",
url: "javascript:alert(1)",
kind: ReviewTargetKind.DOCUMENT,
},
}),
]}
/>,
);
expect(textContent(tree.toJSON())).toContain(fallback);
expect(tree.root.findAllByType("a")).toHaveLength(0);
});
test("yes/no question shows two buttons", () => {
const tree = render(
<InterviewDock runId="run-1" questions={[makeQuestion()]} />,

19
apps/fabro-web/app/components/interview-dock.tsx
View file

@ -16,6 +16,10 @@ import {
} from "../lib/mutations";
import { ApiError } from "../lib/api-client";
import { displayLabel } from "./interview-label";
import {
ReviewTargetQuestion,
safeReviewTarget,
} from "./review-target-question";
import {
DockComposer,
RunDockShell,
@ -35,6 +39,9 @@ import {
*/
const STACK_LABEL_LENGTH = 40;
/** Shared by the plain question text and the review target rendering. */
const QUESTION_TEXT = "max-w-[78ch] text-base/6 font-medium text-pretty text-fg";
type SubmitInterviewAnswer = SubmitInterviewAnswerArg["answer"];
export interface InterviewDockProps {
@ -82,6 +89,7 @@ function InterviewQuestionDock({
const [error, setError] = useState<string | null>(null);
const [collapsed, setCollapsed] = useState(false);
const submitting = submitMutation.isMutating;
const reviewTarget = safeReviewTarget(question.review_target);
const submit = useCallback(
async (answer: SubmitInterviewAnswer) => {
@ -120,9 +128,14 @@ function InterviewQuestionDock({
}
body={
<>
<p className="max-w-[78ch] text-base/6 font-medium text-pretty text-fg">
{question.text}
</p>
{reviewTarget ? (
<ReviewTargetQuestion
target={reviewTarget}
className={QUESTION_TEXT}
/>
) : (
<p className={QUESTION_TEXT}>{question.text}</p>
)}
{question.context_display && (
<ContextPanel text={question.context_display} />
)}

59
apps/fabro-web/app/components/review-target-question.tsx Normal file
View file

@ -0,0 +1,59 @@
import { ArrowTopRightOnSquareIcon } from "@heroicons/react/20/solid";
import type { ReviewTarget } from "@qltysh/fabro-api-client";
/**
* Re-check the URL before putting it in an `href`. The server already rejects
* unsafe targets (see `ReviewTarget::new` in
* `lib/foundation/fabro-types/src/interview.rs`), but React does not sanitize
* `href`, so a `javascript:` URL reaching this component would execute. Length
* and control-character limits stay server-side; they cannot affect the DOM.
*/
export function safeReviewTarget(
target: ReviewTarget | null | undefined,
): ReviewTarget | null {
if (!target?.url || !target.label) return null;
try {
const parsed = new URL(target.url);
const safe =
(parsed.protocol === "http:" || parsed.protocol === "https:") &&
Boolean(parsed.host) &&
!parsed.username &&
!parsed.password;
return safe ? target : null;
} catch {
return null;
}
}
/**
* The review question sentence, with the target label as an external link.
* Mirrors `ReviewTarget::question_text_with_link` in
* `lib/foundation/fabro-types/src/interview.rs`.
*/
export function ReviewTargetQuestion({
target,
className,
}: {
target: ReviewTarget;
className?: string;
}) {
return (
<p className={className}>
Review the{" "}
<a
href={target.url}
target="_blank"
rel="noopener noreferrer"
referrerPolicy="no-referrer"
className="inline-flex items-baseline gap-1 font-semibold text-teal-300 underline decoration-teal-500/50 underline-offset-2 transition-colors hover:text-fg focus-visible:rounded-sm focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-teal-500"
>
<span>{target.label}</span>
<ArrowTopRightOnSquareIcon
className="size-3 shrink-0 self-center"
aria-hidden="true"
/>
</a>{" "}
{target.kind}, then choose the next action.
</p>
);
}

27
apps/fabro-web/app/components/stage-renderers/helpers.test.ts
View file

@ -98,6 +98,33 @@ describe("parseHumanInterviewPairs", () => {
});
});
test("preserves a typed review target from started events", () => {
const events: EventEnvelope[] = [
makeEventEnvelope(1, {
event: "interview.started",
properties: {
question_id: "q-1",
question:
"Review the Quarry review exercise document, then choose the next action.",
question_type: "multiple_choice",
review_target: {
label: "Quarry review exercise",
url: "https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef",
kind: "document",
},
},
}),
];
const pairs = parseHumanInterviewPairs(events);
expect(pairs[0].question.reviewTarget).toEqual({
label: "Quarry review exercise",
url: "https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef",
kind: "document",
});
});
test("captures timeout and interrupted resolutions", () => {
const events: EventEnvelope[] = [
makeEventEnvelope(1, {

24
apps/fabro-web/app/components/stage-renderers/helpers.ts
View file

@ -1,7 +1,14 @@
import { StageOutcome } from "@qltysh/fabro-api-client";
import type { EventEnvelope } from "@qltysh/fabro-api-client";
import { ReviewTargetKind, StageOutcome } from "@qltysh/fabro-api-client";
import type { EventEnvelope, ReviewTarget } from "@qltysh/fabro-api-client";
import { getArray, getNumber, getObject, getString, type UnknownRecord } from "../../lib/unknown";
import {
getArray,
getNumber,
getObject,
getString,
isRecord,
type UnknownRecord,
} from "../../lib/unknown";
const STAGE_OUTCOMES: ReadonlySet<string> = new Set(Object.values(StageOutcome));
@ -25,6 +32,7 @@ export interface HumanQuestion {
allowFreeform: boolean;
timeoutSeconds: number | null;
contextDisplay: string | null;
reviewTarget: ReviewTarget | null;
}
export type HumanResolution =
@ -75,6 +83,15 @@ function parseInterviewOptions(value: unknown): InterviewOption[] {
return out;
}
function parseReviewTarget(value: unknown): ReviewTarget | null {
if (!isRecord(value)) return null;
const label = getString(value, "label");
const url = getString(value, "url");
const kind = getString(value, "kind");
if (!label || !url || kind !== ReviewTargetKind.DOCUMENT) return null;
return { label, url, kind };
}
/**
* Pair `interview.started` events with the matching `interview.completed`,
* `.timeout`, or `.interrupted` resolution by `question_id`. Unanswered
@ -98,6 +115,7 @@ export function parseHumanInterviewPairs(events: EventEnvelope[]): HumanIntervie
allowFreeform: props.allow_freeform === true,
timeoutSeconds: getNumber(props, "timeout_seconds") ?? null,
contextDisplay: getString(props, "context_display") ?? null,
reviewTarget: parseReviewTarget(props.review_target),
},
resolution: null,
});

14
apps/fabro-web/app/components/stage-renderers/human-qa.tsx
View file

@ -10,6 +10,10 @@ import {
import type { EventEnvelope } from "@qltysh/fabro-api-client";
import type { Stage } from "../stage-sidebar";
import {
ReviewTargetQuestion,
safeReviewTarget,
} from "../review-target-question";
import { Tooltip } from "../ui";
import { formatAbsoluteTs, formatDurationMs } from "../../lib/format";
import { ACTIVE_STAGE_STATES } from "../../lib/stage-sidebar";
@ -148,6 +152,7 @@ function QuestionBlock({
stageActive: boolean;
}) {
const { question, resolution } = pair;
const reviewTarget = safeReviewTarget(question.reviewTarget);
return (
<article className="space-y-3">
<header className="flex flex-wrap items-baseline gap-x-3 gap-y-1">
@ -168,7 +173,14 @@ function QuestionBlock({
</header>
<div className="rounded-lg bg-panel p-4 outline-1 -outline-offset-1 outline-line">
<Markdown content={question.question} />
{reviewTarget ? (
<ReviewTargetQuestion
target={reviewTarget}
className="text-sm/6 text-fg-2"
/>
) : (
<Markdown content={question.question} />
)}
{question.contextDisplay && (
<div className="mt-3 border-t border-line pt-3 text-xs text-fg-muted">
<Markdown content={question.contextDisplay} />

39
docs/public/api-reference/fabro-api.yaml
View file

@ -9597,6 +9597,11 @@ components:
type: ["string", "null"]
description: Optional contextual text shown alongside the question.
example: Latest draft
review_target:
description: Optional validated external resource that is the primary subject of this review question.
oneOf:
- $ref: "#/components/schemas/ReviewTarget"
- type: "null"
QuestionType:
description: The interaction type of a human-in-the-loop question.
@ -11171,6 +11176,36 @@ components:
type: ["string", "null"]
description: Optional untrusted model-authored option preview captured for clients.
ReviewTargetKind:
description: The type of resource presented for human review.
type: string
enum:
- document
ReviewTarget:
description: A validated external resource presented as the primary subject of a human review question.
type: object
required:
- label
- url
- kind
properties:
label:
type: string
minLength: 1
maxLength: 200
description: Human-readable link label.
example: Quarry review exercise
url:
type: string
format: uri
minLength: 1
maxLength: 2048
description: Absolute HTTP or HTTPS URL opened by the reviewer.
example: https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef
kind:
$ref: "#/components/schemas/ReviewTargetKind"
InterviewQuestionRecord:
description: Storage shape of an interview question recorded in the event log.
type: object
@ -11200,6 +11235,10 @@ components:
format: double
context_display:
type: ["string", "null"]
review_target:
oneOf:
- $ref: "#/components/schemas/ReviewTarget"
- type: "null"
PendingInterviewRecord:
description: Pending interview question plus the time it entered the unresolved set.

5
docs/public/execution/context.mdx
View file

@ -40,6 +40,11 @@ Each handler type writes specific keys into the context after execution:
Agents can also emit arbitrary context updates by including a JSON object with a `context_updates` field in their response. See [Transitions](/workflows/transitions#agent-transitions).
The `review_target` key has an optional typed convention for human review
workflows. A human gate with `review_target=true` reads this exact flat key and
presents its document URL as the primary question link. See
[Review targets](/workflows/human-in-the-loop#review-targets).
### Command nodes
| Key | Value |

1
docs/public/reference/dot-language.mdx
View file

@ -267,6 +267,7 @@ For the first node in each branch, `fidelity` resolves from the fork-to-branch e
| Attribute | Type | Description |
|---|---|---|
| `question_type` | String | Optional interview question type override: `yes_no`, `confirmation`, `multiple_choice`, `multi_select`, or `freeform`. Defaults to `freeform` when the gate only has a freeform edge; otherwise defaults to `multiple_choice`. |
| `review_target` | Boolean | When `true`, read and validate the typed `review_target` context value, then present it as the primary link in the question. Fabro generates the question text, so the node's `label` is not used. See [Review targets](/workflows/human-in-the-loop#review-targets). |
| `human.default_choice` | String | Target node to use when the question times out. |
### Manager loop (sub-workflow) nodes

62
docs/public/workflows/human-in-the-loop.mdx
View file

@ -60,6 +60,68 @@ confirm -> exit [label="[N] No"]
Supported values are `yes_no`, `confirmation`, `multiple_choice`, `multi_select`, and `freeform`.
### Review targets
A human gate can present one external document as the primary review link. Set
`review_target=true` on the gate:
```dot
review [
shape=hexagon,
review_target=true
]
review -> sync [label="[S] Review complete; sync the current Markdown"]
review -> address [label="[A] Ask the agent to address human feedback"]
review -> review [label="[C] Continue reviewing"]
```
Before the workflow reaches the gate, an agent, prompt, or command node must set
the flat `review_target` context key. A routing response can do this directly:
```json
{
"outcome": "succeeded",
"context_updates": {
"review_target": {
"label": "Quarry review exercise",
"url": "https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef",
"kind": "document"
}
}
}
```
Fabro then presents this question:
> Review the [Quarry review exercise](https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef) document, then choose the next action.
The link uses the target URL from context. The example uses a placeholder
secret, not a live Quarry document.
Fabro generates this question text from the target. A `label` on the gate is
not used while `review_target=true`.
The target object has three required fields:
| Field | Meaning |
|---|---|
| `label` | The link text. It must contain 1 to 200 characters and no control characters. |
| `url` | An absolute HTTP or HTTPS URL. It must contain a host, must not contain URL credentials, and must be at most 2048 characters. |
| `kind` | The resource type. The supported value is `document`. |
Fabro validates the target before it starts the interview. A missing or invalid
target fails the gate deterministically. A gate without `review_target=true`
ignores this context key and keeps its normal label.
The context value remains available after the human answers. A feedback loop
can return to the same gate without recreating the target. A later stage can
replace the target by writing a new value to the same context key.
Fabro does not fetch the URL. Web and Slack clients open it as an external link.
Treat bearer-capability links as secrets and only provide them to people who
can access the run.
### Default choice on timeout
If a human gate has a timeout configured, you can specify a default choice using the `human.default_choice` attribute:

4
lib/apps/fabro-cli/src/commands/run/attach.rs
View file

@ -438,6 +438,7 @@ fn api_question_to_question(question: &types::ApiQuestion) -> Question {
converted
.context_display
.clone_from(&question.context_display);
converted.review_target.clone_from(&question.review_target);
converted
}
@ -451,6 +452,9 @@ async fn ask_attach_question(question: Question, styles: &'static Styles) -> Ans
let rendered = styles.render_markdown(context_text);
eprint!("{rendered}");
}
if let Some(line) = fabro_interview::review_target_line(&question) {
eprintln!("{line}");
}
eprintln!("{} {}", styles.bold_cyan.apply_to("?"), question.text);
match question.question_type {

1
lib/apps/fabro-cli/src/commands/run/runner.rs
View file

@ -1352,6 +1352,7 @@ mod tests {
allow_freeform: false,
timeout_seconds: None,
context_display: None,
review_target: None,
})),
Some(WorkerTitlePhase::Waiting)
);

2
lib/apps/fabro-server/src/demo/mod.rs
View file

@ -1753,6 +1753,7 @@ mod runs {
allow_freeform: false,
timeout_seconds: None,
context_display: None,
review_target: None,
},
ApiQuestion {
id: "q-002".into(),
@ -1776,6 +1777,7 @@ mod runs {
allow_freeform: true,
timeout_seconds: None,
context_display: None,
review_target: None,
},
]
}

3
lib/apps/fabro-server/src/server.rs
View file

@ -719,6 +719,7 @@ impl SlackService {
allow_freeform: props.allow_freeform,
timeout_seconds: props.timeout_seconds,
context_display: props.context_display.clone(),
review_target: props.review_target.clone(),
});
let blocks = slack_blocks::question_to_blocks(
&event.run_id.to_string(),
@ -3708,6 +3709,7 @@ fn runtime_question_from_interview_record(question: &InterviewQuestionRecord) ->
stage: question.stage.clone(),
metadata: HashMap::new(),
context_display: question.context_display.clone(),
review_target: question.review_target.clone(),
}
}
@ -3721,6 +3723,7 @@ fn api_question_from_interview_record(question: &InterviewQuestionRecord) -> Api
allow_freeform: question.allow_freeform,
timeout_seconds: question.timeout_seconds,
context_display: question.context_display.clone(),
review_target: question.review_target.clone(),
}
}

26
lib/apps/fabro-server/src/server/tests.rs
View file

@ -4765,6 +4765,7 @@ channel = "#deploys"
allow_freeform: true,
timeout_seconds: None,
context_display: None,
review_target: None,
},
)
.await;
@ -8191,6 +8192,7 @@ async fn submit_pending_interview_answer_rejects_invalid_answer_shape() {
allow_freeform: false,
timeout_seconds: None,
context_display: None,
review_target: None,
},
};
@ -8219,6 +8221,7 @@ fn validate_answer_for_question_accepts_no_for_confirmation() {
allow_freeform: false,
timeout_seconds: None,
context_display: None,
review_target: None,
};
let result = validate_answer_for_question(&question, &Answer::no());
@ -8237,6 +8240,7 @@ fn answer_from_typed_yes_request_maps_to_yes_answer() {
allow_freeform: false,
timeout_seconds: None,
context_display: None,
review_target: None,
};
let req: SubmitAnswerRequest = serde_json::from_value(json!({ "kind": "yes" })).unwrap();
@ -8256,6 +8260,7 @@ fn answer_from_typed_no_request_maps_to_no_answer() {
allow_freeform: false,
timeout_seconds: None,
context_display: None,
review_target: None,
};
let req: SubmitAnswerRequest = serde_json::from_value(json!({ "kind": "no" })).unwrap();
@ -8280,6 +8285,7 @@ fn answer_from_typed_selected_request_validates_and_attaches_option() {
allow_freeform: false,
timeout_seconds: None,
context_display: None,
review_target: None,
};
let req: SubmitAnswerRequest =
serde_json::from_value(json!({ "kind": "selected", "option_key": "approve" })).unwrap();
@ -8320,6 +8326,7 @@ fn answer_from_typed_multi_selected_request_validates_option_keys() {
allow_freeform: false,
timeout_seconds: None,
context_display: None,
review_target: None,
};
let req: SubmitAnswerRequest = serde_json::from_value(json!({
"kind": "multi_selected",
@ -9737,6 +9744,11 @@ async fn get_run_state_exposes_pending_interviews() {
"allow_freeform": false,
"context_display": null,
"timeout_seconds": null,
"review_target": {
"label": "Quarry review exercise",
"url": "https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef",
"kind": "document"
},
}),
Some("gate"),
)
@ -9816,6 +9828,11 @@ async fn cache_backed_run_endpoints_reflect_events_appended_after_warmup() {
"allow_freeform": false,
"context_display": null,
"timeout_seconds": null,
"review_target": {
"label": "Quarry review exercise",
"url": "https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef",
"kind": "document"
},
}),
Some("review"),
)
@ -9873,6 +9890,10 @@ async fn cache_backed_run_endpoints_reflect_events_appended_after_warmup() {
state_body["pending_interviews"]["q-cache"]["question"]["text"].as_str(),
Some("Approve cached deploy?")
);
assert_eq!(
state_body["pending_interviews"]["q-cache"]["question"]["review_target"]["url"].as_str(),
Some("https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef")
);
let stages = app
.clone()
@ -9904,6 +9925,10 @@ async fn cache_backed_run_endpoints_reflect_events_appended_after_warmup() {
questions["data"][0]["text"].as_str(),
Some("Approve cached deploy?")
);
assert_eq!(
questions["data"][0]["review_target"]["label"].as_str(),
Some("Quarry review exercise")
);
let settings = app
.clone()
@ -16649,6 +16674,7 @@ async fn list_runs_includes_live_metadata_from_run_state() {
allow_freeform: false,
timeout_seconds: None,
context_display: None,
review_target: None,
},
] {
workflow_event::append_event(&run_store, &run_id, &event)

45
lib/components/fabro-interview/src/console.rs
View file

@ -111,6 +111,18 @@ fn parse_non_tty_freeform_response(prompt_read: PromptRead) -> Answer {
}
}
/// The review target line printed above a question in terminal clients, which
/// cannot render a hyperlink label. The label and resource noun are already in
/// `question.text`, so only the URL is shown. Shared with `fabro-cli`'s attach
/// client.
#[must_use]
pub fn review_target_line(question: &Question) -> Option<String> {
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<String> = 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 {

8
lib/components/fabro-interview/src/lib.rs
View file

@ -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<String, serde_json::Value>,
#[serde(default)]
pub context_display: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub review_target: Option<ReviewTarget>,
}
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]

71
lib/components/fabro-slack/src/blocks.rs
View file

@ -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 `<url|label>` 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("<http://127.0.0.1:32276/runs/run-1|Open in Fabro>"));
}
#[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 \
<https://quarry.lithos.computer/tmp/0123456789abcdef0123456789abcdef|Quarry review \
exercise> 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);

17
lib/components/fabro-store/src/run_state.rs
View file

@ -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(

1
lib/components/fabro-store/tests/serializable_projection.rs
View file

@ -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(),
})]);

20
lib/components/fabro-validate/src/rules/inert_attribute.rs
View file

@ -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();

2
lib/components/fabro-workflow/src/context.rs
View file

@ -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"));
}
}

2
lib/components/fabro-workflow/src/event/convert.rs
View file

@ -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: _,

9
lib/components/fabro-workflow/src/event/events.rs
View file

@ -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<f64>,
#[serde(default, skip_serializing_if = "Option::is_none")]
context_display: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
review_target: Option<ReviewTarget>,
},
InterviewCompleted {
#[serde(default, skip_serializing_if = "Option::is_none")]

183
lib/components/fabro-workflow/src/handler/human.rs
View file

@ -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::<ReviewTarget>(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());

1
lib/components/fabro-workflow/src/interview_runtime.rs
View file

@ -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,
);

2
lib/foundation/fabro-api/build.rs
View file

@ -468,6 +468,8 @@ fn main() {
&[],
),
("InterviewOption", "fabro_types::InterviewOption", &[]),
("ReviewTarget", "fabro_types::ReviewTarget", &[]),
("ReviewTargetKind", "fabro_types::ReviewTargetKind", &[]),
(
"InterviewQuestionRecord",
"fabro_types::InterviewQuestionRecord",

12
lib/foundation/fabro-api/src/lib.rs
View file

@ -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,

16
lib/foundation/fabro-api/tests/interview_question_record_round_trip.rs
View file

@ -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::<ApiInterviewQuestionRecord, InterviewQuestionRecord>();
assert_same_type::<ApiReviewTarget, ReviewTarget>();
assert_same_type::<ApiReviewTargetKind, ReviewTargetKind>();
}
#[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();

7
lib/foundation/fabro-api/tests/pending_interview_record_round_trip.rs
View file

@ -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"
});

1
lib/foundation/fabro-types/Cargo.toml
View file

@ -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

9
lib/foundation/fabro-types/src/graph.rs
View file

@ -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));
}

216
lib/foundation/fabro-types/src/interview.rs
View file

@ -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<String>,
url: impl Into<String>,
kind: ReviewTargetKind,
) -> Result<Self, ReviewTargetError> {
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 `<url|label>` 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<D>(deserializer: D) -> Result<Self, D::Error>
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<f64>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub context_display: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub review_target: Option<ReviewTarget>,
}
#[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)
);
}
}

4
lib/foundation/fabro-types/src/lib.rs
View file

@ -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::{

6
lib/foundation/fabro-types/src/run_event/misc.rs
View file

@ -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<f64>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub context_display: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub review_target: Option<ReviewTarget>,
}
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]

2
lib/packages/fabro-api-client/src/.openapi-generator/FILES generated
View file

@ -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

4
lib/packages/fabro-api-client/src/models/api-question.ts generated
View file

@ -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;
}

2
lib/packages/fabro-api-client/src/models/index.ts generated
View file

@ -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';

4
lib/packages/fabro-api-client/src/models/interview-question-record.ts generated
View file

@ -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;
}

25
lib/packages/fabro-api-client/src/models/review-target-kind.ts generated Normal file
View file

@ -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];

33
lib/packages/fabro-api-client/src/models/review-target.ts generated Normal file
View file

@ -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;
}