Add structured review targets to human gates

[brynary]

Summary

• add a typed review target contract for human gates that opt in with review_target=true
• read the target from context.review_target and snapshot it into interview events and API projections
• render the document label as an external link in the web UI and Slack, with an explicit URL in console clients
• validate URLs, update generated clients, and document the workflow contract

Testing

• cargo nextest run --workspace: 7,409 tests passed; the one load-related OpenAPI conformance timeout passed on isolated retry
• cargo +nightly-2026-04-14 clippy across changed Rust packages and all targets
• cargo +nightly-2026-04-14 fmt --check --all
• focused web tests, TypeScript typechecks, production web build, and API client regeneration

[brynary]

Reviewer note: This is intentionally opt-in. A human gate with review_target=true reads the exact flat context.review_target value (label, url, and kind: "document"). Missing or invalid targets fail before the interview starts. Fabro never fetches the URL; it only exposes the link through the normal run surfaces. The full workspace run had one OpenAPI conformance timeout under load, and that test passed alone in 16.6 seconds.

[Copilot]

Pull request overview

Adds an opt-in, typed “review target” contract for human gates (review_target=true) so workflows can attach a validated external document link that is propagated through events/projections/API and rendered consistently across clients (web, Slack, console), with corresponding OpenAPI + docs updates.

Changes:

• Introduces ReviewTarget / ReviewTargetKind in fabro-types with validation + roundtrip tests, and threads it through interview records and workflow events.
• Updates the OpenAPI schema and generated Rust/TypeScript API clients to include review_target on interview questions.
• Renders review targets as external links in the web UI and Slack, and prints explicit URL context in console-based interview flows; documents the workflow contract.

Reviewed changes

Copilot reviewed 41 out of 42 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
lib/packages/fabro-api-client/src/models/review-target.ts	Adds generated TS ReviewTarget model.
lib/packages/fabro-api-client/src/models/review-target-kind.ts	Adds generated TS ReviewTargetKind enum.
lib/packages/fabro-api-client/src/models/interview-question-record.ts	Adds review_target to interview question record model.
lib/packages/fabro-api-client/src/models/index.ts	Re-exports new review target models.
lib/packages/fabro-api-client/src/models/api-question.ts	Adds review_target to pending question model.
lib/packages/fabro-api-client/src/.openapi-generator/FILES	Tracks new generated files.
lib/foundation/fabro-types/src/run_event/misc.rs	Adds review_target to interview started event props.
lib/foundation/fabro-types/src/lib.rs	Re-exports review target types/errors from interview.
lib/foundation/fabro-types/src/interview.rs	Implements ReviewTarget validation + serde, and wires into InterviewQuestionRecord.
lib/foundation/fabro-types/src/graph.rs	Adds Node::review_target() attribute accessor + tests.
lib/foundation/fabro-types/Cargo.toml	Adds thiserror dependency for typed validation errors.
lib/foundation/fabro-api/tests/pending_interview_record_round_trip.rs	Extends API roundtrip test JSON with review_target.
lib/foundation/fabro-api/tests/interview_question_record_round_trip.rs	Asserts canonical-type reuse + JSON parity for review target types.
lib/foundation/fabro-api/src/lib.rs	Exposes ReviewTarget* via generated types module.
lib/foundation/fabro-api/build.rs	Reuses canonical fabro_types::{ReviewTarget, ReviewTargetKind} in API generation.
lib/components/fabro-workflow/src/interview_runtime.rs	Snapshots review_target into interview-started props.
lib/components/fabro-workflow/src/handler/human.rs	Opt-in gate behavior: reads context.review_target, validates, sets question text/target, and snapshots into events; adds tests.
lib/components/fabro-workflow/src/event/events.rs	Adds review_target field to InterviewStarted event variant.
lib/components/fabro-workflow/src/event/convert.rs	Converts review_target through event → event body.
lib/components/fabro-workflow/src/context.rs	Adds review_target context key constant and internal-key assertions.
lib/components/fabro-validate/src/rules/inert_attribute.rs	Allows review_target attribute only on human nodes + tests.
lib/components/fabro-store/tests/serializable_projection.rs	Updates projection test fixtures to include review_target: None.
lib/components/fabro-store/src/run_state.rs	Projects review_target into pending interviews; adds coverage in reducer tests.
lib/components/fabro-slack/src/blocks.rs	Renders review target as the primary link in Slack question header + tests.
lib/components/fabro-interview/src/lib.rs	Adds review_target to runtime Question struct (serde + defaults).
lib/components/fabro-interview/src/console.rs	Prints explicit review target line (label + URL) for TTY and non-TTY console flows + tests.
lib/apps/fabro-server/src/server/tests.rs	Updates server tests to include/expose review_target through endpoints.
lib/apps/fabro-server/src/server.rs	Threads review_target into Slack notifications + API/runtime question projections.
lib/apps/fabro-server/src/demo/mod.rs	Updates demo API questions to include review_target: None.
lib/apps/fabro-cli/src/commands/run/runner.rs	Updates CLI tests for updated question shape.
lib/apps/fabro-cli/src/commands/run/attach.rs	Carries review_target from API to runtime question; prints explicit review target line.
docs/public/workflows/human-in-the-loop.mdx	Documents workflow contract and expected question/link behavior.
docs/public/reference/dot-language.mdx	Documents review_target DOT attribute on human nodes.
docs/public/execution/context.mdx	Documents the review_target context key convention.
docs/public/api-reference/fabro-api.yaml	Adds ReviewTarget* schemas and review_target fields to question shapes.
Cargo.lock	Adds thiserror to the lockfile.
apps/fabro-web/app/components/stage-renderers/human-qa.tsx	Uses ReviewTargetQuestion when a review target is present.
apps/fabro-web/app/components/stage-renderers/helpers.ts	Parses review_target from interview.started events into UI model.
apps/fabro-web/app/components/stage-renderers/helpers.test.ts	Adds test coverage for preserving review_target in parsed interview pairs.
apps/fabro-web/app/components/review-target-question.tsx	New component: safely renders external link for review targets with validation/fallback.
apps/fabro-web/app/components/interview-dock.tsx	Renders dock question text via ReviewTargetQuestion with link when safe.
apps/fabro-web/app/components/interview-dock.test.tsx	Tests safe linking and unsafe-target fallback behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[brynary]

