checkpoint

⚒️ Generated with [Fabro](https://fabro.sh)
This commit is contained in:
Fabro 2026-05-04 14:54:35 -04:00
parent 6f5fa9f9ef
commit de49710577
8 changed files with 563 additions and 8 deletions

175
run.json

File diff suppressed because one or more lines are too long

View file

@ -0,0 +1,11 @@
{
"stdout": "blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126",
"stderr": "blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126",
"exit_code": 0,
"duration_ms": 133002,
"termination": "exited",
"stdout_bytes": 0,
"stderr_bytes": 0,
"streams_separated": true,
"live_streaming": false
}

View file

@ -0,0 +1,6 @@
{
"outcome": "succeeded",
"notes": "Script completed: cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1",
"failure_reason": null,
"timestamp": "2026-05-04T18:39:22.660548Z"
}

View file

@ -0,0 +1 @@
blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126

View file

@ -0,0 +1 @@
blob://sha256/12ae32cb1ec02d01eda3581b127c1fee3b0dc53572ed6baf239721a03d82e126

View file

@ -0,0 +1,321 @@
Goal: # Compound-engineering PR title and body recipe for Fabro
## Context
Today Fabro's PR body is generated by a hardcoded two-line system prompt at `lib/crates/fabro-workflow/src/pipeline/pull_request.rs:373-377`. The title is derived deterministically from the workflow goal's first line (`pr_title_from_goal`). Compound-engineering's `git-commit-push-pr` skill produces noticeably better PR descriptions because it embeds a sizing matrix, writing principles, a visual-aid table, and the `#1`-as-issue-ref footgun, and it generates the title from change context.
We want to port the *valuable* parts of that recipe into Fabro's existing pipeline while keeping Fabro's signature trailing sections (Plan `<details>`, `### Retro`, `### Fabro Details`, `⚒️ Generated with [Fabro]` footer). Per user direction, the LLM self-classifies the change against the sizing matrix (no Rust-side bucketing), and goal truncation is silent (option A: accept the loss; do not attach the full goal to the rendered body).
## Approach
Replace the plain-text LLM call with a single `generate_object` call returning a structured `{title, body}` JSON object. Embed the compound-engineering recipe in the system prompt with explicit "do not duplicate the trailing sections" guardrails. Raise input truncation caps to fit modern context windows and add a goal cap (previously uncapped).
The trailing programmatic sections are unchanged — `assemble_pr_body` still prepends the LLM body to Plan / Retro / Fabro Details / footer. `pr_title_from_goal` survives as the fallback for the narrow case where structured output succeeded with a usable body but the LLM returned an empty title; every other generation failure remains fatal (run still completes, no PR is opened).
## Critical files
- `lib/crates/fabro-workflow/src/pipeline/pull_request.rs` — primary; replace prompt + signatures
- `lib/crates/fabro-workflow/tests/it/integration.rs:6893` — one test that calls `build_pr_body` directly; signature update
## Reuse (do not reimplement)
- `fabro_llm::generate::generate_object(params, schema)``lib/crates/fabro-llm/src/generate.rs:920-945`. Existing pattern in `fabro-hooks/src/executor.rs:27-37, 315-338` (LazyLock schema + typed deserialize + warn-on-failure). Mirror that pattern.
- `assemble_pr_body`, `format_retro_section`, `format_arc_details_section`, `read_plan_text`, `truncate_pr_body`, `load_pull_request_diff` — unchanged.
- `pr_title_from_goal`, `strip_goal_decoration` — unchanged; demoted from "always-used" to "fallback path."
## Changes
### 1. New schema and typed struct
Add at module top:
```rust
use std::sync::LazyLock;
#[derive(Debug, serde::Deserialize)]
struct GeneratedPrContent {
title: String,
body: String,
}
static PR_CONTENT_SCHEMA: LazyLock<serde_json::Value> = LazyLock::new(|| {
serde_json::json!({
"type": "object",
"properties": {
"title": { "type": "string", "maxLength": 72 },
"body": { "type": "string", "minLength": 1 }
},
"required": ["title", "body"],
"additionalProperties": false
})
});
```
**Schema vs fallback semantics.** Both fields are `required` so a missing field fails structured-output validation (fatal — propagates as Err). The schema does *not* set `minLength` on `title`, so an empty-string title deserializes successfully and is the *only* signal that triggers the deterministic title fallback in `maybe_open_pull_request`. `body` has `minLength: 1` because we have no body fallback — an empty body is fatal too. This narrower fallback promise (empty title only, not missing title) keeps the schema strict without making the fallback dead code.
### 2. New system prompt as a `const &str`
Add as a module-level `const PR_BODY_SYSTEM_PROMPT: &str = "..."`. Verbatim content:
```text
You are writing a pull request title and description for a code change produced by an AI workflow.
OUTPUT FORMAT
Return a JSON object with exactly two fields:
- "title": a one-line title, max 72 characters, no trailing period.
- "body": the markdown body as described below.
DO NOT INCLUDE in the body
- A `#` or `##` title heading at the top — the title goes in the `title` field.
- A "Retro" section, "Fabro Details" section, cost/duration table, or "Generated with" footer — those are appended programmatically after your output.
- The full plan text — the full plan is appended programmatically as a <details> block.
- Bare `#1`, `#2` list prefixes — GitHub auto-links those as issue references. Use plain `1.`, `2.` instead.
- A test plan unless the testing approach is non-obvious.
SIZE THE BODY TO THE CHANGE
First classify along two axes from the diff:
- Size: how many files changed, how large the diff is.
- Complexity: trivial (rename / typo / dep bump / config) vs. design decisions / new patterns / cross-cutting concerns.
Then write at the matching depth:
| Profile | Body shape |
|---|---|
| Small + simple (typo, config, dep bump) | 12 sentences, no headers, total under ~300 characters |
| Small + non-trivial (targeted bugfix, behavioral change) | Short "Problem / Fix" narrative, 35 sentences. No headers unless two distinct concerns. |
| Medium feature or refactor | Summary paragraph, then a section explaining what changed and why. Call out design decisions. |
| Large or architecturally significant | Full narrative: problem context, approach chosen (and why), key decisions, migration/rollback notes if relevant. |
| Performance improvement | Include before/after measurements if available. A markdown table works well here. |
Brevity matters for small changes. A 3-line bugfix with a 20-line description signals miscalibration. When in doubt, shorter is better — reviewers can read the diff.
WRITING PRINCIPLES
- Lead with value: the first sentence tells the reviewer *why this PR exists*, not *what files changed*.
- Describe the net result, not the journey: skip intermediate failures, debugging steps, and refactors done during development.
- Trust the final diff: if the goal or plan disagree with the diff, the diff is authoritative.
- Explain the non-obvious: spend description space on what the diff doesn't show — why this approach, what was rejected, what to look at first.
- Use structure when it earns its keep: no empty sections, no template headers without content.
- If the body uses any `##` heading, the opening summary must also be under a heading (e.g. `## Summary`); otherwise a bare paragraph is fine.
PLAN SUMMARY
The full plan is attached separately as a <details> block, so do not restate it. Include a brief `### Plan Summary` with bullet points only when the change is medium or larger in the sizing matrix above. Skip it for small changes.
VISUAL AIDS
Include a visual aid only when a reviewer would struggle to reconstruct the mental model from prose alone — based on what changes structurally, not on PR size. Skip for trivial / mechanical changes, or when prose already communicates clearly.
| PR changes... | Visual aid |
|---|---|
| 3+ interacting components or services | Mermaid component / interaction diagram |
| Multi-step workflow or pipeline with non-obvious sequencing | Mermaid flow diagram |
| 3+ behavioral modes or variants | Markdown comparison table |
| Before/after data or trade-offs | Markdown table |
| Data model changes with 3+ related entities | Mermaid ERD |
Mermaid: prefer `TB` direction, ≤10 nodes typical. Place inline at the point of relevance, not in a separate "Diagrams" section.
```
### 3. Truncation constants and small-model fallback
Replace inline `50_000` and `20_000` with two tiered constant sets:
```rust
// Generous tier (≥200k context window)
const MAX_GOAL_CHARS_LARGE: usize = 75_000;
const MAX_PLAN_CHARS_LARGE: usize = 75_000;
const MAX_DIFF_CHARS_LARGE: usize = 250_000;
// Conservative tier (matches the previous values)
const MAX_GOAL_CHARS_SMALL: usize = 20_000;
const MAX_PLAN_CHARS_SMALL: usize = 20_000;
const MAX_DIFF_CHARS_SMALL: usize = 50_000;
```
Resolve the tier by looking up the model in `fabro_model::Catalog::builtin()`:
```rust
fn truncation_caps(model: &str) -> (usize, usize, usize) {
let large_enough = Catalog::builtin()
.get(model)
.is_some_and(|m| m.limits.context_window >= 200_000);
if large_enough {
(MAX_GOAL_CHARS_LARGE, MAX_PLAN_CHARS_LARGE, MAX_DIFF_CHARS_LARGE)
} else {
(MAX_GOAL_CHARS_SMALL, MAX_PLAN_CHARS_SMALL, MAX_DIFF_CHARS_SMALL)
}
}
```
Unknown models (`get` returns `None`) fall through to the conservative tier — safer than assuming large context for an unrecognized id. `fabro_model::Catalog` is already a transitive dep of `fabro-workflow` via `start.rs`'s use of `fabro_model::Provider`; no new Cargo entry needed.
For the large tier, worst-case input is ~400k chars ≈ 100k tokens. Fits in 200k-context Sonnet/Haiku with ~50k-token headroom for system prompt + output. Comfortable on 1M-context models. The conservative tier preserves today's behavior for the two ≤131k-context models in the catalog (`gpt-5.3-codex-spark`, `mercury-2`).
Goal truncation is silent (no log, no marker). The full goal is *not* attached to the rendered body.
### 4. Refactor `build_pr_body*` to return both title and body
Signature change for the three internal builders and the public `build_pr_body`:
```rust
pub async fn build_pr_body(
diff: &str, goal: &str, model: &str,
run_store: &RunStoreHandle,
llm_source: &dyn CredentialSource,
conclusion: Option<&Conclusion>,
) -> Result<(String, String), String> // was: Result<String, String>
```
Returns `(title, body)`. Callers destructure.
`build_pr_body_with_client_and_state` becomes:
1. Truncate `goal`, `diff`, `plan_text` against the tier from `truncation_caps(model)`.
2. Build user prompt with `Goal:` / `Plan:` (when present) / `Diff:` sections.
3. Call `generate_object(params.system(PR_BODY_SYSTEM_PROMPT).prompt(user_prompt), PR_CONTENT_SCHEMA.clone())`.
4. On success: deserialize `result.output` as `GeneratedPrContent`, trim title, enforce 72-char cap via `enforce_title_cap` (truncate at `floor_char_boundary` + `…` if exceeded — same pattern as `pr_title_from_goal`), keep body as-is.
5. Pass body through `assemble_pr_body(llm_body, plan_text, retro_section, arc_details_section)` — unchanged. Return `(title, assembled_body)` where `title` may be the empty string after trimming.
**Failure modes — explicit:**
| Condition | Outcome |
|---|---|
| `generate_object` returns Err (LLM call failure, schema validation failure, JSON parse failure) | Return Err. Caller logs and emits `PullRequestFailed`; no PR is opened. |
| `result.output` is `None` | Return Err. Same as above. |
| Deserialize as `GeneratedPrContent` fails (missing required field, wrong type) | Return Err. Same as above. |
| Body is blank (`generated.body.trim().is_empty()`) | Return Err. Schema's `minLength: 1` rejects truly empty strings, but a body of `" \n"` would pass the schema; the Rust trim-check catches whitespace-only bodies as well. The body content is preserved as-is once it passes the check — no leading/trailing whitespace stripping, since markdown can rely on it. |
| Title is empty (after trim) | **Allowed through** — return `("", body)`. The caller is responsible for the deterministic title fallback. |
| Title is non-empty but >72 chars | Allowed through — `enforce_title_cap` truncates with `…`. |
The narrower fallback promise: only an empty/whitespace title triggers the deterministic title fallback. Every other error path is fatal. This makes the fallback path actually reachable from the caller and prevents the bug where structured-output failure swallows both fields and there's no body to use anyway.
### 5. `maybe_open_pull_request` invokes the fallback when the title is empty
Today it calls `pr_title_from_goal(req.goal)` unconditionally. New flow — the deterministic fallback only fires when the LLM returned a body but no usable title:
```rust
let (llm_title, body) = build_pr_body_with_source_and_state(...).await
.map_err(|err| format!("{err:#}"))?; // any non-title failure: fatal
let title = if llm_title.trim().is_empty() {
pr_title_from_goal(req.goal) // fallback path: only reached when body succeeded
} else {
llm_title
};
let title = enforce_title_cap(&title); // unconditional 72-char guarantee, covers fallback path
let body = truncate_pr_body(&body); // unchanged 65,536-char hard cap
```
The unconditional `enforce_title_cap` after fallback selection is load-bearing: `pr_title_from_goal`'s built-in cap is 120 chars (a holdover from the previous deterministic-only flow), so without re-capping here a long goal would breach the new 72-char contract. Don't lower `pr_title_from_goal`'s internal cap — keep the cap enforcement in one place at the caller, where it covers both branches.
The pipeline stage at `pull_request.rs:538-623` already converts the propagated Err into a `PullRequestFailed` event without aborting the run, so there is no behavior change for callers when generation fails fully.
### 6. Title cap helper
Add a small private helper:
```rust
fn enforce_title_cap(title: &str) -> String {
const MAX: usize = 72;
if title.chars().count() > MAX {
let truncated: String = title.chars().take(MAX - 1).collect();
format!("{truncated}\u{2026}")
} else {
title.to_string()
}
}
```
Applied inside `build_pr_body_with_client_and_state` to the LLM-returned title before returning. `pr_title_from_goal`'s existing 120-char cap stays as-is (it's the fallback and the goal-derived title is shorter in practice; introducing a 72-char cap there is a separate, low-value change).
### 7. Test updates
In `lib/crates/fabro-workflow/src/pipeline/pull_request.rs`:
- `MockProvider::complete` and `::stream` currently return plain text. Update to return JSON matching the schema: `{"title":"Mock title","body":"Narrative from mock."}`. Both call sites (`response_text` field) get this JSON string.
- `openai_responses_payload` helper (line ~761) returns a JSON-shaped fake API response; update its `text` to be the JSON string `{"title":"…","body":"Narrative from vault source."}`.
- `build_pr_body_uses_in_memory_conclusion`, `build_pr_body_uses_store_records_without_legacy_files`, `build_pr_body_uses_plan_text_from_store_without_response_md`, `build_pr_body_uses_explicit_llm_client`, `build_pr_body_uses_vault_only_openai_codex_source` — destructure the new tuple, assert on title and body separately. Existing body assertions (`contains("Narrative from mock.")` etc.) become body-side; add a `title == "Mock title"` assertion to one of them.
- `empty_diff_returns_none` — unchanged behavior, still returns `Ok(None)` from `maybe_open_pull_request`.
- `pr_title_from_goal` tests (10 of them, lines 14161485) — unchanged; the function still exists as a fallback.
- New test: `maybe_open_pull_request_falls_back_to_goal_title_when_llm_returns_empty_title` — exercises the actual fallback branch in `maybe_open_pull_request`, not just the builder. **`MockProvider` cannot drive this path**: `maybe_open_pull_request``build_pr_body_with_source_and_state``Client::from_source(llm_source)` builds a real provider client from the credential source, bypassing any in-process provider injection. Use a real provider HTTP mock instead. Setup:
- **OpenAI mock** (mirrors `build_pr_body_uses_vault_only_openai_codex_source` at `pull_request.rs:1322`): start an `httpmock::MockServer`, register `POST /v1/responses` with `Authorization: Bearer vault-openai-key`, return `openai_responses_payload(r#"{"title":"","body":"Narrative."}"#)`. The existing helper wraps any string into the OpenAI Responses-API envelope; here that string is the structured-output JSON.
- **Credential source**: load a `Vault` with an `openai_codex` API-key credential of `vault-openai-key`, wrap it in `VaultCredentialSource::with_env_lookup` returning the OpenAI mock's `/v1` URL for `OPENAI_BASE_URL`. Use this `Arc<dyn CredentialSource>` as the `llm_source`.
- **GitHub mock**: a separate `httpmock::MockServer` exposing only `POST /repos/{owner}/{repo}/pulls`, returning a valid PR JSON (e.g. `{ "number": 1, "html_url": "...", "node_id": "..." }` with a 201 status).
- **GitHub credentials**: `fabro_github::GitHubCredentials::Token("test-token".to_string())` rather than the App variant. App credentials would force this test to also mock JWT signing and the installation-token exchange; Token credentials let `create_pull_request` go straight to the PR endpoint with a static `Authorization: Bearer test-token` header. (Implementer: confirm the variant name in `lib/crates/fabro-github/src/lib.rs`; if it's `Pat` or similar instead of `Token`, use that.)
- Pass `&github_server.url("")` as the second arg to `GitHubContext::new(&creds, ...)` so the mock's base URL replaces the production `github_api_base_url()`.
- Pass a non-empty diff so the early-return at `pull_request.rs:460` doesn't fire.
- Pass a goal like `"Fix telemetry leak\n\ndetails..."`. Use a `model` of `"gpt-5.4"` (catalog hit, large-tier truncation, matches the OpenAI mock).
- Pass a `RunStoreHandle` whose `state().final_patch` returns a non-empty diff (mirror the `load_pull_request_diff_uses_store_without_disk_patch` test at line 1521 for the event-append pattern).
- Assert: `PullRequestRecord.title == "Fix telemetry leak"`, the OpenAI mock fired exactly once, the GitHub mock fired exactly once with `Authorization: Bearer test-token`. Optionally inspect the captured GitHub request body to confirm the title sent on the wire matches.
- New test: `maybe_open_pull_request_caps_fallback_title_at_72_chars` — guards the unconditional `enforce_title_cap` in §5. Same harness as the test above (extract a private `setup_fallback_test_harness()` helper to avoid duplicating the OpenAI + GitHub + Vault wiring). Differences: pass a goal that's a single ~200-char line with no newlines and no `Plan:` prefix, so `pr_title_from_goal` returns close to its 120-char cap. The OpenAI mock returns `{"title":"","body":"Narrative."}` so the fallback fires. Assert `PullRequestRecord.title.chars().count() == 72` and the title ends with `…`.
- New test: `build_pr_body_truncates_long_title` — MockProvider returns `{"title":"x".repeat(200),"body":"…"}`; assert the title returned from `build_pr_body` is exactly 72 chars and ends with `…`. (This one stays at the builder level — it's testing `enforce_title_cap`, not the fallback.)
- New test: `build_pr_body_returns_err_when_body_blank` — two cases via parameterized assertion or two `#[test]`s sharing a helper:
- MockProvider returns `{"title":"Mock","body":""}` — schema rejects via `minLength: 1`, builder returns `Err`.
- MockProvider returns `{"title":"Mock","body":" \n"}` — schema accepts (length ≥ 1), Rust trim-check returns `Err`.
Both cases validate the blank-body-is-fatal contract from §4's failure-mode table.
In `lib/crates/fabro-workflow/tests/it/integration.rs`:
- `workflow_run_with_vault_only_openai_codex_builds_pr_body` (line 6893 area) — destructure the tuple, update assertions.
### 8. What is *not* changing
- `assemble_pr_body` — body still slots in front of the four programmatic sections in the same order.
- `format_retro_section`, `format_arc_details_section`, `read_plan_text`, `parse_dot_summary`, `format_duration_ms`, `format_cost`, `truncate_pr_body` — unchanged.
- `OpenPullRequestRequest`, `PullRequestRecord`, `AutoMergeOptions` — unchanged shapes.
- The two production callers (`fabro-server/src/server/handler/pull_requests.rs:262-277` and the `pull_request` pipeline stage at `pull_request.rs:573`) — unchanged. Both go through `maybe_open_pull_request`, which absorbs the new title flow internally.
- The 65,536-char body cap and `_(truncated)_` suffix.
## Verification
```sh
cargo build --workspace
cargo nextest run -p fabro-workflow
cargo nextest run -p fabro-server
cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings
cargo +nightly-2026-04-14 fmt --check --all
```
End-to-end (optional, requires credentials):
```sh
set -a && source .env && set +a
cargo nextest run -p fabro-workflow --profile e2e --run-ignored only
```
Manual smoke: run any workflow that opens a PR and inspect the resulting PR title and body on GitHub. Confirm:
- Title ≤ 72 chars, no markdown decoration.
- Body opens with a value-first sentence (per "lead with value" principle).
- Body does *not* contain a duplicate Retro / Fabro Details / footer / full-plan section.
- Trailing sections render as before: Plan `<details>`, `### Retro`, `### Fabro Details`, `⚒️ Generated with [Fabro]`.
- For a small mechanical change, the body is short (sizing matrix small+simple); for a multi-stage feature run, the body is multi-paragraph with sections.
## Out of scope (explicit)
- Fully dynamic truncation budgeting (`min(MAX_DIFF_CHARS, ctx_window / 4)`). The plan uses a two-tier static lookup keyed on a 200k threshold instead — simpler, addresses the small-context-model risk, and avoids tokenizer math.
- Stage-response inclusion (e.g. the implement node's response as commit-message analog). Worth doing later as a separate change.
- Configurable `pull_request.prompt_preset` or `prompt_override` in `PullRequestSettings`. Not needed for this iteration; the prompt is opinionated by design.
- Separate `pull_request.model` override (use Haiku/mini for body generation to cut cost). Defensible follow-up; out of scope here.
- Attaching the full goal as a `<details>` block (option B). User chose option A.
- Anthropic probe override to keep connectivity checks on Haiku after the Opus 4.7 default change. Separate concern, surfaced earlier.
## Completed stages
- **toolchain**: succeeded
- Script: `command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1`
- Stdout:
```
cargo 1.95.0 (f2d3ce0bd 2026-03-21)
```
- Stderr: (empty)
- **preflight_compile**: succeeded
- Script: `cargo check -q --workspace 2>&1`
- Stdout: (empty)
- Stderr: (empty)
- **preflight_lint**: succeeded
- Script: `cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1`
- Stdout: (empty)
- Stderr: (empty)
Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD.

View file

@ -0,0 +1,5 @@
{
"mode": "agent",
"provider": "anthropic",
"model": "claude-opus-4-7"
}

View file

@ -0,0 +1,51 @@
Everything is green. Let me summarize what was done:
## Summary
Implemented the compound-engineering PR title and body recipe per plan:
### Code changes (`lib/crates/fabro-workflow/src/pipeline/pull_request.rs`)
1. **Schema + typed struct** — Added `PR_CONTENT_SCHEMA` (`LazyLock<serde_json::Value>`) with required `title` (max 72) and `body` (min 1), and `GeneratedPrContent { title, body }` deserialized from structured output. Strict (no `additionalProperties`).
2. **System prompt** — Added `PR_BODY_SYSTEM_PROMPT` constant containing the verbatim recipe from the plan: output format, "do not include" guardrails, sizing matrix, writing principles, plan-summary rule, visual-aid table.
3. **Truncation tiers** — Added six constants and `truncation_caps(model)` that selects the large tier (75k/75k/250k) when `Catalog::builtin().get(model).limits.context_window >= 200_000`, else the conservative tier (20k/20k/50k matching the previous values). Added `truncate_chars` helper for char-boundary-safe slicing.
4. **`enforce_title_cap(&str) -> String`** — 72-char Unicode-scalar cap, replacing the trailing char with `…`.
5. **Refactored builders**`build_pr_body`, `build_pr_body_with_client`, `build_pr_body_with_source_and_state`, `build_pr_body_with_client_and_state` now return `Result<(String, String), String>`. The inner builder switches `generate``generate_object`, deserializes to `GeneratedPrContent`, applies the failure-mode table from the plan: missing/invalid output → `Err`, blank/whitespace body → `Err`, empty title → passes through unchanged (caller handles fallback), oversized title → capped via `enforce_title_cap`. The body is unchanged from the LLM and assembled normally with the trailing programmatic sections.
6. **`maybe_open_pull_request`** — Destructures `(llm_title, body)`. If `llm_title.trim().is_empty()`, falls back to `pr_title_from_goal(req.goal)`; otherwise uses the LLM title. Then `enforce_title_cap` runs unconditionally (covers both branches), so the fallback path can never breach the 72-char contract that the LLM path satisfies via the schema.
### Tests added (red→green)
- `build_pr_body_truncates_long_title` — MockProvider returns 200-char title; assert `chars().count() == 72` and ends with `…`.
- `build_pr_body_returns_err_when_body_empty` — schema rejects `""` (minLength 1).
- `build_pr_body_returns_err_when_body_whitespace` — schema accepts `" \n"`, but the Rust trim-check rejects it; asserts the specific error message.
- `setup_fallback_test_harness` — shared helper standing up an `httpmock::MockServer` for OpenAI (`POST /v1/responses`, bearer `vault-openai-key`), a separate mock for GitHub (`POST /repos/owner/repo/pulls`, bearer `test-token`), a `Vault`-backed `VaultCredentialSource` with `OPENAI_BASE_URL` env override, `GitHubCredentials::Token("test-token")`, and a `RunStoreHandle` seeded with `WorkflowRunCompleted { final_patch: Some(...) }` so `load_pull_request_diff` returns content. The `_openai_server` field is held to keep the mock listener alive.
- `maybe_open_pull_request_falls_back_to_goal_title_when_llm_returns_empty_title` — OpenAI mock returns `{"title":"","body":"Narrative."}`, goal `"Fix telemetry leak\n\ndetails..."`; asserts `record.title == "Fix telemetry leak"`.
- `maybe_open_pull_request_caps_fallback_title_at_72_chars` — same harness, 200-char goal forces `pr_title_from_goal` to return ~120 chars; asserts the final title is exactly 72 chars and ends with `…`, proving the load-bearing unconditional `enforce_title_cap` after fallback selection.
### Existing tests updated
- All 5 existing `build_pr_body_*` tests in `pull_request.rs` now destructure `(title, body)` and route their MockProvider/openai responses through the new `pr_content_json(title, body)` helper. `build_pr_body_uses_in_memory_conclusion` and `build_pr_body_uses_vault_only_openai_codex_source` additionally assert on title equality.
- `lib/crates/fabro-workflow/tests/it/integration.rs:6893` (`workflow_run_with_vault_only_openai_codex_builds_pr_body`) — updated to destructure the tuple, return structured-output JSON from the OpenAI mock, and assert `title == "Vault title"`.
- `lib/crates/fabro-server/src/server/tests.rs:3668` (`create_run_pull_request_creates_and_persists_record`) — updated the OpenAI responses mock to return structured-output JSON instead of plain text.
### Verification
- `cargo build --workspace`
- `cargo nextest run -p fabro-workflow` ✅ (1057/1057 pass, 34 skipped)
- `cargo nextest run -p fabro-server` ✅ (441/443 pass; the two `*_returns_svg` failures are pre-existing and unrelated — confirmed by reproducing them on the parent commit `dc09842a`)
- `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings`
- `cargo +nightly-2026-04-14 fmt --check --all`
### Out of scope items (per plan) — not changed
- No `pull_request.prompt_preset`/`prompt_override` config.
- No separate `pull_request.model` override.
- No dynamic budgeting based on tokenizer math (two-tier static lookup as planned).
- No stage-response inclusion.
- Goal truncation is silent — full goal is *not* attached as a `<details>` block (option A).
- `assemble_pr_body`, `format_retro_section`, `format_arc_details_section`, `read_plan_text`, `truncate_pr_body`, `load_pull_request_diff`, `pr_title_from_goal` (with its 120-char cap), `OpenPullRequestRequest`, `PullRequestRecord`, `AutoMergeOptions`, the 65,536-char body cap, and the `_(truncated)_` suffix are unchanged.