feat(agent): gate loop completion on unresolved blockers

This commit is contained in:
Fini 2026-07-24 21:12:05 +08:00
parent ac9aa89c67
commit 47be58eb39
11 changed files with 794 additions and 24 deletions

View file

@ -362,6 +362,44 @@ pub struct ChatToolResult {
/// [`ChatToolExecutor::finalize`].
pub const LOOP_FINALIZE_OP: &str = "__loop_finalize";
/// Reserved pseudo-tool name a [`ChatToolExecutor`] forwards over its tool
/// channel to ask the host for a read-only unresolved-blocker scan against
/// the live `EditorState` — the completion-gate counterpart of
/// [`LOOP_FINALIZE_OP`]'s promise-delivery check. Never mutates the
/// document (mirrors the `checkOnly: true` half of `LOOP_FINALIZE_OP`, not
/// the finalize half). Never a real model-visible tool — the loop sends it
/// itself via [`ChatToolExecutor::check_blockers`].
pub const CHECK_BLOCKERS_OP: &str = "__loop_check_blockers";
/// One unresolved structural blocker found by the host's live blocker scan
/// (`op_host_services::loop_blocker_ledger::detect_blockers`). `category` is
/// a coarse bucket (`"structure"` / `"empty-shell"` / `"nav"` today) for
/// grouping in a corrective message; `detail` is the same human-readable,
/// node/screen-identifying line the per-batch `structureIssues` /
/// `shellsRemaining` / `navIssues` tool-result fields already carry, so the
/// loop-end nudge reads exactly like the per-batch hints the model has
/// already been following all run.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct BlockerEntry {
pub category: String,
pub detail: String,
}
/// Unresolved-blocker scan result. Deliberately NOT an accumulating ledger —
/// like [`UnfilledScreensReport`], this is always a fresh recompute against
/// the CURRENT document, so an issue a later batch already fixed simply
/// never appears again; there is nothing to prune or expire.
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct BlockerReport {
pub blockers: Vec<BlockerEntry>,
}
impl BlockerReport {
pub fn has_blockers(&self) -> bool {
!self.blockers.is_empty()
}
}
/// Promise-delivery snapshot: every top-level "screen" the run committed to
/// (`committed`, filled or not) alongside the subset still empty (`unfilled`,
/// always a subset of `committed`). Carries enough for the loop to build the
@ -408,6 +446,18 @@ pub trait ChatToolExecutor: Send + Sync {
fn check_unfilled_screens(&self) -> UnfilledScreensReport {
UnfilledScreensReport::default()
}
/// Cheap, read-only unresolved-blocker scan — the completion-gate
/// counterpart of [`Self::check_unfilled_screens`]. The loop calls this
/// whenever it's deciding whether to let a `calls.is_empty()` model stop
/// count as done: a document that still has an unbound primary-mobile
/// nav tab or a structural defect (duplicate status bar / broken ring /
/// empty shell) gets a corrective nudge instead of a silent finish. The
/// default no-op mirrors [`Self::check_unfilled_screens`]'s — only the
/// host executor that owns a live `EditorState` overrides it.
fn check_blockers(&self) -> BlockerReport {
BlockerReport::default()
}
}
/// Test double — replays a fixed delta script. Lets the chat widget
@ -430,6 +480,33 @@ impl ChatProvider for EchoProvider {
mod tests {
use super::*;
/// An executor that overrides NOTHING but the mandatory `execute` — the
/// shape of every host that hasn't wired up the unresolved-blocker scan
/// (web, any future non-desktop host). Anchors the zero-impact
/// requirement: an unwired host must behave EXACTLY as it did before
/// `check_blockers` existed, purely by inheriting the trait default.
struct BareExecutor;
impl ChatToolExecutor for BareExecutor {
fn execute(&self, _name: &str, _args_json: &str) -> ChatToolResult {
ChatToolResult {
content: "{}".into(),
is_error: false,
}
}
}
#[test]
fn unwired_executor_check_blockers_default_is_empty_and_never_blocks() {
let report = BareExecutor.check_blockers();
assert_eq!(report, BlockerReport::default());
assert!(
!report.has_blockers(),
"an executor that never overrides check_blockers must report no blockers, \
so the loop's completion gate takes the exact same path it did before \
this feature existed"
);
}
#[test]
fn cli_name_backend_table_matches_architecture_memo() {
// project_agent_runtime memory:

View file

@ -6,8 +6,8 @@ use std::sync::Mutex;
use std::thread;
use op_ai::chat_provider::{
ChatDelta, ChatHistoryRole, ChatProvider, ChatRequest, ChatToolExecutor, ChatToolResult,
UnfilledScreensReport, LOOP_FINALIZE_OP,
BlockerReport, ChatDelta, ChatHistoryRole, ChatProvider, ChatRequest, ChatToolExecutor,
ChatToolResult, UnfilledScreensReport, CHECK_BLOCKERS_OP, LOOP_FINALIZE_OP,
};
use op_editor_core::{ChatMessage, ChatRole, ChatToolCall};
@ -54,6 +54,14 @@ impl ChatToolExecutor for UiChatToolExecutor {
fn check_unfilled_screens(&self) -> UnfilledScreensReport {
unfilled_report_from_ack(&self.forward(LOOP_FINALIZE_OP, r#"{"checkOnly":true}"#))
}
/// Forward the reserved [`CHECK_BLOCKERS_OP`] over the same tool
/// channel so the host runs a read-only unresolved-blocker scan
/// (`op_host_services::loop_blocker_ledger::detect_blockers`) against
/// the live `EditorState`. Never mutates the document.
fn check_blockers(&self) -> BlockerReport {
blocker_report_from_ack(&self.forward(CHECK_BLOCKERS_OP, "{}"))
}
}
/// Parse the `{"success":true,"committed":[...],"unfilled":[...]}` envelope
@ -78,6 +86,31 @@ fn unfilled_report_from_ack(result: &ChatToolResult) -> UnfilledScreensReport {
}
}
/// Parse the `{"success":true,"blockers":[{"category":...,"detail":...}]}`
/// envelope the host's `CHECK_BLOCKERS_OP` interception acks with. Any other
/// shape (transport error, an old host build that doesn't know this op)
/// degrades to an empty report — same best-effort discipline as
/// [`unfilled_report_from_ack`].
fn blocker_report_from_ack(result: &ChatToolResult) -> BlockerReport {
let Ok(v) = serde_json::from_str::<serde_json::Value>(&result.content) else {
return BlockerReport::default();
};
let blockers = v
.get("blockers")
.and_then(|v| v.as_array())
.map(|arr| {
arr.iter()
.filter_map(|entry| {
let category = entry.get("category")?.as_str()?.to_string();
let detail = entry.get("detail")?.as_str()?.to_string();
Some(op_ai::chat_provider::BlockerEntry { category, detail })
})
.collect()
})
.unwrap_or_default();
BlockerReport { blockers }
}
impl UiChatToolExecutor {
/// Send one request over the tool channel and block until the host acks.
fn forward(&self, name: &str, args_json: &str) -> ChatToolResult {

View file

@ -293,6 +293,29 @@ fn execute_tool_requests(
});
continue;
}
// Intercept the reserved unresolved-blocker scan op: the completion
// gate counterpart of `LOOP_FINALIZE_OP`'s `checkOnly` half. Always
// read-only — a live recompute against the current document, never
// an accumulating ledger (see
// `op_host_services::loop_blocker_ledger`'s module doc) — so an
// issue a later batch already fixed simply stops appearing here.
if req.name == op_ai::chat_provider::CHECK_BLOCKERS_OP {
let report = op_host_services::loop_blocker_ledger::detect_blockers(state);
let blockers: Vec<serde_json::Value> = report
.blockers
.iter()
.map(|b| serde_json::json!({ "category": b.category, "detail": b.detail }))
.collect();
let _ = req.ack.send(ChatToolResult {
content: serde_json::json!({
"success": true,
"blockers": blockers,
})
.to_string(),
is_error: false,
});
continue;
}
// Intercept `spawn_agents`: parse the specs, stash them for the
// host to launch after this (parent) pump, and ack immediately
// (fire-and-forget). A SUB calling `spawn_agents` is refused —

View file

@ -30,6 +30,10 @@ use retry::{
CORRECTIVE_WRITE_PROGRESS,
};
#[path = "chat_agent_loop_blockers.rs"]
mod blockers;
use blockers::{blocker_nudge_if_owed, report_blockers_if_any};
/// Everything one agent-loop run needs. `max_turns` is the TS `maxTurns`
/// cap — `MAX_TOOL_TURNS = 20` for plain chat, `DESIGN_LOOP_MAX_TURNS = 28`
/// for the gated design-generation loop (`chat_builtin_http.rs`; the two
@ -269,6 +273,26 @@ async fn report_unfilled_if_any(tx: &mpsc::Sender<ChatDelta>, names: &[String])
let _ = tx.send(ChatDelta::TextDelta(text)).await;
}
/// Shared finalize + honest-report tail for every loop exit tier (turn-cap
/// salvage-exhausted, salvage-nothing-eligible, and the ordinary
/// voluntary-model-stop path) — replaces what used to be three near-identical
/// `run_loop_finalize` + `report_unfilled_if_any` call sites per provider
/// loop, and folds the unresolved-blocker report into the SAME tail so every
/// exit unconditionally surfaces both: a run that spent its whole turn
/// budget with blockers still open must never look, in the transcript, like
/// a run that had nothing wrong with it.
async fn finalize_and_report(
tx: &mpsc::Sender<ChatDelta>,
executor: &Arc<dyn ChatToolExecutor>,
enabled: bool,
) -> UnfilledScreensReport {
let still_unfilled = run_loop_finalize(executor, enabled).await;
report_unfilled_if_any(tx, &still_unfilled.unfilled).await;
let blocker_report = blockers::check_blockers(executor, enabled).await;
report_blockers_if_any(tx, &blocker_report).await;
still_unfilled
}
/// Self-diagnostic signal for the finalize-lifecycle invariant (0718-1-k3-1
/// postmortem) — a `run_anthropic_agent_loop` / `run_openai_agent_loop`
/// outer wrapper's backstop just ran [`run_loop_finalize`] on an `Err` exit
@ -606,6 +630,11 @@ async fn run_anthropic_agent_loop_inner(
// `fill_attempts` above (see `SALVAGE_MAX_ROUNDS`'s doc comment).
let mut salvaged_screens: std::collections::HashSet<String> = std::collections::HashSet::new();
let mut salvage_rounds_used = 0usize;
// Tier 2c — unresolved-blocker corrective-round budget. A SEPARATE pool
// from both of the above (see `chat_agent_loop_blockers::
// BLOCKER_NUDGE_MAX_ROUNDS`'s doc comment): blockers and unfilled
// screens are different failure modes with independent budgets.
let mut blocker_rounds_used = 0usize;
let mut write_retry = CorrectiveWriteRetry::default();
let turn_cap = cfg.max_turns.max(1);
let mut turn = 0usize;
@ -620,8 +649,7 @@ async fn run_anthropic_agent_loop_inner(
// against `turn`; it draws from `SALVAGE_MAX_ROUNDS` instead.
if turn >= turn_cap && !corrective_write_round {
if salvage_rounds_used >= SALVAGE_MAX_ROUNDS {
let still_unfilled = run_loop_finalize(&cfg.executor, cfg.finalize_on_exit).await;
report_unfilled_if_any(tx, &still_unfilled.unfilled).await;
finalize_and_report(tx, &cfg.executor, cfg.finalize_on_exit).await;
let _ = tx
.send(ChatDelta::Done {
stop_reason: StopReason::MaxTokens,
@ -637,8 +665,7 @@ async fn run_anthropic_agent_loop_inner(
// unfilled screen already spent its one salvage round. Still
// run the Step-4 structural backstop once over whatever the
// run assembled, and report unconditionally.
let still_unfilled = run_loop_finalize(&cfg.executor, cfg.finalize_on_exit).await;
report_unfilled_if_any(tx, &still_unfilled.unfilled).await;
finalize_and_report(tx, &cfg.executor, cfg.finalize_on_exit).await;
let _ = tx
.send(ChatDelta::Done {
stop_reason: StopReason::MaxTokens,
@ -734,15 +761,32 @@ async fn run_anthropic_agent_loop_inner(
);
continue; // Does not count against `turn`.
}
// No screen left to chase — try one dedicated corrective round
// for any unresolved structural blocker (duplicate root / broken
// ring / empty shell / unbound primary-mobile nav tab), bounded
// by its own `BLOCKER_NUDGE_MAX_ROUNDS` budget so this can never
// compound into an unbounded loop alongside the fill/salvage
// budgets above.
if let Some(nudge) = blocker_nudge_if_owed(
&cfg.executor,
cfg.finalize_on_exit,
&mut blocker_rounds_used,
)
.await
{
messages
.push(json!({ "role": "assistant", "content": collector.assistant_content() }));
messages.push(json!({ "role": "user", "content": nudge }));
continue; // Does not count against `turn`.
}
// Nothing committed is left worth trying for: run the Step-4
// structural backstop ONCE over the assembled doc BEFORE the
// Done delta, so the finalized document is what the UI
// persists/displays for this turn. Tier 3 — honest report —
// fires unconditionally if anything is still unfilled (a screen
// outside this run's committed set, or the executor being a
// no-op).
let still_unfilled = run_loop_finalize(&cfg.executor, cfg.finalize_on_exit).await;
report_unfilled_if_any(tx, &still_unfilled.unfilled).await;
// fires unconditionally if anything is still unfilled/blocked (a
// screen outside this run's committed set, a blocker whose
// round budget ran out, or the executor being a no-op).
finalize_and_report(tx, &cfg.executor, cfg.finalize_on_exit).await;
let _ = tx
.send(ChatDelta::Done {
stop_reason: reason,
@ -997,6 +1041,11 @@ async fn run_openai_agent_loop_inner(
let mut fill_attempts: HashMap<String, usize> = HashMap::new();
let mut salvaged_screens: std::collections::HashSet<String> = std::collections::HashSet::new();
let mut salvage_rounds_used = 0usize;
// Tier 2c — unresolved-blocker corrective-round budget. A SEPARATE pool
// from both of the above (see `chat_agent_loop_blockers::
// BLOCKER_NUDGE_MAX_ROUNDS`'s doc comment): blockers and unfilled
// screens are different failure modes with independent budgets.
let mut blocker_rounds_used = 0usize;
let mut write_retry = CorrectiveWriteRetry::default();
let turn_cap = cfg.max_turns.max(1);
let mut turn = 0usize;
@ -1006,8 +1055,7 @@ async fn run_openai_agent_loop_inner(
let corrective_write_round = write_retry.begin_round();
if turn >= turn_cap && !corrective_write_round {
if salvage_rounds_used >= SALVAGE_MAX_ROUNDS {
let still_unfilled = run_loop_finalize(&cfg.executor, cfg.finalize_on_exit).await;
report_unfilled_if_any(tx, &still_unfilled.unfilled).await;
finalize_and_report(tx, &cfg.executor, cfg.finalize_on_exit).await;
let _ = tx
.send(ChatDelta::Done {
stop_reason: StopReason::MaxTokens,
@ -1018,8 +1066,7 @@ async fn run_openai_agent_loop_inner(
let report = check_unfilled(&cfg.executor, cfg.finalize_on_exit).await;
let eligible = salvage_eligible(&report.unfilled, &salvaged_screens);
if eligible.is_empty() {
let still_unfilled = run_loop_finalize(&cfg.executor, cfg.finalize_on_exit).await;
report_unfilled_if_any(tx, &still_unfilled.unfilled).await;
finalize_and_report(tx, &cfg.executor, cfg.finalize_on_exit).await;
let _ = tx
.send(ChatDelta::Done {
stop_reason: StopReason::MaxTokens,
@ -1119,11 +1166,26 @@ async fn run_openai_agent_loop_inner(
);
continue; // Does not count against `turn`.
}
// No screen left to chase — try one dedicated corrective round
// for any unresolved structural blocker, bounded by its own
// `BLOCKER_NUDGE_MAX_ROUNDS` budget (see the Anthropic loop
// above for the full rationale).
if let Some(nudge) = blocker_nudge_if_owed(
&cfg.executor,
cfg.finalize_on_exit,
&mut blocker_rounds_used,
)
.await
{
messages.push(json!({ "role": "assistant", "content": stop_content() }));
messages.push(json!({ "role": "user", "content": nudge }));
continue; // Does not count against `turn`.
}
// Normal model-stop exit: run the Step-4 structural backstop ONCE
// over the assembled doc BEFORE the Done delta. Tier 3 — honest
// report — fires unconditionally if anything is still unfilled.
let still_unfilled = run_loop_finalize(&cfg.executor, cfg.finalize_on_exit).await;
report_unfilled_if_any(tx, &still_unfilled.unfilled).await;
// report — fires unconditionally if anything is still
// unfilled/blocked.
finalize_and_report(tx, &cfg.executor, cfg.finalize_on_exit).await;
let _ = tx
.send(ChatDelta::Done {
stop_reason: reason,
@ -1249,6 +1311,13 @@ mod tests;
#[path = "chat_agent_loop_finalize_tests.rs"]
mod finalize_tests;
// Unresolved-blocker completion-gate tests — same split rationale as
// `finalize_tests` above, reusing `chat_agent_loop_tests.rs`'s scripted-
// executor + loopback-SSE infra via `pub(super)`.
#[cfg(test)]
#[path = "chat_agent_loop_blockers_tests.rs"]
mod blockers_tests;
#[cfg(test)]
#[path = "chat_agent_loop_retry_tests.rs"]
mod retry_tests;

View file

@ -0,0 +1,106 @@
//! Unresolved-blocker completion gate — shared between the Anthropic and
//! OpenAI-compatible agent loops in `chat_agent_loop.rs` (split out as a
//! sibling module the same way `chat_agent_loop_retry.rs` splits out the
//! corrective-write retry policy, so neither loop function duplicates this
//! logic). See `op_host_services::loop_blocker_ledger`'s module doc for what
//! counts as a blocker (structure / empty-shell / nav) and why this is a
//! live recompute against the document rather than an accumulating ledger.
use std::sync::Arc;
use op_ai::chat_provider::{BlockerReport, ChatDelta, ChatToolExecutor};
use tokio::sync::mpsc;
/// Per-run budget for the unresolved-blocker corrective round — a SEPARATE
/// pool from `FILL_BUDGET_MAX_ROUNDS_PER_SCREEN` / `SALVAGE_MAX_ROUNDS`
/// (blockers and unfilled screens are different failure modes; see those
/// constants' doc comments in `chat_agent_loop.rs` for "budget guards
/// runaway retry, never truncates promised work"). Blockers are a
/// narrower, already-detected class — the model already saw each one as a
/// per-batch tool-result hint (`structureIssues` / `shellsRemaining` /
/// `navIssues`) — so a flat per-run cap (not a per-item pool like the fill
/// budget) is enough to give the model a real chance to react without room
/// for an avalanche.
pub(super) const BLOCKER_NUDGE_MAX_ROUNDS: usize = 2;
/// Cheap, read-only unresolved-blocker scan. Gated the same way
/// `check_unfilled`/`run_loop_finalize` are: `enabled` mirrors
/// `cfg.finalize_on_exit`, so a plain (non-design) chat turn never touches
/// the document.
pub(super) async fn check_blockers(
executor: &Arc<dyn ChatToolExecutor>,
enabled: bool,
) -> BlockerReport {
if !enabled {
return BlockerReport::default();
}
let executor = executor.clone();
tokio::task::spawn_blocking(move || executor.check_blockers())
.await
.unwrap_or_default()
}
/// Corrective-message contract text for a still-blocked loop completion —
/// mirrors `contract_nudge_text`'s "state the full commitment, not just
/// what's missing" shape, but for structural blockers instead of unfilled
/// screens.
pub(super) fn blocker_nudge_text(report: &BlockerReport) -> String {
let lines: Vec<String> = report
.blockers
.iter()
.map(|b| format!("- [{}] {}", b.category, b.detail))
.collect();
format!(
"The design still has {} unresolved blocker(s) that must be fixed before finishing:\n{}",
lines.len(),
lines.join("\n")
)
}
/// Decide whether this `calls.is_empty()` model-stop is owed one more
/// corrective round for unresolved blockers — mirrors
/// `eligible_for_fill_round`'s per-screen budget check, but against a flat
/// per-run round counter instead of a per-screen map (blockers aren't keyed
/// by screen name the way unfilled screens are). Spends a round from
/// `rounds_used` when it returns `Some`; the caller pushes the text as a
/// user message and `continue`s WITHOUT counting it against the ordinary
/// turn budget, exactly like the fill round.
pub(super) async fn blocker_nudge_if_owed(
executor: &Arc<dyn ChatToolExecutor>,
enabled: bool,
rounds_used: &mut usize,
) -> Option<String> {
if !enabled || *rounds_used >= BLOCKER_NUDGE_MAX_ROUNDS {
return None;
}
let report = check_blockers(executor, enabled).await;
if !report.has_blockers() {
return None;
}
*rounds_used += 1;
Some(blocker_nudge_text(&report))
}
/// Tier-3 unconditional honest report for blockers — appended right
/// alongside `report_unfilled_if_any`'s screen line whenever the loop is
/// about to send `Done` with blockers still unresolved (round budget
/// spent, or this exit tier never spent a round at all — e.g. the
/// turn-cap/salvage tiers, which only nudge for unfilled screens, not
/// blockers). Never silently succeeds: any blocker still present at ANY
/// exit tier surfaces here.
pub(super) async fn report_blockers_if_any(tx: &mpsc::Sender<ChatDelta>, report: &BlockerReport) {
if !report.has_blockers() {
return;
}
let text = format!(
"\n\n• {} unresolved blocker(s): {}",
report.blockers.len(),
report
.blockers
.iter()
.map(|b| b.detail.as_str())
.collect::<Vec<_>>()
.join("; ")
);
let _ = tx.send(ChatDelta::TextDelta(text)).await;
}

View file

@ -0,0 +1,187 @@
//! Unresolved-blocker completion-gate tests — mirrors the "承诺-交付"
//! fill-round tests in `chat_agent_loop_tests.rs` (search for that banner),
//! but for structural blockers (`check_blockers`) instead of unfilled
//! screens (`check_unfilled_screens`). Three tiers: no blocker → unaffected;
//! blocker present with round budget left → one corrective nudge, then
//! completes once fixed; blocker persists past `BLOCKER_NUDGE_MAX_ROUNDS` →
//! never silently succeeds, the honest report still lands.
use super::tests::{run_loop_collect, serve_sse_script, update_node_tool_def, ScriptedExecutor};
use super::*;
fn anthropic_text_turn() -> String {
[
r#"data: {"type":"content_block_start","index":0,"content_block":{"type":"text","text":""}}"#,
"",
r#"data: {"type":"content_block_delta","index":0,"delta":{"type":"text_delta","text":"Done."}}"#,
"",
r#"data: {"type":"message_delta","delta":{"stop_reason":"end_turn"}}"#,
"",
r#"data: {"type":"message_stop"}"#,
"",
"",
]
.join("\n")
}
fn base_cfg(
base: &str,
executor: std::sync::Arc<ScriptedExecutor>,
max_turns: usize,
) -> AgentLoopConfig {
AgentLoopConfig {
url: format!("{base}/v1/messages"),
api_key: "sk-test".into(),
model: "claude-test".into(),
system_prompt: String::new(),
history: Vec::new(),
user_prompt: "build the app".into(),
max_output_tokens: 512,
tools: vec![update_node_tool_def()],
executor,
max_turns,
finalize_on_exit: true,
disable_thinking: false,
dial_policy: crate::provider_dial::EndpointDialPolicy::Trusted,
}
}
#[test]
fn no_blockers_completes_without_nudge_or_report() {
// Nothing scripted for `check_blockers` — the default (empty) report —
// must behave EXACTLY like today: straight to finalize + Done, no
// corrective round, no "unresolved blocker" line anywhere.
let (base, req_rx) = serve_sse_script(vec![anthropic_text_turn()]);
let executor = ScriptedExecutor::ok(r#"{"success":true,"data":{}}"#);
let cfg = base_cfg(&base, executor.clone(), 5);
let (outcome, deltas) = run_loop_collect(cfg, true);
assert_eq!(outcome, Ok(true));
// Exactly one request went out — no corrective round was injected.
let _first = req_rx.recv().expect("first request captured");
assert!(
req_rx.recv().is_err(),
"no follow-up request when there is nothing to nudge about"
);
assert_eq!(executor.finalizes(), 1);
assert!(
!deltas
.iter()
.any(|d| matches!(d, ChatDelta::TextDelta(s) if s.contains("unresolved blocker"))),
"a clean run must never print a blocker report: {deltas:?}"
);
assert!(matches!(
deltas.last(),
Some(ChatDelta::Done {
stop_reason: StopReason::EndTurn
})
));
}
#[test]
fn blocker_present_gets_one_corrective_round_then_completes_once_fixed() {
// Turn 1: model stops with a structural blocker still on the canvas —
// the loop injects one corrective round instead of finalizing
// immediately. Turn 2: model stops again; this time the scan reports
// nothing left — straight to finalize, which also finds nothing.
let (base, req_rx) = serve_sse_script(vec![anthropic_text_turn(), anthropic_text_turn()]);
let executor = ScriptedExecutor::ok(r#"{"success":true,"data":{}}"#)
.with_blocker_check(&[("nav", "tab-profile is not bound to events.onTap yet")])
.with_blocker_check(&[]);
let cfg = base_cfg(&base, executor.clone(), 5);
let (outcome, deltas) = run_loop_collect(cfg, true);
assert_eq!(outcome, Ok(true));
// 3 checks total: the nudge round's probe (finds it), the nudge round's
// probe on the NEXT pass (confirms it's fixed), and the final tail
// check inside `finalize_and_report` (also confirms nothing left).
assert_eq!(executor.blocker_checks(), 3);
assert_eq!(
executor.finalizes(),
1,
"finalize still runs exactly once, at the real exit"
);
// The corrective round's contract line actually rode the follow-up
// request as a real turn.
let _first = req_rx.recv().expect("first request captured");
let second = req_rx
.recv()
.expect("corrective-round follow-up request captured");
assert!(
second.contains("unresolved blocker")
&& second.contains("[nav] tab-profile is not bound to events.onTap yet"),
"the corrective round must name the exact blocker, got: {second}"
);
// Nothing left unresolved after the corrective round → no tier-3 report.
assert!(
!deltas
.iter()
.any(|d| matches!(d, ChatDelta::TextDelta(s) if s.contains("unresolved blocker"))),
"a blocker the corrective round fixed must not also be reported: {deltas:?}"
);
assert!(matches!(
deltas.last(),
Some(ChatDelta::Done {
stop_reason: StopReason::EndTurn
})
));
}
#[test]
fn blocker_persisting_past_round_budget_still_reports_needs_attention() {
// The model voluntarily stops 3 times in a row and the SAME blocker is
// still present every time. `BLOCKER_NUDGE_MAX_ROUNDS` (2) must stop the
// corrective nudging after 2 rounds — the 3rd stop must NOT spend a
// 3rd nudge round, but the run must still end with an honest report,
// never a silent success.
let (base, req_rx) = serve_sse_script(vec![
anthropic_text_turn(),
anthropic_text_turn(),
anthropic_text_turn(),
]);
let executor = ScriptedExecutor::ok(r#"{"success":true,"data":{}}"#)
.with_blocker_check(&[("structure", "duplicate Explore root r1/r2")])
.with_blocker_check(&[("structure", "duplicate Explore root r1/r2")])
.with_blocker_check(&[("structure", "duplicate Explore root r1/r2")]);
// Turn budget is generous — neither nudge round counts against `turn`,
// so ONLY the round budget, not the ordinary turn cap, ends this.
let cfg = base_cfg(&base, executor.clone(), 5);
let (outcome, deltas) = run_loop_collect(cfg, true);
assert_eq!(outcome, Ok(true));
// 3 checks: round 1 (nudge), round 2 (nudge), round 3 (budget spent —
// the gate short-circuits before probing again for the NUDGE decision,
// but the final tail check inside `finalize_and_report` still runs
// once more) — so exactly 3 scripted values get consumed.
assert_eq!(executor.blocker_checks(), 3);
assert_eq!(executor.finalizes(), 1);
// Only 2 corrective requests went out (round 1 and round 2); the 3rd
// model stop goes straight to finalize+Done — no 3rd nudge request.
let _first = req_rx.recv().expect("initial request");
let second = req_rx.recv().expect("round 1 corrective request");
assert!(second.contains("unresolved blocker"));
let third = req_rx.recv().expect("round 2 corrective request");
assert!(third.contains("unresolved blocker"));
assert!(
req_rx.recv().is_err(),
"the round budget must stop a 3rd corrective request from going out"
);
let report = deltas.iter().find_map(|d| match d {
ChatDelta::TextDelta(s) if s.contains("unresolved blocker") => Some(s.clone()),
_ => None,
});
assert!(
report.is_some_and(|s| s.contains("duplicate Explore root r1/r2")),
"a blocker still present after exhausting its round budget must be reported, not silently dropped: {deltas:?}"
);
assert!(matches!(
deltas.last(),
Some(ChatDelta::Done {
stop_reason: StopReason::EndTurn
})
));
}

View file

@ -14,7 +14,10 @@ use std::time::Duration;
use super::*;
use crate::chat_runtime::shared_runtime;
use base64::Engine as _;
use op_ai::chat_provider::{ChatToolDef, ChatToolExecutor, ChatToolResult, UnfilledScreensReport};
use op_ai::chat_provider::{
BlockerEntry, BlockerReport, ChatToolDef, ChatToolExecutor, ChatToolResult,
UnfilledScreensReport,
};
/// Executor double — records calls + loop-finalize invocations, replays a
/// fixed result.
@ -29,6 +32,10 @@ pub(super) struct ScriptedExecutor {
unfilled_checks: Mutex<VecDeque<UnfilledScreensReport>>,
/// Scripted `finalize` return values, popped the same way.
unfilled_finalizes: Mutex<VecDeque<UnfilledScreensReport>>,
/// Scripted `check_blockers` return values, popped in call order; once
/// exhausted, further calls return the default (empty) report.
blocker_checks: Mutex<VecDeque<BlockerReport>>,
blocker_check_calls: std::sync::atomic::AtomicUsize,
}
impl ScriptedExecutor {
@ -52,6 +59,8 @@ impl ScriptedExecutor {
),
unfilled_checks: Mutex::new(VecDeque::new()),
unfilled_finalizes: Mutex::new(VecDeque::new()),
blocker_checks: Mutex::new(VecDeque::new()),
blocker_check_calls: std::sync::atomic::AtomicUsize::new(0),
})
}
@ -89,6 +98,23 @@ impl ScriptedExecutor {
.push_back(report_of(committed, unfilled));
self
}
/// Queue one scripted `check_blockers` return value. `entries` is
/// `(category, detail)` pairs.
pub(super) fn with_blocker_check(self: Arc<Self>, entries: &[(&str, &str)]) -> Arc<Self> {
self.blocker_checks
.lock()
.unwrap()
.push_back(blocker_report_of(entries));
self
}
/// How many times the loop ran the cheap read-only unresolved-blocker
/// probe (`check_blockers`).
pub(super) fn blocker_checks(&self) -> usize {
self.blocker_check_calls
.load(std::sync::atomic::Ordering::SeqCst)
}
}
fn report_of(committed: &[&str], unfilled: &[&str]) -> UnfilledScreensReport {
@ -98,6 +124,18 @@ fn report_of(committed: &[&str], unfilled: &[&str]) -> UnfilledScreensReport {
}
}
fn blocker_report_of(entries: &[(&str, &str)]) -> BlockerReport {
BlockerReport {
blockers: entries
.iter()
.map(|(category, detail)| BlockerEntry {
category: category.to_string(),
detail: detail.to_string(),
})
.collect(),
}
}
impl ChatToolExecutor for ScriptedExecutor {
fn execute(&self, name: &str, args_json: &str) -> ChatToolResult {
self.calls
@ -131,6 +169,16 @@ impl ChatToolExecutor for ScriptedExecutor {
.pop_front()
.unwrap_or_default()
}
fn check_blockers(&self) -> BlockerReport {
self.blocker_check_calls
.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
self.blocker_checks
.lock()
.unwrap()
.pop_front()
.unwrap_or_default()
}
}
fn read_http_request(stream: &mut TcpStream) -> String {

View file

@ -924,7 +924,7 @@ const CONTRAST_ICON_TARGET: f64 = 3.0;
/// empty AppContent and built everything in a second `Explore` — the user
/// sees a blank artboard mid-run). Finalize's duplicate-root pass repairs
/// the END state, but the in-loop model should merge NOW.
fn scan_duplicate_root_issues(nodes: &[PenNode]) -> Vec<String> {
pub(crate) fn scan_duplicate_root_issues(nodes: &[PenNode]) -> Vec<String> {
use std::collections::HashMap;
let mut by_name: HashMap<&str, Vec<&PenNode>> = HashMap::new();
for node in nodes {
@ -974,7 +974,7 @@ fn scan_duplicate_root_issues(nodes: &[PenNode]) -> Vec<String> {
/// as a SIBLING — the bell floats alone in a full-width strip above the
/// greeting (measured: "Header Row" = [bell], "Good evening" outside it,
/// test0711-22). Which text belongs in the row is intent, so this echoes.
fn scan_header_icon_row_issues(nodes: &[PenNode]) -> Vec<String> {
pub(crate) fn scan_header_icon_row_issues(nodes: &[PenNode]) -> Vec<String> {
let mut out = Vec::new();
fn walk(nodes: &[PenNode], out: &mut Vec<String>) {
for node in nodes {
@ -1012,7 +1012,7 @@ fn scan_header_icon_row_issues(nodes: &[PenNode]) -> Vec<String> {
out
}
fn scan_empty_shells(nodes: &[PenNode]) -> Vec<String> {
pub(crate) fn scan_empty_shells(nodes: &[PenNode]) -> Vec<String> {
let mut out = Vec::new();
fn walk(nodes: &[PenNode], out: &mut Vec<String>) {
for node in nodes {
@ -1025,7 +1025,11 @@ fn scan_empty_shells(nodes: &[PenNode]) -> Vec<String> {
&& !named.is_empty()
&& node.base().role.as_deref() != Some("status-bar")
{
out.push(named.to_string());
// Carry the node id alongside the name (matches the other
// structural scans' shape) so a loop-end corrective nudge
// can name a specific, D()-able / M()-able target instead
// of a possibly-ambiguous name alone.
out.push(format!("{named} ({})", node.id_str()));
} else {
walk(children, out);
}
@ -1036,7 +1040,7 @@ fn scan_empty_shells(nodes: &[PenNode]) -> Vec<String> {
out
}
fn scan_ring_issues(nodes: &[PenNode]) -> Vec<String> {
pub(crate) fn scan_ring_issues(nodes: &[PenNode]) -> Vec<String> {
const MIN_RING_SIZE: f64 = 48.0;
const HAIRLINE: f32 = 2.5;
let mut out = op_design_lint::detect_missing_progress_rings(nodes)

View file

@ -59,6 +59,7 @@ pub mod export;
pub mod export_pdf;
mod figma_convert;
mod import_html_url;
pub mod loop_blocker_ledger;
pub mod mcp_live;
pub mod mcp_serve;
pub mod model_discovery;

View file

@ -0,0 +1,82 @@
//! Unresolved-blocker detection for agent-loop completion gating.
//!
//! [`detect_blockers`] is a LIVE recompute against the current `EditorState`
//! — deliberately NOT an accumulating ledger populated incrementally by
//! `design_agent_tools.rs`'s per-`batch_design` diagnostics. It mirrors the
//! discipline `op_orchestrator::unfilled_screens::detect_unfilled_screens`
//! already established for `ChatToolExecutor::check_unfilled_screens`: run
//! the same scans fresh every time, straight off the live document, so an
//! issue a later batch already fixed simply stops appearing — nothing to
//! prune, nothing that can go stale.
//!
//! Scope (MVP, conservative): only failure modes with an unambiguous
//! structural signal count as blockers —
//!
//! - **structure** — duplicate status bars / broken rings / duplicate
//! header-icon rows (`design_agent_tools::scan_duplicate_root_issues` /
//! `scan_ring_issues` / `scan_header_icon_row_issues` — the same scans
//! whose hits ride into `batch_design`'s `structureIssues` field).
//! - **empty-shell** — a scaffolded section never filled
//! (`design_agent_tools::scan_empty_shells`, `shellsRemaining` per-batch).
//! - **nav** — an unbound primary-mobile-screen nav tab
//! (`op_orchestrator::nav_issues::scan_nav_issues`, `navIssues` per-batch).
//!
//! `layoutIssues` (`op_orchestrator::geometry_validation::geometry_diagnostics`)
//! is deliberately EXCLUDED from this MVP: its ~15 detectors (overflow /
//! collapse / jam / starvation / spill / …) all return the same flat
//! `Vec<String>` with no structured category field, so classifying "which of
//! these are overflow/collapse" would mean pattern-matching free-form
//! message text — exactly the name/string-heuristic fragility this codebase
//! has been burned by before. `layoutIssues` stays advisory (it still rides
//! in the per-batch tool result, unaffected by this module) until
//! `geometry_validation` grows a real category enum on its diagnostics.
use op_ai::chat_provider::{BlockerEntry, BlockerReport};
use op_editor_core::EditorState;
use crate::design_agent_tools::{
scan_duplicate_root_issues, scan_empty_shells, scan_header_icon_row_issues, scan_ring_issues,
};
/// Recompute every unresolved blocker against the CURRENT active page of
/// `state` — same scope `design_agent_tools`'s per-batch diagnostics use.
/// Read-only: never mutates the document.
pub fn detect_blockers(state: &EditorState) -> BlockerReport {
let children = state.active_children();
let mut blockers = Vec::new();
for detail in scan_duplicate_root_issues(children) {
blockers.push(BlockerEntry {
category: "structure".to_string(),
detail,
});
}
for detail in scan_ring_issues(children) {
blockers.push(BlockerEntry {
category: "structure".to_string(),
detail,
});
}
for detail in scan_header_icon_row_issues(children) {
blockers.push(BlockerEntry {
category: "structure".to_string(),
detail,
});
}
for detail in scan_empty_shells(children) {
blockers.push(BlockerEntry {
category: "empty-shell".to_string(),
detail,
});
}
for detail in op_orchestrator::nav_issues::scan_nav_issues(state) {
blockers.push(BlockerEntry {
category: "nav".to_string(),
detail,
});
}
BlockerReport { blockers }
}
#[cfg(test)]
#[path = "loop_blocker_ledger_tests.rs"]
mod tests;

View file

@ -0,0 +1,140 @@
//! Tests for the unresolved-blocker completion-gate scan — see
//! `loop_blocker_ledger.rs` module doc for scope (structure / empty-shell /
//! nav are blockers; layoutIssues stays advisory and out of scope here).
use super::*;
use jian_ops_schema::PenDocument;
use op_editor_core::pen_node_ext::PenNodeExt;
fn state_from_json(json: &str) -> EditorState {
let doc: PenDocument = serde_json::from_str(json).expect("valid PenDocument");
EditorState::from_document(doc)
}
#[test]
fn clean_document_has_no_blockers() {
let state = state_from_json(
r##"{ "version": "1.0", "children": [
{ "type": "frame", "id": "home", "name": "Home", "width": 390, "height": 844,
"children": [ { "type": "text", "id": "t1", "content": "Hello" } ] }
] }"##,
);
let report = detect_blockers(&state);
assert!(!report.has_blockers(), "{report:?}");
}
#[test]
fn duplicate_top_level_frame_is_a_structure_blocker() {
// Same test0711-1-m3.op shape `duplicate_root_tests` covers in
// design_agent_tools.rs: model abandoned the first `Explore` and
// rebuilt everything in a second one of the same name.
let state = state_from_json(
r##"{ "version": "1.0", "children": [
{ "type": "frame", "id": "r1", "name": "Explore", "width": 390, "height": 844,
"children": [ { "type": "frame", "id": "empty", "name": "AppContent",
"width": "fill_container", "height": "fit_content" } ] },
{ "type": "frame", "id": "r2", "name": "Explore", "width": 390,
"height": "fit_content",
"children": [ { "type": "frame", "id": "rich", "name": "AppContent",
"width": "fill_container", "height": "fit_content" } ] }
] }"##,
);
let report = detect_blockers(&state);
assert!(report.has_blockers());
let hit = report
.blockers
.iter()
.find(|b| b.category == "structure")
.expect("a structure blocker");
assert!(
hit.detail.contains("Explore") && hit.detail.contains("r1") && hit.detail.contains("r2")
);
}
#[test]
fn empty_named_shell_is_an_empty_shell_blocker() {
let state = state_from_json(
r##"{ "version": "1.0", "children": [
{ "type": "frame", "id": "home", "name": "Home", "width": 390, "height": 844,
"children": [
{ "type": "frame", "id": "section", "name": "RecentActivity",
"width": "fill_container", "height": "fit_content", "children": [] }
] }
] }"##,
);
let report = detect_blockers(&state);
assert!(report.has_blockers());
let hit = report
.blockers
.iter()
.find(|b| b.category == "empty-shell")
.expect("an empty-shell blocker");
// Must carry a concrete, locatable node id alongside the name — a
// corrective nudge built from name alone could be ambiguous.
assert!(hit.detail.contains("RecentActivity"), "{hit:?}");
assert!(hit.detail.contains("section"), "{hit:?}");
}
#[test]
fn unbound_nav_tab_is_a_nav_blocker() {
// Mirrors op_orchestrator::nav_issues_tests's
// TWO_SCREENS_UNBOUND_PROFILE_TAB fixture: two screen-marked frames, the
// "Profile" tab in Home's bottom nav has no events bound yet.
let state = state_from_json(
r##"{ "version": "1.0", "children": [
{ "type": "frame", "id": "home", "name": "Home", "screen": "/",
"width": 390, "height": 844, "layout": "vertical",
"children": [
{ "type": "frame", "id": "nav", "name": "Bottom Nav", "role": "bottom-tab-bar",
"layout": "horizontal", "width": "fill_container",
"children": [
{ "type": "frame", "id": "tab-home", "layout": "vertical",
"events": { "onTap": [ { "replace": "\"/\"" } ] },
"children": [ { "type": "text", "id": "t1", "content": "Home" } ] },
{ "type": "frame", "id": "tab-profile", "layout": "vertical",
"children": [ { "type": "text", "id": "t2", "content": "Profile" } ] }
] }
] },
{ "type": "frame", "id": "profile", "name": "Profile", "screen": "/profile",
"width": 390, "height": 844 }
] }"##,
);
let report = detect_blockers(&state);
assert!(report.has_blockers());
let hit = report
.blockers
.iter()
.find(|b| b.category == "nav")
.expect("a nav blocker");
assert!(hit.detail.contains("tab-profile"), "{hit:?}");
}
#[test]
fn fixing_the_document_makes_the_blocker_disappear() {
// The "no accumulating ledger" contract: re-running the SAME scan
// against a document where the issue was fixed must report nothing —
// there is no stale entry to prune because nothing was ever stored.
let mut state = state_from_json(
r##"{ "version": "1.0", "children": [
{ "type": "frame", "id": "home", "name": "Home", "width": 390, "height": 844,
"children": [
{ "type": "frame", "id": "section", "name": "RecentActivity",
"width": "fill_container", "height": "fit_content", "children": [] }
] }
] }"##,
);
assert!(detect_blockers(&state).has_blockers());
// Fill the shell with real content.
let home = &mut state.active_children_mut()[0];
let section = &mut home.children_mut().unwrap()[0];
*section.children_mut().unwrap() = vec![serde_json::from_value(serde_json::json!(
{ "type": "text", "id": "t1", "content": "No recent activity" }
))
.expect("text node")];
assert!(
!detect_blockers(&state).has_blockers(),
"fixed shell must not still be reported"
);
}