diff --git a/crates/rhei-cli/src/cli/agent_command.rs b/crates/rhei-cli/src/cli/agent_command.rs index 8e3b8990..c55aa58b 100644 --- a/crates/rhei-cli/src/cli/agent_command.rs +++ b/crates/rhei-cli/src/cli/agent_command.rs @@ -63,6 +63,8 @@ fn build_agent_command( task_id: &str, state_name: &str, visit_count: u64, + // Which attempt of this visit the invocation is. §FS-rhei-agents.4 + attempt: u64, tooling: &ResolvedTooling, runtime_dir: &Path, // Fan-out key of this invocation, when the state fans out: it decides which @@ -179,6 +181,9 @@ fn build_agent_command( ) .env("RHEI_STATE", state_name) .env("RHEI_VISIT_COUNT", visit_count.to_string()) + // A retry that is handed the same environment as the attempt it is + // recovering from has no way to know it is one. §FS-rhei-agents.4 + .env("RHEI_ATTEMPT", attempt.to_string()) .env("RHEI_AGENT", id); if let Some(path) = worktree_root { cmd.env("RHEI_WORKTREE_ROOT", path); @@ -570,16 +575,3 @@ fn inject_tooling_env(cmd: &mut std::process::Command, tooling: &ResolvedTooling } } -/// Construct the log file path for a task/state invocation. -fn agent_log_path( - runtime_dir: &Path, - task_id: &str, - state_name: &str, - suffix: Option<&str>, -) -> PathBuf { - let suffix = suffix - .filter(|value| !value.is_empty()) - .map(|value| format!("-{value}")) - .unwrap_or_default(); - runtime_dir.join("logs").join(format!("task-{task_id}-{state_name}{suffix}.log")) -} diff --git a/crates/rhei-cli/src/cli/agent_log_files.rs b/crates/rhei-cli/src/cli/agent_log_files.rs new file mode 100644 index 00000000..dfeaaf33 --- /dev/null +++ b/crates/rhei-cli/src/cli/agent_log_files.rs @@ -0,0 +1,93 @@ +// How `rhei run` names the transcript of one agent invocation, and how anything +// that later wants to read one finds it. +// +// Four sides have to agree on the rule: the spawn that opens the file, the +// prompt that cites an earlier visit's, the reset that sweeps a ticket's +// runtime, and the engine's own account of a ticket it finished without +// spawning a worker. A second copy of the rule is how one of them drifts, so +// the rule lives here and only here. +// +// The name says which invocation and which attempt; *which* attempt is a fact +// about the ticket's history, not about the directory listing, and it is kept +// in the spawn record next door. + +// §AR-source-file-size.3 §FS-rhei-agents.8.1 §FS-rhei-memory.4.4 §FS-rhei-run.3 + +fn resolved_agent_log_suffix(resolved: &ResolvedAgent, visit_count: Option) -> Option { + agent_log_suffix(resolved.target.as_ref(), resolved.model.as_deref(), visit_count) +} + +/// The part of a log file name that follows `task-{task_id}-{state}`. +/// +/// Split from the resolved-agent form so prompt composition can name the log of +/// an *earlier* visit, where no `ResolvedAgent` for that visit exists — only +/// the identity this one carries, which is the identity that wrote it. +// §FS-rhei-agents.8.1 §FS-rhei-memory.4.4 +fn agent_log_suffix( + target: Option<&ExecutionTarget>, + model: Option<&str>, + visit_count: Option, +) -> Option { + let base = target + .map(ExecutionTarget::slug) + .or_else(|| model.map(str::to_string).filter(|value| !value.is_empty())); + let visit_suffix = visit_count.filter(|count| *count > 1).map(|count| count.to_string()); + match (base, visit_suffix) { + (Some(base), Some(visit)) => Some(format!("{base}-{visit}")), + (Some(base), None) => Some(base), + (None, Some(visit)) => Some(visit), + (None, None) => None, + } +} + +/// The log file of one attempt at one visit of a state. +/// +/// The first attempt of a visit keeps the plain `task-{id}-{state}{suffix}.log` +/// name every other reader already knows; a re-spawn *within the same visit* +/// appends `-attempt{n}` instead of truncating the file that says why the +/// attempt before it did not finish. The visit count cannot do that job on its +/// own: a ticket that stalls never leaves the state, so it is still on the same +/// visit when the run spawns it again — and, the other way round, a ticket that +/// leaves and returns is on a new visit that the count does not register +/// either, so it starts again at `-attempt1`'s plain name. +// §FS-rhei-agents.8.1 +fn agent_log_attempt_path( + runtime_dir: &Path, + task_id: &str, + state_name: &str, + suffix: Option<&str>, + attempt: u64, +) -> PathBuf { + let suffix = suffix + .filter(|value| !value.is_empty()) + .map(|value| format!("-{value}")) + .unwrap_or_default(); + let attempt = if attempt > 1 { format!("-attempt{attempt}") } else { String::new() }; + runtime_dir.join("logs").join(format!("task-{task_id}-{state_name}{suffix}{attempt}.log")) +} + +/// The log of the *last thing that actually ran* for one invocation — whichever +/// attempt that was — and `None` when nothing did. +/// +/// Read from the spawn record rather than by probing names: probing costs one +/// `exists()` per attempt ever made, and it names attempt files a spawn opened +/// and never wrote a line into. The unsuffixed name is still accepted when no +/// record answers, because a runtime written before records existed still has +/// transcripts worth citing, and citing a transcript is not a claim that a +/// worker ran — that claim has one source, and it is the record. +// §FS-rhei-agents.8.1 §FS-rhei-agents.8.4 +fn latest_agent_log_path( + runtime_dir: &Path, + task_id: &str, + state_name: &str, + suffix: Option<&str>, +) -> Option { + let record = spawn_record_path(runtime_dir, task_id, state_name, suffix); + if let Some(record) = read_spawn_record(&record) { + if record.log.exists() { + return Some(record.log); + } + } + let first = agent_log_attempt_path(runtime_dir, task_id, state_name, suffix, 1); + first.exists().then_some(first) +} diff --git a/crates/rhei-cli/src/cli/agent_resolution.rs b/crates/rhei-cli/src/cli/agent_resolution.rs index e0369446..b4cfb05b 100644 --- a/crates/rhei-cli/src/cli/agent_resolution.rs +++ b/crates/rhei-cli/src/cli/agent_resolution.rs @@ -429,33 +429,6 @@ fn ensure_orchestrator_timeout(resolved: &ResolvedAgent, state_name: &str) -> Mi )) } -fn resolved_agent_log_suffix(resolved: &ResolvedAgent, visit_count: Option) -> Option { - agent_log_suffix(resolved.target.as_ref(), resolved.model.as_deref(), visit_count) -} - -/// The part of a log file name that follows `task-{task_id}-{state}`. -/// -/// Split from the resolved-agent form so prompt composition can name the log of -/// an *earlier* visit, where no `ResolvedAgent` for that visit exists — only -/// the identity this one carries, which is the identity that wrote it. -// §FS-rhei-agents.8.1 §FS-rhei-memory.4.4 -fn agent_log_suffix( - target: Option<&ExecutionTarget>, - model: Option<&str>, - visit_count: Option, -) -> Option { - let base = target - .map(ExecutionTarget::slug) - .or_else(|| model.map(str::to_string).filter(|value| !value.is_empty())); - let visit_suffix = visit_count.filter(|count| *count > 1).map(|count| count.to_string()); - match (base, visit_suffix) { - (Some(base), Some(visit)) => Some(format!("{base}-{visit}")), - (Some(base), None) => Some(base), - (None, Some(visit)) => Some(visit), - (None, None) => None, - } -} - #[allow(clippy::too_many_arguments)] fn state_outputs_exist_for_resolved_invocation( workspace_root: &Path, diff --git a/crates/rhei-cli/src/cli/agent_spawn.rs b/crates/rhei-cli/src/cli/agent_spawn.rs index a6dd8f37..6361b48f 100644 --- a/crates/rhei-cli/src/cli/agent_spawn.rs +++ b/crates/rhei-cli/src/cli/agent_spawn.rs @@ -163,6 +163,10 @@ fn spawn_and_wait_agent( slot: rhei_tui::Slot, sink: Arc, intervene: Option<&Arc>, + // Which attempt of which visit this is. Recorded here, beside the footer, + // because this is the one place that knows the subprocess actually ran. + // §FS-rhei-agents.8.4 + plan: &SpawnPlan, // Fan-out key of this invocation; decides the `RHEI_RESULT_PATH` it is // handed. §FS-rhei-states.3.3 result_identity: Option<&str>, @@ -272,6 +276,7 @@ fn spawn_and_wait_agent( task_id, state_name, visit_count, + plan.attempt, tooling, runtime_dir, result_identity, @@ -452,23 +457,40 @@ fn spawn_and_wait_agent( // §FS-rhei-agents.8: Agent log footer, timeout and interruption causes. let elapsed = start.elapsed(); + let duration = format_duration_human(elapsed.as_secs()); + let ended_wall = std::time::SystemTime::now(); let timeout_message = if timed_out { resolved.timeout_secs.map(format_duration_human) } else { None }; + // The spawn ran. Say so where the next pass, the next run, and the engine's + // own account of this state will all read it. §FS-rhei-agents.8.4 + plan.record_spawn(SpawnEnding { + task_id, + state_name, + kind: "agent", + worker: resolved.agent.id(), + started: &format_iso8601_utc(started_wall), + ended: &format_iso8601_utc(ended_wall), + duration: &duration, + code: status.code(), + ending: if timed_out { + "timed out" + } else if interrupted { + "interrupted" + } else { + "exited" + }, + }); with_agent_log(&log_file, |f| { if let Some(duration) = &timeout_message { writeln!(f, "\nagent timed out after {duration}")?; } else if interrupted { // The run was shutting down, not the agent failing. §FS-rhei-run.3.2 - writeln!( - f, - "\nagent interrupted by run shutdown after {}", - format_duration_human(elapsed.as_secs()) - )?; + writeln!(f, "\nagent interrupted by run shutdown after {duration}")?; } writeln!(f, "\n=== exit ===")?; writeln!(f, "code: {}", status.code().unwrap_or(-1))?; - writeln!(f, "duration: {}", format_duration_human(elapsed.as_secs()))?; - writeln!(f, "ended: {}", format_iso8601_utc(std::time::SystemTime::now()))?; + writeln!(f, "duration: {duration}")?; + writeln!(f, "ended: {}", format_iso8601_utc(ended_wall))?; if timed_out { writeln!(f, "timed_out: true")?; } diff --git a/crates/rhei-cli/src/cli/agent_spawn_records.rs b/crates/rhei-cli/src/cli/agent_spawn_records.rs new file mode 100644 index 00000000..e9059c01 --- /dev/null +++ b/crates/rhei-cli/src/cli/agent_spawn_records.rs @@ -0,0 +1,404 @@ +// What `rhei run` knows about a worker it already spawned: that one ran at all, +// which visit of the state it belonged to, which attempt of that visit it was, +// and how it ended. +// +// Its own part because none of that is derivable from `runtime/logs/`. A log is +// opened before its subprocess starts, so its existence proves only that a +// spawn was attempted; and its name carries `{visit_count}`, which is pinned at +// 1 for every ordinary state in a cycle, so it cannot tell one stay in a state +// from the next. Both questions used to be answered by pattern-matching file +// names, and both answers were wrong. + +// §AR-source-file-size.3 §FS-rhei-agents.8.4 §FS-rhei-run.3 + +/// A visit gets at least the one invocation that makes it a visit, and by +/// default one informed retry after it. §FS-rhei-agents.3.2.3 +const DEFAULT_ATTEMPT_BUDGET: u64 = 2; + +/// One worker spawn that actually ran, as it is left on disk. +/// +/// `moves` is the visit key: the number of transitions the ticket had already +/// made when this spawn started. It changes the moment the ticket moves — a hop, +/// a self-loop, a hand `rhei transition` — and does not change while the ticket +/// stalls in place, which is exactly the distinction `{visit_count}` cannot +/// draw. `task` and `state` are stored so a reader can match them as *fields*: +/// matching record file names by prefix is how state `review` came to claim +/// `review-fix`'s worker, log, and duration. +// §FS-rhei-agents.8.4 +#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] +struct SpawnRecord { + task: String, + state: String, + moves: u64, + attempt: u64, + /// How many attempts of this visit have been charged against its budget. + /// An invocation the run itself interrupted is not one of them: the + /// shutdown ended it, and the next run re-executes it. Defaulted so a + /// record written before the budget existed still reads. + // §FS-rhei-run.3.2 §FS-rhei-agents.3.2.3 + #[serde(default)] + charged: u64, + /// `agent` or `program`, so the account of a state says which kind ran + /// rather than assuming the one the state would resolve to today. + kind: String, + /// The resolved agent id, or the program's command line. + worker: String, + log: PathBuf, + started: String, + ended: String, + duration: String, + code: Option, + /// Why the spawn stopped: `exited`, `timed out`, or `interrupted`. A retry + /// reports the ending it is retrying, and these are different rules. + // §FS-rhei-agents.3.2.1 + ending: String, +} + +impl SpawnRecord { + /// How the previous attempt ended, as the retry note and the retried + /// prompt both say it. + /// + /// Exit `0` is the one ending that has to be inferred rather than read: the + /// scheduler re-spawns an invocation only when its completion condition is + /// still unmet, so an attempt that exited cleanly and is being retried is an + /// attempt whose artifacts never answered for it. + // §FS-rhei-agents.3.2 §FS-rhei-agents.3.2.1 + fn ending_sentence(&self) -> String { + match (self.ending.as_str(), self.code) { + ("timed out", _) => format!("timed out after {}", self.duration), + ("interrupted", _) => "was interrupted by a run shutdown".to_string(), + (_, Some(0)) | (_, None) => { + "exited 0 without meeting this state's completion condition".to_string() + } + (_, Some(code)) => format!("exited {code}"), + } + } +} + +/// `runtime/spawns/` — beside `runtime/logs/`, and swept with it by `rhei +/// reset`, because it answers for the same invocations. +// §FS-rhei-agents.8.4 +fn spawn_records_dir(runtime_dir: &Path) -> PathBuf { + runtime_dir.join("spawns") +} + +/// The record of one invocation, named as its log is minus the attempt suffix: +/// one file per invocation, rewritten by each attempt, so reading the current +/// attempt count costs one `open` rather than a walk over every name a retry +/// might have taken. +// §FS-rhei-agents.8.4 +fn spawn_record_path( + runtime_dir: &Path, + task_id: &str, + state_name: &str, + suffix: Option<&str>, +) -> PathBuf { + let suffix = suffix + .filter(|value| !value.is_empty()) + .map(|value| format!("-{value}")) + .unwrap_or_default(); + spawn_records_dir(runtime_dir).join(format!("task-{task_id}-{state_name}{suffix}.json")) +} + +fn read_spawn_record(path: &Path) -> Option { + serde_json::from_str(&fs::read_to_string(path).ok()?).ok() +} + +/// How many times this ticket has moved, from the central ledger every verb +/// appends to. +/// +/// Both candidate ledgers are counted and summed. A single-file plan has one: +/// the ticket's owning root and the run's runtime directory are the same place. +/// A Panta project can route a ticket's moves to its owning rhei's root while +/// the run's logs go to the project's, and this question must be answered the +/// same either way. Summing is safe because the answer is only ever compared +/// with itself: both counts grow when the ticket moves and neither moves while +/// it stalls, which is the whole property a visit key needs. +/// +/// A missing or unreadable ledger reads as zero, which makes a fresh ticket's +/// first visit look exactly like what it is. +// §FS-rhei-viz.4 §FS-rhei-panta.6.2 +fn ticket_move_count(task_root: &Path, runtime_dir: &Path, task_id: &str) -> u64 { + let owning = task_root.join("runtime").join("state-transitions.log"); + let running = runtime_dir.join("state-transitions.log"); + let prefix = format!("{task_id} "); + let lines_in = |path: &Path| -> u64 { + fs::read_to_string(path) + .map(|raw| raw.lines().filter(|line| line.starts_with(&prefix)).count() as u64) + .unwrap_or(0) + }; + let mut moves = lines_in(&owning); + if running != owning { + moves += lines_in(&running); + } + moves +} + +/// What the next spawn of one invocation is: where it writes, which attempt of +/// which visit it is, and what the attempt before it left behind. +// §FS-rhei-agents.8.1 §FS-rhei-agents.8.4 +struct SpawnPlan { + /// The transcript this spawn opens. + log: PathBuf, + /// Where its record goes once it has actually run. + record: PathBuf, + /// The visit this spawn belongs to. §FS-rhei-agents.8.4 + moves: u64, + /// 1 for the first spawn of this visit. + attempt: u64, + /// What this visit has already spent of its budget. §FS-rhei-agents.3.2.3 + charged: u64, + /// The previous attempt *of this same visit*, when there was one. A record + /// left by an earlier visit is not one: re-entering a state is a fresh + /// start, not a second attempt at the last one. + previous: Option, +} + +impl SpawnPlan { + /// Whether this visit's budget is already spent, so the spawn must not + /// happen at all. §FS-rhei-agents.3.2.3 + fn budget_spent(&self, budget: u64) -> bool { + self.charged >= budget + } + + /// The line the run prints beside `Log:` when it is retrying rather than + /// starting, naming the attempt, the budget it comes out of, what ended the + /// attempt before it, and where that attempt's transcript is. + // §FS-rhei-agents.3.2.1 + fn respawn_note(&self, task_id: &str, state_name: &str, budget: u64) -> Option { + let previous = self.previous.as_ref()?; + Some(format!( + " Re-spawning Task {task_id} in state '{state_name}': attempt {} of {budget}; \ + the previous attempt {} (previous log: {}).", + self.charged + 1, + previous.ending_sentence(), + previous.log.display() + )) + } + + /// Whether the attempt about to run leaves another one behind it. + /// + /// Asked *before* the spawn and answered about the state of the visit after + /// it, because the message that needs it is printed when that spawn has + /// already finished. An interrupted spawn never reaches that message, so the + /// charge this predicts is the charge that happens. + // §FS-rhei-agents.3.2.1 §FS-rhei-agents.3.2.3 + fn retry_outlook(&self, budget: u64) -> RetryOutlook { + if self.charged.saturating_add(1) < budget { + RetryOutlook::AttemptsLeft + } else { + RetryOutlook::BudgetSpent { budget } + } + } + + /// Record this spawn, now that it has ended. + /// + /// Called from the two places a subprocess is waited on, after the footer + /// its log gets, and from nowhere a spawn can fail to start — the record's + /// whole value is that its presence proves a worker ran. + // §FS-rhei-agents.8.4 + fn record_spawn(&self, ended: SpawnEnding<'_>) { + if let Some(parent) = self.record.parent() { + if fs::create_dir_all(parent).is_err() { + return; + } + } + let record = SpawnRecord { + task: ended.task_id.to_string(), + state: ended.state_name.to_string(), + moves: self.moves, + attempt: self.attempt, + // An interrupted invocation is not an attempt the ticket spent: the + // run ended it and the next one re-executes it. It keeps its + // attempt log all the same. §FS-rhei-run.3.2 §FS-rhei-agents.3.2.3 + charged: self.charged + u64::from(ended.ending != "interrupted"), + kind: ended.kind.to_string(), + worker: ended.worker.to_string(), + log: self.log.clone(), + started: ended.started.to_string(), + ended: ended.ended.to_string(), + duration: ended.duration.to_string(), + code: ended.code, + ending: ended.ending.to_string(), + }; + if let Ok(body) = serde_json::to_string_pretty(&record) { + let _ = fs::write(&self.record, body); + } + } +} + +/// The facts a finished spawn records about itself. §FS-rhei-agents.8.4 +struct SpawnEnding<'a> { + task_id: &'a str, + state_name: &'a str, + kind: &'a str, + worker: &'a str, + started: &'a str, + ended: &'a str, + duration: &'a str, + code: Option, + ending: &'a str, +} + +/// What the completion condition still owes, as the halt line names it. +/// +/// The same list the exit-0 stall warning prints, so an operator reading +/// "attempts spent" and an operator reading "outputs are missing" are looking at +/// the same artifacts. An empty list is said plainly rather than skipped: it +/// means the condition that failed named no file, and hiding that would leave +/// the halt looking like it had forgotten to say what it was waiting for. +// §FS-rhei-agents.3.2 §FS-rhei-agents.3.2.3 +fn completion_debt_label(missing: &[String]) -> String { + if missing.is_empty() { + "this state's completion condition names nothing on disk".to_string() + } else { + missing.join(", ") + } +} + +/// The halt an exhausted budget prints, wherever it is printed from. +/// +/// Two moments reach it: the pass that declines to spawn because the budget is +/// gone, and the attempt that spent the last of it, which knows one run earlier +/// that no later pass will run the state again. One function so the operator +/// reads one sentence rather than two that disagree about what happens next. +// §FS-rhei-agents.3.2.1 §FS-rhei-agents.3.2.3 +fn budget_spent_halt_line(task_id: &str, state_name: &str, budget: u64, owed: &str) -> String { + format!( + " halting Task {task_id} in state '{state_name}': {budget} attempts spent on this \ + visit and the completion condition is still unmet: {owed}. The ticket stays in \ + '{state_name}'." + ) +} + +/// What the run will do about a ticket whose attempt just stalled. +/// +/// The stall message predicts engine behaviour, so it has to be conditioned on +/// the thing that decides that behaviour. Conditioning it on the completion +/// condition alone made the run promise a retry it had already ruled out — the +/// same class of untruth as the result stub that said no agent had run. +// §FS-rhei-agents.3.2.1 +#[derive(Clone, Copy)] +enum RetryOutlook { + /// A later pass will spawn this invocation again. + AttemptsLeft, + /// The attempt that just finished was the last one this visit gets. + BudgetSpent { budget: u64 }, +} + +impl RetryOutlook { + /// The halt line for this outlook, given what the completion condition is + /// still owed. §FS-rhei-agents.3.2.1 + fn halt_line(self, task_id: &str, state_name: &str, missing: &[String]) -> String { + match self { + RetryOutlook::AttemptsLeft => format!( + " halting Task {task_id} in state '{state_name}': the completion condition is \ + not met, so no transition fires; a later pass runs the state again." + ), + RetryOutlook::BudgetSpent { budget } => budget_spent_halt_line( + task_id, + state_name, + budget, + &completion_debt_label(missing), + ), + } + } +} + +/// Which attempt of which visit the next spawn of this invocation is. +/// +/// One rule, one place: the scheduler asks it to name the log and to check the +/// budget, and prompt composition asks it to tell the invocation it is a retry. +/// Answering it twice, differently, is how the log names and the run's narration +/// came apart in the first place. +// §FS-rhei-agents.8.1 §FS-rhei-agents.8.4 §FS-rhei-memory.4.4 +fn plan_spawn_attempt( + runtime_dir: &Path, + task_root: &Path, + task_id: &str, + state_name: &str, + suffix: Option<&str>, +) -> SpawnPlan { + let record_path = spawn_record_path(runtime_dir, task_id, state_name, suffix); + let moves = ticket_move_count(task_root, runtime_dir, task_id); + let previous = read_spawn_record(&record_path).filter(|record| record.moves == moves); + let attempt = previous.as_ref().map(|record| record.attempt + 1).unwrap_or(1); + let charged = previous.as_ref().map(|record| record.charged).unwrap_or(0); + SpawnPlan { + log: agent_log_attempt_path(runtime_dir, task_id, state_name, suffix, attempt), + record: record_path, + moves, + attempt, + charged, + previous, + } +} + +/// The most recent worker that actually ran in this state on this ticket, of +/// whatever identity or visit — or `None` when none did. +/// +/// Asked where the engine is about to speak for a state it did not spawn a +/// worker in, so it holds no resolved identity to key a record by and must look +/// for one. Matching is on the record's `task` and `state` fields: a name-prefix +/// match would let `review` answer with `review-fix`'s worker. +// §FS-rhei-agents.8.4 §FS-rhei-run.3 +fn newest_spawn_record_for_state( + runtime_dir: &Path, + task_id: &str, + state_name: &str, +) -> Option { + let mut newest: Option<(String, SpawnRecord)> = None; + for entry in fs::read_dir(spawn_records_dir(runtime_dir)).ok()?.flatten() { + // One unreadable or half-written record must not discard the matches + // already found; it is one file, not an answer about the state. + let Some(record) = read_spawn_record(&entry.path()) else { continue }; + if record.task != task_id || record.state != state_name { + continue; + } + if newest.as_ref().is_none_or(|(seen, _)| record.ended >= *seen) { + newest = Some((record.ended.clone(), record)); + } + } + newest.map(|(_, record)| record) +} + +/// How many spawns one visit to this state may have. +/// +/// The chain a timeout resolves through, one level shorter because a budget has +/// no per-agent meaning: the state's own `attempts:`, then `defaults.attempts`, +/// then the built-in. Below `1` is raised to `1` — every visit gets the +/// invocation that makes it a visit. +/// +/// A poll state is exempt. Re-spawning without moving *is* what a poll state +/// does, and it already carries its own bound in `poll.max_attempts`; a second +/// bound over the same spawns would stop the loop before its own cap and +/// silently change what the machine's author declared. +// §FS-rhei-agents.3.2.3 §FS-rhei-agents.7.1 §FS-rhei-states.2 +fn resolve_attempt_budget( + state_def: Option<&rhei_validator::StateDef>, + settings: &RheiSettings, +) -> u64 { + if state_def.is_some_and(|def| def.poll.is_some()) { + return u64::MAX; + } + state_def + .and_then(|def| def.attempts) + .or(settings.defaults.attempts) + .map(u64::from) + .unwrap_or(DEFAULT_ATTEMPT_BUDGET) + .max(1) +} + +/// A plan for a unit test that only cares about the transcript a spawn writes: +/// the first attempt of a first visit, recording beside the log it is given. +#[cfg(test)] +fn spawn_plan_for_test(log: &Path) -> SpawnPlan { + SpawnPlan { + log: log.to_path_buf(), + record: log.with_extension("spawn.json"), + moves: 0, + attempt: 1, + charged: 0, + previous: None, + } +} diff --git a/crates/rhei-cli/src/cli/complete_reset_commands.rs b/crates/rhei-cli/src/cli/complete_reset_commands.rs index b81899a3..1f3ce00a 100644 --- a/crates/rhei-cli/src/cli/complete_reset_commands.rs +++ b/crates/rhei-cli/src/cli/complete_reset_commands.rs @@ -584,6 +584,8 @@ fn scoped_runtime_targets( ScopedTarget::Exact(runtime.join("results").join(format!("{task_id}.md"))), // §FS-rhei-agents.9 / §FS-rhei-programs.5: `task--[-…].log`. ScopedTarget::Prefixed { dir: runtime.join("logs"), prefix: format!("task-{task_id}-") }, + // The record of every spawn those logs came from. §FS-rhei-agents.8.4 + ScopedTarget::Prefixed { dir: runtime.join("spawns"), prefix: format!("task-{task_id}-") }, // §FS-rhei-snapshots.4: `---/` session dirs. ScopedTarget::Prefixed { dir: runtime.join("snapshot-sessions"), diff --git a/crates/rhei-cli/src/cli/programs.rs b/crates/rhei-cli/src/cli/programs.rs index 76533281..b5a6cd1d 100644 --- a/crates/rhei-cli/src/cli/programs.rs +++ b/crates/rhei-cli/src/cli/programs.rs @@ -1,7 +1,3 @@ -fn program_log_path(runtime_dir: &Path, task_id: &str, state_name: &str) -> PathBuf { - runtime_dir.join("logs").join(format!("task-{task_id}-{state_name}.log")) -} - /// `interrupted` is set when the run was shutting down: the engine ended the /// program's process group, so its exit status says nothing about the ticket /// and **no transition may fire** for this invocation. §FS-rhei-run.3.2 @@ -30,6 +26,8 @@ impl InvocationOutcome for ProgramSpawnOutcome { fn build_program_command( resolved: &ResolvedProgram, render_context: &RuntimeTemplateContext<'_>, + // Which attempt of this state visit this run is. §FS-rhei-programs.2 + attempt: u64, ) -> MietteResult { let working_dir = resolved .program @@ -97,7 +95,10 @@ fn build_program_command( render_context.machine, ) .to_string(), - ); + ) + // A program has no prompt, so the environment is the only place it can + // be told it is a retry. §FS-rhei-programs.2 + .env("RHEI_ATTEMPT", attempt.to_string()); if let Some(path) = render_context.state_machine_path { cmd.env("RHEI_STATE_MACHINE_PATH", path); } @@ -154,6 +155,9 @@ fn spawn_and_wait_program( resolved: &ResolvedProgram, render_context: &RuntimeTemplateContext<'_>, log_path: &Path, + // Which attempt of which visit this is, and where the record of it goes + // once the command has actually run. §FS-rhei-agents.8.4 + plan: &SpawnPlan, // Only to carry the shutdown notice: a program's own output goes to its // log, not the journal. §FS-rhei-run.3.2 sink: &Arc, @@ -171,19 +175,18 @@ fn spawn_and_wait_program( help = program_log_help(), "failed to create log file '{}': {e}", log_path.display() ))?; + let command_label = match &resolved.program.command { + ProgramCommand::Shell(command) => resolve_runtime_template_text(command, render_context), + ProgramCommand::Exec(args) => args + .iter() + .map(|arg| resolve_runtime_template_text(arg, render_context)) + .collect::>() + .join(" "), + }; + let started_wall = std::time::SystemTime::now(); { use std::io::Write as _; let mut f = &log_file; - let command_label = match &resolved.program.command { - ProgramCommand::Shell(command) => { - resolve_runtime_template_text(command, render_context) - } - ProgramCommand::Exec(args) => args - .iter() - .map(|arg| resolve_runtime_template_text(arg, render_context)) - .collect::>() - .join(" "), - }; let _ = writeln!(f, "=== rhei program log v1 ==="); let _ = writeln!(f, "program: {command_label}"); let _ = writeln!(f, "task: {}", render_context.task.id); @@ -205,7 +208,7 @@ fn spawn_and_wait_program( help = program_log_help(), "failed to clone log file handle: {e}" ))?; - let mut cmd = build_program_command(resolved, render_context)?; + let mut cmd = build_program_command(resolved, render_context, plan.attempt)?; cmd.stdout(log_stdout).stderr(log_stderr); // A program is never handed the operator's terminal, and it leads its own // process group so its children go with it. §FS-rhei-run.3.2 @@ -257,6 +260,26 @@ fn spawn_and_wait_program( let timed_out = ended.cause == EndCause::TimedOut; let interrupted = ended.cause == EndCause::Interrupted; + let elapsed = start.elapsed().as_secs(); + // The command ran. Recorded beside the footer, for the same reason an + // agent's spawn is: a log alone cannot prove it. §FS-rhei-agents.8.4 + plan.record_spawn(SpawnEnding { + task_id: &render_context.task.id.to_string(), + state_name: render_context.state_name, + kind: "program", + worker: &command_label, + started: &format_iso8601_utc(started_wall), + ended: &format_iso8601_utc(std::time::SystemTime::now()), + duration: &format!("{elapsed}s"), + code: status.code(), + ending: if timed_out { + "timed out" + } else if interrupted { + "interrupted" + } else { + "exited" + }, + }); { use std::io::Write as _; let mut f = fs::OpenOptions::new() @@ -280,12 +303,12 @@ fn spawn_and_wait_program( let _ = writeln!( f, "\nprogram interrupted by run shutdown after {}", - format_duration_human(start.elapsed().as_secs()) + format_duration_human(elapsed) ); } let _ = writeln!(f, "\n=== exit ==="); let _ = writeln!(f, "code: {}", status.code().unwrap_or(-1)); - let _ = writeln!(f, "duration: {}s", start.elapsed().as_secs()); + let _ = writeln!(f, "duration: {elapsed}s"); if timed_out { let _ = writeln!(f, "timed_out: true"); } diff --git a/crates/rhei-cli/src/cli/run_agent_mode.rs b/crates/rhei-cli/src/cli/run_agent_mode.rs index f8b22db0..b20d8107 100644 --- a/crates/rhei-cli/src/cli/run_agent_mode.rs +++ b/crates/rhei-cli/src/cli/run_agent_mode.rs @@ -364,25 +364,18 @@ fn run_agent_mode( )); } - let pending = if state_def.outputs.is_empty() { - invocations - } else { - invocations - .into_iter() - .filter(|resolved| { - !state_outputs_exist_for_resolved_invocation( - &workspace_root, - task, - ¤t_state, - task.state.as_str(), - machine, - loaded.rhei.metadata.as_ref(), - state_def, - resolved, - ) - }) - .collect::>() - }; + // The whole completion condition decides this, not the + // declared outputs alone: an invocation that wrote its outputs + // but not the result has not finished. §FS-rhei-agents.3.2 + let pending = agent_invocations_to_spawn( + &loaded, + &workspace_root, + task, + machine, + ¤t_state, + state_def, + invocations, + ); if pending.is_empty() { callback_tasks.push((task_id_str, current_state_raw, current_state)); @@ -487,6 +480,7 @@ fn run_agent_mode( current_state, &to_state, opts.no_callbacks(), + &runtime_dir, ) { Ok(effective_to) => { run_info!( @@ -600,6 +594,7 @@ fn run_agent_mode( &plan_title, input, machines, + settings, opts, &workspace_root, &runtime_dir, diff --git a/crates/rhei-cli/src/cli/run_agent_pool.rs b/crates/rhei-cli/src/cli/run_agent_pool.rs index 8b353bba..e9c330eb 100644 --- a/crates/rhei-cli/src/cli/run_agent_pool.rs +++ b/crates/rhei-cli/src/cli/run_agent_pool.rs @@ -61,6 +61,7 @@ fn run_agent_worker_pool( &tx, input, machines, + settings, workspace_root, runtime_dir, sink, @@ -216,6 +217,7 @@ fn run_agent_worker_pool( log, snapshot_preload, visit_count, + retry_outlook, result, accounting_recorded, accounting_warning, @@ -244,6 +246,7 @@ fn run_agent_worker_pool( log, snapshot_preload, visit_count, + retry_outlook, accounting_recorded, outcome, }, diff --git a/crates/rhei-cli/src/cli/run_agent_sequential.rs b/crates/rhei-cli/src/cli/run_agent_sequential.rs index cd7f6264..c914ffcb 100644 --- a/crates/rhei-cli/src/cli/run_agent_sequential.rs +++ b/crates/rhei-cli/src/cli/run_agent_sequential.rs @@ -110,6 +110,49 @@ fn run_sequential_agent_invocation( let tooling = gate.tooling; // §FS-rhei-panta.6.2: the agent works in the owning rhei's root. let task_workspace_root = loaded.task_root(task_id_str, workspace_root); + let visit_count = render_visit_count( + loaded.rhei.metadata.as_ref(), + &task.id, + current_state, + task.state.as_str(), + machine, + ); + // Settled before anything is composed or staged: a spawn this visit may not + // have costs nothing to decline, and every step below it costs something. + // §FS-rhei-agents.3.2.3 §FS-rhei-agents.8.1 + let plan = plan_spawn_attempt( + runtime_dir, + &task_workspace_root, + task_id_str, + current_state, + resolved_agent_log_suffix(resolved, Some(visit_count)).as_deref(), + ); + let budget = resolve_attempt_budget(machine.states.get(current_state), settings); + if plan.budget_spent(budget) { + // The same stall step 5 gives any unmet completion condition: the ticket + // keeps its state, no transition fires, and the pass moves on. + // §FS-rhei-run.3 §FS-rhei-agents.3.2.3 + let owed = collect_missing_required_outputs( + workspace_root, + &task_workspace_root, + machine, + loaded.rhei.metadata.as_ref(), + task, + current_state, + selected_forward_transition(&loaded.rhei, machine, task).as_deref(), + ); + run_warn!( + "{}", + budget_spent_halt_line( + task_id_str, + current_state, + budget, + &completion_debt_label(&owed) + ) + ); + progress.stalled_tasks.insert(task_id_str.clone()); + return Ok(()); + } let checkout_root = resolve_agent_checkout_root(&task_workspace_root, task_id_str)?; // A sequential pass runs one invocation at a time, so nothing else of this // run is in flight. §FS-rhei-memory.4.3 @@ -149,19 +192,9 @@ fn run_sequential_agent_invocation( return Ok(()); } }; - let visit_count = render_visit_count( - loaded.rhei.metadata.as_ref(), - &task.id, - current_state, - task.state.as_str(), - machine, - ); - let log = agent_log_path( - runtime_dir, - task_id_str, - current_state, - resolved_agent_log_suffix(resolved, Some(visit_count)).as_deref(), - ); + // A retry gets its own attempt log rather than truncating the transcript + // that explains the miss it is retrying. §FS-rhei-agents.8.1 + let log = plan.log.clone(); run_info!( "\nSpawning agent '{}' for Task {}: {}", @@ -174,6 +207,12 @@ fn run_sequential_agent_invocation( } run_info!(" Checkout: {}", checkout_root.path.display()); run_info!(" Log: {}", log.display()); + // Names the rule, the attempt, and the budget it comes out of, so a loop is + // visible while it spends rather than at the halt. + // §FS-rhei-agents.3.2.1 §FS-rhei-run.3 + if let Some(note) = plan.respawn_note(task_id_str, current_state, budget) { + run_info!("{note}"); + } // Spec § Execution Loop step 3: if the state declares // `snapshot.inherit:`, resolve and preload the source snapshot @@ -225,6 +264,9 @@ fn run_sequential_agent_invocation( 0, sink.clone(), intervene, + // Written when this spawn ends, so its presence proves one ran. + // §FS-rhei-agents.8.4 + &plan, // A fanned-out invocation writes its own result fragment. // §FS-rhei-states.3.3 fanout_result_identity( @@ -294,6 +336,7 @@ fn run_sequential_agent_invocation( log, snapshot_preload, visit_count, + retry_outlook: plan.retry_outlook(budget), result: spawn_result, }, progress, diff --git a/crates/rhei-cli/src/cli/run_agent_sequential_completion.rs b/crates/rhei-cli/src/cli/run_agent_sequential_completion.rs index b2d585c2..c47c644c 100644 --- a/crates/rhei-cli/src/cli/run_agent_sequential_completion.rs +++ b/crates/rhei-cli/src/cli/run_agent_sequential_completion.rs @@ -20,6 +20,11 @@ struct SequentialAgentCompletion<'a> { log: PathBuf, snapshot_preload: SnapshotPreload, visit_count: u64, + /// Whether the visit this invocation belongs to has an attempt left after + /// it. Decided at the spawn, because that is where the budget is resolved, + /// and read here, where the run says what it will do next. + // §FS-rhei-agents.3.2.1 §FS-rhei-agents.3.2.3 + retry_outlook: RetryOutlook, result: MietteResult, } @@ -50,6 +55,7 @@ fn handle_sequential_agent_completion( log, snapshot_preload, visit_count, + retry_outlook, result: spawn_result, } = completion; let task_id_str = &task_id_str; @@ -239,6 +245,7 @@ fn handle_sequential_agent_completion( task_id_str, state_before, &missing_required_outputs, + retry_outlook, sink, ); progress.stalled_tasks.insert(task_id_str.clone()); @@ -352,6 +359,7 @@ fn handle_sequential_agent_completion( state_before, selected_forward_transition(&loaded.rhei, machine, task) .as_deref(), + retry_outlook, sink, ); // Did not move; the rest of the pass must look diff --git a/crates/rhei-cli/src/cli/run_callback_mode.rs b/crates/rhei-cli/src/cli/run_callback_mode.rs index 974fa47f..c90375e8 100644 --- a/crates/rhei-cli/src/cli/run_callback_mode.rs +++ b/crates/rhei-cli/src/cli/run_callback_mode.rs @@ -310,6 +310,7 @@ fn run_callback_mode( ¤t_state, &to_state, opts.no_callbacks(), + &runtime_dir, ) { Ok(effective_to) => { run_info!( @@ -490,6 +491,9 @@ fn emit_exit_zero_warnings( task_id_str: &str, state_name: &str, selected_to: Option<&str>, + // Carried from the spawn that just finished: only it knows whether the + // visit has an attempt left. §FS-rhei-agents.3.2.1 + outlook: RetryOutlook, sink: &Arc, ) { let missing = collect_missing_required_outputs( @@ -515,6 +519,7 @@ fn emit_exit_zero_warnings( task_id_str, state_name, &missing, + outlook, sink, ); } @@ -536,6 +541,9 @@ fn emit_exit_zero_missing_required_outputs_warning( task_id_str: &str, state_name: &str, missing: &[String], + // What the run will actually do next, which is not decided by the missing + // artifacts alone. §FS-rhei-agents.3.2.1 + outlook: RetryOutlook, sink: &Arc, ) { sink.emit(rhei_tui::RunEvent::Message { @@ -548,6 +556,13 @@ fn emit_exit_zero_missing_required_outputs_warning( missing.join(", ") ), }); + // The warning says *what* is missing; this says what the run is doing about + // it — and after the last budgeted attempt what it does is nothing, so this + // is conditioned on the budget. §FS-rhei-agents.3.2.1 + sink.emit(rhei_tui::RunEvent::Message { + level: rhei_tui::MessageLevel::Warn, + text: outlook.halt_line(task_id_str, state_name, missing), + }); sink.emit(rhei_tui::RunEvent::TaskOutputsMissing { task: task_id_str.to_string(), state: state_name.to_string(), diff --git a/crates/rhei-cli/src/cli/run_completion_condition.rs b/crates/rhei-cli/src/cli/run_completion_condition.rs new file mode 100644 index 00000000..e7ac22f0 --- /dev/null +++ b/crates/rhei-cli/src/cli/run_completion_condition.rs @@ -0,0 +1,181 @@ +// The completion condition asked of one invocation at a time, and the two +// questions `rhei run` puts to it: *may this invocation be skipped* before a +// pass spawns it, and *has the state finished* once one has exited. +// +// Its own part because those two moments used to hold two different rules. The +// scheduler asked only whether the declared `outputs:` were on disk, so an +// invocation that had failed the condition on one pass — outputs written, the +// ticket's terminal result never written — was read on the next as having +// nothing left to do, and the ticket advanced into a `final: true` state on the +// strength of artifacts that had never answered for it. One rule, one place. + +// §AR-source-file-size.3 §FS-rhei-agents.3.2 §FS-rhei-run.3 + +/// One state visit, ready to be asked the completion condition about each of +/// its invocations. +/// +/// `workspace_root` is where declared `outputs:` resolve; `result_root` is the +/// owning rhei's execution root, which is where results live. +/// `finishes_ticket` is a property of the *edge* the exit would select, not of +/// the state, which is why it is settled once here rather than re-derived per +/// invocation. +// §FS-rhei-agents.3.2 §FS-rhei-states.3.3 §FS-rhei-panta.6.2 +struct InvocationCompletion<'a> { + workspace_root: &'a Path, + result_root: &'a Path, + task: &'a rhei_core::ast::Task, + state_name: &'a str, + current_state_raw: &'a str, + machine: &'a rhei_validator::StateMachine, + metadata: Option<&'a Metadata>, + state_def: &'a rhei_validator::StateDef, + finishes_ticket: bool, + visit_count: u64, +} + +impl InvocationCompletion<'_> { + /// Whether this invocation still owes the ticket something: a declared + /// `outputs:` artifact of *its* identity is missing, or — when the edge this + /// exit would select finishes the ticket — its own result is. + /// + /// Exit code is deliberately not part of it. Before a spawn there is no exit + /// to read, and after one the caller has the status in hand; what this + /// answers is the artifact half of the condition, which is the half that is + /// the same question at both moments. + // §FS-rhei-agents.3.2 §FS-rhei-states.3.3 + fn invocation_is_pending(&self, resolved: &ResolvedAgent) -> bool { + if !state_outputs_exist_for_resolved_invocation( + self.workspace_root, + self.task, + self.state_name, + self.current_state_raw, + self.machine, + self.metadata, + self.state_def, + resolved, + ) { + return true; + } + if !self.finishes_ticket { + return false; + } + let identity = fanout_result_identity( + Some(self.state_def), + resolved.target.as_ref(), + resolved.model.as_deref(), + ); + let path = invocation_result_file_path( + self.result_root, + &self.task.id.to_string(), + ResultInvocation { + state: self.state_name, + visit_count: self.visit_count, + identity: identity.as_deref(), + }, + ); + !file_has_content(&path) + } +} + +/// Whether any invocation of this state still owes the ticket something. +/// +/// One invocation exiting is not the state finishing: the run must not select a +/// transition while a sibling is still to write. Gating on declared outputs +/// alone let a fan-out state with none advance on the first exit, with the merge +/// then running once per invocation and, on a terminal edge, once per invocation +/// that arrived after the ticket had left. +// §FS-rhei-agents.3.2 §FS-rhei-states.3.3 §FS-rhei-panta.6.2 +#[allow(clippy::too_many_arguments)] +fn task_has_pending_agent_invocations( + workspace_root: &Path, + result_root: &Path, + task: &rhei_core::ast::Task, + state_name: &str, + current_state_raw: &str, + machine: &rhei_validator::StateMachine, + metadata: Option<&Metadata>, + state_def: &rhei_validator::StateDef, + settings: &RheiSettings, + selected_to: Option<&str>, +) -> MietteResult { + let invocations = resolve_agent_invocations_for_task( + machine, + state_name, + settings, + &default_run_options(), + Some(task), + )?; + let completion = InvocationCompletion { + workspace_root, + result_root, + task, + state_name, + current_state_raw, + machine, + metadata, + state_def, + finishes_ticket: selected_to.is_some_and(|to| is_terminal_state(to, machine)), + visit_count: render_visit_count( + metadata, + &task.id, + state_name, + current_state_raw, + machine, + ), + }; + Ok(invocations.iter().any(|resolved| completion.invocation_is_pending(resolved))) +} + +/// Which of the invocations a pass resolved for `task` it must actually spawn. +/// +/// A pass skips an invocation only when the whole completion condition already +/// holds for it — the declared artifacts *and*, on a terminal edge, its own +/// result. Skipping on the declared outputs alone is what let a stalled ticket +/// be reclassified as finished a pass later; the recovery the execution loop +/// prescribes is to run the state again, and this is where that happens. +/// +/// A state that declares no `outputs:` is never skipped. It has no artifact of +/// its own that could stand as proof its work was done, and the ticket's result +/// file cannot stand in for one: it is shared with every state the ticket has +/// passed through, so a result written earlier would excuse a state that has not +/// run at all. +// §FS-rhei-agents.3.2 §FS-rhei-run.3 +fn agent_invocations_to_spawn( + loaded: &LoadedPlan, + workspace_root: &Path, + task: &rhei_core::ast::Task, + machine: &rhei_validator::StateMachine, + state_name: &str, + state_def: &rhei_validator::StateDef, + invocations: Vec, +) -> Vec { + if state_def.outputs.is_empty() { + return invocations; + } + let current_state_raw = task.state.as_str(); + let metadata = loaded.rhei.metadata.as_ref(); + let result_root = loaded.task_root(&task.id.to_string(), workspace_root); + let completion = InvocationCompletion { + workspace_root, + result_root: &result_root, + task, + state_name, + current_state_raw, + machine, + metadata, + state_def, + finishes_ticket: selected_forward_transition(&loaded.rhei, machine, task) + .is_some_and(|to| is_terminal_state(&to, machine)), + visit_count: render_visit_count( + metadata, + &task.id, + state_name, + current_state_raw, + machine, + ), + }; + invocations + .into_iter() + .filter(|resolved| completion.invocation_is_pending(resolved)) + .collect() +} diff --git a/crates/rhei-cli/src/cli/run_helpers.rs b/crates/rhei-cli/src/cli/run_helpers.rs index 2c87c4c5..4b497bbe 100644 --- a/crates/rhei-cli/src/cli/run_helpers.rs +++ b/crates/rhei-cli/src/cli/run_helpers.rs @@ -150,73 +150,6 @@ fn ensure_state_outputs_exist_for_transition( Ok(()) } -/// Whether any invocation of this state still owes the ticket something. -/// -/// One invocation exiting is not the state finishing: the run must not select a -/// transition while a sibling is still to write. An invocation is pending when a -/// declared `outputs:` artifact of *its* identity is missing, or — when the edge -/// this exit would select finishes the ticket — when its own result fragment is. -/// Gating on declared outputs alone let a fan-out state with none advance on the -/// first exit, with the merge then running once per invocation and, on a -/// terminal edge, once per invocation that arrived after the ticket had left. -/// -/// `result_root` is the owning rhei's execution root, which is where results -/// live; declared outputs resolve against `workspace_root`. -// §FS-rhei-agents.3.2 §FS-rhei-states.3.3 §FS-rhei-panta.6.2 -#[allow(clippy::too_many_arguments)] -fn task_has_pending_agent_invocations( - workspace_root: &Path, - result_root: &Path, - task: &rhei_core::ast::Task, - state_name: &str, - current_state_raw: &str, - machine: &rhei_validator::StateMachine, - metadata: Option<&Metadata>, - state_def: &rhei_validator::StateDef, - settings: &RheiSettings, - selected_to: Option<&str>, -) -> MietteResult { - let invocations = resolve_agent_invocations_for_task( - machine, - state_name, - settings, - &default_run_options(), - Some(task), - )?; - let finishes_ticket = selected_to.is_some_and(|to| is_terminal_state(to, machine)); - let visit_count = - render_visit_count(metadata, &task.id, state_name, current_state_raw, machine); - let task_id = task.id.to_string(); - Ok(invocations.iter().any(|resolved| { - if !state_outputs_exist_for_resolved_invocation( - workspace_root, - task, - state_name, - current_state_raw, - machine, - metadata, - state_def, - resolved, - ) { - return true; - } - if !finishes_ticket { - return false; - } - let identity = fanout_result_identity( - Some(state_def), - resolved.target.as_ref(), - resolved.model.as_deref(), - ); - let path = invocation_result_file_path( - result_root, - &task_id, - ResultInvocation { state: state_name, visit_count, identity: identity.as_deref() }, - ); - !file_has_content(&path) - })) -} - fn parse_program_spec(value: &YamlValue) -> MietteResult { match value { YamlValue::String(command) => Ok(ProgramSpec { diff --git a/crates/rhei-cli/src/cli/run_parallel_agent_exit.rs b/crates/rhei-cli/src/cli/run_parallel_agent_exit.rs index 7ee1724f..e3cd2c03 100644 --- a/crates/rhei-cli/src/cli/run_parallel_agent_exit.rs +++ b/crates/rhei-cli/src/cli/run_parallel_agent_exit.rs @@ -33,6 +33,7 @@ fn handle_parallel_agent_exit( log, snapshot_preload, visit_count, + retry_outlook, accounting_recorded, outcome, } = exit; @@ -228,6 +229,7 @@ fn handle_parallel_agent_exit( &task_id_str, &state_name, &missing_required_outputs, + retry_outlook, sink, ); progress.stalled_tasks.insert(task_id_str.clone()); @@ -380,6 +382,7 @@ fn handle_parallel_agent_exit( task, ) .as_deref(), + retry_outlook, sink, ); progress.stalled_tasks.insert(task_id_str.clone()); diff --git a/crates/rhei-cli/src/cli/run_parallel_program_completion.rs b/crates/rhei-cli/src/cli/run_parallel_program_completion.rs index d0a93178..7b4965f1 100644 --- a/crates/rhei-cli/src/cli/run_parallel_program_completion.rs +++ b/crates/rhei-cli/src/cli/run_parallel_program_completion.rs @@ -24,6 +24,7 @@ fn handle_parallel_program_completion( let ParallelProgramCompletion { task_id_str, state_name, + retry_outlook, result, slot: _, } = completion; @@ -153,6 +154,7 @@ fn handle_parallel_program_completion( &task_id_str, &state_name, &missing_required_outputs, + retry_outlook, sink, ); return Ok(ParallelProgramCompletionEffect { diff --git a/crates/rhei-cli/src/cli/run_parallel_program_spawn.rs b/crates/rhei-cli/src/cli/run_parallel_program_spawn.rs new file mode 100644 index 00000000..e8a6743a --- /dev/null +++ b/crates/rhei-cli/src/cli/run_parallel_program_spawn.rs @@ -0,0 +1,194 @@ +// Putting one program work item on a worker thread: the log and record it +// writes, the slot it holds, and the completion it sends back down the channel. +// +// Its own part because a program invocation shares nothing with an agent's but +// the channel it answers on — no prompt, no tooling gate, no snapshot staging, +// no checkout — and the two together outgrew one file. + +// §AR-source-file-size.3 §FS-rhei-programs.6.3 §FS-rhei-run.3 + +#[allow(clippy::too_many_arguments)] +fn spawn_parallel_program_work_item( + item: &ProgramWorkItem, + slot: rhei_tui::Slot, + tx: std::sync::mpsc::Sender, + input: &Path, + machines: &ExecutionMachines, + settings: &RheiSettings, + workspace_root: &Path, + runtime_dir: &Path, + sink: &Arc, +) -> MietteResult { + // As for agents: a slot was reserved for this item before the interrupt, + // and the shutdown starts nothing further. §FS-rhei-run.3.2 + if interrupt_requested() { + return Ok(ParallelProgramSpawnOutcome::Skipped); + } + let loaded = load_plan(input)?; + let target_id = parse_task_id(&item.task_id_str); + // The item's owning rhei supplies its machine and callback base. + // §DA-per-rhei-state-machines + let machine = machines.for_task_str(&item.task_id_str); + let callback_paths = machines.callbacks_for_str(&item.task_id_str); + let task = find_task_by_id(&loaded.rhei.tasks, &target_id); + let Some(task) = task else { return Ok(ParallelProgramSpawnOutcome::Skipped) }; + + // Programs run against the owning rhei's execution root. §FS-rhei-panta.6.2 + let task_workspace_root = loaded.task_root(&item.task_id_str, workspace_root); + // Same attempt log and same per-visit budget as an agent: a program state + // is never skipped at scheduling either. §FS-rhei-agents.8.1 + let plan = plan_spawn_attempt( + runtime_dir, + &task_workspace_root, + &item.task_id_str, + &item.current_state, + None, + ); + let budget = + resolve_attempt_budget(machine.states.get(item.current_state.as_str()), settings); + if plan.budget_spent(budget) { + let owed = collect_missing_required_outputs( + workspace_root, + &task_workspace_root, + machine, + loaded.rhei.metadata.as_ref(), + task, + &item.current_state, + selected_forward_transition(&loaded.rhei, machine, task).as_deref(), + ); + // `Skipped` is the pool's stall. §FS-rhei-run.3 §FS-rhei-agents.3.2.3 + emit_run_message( + sink, + rhei_tui::MessageLevel::Warn, + budget_spent_halt_line( + &item.task_id_str, + &item.current_state, + budget, + &completion_debt_label(&owed), + ), + ); + return Ok(ParallelProgramSpawnOutcome::Skipped); + } + let workspace_root = task_workspace_root.as_path(); + + let log = plan.log.clone(); + emit_run_message( + sink, + rhei_tui::MessageLevel::Info, + format!("\nSpawning program for Task {}: {} (parallel)", item.task_id_str, task.title), + ); + emit_run_message(sink, rhei_tui::MessageLevel::Info, format!(" Log: {}", log.display())); + // §FS-rhei-agents.3.2.1: a retry says it is one, and what it is retrying. + if let Some(note) = plan.respawn_note(&item.task_id_str, &item.current_state, budget) { + emit_run_message(sink, rhei_tui::MessageLevel::Info, note); + } + + let from_state = task.state.as_str().to_string(); + let started_at = std::time::Instant::now(); + let started_wall = std::time::SystemTime::now(); + sink.emit(rhei_tui::RunEvent::SlotAssigned { + slot, + task: item.task_id_str.clone(), + from: from_state.clone(), + to: item.current_state.clone(), + agent: None, + template_context: None, + log_path: log.clone(), + started_at, + wall_clock: started_wall, + }); + + // Read before the plan moves into the worker: only here are the plan and + // the resolved budget both in hand. §FS-rhei-agents.3.2.1 + let outlook_for_result = plan.retry_outlook(budget); + let plan_for_thread = plan; + let resolved_for_thread = item.resolved.clone(); + let workspace_root_for_thread = workspace_root.to_path_buf(); + let task_roots_for_thread = loaded.task_roots.clone(); + let callback_paths_for_thread = callback_paths.clone(); + let plan_title_for_thread = loaded.rhei.title.clone(); + let task_for_thread = task.clone(); + let state_name_for_thread = item.current_state.clone(); + let current_state_raw_for_thread = task.state.as_str().to_string(); + let machine_for_thread = machine.clone(); + let metadata_for_thread = loaded.rhei.metadata.clone(); + let log_for_thread = log.clone(); + let sink_for_thread = sink.clone(); + let task_id_for_result = item.task_id_str.clone(); + let state_name_for_result = item.current_state.clone(); + let task_id_for_panic = item.task_id_str.clone(); + let state_for_panic = item.current_state.clone(); + // §FS-rhei-run.3.2: the program's group belongs to this run. + let run_owner = current_run_owner(); + + let handle = std::thread::spawn(move || { + inherit_run_owner(run_owner); + let thread_result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let render_context = RuntimeTemplateContext { + workspace_root: &workspace_root_for_thread, + task_roots: Some(&task_roots_for_thread), + // A program state renders no supervisor brief, and the task + // tree does not cross into the worker thread. + plan_tasks: None, + checkout_root: &workspace_root_for_thread, + plan_path: &callback_paths_for_thread.plan_path, + state_machine_path: callback_paths_for_thread.state_machine_path.as_deref(), + plan_title: &plan_title_for_thread, + task: &task_for_thread, + state_name: &state_name_for_thread, + current_state_raw: ¤t_state_raw_for_thread, + machine: &machine_for_thread, + metadata: metadata_for_thread.as_ref(), + target: None, + model: None, + model_provider: None, + model_name: None, + agent: None, + agent_mode: None, + tooling: None, + memory: None, + }; + let result = spawn_and_wait_program( + &resolved_for_thread, + &render_context, + &log_for_thread, + // §FS-rhei-agents.8.4: written when this command ends. + &plan_for_thread, + &sink_for_thread, + ); + let duration_ms = started_at.elapsed().as_millis() as u64; + let (outcome, exit_code) = slot_outcome(&result); + sink_for_thread.emit(rhei_tui::RunEvent::SlotReleased { + slot, + task: task_id_for_result.clone(), + from: from_state, + to: state_name_for_result.clone(), + log_path: log_for_thread, + outcome, + finished_at: std::time::Instant::now(), + wall_clock: std::time::SystemTime::now(), + exit_code, + duration_ms, + }); + ParallelAgentThreadMessage::ProgramCompleted(ParallelProgramCompletion { + task_id_str: task_id_for_result, + state_name: state_name_for_result, + retry_outlook: outlook_for_result, + result, + slot, + }) + })); + let message = thread_result.unwrap_or(ParallelAgentThreadMessage::Panicked { + task_id_str: task_id_for_panic, + state_name: state_for_panic, + slot, + }); + let _ = tx.send(message); + }); + + Ok(ParallelProgramSpawnOutcome::Spawned(ParallelProgramSpawned { + task_id_str: item.task_id_str.clone(), + state_name: item.current_state.clone(), + handle, + })) +} diff --git a/crates/rhei-cli/src/cli/run_parallel_schedule.rs b/crates/rhei-cli/src/cli/run_parallel_schedule.rs index 234d8542..b2d70b42 100644 --- a/crates/rhei-cli/src/cli/run_parallel_schedule.rs +++ b/crates/rhei-cli/src/cli/run_parallel_schedule.rs @@ -111,6 +111,7 @@ fn schedule_program_work_items( tx: &std::sync::mpsc::Sender, input: &Path, machines: &ExecutionMachines, + settings: &RheiSettings, workspace_root: &Path, runtime_dir: &Path, sink: &Arc, @@ -140,6 +141,7 @@ fn schedule_program_work_items( tx.clone(), input, machines, + settings, workspace_root, runtime_dir, sink, @@ -265,6 +267,7 @@ fn refill_parallel_worker_pool( tx, input, machines, + settings, workspace_root, runtime_dir, sink, diff --git a/crates/rhei-cli/src/cli/run_parallel_spawn.rs b/crates/rhei-cli/src/cli/run_parallel_spawn.rs index 1bcac38f..75a2d9a1 100644 --- a/crates/rhei-cli/src/cli/run_parallel_spawn.rs +++ b/crates/rhei-cli/src/cli/run_parallel_spawn.rs @@ -1,10 +1,11 @@ -// Putting one work item on a worker thread: the slot it takes, the prompt and -// snapshot staging it needs before the process starts, and the completion it +// Putting one agent work item on a worker thread: the slot it takes, the prompt +// and snapshot staging it needs before the process starts, and the completion it // sends back down the channel. // // Its own part because spawning is where an invocation stops being schedulable // data and becomes a live subprocess; the scheduler next door only decides -// which items get this far, and how many at a time. +// which items get this far, and how many at a time. A program work item is +// spawned by the part after this one. // §AR-source-file-size.3 §FS-rhei-run.3 @@ -54,6 +55,50 @@ fn spawn_parallel_agent_work_item( // Attribute the spawned unit to its owning rhei: prompts, logs, and // artifacts resolve against that rhei's execution root. §FS-rhei-panta.6.2 let task_workspace_root = loaded.task_root(&item.task_id_str, workspace_root); + let visit_count = render_visit_count( + loaded.rhei.metadata.as_ref(), + &task.id, + &item.current_state, + task.state.as_str(), + machine, + ); + // Settled before anything is composed or staged, as in the sequential path: + // a spawn this visit may not have costs nothing to decline. + // §FS-rhei-agents.3.2.3 §FS-rhei-agents.8.1 + let plan = plan_spawn_attempt( + runtime_dir, + &task_workspace_root, + &item.task_id_str, + &item.current_state, + resolved_agent_log_suffix(&item.resolved, Some(visit_count)).as_deref(), + ); + let budget = + resolve_attempt_budget(machine.states.get(item.current_state.as_str()), settings); + if plan.budget_spent(budget) { + // `Skipped` is the pool's stall: the scheduler records it in + // `stalled_tasks`, so the ticket keeps its state and is out of the + // running for the rest of the run. §FS-rhei-run.3 §FS-rhei-agents.3.2.3 + let owed = collect_missing_required_outputs( + workspace_root, + &task_workspace_root, + machine, + loaded.rhei.metadata.as_ref(), + task, + &item.current_state, + selected_forward_transition(&loaded.rhei, machine, task).as_deref(), + ); + emit_run_message( + sink, + rhei_tui::MessageLevel::Warn, + budget_spent_halt_line( + &item.task_id_str, + &item.current_state, + budget, + &completion_debt_label(&owed), + ), + ); + return Ok(ParallelAgentSpawnOutcome::Skipped); + } let workspace_root = task_workspace_root.as_path(); let tooling = resolve_tooling(machine, &item.current_state, settings); @@ -148,19 +193,9 @@ fn spawn_parallel_agent_work_item( return Ok(ParallelAgentSpawnOutcome::Unpromptable(item.task_id_str.clone())); } }; - let visit_count = render_visit_count( - loaded.rhei.metadata.as_ref(), - &task.id, - &item.current_state, - task.state.as_str(), - machine, - ); - let log = agent_log_path( - runtime_dir, - &item.task_id_str, - &item.current_state, - resolved_agent_log_suffix(&item.resolved, Some(visit_count)).as_deref(), - ); + // A retry gets its own attempt log rather than truncating the transcript + // that explains the miss it is retrying. §FS-rhei-agents.8.1 + let log = plan.log.clone(); let working_dir = checkout_root.path.clone(); let worktree_root = checkout_root.worktree_root.clone(); let plan_path = callback_paths.plan_path.clone(); @@ -194,6 +229,12 @@ fn spawn_parallel_agent_work_item( rhei_tui::MessageLevel::Info, format!(" Log: {}", log.display()), ); + // Names the rule, the attempt, and the budget it comes out of, so a loop is + // visible while it spends rather than at the halt. + // §FS-rhei-agents.3.2.1 §FS-rhei-run.3 + if let Some(note) = plan.respawn_note(&item.task_id_str, &item.current_state, budget) { + emit_run_message(sink, rhei_tui::MessageLevel::Info, note); + } let snapshot_preload = preload_snapshot_inherit_before_spawn( input, @@ -233,6 +274,10 @@ fn spawn_parallel_agent_work_item( let to_for_thread = item.current_state.clone(); let tid_for_event = item.task_id_str.clone(); let runtime_dir_for_thread = runtime_dir.to_path_buf(); + // Read before the plan moves into the worker: only here are the plan and + // the resolved budget both in hand. §FS-rhei-agents.3.2.1 + let outlook_for_result = plan.retry_outlook(budget); + let plan_for_thread = plan; let snapshot_preload_for_thread = snapshot_preload.clone(); let snapshot_preload_for_result = snapshot_preload.clone(); let visit_for_result = visit_count; @@ -269,6 +314,9 @@ fn spawn_parallel_agent_work_item( slot, sink_for_thread.clone(), intervene_for_thread.as_ref(), + // Written when this spawn ends, so its presence proves one ran. + // §FS-rhei-agents.8.4 + &plan_for_thread, result_identity.as_deref(), ); let duration_ms = started_at.elapsed().as_millis() as u64; @@ -313,6 +361,7 @@ fn spawn_parallel_agent_work_item( log: log_for_result, snapshot_preload: snapshot_preload_for_result, visit_count: visit_for_result, + retry_outlook: outlook_for_result, result, accounting_recorded, accounting_warning, @@ -333,143 +382,3 @@ fn spawn_parallel_agent_work_item( handle, })) } - -#[allow(clippy::too_many_arguments)] -fn spawn_parallel_program_work_item( - item: &ProgramWorkItem, - slot: rhei_tui::Slot, - tx: std::sync::mpsc::Sender, - input: &Path, - machines: &ExecutionMachines, - workspace_root: &Path, - runtime_dir: &Path, - sink: &Arc, -) -> MietteResult { - // As for agents: a slot was reserved for this item before the interrupt, - // and the shutdown starts nothing further. §FS-rhei-run.3.2 - if interrupt_requested() { - return Ok(ParallelProgramSpawnOutcome::Skipped); - } - let loaded = load_plan(input)?; - let target_id = parse_task_id(&item.task_id_str); - // The item's owning rhei supplies its machine and callback base. - // §DA-per-rhei-state-machines - let machine = machines.for_task_str(&item.task_id_str); - let callback_paths = machines.callbacks_for_str(&item.task_id_str); - let task = find_task_by_id(&loaded.rhei.tasks, &target_id); - let Some(task) = task else { return Ok(ParallelProgramSpawnOutcome::Skipped) }; - - // Programs run against the owning rhei's execution root. §FS-rhei-panta.6.2 - let task_workspace_root = loaded.task_root(&item.task_id_str, workspace_root); - let workspace_root = task_workspace_root.as_path(); - - let log = program_log_path(runtime_dir, &item.task_id_str, &item.current_state); - emit_run_message( - sink, - rhei_tui::MessageLevel::Info, - format!("\nSpawning program for Task {}: {} (parallel)", item.task_id_str, task.title), - ); - emit_run_message(sink, rhei_tui::MessageLevel::Info, format!(" Log: {}", log.display())); - - let from_state = task.state.as_str().to_string(); - let started_at = std::time::Instant::now(); - let started_wall = std::time::SystemTime::now(); - sink.emit(rhei_tui::RunEvent::SlotAssigned { - slot, - task: item.task_id_str.clone(), - from: from_state.clone(), - to: item.current_state.clone(), - agent: None, - template_context: None, - log_path: log.clone(), - started_at, - wall_clock: started_wall, - }); - - let resolved_for_thread = item.resolved.clone(); - let workspace_root_for_thread = workspace_root.to_path_buf(); - let task_roots_for_thread = loaded.task_roots.clone(); - let callback_paths_for_thread = callback_paths.clone(); - let plan_title_for_thread = loaded.rhei.title.clone(); - let task_for_thread = task.clone(); - let state_name_for_thread = item.current_state.clone(); - let current_state_raw_for_thread = task.state.as_str().to_string(); - let machine_for_thread = machine.clone(); - let metadata_for_thread = loaded.rhei.metadata.clone(); - let log_for_thread = log.clone(); - let sink_for_thread = sink.clone(); - let task_id_for_result = item.task_id_str.clone(); - let state_name_for_result = item.current_state.clone(); - let task_id_for_panic = item.task_id_str.clone(); - let state_for_panic = item.current_state.clone(); - // §FS-rhei-run.3.2: the program's group belongs to this run. - let run_owner = current_run_owner(); - - let handle = std::thread::spawn(move || { - inherit_run_owner(run_owner); - let thread_result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - let render_context = RuntimeTemplateContext { - workspace_root: &workspace_root_for_thread, - task_roots: Some(&task_roots_for_thread), - // A program state renders no supervisor brief, and the task - // tree does not cross into the worker thread. - plan_tasks: None, - checkout_root: &workspace_root_for_thread, - plan_path: &callback_paths_for_thread.plan_path, - state_machine_path: callback_paths_for_thread.state_machine_path.as_deref(), - plan_title: &plan_title_for_thread, - task: &task_for_thread, - state_name: &state_name_for_thread, - current_state_raw: ¤t_state_raw_for_thread, - machine: &machine_for_thread, - metadata: metadata_for_thread.as_ref(), - target: None, - model: None, - model_provider: None, - model_name: None, - agent: None, - agent_mode: None, - tooling: None, - memory: None, - }; - let result = spawn_and_wait_program( - &resolved_for_thread, - &render_context, - &log_for_thread, - &sink_for_thread, - ); - let duration_ms = started_at.elapsed().as_millis() as u64; - let (outcome, exit_code) = slot_outcome(&result); - sink_for_thread.emit(rhei_tui::RunEvent::SlotReleased { - slot, - task: task_id_for_result.clone(), - from: from_state, - to: state_name_for_result.clone(), - log_path: log_for_thread, - outcome, - finished_at: std::time::Instant::now(), - wall_clock: std::time::SystemTime::now(), - exit_code, - duration_ms, - }); - ParallelAgentThreadMessage::ProgramCompleted(ParallelProgramCompletion { - task_id_str: task_id_for_result, - state_name: state_name_for_result, - result, - slot, - }) - })); - let message = thread_result.unwrap_or(ParallelAgentThreadMessage::Panicked { - task_id_str: task_id_for_panic, - state_name: state_for_panic, - slot, - }); - let _ = tx.send(message); - }); - - Ok(ParallelProgramSpawnOutcome::Spawned(ParallelProgramSpawned { - task_id_str: item.task_id_str.clone(), - state_name: item.current_state.clone(), - handle, - })) -} diff --git a/crates/rhei-cli/src/cli/run_program_sequential.rs b/crates/rhei-cli/src/cli/run_program_sequential.rs index 0bb9e40f..f0b28ea0 100644 --- a/crates/rhei-cli/src/cli/run_program_sequential.rs +++ b/crates/rhei-cli/src/cli/run_program_sequential.rs @@ -19,6 +19,7 @@ fn run_sequential_program_work_items( plan_title: &str, input: &Path, machines: &ExecutionMachines, + settings: &RheiSettings, opts: &RunOptions, workspace_root: &Path, runtime_dir: &Path, @@ -67,10 +68,47 @@ fn run_sequential_program_work_items( tooling: None, memory: None, }; - let log = program_log_path(runtime_dir, task_id_str, current_state); + // A program is never skipped at scheduling, so it re-spawns for the + // same reason an agent does — and gets the same attempt log and the + // same per-visit budget. §FS-rhei-agents.8.1 §FS-rhei-agents.3.2.3 + let plan = plan_spawn_attempt( + runtime_dir, + &task_workspace_root, + task_id_str, + current_state, + None, + ); + let budget = resolve_attempt_budget(machine.states.get(current_state.as_str()), settings); + if plan.budget_spent(budget) { + let owed = collect_missing_required_outputs( + workspace_root, + &task_workspace_root, + machine, + loaded.rhei.metadata.as_ref(), + task, + current_state, + selected_forward_transition(&loaded.rhei, machine, task).as_deref(), + ); + run_warn!( + "{}", + budget_spent_halt_line( + task_id_str, + current_state, + budget, + &completion_debt_label(&owed) + ) + ); + progress.stalled_tasks.insert(task_id_str.clone()); + continue; + } + let log = plan.log.clone(); run_info!("\nSpawning program for Task {}: {}", task_id_str, task.title); run_info!(" Log: {}", log.display()); + // §FS-rhei-agents.3.2.1: a retry says it is one, and what it is retrying. + if let Some(note) = plan.respawn_note(task_id_str, current_state, budget) { + run_info!("{note}"); + } let started_at = std::time::Instant::now(); let started_wall = std::time::SystemTime::now(); @@ -87,7 +125,7 @@ fn run_sequential_program_work_items( }); let spawn_result = - spawn_and_wait_program(resolved, &render_context, &log, sink); + spawn_and_wait_program(resolved, &render_context, &log, &plan, sink); let duration_ms = started_at.elapsed().as_millis() as u64; let finished_wall = SystemTime::now(); let (outcome, exit_code) = slot_outcome(&spawn_result); @@ -206,6 +244,7 @@ fn run_sequential_program_work_items( task_id_str, current_state, &missing_required_outputs, + plan.retry_outlook(budget), sink, ); progress.stalled_tasks.insert(task_id_str.clone()); diff --git a/crates/rhei-cli/src/cli/run_prompt_sections.rs b/crates/rhei-cli/src/cli/run_prompt_sections.rs index bf4fa53d..580ae677 100644 --- a/crates/rhei-cli/src/cli/run_prompt_sections.rs +++ b/crates/rhei-cli/src/cli/run_prompt_sections.rs @@ -162,6 +162,30 @@ fn render_declared_exports(render_context: &RuntimeTemplateContext<'_>) -> Strin /// same reason. // §FS-rhei-agents.3 §FS-rhei-states.3.3 fn render_terminal_result(render_context: &RuntimeTemplateContext<'_>) -> String { + let Some(shown) = terminal_result_path_shown(render_context) else { return String::new() }; + // §FS-rhei-supervision.4.1: on a supervising state only the visit that finds + // the subtree closed finishes the task; every earlier one just releases. + let qualifier = if task_is_supervising(render_context.task, render_context.machine) { + format!(" {SUPERVISOR_RESULT_QUALIFIER}") + } else { + String::new() + }; + format!( + "\n## Result\n\n\ + A transition from this state can finish this task. The finished task's result is read \ + from this file.{qualifier}\n\n- `{shown}`\n" + ) +} + +/// The result path this invocation is handed, exactly as the `## Result` +/// section shows it — or `None` on a state no terminal edge leaves. +/// +/// One expression, two readers: the section that states the obligation and the +/// retry paragraph that says the previous attempt did not meet it. A second copy +/// would be a second chance to name a different file than the one +/// `RHEI_RESULT_PATH` holds. +// §FS-rhei-states.3.3 §FS-rhei-memory.4.4 +fn terminal_result_path_shown(render_context: &RuntimeTemplateContext<'_>) -> Option { let can_finish = render_context.machine.transitions().iter().any(|rule| { rule.from.0 == render_context.state_name && render_context @@ -172,7 +196,7 @@ fn render_terminal_result(render_context: &RuntimeTemplateContext<'_>) -> String .unwrap_or(false) }); if !can_finish { - return String::new(); + return None; } let task_id = render_context.task.id.to_string(); // A fanned-out invocation writes its own fragment, so the path it is shown @@ -198,25 +222,13 @@ fn render_terminal_result(render_context: &RuntimeTemplateContext<'_>) -> String // Same rule declared artifacts follow: relative under the artifact root, // absolute when the agent's cwd is somewhere else entirely. // §FS-rhei-agents.4 - let shown = if render_context.checkout_root == render_context.workspace_root { + Some(if render_context.checkout_root == render_context.workspace_root { relative } else { invocation_result_file_path(render_context.workspace_root, &task_id, invocation) .display() .to_string() - }; - // §FS-rhei-supervision.4.1: on a supervising state only the visit that finds - // the subtree closed finishes the task; every earlier one just releases. - let qualifier = if task_is_supervising(render_context.task, render_context.machine) { - format!(" {SUPERVISOR_RESULT_QUALIFIER}") - } else { - String::new() - }; - format!( - "\n## Result\n\n\ - A transition from this state can finish this task. The finished task's result is read \ - from this file.{qualifier}\n\n- `{shown}`\n" - ) + }) } /// Render the exports this task consumes from prior tasks. diff --git a/crates/rhei-cli/src/cli/run_prompt_visits.rs b/crates/rhei-cli/src/cli/run_prompt_visits.rs index d0bcf671..bebc86ee 100644 --- a/crates/rhei-cli/src/cli/run_prompt_visits.rs +++ b/crates/rhei-cli/src/cli/run_prompt_visits.rs @@ -68,18 +68,68 @@ fn render_previous_log(render_context: &RuntimeTemplateContext<'_>) -> String { if visit <= 1 { return String::new(); } - let path = agent_log_path( + // The previous visit's *last* attempt: where it was retried, that is the + // one that ran, and the earlier ones are kept beside it. §FS-rhei-agents.8.1 + let Some(path) = latest_agent_log_path( &memory.runtime_dir, &render_context.task.id.to_string(), render_context.state_name, agent_log_suffix(render_context.target, render_context.model, Some(visit - 1)).as_deref(), - ); - if !path.exists() { + ) else { return String::new(); - } + }; format!("\nPrevious log: `{}`\n", memory_path(render_context, &path)) } +/// What this visit already tried, when it has already tried something. +/// +/// A re-spawn used to receive the prompt of the attempt it was recovering from, +/// byte for byte: same `RHEI_VISIT_COUNT`, no attempt number, and no +/// `Previous log:` line, because that line keys off the *previous visit* and a +/// stalled ticket never left this one. So attempt two did what attempt one did +/// and left the same thing unwritten. This paragraph is the difference: it says +/// that this is a retry, which attempt it is, how the last one ended, and which +/// file that attempt was obliged to write and did not — the result path, which +/// the prompt already showed as where a finished task's result is *read from*, +/// and which agents read as description rather than as obligation. +/// +/// Rendered only when the record belongs to *this* visit. A record from an +/// earlier stay in the state is not a retry, and telling a fresh entry that it +/// is one is the same untruth in the other direction. +// §FS-rhei-memory.3.3 §FS-rhei-memory.4.4 §FS-rhei-agents.3.2.1 +fn render_retry_notice(render_context: &RuntimeTemplateContext<'_>, task_root: &Path) -> String { + let Some(memory) = render_context.memory else { return String::new() }; + let visit = render_visit_count( + render_context.metadata, + &render_context.task.id, + render_context.state_name, + render_context.current_state_raw, + render_context.machine, + ); + let plan = plan_spawn_attempt( + &memory.runtime_dir, + task_root, + &render_context.task.id.to_string(), + render_context.state_name, + agent_log_suffix(render_context.target, render_context.model, Some(visit)).as_deref(), + ); + let Some(previous) = plan.previous.as_ref() else { return String::new() }; + let owed = match terminal_result_path_shown(render_context) { + Some(path) => format!( + " It did not write `{path}`, which a transition out of this state reads to finish \ + this task." + ), + None => String::new(), + }; + format!( + "\nRetrying this visit: attempt {}. The previous attempt {}.{owed} Its transcript is \ + `{}`.\n", + plan.attempt, + previous.ending_sentence(), + memory_path(render_context, &previous.log) + ) +} + /// Every verdict recorded against this task so far, pasted whole. /// /// This is where a worker's `--result` message and the engine's own failure @@ -124,7 +174,11 @@ fn render_previous_visits(render_context: &RuntimeTemplateContext<'_>) -> Miette let task_id = render_context.task.id.to_string(); let has_trail = ledger.iter().any(|(entry_task, _, _)| entry_task == &task_id); let result = read_task_result(render_context, &render_context.task.id)?; - if !has_trail && result.is_none() { + // A ticket retried on its first visit has neither a ledger line nor a + // result, and it is exactly the invocation that most needs to be told it is + // a retry. §FS-rhei-memory.4.4 + let retry = render_retry_notice(render_context, root); + if !has_trail && result.is_none() && retry.is_empty() { return Ok(String::new()); } let mut out = String::from("\n## Previous Visits\n\n"); @@ -133,5 +187,6 @@ fn render_previous_visits(render_context: &RuntimeTemplateContext<'_>) -> Miette out.push_str(&render_result_entries(render_context, &body)); } out.push_str(&render_previous_log(render_context)); + out.push_str(&retry); Ok(out) } diff --git a/crates/rhei-cli/src/cli/run_work_items.rs b/crates/rhei-cli/src/cli/run_work_items.rs index 17b67d6f..f171e7ad 100644 --- a/crates/rhei-cli/src/cli/run_work_items.rs +++ b/crates/rhei-cli/src/cli/run_work_items.rs @@ -54,6 +54,11 @@ struct ParallelAgentCompletion { log: PathBuf, snapshot_preload: SnapshotPreload, visit_count: u64, + /// Whether the visit this invocation belongs to has an attempt left after + /// it. Decided at the spawn, where the budget is resolved, and read where + /// the run says what it will do next. + // §FS-rhei-agents.3.2.1 §FS-rhei-agents.3.2.3 + retry_outlook: RetryOutlook, result: MietteResult, accounting_recorded: bool, accounting_warning: Option, @@ -70,6 +75,11 @@ struct ParallelAgentExit { log: PathBuf, snapshot_preload: SnapshotPreload, visit_count: u64, + /// Whether the visit this invocation belongs to has an attempt left after + /// it. Decided at the spawn, where the budget is resolved, and read where + /// the run says what it will do next. + // §FS-rhei-agents.3.2.1 §FS-rhei-agents.3.2.3 + retry_outlook: RetryOutlook, accounting_recorded: bool, outcome: AgentSpawnOutcome, } @@ -77,6 +87,11 @@ struct ParallelAgentExit { struct ParallelProgramCompletion { task_id_str: String, state_name: String, + /// Whether the visit this invocation belongs to has an attempt left after + /// it. Decided at the spawn, where the budget is resolved, and read where + /// the run says what it will do next. + // §FS-rhei-agents.3.2.1 §FS-rhei-agents.3.2.3 + retry_outlook: RetryOutlook, result: MietteResult, slot: rhei_tui::Slot, } @@ -274,25 +289,18 @@ fn collect_ready_agent_work_items( continue; } - let pending = if state_def.outputs.is_empty() { - invocations - } else { - invocations - .into_iter() - .filter(|resolved| { - !state_outputs_exist_for_resolved_invocation( - workspace_root, - task, - ¤t_state, - task.state.as_str(), - machine, - loaded.rhei.metadata.as_ref(), - state_def, - resolved, - ) - }) - .collect::>() - }; + // The rule the pass driver and the post-exit check apply: a refill + // re-spawns an invocation whose completion condition is unmet, and skips + // only one that is genuinely finished. §FS-rhei-agents.3.2 + let pending = agent_invocations_to_spawn( + loaded, + workspace_root, + task, + machine, + ¤t_state, + state_def, + invocations, + ); if pending.is_empty() { continue; diff --git a/crates/rhei-cli/src/cli/settings_load_validate.rs b/crates/rhei-cli/src/cli/settings_load_validate.rs index 158795eb..ed79f0a9 100644 --- a/crates/rhei-cli/src/cli/settings_load_validate.rs +++ b/crates/rhei-cli/src/cli/settings_load_validate.rs @@ -143,6 +143,11 @@ fn load_merged_settings(plan_root: &Path) -> MietteResult { } else { global.defaults.program_timeout }, + attempts: if json_nested_field_present(project_raw, "defaults", "attempts") { + project.defaults.attempts + } else { + global.defaults.attempts + }, mcp_servers: if json_nested_field_present(project_raw, "defaults", "mcp_servers") { project.defaults.mcp_servers } else { diff --git a/crates/rhei-cli/src/cli/settings_types.rs b/crates/rhei-cli/src/cli/settings_types.rs index f776c11b..5189c8b3 100644 --- a/crates/rhei-cli/src/cli/settings_types.rs +++ b/crates/rhei-cli/src/cli/settings_types.rs @@ -162,6 +162,10 @@ struct SettingsDefaults { /// §FS-rhei-agents.1.1.1: Default program timeout. #[serde(default)] program_timeout: Option, + /// Default per-visit attempt budget for states that do not set `attempts:`. + // §FS-rhei-agents.3.2.3: the resolution chain this is the second level of. + #[serde(default)] + attempts: Option, #[serde(default)] mcp_servers: Option>, #[serde(default)] diff --git a/crates/rhei-cli/src/cli/states_render.rs b/crates/rhei-cli/src/cli/states_render.rs index 71d3e7fc..333e5e4c 100644 --- a/crates/rhei-cli/src/cli/states_render.rs +++ b/crates/rhei-cli/src/cli/states_render.rs @@ -200,6 +200,9 @@ fn render_state_machine_json(machine: &rhei_validator::StateMachine) -> Result, - /// A message the engine records *only* when the effective target turns out - /// to be `final: true` and nothing else answered for the ticket. + /// The engine's own account of the move, recorded *only* when the effective + /// target turns out to be `final: true` and nothing else answered for the + /// ticket. /// /// Callback-only advancement sets it: taking the edge is an outcome the /// engine produced, with no subprocess in the source state that could know /// better, so the engine narrates exactly that. It is not a carried /// message, because the same sentence would be a lie on a non-terminal hop - /// and noise beside a result a callback did write. - // §FS-rhei-run.3 - terminal_result_fallback: Option, + /// and noise beside a result a callback did write. It is carried as facts + /// rather than as a finished sentence because one of its clauses — whether + /// the source state's declared `outputs:` were verified — is only settled + /// further down this very transition, by the check that the reserved + /// `cancelled` target waives. + // §FS-rhei-run.3 §FS-rhei-states.1.4 + terminal_result_fallback: Option, } -/// The message `rhei run` records when it takes an edge itself, with no agent -/// or program having run in the source state. +/// What the engine can say about a state it advanced out of without spawning +/// anything, held as facts until the transition settles the last of them. +// §FS-rhei-run.3 §FS-rhei-agents.8.4 +#[derive(Debug, Clone)] +struct CallbackOnlyAccount { + from: String, + /// The worker a spawn record proves ran there earlier, already phrased. + /// `None` when no record answers for the state, which is the only case in + /// which the engine may say that nothing ran. + // §FS-rhei-agents.8.4 + evidence: Option, + /// Whether the source state declares `outputs:` at all. With none there is + /// nothing to report about them either way. + declares_outputs: bool, +} + +impl CallbackOnlyAccount { + /// The sentence, finished at the point the outputs question has an answer. + /// + /// `outputs_verified` is not this function's guess: the caller passes what + /// the transition actually did — the source-outputs check ran and passed, + /// or it was waived for the reserved `cancelled` target. Asserting "all + /// present" without it is how this sentence came to claim a check that the + /// waiver had skipped. + // §FS-rhei-run.3 §FS-rhei-states.1.4 + fn sentence(&self, outputs_verified: bool) -> String { + let from = &self.from; + let opening = format!( + "`rhei run`: this task was finished by callback-only orchestration from state \ + '{from}'." + ); + let Some(evidence) = &self.evidence else { + return format!( + "{opening} No agent or program ran in that state, so no worker result was \ + recorded." + ); + }; + let outputs = match (self.declares_outputs, outputs_verified) { + (false, _) => "", + (true, true) => " The state's declared outputs were checked on this edge and were \ + all present.", + (true, false) => " The state's declared outputs were not checked: this edge \ + abandons the work, which waives them.", + }; + format!( + "{opening} No worker was spawned there on this run, but {evidence} and wrote no \ + result.{outputs} No worker result was recorded here; that worker's account of its \ + work is its log, not this file." + ) + } +} + +/// The facts `rhei run` records when it takes an edge itself rather than on a +/// worker's behalf. /// /// It says plainly that no worker result was recorded — which is the fact a /// reader of the file needs, and the fact the old empty result file withheld. /// It is a fallback: a result a callback wrote wins, and on a non-terminal hop /// nothing is written at all. -// §FS-rhei-run.3 §FS-rhei-states.3.3 -fn callback_only_terminal_result(from: &str) -> String { - format!( - "`rhei run`: this task was finished by callback-only orchestration from state '{from}'. \ - No agent or program ran in that state, so no worker result was recorded." - ) +/// +/// What it says *about the worker* is checked rather than assumed. "No agent +/// ran" is true of a machine with no autonomous state and of a run that was told +/// not to spawn one, but it is a lie on a state an earlier run did work in — +/// which is exactly the state a `--no-agent` run is most likely to walk out of. +/// The evidence is the spawn record, and only that: a log file is opened before +/// its subprocess starts, so a `command:` naming a binary that does not exist +/// leaves one behind for a worker that never ran, and crediting that worker +/// would be the same class of lie as denying a real one. The accounting record +/// could not serve either — it exists only for agents that resolve a provider +/// and model. +// §FS-rhei-run.3 §FS-rhei-states.3.3 §FS-rhei-agents.8.4 +fn callback_only_terminal_result( + runtime_dir: &Path, + task_id: &str, + from: &str, + from_state_def: Option<&rhei_validator::StateDef>, +) -> CallbackOnlyAccount { + CallbackOnlyAccount { + from: from.to_string(), + evidence: prior_worker_run(runtime_dir, task_id, from), + declares_outputs: from_state_def.is_some_and(|def| !def.outputs.is_empty()), + } +} + +/// One sentence about the run a spawn record is evidence of: who ran, where the +/// transcript is, and how it ended. +/// +/// The record says which *kind* of worker ran, so the sentence names the agent +/// or the program that was actually there rather than the agent this state +/// would resolve to today. +// §FS-rhei-agents.8.4 §FS-rhei-programs.5 §FS-rhei-run.3 +fn prior_worker_run(runtime_dir: &Path, task_id: &str, state: &str) -> Option { + let record = newest_spawn_record_for_state(runtime_dir, task_id, state)?; + let ending = match record.code { + Some(code) => format!(", exit code {code}, after {}", record.duration), + None => format!(", after {}", record.duration), + }; + let who = match record.kind.as_str() { + "program" => format!("program `{}` ran in that state earlier", record.worker), + _ => format!("agent '{}' ran in that state earlier", record.worker), + }; + // Absolute, for the reason a missing result's path is absolute: the result + // file this lands in may sit under a different root than the run's logs, so + // a relative path is one the reader cannot follow. §FS-rhei-panta.6.2 + let shown = std::path::absolute(&record.log).unwrap_or(record.log); + Some(format!("{who} (log: {}{ending})", shown.display())) } /// Variant of [`execute_transition`] for `rhei run`'s callback-only @@ -64,7 +162,16 @@ fn execute_callback_only_transition( from: &str, to: &str, no_callbacks: bool, + // Where the run keeps spawn records, which is the evidence the engine's own + // account of the source state is built from. §FS-rhei-agents.8.4 + runtime_dir: &Path, ) -> MietteResult { + let account = callback_only_terminal_result( + runtime_dir, + files.artifact_id, + from, + machine.states.get(from), + ); execute_transition_with_origin( files, callback_paths, @@ -74,7 +181,7 @@ fn execute_callback_only_transition( to, no_callbacks, TransitionOrigin { - terminal_result_fallback: Some(callback_only_terminal_result(from)), + terminal_result_fallback: Some(account), ..TransitionOrigin::default() }, ) diff --git a/crates/rhei-cli/src/cli/tests_agent_execution_validation.rs b/crates/rhei-cli/src/cli/tests_agent_execution_validation.rs index 9dbfb27e..080dfb60 100644 --- a/crates/rhei-cli/src/cli/tests_agent_execution_validation.rs +++ b/crates/rhei-cli/src/cli/tests_agent_execution_validation.rs @@ -36,6 +36,7 @@ 0, recorder.clone(), None, + &spawn_plan_for_test(&log_path), None, ) .expect("fake agent runs"); @@ -209,6 +210,7 @@ for line in sys.stdin: 0, recorder, Some(&intervene), + &spawn_plan_for_test(&log_path), None, ) .expect("fake stdin agent runs"); @@ -280,6 +282,7 @@ for line in sys.stdin: 0, recorder.clone(), None, + &spawn_plan_for_test(&log_path), None, ) .expect("timeout returns process status"); @@ -342,6 +345,7 @@ for line in sys.stdin: 0, recorder, None, + &spawn_plan_for_test(&log_path), None, ) .expect("agent should complete without waiting for inherited pipe EOF"); @@ -702,6 +706,7 @@ for line in sys.stdin: "task-1", "pending", 1, + 1, &tooling, runtime_dir.path(), None, diff --git a/crates/rhei-cli/src/cli/tests_agent_resolution.rs b/crates/rhei-cli/src/cli/tests_agent_resolution.rs index 56a79735..28a5f51d 100644 --- a/crates/rhei-cli/src/cli/tests_agent_resolution.rs +++ b/crates/rhei-cli/src/cli/tests_agent_resolution.rs @@ -12,6 +12,7 @@ agent_mode: None, agent_timeout: Some("45m".to_string()), program_timeout: None, + attempts: None, mcp_servers: None, skills: None, }, @@ -195,6 +196,7 @@ "task-1", "pending", 7, + 1, &tooling, runtime_dir.path(), None, @@ -248,6 +250,7 @@ "task-1", "pending", 1, + 1, &tooling, runtime_dir.path(), None, @@ -297,6 +300,7 @@ "task-1", "pending", 1, + 1, &tooling, runtime_dir.path(), None, @@ -394,6 +398,7 @@ 0, recorder, None, + &spawn_plan_for_test(&log_path), None, ) .expect("agent runs"); @@ -445,6 +450,7 @@ "analysis", "analyze", 1, + 1, &tooling, runtime_dir.path(), None, diff --git a/crates/rhei-cli/src/cli/tests_complete_reset_tooling.rs b/crates/rhei-cli/src/cli/tests_complete_reset_tooling.rs index 302c1e75..e7623676 100644 --- a/crates/rhei-cli/src/cli/tests_complete_reset_tooling.rs +++ b/crates/rhei-cli/src/cli/tests_complete_reset_tooling.rs @@ -871,6 +871,7 @@ transitions: agent_mode: None, agent_timeout: None, program_timeout: None, + attempts: None, mcp_servers: defaults_mcp, skills: None, }, diff --git a/crates/rhei-cli/src/cli/tests_settings_tooling.rs b/crates/rhei-cli/src/cli/tests_settings_tooling.rs index df130025..e5f387a4 100644 --- a/crates/rhei-cli/src/cli/tests_settings_tooling.rs +++ b/crates/rhei-cli/src/cli/tests_settings_tooling.rs @@ -35,6 +35,7 @@ "task-7", "pending", 1, + 1, &tooling, runtime_dir.path(), None, @@ -76,6 +77,7 @@ "t", "pending", 1, + 1, &tooling, runtime_dir.path(), None, @@ -1072,6 +1074,7 @@ states: "1", "pending", 1, + 1, &gate.tooling, runtime_dir.path(), None, @@ -1119,6 +1122,7 @@ states: 0, Arc::new(RecordingSink::default()), None, + &spawn_plan_for_test(&log_path), None, ) .expect("agent runs"); diff --git a/crates/rhei-cli/src/cli/tests_snapshots_gc.rs b/crates/rhei-cli/src/cli/tests_snapshots_gc.rs index 332eef88..d8621914 100644 --- a/crates/rhei-cli/src/cli/tests_snapshots_gc.rs +++ b/crates/rhei-cli/src/cli/tests_snapshots_gc.rs @@ -113,6 +113,7 @@ spawns.mkdir(parents=True, exist_ok=True) "1", "pending", &["required-report".to_string()], + RetryOutlook::AttemptsLeft, &sink, ); let events = recorder.events.lock().expect("events"); @@ -180,6 +181,7 @@ spawns.mkdir(parents=True, exist_ok=True) 0, recorder, None, + &spawn_plan_for_test(&log_path), None, ) .expect("timeout returns process status"); diff --git a/crates/rhei-cli/src/cli/tests_spawn_records.rs b/crates/rhei-cli/src/cli/tests_spawn_records.rs new file mode 100644 index 00000000..82fe604f --- /dev/null +++ b/crates/rhei-cli/src/cli/tests_spawn_records.rs @@ -0,0 +1,154 @@ +// The persistence under the attempt number: what makes two spawns one visit, +// what makes the next one a new visit, and what a spawn spends of its budget. +// +// Its own part because the end-to-end tests exercise this through `rhei run`, +// where a wrong answer shows up as a wrong log name three layers away. + +// §AR-source-file-size.3 §FS-rhei-agents.8.4 §FS-rhei-agents.3.2.3 + +mod spawn_records { + use super::super::*; + + /// One ticket's ledger, written where `ticket_move_count` reads it. + fn ledger(root: &std::path::Path, lines: &str) { + let runtime = root.join("runtime"); + fs::create_dir_all(&runtime).expect("runtime dir"); + fs::write(runtime.join("state-transitions.log"), lines).expect("ledger"); + } + + fn ended(plan: &SpawnPlan, ending: &str, code: i32) { + plan.record_spawn(SpawnEnding { + task_id: "plan.1", + state_name: "implement", + kind: "agent", + worker: "mock", + started: "2026-08-29T10:00:00Z", + ended: "2026-08-29T10:00:01Z", + duration: "1s", + code: Some(code), + ending, + }); + } + + fn plan_for(root: &std::path::Path) -> SpawnPlan { + plan_spawn_attempt(&root.join("runtime"), root, "plan.1", "implement", None) + } + + /// Two spawns with the ticket standing still are two attempts at one visit: + /// the second gets its own transcript rather than truncating the first, and + /// it carries the first forward as what it is retrying. + // §FS-rhei-agents.8.1 §FS-rhei-agents.8.4 + #[test] + fn a_second_spawn_without_a_move_is_the_second_attempt_of_one_visit() { + let dir = tempfile::tempdir().expect("tmpdir"); + ledger(dir.path(), "plan.1 draft@implement\n"); + + let first = plan_for(dir.path()); + assert_eq!(first.attempt, 1); + assert!(first.previous.is_none()); + assert!(first.log.ends_with("task-plan.1-implement.log")); + ended(&first, "exited", 0); + + let second = plan_for(dir.path()); + assert_eq!(second.attempt, 2); + assert!(second.log.ends_with("task-plan.1-implement-attempt2.log")); + assert_eq!( + second.previous.as_ref().map(SpawnRecord::ending_sentence).as_deref(), + Some("exited 0 without meeting this state's completion condition") + ); + } + + /// The ticket moving is what ends a visit. Everything about the next spawn + /// starts over: the plain log name, no previous attempt to narrate, and a + /// budget that has not been spent. + // §FS-rhei-agents.8.1 §FS-rhei-agents.3.2.3 + #[test] + fn a_move_starts_a_new_visit_with_a_fresh_name_and_budget() { + let dir = tempfile::tempdir().expect("tmpdir"); + ledger(dir.path(), "plan.1 draft@implement\n"); + let first = plan_for(dir.path()); + ended(&first, "exited", 0); + let second = plan_for(dir.path()); + ended(&second, "exited", 0); + assert!( + plan_for(dir.path()).budget_spent(2), + "two recorded attempts spend a budget of two" + ); + + // The ticket leaves and comes back: one more ledger line either way. + ledger(dir.path(), "plan.1 draft@implement\nplan.1 implement@review\nplan.1 review@implement\n"); + + let after = plan_for(dir.path()); + assert_eq!(after.attempt, 1, "a fresh entry is not a third attempt at the last one"); + assert!(after.previous.is_none(), "and has nothing to narrate as a retry"); + assert!(after.log.ends_with("task-plan.1-implement.log")); + assert!(!after.budget_spent(2), "the budget came back with the visit"); + } + + /// An interrupted invocation keeps its transcript — it ran — but the run + /// ended it, so it is not an attempt the ticket spent. Without this, two + /// Ctrl-Cs would halt a ticket that never had an attempt of its own. + // §FS-rhei-run.3.2 §FS-rhei-agents.3.2.3 + #[test] + fn an_interrupted_spawn_takes_an_attempt_log_but_not_a_budgeted_attempt() { + let dir = tempfile::tempdir().expect("tmpdir"); + ledger(dir.path(), "plan.1 draft@implement\n"); + + let first = plan_for(dir.path()); + ended(&first, "interrupted", -1); + let second = plan_for(dir.path()); + assert_eq!(second.attempt, 2, "the interrupted transcript is kept beside the retry"); + assert!(!second.budget_spent(1), "but it did not spend the visit's only attempt"); + + ended(&second, "interrupted", -1); + let third = plan_for(dir.path()); + assert!(!third.budget_spent(1), "and neither did the next interruption"); + + ended(&third, "timed out", -1); + let fourth = plan_for(dir.path()); + assert!(fourth.budget_spent(1), "a timeout is the ticket's own attempt, and spends it"); + assert_eq!( + fourth.previous.as_ref().map(SpawnRecord::ending_sentence).as_deref(), + Some("timed out after 1s") + ); + } + + /// A state's account of its own worker is matched on the record's fields, + /// never on the file name it happens to have: `review` and `review-fix` + /// share a prefix, and one used to answer with the other's transcript. + // §FS-rhei-agents.8.4 + #[test] + fn a_state_never_answers_with_a_prefix_siblings_worker() { + let dir = tempfile::tempdir().expect("tmpdir"); + let runtime = dir.path().join("runtime"); + let fix = SpawnPlan { + log: runtime.join("logs").join("task-plan.1-review-fix.log"), + record: spawn_record_path(&runtime, "plan.1", "review-fix", None), + moves: 0, + attempt: 1, + charged: 0, + previous: None, + }; + fix.record_spawn(SpawnEnding { + task_id: "plan.1", + state_name: "review-fix", + kind: "agent", + worker: "mock", + started: "2026-08-29T10:00:00Z", + ended: "2026-08-29T10:00:01Z", + duration: "1s", + code: Some(0), + ending: "exited", + }); + + assert!( + newest_spawn_record_for_state(&runtime, "plan.1", "review").is_none(), + "'review' had no worker of its own, whatever its neighbour is called" + ); + assert_eq!( + newest_spawn_record_for_state(&runtime, "plan.1", "review-fix") + .map(|record| record.worker), + Some("mock".to_string()) + ); + } +} diff --git a/crates/rhei-cli/src/lib.rs b/crates/rhei-cli/src/lib.rs index 0fb52e61..848ccaa6 100644 --- a/crates/rhei-cli/src/lib.rs +++ b/crates/rhei-cli/src/lib.rs @@ -56,7 +56,10 @@ include!("cli/settings_types.rs"); include!("cli/settings_load_validate.rs"); include!("cli/tooling_resolution.rs"); include!("cli/agent_resolution.rs"); +include!("cli/agent_log_files.rs"); +include!("cli/agent_spawn_records.rs"); include!("cli/run_helpers.rs"); +include!("cli/run_completion_condition.rs"); include!("cli/run_prompt_sections.rs"); include!("cli/run_prompt_handoffs.rs"); include!("cli/subtree_supervision_prompt.rs"); @@ -92,6 +95,7 @@ include!("cli/new_verify.rs"); include!("cli/run_command.rs"); include!("cli/run_work_items.rs"); include!("cli/run_parallel_spawn.rs"); +include!("cli/run_parallel_program_spawn.rs"); include!("cli/run_parallel_schedule.rs"); include!("cli/run_parallel_program_completion.rs"); include!("cli/run_agent_mode.rs"); @@ -137,6 +141,7 @@ mod tests { include!("cli/tests_complete_reset_tooling.rs"); include!("cli/tests_file_locks.rs"); include!("cli/tests_agent_resolution.rs"); + include!("cli/tests_spawn_records.rs"); include!("cli/tests_agent_execution_validation.rs"); include!("cli/tests_accounting.rs"); include!("cli/tests_settings_tooling.rs"); diff --git a/crates/rhei-cli/src/rhei_validator/validator/state_defs.rs b/crates/rhei-cli/src/rhei_validator/validator/state_defs.rs index 39cd5900..f20dbf59 100644 --- a/crates/rhei-cli/src/rhei_validator/validator/state_defs.rs +++ b/crates/rhei-cli/src/rhei_validator/validator/state_defs.rs @@ -89,6 +89,12 @@ pub struct StateDef { /// Maximum time an agent may work in this state (e.g., `"30m"`, `"1h"`). #[serde(default)] pub agent_timeout: Option, + /// How many times one *visit* to this state may be spawned before `rhei + /// run` halts the ticket. Distinct from `visits`, which bounds how many + /// times the ticket may enter the state at all. + // §FS-rhei-agents.3.2.3: the per-visit attempt budget. + #[serde(default)] + pub attempts: Option, /// Deterministic program command for this state (mutually exclusive with `agent`). #[serde(default)] pub program: Option, diff --git a/crates/rhei-cli/tests/e2e/mod.rs b/crates/rhei-cli/tests/e2e/mod.rs index 3fc80509..d842aa11 100644 --- a/crates/rhei-cli/tests/e2e/mod.rs +++ b/crates/rhei-cli/tests/e2e/mod.rs @@ -27,6 +27,7 @@ mod supervision_tests; mod template_example_sync_tests; mod templates_render_tests; mod templates_tests; +mod terminal_result_attempt_tests; mod terminal_result_fanout_tests; mod terminal_result_redirect_tests; mod terminal_result_stall_tests; diff --git a/crates/rhei-cli/tests/e2e/terminal_result_attempt_tests.rs b/crates/rhei-cli/tests/e2e/terminal_result_attempt_tests.rs new file mode 100644 index 00000000..b140a52a --- /dev/null +++ b/crates/rhei-cli/tests/e2e/terminal_result_attempt_tests.rs @@ -0,0 +1,508 @@ +//! Re-spawning a state that did not finish: which spawns are attempts at the +//! same visit, which are a fresh visit, and how many of the first kind a visit +//! gets before the run stops paying for them. +//! +//! The distinction is the whole subject. `visits:` bounds how many times a +//! ticket may *enter* a state; `attempts:` bounds how many times one entry may +//! be *spawned*. Conflating them is how a ticket that legitimately came back to +//! a state was narrated as a failed retry, and how a state that really was +//! stuck was re-spawned once per `rhei run`, forever. + +// §FS-rhei-agents.3.2.3 §FS-rhei-agents.8.1 §FS-rhei-agents.8.4 §FS-rhei-run.3 + +use std::fs; + +use super::terminal_result_tests::write_mock_agent_settings; +use super::*; + +/// Publishes the state's declared output, counts the spawn, and exits 0 without +/// touching `RHEI_RESULT_PATH` — the shape of issue #105. +const OUTPUT_WITHOUT_RESULT_AGENT: &str = r#"root = pathlib.Path(env('RHEI_ROOT')) +counter = root / ('attempts-' + env('RHEI_TASK_ID') + '.txt') +n = int(counter.read_text().strip()) + 1 if counter.exists() else 1 +write(counter, str(n)) +write(root / 'artifacts' / ('report-' + env('RHEI_TASK_ID') + '.md'), 'the report\n') +sys.stdout.write('ATTEMPT-{} OF-VISIT-{}\n'.format(env('RHEI_ATTEMPT'), env('RHEI_VISIT_COUNT'))) +"#; + +/// Writes the ticket's result every time, so every spawn finishes its state and +/// the only thing a second spawn can mean is a second *visit*. +const FINISHING_AGENT: &str = r#"root = pathlib.Path(env('RHEI_ROOT')) +counter = root / ('spawns-' + env('RHEI_STATE') + '.txt') +n = int(counter.read_text().strip()) + 1 if counter.exists() else 1 +write(counter, str(n)) +result('done in ' + env('RHEI_STATE') + '\n') +sys.stdout.write('RAN-{}-{}\n'.format(env('RHEI_STATE'), env('RHEI_ATTEMPT'))) +"#; + +const ONE_TICKET_PLAN: &str = r#"# Rhei: Attempts + +## Tasks + +### Task 1: Implement +**State:** implement +"#; + +fn result_only_missing_machine(extra_state_fields: &str) -> String { + format!( + r#"name: attempt-budget +version: 1 +states: + implement: + initial: true + description: Writes its declared output and never its result + agent: mock + agent_timeout: 20s + concurrent: true +{extra_state_fields} outputs: + - name: report + path: artifacts/report-{{task_id}}.md + completed: + final: true + description: Done +transitions: + - from: implement + to: completed +"# + ) +} + +fn setup(name: &str, plan: &str, machine: &str, agent_body: &str) -> (TestDir, PathBuf, PathBuf) { + let dir = unique_temp_dir(name); + let plan_path = write_fixture_file(&dir, "plan.rhei.md", plan); + let machine_path = write_fixture_file(&dir, "states.yaml", machine); + let agent = write_python_agent(&dir, "mock-agent.py", agent_body); + write_mock_agent_settings(&dir, &agent); + (dir, plan_path, machine_path) +} + +fn spawn_count(dir: &Path, name: &str) -> u32 { + fs::read_to_string(dir.join(name)).map(|raw| raw.trim().parse().unwrap_or(0)).unwrap_or(0) +} + +fn log_names(dir: &Path) -> Vec { + let mut names = fs::read_dir(dir.join("runtime/logs")) + .map(|entries| { + entries + .flatten() + .map(|entry| entry.file_name().to_string_lossy().into_owned()) + .collect::>() + }) + .unwrap_or_default(); + names.sort(); + names +} + +/// A budget is per *visit* and persisted with it, so it cannot be refreshed by +/// starting another `rhei run`. Before this, "one spawn per run" was unbounded +/// across runs, which is exactly how a cycling machine reached 903 transcripts. +// §FS-rhei-agents.3.2.3 §FS-rhei-run.3 +#[test] +fn a_visit_is_spawned_at_most_its_attempt_budget_across_separate_runs() { + let (dir, plan_path, machine_path) = setup( + "attempts-budget", + ONE_TICKET_PLAN, + &result_only_missing_machine(""), + OUTPUT_WITHOUT_RESULT_AGENT, + ); + + for _ in 0..4 { + let run = run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks"]); + assert!(!run.status.success(), "the ticket never finishes, so no run succeeds"); + } + + assert_eq!( + spawn_count(&dir, "attempts-plan.1.txt"), + 2, + "four runs, one visit, the built-in budget of two spawns" + ); + assert_task_state(&plan_path, &machine_path, "1", "implement"); + assert_eq!( + log_names(&dir), + vec!["task-plan.1-implement-attempt2.log", "task-plan.1-implement.log"], + "and two transcripts, not one per run" + ); +} + +/// The halt has to say what it spent and what is still owed, at the moment it +/// stops spawning — an operator who only learns at the end of the run that the +/// ticket never moved has no idea which of the two bounds applied. +// §FS-rhei-agents.3.2.3 +#[test] +fn an_exhausted_budget_halts_the_ticket_where_it_is_and_says_what_it_owes() { + let (_dir, plan_path, machine_path) = setup( + "attempts-halt", + ONE_TICKET_PLAN, + &result_only_missing_machine(""), + OUTPUT_WITHOUT_RESULT_AGENT, + ); + + for _ in 0..2 { + run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks"]); + } + let spent = run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks"]); + let combined = format!("{}{}", spent.stdout, spent.stderr); + + assert!( + combined + .contains("halting Task plan.1 in state 'implement': 2 attempts spent on this visit"), + "the halt names the ticket, the state, and the attempts; got:\n{combined}" + ); + assert!( + combined.contains("result ("), + "and the artifact the completion condition still owes; got:\n{combined}" + ); + assert!(!spent.status.success(), "a run that ends with the ticket unfinished exits non-zero"); + // No error edge, no timeout edge, no `cancelled`: the ticket keeps its + // state exactly as any other stall leaves it. §FS-rhei-run.3 + assert_task_state(&plan_path, &machine_path, "1", "implement"); +} + +/// The re-spawn line is the only place an operator sees the loop while it is +/// running, so it carries the attempt, the budget, and *why* the attempt before +/// it did not finish — not one canned rule for every ending. +// §FS-rhei-agents.3.2.1 +#[test] +fn a_respawn_names_the_attempt_the_budget_and_what_ended_the_previous_one() { + let (_dir, plan_path, machine_path) = setup( + "attempts-note", + ONE_TICKET_PLAN, + &result_only_missing_machine(""), + OUTPUT_WITHOUT_RESULT_AGENT, + ); + + run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks"]); + let second = run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks"]); + let combined = format!("{}{}", second.stdout, second.stderr); + + assert!( + combined.contains( + "Re-spawning Task plan.1 in state 'implement': attempt 2 of 2; the previous attempt \ + exited 0 without meeting this state's completion condition" + ), + "the note says which attempt, out of how many, and what happened; got:\n{combined}" + ); +} + +/// The halt line predicts what the run will do next, so it has to be wrong in +/// neither direction. While attempts remain it promises another pass; on the +/// run that *spends the last one* it must not, because no later pass will spawn +/// the state again — and the run after it says exactly that. Promising a retry +/// and then silently never making one is the defect this whole change exists to +/// remove, one message over. +// §FS-rhei-agents.3.2.1 §FS-rhei-agents.3.2.3 +#[test] +fn the_run_that_spends_the_last_attempt_does_not_promise_another() { + let (_dir, plan_path, machine_path) = setup( + "attempts-last-promise", + ONE_TICKET_PLAN, + &result_only_missing_machine(""), + OUTPUT_WITHOUT_RESULT_AGENT, + ); + let halt_line = |run: &CliRun| -> String { + format!("{}{}", run.stdout, run.stderr) + .lines() + .find(|line| line.contains("halting Task plan.1")) + .unwrap_or_default() + .to_string() + }; + let run = + || halt_line(&run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks"])); + + // Attempt 1 of 2: a later pass really does run the state again. + let first = run(); + assert!( + first.contains("a later pass runs the state again"), + "with an attempt left the promise is true and stays; got:\n{first}" + ); + + // Attempt 2 of 2: the budget is spent by the attempt that just finished. + let second = run(); + assert!( + !second.contains("a later pass runs the state again"), + "the last attempt must not promise a pass that will not spawn; got:\n{second}" + ); + assert!( + second.contains("2 attempts spent on this visit"), + "it says the budget is spent, at the moment it becomes true; got:\n{second}" + ); + + // And the pass that declines to spawn says the very same thing, because it + // is the very same fact. + assert_eq!(second, run(), "one sentence with two moments, not two that disagree"); +} + +/// The budget resolves through the same shape a timeout does: the state's own +/// field first, then `defaults.attempts`, then the built-in. +// §FS-rhei-agents.3.2.3 +#[test] +fn a_state_level_attempts_field_wins_over_the_settings_default() { + let (dir, plan_path, machine_path) = setup( + "attempts-resolution", + ONE_TICKET_PLAN, + &result_only_missing_machine(" attempts: 3\n"), + OUTPUT_WITHOUT_RESULT_AGENT, + ); + // A settings default of 1 would stop after the first spawn; the state's + // own `attempts: 3` is the more specific level and wins. + let settings = dir.join(".agents/rhei/settings.json"); + let raw = fs::read_to_string(&settings).expect("read settings"); + fs::write( + &settings, + raw.replace( + r#""defaults": { "agent": "mock""#, + r#""defaults": { "attempts": 1, "agent": "mock""#, + ), + ) + .expect("write settings"); + + for _ in 0..5 { + run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks"]); + } + + assert_eq!( + spawn_count(&dir, "attempts-plan.1.txt"), + 3, + "the state's own budget is the one spent" + ); +} + +/// The settings default applies where a state declares no `attempts:` of its +/// own — the second level of the chain, and the only one an operator can set +/// once for a whole project. +// §FS-rhei-agents.3.2.3 +#[test] +fn a_settings_default_supplies_the_budget_a_state_does_not_declare() { + let (dir, plan_path, machine_path) = setup( + "attempts-settings-default", + ONE_TICKET_PLAN, + &result_only_missing_machine(""), + OUTPUT_WITHOUT_RESULT_AGENT, + ); + let settings = dir.join(".agents/rhei/settings.json"); + let raw = fs::read_to_string(&settings).expect("read settings"); + fs::write( + &settings, + raw.replace( + r#""defaults": { "agent": "mock""#, + r#""defaults": { "attempts": 1, "agent": "mock""#, + ), + ) + .expect("write settings"); + + for _ in 0..3 { + run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks"]); + } + + assert_eq!( + spawn_count(&dir, "attempts-plan.1.txt"), + 1, + "one spawn, and no informed retry, because the project asked for none" + ); +} + +const CYCLE_PLAN: &str = r#"# Rhei: Re-entry + +## Tasks + +### Task 1: Loop +**State:** a +"#; + +/// `a → b → done`, and `done → a` so an operator can send the ticket round +/// again. Every state finishes its work, so a second spawn of `a` can only mean +/// a second *visit* — which is the case the engine used to read as a retry. +// §FS-rhei-agents.8.1 +const CYCLE_MACHINE: &str = r#"name: reentry +version: 1 +states: + a: + initial: true + description: First + agent: mock + agent_timeout: 20s + b: + description: Second + agent: mock + agent_timeout: 20s + done: + final: true + description: Done +transitions: + - from: a + to: b + - from: b + to: done + - from: done + to: a +"#; + +/// Entering a state again is a new visit, so it starts over: the plain log name, +/// no retry narration, and a fresh `attempts:` budget. This is the seam between +/// the two bounds — `visits:` ticks here, and an `attempts:` budget that did not +/// reset with it would make a ticket sent round the loop unrunnable on its +/// second lap. +// §FS-rhei-agents.3.2.3 §FS-rhei-agents.8.1 +#[test] +fn re_entering_a_state_is_a_new_visit_with_a_fresh_attempt_budget() { + let (dir, plan_path, machine_path) = + setup("attempts-reentry", CYCLE_PLAN, CYCLE_MACHINE, FINISHING_AGENT); + + let first = run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks"]); + assert_success(&first); + assert_task_state(&plan_path, &machine_path, "1", "done"); + + // Send it round again, the way an operator does: a hand transition out of + // the terminal state, and the finished ticket's result block goes with it. + assert_success(&run_transition(&plan_path, &machine_path, "1", "done", "a")); + let plan = fs::read_to_string(&plan_path).expect("read plan"); + fs::write( + &plan_path, + plan.lines() + .filter(|line| !line.starts_with("> **Result:**")) + .collect::>() + .join("\n"), + ) + .expect("write plan"); + + let second = run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks"]); + let combined = format!("{}{}", second.stdout, second.stderr); + assert_success(&second); + assert_task_state(&plan_path, &machine_path, "1", "done"); + + assert_eq!(spawn_count(&dir, "spawns-a.txt"), 2, "the second lap ran state 'a' again"); + assert!( + !combined.contains("Re-spawning"), + "a fresh entry is not a retry of the last one; got:\n{combined}" + ); + assert!( + !combined.contains("attempts spent"), + "and it did not arrive with the first lap's budget already gone; got:\n{combined}" + ); + assert_eq!( + log_names(&dir), + vec!["task-plan.1-a.log", "task-plan.1-b.log"], + "each visit writes the plain name; `-attempt` is for retries within one visit" + ); +} + +const TWO_TICKET_PLAN: &str = r#"# Rhei: Attempts In Parallel + +## Tasks + +### Task 1: Implement one +**State:** implement + +### Task 2: Implement two +**State:** implement +"#; + +/// The worker pool schedules through its own code path, and `--parallel` +/// defaults to 1, so every other test in this suite exercises the sequential +/// one. The completion condition, the attempt log, the retry narration and the +/// budget all have to hold in the pool too — they are one rule, and a rule with +/// one tested caller is a rule with one caller that works. +// §FS-rhei-agents.3.2 §FS-rhei-agents.3.2.3 §FS-rhei-run.5 +#[test] +fn the_worker_pool_respawns_and_bounds_the_same_way_the_sequential_path_does() { + let (dir, plan_path, machine_path) = setup( + "attempts-parallel", + TWO_TICKET_PLAN, + &result_only_missing_machine(""), + OUTPUT_WITHOUT_RESULT_AGENT, + ); + let args = ["--no-tui", "--no-callbacks", "--parallel", "2"]; + + let first = run_cli("run", &plan_path, &machine_path, &args); + assert!(!first.status.success(), "both tickets owe their result"); + assert_task_state(&plan_path, &machine_path, "1", "implement"); + assert_task_state(&plan_path, &machine_path, "2", "implement"); + + let second = run_cli("run", &plan_path, &machine_path, &args); + let combined = format!("{}{}", second.stdout, second.stderr); + for task in ["plan.1", "plan.2"] { + assert!( + combined + .contains(&format!("Re-spawning Task {task} in state 'implement': attempt 2 of 2")), + "the pool re-spawns {task} rather than advancing it; got:\n{combined}" + ); + } + + assert!( + !combined.contains("a later pass runs the state again"), + "and neither ticket is promised a pass the pool will not run; got:\n{combined}" + ); + + let third = run_cli("run", &plan_path, &machine_path, &args); + let combined = format!("{}{}", third.stdout, third.stderr); + for task in ["plan.1", "plan.2"] { + assert!( + combined + .contains(&format!("halting Task {task} in state 'implement': 2 attempts spent")), + "and the pool honours the same budget; got:\n{combined}" + ); + } + assert_eq!(spawn_count(&dir, "attempts-plan.1.txt"), 2); + assert_eq!(spawn_count(&dir, "attempts-plan.2.txt"), 2); +} + +/// A spawn that never started leaves a complete-looking log header behind — the +/// engine writes it before the subprocess exists. Crediting that log to a worker +/// is the mirror of the bug this whole change fixes, so the evidence is the +/// spawn record, which only a subprocess that ran can produce. +// §FS-rhei-agents.8.4 §FS-rhei-run.3 +#[test] +fn a_spawn_that_never_started_is_not_evidence_that_a_worker_ran() { + let dir = unique_temp_dir("attempts-unspawnable"); + let plan_path = write_fixture_file(&dir, "plan.rhei.md", ONE_TICKET_PLAN); + // No declared `outputs:`, so the `--no-agent` pass below can take the edge + // and reach the sentence under test. §FS-rhei-states.1.4 + let machine_path = write_fixture_file( + &dir, + "states.yaml", + r#"name: unspawnable +version: 1 +states: + implement: + initial: true + description: An agent command that does not exist + agent: mock + agent_timeout: 20s + completed: + final: true + description: Done +transitions: + - from: implement + to: completed +"#, + ); + let settings_dir = dir.join(".agents/rhei"); + fs::create_dir_all(&settings_dir).expect("create settings dir"); + fs::write( + settings_dir.join("settings.json"), + r#"{ + "defaults": { "agent": "mock", "agent_timeout": "10s" }, + "agents": { + "mock": { "command": ["rhei-no-such-binary-anywhere"], "timeout": "10s" } + } +}"#, + ) + .expect("write settings"); + + let failed = run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks"]); + assert!(!failed.status.success(), "the agent command does not exist"); + assert!( + dir.join("runtime/logs/task-plan.1-implement.log").exists(), + "the header was written before the spawn was attempted, which is the trap" + ); + + let advanced = + run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks", "--no-agent"]); + assert_success(&advanced); + let recorded = + fs::read_to_string(dir.join("runtime/results/plan.1.md")).expect("read result file"); + assert!( + recorded.contains("No agent or program ran in that state"), + "nothing ran, and the engine says so; got:\n{recorded}" + ); +} diff --git a/crates/rhei-cli/tests/e2e/terminal_result_stall_tests.rs b/crates/rhei-cli/tests/e2e/terminal_result_stall_tests.rs index b3aa1714..c6f7871b 100644 --- a/crates/rhei-cli/tests/e2e/terminal_result_stall_tests.rs +++ b/crates/rhei-cli/tests/e2e/terminal_result_stall_tests.rs @@ -277,3 +277,161 @@ if task != 'plan.2': assert_task_state(&plan_path, &machine_path, "2", "work"); assert_task_state(&plan_path, &machine_path, "3", "completed"); } + +const RESULT_ONLY_MISSING_PLAN: &str = r#"# Rhei: Result Only Missing + +## Tasks + +### Task 1: Implement +**State:** implement +"#; + +/// The shape of issue #105: the state declares an output, the agent writes it +/// and exits 0, and the edge out of the state is terminal — so the ticket's +/// result is the one artifact of the completion condition still owed. +// §FS-rhei-agents.3.2 §FS-rhei-states.3.3 +const RESULT_ONLY_MISSING_MACHINE: &str = r#"name: result-only-missing +version: 1 +states: + implement: + initial: true + description: Writes its declared output and never its result + agent: mock + agent_timeout: 20s + outputs: + - name: report + path: artifacts/report-{task_id}.md + completed: + final: true + description: Done +transitions: + - from: implement + to: completed +"#; + +/// Publishes the declared output, counts the attempt so the test can tell one +/// spawn from the next, and exits 0 without touching `RHEI_RESULT_PATH`. +const OUTPUT_WITHOUT_RESULT_AGENT: &str = r#"root = pathlib.Path(env('RHEI_ROOT')) +counter = root / 'attempts.txt' +n = int(counter.read_text().strip()) + 1 if counter.exists() else 1 +write(counter, str(n)) +write(root / 'artifacts' / ('report-' + env('RHEI_TASK_ID') + '.md'), 'the report\n') +sys.stdout.write('ATTEMPT-{}\n'.format(n)) +"#; + +fn setup_result_only_missing(name: &str) -> (TestDir, PathBuf, PathBuf) { + let dir = unique_temp_dir(name); + let plan_path = write_fixture_file(&dir, "plan.rhei.md", RESULT_ONLY_MISSING_PLAN); + let machine_path = write_fixture_file(&dir, "states.yaml", RESULT_ONLY_MISSING_MACHINE); + let agent = write_python_agent(&dir, "mock-agent.py", OUTPUT_WITHOUT_RESULT_AGENT); + write_mock_agent_settings(&dir, &agent); + (dir, plan_path, machine_path) +} + +/// A ticket that failed the completion condition on one pass was read on the +/// next as having nothing left to do — its declared outputs were on disk — and +/// was advanced into its terminal state carrying a result that said no agent had +/// run. The scheduler asks the whole condition now, so the recovery is the one +/// the execution loop prescribes: run the state again. +// §FS-rhei-agents.3.2 §FS-rhei-run.3 +#[test] +fn an_agent_owing_only_the_result_is_respawned_rather_than_advanced() { + let (dir, plan_path, machine_path) = setup_result_only_missing("terminal-result-only-missing"); + + let first = run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks"]); + assert!(!first.status.success(), "the first run halts on the result the agent owes"); + assert_task_state(&plan_path, &machine_path, "1", "implement"); + + let second = run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks"]); + let combined = format!("{}{}", second.stdout, second.stderr); + assert!( + combined.contains("Re-spawning Task plan.1 in state 'implement'"), + "the next pass runs the state again, and says why; got:\n{combined}" + ); + assert!(!second.status.success(), "a second silent attempt halts the same way"); + assert_task_state(&plan_path, &machine_path, "1", "implement"); + assert_eq!( + fs::read_to_string(dir.join("attempts.txt")).expect("attempt counter").trim(), + "2", + "the agent was spawned again rather than skipped" + ); + + // Asserting on the file's *contents* would pass against an empty string + // whatever the engine had said; the fact under test is that the engine wrote + // nothing for a ticket it did not finish. §FS-rhei-states.3.3 + let result_path = dir.join("runtime/results/plan.1.md"); + assert!( + !result_path.exists(), + "no transition fired, so no result was recorded; got:\n{}", + fs::read_to_string(&result_path).unwrap_or_default() + ); +} + +/// The re-spawn used to truncate the log of the attempt it was retrying, which +/// is the one file that says why that attempt did not finish. +/// +/// Asserting only that `-attempt2` appeared would not have caught the other +/// half of the same mistake: an `-attempt2` written for a spawn that is not a +/// retry at all. So the whole listing is checked — two spawns of one visit, +/// two transcripts, and no third name from anywhere else. +// §FS-rhei-agents.8.1 +#[test] +fn a_respawn_keeps_the_earlier_attempts_transcript() { + let (dir, plan_path, machine_path) = setup_result_only_missing("terminal-result-attempt-logs"); + + for _ in 0..2 { + run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks"]); + } + + let logs = dir.join("runtime/logs"); + let first = fs::read_to_string(logs.join("task-plan.1-implement.log")) + .expect("the first attempt keeps the unsuffixed name"); + let second = fs::read_to_string(logs.join("task-plan.1-implement-attempt2.log")) + .expect("the re-spawn writes its own attempt log"); + assert!(first.contains("ATTEMPT-1"), "attempt 1's transcript survives; got:\n{first}"); + assert!(second.contains("ATTEMPT-2"), "attempt 2 wrote its own file; got:\n{second}"); + + let mut names = fs::read_dir(&logs) + .expect("read logs") + .flatten() + .map(|entry| entry.file_name().to_string_lossy().into_owned()) + .collect::>(); + names.sort(); + assert_eq!( + names, + vec!["task-plan.1-implement-attempt2.log", "task-plan.1-implement.log"], + "two spawns of one visit leave two transcripts and nothing else" + ); +} + +/// `--no-agent` walks an edge with no worker spawned under it, but a worker may +/// well have run in that state on an earlier run. The engine's own account says +/// what the log proves rather than that no agent ran. +// §FS-rhei-run.3 §FS-rhei-agents.8.1 +#[test] +fn callback_only_advancement_names_an_agent_that_ran_earlier() { + let (dir, plan_path, machine_path) = setup_result_only_missing("terminal-result-no-agent-stub"); + + let first = run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks"]); + assert!(!first.status.success(), "the agent ran and left the result unwritten"); + + let advanced = + run_cli("run", &plan_path, &machine_path, &["--no-tui", "--no-callbacks", "--no-agent"]); + assert_success(&advanced); + assert_task_state(&plan_path, &machine_path, "1", "completed"); + + let recorded = + fs::read_to_string(dir.join("runtime/results/plan.1.md")).expect("read result file"); + assert!( + recorded.contains("agent 'mock' ran in that state earlier"), + "the account names the agent the log proves ran; got:\n{recorded}" + ); + assert!( + recorded.contains("task-plan.1-implement.log"), + "and where that agent's transcript is; got:\n{recorded}" + ); + assert!( + !recorded.contains("No agent or program ran"), + "which is the opposite of what it used to say; got:\n{recorded}" + ); +} diff --git a/docs/changelog.md b/docs/changelog.md index cca1e20e..50d258f5 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,6 +2,44 @@ ## Unreleased +- **`rhei run` asks the whole completion condition before a pass skips an agent + invocation.** The condition has three parts — exit `0`, the declared + `outputs:` on disk, and, when the edge the exit selects lands on a `final: + true` state, the ticket's non-empty terminal result — and it lived in two + places with two different rules. The post-exit check asked all three; the + scheduling filter asked only whether the declared outputs existed. So a ticket + that correctly failed the condition on one pass, with its outputs written and + its result never written, was read on the next as having nothing left to do: + it fell through to callback-only advancement, took the terminal edge the + condition had just refused, and had "No agent or program ran in that state, so + no worker result was recorded" written into its permanent result — of a state + where an agent had run for twelve minutes and published its export. + §FS-rhei-run.3 step 5 already said that no transition fires and the ticket + stays where it is, and that the engine never speaks for a worker that ran; the + rule is now asked in one place and a state that has not met it is run again + rather than reclassified as finished. The same weak filter on the parallel + refill path goes with it. Two things follow from running the state again. + First, the engine no longer *infers* that a worker ran, and that this is a + retry, from a log file's name and existence — an inference that credited a + header-only log from a spawn that never started, claimed a hyphenated sibling + state's log as its own, and narrated a re-entry as a retry because an uncounted + state's visit number is pinned at `1`. A record written when a subprocess ends, + keyed by the ticket's move count, answers both questions instead + (§FS-rhei-agents.8.4), and each attempt keeps its own transcript rather than + truncating the one that explains the previous miss. Second, a retry that + repeats the previous prompt byte-for-byte is only spend, so a re-spawned + invocation is told it is retrying and which artifact the previous attempt left + unwritten, and a new **`attempts:` budget** bounds a visit — the state's own + field, then `defaults.attempts`, then `2` — after which the ticket halts where + it is and the run names what it owes (§FS-rhei-agents.3.2.3). `visits:` bounds + how many times a ticket may *enter* a state; `attempts:` bounds how many times + one entry may be *spawned*. The budget rides the same record, so it holds + across separate `rhei run` invocations, and a genuine re-entry starts a fresh + one; poll states keep their own `poll.max_attempts`, and an interrupted spawn + takes an attempt log without spending budget. Without the budget the fix would + trade a false result for unbounded re-spawning, which is why the two land + together. §FS-rhei-agents.3.2 §FS-rhei-states.3.3 (PR #106) + - **`rhei list --ready` is the ready set, not a second opinion about it.** It re-derived readiness inline from four of the six conditions the scan applies — terminal state, gating state, `**Prior:**` satisfaction and the supervision diff --git a/docs/functional-spec/rhei-agents.spec.md b/docs/functional-spec/rhei-agents.spec.md index 22009880..3d24bc5b 100644 --- a/docs/functional-spec/rhei-agents.spec.md +++ b/docs/functional-spec/rhei-agents.spec.md @@ -144,6 +144,7 @@ key rather than replacing the whole file. | `agent_mode` | string or null | No | Default agent mode (named flag set) applied when a state does not set `agent_mode`. | | `agent_timeout` | string or null | No | Default autonomous agent timeout | | `program_timeout` | string or null | No | Default program timeout | +| `attempts` | integer or null | No | Default number of spawns one state visit may have before the run halts. See [Attempt Budget](#323-attempt-budget). | | `mcp_servers` | array | No | Default MCP server entries applied to every agent state. Entries are ids or inline definitions. See [MCP Servers](#114-mcp_servers). | | `skills` | array | No | Default skill entries applied to every agent state. Entries are ids or inline definitions. See [Skills](#115-skills). | @@ -767,6 +768,22 @@ per-invocation for the same reason condition (2) is: a fanned-out state's artifacts are keyed by invocation identity, and one worker's file must not excuse a sibling that wrote nothing. +The same condition is also what lets `rhei run` decide *not* to spawn. Before a +pass spawns an invocation of a state that declares `outputs:`, it asks whether +that invocation's work is already on disk, and it skips the spawn only when +conditions (2) **and** (3) both hold for that invocation — the declared +artifacts, and, on a terminal edge, its own result. There is one completion +condition, asked at two moments; a scheduling test that looked only at the +declared outputs would read an invocation that *failed* the condition on one +pass as having nothing left to do on the next, and the ticket would advance on +the strength of artifacts that never answered for it. + +A state that declares no `outputs:` is never skipped this way. It has no +artifact of its own that could stand as proof its work was done, and +`runtime/results/.md` cannot stand in for one: that file is shared with +every other state the ticket has passed through, so a result written earlier +would excuse a state that has not run at all. + This contract maps 1:1 onto the native headless mode of every supported agent. All six built-ins — `claude-code -p`, `codex exec`, `gemini --prompt --yolo`, `cursor-agent --print --force`, `kilo --auto --yolo`, and `pi -p` — run one @@ -799,12 +816,55 @@ Under `orchestrator` authority, `rhei run`: - If any is missing, the task stays in its current state and the engine logs `warning: agent exited 0 but required outputs are missing for task {id} in state '{state}': (), ()` — with - `program` in place of `agent` when a program state stalls the same way. No - transition fires. The same facts are recorded structurally, so the run - report classifies the ticket by the artifacts it owes rather than as a - generic stall (§FS-rhei-run-report.3.1). The ticket is not spawned again - within the pass and the run continues with the other claimable tickets - (§FS-rhei-run.3 step 5). + `program` in place of `agent` when a program state stalls the same way — + followed by a halt line that says what the run will do next. That line is + a **prediction**, so it is conditioned on the attempt budget of §3.2.3 and + not on the completion condition alone. While the visit has an attempt left + it reads `halting Task {id} in state '{state}': the completion condition is + not met, so no transition fires; a later pass runs the state again.` When + the attempt that just finished was the *last* one this visit gets, that + sentence would be false — no later pass will run the state again — and the + exhaustion line of §3.2.3 is printed in its place, word for word, at the + moment it becomes true rather than one run later. An operator must never + read a promise of a retry from the run that spent the last attempt and the + denial of one from the run after it. + No transition fires either way. The same facts are recorded structurally, + so the run report classifies the ticket by the artifacts it owes rather + than as a generic stall (§FS-rhei-run-report.3.1). The ticket is not + spawned again within the pass and the run continues with the other + claimable tickets (§FS-rhei-run.3 step 5). + - A later pass that reaches the same ticket spawns the state again, because + the invocation still fails the completion condition it is scheduled + against (§3.2). The spawn says so as it happens, beside the `Log:` line: + `Re-spawning Task {id} in state '{state}': attempt {n} of {budget}; the + previous attempt {ending} (previous log: {path})` — one line, naming the + attempt being spent, the budget it comes out of, and the transcript it is + retrying, so a re-spawn is not read as a state being started for the first + time. `{ending}` is read from that attempt's spawn record (§8.4) and says + what actually happened to it — `timed out after 30m`, `was interrupted by + a run shutdown`, `exited 3`, or `exited 0 without meeting this state's + completion condition`. The engine never reports one ending as another: a + run that was Ctrl-C'd and a state that stalled on its outputs are + different facts and a retry says which it is retrying. + - A retry is only worth spawning if it can do better than the attempt before + it, so a re-spawned invocation is told it is one. Its prompt names the + attempt number, what the previous attempt left unmet, and the result path + it did not write (§FS-rhei-memory.4.4). Without that the retry receives a + byte-identical prompt and repeats the attempt it is meant to recover from. + - The retry is bounded, per state visit, by the **attempt budget** of §3.2.3. + Spawn number `{budget} + 1` of a visit does not happen: the ticket stays + where it is, is not scheduled again for the rest of the run, and the engine + logs `halting Task {id} in state '{state}': {budget} attempts spent on this + visit and the completion condition is still unmet: (). The + ticket stays in '{state}'.` No transition fires — not an error transition, + not a timeout transition, and not a move to a terminal state. An exhausted + budget is a stall like any other stall of step 5 (§FS-rhei-run.3), and it + is reported as one; the engine never converts "I stopped paying for this" + into "this ticket failed". + This is one sentence with two moments, not two sentences: it is printed by + the attempt that spends the last of the budget, and again by any later pass + that reaches the ticket and declines to spawn it. Both say the same thing + because both are the same fact. - Each entry names the artifact and the **resolved** path that was checked, so a stale or mis-templated path is visible without re-deriving it. When a resolved path still contains an unresolved `{...}` template, the entry is @@ -828,6 +888,57 @@ agent that hangs without producing outputs is bounded by the timeout and routed to the state's timeout transition (or fails the task with a warning when no timeout transition is declared). +#### 3.2.3. Attempt Budget + +One state **visit** — the span between two consecutive moves of the ticket +(§8.1) — may be spawned a bounded number of times. This is not `visits:`: +`visits:` counts how many times a ticket may **enter** the state; `attempts:` +counts how many times a single entry may be **spawned** before the run halts. +Entering the state again is a new visit, and a new visit brings a fresh +`attempts:` budget. + +The budget can be set at two levels, resolved the way a timeout is (§7.1): + +1. **Per-state** — `attempts` field on a state definition: + ```yaml + states: + implement: + attempts: 3 + ``` + +2. **Defaults** — `defaults.attempts` in settings: + ```json + { "defaults": { "attempts": 3 } } + ``` + +Resolution: state-level > settings defaults > the built-in **2**. + +Two is the initial invocation plus one informed retry. It is deliberate. A +retry that is told nothing repeats its predecessor exactly and buys nothing, so +the one retry is worth spending only because §3.2.1 makes it an informed one; a +third attempt that has learned nothing new since the second is spend without a +mechanism for converging. A machine whose recovery genuinely needs more sets +`attempts:` and says so where a reader will find it. + +The budget is **persisted with the visit**, in the spawn record of §8.4, so it +holds across separate `rhei run` invocations. A budget that reset when the +operator ran `rhei run` again would bound nothing: the runaway case this exists +for is a state re-spawned once per run, forever. + +An invocation the **run itself interrupted** does not spend the budget. The +shutdown ended it, no transition fired, and the next `rhei run` re-executes it +(§FS-rhei-run.3.2); charging it would let two Ctrl-Cs halt a ticket that has not +yet had one attempt of its own. It still gets its own attempt log and its own +spawn record — a transcript cut short is worth as much as any other. + +A budget below `1` is raised to `1`: every visit gets at least the invocation +that makes it a visit. + +A **poll state** (§FS-rhei-states.2) is exempt from the budget. Re-spawning +without moving is what a poll state is for, and it already declares its own +bound in `poll.max_attempts`; a second bound over the same spawns would end the +loop earlier than the machine's author said it should. + ## 4. Environment Variables The agent subprocess inherits these environment variables, consistent with the @@ -843,6 +954,7 @@ callback environment: | `RHEI_TASK_ID_LOCAL` | Ticket id as written in its rhei file's heading (`1`) — matches what a script that edits or greps the plan file needs | | `RHEI_RESULT_PATH` | Absolute path to the result file **this invocation** must write: `$RHEI_ROOT/runtime/results/$RHEI_TASK_ID.md` normally, and `$RHEI_ROOT/runtime/results/$RHEI_TASK_ID/$RHEI_STATE/$RHEI_VISIT_COUNT/.md` for one invocation of a fanned-out state, where `` is the target slug or model id that keys the rest of that invocation's artifacts (§FS-rhei-states.3.3). Always set, for every state — a program has no prompt to read the path from, and deriving it from four other variables is a contract nobody can be held to. A task does not enter a `final: true` state until the ticket's result has content (§FS-rhei-states.3.3) | | `RHEI_STATE` | Current state name | +| `RHEI_ATTEMPT` | Which attempt of this state visit the invocation is, counting from 1. `2` and above mean the invocation is a retry of an attempt that did not finish, and the prompt's `## Previous Visits` says how that one ended (§FS-rhei-memory.3.3). Distinct from `RHEI_VISIT_COUNT`, which counts entries into the state and not spawns within one (§FS-rhei-agents.8.1) | | `RHEI_MODEL` | Model profile id, if configured | | `RHEI_MODEL_PROVIDER` | Resolved provider id, if configured | | `RHEI_MODEL_NAME` | Resolved provider model name, if configured | @@ -1195,6 +1307,28 @@ parsing the log body for billing facts. §FS-rhei-cost-accounting | Counted-loop state | `runtime/logs/task-{task_id}-{state}-{visit_count}.log` | | Model-specific state | `runtime/logs/task-{task_id}-{state}-{model}.log` | | Both visits and model | `runtime/logs/task-{task_id}-{state}-{model}-{visit_count}.log` | +| Retry within one visit | the name above with `-attempt{n}` appended, `n` counting from 2 | + +`{visit_count}` counts only where §FS-rhei-transitions.4.3 keeps a counter, so +it cannot separate one stay in a state from the next: an ordinary state in a +cycle is `{visit_count}` 1 on every entry. The attempt suffix is therefore keyed +to a **state visit**, which this specification defines as the span between two +consecutive moves of the ticket. The first spawn of a visit uses the unsuffixed +name above; each further spawn *within that same visit* is `-attempt2`, +`-attempt3`, and so on, and the first spawn after the ticket moves again starts +over at the unsuffixed name — a fresh entry into a state is not a retry of the +last one, and must not be named or narrated as one. Without the suffix a retry +would truncate the one file that says why the attempt before it did not finish, +which is the file both a human and the retrying agent need most. + +A visit's attempt number is read from the spawn record of §8.4, never inferred +from which log files happen to exist: a log is opened before its subprocess +starts, so its presence proves only that a spawn was attempted. + +Where something names "the log of a visit" — the `Previous log:` line of a +prompt (§FS-rhei-memory.4.4), the evidence behind an engine-written result +(§FS-rhei-run.3) — it means the log named by that visit's spawn record, which is +the last thing that actually ran there. ### 8.2. Log Format @@ -1248,6 +1382,41 @@ missing line means the state declared no entries of that kind. `runtime/logs/` is created automatically by `rhei run` if it does not exist. `rhei reset` removes the entire `runtime/` directory, including logs. +### 8.4. Spawn Records + +A log answers *what a worker said*. Two other questions are asked of a finished +spawn — *did a worker actually run in this state* and *is the next spawn a retry +of the same visit* — and neither can be answered from `runtime/logs/`. The log +file is created and its header written before the subprocess starts, so a +`command:` naming a binary that does not exist leaves a complete-looking header +behind; and the log's own name cannot say which visit it belongs to, because +`{visit_count}` does not count every visit (§8.1). + +So `rhei run` writes one **spawn record** per invocation, in +`runtime/spawns/`, named after the invocation exactly as its log is minus the +`-attempt{n}` suffix: `task-{task_id}-{state}{suffix}.json`. It is written when +the subprocess **ends**, never when it is merely opened, and it is rewritten in +place by each further attempt of the same invocation. It holds: + +| Field | Meaning | +|-------|---------| +| `task`, `state` | the invocation this record answers for, spelled out | +| `moves` | how many times the ticket had moved when this spawn started — the visit key of §8.1 | +| `attempt` | which attempt of that visit this was, counting from 1 | +| `kind`, `worker` | `agent` or `program`, and the resolved agent id or command | +| `log` | the transcript this spawn wrote | +| `started`, `ended`, `duration`, `code` | when it ran, for how long, and how it exited | +| `ending` | `exited`, `timed out`, or `interrupted` — why it stopped | + +`task` and `state` are stored as fields and matched as fields. A reader looking +for "a worker that ran in state `review`" must not match record *file names* by +prefix: state names share prefixes — `agent-review` and `agent-review-fix` ship +in one profile in §FS-rhei-states.8 — and a prefix match would attribute one +state's transcript, worker, and duration to its neighbour. + +`runtime/spawns/` is created on demand and, like `runtime/logs/`, is removed by +`rhei reset`. + ## 9. Dry-Run Output `rhei run --dry-run` in agent mode shows what would be spawned without executing: diff --git a/docs/functional-spec/rhei-memory.spec.md b/docs/functional-spec/rhei-memory.spec.md index 9d57c214..6ef938b2 100644 --- a/docs/functional-spec/rhei-memory.spec.md +++ b/docs/functional-spec/rhei-memory.spec.md @@ -215,6 +215,10 @@ Result entries so far: ``` Previous log: `runtime/logs/{log file of the previous visit of this state}` + +Retrying this visit: attempt {n}. The previous attempt {ending}. It did not +write `{result path}`, which a transition out of this state reads to finish +this task. Its transcript is `{previous attempt log}`. ``` - The trail is the state sequence of this task's ledger lines, in order — the @@ -230,6 +234,15 @@ Previous log: `runtime/logs/{log file of the previous visit of this state}` - `Previous log:` names the log file of the previous visit of this same state by the naming rule of §FS-rhei-agents.8.1, only if that file exists. The log is not pasted; it is a transcript, and the path is enough. +- The retry paragraph is rendered only when *this visit* has already been + spawned — when a spawn record for this invocation belongs to this visit + (§FS-rhei-agents.8.4). It exists because a re-spawn that is handed the same + prompt as the attempt it is recovering from will do the same thing again: the + invocation has to be told that it is a retry, what ended the last attempt, and + which file that attempt was obliged to write and did not. The result path is + named because naming it is the whole point — the built-in prompt already shows + it as where a finished task's result is *read from*, which agents read as + description rather than obligation. ### 3.4. `## Rhei Commands` Additions @@ -361,8 +374,21 @@ Given an invocation `I = (task, state, visit_count, identity)`: **last** 100 with the overflow line `… earlier entries omitted; read ` first. The legacy fallback of §4.3.3 applies here too, and `` names whichever file was read. -3. `prev_log` = the path of §FS-rhei-agents.8.1 for `(task, state, identity, - visit_count − 1)`; emit the `Previous log:` line only when that file exists. +3. `prev_log` = the log named by the spawn record (§FS-rhei-agents.8.4) of + `(task, state, identity, visit_count − 1)` — the last thing that actually + ran there, whichever attempt of that visit it was — falling back to that + visit's unsuffixed log file where no record answers, which is what a runtime + written before records existed has. Emit the `Previous log:` line only when + the named file is on disk. +4. `retry` = the spawn record of `(task, state, identity, visit_count)`, when + one exists **and** it belongs to this visit: its `moves` equals the number of + moves the ticket has made, i.e. the ticket has not left the state since that + spawn. Render the retry paragraph from it — `attempt` + 1 as `{n}`, its + `ending` and `code` as `{ending}`, its `log` as `{previous attempt log}` — + and name the result path this invocation is handed as `{result path}`, + omitting that clause on a state no terminal edge leaves. A record from an + earlier visit is not a retry and renders nothing: re-entering a state is a + fresh start, not a second attempt. ### 4.5. Fencing and Rendering diff --git a/docs/functional-spec/rhei-programs.spec.md b/docs/functional-spec/rhei-programs.spec.md index d815cc44..0a831c4c 100644 --- a/docs/functional-spec/rhei-programs.spec.md +++ b/docs/functional-spec/rhei-programs.spec.md @@ -80,6 +80,7 @@ Program subprocesses inherit the same base environment as agent subprocesses: | `RHEI_RESULT_PATH` | Absolute path to the ticket's result file, `runtime/results/.md` under the Rhei artifact root. Always set, and always the ticket-level file: `rhei run` spawns a program once per ticket, so a program state has one invocation and never writes the per-invocation result fragments a fanned-out agent state does, whatever else the state declares (§FS-rhei-states.3.3). A program whose exit routes the ticket into a `final: true` state must leave this file non-empty (§FS-rhei-states.3.3) | | `RHEI_STATE` | Current state name | | `RHEI_VISIT_COUNT` | Current visit number (for counted-loop states) | +| `RHEI_ATTEMPT` | Which attempt of this state visit the program is, counting from 1. A program has no prompt, so this is the only way it can tell a retry from a first run — and it re-spawns on an unmet completion condition exactly as an agent does (§FS-rhei-agents.3.2.1) | | `RHEI_INPUT__EXISTS` | `true` or `false` — whether the declared input artifact exists on disk. Set for every declared input, required or optional. `` is the artifact `name` uppercased with hyphens and spaces replaced by underscores (e.g., `continuation-notes` → `RHEI_INPUT_CONTINUATION_NOTES_EXISTS`). | | `RHEI_INPUT__PATH` | Resolved path of the declared input artifact. Set for every declared input regardless of whether the file exists. Same name transform as `RHEI_INPUT__EXISTS`. | @@ -215,6 +216,18 @@ Program stdout and stderr are captured using the same log format and naming conv |----------|---------------| | Simple state | `runtime/logs/task-{task_id}-{state}.log` | | Counted-loop state | `runtime/logs/task-{task_id}-{state}-{visit_count}.log` | +| Retry within one visit | the name above with `-attempt{n}` appended, `n` counting from 2 | + +A program state re-spawns for the same reason an agent state does — it is never +skipped at scheduling, and a pass that finds its completion condition unmet runs +it again — so the attempt rule of +[Agents Specification — Log File Naming](rhei-agents.spec.md#81-log-file-naming) +applies here in full: the attempt is keyed to the state visit, a fresh entry +into the state starts over at the unsuffixed name, and a retry never truncates +the transcript that says why the attempt before it did not finish. A program +spawn writes the same record as an agent spawn +(§FS-rhei-agents.8.4), with `kind` `program` and `worker` the resolved command, +and it spends the same per-visit attempt budget (§FS-rhei-agents.3.2.3). ### 5.2. Log Format diff --git a/docs/functional-spec/rhei-run.spec.md b/docs/functional-spec/rhei-run.spec.md index 49b7c84a..d7aea8b5 100644 --- a/docs/functional-spec/rhei-run.spec.md +++ b/docs/functional-spec/rhei-run.spec.md @@ -209,6 +209,29 @@ from silently completing fresh tasks without executing them. work is never a verdict on the tickets beside it. The run halts only when a pass makes no progress at all (step 9), and then exits non-zero with every stalled ticket named. + + The recovery is to **run the state again**, and only that. A later pass + that reaches the ticket — this run's, or the next `rhei run`'s — schedules + the same invocation, because step 3 skips an invocation only when the whole + completion condition already holds for it (§FS-rhei-agents.3.2). The engine + never advances the ticket instead: doing so would put it in a `final: true` + state with no account of the work, and the only sentence the engine could + write there would be about a worker it did not watch. + + Running the state again is bounded. One visit to a state — the span + between two consecutive moves of the ticket — may be spawned at most + `attempts` times (§FS-rhei-agents.3.2.3), a budget that is persisted with + the visit and therefore survives the end of a run: a fresh `rhei run` does + not buy the ticket a fresh allowance, because "once per run, forever" is + exactly the unbounded case the budget exists for. Entering the state again + is a new visit and does bring a fresh budget. When the budget is spent the + ticket stalls here, through this same path and no other: it stays in its + state, is out of the running for the rest of the run, is named in the halt + with the attempts it spent and the artifact it still owes, and the run's + exit code is the one step 9 gives any run that ends with tickets + unfinished. No transition fires on an exhausted budget — an error edge, a + timeout edge, or a move to `cancelled` would record a verdict on work the + engine never saw. 6. For agent invocations, extract measured usage and write the accounting invocation record when the resolved agent supports accounting. Accounting failures affect cost coverage but do not alter transition selection. §FS-rhei-cost-accounting @@ -230,6 +253,29 @@ from silently completing fresh tasks without executing them. [Complete Command — Result File](rhei-complete.spec.md#3-result-file). 9. Repeat until no pass makes progress. Exit `0` when the plan reaches a state where every task is terminal. Exit non-zero when progress halts with non-terminal tasks remaining and no further advancement is possible. + This is the **pass loop's** bound, and it is not the attempt budget of step + 5. The two answer different questions and neither substitutes for the other: + the budget bounds how many times one visit to a state may be spawned, across + runs; the pass loop bounds how long *this* run keeps trying, given what its + passes achieve. + + A ticket that stalled under step 5 is out of the running for the rest of that + pass. A pass that moved *something* — any ticket, any transition — does not + release those tickets; it records that the run has made progress since the + last release, and the pass loop goes on to the tickets that are still + claimable. A pass that ends with a ticket newly stalled and other claimable + tickets still untried also continues, since it has not yet asked everything + it could ask. + + The release happens at the one moment the run would otherwise stop: a pass + that moved nothing and has no untried ticket left. If some earlier pass had + made progress, every stalled ticket is released and given another turn, and + the run continues; if that turn moves nothing either, the run ends and names + every ticket still stalled. So the allowance is not one extra pass per run — + it renews every time the run makes progress — and it is not a bound on how + often one ticket may be re-spawned. That bound is the attempt budget above, + which is per state visit and outlives the run. + ### Who supplies the result on a terminal edge `rhei run` never invents one. Each route says who does: @@ -241,7 +287,7 @@ from silently completing fresh tasks without executing them. | Timeout (§FS-rhei-agents.7.3) | The engine, which knows the timeout that ended the work and writes it as the result message. | | Unavailable required tooling (§FS-rhei-agents.6) | The engine, which names the kind and the unavailable ids. | | Non-zero subprocess exit routed by `exit_code:` or an error transition | The engine, which names the exit code. | -| Callback-only advancement (`--no-agent`, or a machine with no autonomous state) | A callback that wrote the result file, if one did — otherwise the engine, which records that it took the edge itself and that **no worker result was recorded**. No subprocess ran in the source state, so there is nobody else who knows more than the engine does. | +| Callback-only advancement (`--no-agent`, or a machine with no autonomous state) | A callback that wrote the result file, if one did — otherwise the engine, which records that it took the edge itself and that **no worker result was recorded**. What it says about the worker is what it can prove: with a spawn record for the source state on disk (§FS-rhei-agents.8.4) the sentence names the worker that ran — `agent ''` or `program \`\`` — its log, and how it ended; only with no such record does it say that no worker ran. | | Human gate released from a live surface — browser dashboard (§FS-rhei-viz.5.1) or TUI (§FS-rhei-run-tui.1.5.5) | The human who released it, through the gate surface's own optional **Result** field. The message rides the transition like `rhei transition --result` does. Left blank with no result on disk, a release into a terminal state is refused, and the refusal names `rhei transition --from --to --result ""`. Releasing a gate into a non-terminal state is unaffected either way. | The line the table draws is one rule: **the engine writes a result only for the @@ -260,6 +306,32 @@ contradicted. Recording "no worker result was recorded" is the point — it is the fact the old, empty result file withheld, and the reason the audit trail used to depend on which verb drove the plan. +The clause about the worker is checked, not assumed, and what it is checked +against is the spawn record of §FS-rhei-agents.8.4 — never the presence of a log +file. A log is opened, and its header written, *before* the subprocess starts, +so a `command:` naming a binary that does not exist leaves a log behind for a +worker that never ran; recording that such a worker "ran in that state earlier" +would be the same class of lie as recording that none did. A spawn record is +written when a subprocess **ends**, so its presence is proof one ran. + +Where a record is found the recorded sentence names the worker — the agent id, +or the program's command — the log it wrote, how it ended, and that it wrote no +result: the fact the reader needs, and the opposite of what "no agent ran" would +have told them. The record is matched by its `task` and `state` fields, so a +state never inherits the account of a state whose name it is a prefix of. With +no record, the sentence says that no agent or program ran in that state, which +is then true. + +The account also says whether the state's declared `outputs:` were verified on +this edge — not asserted, reported: the check either ran and passed immediately +before the result was recorded, or it was waived because the edge lands on the +reserved `cancelled` state (§FS-rhei-states.1.4), and the sentence says which. +A state that declares no `outputs:` has nothing to report and the clause is +omitted. + +The accounting record is deliberately not the evidence: it is written only for +agents that support accounting (step 6), so a missing one proves nothing. + `rhei run` does not transition out of [gating states](rhei-states.spec.md#12-per-state-fields) — exiting one requires an explicit human-initiated `rhei transition` call. Gating states are a barrier, not an immediate global abort. If one task enters a diff --git a/docs/functional-spec/rhei-states.spec.md b/docs/functional-spec/rhei-states.spec.md index ac238770..7c00b83e 100644 --- a/docs/functional-spec/rhei-states.spec.md +++ b/docs/functional-spec/rhei-states.spec.md @@ -97,6 +97,7 @@ can start in different states within the same state machine. | `agent` | string | No | The coding agent CLI that executes work in this state. Must be an agent id resolved against the merged `agents` registry (built-ins → global → project `settings.json`). Inline agent objects are not permitted — define custom agents in the `agents` registry. See [Agents Specification](rhei-agents.spec.md). | | `agent_mode` | string | No | Named flag set applied to the resolved agent for this state. Must match a key in the resolved agent's `modes` map. See [Agents Specification — Modes](rhei-agents.spec.md#22-modes). | | `agent_timeout` | string | No | Maximum time an agent may work in this state before being killed (e.g., `30m`, `1h`). See [Agents Specification — Timeout Handling](rhei-agents.spec.md#7-timeout-handling). | +| `attempts` | integer | No | How many times **one visit** to this state may be spawned before `rhei run` halts the ticket. Distinct from `visits`, which bounds how many times the ticket may *enter* the state. Defaults to `2` — the invocation plus one informed retry. See [Agents Specification — Attempt Budget](rhei-agents.spec.md#323-attempt-budget). | | `program` | string or object | No | The program command to execute in this state. String form runs via shell. Object form specifies `command`, `env`, `working_directory`, and `shell`. Mutually exclusive with `agent`. See [Program States Specification](rhei-programs.spec.md). | | `program_timeout` | string | No | Maximum time the program may run before being killed (e.g., `10m`, `1h`). Same duration format and timeout handling as `agent_timeout`. See [Program States Specification](rhei-programs.spec.md#4-timeout-handling). | | `inputs` | artifact array | No | Artifacts that must exist before the task can enter this state. Individual entries may be marked `optional: true` to skip the existence check. | @@ -176,6 +177,7 @@ implicit rather than declared: see [Terminal Result](#33-terminal-result). - `state.agent` on a `gating: true` state is a validation warning (gating states are human-only; the agent will never be invoked by `rhei run`). - `state.agent_mode`, when present, must be a non-empty string and requires `state.agent` to be set. The mode name must match a key in the resolved agent's `modes` map, or the agent must declare no modes. See [Agents Specification — Mode Resolution Order](rhei-agents.spec.md#141-mode-resolution-order). - `state.agent_timeout`, when present, must be a valid duration string (e.g., `30s`, `5m`, `1h`, `2h30m`). +- `state.attempts`, when present, must be a positive integer. A value below `1` is raised to `1`: a visit always gets the invocation that makes it a visit. - A state must not declare both `agent` and `program`. - `state.program`, when present, must be a non-empty string or a valid program object with at least a `command` field. See [Program States Specification](rhei-programs.spec.md). - `state.program` on a `final: true` state is a validation error (terminal states have no work to execute).