Ran a simplification pass over the branch (reuse, quality, and efficiency reviews) and pushed one refactor commit.

The main finding was duplication: the review question sentence was written in four places and the URL safety rules in three.

• ReviewTarget::question_text_with_link is now the single definition of the wording. question_text() and the Slack header both call it.
• Deleted ReviewTargetKind::noun() — the enum already derives strum::Display with the same output.
• The console interviewer and the CLI attach client shared a byte-identical block; they now call one helper. It prints only the URL, since question.text already has the label and the noun.
• The web check is down to URL scheme, host, and credentials — the parts a raw href can act on. Label length and control characters cannot affect the DOM, so they stay server-side.
• Split validation from presentation in the web UI. safeReviewTarget returns the target or null, so an unsafe target now falls back to the same Markdown rendering as a question with no target, instead of flat text.
• The web UI derives the noun from kind instead of hardcoding "document".
• Dropped deny_unknown_fields from the wire struct. The OpenAPI schema leaves additionalProperties permissive, so adding a field later would have made already-persisted events unreadable.
• Documented that review_target=true replaces the gate's label.

The efficiency review found nothing worth changing: validation costs ~242 ns in Rust and runs about a dozen times per run, and neither web call site is on a hot render path.

Three findings I did not act on, for a separate change:

1. ApiQuestion and InterviewQuestionRecord now have identical fields and semantics, bridged by a field-by-field copy. Folding them together would also remove the 13 meaningless review_target: None test-fixture lines this PR added.
2. Validation runs inside Deserialize, so tightening a rule later would make already-stored events unparseable. Kept as-is because PullRequestLink uses the same pattern; worth deciding for both at once.
3. The URL safety predicate now exists in four spellings across the repo (ReviewTarget, is_safe_slack_link_url, parse_install_upstream_url, and the web check). One shared predicate would collapse three of them.

Verification: 7,411 workspace tests pass, clippy and fmt clean, web typecheck and production build pass. The web suite has 13 pre-existing failures in importChunk, isStaleBuild, and RunFiles — identical with and without this commit.

[Copilot]

Pull request overview

Copilot reviewed 41 out of 42 changed files in this pull request and generated no new comments.

[claude]

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

_{Tip: disable this comment in your organization's Code Review settings.}

[Copilot]

Pull request overview

Copilot reviewed 41 out of 42 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

lib/components/fabro-slack/src/blocks.rs:443

• slack_link() escapes Slack control characters in the label but interpolates the URL verbatim into <url|label>. Slack’s mrkdwn formatting treats & as an escape character, so a valid review target URL with query params (e.g. ...?a=1&b=2) can produce malformed markup / rejected blocks. Escaping the URL with the same escape_slack_controls() used elsewhere keeps the link syntax intact while making & safe.

fn slack_link(url: &str, label: &str) -> String {
    if is_safe_slack_link_url(url) {
        let label = escape_slack_controls(label).replace('|', "/");
        format!("<{url}|{label}>")
    } else {
        escape_slack_controls(label)
    }

[Copilot]

Pull request overview

Copilot reviewed 41 out of 42 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

apps/fabro-web/app/components/stage-renderers/helpers.ts:93

• parseReviewTarget checks kind !== ReviewTargetKind.DOCUMENT, but then returns { label, url, kind } where kind is still typed as a general string (from getString). That’s not safely assignable to the generated ReviewTarget.kind (a string-literal union), and can cause TypeScript to lose the literal type or fail typechecking depending on settings. Prefer returning the literal constant when the check passes.

  const kind = getString(value, "kind");
  if (!label || !url || kind !== ReviewTargetKind.DOCUMENT) return null;
  return { label, url, kind };
}

lib/foundation/fabro-types/src/interview.rs:74

• The doc comment says question_text_with_link is the “single definition” of the review question wording, but the web UI also hardcodes/mirrors this sentence in apps/fabro-web/app/components/review-target-question.tsx. This wording is still canonical server-side, but it’s not literally the only definition in the repo, so the comment is misleading.