diff --git a/crates/rhei-cli/src/cli/agent_spawn_records.rs b/crates/rhei-cli/src/cli/agent_spawn_records.rs index e9059c01..fac7d568 100644 --- a/crates/rhei-cli/src/cli/agent_spawn_records.rs +++ b/crates/rhei-cli/src/cli/agent_spawn_records.rs @@ -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 { + 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 { + fn respawn_note( + &self, + task_id: &str, + state_name: &str, + budget: AttemptBudget, + ) -> Option { 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: {}).", @@ -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 { @@ -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 @@ -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: diff --git a/crates/rhei-cli/src/cli/run_agent_sequential.rs b/crates/rhei-cli/src/cli/run_agent_sequential.rs index c914ffcb..859ff3a3 100644 --- a/crates/rhei-cli/src/cli/run_agent_sequential.rs +++ b/crates/rhei-cli/src/cli/run_agent_sequential.rs @@ -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 @@ -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) ) ); diff --git a/crates/rhei-cli/src/cli/run_parallel_program_spawn.rs b/crates/rhei-cli/src/cli/run_parallel_program_spawn.rs index e8a6743a..c8645342 100644 --- a/crates/rhei-cli/src/cli/run_parallel_program_spawn.rs +++ b/crates/rhei-cli/src/cli/run_parallel_program_spawn.rs @@ -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, @@ -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), ), ); diff --git a/crates/rhei-cli/src/cli/run_parallel_spawn.rs b/crates/rhei-cli/src/cli/run_parallel_spawn.rs index 75a2d9a1..f3a9e40f 100644 --- a/crates/rhei-cli/src/cli/run_parallel_spawn.rs +++ b/crates/rhei-cli/src/cli/run_parallel_spawn.rs @@ -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 @@ -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), ), ); diff --git a/crates/rhei-cli/src/cli/run_program_sequential.rs b/crates/rhei-cli/src/cli/run_program_sequential.rs index f0b28ea0..24909950 100644 --- a/crates/rhei-cli/src/cli/run_program_sequential.rs +++ b/crates/rhei-cli/src/cli/run_program_sequential.rs @@ -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, @@ -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) ) ); diff --git a/crates/rhei-cli/src/cli/tests_spawn_records.rs b/crates/rhei-cli/src/cli/tests_spawn_records.rs index 82fe604f..e2c3d5a4 100644 --- a/crates/rhei-cli/src/cli/tests_spawn_records.rs +++ b/crates/rhei-cli/src/cli/tests_spawn_records.rs @@ -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" ); @@ -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 @@ -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") diff --git a/crates/rhei-cli/tests/integration_markdown_plans.rs b/crates/rhei-cli/tests/integration_markdown_plans.rs index d4ce96af..ef87de68 100644 --- a/crates/rhei-cli/tests/integration_markdown_plans.rs +++ b/crates/rhei-cli/tests/integration_markdown_plans.rs @@ -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"); diff --git a/crates/rhei-cli/tests/integration_markdown_plans/run_poll_respawn_budget.rs b/crates/rhei-cli/tests/integration_markdown_plans/run_poll_respawn_budget.rs new file mode 100644 index 00000000..87750311 --- /dev/null +++ b/crates/rhei-cli/tests/integration_markdown_plans/run_poll_respawn_budget.rs @@ -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 + ); +} diff --git a/docs/changelog.md b/docs/changelog.md index 793decb9..8b571377 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -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 diff --git a/docs/functional-spec/rhei-agents.spec.md b/docs/functional-spec/rhei-agents.spec.md index 3d24bc5b..a0245b07 100644 --- a/docs/functional-spec/rhei-agents.spec.md +++ b/docs/functional-spec/rhei-agents.spec.md @@ -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 @@ -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