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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 51 additions & 17 deletions crates/rhei-cli/src/cli/agent_spawn_records.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,18 +155,31 @@ struct SpawnPlan {
}

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 budget spent, if this visit's is already gone and the spawn must
/// not happen at all. A poll state's own bound never spends here — its
/// exhaustion is `poll.max_attempts`, checked elsewhere. §FS-rhei-agents.3.2.3
fn budget_spent(&self, budget: AttemptBudget) -> Option<u64> {
match budget {
AttemptBudget::Visit(budget) if self.charged >= budget => Some(budget),
AttemptBudget::Visit(_) | AttemptBudget::Poll { .. } => None,
}
}

/// 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<String> {
fn respawn_note(
&self,
task_id: &str,
state_name: &str,
budget: AttemptBudget,
) -> Option<String> {
let previous = self.previous.as_ref()?;
let budget = match budget {
AttemptBudget::Visit(budget) => budget.to_string(),
AttemptBudget::Poll { max_attempts } => format!("{max_attempts} (poll.max_attempts)"),
};
Some(format!(
" Re-spawning Task {task_id} in state '{state_name}': attempt {} of {budget}; \
the previous attempt {} (previous log: {}).",
Expand All @@ -181,9 +194,13 @@ impl SpawnPlan {
/// 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.
/// charge this predicts is the charge that happens. A poll state's own
/// exhaustion rule is its only bound, so it always has attempts left here.
// §FS-rhei-agents.3.2.1 §FS-rhei-agents.3.2.3
fn retry_outlook(&self, budget: u64) -> RetryOutlook {
fn retry_outlook(&self, budget: AttemptBudget) -> RetryOutlook {
let AttemptBudget::Visit(budget) = budget else {
return RetryOutlook::AttemptsLeft;
};
if self.charged.saturating_add(1) < budget {
RetryOutlook::AttemptsLeft
} else {
Expand Down Expand Up @@ -362,6 +379,21 @@ fn newest_spawn_record_for_state(
newest.map(|(_, record)| record)
}

/// Which rule bounds one visit's spawns, and by how much. `Poll` is carried
/// only to be named in the respawn note, never as a second bound alongside
/// the poll's own exhaustion rule.
// §FS-rhei-agents.3.2.1 §FS-rhei-agents.3.2.3
#[derive(Clone, Copy)]
enum AttemptBudget {
/// The state's own `attempts:`, `defaults.attempts`, or the built-in.
/// `SpawnPlan::budget_spent` and `SpawnPlan::retry_outlook` bound spawns
/// at this count.
Visit(u64),
/// A poll state's `poll.max_attempts`. Never bounds a spawn — the poll's
/// own exhaustion path (`ready_auto_advance.rs`) does that.
Poll { max_attempts: u64 },
}

/// How many spawns one visit to this state may have.
///
/// The chain a timeout resolves through, one level shorter because a budget has
Expand All @@ -373,20 +405,22 @@ fn newest_spawn_record_for_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
// §FS-rhei-agents.3.2.1 §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;
) -> AttemptBudget {
if let Some(max_attempts) = state_def.and_then(|def| def.poll.as_ref()) {
return AttemptBudget::Poll { max_attempts: u64::from(max_attempts.max_attempts) };
}
state_def
.and_then(|def| def.attempts)
.or(settings.defaults.attempts)
.map(u64::from)
.unwrap_or(DEFAULT_ATTEMPT_BUDGET)
.max(1)
AttemptBudget::Visit(
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:
Expand Down
4 changes: 2 additions & 2 deletions crates/rhei-cli/src/cli/run_agent_sequential.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fn run_sequential_agent_invocation(
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) {
if let Some(spent_budget) = 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
Expand All @@ -146,7 +146,7 @@ fn run_sequential_agent_invocation(
budget_spent_halt_line(
task_id_str,
current_state,
budget,
spent_budget,
&completion_debt_label(&owed)
)
);
Expand Down
4 changes: 2 additions & 2 deletions crates/rhei-cli/src/cli/run_parallel_program_spawn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ fn spawn_parallel_program_work_item(
);
let budget =
resolve_attempt_budget(machine.states.get(item.current_state.as_str()), settings);
if plan.budget_spent(budget) {
if let Some(spent_budget) = plan.budget_spent(budget) {
let owed = collect_missing_required_outputs(
workspace_root,
&task_workspace_root,
Expand All @@ -63,7 +63,7 @@ fn spawn_parallel_program_work_item(
budget_spent_halt_line(
&item.task_id_str,
&item.current_state,
budget,
spent_budget,
&completion_debt_label(&owed),
),
);
Expand Down
4 changes: 2 additions & 2 deletions crates/rhei-cli/src/cli/run_parallel_spawn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ fn spawn_parallel_agent_work_item(
);
let budget =
resolve_attempt_budget(machine.states.get(item.current_state.as_str()), settings);
if plan.budget_spent(budget) {
if let Some(spent_budget) = 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
Expand All @@ -93,7 +93,7 @@ fn spawn_parallel_agent_work_item(
budget_spent_halt_line(
&item.task_id_str,
&item.current_state,
budget,
spent_budget,
&completion_debt_label(&owed),
),
);
Expand Down
4 changes: 2 additions & 2 deletions crates/rhei-cli/src/cli/run_program_sequential.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ fn run_sequential_program_work_items(
None,
);
let budget = resolve_attempt_budget(machine.states.get(current_state.as_str()), settings);
if plan.budget_spent(budget) {
if let Some(spent_budget) = plan.budget_spent(budget) {
let owed = collect_missing_required_outputs(
workspace_root,
&task_workspace_root,
Expand All @@ -94,7 +94,7 @@ fn run_sequential_program_work_items(
budget_spent_halt_line(
task_id_str,
current_state,
budget,
spent_budget,
&completion_debt_label(&owed)
)
);
Expand Down
26 changes: 20 additions & 6 deletions crates/rhei-cli/src/cli/tests_spawn_records.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,9 @@ mod spawn_records {
ended(&first, "exited", 0);
let second = plan_for(dir.path());
ended(&second, "exited", 0);
assert!(
plan_for(dir.path()).budget_spent(2),
assert_eq!(
plan_for(dir.path()).budget_spent(AttemptBudget::Visit(2)),
Some(2),
"two recorded attempts spend a budget of two"
);

Expand All @@ -82,7 +83,10 @@ mod spawn_records {
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");
assert!(
after.budget_spent(AttemptBudget::Visit(2)).is_none(),
"the budget came back with the visit"
);
}

/// An interrupted invocation keeps its transcript — it ran — but the run
Expand All @@ -98,15 +102,25 @@ mod spawn_records {
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");
assert!(
second.budget_spent(AttemptBudget::Visit(1)).is_none(),
"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");
assert!(
third.budget_spent(AttemptBudget::Visit(1)).is_none(),
"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.budget_spent(AttemptBudget::Visit(1)),
Some(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")
Expand Down
1 change: 1 addition & 0 deletions crates/rhei-cli/tests/integration_markdown_plans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ include!("integration_markdown_plans/callbacks_execution.rs");
include!("integration_markdown_plans/callbacks_redirect_context.rs");
include!("integration_markdown_plans/run_basic.rs");
include!("integration_markdown_plans/run_programs_callbacks.rs");
include!("integration_markdown_plans/run_poll_respawn_budget.rs");
include!("integration_markdown_plans/run_agent_regressions.rs");
include!("integration_markdown_plans/release.rs");
include!("integration_markdown_plans/reset.rs");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// §FS-rhei-agents.3.2.1: a poll state's re-spawn note names `poll.max_attempts`,
// not the internal sentinel that marks it exempt from the visit budget.

#[test]
fn run_poll_respawn_note_names_poll_max_attempts_not_the_sentinel() {
let dir = unique_temp_dir("run-poll-respawn-budget");
let script = write_python_agent(
&dir,
"poll.py",
r#"append(pathlib.Path('runtime') / 'attempts.txt', 'attempt\n')
sys.exit(75)
"#,
);
let machine = format!(
r#"name: run-poll-respawn-budget-test
version: 1
states:
waiting:
description: Poll until attempts are exhausted
program:
command: {command}
poll:
interval: 0s
max_attempts: 3
exhausted:
description: Polling exhausted
final: true
transitions:
- from: waiting
to: waiting
exit_code: 75
- from: waiting
to: exhausted
exit_code: 75
"#,
command = fixture_command(&script)
);
let plan = r#"# Rhei: Poll Respawn Budget

## Tasks

### Task 1: Wait for external status
**State:** waiting
"#;

let plan_path = write_fixture_file(&dir, "plan.rhei.md", plan);
let machine_path = write_fixture_file(&dir, "states.yaml", &machine);

let result = run_run_command(&plan_path, &machine_path, &["--no-callbacks"]);
assert!(
result.status.success(),
"poll run should route to exhaustion after three attempts\nstdout:\n{}\nstderr:\n{}",
result.stdout,
result.stderr
);
assert!(
result.stdout.contains("attempt 2 of 3 (poll.max_attempts)"),
"respawn note should name poll.max_attempts\nstdout:\n{}",
result.stdout
);
assert!(
result.stdout.contains("attempt 3 of 3 (poll.max_attempts)"),
"respawn note should name poll.max_attempts\nstdout:\n{}",
result.stdout
);
assert!(
!result.stdout.contains("18446744073709551615"),
"respawn note must never print the internal exempt sentinel\nstdout:\n{}",
result.stdout
);
}
7 changes: 7 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

## Unreleased

- **The re-spawn note on a poll state names its own `poll.max_attempts`
instead of an internal sentinel.** A poll state is exempt from the visit
attempt budget — `poll.max_attempts` already bounds it — and that exemption
was encoded internally as `u64::MAX`, which `rhei run` then printed verbatim:
`attempt 4 of 18446744073709551615`. The exemption was correct; only the
rendering was wrong. The note now reads `attempt 4 of 96 (poll.max_attempts)`
for a poll state, and is unchanged for every other state. (PR #119)
- **The root `CHANGELOG.md` no longer claims the release maintains it.** It
opened by saying this file's `Unreleased` section is what "release automation
promotes into a numbered section here at release time", citing
Expand Down
11 changes: 9 additions & 2 deletions docs/functional-spec/rhei-agents.spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,12 @@ Under `orchestrator` authority, `rhei run`:
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
time. On a poll state (§FS-rhei-states.2) the budget named is the state's
own `poll.max_attempts`, and the line says which mechanism it is:
`attempt {n} of {max_attempts} (poll.max_attempts)` — that is the bound
that actually applies, since §3.2.3 exempts a poll state from `attempts:`.
The engine never prints its internal "no budget applies" sentinel as a
count. `{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
Expand Down Expand Up @@ -937,7 +942,9 @@ 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.
loop earlier than the machine's author said it should. The re-spawn note of
§3.2.1 still names a budget on a poll state — `poll.max_attempts` itself, not
this exemption's internal encoding of "no budget applies here".

## 4. Environment Variables

Expand Down