Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions crates/stella-cli/src/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -1122,7 +1122,7 @@ pub async fn run_interactive(cfg: &Config, budget_limit: Option<f64>) -> 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(())
}
Expand Down
18 changes: 15 additions & 3 deletions crates/stella-cli/src/agent/goal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down
96 changes: 92 additions & 4 deletions crates/stella-cli/src/agent/outcome.rs
Original file line number Diff line number Diff line change
@@ -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.
//!
Expand Down Expand Up @@ -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<PipelineOutcome, PipelineRunError>,
) -> 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
Expand Down Expand Up @@ -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 {
Expand Down
47 changes: 32 additions & 15 deletions crates/stella-cli/src/agent/presence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
}
Expand Down Expand Up @@ -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
Expand Down
13 changes: 7 additions & 6 deletions crates/stella-cli/src/daemon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down