From 087400f41847f93ea8920a610da5d5d6c35d9799 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Sat, 15 Aug 2026 23:53:59 +0200 Subject: [PATCH 1/2] fix(broker): model the startup gate as a verdict, not two booleans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cargo clippy -- -D warnings` fails on main: `try_emit_worker_ready` has 8 arguments against a limit of 7. My own #1529 added the eighth. Local `cargo test` does not run clippy, which is why it reached main. Rather than silence the lint, this collapses the two booleans it flagged into the tri-state they always were: enum StartupGate { Ready, Unrecognised, Blocked } That is back to 7 arguments and strictly better modelling. `ready && blocked` was representable and meaningless, and the distinction that actually governs behaviour — a heuristic that failed to recognise the prompt may be overridden by the deadline, a harness deliberately refusing input never may — now lives in the type instead of a comment two call frames away. No behaviour change: `Ready` is the old `startup_ready`, `Blocked` the old `startup_blocked`, `Unrecognised` the remaining case. Both readiness tests still pass, including the veto guard added in review. Verified: `cargo clippy -p agent-relay-broker --lib -- -D warnings` clean, 24 passed / 0 failed. Co-Authored-By: Claude Opus 5 --- crates/broker/src/pty_worker.rs | 72 ++++++++++++++++++++++++--------- 1 file changed, 52 insertions(+), 20 deletions(-) diff --git a/crates/broker/src/pty_worker.rs b/crates/broker/src/pty_worker.rs index ca95365c9..3f3452f1e 100644 --- a/crates/broker/src/pty_worker.rs +++ b/crates/broker/src/pty_worker.rs @@ -308,6 +308,24 @@ fn evaluate_startup_gate( /// interstitial means the harness is deliberately not accepting work yet, and /// typing into it would answer a security question on the operator's behalf. /// `evaluate_startup_gate` vetoes it for exactly that reason. +/// The startup gate's verdict. +/// +/// Three states, not two booleans: `Ready && Blocked` is not representable, +/// and the difference between "we could not recognise the prompt" and "the +/// harness is deliberately refusing work" decides whether the deadline may +/// release queued work at all. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum StartupGate { + /// A real input prompt was proven. + Ready, + /// No prompt recognised yet. Our heuristic may simply be blind, so the + /// deadline is allowed to release queued work. + Unrecognised, + /// A known blocking dialog is on screen — the harness is deliberately not + /// accepting input. Never released by any deadline. + Blocked, +} + fn startup_gate_blocked(pty: &PtySession) -> bool { detect_codex_trust_prompt(&pty.screen_text()) } @@ -460,8 +478,7 @@ async fn try_emit_worker_ready( init_request_id: &mut Option, init_received_at: Option, readiness: &mut StartupReadinessState, - startup_ready: bool, - startup_blocked: bool, + gate: StartupGate, ) { // init_received_at is Some only after init_worker has been received. // We use it (not init_request_id) as the gate because the broker sends @@ -473,7 +490,11 @@ async fn try_emit_worker_ready( // A deliberate veto is not a blind spot: never time out past a known // blocking dialog, or the brief is typed into a trust prompt and answers a // security question nobody asked us to answer. - let timed_out = !startup_blocked + let startup_ready = gate == StartupGate::Ready; + // A deliberate veto is not a blind spot: never time out past a known + // blocking dialog, or the brief is typed into a trust prompt and answers a + // security question nobody asked us to answer. + let timed_out = gate != StartupGate::Blocked && init_received_at.is_some_and(|started| started.elapsed() >= STARTUP_READY_TIMEOUT); if !startup_ready && !timed_out { @@ -842,7 +863,13 @@ pub(crate) async fn run_pty_worker(cmd: PtyCommand) -> Result<()> { &post_boot_output, &pty, ); - let startup_blocked = startup_gate_blocked(&pty); + let gate = if startup_ready { + StartupGate::Ready + } else if startup_gate_blocked(&pty) { + StartupGate::Blocked + } else { + StartupGate::Unrecognised + }; try_emit_worker_ready( &out_tx, &worker_name, @@ -850,8 +877,7 @@ pub(crate) async fn run_pty_worker(cmd: PtyCommand) -> Result<()> { &mut init_request_id, init_received_at, &mut startup_readiness, - startup_ready, - startup_blocked, + gate, ) .await; } @@ -1236,7 +1262,13 @@ pub(crate) async fn run_pty_worker(cmd: PtyCommand) -> Result<()> { &post_boot_output, &pty, ); - let startup_blocked = startup_gate_blocked(&pty); + let gate = if startup_ready { + StartupGate::Ready + } else if startup_gate_blocked(&pty) { + StartupGate::Blocked + } else { + StartupGate::Unrecognised + }; try_emit_worker_ready( &out_tx, &worker_name, @@ -1244,8 +1276,7 @@ pub(crate) async fn run_pty_worker(cmd: PtyCommand) -> Result<()> { &mut init_request_id, init_received_at, &mut startup_readiness, - startup_ready, - startup_blocked, + gate, ) .await; @@ -1808,7 +1839,13 @@ pub(crate) async fn run_pty_worker(cmd: PtyCommand) -> Result<()> { &post_boot_output, &pty, ); - let startup_blocked = startup_gate_blocked(&pty); + let gate = if startup_ready { + StartupGate::Ready + } else if startup_gate_blocked(&pty) { + StartupGate::Blocked + } else { + StartupGate::Unrecognised + }; try_emit_worker_ready( &out_tx, &worker_name, @@ -1816,8 +1853,7 @@ pub(crate) async fn run_pty_worker(cmd: PtyCommand) -> Result<()> { &mut init_request_id, init_received_at, &mut startup_readiness, - startup_ready, - startup_blocked, + gate, ) .await; @@ -2231,8 +2267,7 @@ mod tests { &mut request_id, Some(started), &mut readiness, - false, // prompt never recognised - false, // and no blocking dialog on screen + StartupGate::Unrecognised, ) .await; @@ -2265,8 +2300,7 @@ mod tests { &mut request_id, Some(started), &mut readiness, - false, // prompt not ready - true, // ...because a known blocking dialog is on screen + StartupGate::Blocked, ) .await; @@ -2294,8 +2328,7 @@ mod tests { &mut request_id, Some(started), &mut readiness, - false, - false, + StartupGate::Unrecognised, ) .await; @@ -2316,8 +2349,7 @@ mod tests { &mut request_id, Some(started), &mut readiness, - true, - false, + StartupGate::Ready, ) .await; From b05ad4ff5cb7a59706882953debabdbea07c3cf5 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Sun, 16 Aug 2026 09:38:36 +0200 Subject: [PATCH 2/2] fix(broker): separate the gate docs, dedupe the veto note, realign blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review on #1532. All three findings were valid. Doc ownership: the enum was inserted between `startup_gate_blocked`'s doc comment and the function, so the enum silently absorbed documentation written for something else and the function was left undocumented. Each now carries its own. Duplicated comment: the deliberate-veto explanation appeared above both `startup_ready` and `timed_out`. Kept once, above the timeout calculation it actually describes. Indentation: two of the three `gate` blocks sat at 32 spaces against surroundings at 16 — my edit used a fixed indent instead of matching context. Realigned to the call they precede. Worth recording about that last one: the review predicted `cargo fmt -- --check` would fail. It does not — it passes clean both before and after, because rustfmt skips this function (it is inside `tokio::select!`). The diagnosis was right and the predicted symptom was wrong, so a green fmt gate is not evidence of correct formatting here. Trusting it would have justified leaving the code sloppy. Verified: fmt clean, `cargo clippy -- -D warnings` clean, 24 passed / 0 failed including both readiness guards. Co-Authored-By: Claude Opus 5 --- crates/broker/src/pty_worker.rs | 47 +++++++++++++++------------------ 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/crates/broker/src/pty_worker.rs b/crates/broker/src/pty_worker.rs index 3f3452f1e..8a08a0387 100644 --- a/crates/broker/src/pty_worker.rs +++ b/crates/broker/src/pty_worker.rs @@ -300,14 +300,6 @@ fn evaluate_startup_gate( } } -/// Whether a KNOWN blocking dialog is on screen, as opposed to a prompt we -/// simply failed to recognise. -/// -/// The two must not be conflated. An unrecognised prompt means our heuristic is -/// blind and the timeout should eventually release the brief anyway. A trust -/// interstitial means the harness is deliberately not accepting work yet, and -/// typing into it would answer a security question on the operator's behalf. -/// `evaluate_startup_gate` vetoes it for exactly that reason. /// The startup gate's verdict. /// /// Three states, not two booleans: `Ready && Blocked` is not representable, @@ -326,6 +318,14 @@ pub(crate) enum StartupGate { Blocked, } +/// Whether a KNOWN blocking dialog is on screen, as opposed to a prompt we +/// simply failed to recognise. +/// +/// The two must not be conflated. An unrecognised prompt means our heuristic is +/// blind and the timeout should eventually release the brief anyway. A trust +/// interstitial means the harness is deliberately not accepting work yet, and +/// typing into it would answer a security question on the operator's behalf. +/// `evaluate_startup_gate` vetoes it for exactly that reason. fn startup_gate_blocked(pty: &PtySession) -> bool { detect_codex_trust_prompt(&pty.screen_text()) } @@ -487,9 +487,6 @@ async fn try_emit_worker_ready( return; } - // A deliberate veto is not a blind spot: never time out past a known - // blocking dialog, or the brief is typed into a trust prompt and answers a - // security question nobody asked us to answer. let startup_ready = gate == StartupGate::Ready; // A deliberate veto is not a blind spot: never time out past a known // blocking dialog, or the brief is typed into a trust prompt and answers a @@ -1262,13 +1259,13 @@ pub(crate) async fn run_pty_worker(cmd: PtyCommand) -> Result<()> { &post_boot_output, &pty, ); - let gate = if startup_ready { - StartupGate::Ready - } else if startup_gate_blocked(&pty) { - StartupGate::Blocked - } else { - StartupGate::Unrecognised - }; + let gate = if startup_ready { + StartupGate::Ready + } else if startup_gate_blocked(&pty) { + StartupGate::Blocked + } else { + StartupGate::Unrecognised + }; try_emit_worker_ready( &out_tx, &worker_name, @@ -1839,13 +1836,13 @@ pub(crate) async fn run_pty_worker(cmd: PtyCommand) -> Result<()> { &post_boot_output, &pty, ); - let gate = if startup_ready { - StartupGate::Ready - } else if startup_gate_blocked(&pty) { - StartupGate::Blocked - } else { - StartupGate::Unrecognised - }; + let gate = if startup_ready { + StartupGate::Ready + } else if startup_gate_blocked(&pty) { + StartupGate::Blocked + } else { + StartupGate::Unrecognised + }; try_emit_worker_ready( &out_tx, &worker_name,