From fadfac14ad8e7bbcfaa0c511d20bb49db35142c2 Mon Sep 17 00:00:00 2001 From: Stella Test Date: Thu, 6 Aug 2026 04:00:43 -0700 Subject: [PATCH 1/3] fix(stella-cli): record a deliberate stop distinctly on the unsupervised writers too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #1828 (#1653) taught the supervised registry writers to record SessionStatus::Stopped for a policy stop, but SessionPresence::finish and its callers still collapsed the outcome to a bool on the way in — so an UNSUPERVISED headless run (a pipe, CI, --foreground) that ended itself by policy still aged into the SESSIONS overlay as Error, indistinguishable from a crash, with no later supervised write to correct it. Widen SessionPresence::finish (and one_shot_notification's wording) past the bool: the caller now hands it the terminal SessionStatus, projected by the one existing decider (daemon::outcome_status) via a fifth projection in agent/outcome.rs, pipeline_session_status, so every writer reads a deliberate stop (Stopped), an interrupt (Cancelled), and a crash (Error) the same way. The goal loop still answers with a String that cannot carry the abort kind, and the deck's session_exit reads the same stringly run_lead_turn — both audited and filed as #1862. Witness: outcome::tests::an_unsupervised_deliberate_stop_projects_stopped_not_error (structural — the projection did not exist and the widened finish call sites do not compile on the old signature, mirroring the #1653 witness). Closes #1826 Refs #1653 Refs #1862 --- crates/stella-cli/src/agent.rs | 12 ++-- crates/stella-cli/src/agent/goal.rs | 18 ++++- crates/stella-cli/src/agent/outcome.rs | 96 +++++++++++++++++++++++-- crates/stella-cli/src/agent/presence.rs | 47 ++++++++---- crates/stella-cli/src/daemon.rs | 13 ++-- 5 files changed, 152 insertions(+), 34 deletions(-) diff --git a/crates/stella-cli/src/agent.rs b/crates/stella-cli/src/agent.rs index a507c72a9..28fba98fe 100644 --- a/crates/stella-cli/src/agent.rs +++ b/crates/stella-cli/src/agent.rs @@ -65,8 +65,8 @@ pub(crate) use graph::spawn_session_graph; #[cfg(test)] use graph::{GraphSummary, format_graph_stats, index_workspace_graph_blocking}; use outcome::{ - pipeline_episode_outcome, pipeline_failure_reason, pipeline_status_label, - pipeline_status_result, + pipeline_episode_outcome, pipeline_failure_reason, pipeline_session_status, + pipeline_status_label, pipeline_status_result, }; pub(crate) use outcome::{pipeline_execution_closeout, settled_cost_since}; use output::*; @@ -605,10 +605,10 @@ async fn run_pipeline_one_shot( // always lands a notification; a successful one only when it ran long // enough that the user has plausibly looked away. `Enter` on the // notification (or the SESSIONS overlay) replays the journal. - let run_ok = matches!(&result, Ok(o) if matches!(o.status, PipelineStatus::Completed)); + let session_status = pipeline_session_status(&result); let run_secs = turn_start.elapsed().as_secs(); - let notify = presence.one_shot_notification(run_ok, run_secs, prompt); - presence.finish(run_ok, notify); + let notify = presence.one_shot_notification(session_status, run_secs, prompt); + presence.finish(session_status, notify); match &result { Ok(outcome) => { @@ -1122,7 +1122,7 @@ pub async fn run_interactive(cfg: &Config, budget_limit: Option) -> Result< if let Some(set) = &mcp { set.close_all().await; } - presence.finish(true, None); + presence.finish(stella_store::SessionStatus::Complete, None); println!("\n {}", "Goodbye! ✦".magenta()); Ok(()) } diff --git a/crates/stella-cli/src/agent/goal.rs b/crates/stella-cli/src/agent/goal.rs index 1061f574a..ba9ff5974 100644 --- a/crates/stella-cli/src/agent/goal.rs +++ b/crates/stella-cli/src/agent/goal.rs @@ -182,8 +182,11 @@ pub(crate) async fn run_raw_one_shot( // `SessionPresence::one_shot_notification`, shared with the pipeline path). let run_secs = u64::try_from(crate::memory::unix_now_secs().saturating_sub(started_unix)).unwrap_or(0); - let notify = presence.one_shot_notification(outcome.is_ok(), run_secs, prompt); - presence.finish(outcome.is_ok(), notify); + // The failure itself, not a bool: an unsupervised deliberate stop must + // reach the registry as `Stopped`, never age into it as a crash (#1826). + let status = crate::daemon::outcome_status(outcome.as_ref().map(|_| ())); + let notify = presence.one_shot_notification(status, run_secs, prompt); + presence.finish(status, notify); outcome } @@ -372,8 +375,17 @@ pub async fn run_goal_cmd( } else { format!("{}: goal run FAILED", presence.name()) }; + // The goal loop still answers with a `String`, which has no room for the + // abort's typed kind (#1637's collapse one level deeper — #1862), so a + // policy-stopped goal round can only record `Error` here. Projected + // through `outcome_status` all the same, so a user interrupt records + // `Cancelled` rather than aging into a crash (#1826). + let terminal = outcome + .as_ref() + .map(|_| ()) + .map_err(|e| crate::failure::CliFailure::error(e.clone())); presence.finish( - outcome.is_ok(), + crate::daemon::outcome_status(terminal.as_ref().map(|_| ())), Some((notify, crate::command_deck::prompt_line(goal, 160))), ); outcome diff --git a/crates/stella-cli/src/agent/outcome.rs b/crates/stella-cli/src/agent/outcome.rs index 65d8b6610..8b5af2550 100644 --- a/crates/stella-cli/src/agent/outcome.rs +++ b/crates/stella-cli/src/agent/outcome.rs @@ -1,9 +1,9 @@ //! One reading of a finished pipeline run, shared by every surface. //! -//! A [`PipelineStatus`] has to be projected four different ways — a store -//! label, a JSON `reason`, an episodic-memory outcome, and the process exit -//! `Result` — and each surface (one-shot, deck, fleet, arena) needs the same -//! projections. Keeping them here, as total `match`es over the enum, is what +//! A [`PipelineStatus`] has to be projected five different ways — a store +//! label, a JSON `reason`, an episodic-memory outcome, the process exit +//! `Result`, and the terminal SESSIONS-registry status — and each surface +//! (one-shot, deck, fleet, arena) needs the same projections. Keeping them here, as total `match`es over the enum, is what //! stops `stella run --output-format json` and the audit row from disagreeing //! about whether the same run passed. //! @@ -79,6 +79,27 @@ pub(super) fn pipeline_status_result(status: &PipelineStatus) -> Result<(), CliF } } +/// The terminal SESSIONS-registry status a finished pipeline run records — +/// the fifth projection, feeding `SessionPresence::finish` on the +/// unsupervised paths (`stella run` in a pipe, CI, `--foreground`) that +/// `crate::daemon::record_outcome_if_supervised` never reaches (#1826). +/// +/// Routed through [`crate::daemon::outcome_status`] rather than re-matched +/// here, so every registry writer — supervised or not — reads a deliberate +/// stop (`Stopped`), a user interrupt (`Cancelled`), and a crash (`Error`) +/// off the same decider (#1653). +pub(super) fn pipeline_session_status( + result: &Result, +) -> stella_store::SessionStatus { + let terminal = match result { + Ok(outcome) => pipeline_status_result(&outcome.status), + // A hard pipeline error carries no abort kind — a genuine failure, + // the same reading the process exit gives it. + Err(error) => Err(CliFailure::error(error.to_string())), + }; + crate::daemon::outcome_status(terminal.as_ref().map(|_| ())) +} + /// The process-boundary answer a finished engine turn owes `main`. /// /// The one projection from [`TurnOutcome`] to an exit code, so every surface @@ -128,6 +149,73 @@ mod tests { assert_eq!(settled_cost_since(1.25, 1.25), 0.0); } + /// A pipeline run terminal enough to project, with every field the + /// projection ignores held constant. + fn pipeline_outcome(status: PipelineStatus) -> PipelineOutcome { + PipelineOutcome { + status, + task_class: stella_pipeline::TaskClass::SingleTask, + final_text: String::new(), + total_cost_usd: 0.0, + verdict: None, + score: None, + revisions: 0, + candidates_run: 1, + } + } + + /// #1826 witness — the presence half of #1653: the projection the + /// unsupervised registry writers feed `SessionPresence::finish` + /// distinguishes a deliberate stop from a crash. Before, `finish(ok: + /// bool, …)` collapsed both to [`stella_store::SessionStatus::Error`] — + /// this projection did not exist, and the widened `finish` call sites do + /// not compile against the old signature. + /// + /// Like `daemon::tests::a_deliberate_stop_records_a_status_distinct_from_a_crash`, + /// this relies on no test in this binary ever calling + /// `signals::note_interrupt`, so `interrupted_exit_code()` is reliably + /// `None` here. + #[test] + fn an_unsupervised_deliberate_stop_projects_stopped_not_error() { + let stopped = Ok(pipeline_outcome(PipelineStatus::Aborted { + reason: "stuck-loop detected (persisted after a steering warning)".into(), + kind: AbortKind::DeliberateStop, + })); + let crashed = Ok(pipeline_outcome(PipelineStatus::Aborted { + reason: "the model call would not commit after retries".into(), + kind: AbortKind::Failure, + })); + + assert_eq!( + pipeline_session_status(&stopped), + stella_store::SessionStatus::Stopped, + "a run that ended itself by policy must not read as a crash" + ); + assert_eq!( + pipeline_session_status(&crashed), + stella_store::SessionStatus::Error + ); + } + + /// The arms the widening must not disturb: a completed run stays + /// `Complete`, and a hard pipeline error — which carries no abort kind — + /// stays `Error`. + #[test] + fn the_remaining_terminal_arms_project_unchanged() { + assert_eq!( + pipeline_session_status(&Ok(pipeline_outcome(PipelineStatus::Completed))), + stella_store::SessionStatus::Complete + ); + let hard = Err(PipelineRunError { + cause: PipelineError::ScopeReviewRequiredHeadless, + total_cost_usd: 0.0, + }); + assert_eq!( + pipeline_session_status(&hard), + stella_store::SessionStatus::Error + ); + } + #[test] fn a_deliberately_stopped_turn_carries_its_own_exit_code() { let failure = turn_outcome_result(&TurnOutcome::Aborted { diff --git a/crates/stella-cli/src/agent/presence.rs b/crates/stella-cli/src/agent/presence.rs index 4d08c4a5f..c533ec067 100644 --- a/crates/stella-cli/src/agent/presence.rs +++ b/crates/stella-cli/src/agent/presence.rs @@ -81,20 +81,30 @@ impl SessionPresence { /// The headless one-shot → `/inbox` notification decision, shared by the /// pipeline and raw (`--no-pipeline`) paths so the two cannot drift: a - /// failed run always lands a notification; a successful one only when it - /// ran long enough (60s) that the user has plausibly looked away. + /// run that did not complete always lands a notification, worded by how + /// it actually ended — a policy stop is deliberate, and "FAILED" for it + /// was the same dishonesty the registry status carried (#1826); a + /// successful one notifies only when it ran long enough (60s) that the + /// user has plausibly looked away. pub(crate) fn one_shot_notification( &self, - run_ok: bool, + status: stella_store::SessionStatus, run_secs: u64, prompt: &str, ) -> Option<(String, String)> { - let title = if !run_ok { - format!("{}: run FAILED", self.name()) - } else if run_secs >= 60 { - format!("{}: run finished ({run_secs}s)", self.name()) - } else { - return None; + let title = match status { + stella_store::SessionStatus::Complete if run_secs >= 60 => { + format!("{}: run finished ({run_secs}s)", self.name()) + } + stella_store::SessionStatus::Complete => return None, + stella_store::SessionStatus::Stopped => { + format!("{}: run stopped by policy", self.name()) + } + stella_store::SessionStatus::Cancelled => { + format!("{}: run cancelled", self.name()) + } + // Anything else a terminal write carries is a genuine failure. + _ => format!("{}: run FAILED", self.name()), }; Some((title, crate::command_deck::prompt_line(prompt, 160))) } @@ -122,12 +132,19 @@ impl SessionPresence { /// notification linked to this session — the headless → `/inbox` flow: /// a finished `stella run` surfaces in every deck's inbox, and `Enter` /// replays it. - pub(crate) fn finish(&mut self, ok: bool, notify: Option<(String, String)>) { - self.record.status = if ok { - stella_store::SessionStatus::Complete - } else { - stella_store::SessionStatus::Error - }; + /// + /// `status` is the caller's own terminal answer projected by + /// [`crate::daemon::outcome_status`] (or `super::outcome`'s pipeline + /// projection) — never a bool. The bool collapsed a deliberate stop into + /// `Error`, and on an unsupervised headless run no later supervised write + /// corrected it, so the SESSIONS overlay painted a policy stop as a crash + /// (#1826, the presence half of #1653). + pub(crate) fn finish( + &mut self, + status: stella_store::SessionStatus, + notify: Option<(String, String)>, + ) { + self.record.status = status; let _ = self.registry.upsert(&self.record); // The headless counterpart of the deck's exit compaction. A one-shot // run writes fewer objects than a long deck session, but it is also the diff --git a/crates/stella-cli/src/daemon.rs b/crates/stella-cli/src/daemon.rs index d131056c3..301dad126 100644 --- a/crates/stella-cli/src/daemon.rs +++ b/crates/stella-cli/src/daemon.rs @@ -989,12 +989,13 @@ const LOCK_FD_SCAN_LIMIT: i32 = 64; /// be indistinguishable from one that died the moment its window closed. /// /// A no-op in an unsupervised process, and harmless where a surface already -/// wrote its own answer: `SessionPresence::finish` runs first on the two paths -/// that have one, and this write lands after it (agreeing on every outcome -/// except a deliberate stop, where this one knows better — the presence sees -/// only a bool, #1653). This is what covers the paths that do not — -/// `stella fleet`, and any future long-running verb that is handed to the -/// supervisor before it grows a session presence. +/// wrote its own answer: `SessionPresence::finish` runs first on the paths +/// that have one, and this write lands after it — both now project the +/// terminal answer through [`outcome_status`] (#1653, #1826), so the two +/// writes agree on every outcome, deliberate stops included. This is what +/// covers the paths that do not — `stella fleet`, and any future +/// long-running verb that is handed to the supervisor before it grows a +/// session presence. pub(crate) fn record_outcome_if_supervised(outcome: Result<(), &crate::failure::CliFailure>) { let Some(id) = supervised_id() else { return; From aeb640bfd668d219b69445f5c59a0b168c0b197f Mon Sep 17 00:00:00 2001 From: Stella Test Date: Thu, 6 Aug 2026 04:29:58 -0700 Subject: [PATCH 2/3] fix(stella-cli): carry the typed abort through the goal loop and lead turn to the terminal writers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The goal loop and the deck's lead turn answered with a `Result<(), String>`, which has no room for the abort's typed `AbortKind` — so on their paths a deliberate stop (stuck-loop escalation, step cap, enforced budget) was indistinguishable from a crash by the time the terminal SESSIONS-registry status was written: a policy-stopped goal run aged into the registry as `Error`, never `Stopped`. Chase #1637's shape one level deeper: - `stella-core`: `GoalOutcome::Unmet` now carries the typed kind of the working turn's abort (`kind: Option`; `None` for the backstops that are not turn aborts — round cap, unreachable verifier). - `run_goal_cmd` / `run_goal_turn` / `run_goal_pipeline_turn` answer with `CliFailure` instead of `String`; the folds that stringified `PipelineStatus::Aborted` and `GoalOutcome::Unmet` are now the shared projections `agent::outcome::goal_round_break` / `goal_unmet_failure`, and the terminal write projects through `daemon::outcome_status` with the real failure rather than a reconstructed `CliFailure::error`. A stopped goal run also exits 3, per the exit-code taxonomy. - The deck's `run_lead_turn` / `run_lead_pipeline_turn` answer with `CliFailure` through the existing `turn_outcome_result` / `pipeline_status_result` projections, and `session_exit` reads `outcome_status` — one decider for every terminal writer. Closes #1862 Refs #1826, #1653, #1637 --- crates/stella-cli/src/agent.rs | 2 +- crates/stella-cli/src/agent/goal.rs | 70 ++++++------ crates/stella-cli/src/agent/outcome.rs | 104 +++++++++++++++++- crates/stella-cli/src/command_deck.rs | 35 +++--- .../stella-cli/src/command_deck/authoring.rs | 2 +- crates/stella-core/src/goal.rs | 21 +++- 6 files changed, 169 insertions(+), 65 deletions(-) diff --git a/crates/stella-cli/src/agent.rs b/crates/stella-cli/src/agent.rs index 28fba98fe..a7778ea3a 100644 --- a/crates/stella-cli/src/agent.rs +++ b/crates/stella-cli/src/agent.rs @@ -50,7 +50,7 @@ mod coverage; mod engine; mod goal; mod graph; -mod outcome; +pub(crate) mod outcome; mod output; mod persistence; mod presence; diff --git a/crates/stella-cli/src/agent/goal.rs b/crates/stella-cli/src/agent/goal.rs index ba9ff5974..ec8b66a01 100644 --- a/crates/stella-cli/src/agent/goal.rs +++ b/crates/stella-cli/src/agent/goal.rs @@ -208,7 +208,7 @@ pub async fn run_goal_cmd( goal: &str, budget_limit: Option, use_pipeline: bool, -) -> Result<(), String> { +) -> Result<(), crate::failure::CliFailure> { crate::enterprise_telemetry::authorize_execution_surface( crate::enterprise_telemetry::ExecutionSurface::Goal, )?; @@ -370,22 +370,19 @@ pub async fn run_goal_cmd( // Goal runs are long by construction — always land the inbox // notification (Enter on it replays this session's journal). let goal_secs = crate::memory::unix_now_secs().saturating_sub(started_unix); - let notify = if outcome.is_ok() { - format!("{}: goal met ({goal_secs}s)", presence.name()) - } else { - format!("{}: goal run FAILED", presence.name()) + let notify = match &outcome { + Ok(()) => format!("{}: goal met ({goal_secs}s)", presence.name()), + Err(failure) if failure.is_deliberate_stop() => { + format!("{}: goal run stopped by policy", presence.name()) + } + Err(_) => format!("{}: goal run FAILED", presence.name()), }; - // The goal loop still answers with a `String`, which has no room for the - // abort's typed kind (#1637's collapse one level deeper — #1862), so a - // policy-stopped goal round can only record `Error` here. Projected - // through `outcome_status` all the same, so a user interrupt records - // `Cancelled` rather than aging into a crash (#1826). - let terminal = outcome - .as_ref() - .map(|_| ()) - .map_err(|e| crate::failure::CliFailure::error(e.clone())); + // The typed abort survives to this terminal write (#1862): the same + // decider as every other registry writer projects a deliberate stop as + // `Stopped`, a user interrupt as `Cancelled`, and a crash as `Error` + // (#1653, #1826). presence.finish( - crate::daemon::outcome_status(terminal.as_ref().map(|_| ())), + crate::daemon::outcome_status(outcome.as_ref().map(|_| ())), Some((notify, crate::command_deck::prompt_line(goal, 160))), ); outcome @@ -425,7 +422,7 @@ pub(crate) async fn run_goal_turn( // before the turn runs — reflection stores the self-review 1:1 with an // execution, and an unstamped round files an id-less row. session_memory: Option<&mut crate::memory::SessionMemory>, -) -> Result<(), String> { +) -> Result<(), crate::failure::CliFailure> { let turn_start = Instant::now(); let execution = begin_execution(store, "goal", goal, cfg, session); if let (Some((_, id)), Some(m)) = (&execution, session_memory) { @@ -545,13 +542,16 @@ pub(crate) async fn run_goal_turn( rounds, reason, cost_usd, + kind, } => { tui::cost_summary( cost_usd, &format!("{}/{}", cfg.provider.id, cfg.model_id), turn_start.elapsed(), ); - Err(format!("goal not met after {rounds} round(s): {reason}")) + // The typed kind survives the loop (#1862): a working turn's + // deliberate stop exits `3` and records `Stopped`. + Err(outcome::goal_unmet_failure(rounds, &reason, kind)) } } } @@ -588,7 +588,7 @@ async fn run_goal_pipeline_turn( // Same contract as `run_goal_turn`: stamp the execution id into the // caller's memory before the turn runs, so reflection can name its row. session_memory: Option<&mut crate::memory::SessionMemory>, -) -> Result<(), String> { +) -> Result<(), crate::failure::CliFailure> { let turn_start = Instant::now(); let execution = begin_execution(store, "goal", goal, cfg, session); // Rebound mutable and NOT consumed by the id stamp, so the same memory @@ -717,7 +717,7 @@ async fn run_goal_pipeline_turn( .with_calibration(calibration); let mut total_cost_usd = 0.0f64; - let mut result: Option> = None; + let mut result: Option> = None; let mut goal_met = false; for round in 1..=goal_config.max_rounds { @@ -773,25 +773,17 @@ async fn run_goal_pipeline_turn( match pipeline.run(&round_goal, messages, budget).await { Ok(outcome) => { total_cost_usd += outcome.total_cost_usd; - match outcome.status { - PipelineStatus::Completed => {} - PipelineStatus::VerificationFailed { verdict } => { - result = Some(Err(format!( - "goal not met: verification failed: {}", - verdict.summary - ))); - break; - } - PipelineStatus::Aborted { reason, .. } => { - result = Some(Err(format!( - "goal not met: working round aborted: {reason}" - ))); - break; - } + // The fold keeps the abort's typed kind (#1862), so the + // terminal registry write can tell a policy stop from a + // crash. A completed round is no break at all — the loop + // goes on to its verifier assessment below. + if let Some(failure) = outcome::goal_round_break(&outcome.status) { + result = Some(Err(failure)); + break; } } Err(e) => { - result = Some(Err(e.to_string())); + result = Some(Err(crate::failure::CliFailure::error(e.to_string()))); break; } } @@ -807,7 +799,9 @@ async fn run_goal_pipeline_turn( { Ok(pair) => pair, Err(reason) => { - result = Some(Err(format!("goal not met: verifier unavailable: {reason}"))); + result = Some(Err(crate::failure::CliFailure::error(format!( + "goal not met: verifier unavailable: {reason}" + )))); break; } }; @@ -853,10 +847,10 @@ async fn run_goal_pipeline_turn( &format!("{}/{}", cfg.provider.id, cfg.model_id), turn_start.elapsed(), ); - Err(format!( + Err(crate::failure::CliFailure::error(format!( "goal not met after {} round(s): round cap reached without a passing verdict", goal_config.max_rounds - )) + ))) } } }; diff --git a/crates/stella-cli/src/agent/outcome.rs b/crates/stella-cli/src/agent/outcome.rs index 8b5af2550..a756fc254 100644 --- a/crates/stella-cli/src/agent/outcome.rs +++ b/crates/stella-cli/src/agent/outcome.rs @@ -66,7 +66,7 @@ pub(super) fn pipeline_episode_outcome(status: &PipelineStatus) -> EpisodeOutcom } } -pub(super) fn pipeline_status_result(status: &PipelineStatus) -> Result<(), CliFailure> { +pub(crate) fn pipeline_status_result(status: &PipelineStatus) -> Result<(), CliFailure> { match status { PipelineStatus::Completed => Ok(()), PipelineStatus::VerificationFailed { verdict } => Err(CliFailure::error(format!( @@ -114,7 +114,7 @@ pub(super) fn pipeline_session_status( /// could not tell a resumed policy stop from a resumed crash (#1637). /// /// [`AbortKind`]: stella_core::AbortKind -pub(super) fn turn_outcome_result(outcome: &TurnOutcome) -> Result<(), CliFailure> { +pub(crate) fn turn_outcome_result(outcome: &TurnOutcome) -> Result<(), CliFailure> { match outcome { TurnOutcome::Completed { .. } => Ok(()), TurnOutcome::Aborted { reason, kind, .. } => { @@ -123,6 +123,45 @@ pub(super) fn turn_outcome_result(outcome: &TurnOutcome) -> Result<(), CliFailur } } +/// The terminal break a goal round's pipeline status folds to — `None` keeps +/// the loop running to its verifier assessment. The goal-loop analogue of +/// [`pipeline_status_result`], with the loop's own message prefixes: the fold +/// used to stringify the status into `Err(String)`, which dropped the abort's +/// typed [`AbortKind`] right there, so the goal loop's terminal registry +/// write could only record a policy-stopped round as an error (#1862). +/// +/// [`AbortKind`]: stella_core::AbortKind +pub(crate) fn goal_round_break(status: &PipelineStatus) -> Option { + match status { + PipelineStatus::Completed => None, + PipelineStatus::VerificationFailed { verdict } => Some(CliFailure::error(format!( + "goal not met: verification failed: {}", + verdict.summary + ))), + PipelineStatus::Aborted { reason, kind } => Some(CliFailure::from_abort( + format!("goal not met: working round aborted: {reason}"), + *kind, + )), + } +} + +/// The failure an unmet goal loop reports at the process boundary — the raw +/// (`--no-pipeline`) goal loop's half of #1862. The abort's typed kind +/// survives when a working turn aborted; the backstops that are not turn +/// aborts (round cap, unreachable verifier) carry no kind and stay plain +/// failures, exactly as the untyped chain always read them. +pub(crate) fn goal_unmet_failure( + rounds: usize, + reason: &str, + kind: Option, +) -> CliFailure { + let message = format!("goal not met after {rounds} round(s): {reason}"); + match kind { + Some(kind) => CliFailure::from_abort(message, kind), + None => CliFailure::error(message), + } +} + #[cfg(test)] mod tests { use super::*; @@ -216,6 +255,67 @@ mod tests { ); } + /// #1862 witness, the goal loop's half of #1826's: the fold an aborted + /// working round takes on its way to the terminal registry write keeps + /// the abort's typed kind, so `run_goal_cmd`'s `presence.finish` + /// projects a policy-stopped goal round as `Stopped`. Before, the fold + /// stringified the status into `Err(String)` and the terminal write + /// could only reconstruct `CliFailure::error` — every stopped goal run + /// aged into the registry as a crash. + /// + /// Like `an_unsupervised_deliberate_stop_projects_stopped_not_error`, + /// this relies on no test in this binary ever calling + /// `signals::note_interrupt`, so `interrupted_exit_code()` is reliably + /// `None` here. + #[test] + fn a_policy_stopped_goal_round_projects_stopped_not_error() { + let stopped = goal_round_break(&PipelineStatus::Aborted { + reason: "stuck-loop detected (persisted after a steering warning)".into(), + kind: AbortKind::DeliberateStop, + }) + .expect("an aborted round ends the goal loop"); + let crashed = goal_round_break(&PipelineStatus::Aborted { + reason: "the model call would not commit after retries".into(), + kind: AbortKind::Failure, + }) + .expect("an aborted round ends the goal loop"); + + assert_eq!( + crate::daemon::outcome_status(Err(&stopped)), + stella_store::SessionStatus::Stopped, + "a goal round that ended itself by policy must not read as a crash" + ); + assert_eq!( + crate::daemon::outcome_status(Err(&crashed)), + stella_store::SessionStatus::Error + ); + // A completed round is not a break at all — the loop goes on to its + // verifier assessment. + assert!(goal_round_break(&PipelineStatus::Completed).is_none()); + } + + /// The raw (`--no-pipeline`) goal loop's half of the same witness: an + /// `Unmet` outcome whose working turn deliberately stopped projects + /// `Stopped`, while the kind-less backstops keep reading as failures. + #[test] + fn a_policy_stopped_raw_goal_loop_projects_stopped_not_error() { + let stopped = goal_unmet_failure( + 2, + "working turn aborted: session budget limit reached", + Some(AbortKind::DeliberateStop), + ); + let capped = goal_unmet_failure(8, "round cap (8) reached without a passing verdict", None); + + assert_eq!( + crate::daemon::outcome_status(Err(&stopped)), + stella_store::SessionStatus::Stopped + ); + assert_eq!( + crate::daemon::outcome_status(Err(&capped)), + stella_store::SessionStatus::Error + ); + } + #[test] fn a_deliberately_stopped_turn_carries_its_own_exit_code() { let failure = turn_outcome_result(&TurnOutcome::Aborted { diff --git a/crates/stella-cli/src/command_deck.rs b/crates/stella-cli/src/command_deck.rs index ae71e4bb1..73eb734f6 100644 --- a/crates/stella-cli/src/command_deck.rs +++ b/crates/stella-cli/src/command_deck.rs @@ -223,7 +223,7 @@ fn debug_log_path() -> Option { /// How one dispatched turn ended, as seen by the driver loop. enum TurnEnd { /// The turn future resolved (completed or aborted-with-reason). - Finished(Result<(), String>), + Finished(Result<(), crate::failure::CliFailure>), /// The user stopped it mid-flight; the future was dropped. `hold` is the /// double-Esc variant: the interrupted prompt goes back to the FRONT of /// the backlog and dispatch parks until the user's next submission @@ -1591,7 +1591,7 @@ pub async fn run_deck_session( // The lead lane's pause seam — `p` on the lead row (#1219). let lead_pause = lead_control::LeadPause::new(); let end = { - // Both arms return `Result<(), String>`, so one pinned future + // Both arms return `Result<(), CliFailure>`, so one pinned future // drives either path through the same select loop. let turn = async { if pipeline_on { @@ -2120,11 +2120,9 @@ pub async fn run_deck_session( // ones someone comparing turns wants to see — so this is not // conditioned on the outcome. cfg.durability.mark_turn_end(); - session_exit = if outcome.is_err() { - stella_store::SessionStatus::Error - } else { - stella_store::SessionStatus::Complete - }; + // One decider for every terminal writer (#1653/#1826/#1862): + // a lead turn that ended in a deliberate stop exits `Stopped`. + session_exit = crate::daemon::outcome_status(outcome.as_ref().map(|_| ())); session_record.status = stella_store::SessionStatus::NeedsInput; let _ = session_registry.upsert(&session_record); let turn_secs = crate::memory::unix_now_secs().saturating_sub(started_unix); @@ -4228,7 +4226,7 @@ async fn run_lead_turn( // Phase 2 (#713): this turn's `ContextRecall`, carried from the caller // because recall runs before this channel exists. recall_event: Option, -) -> Result<(), String> { +) -> Result<(), crate::failure::CliFailure> { budget.begin_turn(); let (tx, rx) = mpsc::unbounded_channel::(); @@ -4348,10 +4346,9 @@ async fn run_lead_turn( } } - match outcome { - TurnOutcome::Completed { .. } => Ok(()), - TurnOutcome::Aborted { reason, .. } => Err(reason), - } + // The abort's typed kind rides through (#1862): the session-exit writer + // reads it off the same projection as every other terminal writer. + agent::outcome::turn_outcome_result(&outcome) } /// One staged-pipeline turn for the lead agent (`/pipeline` ON): the deck @@ -4396,7 +4393,7 @@ async fn run_lead_pipeline_turn( steering: &Arc, pause: &lead_control::LeadPause, mcp: Option>, -) -> Result<(), String> { +) -> Result<(), crate::failure::CliFailure> { budget.begin_turn(); let (tx, rx) = mpsc::unbounded_channel::(); @@ -4555,14 +4552,10 @@ async fn run_lead_pipeline_turn( } match result { - Ok(outcome) => match outcome.status { - PipelineStatus::Completed => Ok(()), - PipelineStatus::VerificationFailed { verdict } => { - Err(format!("verification failed: {}", verdict.summary)) - } - PipelineStatus::Aborted { reason, .. } => Err(reason), - }, - Err(e) => Err(e.to_string()), + // The shared projection keeps the abort's typed kind (#1862) and the + // exact messages the string arms carried before. + Ok(outcome) => agent::outcome::pipeline_status_result(&outcome.status), + Err(e) => Err(crate::failure::CliFailure::error(e.to_string())), } } diff --git a/crates/stella-cli/src/command_deck/authoring.rs b/crates/stella-cli/src/command_deck/authoring.rs index 1ae905d7f..256bf13ed 100644 --- a/crates/stella-cli/src/command_deck/authoring.rs +++ b/crates/stella-cli/src/command_deck/authoring.rs @@ -81,7 +81,7 @@ pub(super) fn forward_reflection_events( pub(super) async fn record_and_reflect_turn( memory: &mut Option, prompt: &str, - outcome: &Result<(), String>, + outcome: &Result<(), crate::failure::CliFailure>, registry: &ToolRegistry, files_before: usize, started_unix: i64, diff --git a/crates/stella-core/src/goal.rs b/crates/stella-core/src/goal.rs index dcde43084..4cdfbcb60 100644 --- a/crates/stella-core/src/goal.rs +++ b/crates/stella-core/src/goal.rs @@ -45,6 +45,7 @@ use tokio::sync::mpsc::UnboundedSender; use crate::budget::BudgetGuard; use crate::driver::{Engine, TurnOutcome}; +use crate::step::AbortKind; use crate::subagent::{SubAgentHost, SubAgentOutcome, SubAgentSpec, truncate_chars}; /// Tuning for [`Engine::run_goal`]. `Default` is sized for interactive @@ -89,6 +90,13 @@ pub enum GoalOutcome { rounds: usize, reason: String, cost_usd: f64, + /// The typed kind of the abort that ended the loop, carried from + /// [`TurnOutcome::Aborted`] when a working turn aborted — the bit a + /// terminal status writer needs to record a deliberate stop + /// distinctly from a crash (#1862). `None` for the backstops that + /// are not turn aborts (round cap, unreachable verifier), which + /// read as plain failures. + kind: Option, }, } @@ -201,11 +209,12 @@ impl Engine<'_> { self.with_turn_instance(self.config.turn_instance.saturating_add(round_offset)); match round_engine.run_turn(messages, budget, events).await { TurnOutcome::Completed { .. } => {} - TurnOutcome::Aborted { reason, .. } => { + TurnOutcome::Aborted { reason, kind, .. } => { return GoalOutcome::Unmet { rounds: round, reason: format!("working turn aborted: {reason}"), cost_usd: budget.session_spent_usd() - starting_cost_usd, + kind: Some(kind), }; } } @@ -227,6 +236,7 @@ impl Engine<'_> { rounds: round, reason: format!("verifier unavailable: {reason}"), cost_usd: budget.session_spent_usd() - starting_cost_usd, + kind: None, }; } }; @@ -262,6 +272,7 @@ impl Engine<'_> { goal_config.max_rounds ), cost_usd: budget.session_spent_usd() - starting_cost_usd, + kind: None, } } @@ -815,12 +826,16 @@ mod tests { rounds, reason, cost_usd, + kind, } => { // Stopped well before the round cap, on the budget backstop… assert!( rounds < config.max_rounds, "the loop ran to the round cap ({rounds}) instead of the budget" ); + // …whose typed kind survives the loop (#1862): an enforced + // budget is the engine choosing to stop, not falling over. + assert_eq!(kind, Some(AbortKind::DeliberateStop)); assert!( reason.contains("budget"), "the abort reason should cite the budget: {reason}" @@ -1047,8 +1062,10 @@ mod tests { .await; match outcome { - GoalOutcome::Unmet { reason, .. } => { + GoalOutcome::Unmet { reason, kind, .. } => { assert!(reason.contains("working turn aborted"), "{reason}"); + // A provider failure keeps its typed kind too (#1862). + assert_eq!(kind, Some(AbortKind::Failure)); // The verifier was never consulted about an aborted turn. assert_eq!(verifier.calls.load(Ordering::SeqCst), 0); } From f3c9f7b2149bab18bcc2cb870bdb45d00e177ffa Mon Sep 17 00:00:00 2001 From: Stella Test Date: Thu, 6 Aug 2026 04:31:14 -0700 Subject: [PATCH 3/3] fix(stella-cli): compare the deck's soft-stop by message and stringify the error row Refs #1862 --- crates/stella-cli/src/command_deck.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/stella-cli/src/command_deck.rs b/crates/stella-cli/src/command_deck.rs index 73eb734f6..9a8f8bd7f 100644 --- a/crates/stella-cli/src/command_deck.rs +++ b/crates/stella-cli/src/command_deck.rs @@ -2051,7 +2051,7 @@ pub async fn run_deck_session( match end { TurnEnd::Finished(outcome) => { if let Err(reason) = &outcome { - if reason == stella_core::SOFT_STOP_REASON { + if reason.message() == stella_core::SOFT_STOP_REASON { // A user choice, not a failure: no Error row — the // work is kept and the next prompt continues from it. let _ = in_tx.send(Inbound::Event { @@ -2067,7 +2067,7 @@ pub async fn run_deck_session( let _ = in_tx.send(Inbound::Event { agent: LEAD.to_string(), event: AgentEvent::Error { - message: reason.clone(), + message: reason.to_string(), retryable: false, }, });