checkpoint

⚒️ Generated with [Fabro](https://fabro.sh)

stages/005-implement@1/diff.patch

@@ -0,0 +1,779 @@
+diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs
+index 6078c362..91dcdaae 100644
+--- a/lib/crates/fabro-server/src/server/tests.rs
++++ b/lib/crates/fabro-server/src/server/tests.rs
+@@ -3665,7 +3665,13 @@ async fn create_run_pull_request_creates_and_persists_record() {
+                 .header("authorization", "Bearer openai-key");
+             then.status(200)
+                 .header("content-type", "application/json")
+-                .json_body(openai_responses_payload("Narrative from mock."));
++                .json_body(openai_responses_payload(
++                    &serde_json::to_string(&json!({
++                        "title": "Mock title",
++                        "body": "Narrative from mock.",
++                    }))
++                    .unwrap(),
++                ));
+         })
+         .await;
+     let openai_base_url = llm.url("/v1");
+diff --git a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs
+index 5b9355cc..1fb784a4 100644
+--- a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs
++++ b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs
+@@ -1,10 +1,11 @@
+-use std::sync::Arc;
++use std::sync::{Arc, LazyLock};
+ 
+ use fabro_auth::CredentialSource;
+ use fabro_github::{self as github_app, ssh_url_to_https};
+ use fabro_graphviz::parser;
+ use fabro_llm::client::Client;
+-use fabro_llm::generate::{GenerateParams, generate};
++use fabro_llm::generate::{GenerateParams, generate_object};
++use fabro_model::Catalog;
+ use fabro_retro::retro::Retro;
+ use fabro_store::RunProjection;
+ use fabro_types::PullRequestRecord;
+@@ -12,6 +13,141 @@ use fabro_types::settings::run::MergeStrategy;
+ use fabro_util::text::strip_goal_decoration;
+ use tracing::{debug, info};
+ 
++/// Structured output schema for the LLM-generated PR title and body.
++///
++/// `title` is required but allows empty strings (the only signal that
++/// triggers the deterministic title fallback in
++/// [`maybe_open_pull_request`]). `body` requires `minLength: 1` because
++/// there is no body fallback — an empty body is fatal.
++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
++    })
++});
++
++#[derive(Debug, serde::Deserialize)]
++struct GeneratedPrContent {
++    title: String,
++    body:  String,
++}
++
++/// System prompt that instructs the LLM how to write a Fabro PR title and
++/// body. The trailing programmatic sections (Plan `<details>`, Retro,
++/// Fabro Details, footer) are appended after the LLM body — the prompt
++/// explicitly forbids the LLM from duplicating them.
++const PR_BODY_SYSTEM_PROMPT: &str = "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) | 1–2 sentences, no headers, total under ~300 characters |
++| Small + non-trivial (targeted bugfix, behavioral change) | Short \"Problem / Fix\" narrative, 3–5 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.";
++
++// 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 truncation caps for `(goal, plan, diff)` based on the model's
++/// context window. Unknown models fall through to the conservative tier.
++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,
++        )
++    }
++}
++
++/// Truncate `s` to at most `max` chars on a UTF-8 char boundary.
++fn truncate_chars(s: &str, max: usize) -> &str {
++    if s.len() > max {
++        &s[..s.floor_char_boundary(max)]
++    } else {
++        s
++    }
++}
++
++/// Cap a title at 72 chars, replacing the trailing char with `…` when
++/// truncation occurs. Counts Unicode scalar values, not bytes.
++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()
++    }
++}
++
+ use super::types::{Concluded, Finalized, PullRequestOptions};
+ use crate::event::{Event, RunNoticeLevel};
+ use crate::outcome::{StageOutcome, format_cost as outcome_format_cost};
+@@ -286,8 +422,13 @@ async fn load_pull_request_diff(run_store: &RunStoreHandle) -> String {
+         .unwrap_or_default()
+ }
+ 
+-/// Build a complete PR body by combining LLM-generated narrative with
+-/// programmatic sections (plan, retro, fabro details).
++/// Build a complete PR title and body by combining LLM-generated narrative
++/// with programmatic sections (plan, retro, fabro details).
++///
++/// Returns `(title, body)`. The title may be the empty string when the LLM
++/// returns a usable body but no usable title — callers are responsible for
++/// the deterministic title fallback in that case. Every other generation
++/// failure is surfaced as `Err`.
+ pub async fn build_pr_body(
+     diff: &str,
+     goal: &str,
+@@ -295,7 +436,7 @@ pub async fn build_pr_body(
+     run_store: &RunStoreHandle,
+     llm_source: &dyn CredentialSource,
+     conclusion: Option<&Conclusion>,
+-) -> Result<String, String> {
++) -> Result<(String, String), String> {
+     let client = Client::from_source(llm_source)
+         .await
+         .map_err(|e| format!("Failed to create LLM client: {e}"))?;
+@@ -310,7 +451,7 @@ async fn build_pr_body_with_client(
+     run_store: &RunStoreHandle,
+     conclusion: Option<&Conclusion>,
+     client: Arc<Client>,
+-) -> Result<String, String> {
++) -> Result<(String, String), String> {
+     build_pr_body_with_client_and_state(diff, goal, model, run_store, conclusion, client, None)
+         .await
+ }
+@@ -323,7 +464,7 @@ async fn build_pr_body_with_source_and_state(
+     llm_source: &dyn CredentialSource,
+     conclusion: Option<&Conclusion>,
+     run_state: Option<&fabro_store::RunProjection>,
+-) -> Result<String, String> {
++) -> Result<(String, String), String> {
+     let client = Client::from_source(llm_source)
+         .await
+         .map_err(|e| format!("Failed to create LLM client: {e}"))?;
+@@ -348,7 +489,7 @@ async fn build_pr_body_with_client_and_state(
+     conclusion: Option<&Conclusion>,
+     client: Arc<Client>,
+     run_state: Option<&fabro_store::RunProjection>,
+-) -> Result<String, String> {
++) -> Result<(String, String), String> {
+     info!("Building PR body");
+ 
+     let loaded_run_state = if run_state.is_none() {
+@@ -369,45 +510,39 @@ async fn build_pr_body_with_client_and_state(
+     let run_spec = run_state.and_then(|state| state.spec.clone());
+     let dot_source = run_state.and_then(|state| state.graph_source.clone());
+ 
+-    // Build LLM prompt
+-    let system = if plan_text.is_some() {
+-        "Write a PR description with: (1) 2-3 concise paragraphs explaining the change, then (2) a '### Plan Summary' section with bullet points summarizing the plan. Do not include a title. Do not include the full plan.".to_string()
+-    } else {
+-        "Write a concise PR description in 2-3 paragraphs explaining the change. Do not include a title.".to_string()
+-    };
+-
+-    // Truncate diff to fit context windows (~50k chars)
+-    let max_diff_len = 50_000;
+-    let truncated_diff = if diff.len() > max_diff_len {
+-        &diff[..diff.floor_char_boundary(max_diff_len)]
+-    } else {
+-        diff
+-    };
++    let (max_goal, max_plan, max_diff) = truncation_caps(model);
++    let truncated_goal = truncate_chars(goal, max_goal);
++    let truncated_diff = truncate_chars(diff, max_diff);
+ 
+     let prompt = if let Some(ref plan) = plan_text {
+-        // Truncate plan for LLM context (~20k chars)
+-        let max_plan_len = 20_000;
+-        let truncated_plan = if plan.len() > max_plan_len {
+-            &plan[..plan.floor_char_boundary(max_plan_len)]
+-        } else {
+-            plan.as_str()
+-        };
++        let truncated_plan = truncate_chars(plan, max_plan);
+         format!(
+-            "Goal: {goal}\n\nPlan:\n```\n{truncated_plan}\n```\n\nDiff:\n```\n{truncated_diff}\n```"
++            "Goal: {truncated_goal}\n\nPlan:\n```\n{truncated_plan}\n```\n\nDiff:\n```\n{truncated_diff}\n```"
+         )
+     } else {
+-        format!("Goal: {goal}\n\nDiff:\n```\n{truncated_diff}\n```")
++        format!("Goal: {truncated_goal}\n\nDiff:\n```\n{truncated_diff}\n```")
+     };
+ 
+     let params = GenerateParams::new(model, client)
+-        .system(system)
++        .system(PR_BODY_SYSTEM_PROMPT)
+         .prompt(prompt);
+ 
+-    let result = generate(params)
++    let result = generate_object(params, PR_CONTENT_SCHEMA.clone())
+         .await
+         .map_err(|e| format!("LLM generation failed: {e}"))?;
+ 
+-    let llm_output = result.response.text();
++    let output = result
++        .output
++        .ok_or_else(|| "LLM generation returned no structured output".to_string())?;
++    let generated: GeneratedPrContent = serde_json::from_value(output)
++        .map_err(|e| format!("Failed to deserialize PR content: {e}"))?;
++
++    if generated.body.trim().is_empty() {
++        return Err("LLM generated an empty PR body".to_string());
++    }
++
++    let title = enforce_title_cap(generated.title.trim());
++    let llm_body = generated.body;
+ 
+     let retro_section = retro.as_ref().map(format_retro_section).unwrap_or_default();
+     let arc_details_section = conclusion
+@@ -416,7 +551,7 @@ async fn build_pr_body_with_client_and_state(
+         .unwrap_or_default();
+ 
+     let body = assemble_pr_body(
+-        &llm_output,
++        &llm_body,
+         plan_text.as_deref(),
+         &retro_section,
+         &arc_details_section,
+@@ -424,7 +559,7 @@ async fn build_pr_body_with_client_and_state(
+ 
+     info!("PR body generated");
+ 
+-    Ok(body)
++    Ok((title, body))
+ }
+ 
+ /// Auto-merge configuration for a pull request.
+@@ -465,7 +600,7 @@ pub async fn maybe_open_pull_request(
+     let (owner, repo) =
+         github_app::parse_github_owner_repo(&https_url).map_err(|err| format!("{err:#}"))?;
+ 
+-    let body = build_pr_body_with_source_and_state(
++    let (llm_title, body) = build_pr_body_with_source_and_state(
+         req.diff,
+         req.goal,
+         req.model,
+@@ -478,7 +613,12 @@ pub async fn maybe_open_pull_request(
+     .map_err(|err| format!("{err:#}"))?;
+     let body = truncate_pr_body(&body);
+ 
+-    let title = pr_title_from_goal(req.goal);
++    let title = if llm_title.trim().is_empty() {
++        pr_title_from_goal(req.goal)
++    } else {
++        llm_title
++    };
++    let title = enforce_title_cap(&title);
+ 
+     let created = github_app::create_pull_request(
+         &req.github,
+@@ -782,6 +922,16 @@ mod tests {
+         })
+     }
+ 
++    /// JSON string the MockProvider/openai mock returns to simulate the
++    /// structured-output response for `(title, body)`.
++    fn pr_content_json(title: &str, body: &str) -> String {
++        serde_json::to_string(&serde_json::json!({
++            "title": title,
++            "body": body,
++        }))
++        .unwrap()
++    }
++
+     fn make_test_conclusion() -> Conclusion {
+         Conclusion {
+             timestamp:            Utc::now(),
+@@ -1126,17 +1276,21 @@ mod tests {
+     async fn build_pr_body_uses_in_memory_conclusion() {
+         let store = test_store();
+         let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
+-        let body = build_pr_body_with_client(
++        let (title, body) = build_pr_body_with_client(
+             "diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
+             "Implement feature",
+             "mock-model",
+             &run_store.clone().into(),
+             Some(&make_test_conclusion()),
+-            explicit_client("mock", "Narrative from mock."),
++            explicit_client(
++                "mock",
++                &pr_content_json("Mock title", "Narrative from mock."),
++            ),
+         )
+         .await
+         .unwrap();
+ 
++        assert_eq!(title, "Mock title");
+         assert!(body.contains("Narrative from mock."));
+         assert!(body.contains("### Fabro Details"));
+         assert!(body.contains("Ran 3 stages in 2m 30s for $0.42"));
+@@ -1196,13 +1350,16 @@ mod tests {
+         .await
+         .unwrap();
+ 
+-        let body = build_pr_body_with_client(
++        let (_title, body) = build_pr_body_with_client(
+             "diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
+             "Implement feature",
+             "mock-model",
+             &run_store.clone().into(),
+             Some(&make_test_conclusion()),
+-            explicit_client("mock", "Narrative from mock."),
++            explicit_client(
++                "mock",
++                &pr_content_json("Mock title", "Narrative from mock."),
++            ),
+         )
+         .await
+         .unwrap();
+@@ -1283,13 +1440,16 @@ mod tests {
+         .await
+         .unwrap();
+ 
+-        let body = build_pr_body_with_client(
++        let (_title, body) = build_pr_body_with_client(
+             "diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
+             "Implement feature",
+             "mock-model",
+             &run_store.clone().into(),
+             Some(&make_test_conclusion()),
+-            explicit_client("mock", "Narrative from mock."),
++            explicit_client(
++                "mock",
++                &pr_content_json("Mock title", "Narrative from mock."),
++            ),
+         )
+         .await
+         .unwrap();
+@@ -1302,13 +1462,16 @@ mod tests {
+     async fn build_pr_body_uses_explicit_llm_client() {
+         let store = test_store();
+         let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
+-        let body = build_pr_body_with_client(
++        let (_title, body) = build_pr_body_with_client(
+             "diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
+             "Implement feature",
+             "gpt-5.4",
+             &run_store.clone().into(),
+             Some(&make_test_conclusion()),
+-            explicit_client("openai", "Narrative from explicit client."),
++            explicit_client(
++                "openai",
++                &pr_content_json("Explicit title", "Narrative from explicit client."),
++            ),
+         )
+         .await
+         .unwrap();
+@@ -1327,7 +1490,10 @@ mod tests {
+                     .header("authorization", "Bearer vault-openai-key");
+                 then.status(200)
+                     .header("content-type", "application/json")
+-                    .json_body(openai_responses_payload("Narrative from vault source."));
++                    .json_body(openai_responses_payload(&pr_content_json(
++                        "Vault title",
++                        "Narrative from vault source.",
++                    )));
+             })
+             .await;
+ 
+@@ -1355,7 +1521,7 @@ mod tests {
+         let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
+         let run_store_handle: RunStoreHandle = run_store.into();
+ 
+-        let body = build_pr_body(
++        let (title, body) = build_pr_body(
+             "diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
+             "Implement feature",
+             "gpt-5.4",
+@@ -1366,6 +1532,7 @@ mod tests {
+         .await
+         .unwrap();
+ 
++        assert_eq!(title, "Vault title");
+         assert!(body.contains("Narrative from vault source."));
+         response_mock.assert_async().await;
+     }
+@@ -1575,4 +1742,277 @@ mod tests {
+ 
+         assert!(diff.contains("from_store"));
+     }
++
++    // ── Structured-output PR content tests ──────────────────────────────
++
++    /// MockProvider returns an over-long title; builder must cap it at 72
++    /// chars and end with `…`. Exercises [`enforce_title_cap`] inside
++    /// [`build_pr_body_with_client_and_state`].
++    #[tokio::test]
++    async fn build_pr_body_truncates_long_title() {
++        let store = test_store();
++        let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
++        let long_title = "x".repeat(200);
++        let payload = pr_content_json(&long_title, "Body content.");
++        let (title, _body) = build_pr_body_with_client(
++            "diff --git a/src/lib.rs b/src/lib.rs\n+fn x() {}\n",
++            "Implement feature",
++            "mock-model",
++            &run_store.clone().into(),
++            Some(&make_test_conclusion()),
++            explicit_client("mock", &payload),
++        )
++        .await
++        .unwrap();
++
++        assert_eq!(title.chars().count(), 72);
++        assert!(title.ends_with('\u{2026}'));
++    }
++
++    /// Schema rejects truly empty bodies via `minLength: 1`.
++    #[tokio::test]
++    async fn build_pr_body_returns_err_when_body_empty() {
++        let store = test_store();
++        let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
++        let payload = pr_content_json("Mock", "");
++        let result = build_pr_body_with_client(
++            "diff --git a/src/lib.rs b/src/lib.rs\n+fn x() {}\n",
++            "Implement feature",
++            "mock-model",
++            &run_store.clone().into(),
++            Some(&make_test_conclusion()),
++            explicit_client("mock", &payload),
++        )
++        .await;
++
++        assert!(result.is_err(), "expected Err, got {result:?}");
++    }
++
++    /// Whitespace-only bodies pass schema validation but fail the
++    /// `body.trim().is_empty()` check inside the builder.
++    #[tokio::test]
++    async fn build_pr_body_returns_err_when_body_whitespace() {
++        let store = test_store();
++        let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
++        let payload = pr_content_json("Mock", "   \n");
++        let result = build_pr_body_with_client(
++            "diff --git a/src/lib.rs b/src/lib.rs\n+fn x() {}\n",
++            "Implement feature",
++            "mock-model",
++            &run_store.clone().into(),
++            Some(&make_test_conclusion()),
++            explicit_client("mock", &payload),
++        )
++        .await;
++
++        let err = result.expect_err("expected Err for whitespace-only body");
++        assert!(err.contains("empty PR body"), "unexpected error: {err}");
++    }
++
++    // ── maybe_open_pull_request fallback tests ──────────────────────────
++
++    /// Set of mock servers and credentials for the `maybe_open_pull_request`
++    /// fallback path. The builder's `Client::from_source` rebuilds the LLM
++    /// client from the credential source, so the in-process MockProvider
++    /// cannot intercept — we mock the OpenAI HTTP endpoint instead.
++    struct FallbackHarness {
++        _vault_dir:     tempfile::TempDir,
++        // Held to keep the mock listener alive for the duration of the test;
++        // the test interacts with it via `Client::from_source` (which goes
++        // out via HTTP to the mock URL stored in `llm_source`).
++        _openai_server: MockServer,
++        github_server:  MockServer,
++        llm_source:     Arc<dyn CredentialSource>,
++        creds:          fabro_github::GitHubCredentials,
++        run_store:      RunStoreHandle,
++    }
++
++    /// Stand up an OpenAI mock that returns the given structured-output
++    /// payload, a GitHub mock that accepts a PR creation, a vault-backed
++    /// credential source, and a run store seeded with a non-empty
++    /// `final_patch`.
++    async fn setup_fallback_test_harness(openai_payload_text: &str) -> FallbackHarness {
++        let openai_server = MockServer::start_async().await;
++        openai_server
++            .mock_async(|when, then| {
++                when.method(POST)
++                    .path("/v1/responses")
++                    .header("authorization", "Bearer vault-openai-key");
++                then.status(200)
++                    .header("content-type", "application/json")
++                    .json_body(openai_responses_payload(openai_payload_text));
++            })
++            .await;
++
++        let github_server = MockServer::start_async().await;
++        github_server
++            .mock_async(|when, then| {
++                when.method(POST)
++                    .path("/repos/owner/repo/pulls")
++                    .header("authorization", "Bearer test-token");
++                then.status(201)
++                    .header("content-type", "application/json")
++                    .json_body(serde_json::json!({
++                        "number": 1,
++                        "html_url": "https://example.test/owner/repo/pull/1",
++                        "node_id": "PR_kwTest1",
++                    }));
++            })
++            .await;
++
++        let vault_dir = tempfile::tempdir().unwrap();
++        let mut vault = Vault::load(vault_dir.path().join("secrets.json")).unwrap();
++        vault
++            .set(
++                "openai_codex",
++                &serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(),
++                SecretType::Credential,
++                None,
++            )
++            .unwrap();
++        let base_url = openai_server.url("/v1");
++        let llm_source: Arc<dyn CredentialSource> =
++            Arc::new(VaultCredentialSource::with_env_lookup(
++                Arc::new(AsyncRwLock::new(vault)),
++                move |name| match name {
++                    "OPENAI_BASE_URL" => Some(base_url.clone()),
++                    _ => None,
++                },
++            ));
++
++        let creds = fabro_github::GitHubCredentials::Token("test-token".to_string());
++
++        let store = test_store();
++        let run_store = store.create_run(&fixtures::RUN_1).await.unwrap();
++        // Seed a non-empty `final_patch` so `load_pull_request_diff` returns
++        // diff content and the early-return for empty diffs does not fire.
++        let run_spec = RunSpec {
++            run_id:           fixtures::RUN_1,
++            settings:         fabro_types::WorkflowSettings::default(),
++            graph:            Graph::new("test"),
++            workflow_slug:    None,
++            source_directory: None,
++            git:              None,
++            labels:           HashMap::new(),
++            provenance:       None,
++            manifest_blob:    None,
++            definition_blob:  None,
++            fork_source_ref:  None,
++            in_place:         false,
++        };
++        append_event(&run_store, &fixtures::RUN_1, &Event::RunCreated {
++            run_id:           fixtures::RUN_1,
++            settings:         serde_json::to_value(&run_spec.settings).unwrap(),
++            graph:            serde_json::to_value(&run_spec.graph).unwrap(),
++            workflow_source:  None,
++            workflow_config:  None,
++            labels:           run_spec.labels.clone().into_iter().collect(),
++            run_dir:          "/tmp/x".to_string(),
++            source_directory: None,
++            workflow_slug:    None,
++            db_prefix:        None,
++            provenance:       None,
++            manifest_blob:    None,
++            git:              None,
++            fork_source_ref:  None,
++            in_place:         false,
++            web_url:          None,
++        })
++        .await
++        .unwrap();
++        append_event(&run_store, &fixtures::RUN_1, &Event::WorkflowRunCompleted {
++            duration_ms:          1,
++            artifact_count:       0,
++            status:               "succeeded".to_string(),
++            reason:               SuccessReason::Completed,
++            total_usd_micros:     None,
++            final_git_commit_sha: None,
++            final_patch:          Some(
++                "diff --git a/src/lib.rs b/src/lib.rs\n+fn from_store() {}\n".to_string(),
++            ),
++            billing:              None,
++        })
++        .await
++        .unwrap();
++
++        FallbackHarness {
++            _vault_dir: vault_dir,
++            _openai_server: openai_server,
++            github_server,
++            llm_source,
++            creds,
++            run_store: run_store.into(),
++        }
++    }
++
++    /// LLM returns a usable body but an empty title; `maybe_open_pull_request`
++    /// must fall back to `pr_title_from_goal` (first line, decoration
++    /// stripped) and the PR creation must succeed with that title.
++    #[tokio::test]
++    async fn maybe_open_pull_request_falls_back_to_goal_title_when_llm_returns_empty_title() {
++        let payload = pr_content_json("", "Narrative.");
++        let harness = setup_fallback_test_harness(&payload).await;
++
++        let github_base_url = harness.github_server.url("");
++        let github = github_app::GitHubContext::new(&harness.creds, &github_base_url);
++
++        let result = maybe_open_pull_request(OpenPullRequestRequest {
++            github,
++            origin_url: "https://github.com/owner/repo.git",
++            base_branch: "main",
++            head_branch: "fabro/run/123",
++            goal: "Fix telemetry leak\n\ndetails...",
++            diff: "diff --git a/src/lib.rs b/src/lib.rs\n+fn x() {}\n",
++            model: "gpt-5.4",
++            draft: false,
++            auto_merge: None,
++            run_store: &harness.run_store,
++            llm_source: harness.llm_source.as_ref(),
++            conclusion: None,
++            run_state: None,
++        })
++        .await
++        .expect("PR creation should succeed");
++
++        let record = result.expect("PR record should be Some");
++        assert_eq!(record.title, "Fix telemetry leak");
++    }
++
++    /// LLM returns an empty title; the fallback path produces a long title
++    /// (close to `pr_title_from_goal`'s 120-char cap), and the unconditional
++    /// `enforce_title_cap` in `maybe_open_pull_request` must still bring it
++    /// down to 72 chars ending with `…`.
++    #[tokio::test]
++    async fn maybe_open_pull_request_caps_fallback_title_at_72_chars() {
++        let payload = pr_content_json("", "Narrative.");
++        let harness = setup_fallback_test_harness(&payload).await;
++
++        let github_base_url = harness.github_server.url("");
++        let github = github_app::GitHubContext::new(&harness.creds, &github_base_url);
++
++        // Single ~200-char line, no `Plan:` / heading prefix, no newlines.
++        let goal = "x".repeat(200);
++
++        let result = maybe_open_pull_request(OpenPullRequestRequest {
++            github,
++            origin_url: "https://github.com/owner/repo.git",
++            base_branch: "main",
++            head_branch: "fabro/run/123",
++            goal: &goal,
++            diff: "diff --git a/src/lib.rs b/src/lib.rs\n+fn x() {}\n",
++            model: "gpt-5.4",
++            draft: false,
++            auto_merge: None,
++            run_store: &harness.run_store,
++            llm_source: harness.llm_source.as_ref(),
++            conclusion: None,
++            run_state: None,
++        })
++        .await
++        .expect("PR creation should succeed");
++
++        let record = result.expect("PR record should be Some");
++        assert_eq!(record.title.chars().count(), 72);
++        assert!(record.title.ends_with('\u{2026}'));
++    }
+ }
+diff --git a/lib/crates/fabro-workflow/tests/it/integration.rs b/lib/crates/fabro-workflow/tests/it/integration.rs
+index 002d473d..742218c1 100644
+--- a/lib/crates/fabro-workflow/tests/it/integration.rs
++++ b/lib/crates/fabro-workflow/tests/it/integration.rs
+@@ -6809,7 +6809,13 @@ async fn workflow_run_with_vault_only_openai_codex_builds_pr_body() {
+                 .header("authorization", "Bearer vault-openai-key");
+             then.status(200)
+                 .header("content-type", "application/json")
+-                .json_body(openai_responses_payload("Narrative from vault source."));
++                .json_body(openai_responses_payload(
++                    &serde_json::to_string(&serde_json::json!({
++                        "title": "Vault title",
++                        "body": "Narrative from vault source.",
++                    }))
++                    .unwrap(),
++                ));
+         })
+         .await;
+ 
+@@ -6890,7 +6896,7 @@ async fn workflow_run_with_vault_only_openai_codex_builds_pr_body() {
+     let run_store = store.open_run_reader(&run_options.run_id).await.unwrap();
+     let run_store_handle: fabro_workflow::runtime_store::RunStoreHandle = run_store.into();
+ 
+-    let body = fabro_workflow::pull_request::build_pr_body(
++    let (title, body) = fabro_workflow::pull_request::build_pr_body(
+         "diff --git a/src/lib.rs b/src/lib.rs\n+fn new_feature() {}\n",
+         "Implement feature",
+         "gpt-5.4",
+@@ -6910,6 +6916,7 @@ async fn workflow_run_with_vault_only_openai_codex_builds_pr_body() {
+     .await
+     .expect("PR body should build from vault-only credentials");
+ 
++    assert_eq!(title, "Vault title");
+     assert!(body.contains("Narrative from vault source."));
+     response_mock.assert_async().await;
+ }

stages/005-implement@1/status.json

@@ -0,0 +1,6 @@
+{
+  "outcome": "succeeded",
+  "notes": "Stage completed: implement",
+  "failure_reason": null,
+  "timestamp": "2026-05-04T18:54:35.316564Z"
+}

stages/006-simplify_opus@1/prompt.md

@@ -0,0 +1,373 @@
+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) | 1–2 sentences, no headers, total under ~300 characters |
+| Small + non-trivial (targeted bugfix, behavioral change) | Short "Problem / Fix" narrative, 3–5 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 1416–1485) — 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)
+- **implement**: succeeded
+  - Model: claude-opus-4-7, 122.9k tokens in / 32.1k out
+  - Files: /home/daytona/workspace/lib/crates/fabro-server/src/server/tests.rs, /home/daytona/workspace/lib/crates/fabro-workflow/src/pipeline/pull_request.rs, /home/daytona/workspace/lib/crates/fabro-workflow/tests/it/integration.rs
+
+
+# Simplify: Code Review and Cleanup
+
+Review changes vs. origin for reuse, quality, and efficiency. Fix any issues found.
+
+## Phase 1: Identify Changes
+
+Run git diff (or git diff HEAD if there are staged changes) to see what changed. If there are no git changes, review the most recently modified files that the user mentioned or that you edited earlier in this conversation.
+
+## Phase 2: Launch Three Review Agents in Parallel
+
+Use the Agent tool to launch all three agents concurrently in a single message. Pass each agent the full diff so it has the complete context.
+
+### Agent 1: Code Reuse Review
+
+For each change:
+
+1. Search for existing utilities and helpers that could replace newly written code. Use Grep to find similar patterns elsewhere in the codebase — common locations are utility directories, shared modules, and files adjacent to the changed ones.
+2. Flag any new function that duplicates existing functionality. Suggest the existing function to use instead.
+3. Flag any inline logic that could use an existing utility — hand-rolled string manipulation, manual path handling, custom environment checks, ad-hoc type guards, and similar patterns are common candidates.
+
+Note: This is a greenfield app, so focus on maximizing simplicity and don't worry about changing things to achieve it.
+
+### Agent 2: Code Quality Review
+
+Review the same changes for hacky patterns:
+
+1. Redundant state: state that duplicates existing state, cached values that could be derived, observers/effects that could be direct calls
+2. Parameter sprawl: adding new parameters to a function instead of generalizing or restructuring existing ones
+3. Copy-paste with slight variation: near-duplicate code blocks that should be unified with a shared abstraction
+4. Leaky abstractions: exposing internal details that should be encapsulated, or breaking existing abstraction boundaries
+5. Stringly-typed code: using raw strings where constants, enums (string unions), or branded types already exist in the codebase
+
+Note: This is a greenfield app, so be aggressive in optimizing quality.
+
+### Agent 3: Efficiency Review
+
+Review the same changes for efficiency:
+
+1. Unnecessary work: redundant computations, repeated file reads, duplicate network/API calls, N+1 patterns
+2. Missed concurrency: independent operations run sequentially when they could run in parallel
+3. Hot-path bloat: new blocking work added to startup or per-request/per-render hot paths
+4. Unnecessary existence checks: pre-checking file/resource existence before operating (TOCTOU anti-pattern) — operate directly and handle the error
+5. Memory: unbounded data structures, missing cleanup, event listener leaks
+6. Overly broad operations: reading entire files when only a portion is needed, loading all items when filtering for one
+
+## Phase 3: Fix Issues
+
+Wait for all three agents to complete. Aggregate their findings and fix each issue directly. If a finding is a false positive or not worth addressing, note it and move on — do not argue with the finding, just skip it.
+
+When done, briefly summarize what was fixed (or confirm the code was already clean).

stages/006-simplify_opus@1/provider_used.json

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

stages/006-simplify_opus@1/response.md

@@ -0,0 +1,27 @@
+Both clean. 
+
+## Summary of cleanups
+
+Aggregated findings across the three review agents and fixed the actionable ones:
+
+**Code reuse**
+- **Unified `enforce_title_cap` and `pr_title_from_goal`'s truncation logic** into a shared `truncate_with_ellipsis(s, max) -> String` helper. Eliminates the duplicated `chars().count() > N → take(N-1).collect() + '…'` algorithm. `pr_title_from_goal` is now a one-liner.
+- **Switched from field access to the `m.context_window()` accessor** in `truncation_caps`, matching the convention used by the other 4 catalog-consumer call sites in the workspace (`fabro-agent`, `fabro-llm`, etc.).
+
+**Code quality**
+- **Hoisted the magic `72`** into a module-level `PR_TITLE_MAX_CHARS: usize` constant, referenced from both the JSON schema (`maxLength: PR_TITLE_MAX_CHARS`) and `enforce_title_cap`. The system prompt's "max 72 characters" wording got an explicit sync-comment.
+- **Replaced `(String, String)` return type with a named `PrContent { title, body }` struct** across the four `build_pr_body*` signatures and all five call sites. Prevents accidental field swap and self-documents at the call site.
+- **Collapsed the six tier constants** (`MAX_GOAL_CHARS_LARGE/SMALL`, etc.) into a `TruncationCaps` struct with two `const`s (`TRUNCATION_LARGE`, `TRUNCATION_SMALL`). Call site went from `let (g, p, d) = truncation_caps(...)` to `let caps = truncation_caps(...)`.
+- **Consolidated split `use` statements** at the top of the file. The file previously had ~150 lines of new constants/functions wedged between two `use` blocks; all imports are now at the top.
+- **Eliminated the redundant double `enforce_title_cap`** in `maybe_open_pull_request` by moving it into the fallback branch only — the LLM-title branch is already capped inside the builder. Added a comment explaining why the fallback branch needs re-capping (`pr_title_from_goal` has a wider 120-char cap).
+- **Fixed misleading `truncate_chars` doc comment** — it was documented as "chars" but actually byte-caps and aligns to a UTF-8 boundary. Doc now correctly says "bytes" and notes the char/byte equivalence for ASCII.
+
+**Skipped (false positives or low value)**
+- `Catalog::builtin()` cost — verified to be a static `LazyLock`, no rebuild per call.
+- `PR_CONTENT_SCHEMA.clone()` — `generate_object` takes `Value` by value; clone is dwarfed by the LLM HTTP call.
+- `pr_content_json` cross-crate sharing — only two external duplications, low value to expose via `test-support`.
+- `setup_fallback_test_harness` partial overlap with the existing vault test — different shapes (one needs GitHub mock + final_patch event, the other doesn't); merging would tangle.
+- Whitespace-leak in title — verified non-issue: builder trims via `enforce_title_cap(generated.title.trim())` before storing on the struct.
+- `_vault_dir`/`_openai_server` keepalive pattern in test harness — already commented; standard Rust pattern.
+
+**Verification**: `cargo check --workspace --all-targets` clean, `cargo nextest run -p fabro-workflow` 1057/1057 passed, `cargo nextest run -p fabro-server` 443/443 passed, `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings` clean, `cargo +nightly-2026-04-14 fmt --check --all` clean.