diff --git a/crates/protocol/src/lib.rs b/crates/protocol/src/lib.rs index 07b70f03b5..bd3ee35d99 100644 --- a/crates/protocol/src/lib.rs +++ b/crates/protocol/src/lib.rs @@ -368,6 +368,17 @@ pub enum GoalPauseReason { NoProgress, UsageLimit, BudgetLimit, + /// The model finished a stage and handed control back, rather than a + /// reported blocker or a completion. It is not a judgement about the work, + /// so the user's next message resumes the goal (the same shape as a + /// runtime stop, which is also resumable by writing to it). + Yielded, + /// A reason a **newer** build wrote. Kept so an older binary can still read + /// the durable record instead of failing the whole goal load on an unknown + /// variant; nothing constructs it. The resume path treats it as "not a + /// hand-back", which is the conservative reading. + #[serde(other)] + Unrecognized, } impl GoalPauseReason { @@ -379,6 +390,8 @@ impl GoalPauseReason { Self::NoProgress => "no progress", Self::UsageLimit => "usage limit", Self::BudgetLimit => "budget limit", + Self::Yielded => "handed back", + Self::Unrecognized => "unrecognized", } } } diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index f279cf821f..6e380f25b7 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -5184,6 +5184,37 @@ impl Engine { true } + /// Resume a goal the model handed back at a milestone, when it is the + /// objective this turn names; publish the change like any other goal + /// transition. A yield is a hand-back rather than a judgement about the + /// work, so answering it continues the work. + async fn resume_yielded_goal(&mut self, objective: Option<&str>) -> bool { + let snapshot = match self.config.goal_state.lock() { + Ok(mut state) => { + if normalized_goal_objective(state.objective()) + != normalized_goal_objective(objective) + || !state.resume_after_yield() + { + return false; + } + state.snapshot() + } + Err(err) => { + tracing::warn!("goal state lock poisoned while resuming a yielded goal: {err}"); + return false; + } + }; + self.config.goal_status = GoalStatus::Active; + self.emit_session_updated().await; + let _ = self.send_event(Event::GoalUpdated { snapshot }).await; + let _ = self + .send_event(Event::status( + "Goal resumed: your message continues the work the earlier turn handed back", + )) + .await; + true + } + /// Pause a still-active goal with an inspectable reason and publish every /// host projection in one ordered path. async fn pause_goal_continuation(&mut self, reason: GoalPauseReason, message: String) { @@ -6074,15 +6105,25 @@ impl Engine { // A person writing to a goal that only the runtime stopped (a failed // or timed-out continuation) is continuing the work: resume it as a // new revision instead of running a goalless turn against a stale - // blocker. Blockers the model or user reported stay until an explicit + // blocker. A goal the model handed back at a milestone is the same + // shape — the model stopped for an answer, not because anything is + // wrong — so answering it continues the work too. Blockers the model + // reported and pauses the user asked for stay until an explicit // resume, and automated inputs never resume anything. - let goal_status = if !self.is_acp_turn() - && provenance == UserInputProvenance::ExternalUser - && goal_status == GoalStatus::Blocked - && self - .resume_runtime_blocked_goal(goal_objective.as_deref()) - .await + let resumed_goal = if !self.is_acp_turn() && provenance == UserInputProvenance::ExternalUser { + match goal_status { + GoalStatus::Blocked => { + self.resume_runtime_blocked_goal(goal_objective.as_deref()) + .await + } + GoalStatus::Paused => self.resume_yielded_goal(goal_objective.as_deref()).await, + _ => false, + } + } else { + false + }; + let goal_status = if resumed_goal { GoalStatus::Active } else { goal_status diff --git a/crates/tui/src/core/engine/handle.rs b/crates/tui/src/core/engine/handle.rs index d270b99d21..7e56778894 100644 --- a/crates/tui/src/core/engine/handle.rs +++ b/crates/tui/src/core/engine/handle.rs @@ -318,13 +318,16 @@ impl EngineHandle { ); } } else { - let (status, _) = - crate::tools::goal::thread_goal_status_projection(goal.status.clone()); + let (status, pause_reason) = crate::tools::goal::thread_goal_status_projection( + goal.status.clone(), + goal.pause_reason, + ); if status != crate::tools::goal::GoalStatus::Active { - state.sync_from_host_status( + state.sync_from_host_status_with_reason( current.objective.as_deref(), current.token_budget, status, + pause_reason, ); } } diff --git a/crates/tui/src/core/engine/tests/test_cases_12.rs b/crates/tui/src/core/engine/tests/test_cases_12.rs index 0028b5b4d4..1bf5dba8ee 100644 --- a/crates/tui/src/core/engine/tests/test_cases_12.rs +++ b/crates/tui/src/core/engine/tests/test_cases_12.rs @@ -1581,4 +1581,44 @@ fn turn_tool_context_uses_planned_authority_and_route_not_installed_session() { .model, "planned-next-model" ); +} + +/// Both continuation dispatchers test `is_active()` before re-arming. A goal the +/// model handed back must therefore land as `Inactive` at the gate the loop +/// actually consults — the unit tests pin `GoalState`, not this decision. +#[test] +fn a_yielded_goal_is_not_rearmed_by_the_continuation_gate() { + let (engine, _handle) = Engine::new(EngineConfig::default(), &Config::default()); + { + let mut state = engine.config.goal_state.lock().expect("goal lock"); + state.replace("ship the milestone", None, Some("goal-1".to_string())); + state.mark_yielded().expect("yield an active goal"); + } + + assert!( + matches!( + engine.goal_continuation_if_active(), + GoalContinuationAction::Inactive + ), + "a handed-back goal must not be re-armed by the cross-turn gate" + ); +} + +/// The same gate, against an active goal, still dispatches — so the test above +/// is measuring the yield and not a gate that is dead in test conditions. +#[test] +fn an_active_goal_is_still_rearmed_by_the_continuation_gate() { + let (engine, _handle) = Engine::new(EngineConfig::default(), &Config::default()); + { + let mut state = engine.config.goal_state.lock().expect("goal lock"); + state.replace("ship the milestone", None, Some("goal-1".to_string())); + } + + assert!( + matches!( + engine.goal_continuation_if_active(), + GoalContinuationAction::Dispatch { .. } + ), + "an active goal still continues; otherwise the yield test proves nothing" + ); } \ No newline at end of file diff --git a/crates/tui/src/prompts/text.rs b/crates/tui/src/prompts/text.rs index fea14ceb95..f80a37fd5f 100644 --- a/crates/tui/src/prompts/text.rs +++ b/crates/tui/src/prompts/text.rs @@ -186,8 +186,11 @@ Before deciding the goal is achieved, verify it against the actual current state — files, command output, tests, runtime behavior, issue or PR state, or other authoritative evidence — then call `update_goal` with `status: "complete"` and concise evidence. If something genuinely prevents -progress, call `update_goal` with `status: "blocked"` and explain it. If -`update_goal` is not in your tool list, load it with `tool_search` first. +progress, call `update_goal` with `status: "blocked"` and explain it. If you +finished a stage and the next step is the user's call, call `update_goal` with +`status: "yield"`, say what you need from them, and end your answer there; +their reply resumes the goal. If `update_goal` is not in your tool list, load +it with `tool_search` first. "#; /// Memory hygiene guidance — appended to the system prompt only when the /// session has a non-empty user-memory block. Steers the model toward diff --git a/crates/tui/src/runtime_threads.rs b/crates/tui/src/runtime_threads.rs index 8baac4c57b..f4805631a3 100644 --- a/crates/tui/src/runtime_threads.rs +++ b/crates/tui/src/runtime_threads.rs @@ -14538,7 +14538,11 @@ impl RuntimeThreadManager { let turn_goal_status = turn_goal .as_ref() .map(|goal| { - crate::tools::goal::thread_goal_status_projection(goal.status.clone()).0 + crate::tools::goal::thread_goal_status_projection( + goal.status.clone(), + goal.pause_reason, + ) + .0 }) .unwrap_or(crate::tools::goal::GoalStatus::Active); @@ -15271,9 +15275,11 @@ impl RuntimeThreadManager { ) } else { let snapshot = crate::tools::goal::GoalSnapshot::from_thread_goal(goal); - let status = - crate::tools::goal::thread_goal_status_projection(goal.status.clone()) - .0; + let status = crate::tools::goal::thread_goal_status_projection( + goal.status.clone(), + goal.pause_reason, + ) + .0; ( Some(objective.to_string()), snapshot.token_budget, diff --git a/crates/tui/src/tools/goal.rs b/crates/tui/src/tools/goal.rs index f676c848e9..6fafdb6869 100644 --- a/crates/tui/src/tools/goal.rs +++ b/crates/tui/src/tools/goal.rs @@ -159,6 +159,20 @@ impl GoalState { objective: Option<&str>, token_budget: Option, status: GoalStatus, + ) { + self.sync_from_host_status_with_reason(objective, token_budget, status, None); + } + + /// [`Self::sync_from_host_status`], for a host that knows **why** the goal + /// is paused. Without a reason a `Paused` projection is read as a user + /// pause, which erases a hand-back and leaves the goal un-resumable by the + /// user's next message. + pub fn sync_from_host_status_with_reason( + &mut self, + objective: Option<&str>, + token_budget: Option, + status: GoalStatus, + pause_reason: Option, ) { let objective = objective.map(str::trim).filter(|value| !value.is_empty()); match objective { @@ -206,7 +220,7 @@ impl GoalState { if changed || status_changed || self.status.is_none() { self.status = Some(status); self.pause_reason = if status == GoalStatus::Paused { - Some(GoalPauseReason::User) + Some(pause_reason.unwrap_or(GoalPauseReason::User)) } else { None }; @@ -507,6 +521,43 @@ impl GoalState { true } + /// Resume a goal the model handed back at a milestone, as a new control + /// revision. A yield is a hand-back rather than a judgement about the work, + /// so the user's next message continues it. + /// + /// Returns false, changing nothing, for any other state: a pause the user + /// asked for, and a pause the loop imposed on itself, both stay until an + /// explicit resume. + pub fn resume_after_yield(&mut self) -> bool { + if !(self.status == Some(GoalStatus::Paused) + && self.pause_reason == Some(GoalPauseReason::Yielded)) + { + return false; + } + self.resume(None); + true + } + + /// Hand the goal back to the user at a milestone. + /// + /// Only an **active** goal can be handed back. A goal that is already paused + /// or blocked carries a reason someone else set — a user pause, a budget + /// stop, a reported blocker — and converting it would make it resumable by + /// the next message, which is exactly the contract this state exists to keep + /// narrow. + pub fn mark_yielded(&mut self) -> Result<(), &'static str> { + if self.objective.is_none() { + return Err("No active goal exists to hand back."); + } + if self.status != Some(GoalStatus::Active) { + return Err( + "Only an active goal can be handed back; this one is already paused or \ + blocked for a reason someone else set.", + ); + } + self.mark_paused(GoalPauseReason::Yielded) + } + /// Whether a judged completion has sealed this goal. A sealed goal is /// terminal: blocking or pausing it would overwrite the verified /// completion, so only an explicit resume or a new goal moves it on. @@ -709,7 +760,8 @@ impl GoalSnapshot { #[must_use] pub fn from_thread_goal(goal: &codewhale_protocol::ThreadGoal) -> Self { - let (status, pause_reason) = thread_goal_status_projection(goal.status.clone()); + let (status, pause_reason) = + thread_goal_status_projection(goal.status.clone(), goal.pause_reason); Self { goal_id: Some(goal.goal_id.clone()), objective: Some(goal.objective.clone()), @@ -723,7 +775,7 @@ impl GoalSnapshot { elapsed_seconds: None, evidence: None, blocker: None, - pause_reason: goal.pause_reason.or(pause_reason), + pause_reason, completion_verification: None, advisories: Vec::new(), last_gap_fingerprint: goal.last_gap_fingerprint.clone(), @@ -737,12 +789,16 @@ impl GoalSnapshot { #[must_use] pub fn thread_goal_status_projection( status: codewhale_protocol::ThreadGoalStatus, + pause_reason: Option, ) -> (GoalStatus, Option) { match status { codewhale_protocol::ThreadGoalStatus::Active => (GoalStatus::Active, None), - codewhale_protocol::ThreadGoalStatus::Paused => { - (GoalStatus::Paused, Some(GoalPauseReason::User)) - } + // The durable status alone cannot say why a goal is paused; the record's + // own reason can, and dropping it turns a hand-back into a user pause. + codewhale_protocol::ThreadGoalStatus::Paused => ( + GoalStatus::Paused, + pause_reason.or(Some(GoalPauseReason::User)), + ), codewhale_protocol::ThreadGoalStatus::Complete => (GoalStatus::Complete, None), codewhale_protocol::ThreadGoalStatus::Blocked => (GoalStatus::Blocked, None), codewhale_protocol::ThreadGoalStatus::UsageLimited => { @@ -1044,7 +1100,7 @@ impl ToolSpec for UpdateGoalTool { } fn description(&self) -> &'static str { - "Update the runtime goal completion gate by calling this tool; a prose status in your answer does not change the goal or stop continuation. Critical verification may seal one immutable completion contract. Advisory review is append-only context and never completes, blocks, or pauses the goal. Mark blocked when progress requires user input." + "Update the runtime goal completion gate by calling this tool; a prose status in your answer does not change the goal or stop continuation. Critical verification may seal one immutable completion contract. Advisory review is append-only context and never completes, blocks, or pauses the goal. Mark blocked when progress requires user input. Mark yield when you finished a stage and the next step needs the user's decision: the goal stays unfinished, the turn ends, and their next message resumes it." } fn input_schema(&self) -> Value { @@ -1053,8 +1109,8 @@ impl ToolSpec for UpdateGoalTool { "properties": { "status": { "type": "string", - "enum": ["complete", "blocked", "not_achieved", "advisory"], - "description": "Use complete only when a critical verifier proves the goal; not_achieved to record verifier gaps; blocked when meaningful progress cannot continue; advisory to append best-effort context without changing lifecycle state." + "enum": ["complete", "blocked", "not_achieved", "advisory", "yield"], + "description": "Use complete only when a critical verifier proves the goal; not_achieved to record verifier gaps; blocked when meaningful progress cannot continue; yield when a stage is finished and the next step is the user's call; advisory to append best-effort context without changing lifecycle state." }, "evidence": { "type": "string", @@ -1211,6 +1267,9 @@ impl ToolSpec for UpdateGoalTool { state.record_progress(progress); } } + "yield" => { + state.mark_yielded().map_err(ToolError::invalid_input)?; + } "advisory" => { let advisory = input .get("advisory") @@ -1232,7 +1291,7 @@ impl ToolSpec for UpdateGoalTool { } other => { return Err(ToolError::invalid_input(format!( - "unsupported goal status '{other}'; update_goal can only mark complete or blocked, record not_achieved verifier gaps, or append advisory context" + "unsupported goal status '{other}'; update_goal can only mark complete, blocked, or yield, record not_achieved verifier gaps, or append advisory context" ))); } } @@ -1248,6 +1307,118 @@ mod tests { use super::*; + /// A yield is a hand-back, not a judgement about the work: it stops the + /// auto-continuation, and the user's next message resumes it. A pause the + /// user asked for stays put. + #[test] + fn a_yield_stops_continuation_and_the_users_message_resumes_it() { + let mut state = GoalState::default(); + state.replace("ship the slice", None, Some("goal-1".to_string())); + assert!(state.is_active()); + + state.mark_yielded().expect("yield an active goal"); + assert_eq!(state.snapshot().status, GoalStatus::Paused.as_str()); + assert_eq!( + state.snapshot().pause_reason, + Some(GoalPauseReason::Yielded), + "the pause names the hand-back so the UI can say what it is" + ); + assert!( + !state.is_active(), + "neither continuation dispatcher re-arms a non-active goal" + ); + + assert!( + state.resume_after_yield(), + "the user's next message continues the work" + ); + assert!(state.is_active()); + assert_eq!(state.snapshot().pause_reason, None); + } + + #[test] + fn a_user_pause_is_not_resumed_by_a_yield_resume() { + let mut state = GoalState::default(); + state.replace("ship the slice", None, Some("goal-1".to_string())); + state + .mark_paused(GoalPauseReason::User) + .expect("user pause"); + + assert!( + !state.resume_after_yield(), + "only a hand-back is resumed by answering it" + ); + assert_eq!(state.snapshot().status, GoalStatus::Paused.as_str()); + } + + #[test] + fn a_yield_cannot_overwrite_a_pause_reason_someone_else_set() { + let mut state = GoalState::default(); + state.replace("ship the slice", None, Some("goal-1".to_string())); + + for reason in [ + GoalPauseReason::User, + GoalPauseReason::BudgetLimit, + GoalPauseReason::NoProgress, + GoalPauseReason::UsageLimit, + ] { + state.mark_paused(reason).expect("pause for another reason"); + assert!( + state.mark_yielded().is_err(), + "only an active goal can be handed back, not one paused for {reason:?}" + ); + assert_eq!( + state.snapshot().pause_reason, + Some(reason), + "the pause someone else set survives the rejected hand-back" + ); + } + + // And a blocked goal keeps its blocker rather than becoming resumable. + state + .mark_blocked("waiting on the vendor".to_string()) + .expect("block"); + assert!(state.mark_yielded().is_err()); + assert_eq!( + state.snapshot().blocker.as_deref(), + Some("waiting on the vendor") + ); + } + + #[test] + fn a_pause_reason_written_by_a_newer_build_still_reads() { + let reason: GoalPauseReason = serde_json::from_str("\"invented-later\"") + .expect("an unknown pause reason must not fail the durable load"); + assert_eq!(reason, GoalPauseReason::Unrecognized); + // And the known values still round-trip. + assert_eq!( + serde_json::from_str::("\"yielded\"").expect("yielded reads"), + GoalPauseReason::Yielded + ); + } + + #[tokio::test] + async fn update_goal_yield_pauses_without_completing_or_blocking() { + let state = new_shared_goal_state(); + let ctx = ToolContext::new("."); + CreateGoalTool::new(state.clone()) + .execute(json!({"objective": "ship the runtime slice"}), &ctx) + .await + .expect("create goal"); + + let result = UpdateGoalTool::new(state.clone()) + .execute(json!({"status": "yield"}), &ctx) + .await + .expect("yield is a supported status"); + assert!(result.success, "yield must not be refused"); + + let snapshot = state.lock().expect("goal state").snapshot(); + assert_eq!(snapshot.status, GoalStatus::Paused.as_str()); + assert_eq!(snapshot.pause_reason, Some(GoalPauseReason::Yielded)); + assert_eq!(snapshot.blocker, None, "a hand-back reports no blocker"); + assert_eq!(snapshot.evidence, None, "and claims no completion"); + } + #[tokio::test] async fn update_goal_rejects_objective_knob_instead_of_ignoring_it() { // #5123-class: `objective` used to return a success receipt with no @@ -1909,7 +2080,10 @@ mod tests { .await .expect_err("model resume should fail"); - assert!(err.to_string().contains("complete or blocked")); + assert!( + err.to_string().contains("complete, blocked, or yield"), + "model resume stays rejected: {err}" + ); } #[test] @@ -2028,7 +2202,7 @@ mod tests { GoalPauseReason::BudgetLimit, ), ] { - let (projected, projected_reason) = thread_goal_status_projection(status); + let (projected, projected_reason) = thread_goal_status_projection(status, None); assert_eq!(projected, GoalStatus::Paused); assert_eq!(projected_reason, Some(reason)); }