diff --git a/apps/decodex/src/agent/app_server.rs b/apps/decodex/src/agent/app_server.rs index ee60a6140..7a6c2d82c 100644 --- a/apps/decodex/src/agent/app_server.rs +++ b/apps/decodex/src/agent/app_server.rs @@ -238,6 +238,13 @@ impl RequestWaitPhase { Self::TurnExecution => "turn execution", } } + + fn transport_failure_is_retryable_startup(self) -> bool { + matches!( + self, + Self::Initialize | Self::AccountLogin | Self::ThreadStart | Self::ThreadResume + ) + } } #[derive(Clone, Debug, Eq, PartialEq)] @@ -265,10 +272,24 @@ impl AppServerPhaseGoalFailure { Self { kind: AppServerPhaseGoalFailureKind::Unsupported { method } } } + #[cfg(test)] + pub(crate) fn unsupported_for_test(method: &'static str) -> Self { + Self::unsupported(method) + } + fn missing_terminal_path(phase: PhaseGoalKind) -> Self { Self { kind: AppServerPhaseGoalFailureKind::MissingTerminalPath { phase } } } + #[cfg(test)] + pub(crate) fn missing_terminal_path_for_test(phase: PhaseGoalKind) -> Self { + Self::missing_terminal_path(phase) + } + + pub(crate) fn is_terminal_path_missing(&self) -> bool { + matches!(self.kind, AppServerPhaseGoalFailureKind::MissingTerminalPath { .. }) + } + pub(crate) fn error_class(&self) -> &'static str { match self.kind { AppServerPhaseGoalFailureKind::Unsupported { .. } => @@ -278,6 +299,18 @@ impl AppServerPhaseGoalFailure { } } + pub(crate) fn retry_next_action(&self) -> String { + match self.kind { + AppServerPhaseGoalFailureKind::Unsupported { method } => format!( + "select or upgrade to a Codex app-server that supports required phase-goal method `{method}`" + ), + AppServerPhaseGoalFailureKind::MissingTerminalPath { phase } => format!( + "decodex will retry `{}` terminal-path recovery automatically; the next attempt must run the required review, handoff, closeout, or manual-attention terminal tool instead of treating phase-goal completion as issue completion", + phase.as_str() + ), + } + } + pub(crate) fn terminal_next_action(&self, recovery_gate: &str) -> String { match self.kind { AppServerPhaseGoalFailureKind::Unsupported { method } => format!( @@ -464,6 +497,35 @@ impl AppServerCapabilityPreflightFailure { } } + pub(crate) fn is_retryable_timeout(&self) -> bool { + matches!( + self.kind, + AppServerCapabilityPreflightFailureKind::MethodFailed { timed_out: true, .. } + ) + } + + pub(crate) fn retry_next_action(&self) -> String { + match &self.kind { + AppServerCapabilityPreflightFailureKind::MethodFailed { + method: "plugin/list", + timed_out: true, + .. + } => String::from( + "decodex will retry app-server preflight automatically; inspect local app_server_preflight_failed evidence for the `plugin/list` timeout and restart `decodex serve` if the retry budget exhausts", + ), + AppServerCapabilityPreflightFailureKind::MethodFailed { + method, + timed_out: true, + .. + } => format!( + "decodex will retry app-server preflight automatically; inspect local app_server_preflight_failed evidence for the `{method}` timeout and restart `decodex serve` if the retry budget exhausts" + ), + AppServerCapabilityPreflightFailureKind::MethodFailed { .. } + | AppServerCapabilityPreflightFailureKind::BlockedState => + String::from("app-server preflight requires operator recovery"), + } + } + pub(crate) fn terminal_next_action(&self, recovery_gate: &str) -> String { match &self.kind { AppServerCapabilityPreflightFailureKind::MethodFailed { @@ -539,6 +601,16 @@ impl AppServerDynamicToolFailure { Self { kind: AppServerDynamicToolFailureKind::Tool, tool, message: message.into() } } + #[cfg(test)] + pub(crate) fn protocol_for_test(tool: Option, message: impl Into) -> Self { + Self::protocol(tool, message) + } + + #[cfg(test)] + pub(crate) fn tool_for_test(tool: Option, message: impl Into) -> Self { + Self::tool(tool, message) + } + pub(crate) fn error_class(&self) -> &'static str { match self.kind { AppServerDynamicToolFailureKind::Protocol => "app_server_dynamic_tool_protocol_failure", @@ -557,6 +629,10 @@ impl AppServerDynamicToolFailure { } } + pub(crate) fn retry_next_action(&self) -> String { + format!("decodex will retry automatically; {}", self.diagnostic_next_action()) + } + fn diagnostic_next_action(&self) -> &'static str { match self.kind { AppServerDynamicToolFailureKind::Protocol => @@ -1314,6 +1390,26 @@ pub(crate) fn probe_app_server(listen: &str) -> crate::prelude::Result( + result: crate::prelude::Result, + phase: RequestWaitPhase, +) -> crate::prelude::Result { + result.map_err(|error| transport_failure_at_phase(error, phase)) +} + +fn transport_failure_at_phase(error: Report, phase: RequestWaitPhase) -> Report { + let Some(transport_failure) = error.downcast_ref::() + else { + return error; + }; + + Report::new(json_rpc::AppServerTransportFailure::with_phase( + transport_failure.to_string(), + phase.label(), + phase.transport_failure_is_retryable_startup(), + )) +} + fn running_model_execution_protocol_activity( protocol_activity: &state::ProtocolActivitySummary, ) -> bool { @@ -2515,23 +2611,26 @@ fn initialize_client_for_run( dynamic_tool_handler: Option<&dyn DynamicToolHandler>, expected_codex_home: &ResolvedAppServerCodexHomeEnv, ) -> crate::prelude::Result { - let response = client.initialize_with_handler( - dynamic_tool_handler.is_some(), - |connection, wire_message, server_request| { - handle_server_request_while_waiting( - connection, - recorder, - wire_message, - server_request, - RequestDispatchContext::new( - RequestWaitPhase::Initialize, - dynamic_tool_handler, - None, - None, - None, - ), - ) - }, + let response = annotate_transport_failure_phase( + client.initialize_with_handler( + dynamic_tool_handler.is_some(), + |connection, wire_message, server_request| { + handle_server_request_while_waiting( + connection, + recorder, + wire_message, + server_request, + RequestDispatchContext::new( + RequestWaitPhase::Initialize, + dynamic_tool_handler, + None, + None, + None, + ), + ) + }, + ), + RequestWaitPhase::Initialize, )?; validate_initialize_codex_home(expected_codex_home, &response)?; @@ -3260,23 +3359,26 @@ fn login_codex_account_for_run( record_codex_account_login(recorder, account.summary())?; - let response = client.login_account_with_handler( - login_account_params(&account), - |connection, wire_message, server_request| { - handle_server_request_while_waiting( - connection, - recorder, - wire_message, - server_request, - RequestDispatchContext::new( - RequestWaitPhase::AccountLogin, - request.dynamic_tool_handler, - request.codex_account_provider, - None, - None, - ), - ) - }, + let response = annotate_transport_failure_phase( + client.login_account_with_handler( + login_account_params(&account), + |connection, wire_message, server_request| { + handle_server_request_while_waiting( + connection, + recorder, + wire_message, + server_request, + RequestDispatchContext::new( + RequestWaitPhase::AccountLogin, + request.dynamic_tool_handler, + request.codex_account_provider, + None, + None, + ), + ) + }, + ), + RequestWaitPhase::AccountLogin, )?; match response { @@ -3315,23 +3417,26 @@ fn start_fresh_thread_session( ) -> crate::prelude::Result { let thread_start_request = build_thread_start_request(request)?; - client.start_thread_with_handler( - thread_start_request, - |connection, wire_message, server_request| { - handle_server_request_while_waiting( - connection, - recorder, - wire_message, - server_request, - RequestDispatchContext::new( - RequestWaitPhase::ThreadStart, - request.dynamic_tool_handler, - request.codex_account_provider, - None, - None, - ), - ) - }, + annotate_transport_failure_phase( + client.start_thread_with_handler( + thread_start_request, + |connection, wire_message, server_request| { + handle_server_request_while_waiting( + connection, + recorder, + wire_message, + server_request, + RequestDispatchContext::new( + RequestWaitPhase::ThreadStart, + request.dynamic_tool_handler, + request.codex_account_provider, + None, + None, + ), + ) + }, + ), + RequestWaitPhase::ThreadStart, ) } @@ -3372,7 +3477,7 @@ fn resume_existing_thread_session( start_fresh_thread_session(client, recorder, request) }, - Err(error) => Err(error), + Err(error) => Err(transport_failure_at_phase(error, RequestWaitPhase::ThreadResume)), } } @@ -3468,23 +3573,26 @@ fn start_turn_for_run( thread_id: &str, next_input: &str, ) -> crate::prelude::Result { - let turn_response = client.start_turn_with_handler( - build_turn_start_request(thread_id, next_input), - |connection, wire_message, server_request| { - handle_server_request_while_waiting( - connection, - recorder, - wire_message, - server_request, - RequestDispatchContext::new( - RequestWaitPhase::TurnStart, - dynamic_tool_handler, - codex_account_provider, - Some(thread_id), - None, - ), - ) - }, + let turn_response = annotate_transport_failure_phase( + client.start_turn_with_handler( + build_turn_start_request(thread_id, next_input), + |connection, wire_message, server_request| { + handle_server_request_while_waiting( + connection, + recorder, + wire_message, + server_request, + RequestDispatchContext::new( + RequestWaitPhase::TurnStart, + dynamic_tool_handler, + codex_account_provider, + Some(thread_id), + None, + ), + ) + }, + ), + RequestWaitPhase::TurnStart, )?; Ok(turn_response.turn.id) @@ -4039,7 +4147,9 @@ fn handle_turn_execution_notification( if let Some((failure, will_retry)) = failure_from_error_notification(notification, target_thread_id, target_turn_id)? { - if failure.requires_operator_attention() && will_retry != Some(true) { + if (failure.requires_operator_attention() || failure.should_stop_current_turn()) + && will_retry != Some(true) + { return Err(Report::new(failure)); } @@ -4722,7 +4832,10 @@ fn recv_turn_wire_message( wait_timeout: Duration, latest_turn_failure: Option<&AppServerTurnFailure>, ) -> crate::prelude::Result { - match client.recv(Some(wait_timeout)) { + match annotate_transport_failure_phase( + client.recv(Some(wait_timeout)), + RequestWaitPhase::TurnExecution, + ) { Ok(wire_message) => Ok(wire_message), Err(error) => { if error.downcast_ref::().is_some() diff --git a/apps/decodex/src/agent/app_server/tests.rs b/apps/decodex/src/agent/app_server/tests.rs index b436dd3d4..d45927b3d 100644 --- a/apps/decodex/src/agent/app_server/tests.rs +++ b/apps/decodex/src/agent/app_server/tests.rs @@ -1416,6 +1416,7 @@ fn capability_preflight_method_error_is_typed_operator_blocker() { ); assert_eq!(failure.error_class(), "app_server_introspection_method_failed"); + assert!(!failure.is_retryable_timeout()); assert!(failure.to_string().contains("model/list")); assert!(failure.to_string().contains("Method not found")); assert_eq!(failure.report().checks().len(), 1); @@ -1467,7 +1468,7 @@ fn plugin_list_preflight_timeout_retries_once_before_success() { } #[test] -fn plugin_list_preflight_timeout_failure_is_typed_operator_blocker() { +fn plugin_list_preflight_timeout_failure_is_typed_retryable_timeout() { let state_store = StateStore::open_in_memory().expect("state store should open"); let mut recorder = RunRecorder::new(&state_store, "run-1", 1, None); let report = AppServerCapabilityPreflightReport::new(); @@ -1493,14 +1494,11 @@ fn plugin_list_preflight_timeout_failure_is_typed_operator_blocker() { assert_eq!(attempts, 2); assert_eq!(failure.error_class(), "app_server_plugin_list_timeout"); + assert!(failure.is_retryable_timeout()); assert!(failure.to_string().contains("app_server_preflight_failed")); assert!(failure.to_string().contains("plugin/list")); assert!(failure.to_string().contains("timed out")); - assert!( - failure - .terminal_next_action("clear label `decodex:needs-attention`") - .contains("app_server_preflight_failed evidence for the `plugin/list` timeout") - ); + assert!(failure.retry_next_action().contains("retry app-server preflight automatically")); assert!(failure.report().has_blockers()); assert_eq!(check.name, "plugins"); assert_eq!(check.status, super::AppServerCapabilityPreflightStatus::Blocked); @@ -1746,7 +1744,7 @@ fn app_server_turn_failure_classifies_operator_attention() { "usage limit", "You've hit your usage limit.", Some(String::from("usageLimitExceeded")), - true, + false, "app_server_usage_limit_exceeded", ), ("generic failure", "transient model failure", None, false, "app_server_turn_failed"), @@ -1761,6 +1759,7 @@ fn app_server_turn_failure_classifies_operator_attention() { assert_eq!(failure.requires_operator_attention(), requires_attention, "{case_name}"); assert_eq!(failure.error_class(), error_class); + assert_eq!(failure.should_stop_current_turn(), case_name == "usage limit", "{case_name}"); if let Some(code) = code { assert!(failure.to_string().contains(&code)); @@ -2101,6 +2100,41 @@ fn dynamic_tool_call_can_validate_thread_without_fixed_turn_during_steer_rpc() { assert_eq!(*handler.seen_namespace.borrow(), Some(String::from("tracker"))); } +#[test] +fn usage_limit_notification_stops_current_turn_without_operator_attention() { + let notification = JsonRpcNotification { + method: String::from("error"), + params: serde_json::json!({ + "threadId": "thread-1", + "turnId": "turn-1", + "willRetry": false, + "error": { + "message": "You've hit your usage limit.", + "codexErrorInfo": "usageLimitExceeded" + } + }), + }; + let mut final_output = String::new(); + let mut latest_turn_failure: Option = None; + let error = super::handle_turn_execution_notification( + ¬ification, + "thread-1", + "turn-1", + &mut final_output, + &mut latest_turn_failure, + ); + let Err(error) = error else { + panic!("usage limit should fail the current turn immediately"); + }; + let failure = + error.downcast_ref::().expect("error should be a turn failure"); + + assert_eq!(failure.error_class(), "app_server_usage_limit_exceeded"); + assert!(failure.is_retryable_capacity_failure()); + assert!(!failure.requires_operator_attention()); + assert!(latest_turn_failure.is_none()); +} + #[test] fn turn_notification_ignores_agent_output_for_non_target_turn() { let old_completed = JsonRpcNotification { diff --git a/apps/decodex/src/agent/app_server/turn_failure.rs b/apps/decodex/src/agent/app_server/turn_failure.rs index d319a19f4..c40b4121c 100644 --- a/apps/decodex/src/agent/app_server/turn_failure.rs +++ b/apps/decodex/src/agent/app_server/turn_failure.rs @@ -39,6 +39,14 @@ impl AppServerTurnFailure { } pub(crate) fn requires_operator_attention(&self) -> bool { + matches!(self.codex_error_info.as_deref(), Some("operatorAttentionRequired")) + } + + pub(crate) fn should_stop_current_turn(&self) -> bool { + self.is_retryable_capacity_failure() + } + + pub(crate) fn is_retryable_capacity_failure(&self) -> bool { matches!(self.codex_error_info.as_deref(), Some("usageLimitExceeded")) } @@ -49,6 +57,14 @@ impl AppServerTurnFailure { } } + pub(crate) fn retry_next_action(&self) -> &'static str { + match self.codex_error_info.as_deref() { + Some("usageLimitExceeded") => + "decodex will retry automatically and reselect or refresh the Codex account before the next attempt", + _ => "decodex will retry automatically", + } + } + pub(crate) fn terminal_next_action(&self, recovery_gate: &str) -> String { match self.codex_error_info.as_deref() { Some("usageLimitExceeded") => format!( diff --git a/apps/decodex/src/agent/json_rpc.rs b/apps/decodex/src/agent/json_rpc.rs index e942ba082..a5c39fcb1 100644 --- a/apps/decodex/src/agent/json_rpc.rs +++ b/apps/decodex/src/agent/json_rpc.rs @@ -197,19 +197,45 @@ impl std::error::Error for AppServerOutputTimeout {} #[derive(Debug)] pub(crate) struct AppServerTransportFailure { details: String, + phase: Option<&'static str>, + retryable_startup: bool, } impl AppServerTransportFailure { pub(crate) fn new(details: String) -> Self { - Self { details } + Self { details, phase: None, retryable_startup: false } + } + + pub(crate) fn with_phase( + details: String, + phase: &'static str, + retryable_startup: bool, + ) -> Self { + Self { details, phase: Some(phase), retryable_startup } } pub(crate) fn error_class(&self) -> &'static str { "app_server_transport_disconnected" } + pub(crate) fn is_retryable_startup(&self) -> bool { + self.retryable_startup + } + + pub(crate) fn retry_next_action(&self) -> String { + if let Some(phase) = self.phase { + format!( + "app-server transport disconnected during `{phase}` before a durable turn was running; decodex will restart the app-server and retry automatically" + ) + } else { + String::from("decodex will retry the app-server transport failure automatically") + } + } + pub(crate) fn terminal_next_action(&self, recovery_gate: &str) -> String { + let phase = self.phase.map_or_else(String::new, |phase| format!(" during `{phase}`")); + format!( - "inspect the local app-server stderr tail and process exit status, resolve the Codex app-server transport failure manually, {recovery_gate}" + "inspect the local app-server stderr tail and process exit status, resolve the Codex app-server transport failure{phase} manually, {recovery_gate}" ) } } diff --git a/apps/decodex/src/agent/tracker_tool_bridge/tests/mutation/continuation.rs b/apps/decodex/src/agent/tracker_tool_bridge/tests/mutation/continuation.rs index 7a93be155..5ef8a81e2 100644 --- a/apps/decodex/src/agent/tracker_tool_bridge/tests/mutation/continuation.rs +++ b/apps/decodex/src/agent/tracker_tool_bridge/tests/mutation/continuation.rs @@ -270,6 +270,10 @@ fn manual_attention_requires_explanatory_comment() { ); assert!(response.success); + assert!( + tracker.label_additions.borrow().is_empty(), + "manual-attention intent alone must not mutate Linear" + ); let error = bridge .completion_disposition() @@ -298,26 +302,33 @@ fn failed_needs_attention_label_update_does_not_record_manual_attention() { ISSUE_LABEL_ADD_TOOL_NAME, serde_json::json!({ "label": "decodex:needs-attention" }), ); + let comment_response = DynamicToolHandler::handle_call( + &bridge, + ISSUE_COMMENT_TOOL_NAME, + manual_attention_comment_args(), + ); - assert!(!response.success); + assert!(response.success); + assert!(!comment_response.success); assert!(tracker.label_updates.borrow().is_empty()); assert!(tracker.label_additions.borrow().is_empty()); + assert!(tracker.comments.borrow().is_empty()); let error = bridge .completion_disposition() .expect_err("failed label writes must not count as manual attention"); - assert!(error.to_string().contains("recorded neither")); + assert!(error.to_string().contains("never recorded the required explanatory comment")); } #[test] -fn label_add_refreshes_issue_snapshot_before_merging_label_ids() { +fn opt_out_label_add_uses_refreshed_issue_snapshot_for_label_ids() { let initial_issue = sample_issue(); let mut refreshed_issue = initial_issue.clone(); refreshed_issue.labels.push(TrackerLabel { - id: String::from("label-manual"), - name: String::from("decodex:manual-only"), + id: String::from("label-needs"), + name: String::from("decodex:needs-attention"), }); let tracker = FakeTracker::with_refresh_snapshots(vec![vec![refreshed_issue]]); @@ -326,11 +337,11 @@ fn label_add_refreshes_issue_snapshot_before_merging_label_ids() { let response = DynamicToolHandler::handle_call( &bridge, ISSUE_LABEL_ADD_TOOL_NAME, - serde_json::json!({ "label": "decodex:needs-attention" }), + serde_json::json!({ "label": "decodex:manual-only" }), ); assert!(response.success); - assert_eq!(tracker.label_additions.borrow().as_slice(), [vec![String::from("label-needs")]]); + assert_eq!(tracker.label_additions.borrow().as_slice(), [vec![String::from("label-manual")]]); } #[test] @@ -342,7 +353,7 @@ fn label_add_fails_when_refresh_returns_no_snapshot() { let response = DynamicToolHandler::handle_call( &bridge, ISSUE_LABEL_ADD_TOOL_NAME, - serde_json::json!({ "label": "decodex:needs-attention" }), + serde_json::json!({ "label": "decodex:manual-only" }), ); assert!(!response.success); diff --git a/apps/decodex/src/agent/tracker_tool_bridge/tests/mutation/dispatch.rs b/apps/decodex/src/agent/tracker_tool_bridge/tests/mutation/dispatch.rs index 2d7e416b3..847892483 100644 --- a/apps/decodex/src/agent/tracker_tool_bridge/tests/mutation/dispatch.rs +++ b/apps/decodex/src/agent/tracker_tool_bridge/tests/mutation/dispatch.rs @@ -781,10 +781,16 @@ fn accepts_manual_attention_public_summary_kind() { ISSUE_LABEL_ADD_TOOL_NAME, serde_json::json!({ "label": "decodex:needs-attention" }), ); + + assert!(label_response.success); + assert!( + tracker.label_additions.borrow().is_empty(), + "manual-attention label intent must not mutate Linear before comment validation" + ); + let comment_response = DynamicToolHandler::handle_call(&bridge, ISSUE_COMMENT_TOOL_NAME, manual_attention_comment_args()); - assert!(label_response.success); assert!(comment_response.success); let comments = tracker.comments.borrow(); @@ -792,6 +798,7 @@ fn accepts_manual_attention_public_summary_kind() { let record = records::parse_linear_execution_event_record(comment) .expect("manual attention summary should include a ledger record"); + assert_eq!(tracker.label_additions.borrow().as_slice(), [vec![String::from("label-needs")]]); assert_eq!(comments.len(), 1); assert!(comment.contains("decodex run needs manual attention")); assert!(comment.contains("- worktree_path: `.worktrees/PUB-618`")); @@ -1004,6 +1011,10 @@ fn rejects_manual_attention_comments_with_private_or_unsupported_fields() { assert!(label_response.success); assert!(!response.success, "comment should be rejected for {field_name}"); assert!(tracker.comments.borrow().is_empty()); + assert!( + tracker.label_additions.borrow().is_empty(), + "rejected manual-attention comment must not leave a needs-attention label" + ); assert_eq!( response.content_items, vec![DynamicToolContentItem::InputText { text: String::from(expected_error) }] @@ -1011,6 +1022,62 @@ fn rejects_manual_attention_comments_with_private_or_unsupported_fields() { } } +#[test] +fn rejects_manual_attention_comment_with_runtime_owned_retryable_error_class() { + for error_class in [ + "retryable_execution_failure", + "repo_gate_verify_failed", + "repo_gate_git_lock_contention", + "stalled_run_detected", + "app_server_plugin_list_timeout", + "app_server_turn_failed", + "app_server_usage_limit_exceeded", + "app_server_dynamic_tool_failed", + "phase_goal_terminal_path_missing", + ] { + let issue = sample_issue(); + let tracker = tracker_with_current_issue_snapshot(&issue); + let workflow = sample_workflow(); + let inspector = FakePullRequestInspector::new(Vec::new()); + let local_repo_inspector = FakeLocalRepoInspector::new(Vec::new()); + let bridge = TrackerToolBridge::with_review_handoff_for_test( + &tracker, + &issue, + &workflow, + sample_review_context(), + &inspector, + &local_repo_inspector, + ); + let label_response = DynamicToolHandler::handle_call( + &bridge, + ISSUE_LABEL_ADD_TOOL_NAME, + serde_json::json!({ "label": "decodex:needs-attention" }), + ); + let mut args = manual_attention_comment_args(); + + args["error_class"] = serde_json::json!(error_class); + + let response = DynamicToolHandler::handle_call(&bridge, ISSUE_COMMENT_TOOL_NAME, args); + + assert!(label_response.success); + assert!( + !response.success, + "manual attention should reject runtime-owned class {error_class}" + ); + assert!(tracker.comments.borrow().is_empty()); + assert!( + tracker.label_additions.borrow().is_empty(), + "runtime-owned class {error_class} must not leave a needs-attention label" + ); + assert!(matches!( + response.content_items.as_slice(), + [DynamicToolContentItem::InputText { text }] + if text.contains("cannot use runtime-owned retryable error class") + && text.contains(error_class) + )); + } +} + #[test] fn rejects_unsupported_issue_comment_kinds_without_mutation() { let issue = sample_issue(); @@ -1046,6 +1113,10 @@ fn rejects_unsupported_issue_comment_kinds_without_mutation() { assert!(label_response.success); assert!(!response.success); assert!(tracker.comments.borrow().is_empty()); + assert!( + tracker.label_additions.borrow().is_empty(), + "unsupported comment kind must not leave a needs-attention label" + ); assert!(matches!( response.content_items.as_slice(), [DynamicToolContentItem::InputText { text }] @@ -1109,6 +1180,10 @@ fn rejects_manual_attention_comment_with_non_public_error_class_shape() { assert!(label_response.success); assert!(!response.success); assert!(tracker.comments.borrow().is_empty()); + assert!( + tracker.label_additions.borrow().is_empty(), + "invalid error_class shape must not leave a needs-attention label" + ); assert_eq!( response.content_items, vec![DynamicToolContentItem::InputText { @@ -1141,9 +1216,9 @@ fn rejects_legacy_public_comments_with_sensitive_or_unknown_paths() { } #[test] -fn adds_allowed_workflow_label() { +fn records_manual_attention_label_intent_without_linear_mutation() { let issue = sample_issue(); - let tracker = tracker_with_current_issue_snapshot(&issue); + let tracker = FakeTracker::new(); let workflow = sample_workflow(); let bridge = TrackerToolBridge::new(&tracker, &issue, &workflow); let response = DynamicToolHandler::handle_call( @@ -1153,7 +1228,28 @@ fn adds_allowed_workflow_label() { ); assert!(response.success); - assert_eq!(tracker.label_additions.borrow().as_slice(), [vec![String::from("label-needs")]]); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(matches!( + response.content_items.as_slice(), + [DynamicToolContentItem::InputText { text }] + if text.contains("Manual-attention label intent recorded") + )); +} + +#[test] +fn adds_allowed_workflow_opt_out_label() { + let issue = sample_issue(); + let tracker = tracker_with_current_issue_snapshot(&issue); + let workflow = sample_workflow(); + let bridge = TrackerToolBridge::new(&tracker, &issue, &workflow); + let response = DynamicToolHandler::handle_call( + &bridge, + ISSUE_LABEL_ADD_TOOL_NAME, + serde_json::json!({ "label": "decodex:manual-only" }), + ); + + assert!(response.success); + assert_eq!(tracker.label_additions.borrow().as_slice(), [vec![String::from("label-manual")]]); } #[test] diff --git a/apps/decodex/src/agent/tracker_tool_bridge/tools.rs b/apps/decodex/src/agent/tracker_tool_bridge/tools.rs index f071799eb..9c47bdc0f 100644 --- a/apps/decodex/src/agent/tracker_tool_bridge/tools.rs +++ b/apps/decodex/src/agent/tracker_tool_bridge/tools.rs @@ -502,7 +502,7 @@ impl<'a> TrackerToolBridge<'a> { pub(super) fn label_add_tool_spec(&self) -> DynamicToolSpec { DynamicToolSpec::new( ISSUE_LABEL_ADD_TOOL_NAME, - "Add an allowed workflow label to the currently leased issue.", + "Add an allowed workflow label to the currently leased issue. For `needs_attention_label`, this records a manual-attention intent; Decodex applies the actual label only after the paired `manual_attention` comment validates.", serde_json::json!({ "type": "object", "properties": { @@ -892,6 +892,10 @@ impl<'a> TrackerToolBridge<'a> { Err(error) => return DynamicToolCallResponse::failure(error.to_string()), }; + if let Err(error) = self.apply_manual_attention_label() { + return DynamicToolCallResponse::failure(error); + } + match tracker::create_prepared_linear_execution_event_comment( self.tracker, &self.issue.id, @@ -936,7 +940,7 @@ impl<'a> TrackerToolBridge<'a> { let decision_request = parsed.decision_request.map(Self::normalize_authority_decision_request).transpose()?; - validate_public_error_class(&error_class)?; + validate_manual_attention_error_class(&error_class)?; if blockers.is_empty() { return Err(format!( @@ -961,6 +965,36 @@ impl<'a> TrackerToolBridge<'a> { }) } + fn apply_manual_attention_label(&self) -> Result<(), String> { + let label = self.workflow.frontmatter().tracker().needs_attention_label(); + let current_issue = match self.refreshed_issue_snapshot() { + Ok(Some(issue)) => issue, + Ok(None) => { + return Err(format!( + "Failed to refresh issue `{}` before applying manual-attention label `{label}`: tracker returned no current snapshot.", + self.issue.identifier + )); + }, + Err(error) => { + return Err(format!( + "Failed to refresh issue `{}` before applying manual-attention label `{label}`: {error}", + self.issue.identifier + )); + }, + }; + + tracker::set_issue_label_presence(self.tracker, ¤t_issue, label, true).map_err( + |error| { + format!( + "Failed to add label `{label}` to issue `{}`: {error}", + self.issue.identifier + ) + }, + )?; + + Ok(()) + } + fn normalize_authority_decision_request( parsed: AuthorityDecisionRequestArgs, ) -> Result { @@ -1786,6 +1820,18 @@ impl<'a> TrackerToolBridge<'a> { )); } + let manual_attention_label = + parsed.label == self.workflow.frontmatter().tracker().needs_attention_label(); + + if manual_attention_label { + self.manual_attention_requested.replace(true); + + return DynamicToolCallResponse::success(format!( + "Manual-attention label intent recorded for issue `{}`. Call `{ISSUE_COMMENT_TOOL_NAME}` kind `{COMMENT_KIND_MANUAL_ATTENTION}` next so Decodex can validate the blocker and apply label `{}`.", + self.issue.identifier, parsed.label + )); + } + let current_issue = match self.refreshed_issue_snapshot() { Ok(Some(issue)) => issue, Ok(None) => { @@ -1801,8 +1847,6 @@ impl<'a> TrackerToolBridge<'a> { )); }, }; - let manual_attention_label = - parsed.label == self.workflow.frontmatter().tracker().needs_attention_label(); let label_added = match tracker::set_issue_label_presence( self.tracker, ¤t_issue, @@ -2165,6 +2209,39 @@ fn validate_public_error_class(error_class: &str) -> Result<(), String> { Ok(()) } +fn validate_manual_attention_error_class(error_class: &str) -> Result<(), String> { + validate_public_error_class(error_class)?; + + if is_runtime_owned_manual_attention_error_class(error_class) { + return Err(format!( + "`{ISSUE_COMMENT_TOOL_NAME}` kind `{COMMENT_KIND_MANUAL_ATTENTION}` cannot use runtime-owned retryable error class `{error_class}`; keep repairing or retrying the lane, or use a human-owned blocker class only when automation cannot clear the blocker." + )); + } + + Ok(()) +} + +fn is_runtime_owned_manual_attention_error_class(error_class: &str) -> bool { + matches!( + error_class, + "retryable_execution_failure" + | "repo_gate_canonicalize_failed" + | "repo_gate_verify_failed" + | "repo_gate_tracked_rewrites_left" + | "repo_gate_git_lock_contention" + | "stalled_run_detected" + | "app_server_zero_evidence_start_failed" + | "app_server_plugin_list_timeout" + | "app_server_preflight_timeout" + | "app_server_transport_disconnected" + | "phase_goal_terminal_path_missing" + | "app_server_dynamic_tool_protocol_failure" + | "app_server_dynamic_tool_failed" + | "app_server_turn_failed" + | "app_server_usage_limit_exceeded" + ) +} + fn format_manual_attention_comment( review_context: &ReviewHandoffContext, comment: &NormalizedManualAttentionComment, diff --git a/apps/decodex/src/orchestrator/daemon.rs b/apps/decodex/src/orchestrator/daemon.rs index 7f93a4003..acbc1d1c0 100644 --- a/apps/decodex/src/orchestrator/daemon.rs +++ b/apps/decodex/src/orchestrator/daemon.rs @@ -757,8 +757,13 @@ fn materialize_daemon_spawn_state( summary: &RunSummary, ) -> Result { let worktree = materialize_run_summary_worktree(project, workflow, summary)?; - let retry_budget_base = - retry_budget_base_for_issue_worktree(state_store, &summary.issue_id, &worktree.path)?; + let retry_budget_base = retry_budget_base_for_dispatch_mode( + state_store, + &summary.issue_id, + &worktree.path, + summary.dispatch_mode, + None, + )?; Ok(MaterializedDaemonSpawnState { worktree, retry_budget_base }) } diff --git a/apps/decodex/src/orchestrator/dispatch_policy.rs b/apps/decodex/src/orchestrator/dispatch_policy.rs index 602ba8834..8bc4f2628 100644 --- a/apps/decodex/src/orchestrator/dispatch_policy.rs +++ b/apps/decodex/src/orchestrator/dispatch_policy.rs @@ -447,7 +447,27 @@ fn retry_budget_base_for_issue_worktree( ) -> Result { Ok(state_store .retry_budget_attempt_count(issue_id)? - .max(state::read_run_retry_budget_attempt_count(worktree_path)?.unwrap_or(0))) + .max(state::read_run_retry_budget_attempt_count(worktree_path)?.unwrap_or(0))) +} + +fn retry_budget_base_for_dispatch_mode( + state_store: &StateStore, + issue_id: &str, + worktree_path: &Path, + dispatch_mode: IssueDispatchMode, + preferred_retry_budget_base: Option, +) -> Result { + let preferred_retry_budget_base = preferred_retry_budget_base.unwrap_or(0); + + if dispatch_mode == IssueDispatchMode::Normal { + return Ok(preferred_retry_budget_base); + } + + Ok(preferred_retry_budget_base.max(retry_budget_base_for_issue_worktree( + state_store, + issue_id, + worktree_path, + )?)) } fn issue_retry_budget_exhausted( diff --git a/apps/decodex/src/orchestrator/execution.rs b/apps/decodex/src/orchestrator/execution.rs index 25075bbad..3c0c7e317 100644 --- a/apps/decodex/src/orchestrator/execution.rs +++ b/apps/decodex/src/orchestrator/execution.rs @@ -32,6 +32,13 @@ impl AppServerZeroEvidenceStartFailure { self.run_id ) } + + fn retry_next_action(&self) -> String { + format!( + "restart the app-server and retry automatically for run `{}`; inspect private startup diagnostics if the retry budget exhausts", + self.run_id + ) + } } impl Display for AppServerZeroEvidenceStartFailure { @@ -148,11 +155,6 @@ where codex_account_provider: Option<&'a dyn CodexAccountProvider>, } -enum IssueAppServerRunOutcome { - Completed(AppServerRunResult), - Finalized(RunSummary), -} - struct RepoGatePhaseGoalController<'a> { project: &'a ServiceConfig, workflow: &'a WorkflowDocument, @@ -462,6 +464,27 @@ struct ArchitectureRecoveryTerminalEventInput<'a> { recovery_attempt_number: usize, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum RunFailureWritebackDisposition { + RetryableGeneric, + RetryableStructuredRecovery, + TerminalAttention, +} +impl RunFailureWritebackDisposition { + fn requires_terminal_attention(self) -> bool { + self == Self::TerminalAttention + } + + fn preserves_retry_through_zero_evidence(self) -> bool { + self == Self::RetryableStructuredRecovery + } +} + +enum IssueAppServerRunOutcome { + Completed(AppServerRunResult), + Finalized(RunSummary), +} + enum LoopGuardrailRecoveryDecision { Start(ArchitectureRecoveryStart), HumanRequired(LoopGuardrailStopRequested), @@ -474,6 +497,65 @@ enum TerminalFailureEventRecordStatus { NoLocalStore, } +pub(crate) fn run_failure_writeback_disposition( + error: &Report, +) -> RunFailureWritebackDisposition { + if error.downcast_ref::().is_some() + || error.downcast_ref::().is_some() + || error + .downcast_ref::() + .is_some_and(|failure| !failure.is_terminal_path_missing()) + || error.downcast_ref::().is_some() + || error.downcast_ref::().is_some() + || error + .downcast_ref::() + .is_some_and(|failure| !failure.is_retryable_timeout()) + || error.downcast_ref::().is_some() + || error + .downcast_ref::() + .is_some_and(|failure| !failure.is_retryable_startup()) + || error.downcast_ref::().is_some() + || error + .downcast_ref::() + .is_some_and(AppServerTurnFailure::requires_operator_attention) + || error.downcast_ref::().is_some() + || error + .downcast_ref::() + .is_some_and(|repo_gate_failure| { + repo_gate_failure.disposition() == RepoGateFailureDisposition::NeedsHumanAttention + }) + { + RunFailureWritebackDisposition::TerminalAttention + } else if error + .downcast_ref::() + .is_some() + || error + .downcast_ref::() + .is_some_and(AppServerCapabilityPreflightFailure::is_retryable_timeout) + || error.downcast_ref::().is_some() + || error + .downcast_ref::() + .is_some_and(|repo_gate_failure| { + matches!( + repo_gate_failure.disposition(), + RepoGateFailureDisposition::ContinueRepair + | RepoGateFailureDisposition::RetryAfterBackoff + ) + }) || error + .downcast_ref::() + .is_some_and(AppServerTransportFailure::is_retryable_startup) + || error + .downcast_ref::() + .is_some_and(AppServerPhaseGoalFailure::is_terminal_path_missing) + || error.downcast_ref::().is_some() + || error.downcast_ref::().is_some() + { + RunFailureWritebackDisposition::RetryableStructuredRecovery + } else { + RunFailureWritebackDisposition::RetryableGeneric + } +} + fn execute_issue_run( tracker: &T, project: &ServiceConfig, @@ -2125,7 +2207,11 @@ fn promote_zero_evidence_app_server_start_failure( issue_run: &IssueRunPlan, error: Report, ) -> Report { - if run_failure_requires_terminal_attention(&error) { + let writeback_disposition = run_failure_writeback_disposition(&error); + + if writeback_disposition.requires_terminal_attention() + || writeback_disposition.preserves_retry_through_zero_evidence() + { return error; } @@ -3100,26 +3186,7 @@ fn architecture_recovery_goal_detail( } fn run_failure_requires_terminal_attention(error: &Report) -> bool { - error.downcast_ref::().is_some() - || error.downcast_ref::().is_some() - || error.downcast_ref::().is_some() - || error.downcast_ref::().is_some() - || error.downcast_ref::().is_some() - || error.downcast_ref::().is_some() - || error.downcast_ref::().is_some() - || error.downcast_ref::().is_some() - || error.downcast_ref::().is_some() - || error.downcast_ref::().is_some() - || error.downcast_ref::().is_some() - || error - .downcast_ref::() - .is_some_and(AppServerTurnFailure::requires_operator_attention) - || error.downcast_ref::().is_some() - || error - .downcast_ref::() - .is_some_and(|repo_gate_failure| { - repo_gate_failure.disposition() == RepoGateFailureDisposition::NeedsHumanAttention - }) + run_failure_writeback_disposition(error).requires_terminal_attention() } fn handle_failure( @@ -3158,9 +3225,6 @@ where issue_run, &worktree_path, ); - let retryable_retained_partial_progress = retained_partial_progress - .as_ref() - .filter(|_| retryable_failure_should_stop_for_retained_progress(error)); if let Some(review_policy_stop) = error.downcast_ref::() && review_policy_stop.reason == ReviewPolicyStopReason::Exhausted @@ -3193,10 +3257,7 @@ where }; } - if !requires_terminal_attention - && retry_budget_attempts < max_attempts - && retryable_retained_partial_progress.is_none() - { + if !requires_terminal_attention && retry_budget_attempts < max_attempts { return apply_retryable_failure_writeback(&failure_context, error, max_attempts); } @@ -3482,12 +3543,6 @@ fn retained_partial_progress_error( })) } -fn retryable_failure_should_stop_for_retained_progress(error: &Report) -> bool { - !error.downcast_ref::().is_some_and(|repo_gate_failure| { - repo_gate_failure.disposition() == RepoGateFailureDisposition::ContinueRepair - }) -} - fn retained_progress_should_defer_to_terminal_intent(error: &Report) -> bool { error.downcast_ref::().is_some() || error.downcast_ref::().is_some() @@ -3537,12 +3592,51 @@ fn write_retry_schedule_marker_for_runtime_retry( issue_run: &IssueRunPlan, retry_budget_attempts: i64, ) -> Result<()> { + if error.downcast_ref::().is_some() { + return write_failure_retry_schedule_marker(workflow, issue_run, retry_budget_attempts); + } + if error + .downcast_ref::() + .is_some_and(AppServerCapabilityPreflightFailure::is_retryable_timeout) + { + return write_failure_retry_schedule_marker(workflow, issue_run, retry_budget_attempts); + } + if error + .downcast_ref::() + .is_some_and(AppServerPhaseGoalFailure::is_terminal_path_missing) + { + return write_failure_retry_schedule_marker(workflow, issue_run, retry_budget_attempts); + } + let Some(repo_gate_failure) = error.downcast_ref::() else { return Ok(()); }; let Some(retry_kind) = repo_gate_failure.retry_schedule_kind() else { return Ok(()); }; + + write_retry_schedule_marker( + workflow, + issue_run, + retry_budget_attempts, + retry_kind, + ) +} + +fn write_failure_retry_schedule_marker( + workflow: &WorkflowDocument, + issue_run: &IssueRunPlan, + retry_budget_attempts: i64, +) -> Result<()> { + write_retry_schedule_marker(workflow, issue_run, retry_budget_attempts, "failure") +} + +fn write_retry_schedule_marker( + workflow: &WorkflowDocument, + issue_run: &IssueRunPlan, + retry_budget_attempts: i64, + retry_kind: &str, +) -> Result<()> { let retry_attempt = u32::try_from(retry_budget_attempts).unwrap_or(u32::MAX).max(1); let delay = retry_delay(RetryKind::Failure, retry_attempt, workflow); let retry_ready_at_unix_epoch = OffsetDateTime::now_utc().unix_timestamp().saturating_add( diff --git a/apps/decodex/src/orchestrator/prompting.rs b/apps/decodex/src/orchestrator/prompting.rs index 0ff9c7d38..b84b5d47e 100644 --- a/apps/decodex/src/orchestrator/prompting.rs +++ b/apps/decodex/src/orchestrator/prompting.rs @@ -4,6 +4,25 @@ pub(crate) const TRACKER_PUBLIC_TEXT_BOUNDARY_INSTRUCTION: &str = const SELF_CHECK_INSTRUCTION: &str = "Review your work repeatedly and fix any logic bugs until no new issues are found."; +fn build_manual_attention_guidance( + needs_attention: &str, + label_tool: &str, + terminal_finalize_tool: &str, + review_handoff_tool: Option<&str>, +) -> String { + let review_handoff_guidance = review_handoff_tool + .map(|tool| { + format!( + " Do not call `{tool}` in that case; `decodex` will stop the lane as a human-required failure without automatic retry." + ) + }) + .unwrap_or_default(); + + format!( + "If you determine the issue needs human attention, request label `{needs_attention}` with `{label_tool}`; that records manual-attention label intent only. Then call `issue_comment` with kind `manual_attention` and structured public fields (`error_class`, `next_action`, `blockers`, `evidence`; include `failed_command` and `raw_error` only when public-safe). Decodex applies the actual label only after that manual_attention comment validates. Use a human-owned blocker class; do not use runtime-owned retry/repair classes such as app-server timeout, transport, turn, dynamic-tool, or usage-limit failures; stalled-run detection; phase-goal terminal-path misses; repo-gate canonicalize, verify, tracked-rewrite, or git-lock failures; or generic retryable execution failures. Then call `{terminal_finalize_tool}` with path `manual_attention`. Do not speculate about capabilities you did not directly verify.{review_handoff_guidance}" + ) +} + fn build_phase_goal_runtime_contract() -> String { format!( "Phase goal runtime contract\n- Decodex may set an active phase goal that narrows the immediate turn below the full issue lifecycle checklist.\n- Treat the active phase goal as the authoritative current contract. For `implement_to_validation_ready`, `repair_validation_failures`, and `repair_accepted_review_findings`, stop at validated local work, then explicitly complete the active phase goal with the Codex goal completion mechanism so Decodex can run its repo gate and select the next phase.\n- Do not use `{progress_checkpoint_tool}`, final chat text, or an \"await next phase\" statement as a substitute for completing a satisfied phase goal.\n- Only the later `handoff_evidence` phase should create/update the PR and record the terminal handoff path; that phase still requires the normal review handoff plus `{terminal_finalize_tool}`.", @@ -150,9 +169,22 @@ where .tracker() .resolved_completed_state(); let review_level = project.codex().review_level(); + let needs_attention = workflow.frontmatter().tracker().needs_attention_label(); + let repair_manual_attention_guidance = build_manual_attention_guidance( + needs_attention, + ISSUE_LABEL_ADD_TOOL_NAME, + ISSUE_TERMINAL_FINALIZE_TOOL_NAME, + None, + ); + let handoff_manual_attention_guidance = build_manual_attention_guidance( + needs_attention, + ISSUE_LABEL_ADD_TOOL_NAME, + ISSUE_TERMINAL_FINALIZE_TOOL_NAME, + Some(ISSUE_REVIEW_HANDOFF_TOOL_NAME), + ); let tracker_contract = match issue_run.dispatch_mode { IssueDispatchMode::ReviewRepair => format!( - "Tracker tool contract\n- You own issue-scoped tracker writes for `{issue}` on retained PR `{pr_url}`.\n- This run resumes an existing `{success}` lane. Do not move the issue back to `{in_progress}` and do not call `{review_handoff_tool}`.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n{decodex_review_guidance}- For each actionable review item on `{pr_url}`, including non-thread review summaries, validate the claim against the codebase, tests, and requirements before changing code, and keep pushback or clarification threads open until the repaired head is ready.\n- If this run was triggered by retained landing fallback, handle only the implementation-shaped blocker such as branch sync, conflict resolution, ambiguous mergeability, or repository-specific recovery. Do not merge or land the PR yourself.\n{repair_architecture_guidance}- Repair the current PR head on branch `{branch}`, run the repository validation needed to justify the repaired head, and push the repaired head.\n- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n{github_review_guidance}- After the repaired head is pushed, reply in-thread for every addressed comment and resolve only the GitHub review threads whose fixes landed and verified on the repaired head.\n{completion_guidance}- If you determine the issue needs human attention, add label `{needs_attention}` with `{label_tool}`, then call `issue_comment` with kind `manual_attention` and structured public fields (`error_class`, `next_action`, `blockers`, `evidence`; include `failed_command` and `raw_error` only when public-safe), and then call `{terminal_finalize_tool}` with path `manual_attention`. Do not speculate about capabilities you did not directly verify.\n{retained_tail_guidance}- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}\n- Never write to any other issue.", + "Tracker tool contract\n- You own issue-scoped tracker writes for `{issue}` on retained PR `{pr_url}`.\n- This run resumes an existing `{success}` lane. Do not move the issue back to `{in_progress}` and do not call `{review_handoff_tool}`.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n{decodex_review_guidance}- For each actionable review item on `{pr_url}`, including non-thread review summaries, validate the claim against the codebase, tests, and requirements before changing code, and keep pushback or clarification threads open until the repaired head is ready.\n- If this run was triggered by retained landing fallback, handle only the implementation-shaped blocker such as branch sync, conflict resolution, ambiguous mergeability, or repository-specific recovery. Do not merge or land the PR yourself.\n{repair_architecture_guidance}- Repair the current PR head on branch `{branch}`, run the repository validation needed to justify the repaired head, and push the repaired head.\n- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n{github_review_guidance}- After the repaired head is pushed, reply in-thread for every addressed comment and resolve only the GitHub review threads whose fixes landed and verified on the repaired head.\n{completion_guidance}- {manual_attention_guidance}\n{retained_tail_guidance}- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}\n- Never write to any other issue.", issue = issue_run.issue.identifier, pr_url = recorded_pr_url.unwrap_or("(missing review handoff marker)"), progress_checkpoint_tool = ISSUE_PROGRESS_CHECKPOINT_TOOL_NAME, @@ -161,8 +193,7 @@ where in_progress = workflow.frontmatter().tracker().in_progress_state(), success = workflow.frontmatter().tracker().success_state(), branch = issue_run.worktree.branch_name, - needs_attention = workflow.frontmatter().tracker().needs_attention_label(), - label_tool = ISSUE_LABEL_ADD_TOOL_NAME, + manual_attention_guidance = repair_manual_attention_guidance, continuation_guidance = continuation_guidance, repair_architecture_guidance = repair_architecture_guidance, decodex_review_guidance = build_repair_review_guidance(review_level), @@ -171,7 +202,7 @@ where completion_guidance = build_repair_completion_guidance(review_level), ), IssueDispatchMode::Closeout => format!( - "Tracker tool contract\n- You own issue-scoped tracker writes for `{issue}` on retained PR `{pr_url}`.\n- This run resumes a merged post-review lane for the same PR lineage. The tracker issue may still be in `{success}` or may already be in `{completed}` while deterministic closeout tail work remains. Do not move the issue back to `{in_progress}` and do not call `{review_handoff_tool}` or `{review_repair_tool}`.\n- Treat retained closeout as a short deterministic tail. Reuse the existing merged PR evidence instead of restarting broad discovery, and only rerun the minimum validation needed to justify `Done` plus cleanup.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n- If you call `{progress_checkpoint_tool}` during closeout, either omit `head_sha` and let `decodex` record the exact current lane HEAD automatically, or pass the exact full current `HEAD` SHA. Do not send an abbreviated SHA that differs from the live lane head.\n- Merge is already authoritative for `{pr_url}` before this run starts. Do not land, merge, or request review from this closeout run.\n- If the issue is still in `{success}`, transition it once to `{completed}` with `{transition_tool}` before `{closeout_tool}`. If it is already in `{completed}`, leave it there.\n- Finish the remaining Linear closeout tail work for this same merged PR lineage, then call `{closeout_tool}` with PR `{pr_url}` and a short result summary, then call `{terminal_finalize_tool}` with path `closeout`.\n- Do not end the turn without either `{closeout_tool}` plus `{terminal_finalize_tool}`, or the manual-attention path.\n- If you determine the issue needs human attention, add label `{needs_attention}` with `{label_tool}`, then call `issue_comment` with kind `manual_attention` and structured public fields (`error_class`, `next_action`, `blockers`, `evidence`; include `failed_command` and `raw_error` only when public-safe), and then call `{terminal_finalize_tool}` with path `manual_attention`. Do not speculate about capabilities you did not directly verify.\n- Keep all tracker and PR writes scoped to this retained lane. `decodex` will validate the merged PR lineage, the resolved completed state, and the later cleanup boundary.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}\n- Never write to any other issue.", + "Tracker tool contract\n- You own issue-scoped tracker writes for `{issue}` on retained PR `{pr_url}`.\n- This run resumes a merged post-review lane for the same PR lineage. The tracker issue may still be in `{success}` or may already be in `{completed}` while deterministic closeout tail work remains. Do not move the issue back to `{in_progress}` and do not call `{review_handoff_tool}` or `{review_repair_tool}`.\n- Treat retained closeout as a short deterministic tail. Reuse the existing merged PR evidence instead of restarting broad discovery, and only rerun the minimum validation needed to justify `Done` plus cleanup.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n- If you call `{progress_checkpoint_tool}` during closeout, either omit `head_sha` and let `decodex` record the exact current lane HEAD automatically, or pass the exact full current `HEAD` SHA. Do not send an abbreviated SHA that differs from the live lane head.\n- Merge is already authoritative for `{pr_url}` before this run starts. Do not land, merge, or request review from this closeout run.\n- If the issue is still in `{success}`, transition it once to `{completed}` with `{transition_tool}` before `{closeout_tool}`. If it is already in `{completed}`, leave it there.\n- Finish the remaining Linear closeout tail work for this same merged PR lineage, then call `{closeout_tool}` with PR `{pr_url}` and a short result summary, then call `{terminal_finalize_tool}` with path `closeout`.\n- Do not end the turn without either `{closeout_tool}` plus `{terminal_finalize_tool}`, or the manual-attention path.\n- {manual_attention_guidance}\n- Keep all tracker and PR writes scoped to this retained lane. `decodex` will validate the merged PR lineage, the resolved completed state, and the later cleanup boundary.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}\n- Never write to any other issue.", issue = issue_run.issue.identifier, pr_url = recorded_pr_url.unwrap_or("(missing review handoff marker)"), progress_checkpoint_tool = ISSUE_PROGRESS_CHECKPOINT_TOOL_NAME, @@ -183,22 +214,19 @@ where in_progress = workflow.frontmatter().tracker().in_progress_state(), success = workflow.frontmatter().tracker().success_state(), completed = completed_state, - needs_attention = workflow.frontmatter().tracker().needs_attention_label(), - label_tool = ISSUE_LABEL_ADD_TOOL_NAME, + manual_attention_guidance = repair_manual_attention_guidance, continuation_guidance = continuation_guidance, ), _ => format!( - "Tracker tool contract\n- You own issue-scoped tracker writes for `{issue}`.\n- At the start of execution, call `{transition_tool}` to move the issue to `{in_progress}`. Decodex already records the run-start Linear ledger, so do not add a separate start comment.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n{decodex_review_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- When the implementation is ready, commit the lane, push branch `{branch}`, and create or update a non-draft PR titled `{pr_title}` for that branch.\n{completion_guidance}- If you determine the issue needs human attention, add label `{needs_attention}` with `{label_tool}`, then call `issue_comment` with kind `manual_attention` and structured public fields (`error_class`, `next_action`, `blockers`, `evidence`; include `failed_command` and `raw_error` only when public-safe), and then call `{terminal_finalize_tool}` with path `manual_attention`. Do not speculate about capabilities you did not directly verify. Do not call `{review_handoff_tool}` in that case; `decodex` will stop the lane as a human-required failure without automatic retry.\n- Do not move the issue directly to `{success}` with `{transition_tool}`. `decodex` will complete the success writeback only after its own validation passes.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}\n- Never write to any other issue.", + "Tracker tool contract\n- You own issue-scoped tracker writes for `{issue}`.\n- At the start of execution, call `{transition_tool}` to move the issue to `{in_progress}`. Decodex already records the run-start Linear ledger, so do not add a separate start comment.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n{decodex_review_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- When the implementation is ready, commit the lane, push branch `{branch}`, and create or update a non-draft PR titled `{pr_title}` for that branch.\n{completion_guidance}- {manual_attention_guidance}\n- Do not move the issue directly to `{success}` with `{transition_tool}`. `decodex` will complete the success writeback only after its own validation passes.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}\n- Never write to any other issue.", issue = issue_run.issue.identifier, transition_tool = ISSUE_TRANSITION_TOOL_NAME, - label_tool = ISSUE_LABEL_ADD_TOOL_NAME, progress_checkpoint_tool = ISSUE_PROGRESS_CHECKPOINT_TOOL_NAME, - review_handoff_tool = ISSUE_REVIEW_HANDOFF_TOOL_NAME, terminal_finalize_tool = ISSUE_TERMINAL_FINALIZE_TOOL_NAME, in_progress = workflow.frontmatter().tracker().in_progress_state(), branch = issue_run.worktree.branch_name, success = workflow.frontmatter().tracker().success_state(), - needs_attention = workflow.frontmatter().tracker().needs_attention_label(), + manual_attention_guidance = handoff_manual_attention_guidance, continuation_guidance = continuation_guidance, pr_title = review_pull_request_title(&issue_run.issue), decodex_review_guidance = build_handoff_review_guidance( @@ -238,6 +266,19 @@ where .tracker() .resolved_completed_state(); let review_level = project.codex().review_level(); + let needs_attention = workflow.frontmatter().tracker().needs_attention_label(); + let repair_manual_attention_guidance = build_manual_attention_guidance( + needs_attention, + ISSUE_LABEL_ADD_TOOL_NAME, + ISSUE_TERMINAL_FINALIZE_TOOL_NAME, + None, + ); + let handoff_manual_attention_guidance = build_manual_attention_guidance( + needs_attention, + ISSUE_LABEL_ADD_TOOL_NAME, + ISSUE_TERMINAL_FINALIZE_TOOL_NAME, + Some(ISSUE_REVIEW_HANDOFF_TOOL_NAME), + ); let recovery_context = build_retry_recovery_context(issue_run.dispatch_mode) .into_iter() .chain(build_architecture_recovery_context(project, state_store, issue_run)) @@ -246,7 +287,7 @@ where match issue_run.dispatch_mode { IssueDispatchMode::ReviewRepair => format!( - "Continue retained review repair for Linear issue {identifier}: {title}\n\nDescription:\n{description}\n\nCurrent PR:\n- `{pr_url}`\n\nExecution checklist:\n- Resume from the current branch and PR state in this worktree. Do not move the issue back to `{in_progress}`.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n{decodex_review_guidance}- Read the current review feedback on `{pr_url}`, including non-thread review summaries, validate each actionable claim against the codebase, tests, and requirements, fix only the verified issues on branch `{branch}`, and keep scope limited to the outstanding retained repair.\n- If the lane is here because retained landing was not a deterministic clean path, handle only the branch sync, conflict resolution, ambiguous mergeability, or repository-specific recovery needed to make the PR clean again. Do not merge or land the PR yourself.\n- Leave pushback or clarification threads open until the repaired head is ready.\n{repair_architecture_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- Run the repository validation needed to justify the repaired head.\n- Commit the repair and push the same branch.\n{github_review_guidance}- After the repaired head is pushed, reply in-thread for every addressed comment and resolve only the GitHub review threads whose fixes landed and verified on the repaired head.\n{completion_guidance}- If the issue needs manual attention, add label `{needs_attention}` with `{label_tool}`, call `issue_comment` with kind `manual_attention` and structured public fields, and then call `{terminal_finalize_tool}` with path `manual_attention`.\n- Keep the issue in `{success}` and do not treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}", + "Continue retained review repair for Linear issue {identifier}: {title}\n\nDescription:\n{description}\n\nCurrent PR:\n- `{pr_url}`\n\nExecution checklist:\n- Resume from the current branch and PR state in this worktree. Do not move the issue back to `{in_progress}`.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n{decodex_review_guidance}- Read the current review feedback on `{pr_url}`, including non-thread review summaries, validate each actionable claim against the codebase, tests, and requirements, fix only the verified issues on branch `{branch}`, and keep scope limited to the outstanding retained repair.\n- If the lane is here because retained landing was not a deterministic clean path, handle only the branch sync, conflict resolution, ambiguous mergeability, or repository-specific recovery needed to make the PR clean again. Do not merge or land the PR yourself.\n- Leave pushback or clarification threads open until the repaired head is ready.\n{repair_architecture_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- Run the repository validation needed to justify the repaired head.\n- Commit the repair and push the same branch.\n{github_review_guidance}- After the repaired head is pushed, reply in-thread for every addressed comment and resolve only the GitHub review threads whose fixes landed and verified on the repaired head.\n{completion_guidance}- {manual_attention_guidance}\n- Keep the issue in `{success}` and do not treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}", identifier = issue.identifier, title = issue.title, description = description, @@ -255,8 +296,7 @@ where branch = issue_run.worktree.branch_name, progress_checkpoint_tool = ISSUE_PROGRESS_CHECKPOINT_TOOL_NAME, terminal_finalize_tool = ISSUE_TERMINAL_FINALIZE_TOOL_NAME, - needs_attention = workflow.frontmatter().tracker().needs_attention_label(), - label_tool = ISSUE_LABEL_ADD_TOOL_NAME, + manual_attention_guidance = repair_manual_attention_guidance, success = workflow.frontmatter().tracker().success_state(), continuation_guidance = continuation_guidance, repair_architecture_guidance = repair_architecture_guidance, @@ -265,7 +305,7 @@ where completion_guidance = build_repair_completion_guidance(review_level), ), IssueDispatchMode::Closeout => format!( - "Continue retained closeout for Linear issue {identifier}: {title}\n\nDescription:\n{description}\n\nCurrent PR:\n- `{pr_url}`\n\nExecution checklist:\n- Resume from the current branch and merged PR lineage in this worktree. Do not move the issue back to `{in_progress}`.\n- Treat retained closeout as a short deterministic tail. Reuse the existing merged PR evidence instead of restarting broad discovery, and only rerun the minimum validation needed to justify `Done` plus cleanup.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n- If you call `{progress_checkpoint_tool}` during closeout, either omit `head_sha` and let `decodex` record the exact current lane HEAD automatically, or pass the exact full current `HEAD` SHA.\n- Merge is already authoritative for `{pr_url}` before this run starts. Do not land, merge, or request review from this closeout run.\n- The tracker issue may already be in `{completed}` while this deterministic tail work remains pending.\n- If the issue is still in `{success}`, move it once to `{completed}` with `{transition_tool}` before `{closeout_tool}`.\n- Call `{closeout_tool}` with `{pr_url}` and a short result summary, then call `{terminal_finalize_tool}` with path `closeout`.\n- Do not end the turn without either `{closeout_tool}` plus `{terminal_finalize_tool}`, or the manual-attention path.\n- If the issue needs manual attention, add label `{needs_attention}` with `{label_tool}`, call `issue_comment` with kind `manual_attention` and structured public fields, and then call `{terminal_finalize_tool}` with path `manual_attention`.\n- Keep the lane scoped to this retained post-review work and do not treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}", + "Continue retained closeout for Linear issue {identifier}: {title}\n\nDescription:\n{description}\n\nCurrent PR:\n- `{pr_url}`\n\nExecution checklist:\n- Resume from the current branch and merged PR lineage in this worktree. Do not move the issue back to `{in_progress}`.\n- Treat retained closeout as a short deterministic tail. Reuse the existing merged PR evidence instead of restarting broad discovery, and only rerun the minimum validation needed to justify `Done` plus cleanup.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n- If you call `{progress_checkpoint_tool}` during closeout, either omit `head_sha` and let `decodex` record the exact current lane HEAD automatically, or pass the exact full current `HEAD` SHA.\n- Merge is already authoritative for `{pr_url}` before this run starts. Do not land, merge, or request review from this closeout run.\n- The tracker issue may already be in `{completed}` while this deterministic tail work remains pending.\n- If the issue is still in `{success}`, move it once to `{completed}` with `{transition_tool}` before `{closeout_tool}`.\n- Call `{closeout_tool}` with `{pr_url}` and a short result summary, then call `{terminal_finalize_tool}` with path `closeout`.\n- Do not end the turn without either `{closeout_tool}` plus `{terminal_finalize_tool}`, or the manual-attention path.\n- {manual_attention_guidance}\n- Keep the lane scoped to this retained post-review work and do not treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}", identifier = issue.identifier, title = issue.title, description = description, @@ -277,25 +317,22 @@ where terminal_finalize_tool = ISSUE_TERMINAL_FINALIZE_TOOL_NAME, success = workflow.frontmatter().tracker().success_state(), completed = completed_state, - needs_attention = workflow.frontmatter().tracker().needs_attention_label(), - label_tool = ISSUE_LABEL_ADD_TOOL_NAME, + manual_attention_guidance = repair_manual_attention_guidance, continuation_guidance = continuation_guidance, ), _ => format!( - "Resolve Linear issue {identifier}: {title}\n\nDescription:\n{description}\n\n{recovery_context}Execution checklist:\n- Move the issue to `{in_progress}` with `{transition_tool}`. Decodex already records the run-start Linear ledger, so do not leave a separate start comment.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n- Keep discovery bounded to the minimal implementation files needed for this issue; defer broader docs or upstream reading unless a concrete ambiguity blocks the change.\n- Implement the fix in the current worktree.\n{decodex_review_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- Run the repository validation needed to justify a reviewable PR.\n- Commit the lane, push branch `{branch}`, and create or update a non-draft PR titled `{pr_title}` for that branch.\n{completion_guidance}- If the issue needs manual attention, add label `{needs_attention}` with `{label_tool}`, call `issue_comment` with kind `manual_attention` and structured public fields, and then call `{terminal_finalize_tool}` with path `manual_attention`. Do not call `{review_handoff_tool}` in that case; `decodex` will stop the lane as a human-required failure without automatic retry.\n- Do not move the issue directly to `{success}` with `{transition_tool}`; `decodex` will finish that writeback after its own validation passes.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}", + "Resolve Linear issue {identifier}: {title}\n\nDescription:\n{description}\n\n{recovery_context}Execution checklist:\n- Move the issue to `{in_progress}` with `{transition_tool}`. Decodex already records the run-start Linear ledger, so do not leave a separate start comment.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n- Keep discovery bounded to the minimal implementation files needed for this issue; defer broader docs or upstream reading unless a concrete ambiguity blocks the change.\n- Implement the fix in the current worktree.\n{decodex_review_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- Run the repository validation needed to justify a reviewable PR.\n- Commit the lane, push branch `{branch}`, and create or update a non-draft PR titled `{pr_title}` for that branch.\n{completion_guidance}- {manual_attention_guidance}\n- Do not move the issue directly to `{success}` with `{transition_tool}`; `decodex` will finish that writeback after its own validation passes.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}", identifier = issue.identifier, title = issue.title, description = description, recovery_context = recovery_context, transition_tool = ISSUE_TRANSITION_TOOL_NAME, - label_tool = ISSUE_LABEL_ADD_TOOL_NAME, progress_checkpoint_tool = ISSUE_PROGRESS_CHECKPOINT_TOOL_NAME, - review_handoff_tool = ISSUE_REVIEW_HANDOFF_TOOL_NAME, terminal_finalize_tool = ISSUE_TERMINAL_FINALIZE_TOOL_NAME, in_progress = workflow.frontmatter().tracker().in_progress_state(), branch = issue_run.worktree.branch_name, success = workflow.frontmatter().tracker().success_state(), - needs_attention = workflow.frontmatter().tracker().needs_attention_label(), + manual_attention_guidance = handoff_manual_attention_guidance, continuation_guidance = continuation_guidance, pr_title = review_pull_request_title(issue), decodex_review_guidance = build_handoff_review_guidance( @@ -318,13 +355,27 @@ fn build_continuation_user_input( .frontmatter() .tracker() .resolved_completed_state(); + let needs_attention = workflow.frontmatter().tracker().needs_attention_label(); + let repair_manual_attention_guidance = build_manual_attention_guidance( + needs_attention, + ISSUE_LABEL_ADD_TOOL_NAME, + ISSUE_TERMINAL_FINALIZE_TOOL_NAME, + None, + ); + let handoff_manual_attention_guidance = build_manual_attention_guidance( + needs_attention, + ISSUE_LABEL_ADD_TOOL_NAME, + ISSUE_TERMINAL_FINALIZE_TOOL_NAME, + Some(ISSUE_REVIEW_HANDOFF_TOOL_NAME), + ); match dispatch_mode { IssueDispatchMode::ReviewRepair => format!( - "Continue retained review repair for Linear issue {identifier} in the current thread and worktree.\n\nContinuation checklist:\n- Resume from the current repository state and outstanding review feedback or retained landing fallback on `{pr_url}`.\n- Keep changes scoped to the same retained review lane and do not move the issue out of `{success}`.\n{decodex_review_guidance}- Validate each actionable review claim against the codebase, tests, and requirements before changing code, and keep pushback or clarification threads open until the repaired head is ready.\n- If the blocker is landing fallback, repair only the branch sync, conflict, ambiguous mergeability, or repository-specific recovery issue; do not merge or land the PR yourself.\n- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- If the repaired head is ready, push it.\n{github_review_guidance}- After the repaired head is pushed, reply in-thread for every addressed comment and resolve only the GitHub review threads whose fixes landed and verified on the repaired head.\n{completion_guidance}- If the issue requires manual attention, add the needs-attention label, call `issue_comment` with kind `manual_attention` and structured public fields, then finalize path `manual_attention` before ending the turn.\n- If more work still remains after this turn, you may end the turn without terminal finalization and Decodex will decide whether to continue.", + "Continue retained review repair for Linear issue {identifier} in the current thread and worktree.\n\nContinuation checklist:\n- Resume from the current repository state and outstanding review feedback or retained landing fallback on `{pr_url}`.\n- Keep changes scoped to the same retained review lane and do not move the issue out of `{success}`.\n{decodex_review_guidance}- Validate each actionable review claim against the codebase, tests, and requirements before changing code, and keep pushback or clarification threads open until the repaired head is ready.\n- If the blocker is landing fallback, repair only the branch sync, conflict, ambiguous mergeability, or repository-specific recovery issue; do not merge or land the PR yourself.\n- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- If the repaired head is ready, push it.\n{github_review_guidance}- After the repaired head is pushed, reply in-thread for every addressed comment and resolve only the GitHub review threads whose fixes landed and verified on the repaired head.\n{completion_guidance}- {manual_attention_guidance}\n- If more work still remains after this turn, you may end the turn without terminal finalization and Decodex will decide whether to continue.", identifier = issue.identifier, pr_url = recorded_pr_url.unwrap_or("(missing review handoff marker)"), success = success_state, + manual_attention_guidance = repair_manual_attention_guidance, github_review_guidance = build_repair_github_review_guidance(review_level, ISSUE_REVIEW_REPAIR_COMPLETE_TOOL_NAME), decodex_review_guidance = build_repair_continuation_review_guidance( review_level @@ -334,7 +385,7 @@ fn build_continuation_user_input( ), ), IssueDispatchMode::Closeout => format!( - "Continue retained closeout for Linear issue {identifier} in the current thread and worktree.\n\nContinuation checklist:\n- Resume from the current repository state and merged PR lineage on `{pr_url}`.\n- Keep changes scoped to the same retained post-review lane. Do not move the issue back to implementation; the tracker may already be in `{completed}` while closeout or cleanup remains pending.\n- Treat this resumed closeout as a short deterministic tail. Reuse the existing merged PR evidence instead of restarting broad discovery, and only rerun the minimum validation needed to justify `Done` plus cleanup.\n- If you record `{progress_checkpoint_tool}` during closeout, either omit `head_sha` or pass the exact full current `HEAD` SHA.\n- Merge is already authoritative for `{pr_url}` before this run starts. Do not land, merge, or request review from this closeout run.\n- If the issue is still in `{success}`, transition it once to `{completed}` with `{transition_tool}` before `{closeout_tool}`.\n- If Linear closeout is complete, call `{closeout_tool}` and then call `{terminal_finalize_tool}` with path `closeout`.\n- Do not end the turn without either `{closeout_tool}` plus `{terminal_finalize_tool}`, or the manual-attention path.\n- If the issue requires manual attention, add the needs-attention label, call `issue_comment` with kind `manual_attention` and structured public fields, then finalize path `manual_attention` before ending the turn.", + "Continue retained closeout for Linear issue {identifier} in the current thread and worktree.\n\nContinuation checklist:\n- Resume from the current repository state and merged PR lineage on `{pr_url}`.\n- Keep changes scoped to the same retained post-review lane. Do not move the issue back to implementation; the tracker may already be in `{completed}` while closeout or cleanup remains pending.\n- Treat this resumed closeout as a short deterministic tail. Reuse the existing merged PR evidence instead of restarting broad discovery, and only rerun the minimum validation needed to justify `Done` plus cleanup.\n- If you record `{progress_checkpoint_tool}` during closeout, either omit `head_sha` or pass the exact full current `HEAD` SHA.\n- Merge is already authoritative for `{pr_url}` before this run starts. Do not land, merge, or request review from this closeout run.\n- If the issue is still in `{success}`, transition it once to `{completed}` with `{transition_tool}` before `{closeout_tool}`.\n- If Linear closeout is complete, call `{closeout_tool}` and then call `{terminal_finalize_tool}` with path `closeout`.\n- Do not end the turn without either `{closeout_tool}` plus `{terminal_finalize_tool}`, or the manual-attention path.\n- {manual_attention_guidance}", identifier = issue.identifier, pr_url = recorded_pr_url.unwrap_or("(missing review handoff marker)"), progress_checkpoint_tool = ISSUE_PROGRESS_CHECKPOINT_TOOL_NAME, @@ -343,10 +394,12 @@ fn build_continuation_user_input( completed = completed_state, closeout_tool = ISSUE_DELIVERY_CLOSEOUT_COMPLETE_TOOL_NAME, terminal_finalize_tool = ISSUE_TERMINAL_FINALIZE_TOOL_NAME, + manual_attention_guidance = repair_manual_attention_guidance, ), _ => format!( - "Continue working on Linear issue {identifier} in the current thread and worktree.\n\nContinuation checklist:\n- Resume from the current repository state instead of restarting broad discovery.\n- Keep changes scoped to the same issue lane.\n{decodex_review_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n{completion_guidance}- If the issue requires manual attention, add the needs-attention label, call `issue_comment` with kind `manual_attention` and structured public fields, then finalize path `manual_attention` before ending the turn.\n- If more work still remains after this turn, you may end the turn without terminal finalization and Decodex will decide whether to continue.", + "Continue working on Linear issue {identifier} in the current thread and worktree.\n\nContinuation checklist:\n- Resume from the current repository state instead of restarting broad discovery.\n- Keep changes scoped to the same issue lane.\n{decodex_review_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n{completion_guidance}- {manual_attention_guidance}\n- If more work still remains after this turn, you may end the turn without terminal finalization and Decodex will decide whether to continue.", identifier = issue.identifier, + manual_attention_guidance = handoff_manual_attention_guidance, decodex_review_guidance = build_handoff_continuation_review_guidance( review_level ), diff --git a/apps/decodex/src/orchestrator/run_cycle.rs b/apps/decodex/src/orchestrator/run_cycle.rs index b1d00a87f..4a7101d85 100644 --- a/apps/decodex/src/orchestrator/run_cycle.rs +++ b/apps/decodex/src/orchestrator/run_cycle.rs @@ -243,6 +243,15 @@ where review_state_inspector, )? { RetainedReviewLaneLoad::Skip => continue, + RetainedReviewLaneLoad::Wait(reason) => { + tracing::info!( + project_id = project.service_id(), + reason = reason.as_str(), + "Retained post-review orchestration is waiting for transient readback recovery." + ); + + continue; + }, RetainedReviewLaneLoad::Blocked(blocked) => { apply_passive_retained_manual_attention_with_run_identity( PassiveRetainedAttentionRuntime { tracker, project, workflow, state_store }, @@ -330,12 +339,9 @@ where let local_branch_name = match worktree_checkout_branch_name(worktree.worktree_path()) { Ok(local_branch_name) => local_branch_name, Err(_error) => { - return Ok(blocked_retained_review_lane( - issue, - worktree, - Some(&review_handoff), + return Ok(RetainedReviewLaneLoad::Wait(String::from( "worktree_checkout_branch_read_failed", - )); + ))); }, }; let Some(local_branch_name) = local_branch_name else { @@ -349,12 +355,9 @@ where let local_head_oid = match worktree_head_oid(worktree.worktree_path()) { Ok(local_head_oid) => local_head_oid, Err(_error) => { - return Ok(blocked_retained_review_lane( - issue, - worktree, - Some(&review_handoff), + return Ok(RetainedReviewLaneLoad::Wait(String::from( "worktree_head_read_failed", - )); + ))); }, }; let Some(local_head_oid) = local_head_oid else { @@ -410,7 +413,7 @@ where { let review_state = match load_post_review_lane_review_state(snapshot, review_state_inspector)? { PostReviewLaneStateLoad::Classification(classification) => - return Ok(RetainedReviewLaneReviewLoad::Blocked(classification.reason)), + return Ok(retained_review_lane_review_load_from_classification(classification)), PostReviewLaneStateLoad::ReviewState(review_state) => Box::new(review_state), }; @@ -431,6 +434,16 @@ where Ok(RetainedReviewLaneReviewLoad::ReviewState(review_state)) } +fn retained_review_lane_review_load_from_classification( + classification: PostReviewLaneClassification, +) -> RetainedReviewLaneReviewLoad { + if classification.decision == PostReviewLaneDecision::Block { + RetainedReviewLaneReviewLoad::Blocked(classification.reason) + } else { + RetainedReviewLaneReviewLoad::Skip + } +} + fn blocked_retained_review_lane( issue: TrackerIssue, worktree: WorktreeMapping, @@ -491,9 +504,7 @@ where match phase { ReviewOrchestrationPhase::RequestPending => handle_request_pending_phase( - tracker, project, - workflow, state_store, lane, github_token, @@ -608,16 +619,12 @@ where ) } -fn handle_request_pending_phase( - tracker: &T, +fn handle_request_pending_phase( project: &ServiceConfig, - workflow: &WorkflowDocument, state_store: &StateStore, lane: &RetainedReviewLane, github_token: &mut Option, ) -> Result<()> -where - T: IssueTracker, { match external_review_request_ci_gate(&lane.review_state) { ExternalReviewRequestCiGate::Ready => {}, @@ -630,15 +637,6 @@ where RetainedReviewOrchestrationMarkerFields::from_marker(&lane.orchestration_marker), ); }, - ExternalReviewRequestCiGate::ManualAttention(reason) => { - return apply_passive_retained_manual_attention( - PassiveRetainedAttentionRuntime { tracker, project, workflow, state_store }, - &lane.snapshot.issue, - &lane.snapshot.worktree, - &lane.orchestration_marker, - reason, - ); - }, } let github_token = retained_review_github_token(project, github_token)?; @@ -1048,6 +1046,40 @@ fn ensure_review_orchestration_marker( if let Some(marker) = state_store.review_orchestration_marker(project_id, &issue.id, review_handoff)? { + if marker.branch_name() == review_handoff.branch_name() + && marker.pr_url() == review_handoff.pr_url() + && marker.head_sha() != local_head_oid + { + let rebound_marker = ReviewOrchestrationMarker::new( + marker.run_id().to_owned(), + marker.attempt_number(), + review_handoff.branch_name().to_owned(), + review_handoff.pr_url().to_owned(), + local_head_oid.to_owned(), + ReviewOrchestrationPhase::RequestPending.as_str(), + None, + None, + None, + 0, + marker.external_round_count(), + None, + ); + + state_store.upsert_review_orchestration_marker(project_id, &issue.id, &rebound_marker)?; + + tracing::info!( + service_id = project_id, + issue_id = issue.id.as_str(), + branch = review_handoff.branch_name(), + pr_url = review_handoff.pr_url(), + old_head_sha = marker.head_sha(), + new_head_sha = local_head_oid, + "Rebound stale retained review orchestration marker to current PR head." + ); + + return Ok(rebound_marker); + } + return Ok(marker); } @@ -2232,12 +2264,13 @@ where else { return Ok(None); }; - let retry_budget_base = - context.preferred_retry_budget_base.unwrap_or(0).max(retry_budget_base_for_issue_worktree( - context.state_store, - &issue.id, - &planned_worktree.path, - )?); + let retry_budget_base = retry_budget_base_for_dispatch_mode( + context.state_store, + &issue.id, + &planned_worktree.path, + context.dispatch_mode, + context.preferred_retry_budget_base, + )?; let lease_issue_id = issue.id.clone(); let issue_state = planned_issue_state_for_dispatch( context.workflow, diff --git a/apps/decodex/src/orchestrator/selection.rs b/apps/decodex/src/orchestrator/selection.rs index a290a0de4..a2efacdf0 100644 --- a/apps/decodex/src/orchestrator/selection.rs +++ b/apps/decodex/src/orchestrator/selection.rs @@ -128,6 +128,11 @@ fn format_retry_comment(comment: RetryComment<'_>) -> String { } fn retry_comment_details(error: &Report) -> (&'static str, String) { + debug_assert!( + !run_failure_writeback_disposition(error).requires_terminal_attention(), + "terminal-attention failures must not be formatted as retry comments" + ); + if let Some(repo_gate_failure) = error.downcast_ref::() { match repo_gate_failure.disposition() { RepoGateFailureDisposition::ContinueRepair @@ -140,6 +145,43 @@ fn retry_comment_details(error: &Report) -> (&'static str, String) { RepoGateFailureDisposition::NeedsHumanAttention => {}, } } + if let Some(app_server_failure) = error.downcast_ref::() { + return (app_server_failure.error_class(), app_server_failure.retry_next_action()); + } + if let Some(app_server_failure) = + error.downcast_ref::() + && app_server_failure.is_retryable_timeout() + { + return (app_server_failure.error_class(), app_server_failure.retry_next_action()); + } + if let Some(app_server_failure) = error.downcast_ref::() + && app_server_failure.is_retryable_startup() + { + return (app_server_failure.error_class(), app_server_failure.retry_next_action()); + } + if let Some(app_server_failure) = error.downcast_ref::() + && app_server_failure.is_terminal_path_missing() + { + return (app_server_failure.error_class(), app_server_failure.retry_next_action()); + } + if let Some(app_server_failure) = error.downcast_ref::() { + return (app_server_failure.error_class(), app_server_failure.retry_next_action()); + } + + if error.downcast_ref::().is_some() { + return ( + "stalled_run_detected", + String::from( + "decodex will retry the stalled lane automatically; inspect the worktree and app-server activity if the retry budget exhausts", + ), + ); + } + + if let Some(app_server_failure) = error.downcast_ref::() + && app_server_failure.is_retryable_capacity_failure() + { + return (app_server_failure.error_class(), app_server_failure.retry_next_action().to_owned()); + } ("retryable_execution_failure", String::from("decodex will retry automatically")) } diff --git a/apps/decodex/src/orchestrator/status.rs b/apps/decodex/src/orchestrator/status.rs index 9dc6b6150..257496d07 100644 --- a/apps/decodex/src/orchestrator/status.rs +++ b/apps/decodex/src/orchestrator/status.rs @@ -39,7 +39,6 @@ enum ExternalReviewRequestCiGate { Ready, WaitForGreenChecks, RepairRequired, - ManualAttention(&'static str), } #[derive(Clone, Copy)] @@ -1510,15 +1509,24 @@ fn history_lane_has_current_attention_signal( snapshot: &OperatorStatusSnapshot, lane: &OperatorHistoryLaneStatus, ) -> bool { - if lane.active_label_present == Some(true) || lane.needs_attention_label_present == Some(true) { + if lane.needs_attention_label_present == Some(true) { return true; } let issue_key = history_lane_group_key(lane); + let has_non_attention_post_review_owner = + history_lane_has_current_non_attention_post_review_owner(snapshot, &issue_key); + + if lane.active_label_present == Some(true) && !has_non_attention_post_review_owner { + return true; + } snapshot.worktrees.iter().any(|worktree| { - operator_issue_attention_key(&worktree.issue_id, worktree.issue_identifier.as_deref()) - == issue_key + !has_non_attention_post_review_owner + && operator_issue_attention_key( + &worktree.issue_id, + worktree.issue_identifier.as_deref(), + ) == issue_key }) || snapshot.post_review_lanes.iter().any(|post_review_lane| { post_review_lane_counts_as_attention(post_review_lane) && operator_issue_attention_key( @@ -1532,6 +1540,19 @@ fn history_lane_has_current_attention_signal( }) } +fn history_lane_has_current_non_attention_post_review_owner( + snapshot: &OperatorStatusSnapshot, + issue_key: &str, +) -> bool { + snapshot.post_review_lanes.iter().any(|post_review_lane| { + !post_review_lane_counts_as_attention(post_review_lane) + && operator_issue_attention_key( + &post_review_lane.issue_id, + Some(&post_review_lane.issue_identifier), + ) == issue_key + }) +} + fn history_lane_attention_is_resolved_tracker_echo( snapshot: &OperatorStatusSnapshot, lane: &OperatorHistoryLaneStatus, @@ -4178,9 +4199,6 @@ fn apply_review_orchestration_phase_classification( classification.reason = String::from("external_review_request_ci_red_repair_required"); }, - ExternalReviewRequestCiGate::ManualAttention(reason) => { - *classification = blocked_post_review_lane_from_state(review_state, reason); - }, } }, ReviewOrchestrationPhase::WaitingForAck => { @@ -4833,17 +4851,8 @@ fn external_review_request_ci_gate( match review_state.status_check_rollup_state.as_deref() { None | Some("SUCCESS") => ExternalReviewRequestCiGate::Ready, Some("EXPECTED" | "PENDING") => ExternalReviewRequestCiGate::WaitForGreenChecks, - Some("ERROR" | "FAILURE") - if failed_checks_require_repair( - review_state.status_check_rollup_state.as_deref(), - &review_state.merge_state_status, - ) => - { - ExternalReviewRequestCiGate::RepairRequired - }, - Some("ERROR" | "FAILURE") | Some(_) => ExternalReviewRequestCiGate::ManualAttention( - "external_review_request_ci_red_manual_attention", - ), + Some("ERROR" | "FAILURE") => ExternalReviewRequestCiGate::RepairRequired, + Some(_) => ExternalReviewRequestCiGate::WaitForGreenChecks, } } diff --git a/apps/decodex/src/orchestrator/tests.rs b/apps/decodex/src/orchestrator/tests.rs index 7e42ed611..6dd8f9a84 100644 --- a/apps/decodex/src/orchestrator/tests.rs +++ b/apps/decodex/src/orchestrator/tests.rs @@ -24,7 +24,8 @@ use crate::{orchestrator::RepoGatePhaseGoalController, tracker::records}; use crate::agent::{ ACTIVE_RUN_IDLE_TIMEOUT, MODEL_EXECUTION_IDLE_TIMEOUT, AppServerCapabilityPreflightFailure, - AppServerHomePreflightFailure, AppServerTransportFailure, AppServerTurnFailure, + AppServerDynamicToolFailure, AppServerHomePreflightFailure, AppServerPhaseGoalFailure, + AppServerTransportFailure, AppServerTurnFailure, DynamicToolHandler, PhaseGoalController, PhaseGoalKind, PhaseGoalSpec, PhaseGoalTransition, ReviewPolicyStopReason, ReviewPolicyStopRequested, TrackerToolBridge, TurnContinuationGuard, diff --git a/apps/decodex/src/orchestrator/tests/intake/prepare_issue_run.rs b/apps/decodex/src/orchestrator/tests/intake/prepare_issue_run.rs index 123e5d046..877d897c4 100644 --- a/apps/decodex/src/orchestrator/tests/intake/prepare_issue_run.rs +++ b/apps/decodex/src/orchestrator/tests/intake/prepare_issue_run.rs @@ -139,7 +139,7 @@ Follow the repository policy. } #[test] -fn prepare_issue_run_uses_persisted_retry_budget_marker_after_restart() { +fn prepare_issue_run_starts_fresh_retry_budget_for_normal_queue_intake() { let (_temp_dir, base_config, workflow) = temp_project_layout(); let config = service_config_with_github_token_env_var(&base_config, "HOME"); let issue = sample_issue("Todo", &[]); @@ -174,17 +174,61 @@ fn prepare_issue_run_uses_persisted_retry_budget_marker_after_restart() { .expect("issue preparation should succeed") .expect("startable issue should prepare"); + assert_eq!( + issue_run.retry_budget_base, 0, + "normal queue intake starts a new automatic retry episode instead of inheriting old marker attempts" + ); +} + +#[test] +fn prepare_issue_run_uses_persisted_retry_budget_marker_for_recovered_retry() { + let (_temp_dir, base_config, workflow) = temp_project_layout(); + let config = service_config_with_github_token_env_var(&base_config, "HOME"); + let issue = + sample_issue("In Progress", &[tracker::automation_active_label(TEST_SERVICE_ID).as_str()]); + let tracker = + FakeTracker::with_refresh_snapshots(vec![issue.clone()], vec![vec![issue.clone()]]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let worktree_manager = + WorktreeManager::new(config.service_id(), config.repo_root(), config.worktree_root()); + let worktree = + worktree_manager.ensure_worktree(&issue.identifier, false).expect("worktree should exist"); + + state::write_run_retry_budget_attempt_count(&worktree.path, "older-run", 4, 2) + .expect("retry budget marker should write"); + + let issue_run = orchestrator::prepare_issue_run( + PrepareIssueRunContext { + tracker: &tracker, + project: &config, + workflow: &workflow, + state_store: &state_store, + worktree_manager: &worktree_manager, + dry_run: false, + lease_preacquired: false, + dispatch_mode: IssueDispatchMode::Retry, + preferred_issue_state: None, + preferred_initial_issue_state: None, + preferred_run_identity: None, + preferred_retry_budget_base: None, + }, + issue, + ) + .expect("issue preparation should succeed") + .expect("startable issue should prepare"); + assert_eq!( issue_run.retry_budget_base, 2, - "restart recovery should preserve retry budget from the retained worktree marker" + "recovered retry dispatch should preserve retry budget from the retained worktree marker" ); } #[test] -fn prepare_issue_run_keeps_persisted_retry_budget_when_preferred_base_is_stale() { +fn prepare_issue_run_keeps_persisted_retry_budget_when_preferred_retry_base_is_stale() { let (_temp_dir, base_config, workflow) = temp_project_layout(); let config = service_config_with_github_token_env_var(&base_config, "HOME"); - let issue = sample_issue("Todo", &[]); + let issue = + sample_issue("In Progress", &[tracker::automation_active_label(TEST_SERVICE_ID).as_str()]); let tracker = FakeTracker::with_refresh_snapshots(vec![issue.clone()], vec![vec![issue.clone()]]); let state_store = StateStore::open_in_memory().expect("state store should open"); @@ -205,7 +249,7 @@ fn prepare_issue_run_keeps_persisted_retry_budget_when_preferred_base_is_stale() worktree_manager: &worktree_manager, dry_run: false, lease_preacquired: false, - dispatch_mode: IssueDispatchMode::Normal, + dispatch_mode: IssueDispatchMode::Retry, preferred_issue_state: None, preferred_initial_issue_state: None, preferred_run_identity: None, @@ -214,11 +258,11 @@ fn prepare_issue_run_keeps_persisted_retry_budget_when_preferred_base_is_stale() issue, ) .expect("issue preparation should succeed") - .expect("startable issue should prepare"); + .expect("recovered retry issue should prepare"); assert_eq!( issue_run.retry_budget_base, 2, - "preferred retry-budget base should not hide retained worktree state" + "preferred retry-budget base should not hide retained retry episode state" ); } diff --git a/apps/decodex/src/orchestrator/tests/intake/run_and_prompting.rs b/apps/decodex/src/orchestrator/tests/intake/run_and_prompting.rs index ae7342f12..5dcea1924 100644 --- a/apps/decodex/src/orchestrator/tests/intake/run_and_prompting.rs +++ b/apps/decodex/src/orchestrator/tests/intake/run_and_prompting.rs @@ -16,6 +16,30 @@ impl PromptSurfaces { } } +fn assert_manual_attention_prompt_guidance(prompt: &str, expects_handoff_guard: bool) { + assert!(prompt.contains(&format!( + "request label `decodex:needs-attention` with `{ISSUE_LABEL_ADD_TOOL_NAME}`" + ))); + assert!(prompt.contains("records manual-attention label intent only")); + assert!(prompt.contains( + "Decodex applies the actual label only after that manual_attention comment validates" + )); + assert!(prompt.contains("do not use runtime-owned retry/repair classes")); + assert!(prompt.contains("app-server timeout, transport, turn, dynamic-tool, or usage-limit")); + assert!(prompt.contains("stalled-run detection")); + assert!(prompt.contains("phase-goal terminal-path misses")); + assert!(prompt.contains("repo-gate canonicalize, verify, tracked-rewrite, or git-lock failures")); + assert!(prompt.contains("generic retryable execution failures")); + assert!(!prompt.contains("add label `decodex:needs-attention`")); + assert!(!prompt.contains("add the needs-attention label")); + + if expects_handoff_guard { + assert!(prompt.contains(&format!( + "Do not call `{ISSUE_REVIEW_HANDOFF_TOOL_NAME}` in that case" + ))); + } +} + fn run_and_prompting_service_owned_issue(state_name: &str) -> TrackerIssue { let active_label = tracker::automation_active_label(TEST_SERVICE_ID); @@ -696,6 +720,16 @@ fn developer_instructions_trim_workflow_body_and_preserve_required_guidance() { assert!(!instructions.contains("WORKFLOW.md\n")); } +#[test] +fn normal_prompts_record_manual_attention_label_intent_before_label_application() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let surfaces = build_normal_prompt_surfaces(&config, &workflow); + + for prompt in surfaces.all() { + assert_manual_attention_prompt_guidance(prompt, true); + } +} + #[test] fn review_pull_request_title_normalizes_issue_prefix() { for title in [ @@ -1152,6 +1186,10 @@ fn review_repair_prompts_require_same_pr_repair_completion() { assert!(continuation_input.contains("In Review")); assert!(continuation_input.contains("review_repair")); + for prompt in [&developer_instructions, &user_input, &continuation_input] { + assert_manual_attention_prompt_guidance(prompt, false); + } + assert_prompt_orders_thread_replies_after_push( &developer_instructions, "push the repaired head.", @@ -1567,6 +1605,10 @@ fn closeout_prompts_require_retained_pr_closeout_completion() { "If the issue is still in `In Review`, transition it once to `Done` with `issue_transition` before `issue_closeout_complete`" )); assert!(continuation_input.contains("closeout")); + + for prompt in [&developer_instructions, &user_input, &continuation_input] { + assert_manual_attention_prompt_guidance(prompt, false); + } } #[test] diff --git a/apps/decodex/src/orchestrator/tests/intake/workflow_reload.rs b/apps/decodex/src/orchestrator/tests/intake/workflow_reload.rs index fe5822640..9470d94a7 100644 --- a/apps/decodex/src/orchestrator/tests/intake/workflow_reload.rs +++ b/apps/decodex/src/orchestrator/tests/intake/workflow_reload.rs @@ -181,7 +181,7 @@ fn expected_developer_instructions( )); sections.push(format!( - "Tracker tool contract\n- You own issue-scoped tracker writes for `{issue}`.\n- At the start of execution, call `{transition_tool}` to move the issue to `{in_progress}`. Decodex already records the run-start Linear ledger, so do not add a separate start comment.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n- Self Check: Review your work repeatedly and fix any logic bugs until no new issues are found.\n- Decodex Review: request an independent fresh-context read-only review pass for the actual current diff and branch state. The reviewer must not edit files, push, land, or mutate tracker state.\n- Use the repo-native bounded review method from `WORKFLOW.md`: run the requirements pass and the adversarial reviewer pass against the current `HEAD`, including regression risk, missing tests, docs/config drift, migration fallout, operator-facing fallout, and mismatch with the accepted Loop/Decision Contract.\n- Validate reviewer comments before repair. Record accepted findings separately from rejected or non-actionable comments, fix only the smallest coherent owned batch, rerun verification, and re-read `HEAD` before deciding the normalized review status.\n- Every time the Decodex Review pass produces a result for the current head, call `{review_checkpoint_tool}` with reviewer `independent_fresh_context`, that normalized status, the exact current `HEAD` SHA, concise evidence, checklist notes, and structured accepted/rejected findings.\n- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- When the implementation is ready, commit the lane, push branch `{branch}`, and create or update a non-draft PR titled `{pr_title}` for that branch.\n- Call `{review_handoff_tool}` only after the latest `{review_checkpoint_tool}` for this handoff phase and current `HEAD` is `clean`. Then call `{terminal_finalize_tool}` with path `review_handoff`.\n- If you determine the issue needs human attention, add label `{needs_attention}` with `{label_tool}`, then call `issue_comment` with kind `manual_attention` and structured public fields (`error_class`, `next_action`, `blockers`, `evidence`; include `failed_command` and `raw_error` only when public-safe), and then call `{terminal_finalize_tool}` with path `manual_attention`. Do not speculate about capabilities you did not directly verify. Do not call `{review_handoff_tool}` in that case; `decodex` will stop the lane as a human-required failure without automatic retry.\n- Do not move the issue directly to `{success}` with `{transition_tool}`. `decodex` will complete the success writeback only after its own validation passes.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}\n- Never write to any other issue.", + "Tracker tool contract\n- You own issue-scoped tracker writes for `{issue}`.\n- At the start of execution, call `{transition_tool}` to move the issue to `{in_progress}`. Decodex already records the run-start Linear ledger, so do not add a separate start comment.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n- Self Check: Review your work repeatedly and fix any logic bugs until no new issues are found.\n- Decodex Review: request an independent fresh-context read-only review pass for the actual current diff and branch state. The reviewer must not edit files, push, land, or mutate tracker state.\n- Use the repo-native bounded review method from `WORKFLOW.md`: run the requirements pass and the adversarial reviewer pass against the current `HEAD`, including regression risk, missing tests, docs/config drift, migration fallout, operator-facing fallout, and mismatch with the accepted Loop/Decision Contract.\n- Validate reviewer comments before repair. Record accepted findings separately from rejected or non-actionable comments, fix only the smallest coherent owned batch, rerun verification, and re-read `HEAD` before deciding the normalized review status.\n- Every time the Decodex Review pass produces a result for the current head, call `{review_checkpoint_tool}` with reviewer `independent_fresh_context`, that normalized status, the exact current `HEAD` SHA, concise evidence, checklist notes, and structured accepted/rejected findings.\n- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- When the implementation is ready, commit the lane, push branch `{branch}`, and create or update a non-draft PR titled `{pr_title}` for that branch.\n- Call `{review_handoff_tool}` only after the latest `{review_checkpoint_tool}` for this handoff phase and current `HEAD` is `clean`. Then call `{terminal_finalize_tool}` with path `review_handoff`.\n- If you determine the issue needs human attention, request label `{needs_attention}` with `{label_tool}`; that records manual-attention label intent only. Then call `issue_comment` with kind `manual_attention` and structured public fields (`error_class`, `next_action`, `blockers`, `evidence`; include `failed_command` and `raw_error` only when public-safe). Decodex applies the actual label only after that manual_attention comment validates. Use a human-owned blocker class; do not use runtime-owned retry/repair classes such as app-server timeout, transport, turn, dynamic-tool, or usage-limit failures; stalled-run detection; phase-goal terminal-path misses; repo-gate canonicalize, verify, tracked-rewrite, or git-lock failures; or generic retryable execution failures. Then call `{terminal_finalize_tool}` with path `manual_attention`. Do not speculate about capabilities you did not directly verify. Do not call `{review_handoff_tool}` in that case; `decodex` will stop the lane as a human-required failure without automatic retry.\n- Do not move the issue directly to `{success}` with `{transition_tool}`. `decodex` will complete the success writeback only after its own validation passes.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}\n- Never write to any other issue.", issue = issue_run.issue.identifier, transition_tool = ISSUE_TRANSITION_TOOL_NAME, label_tool = ISSUE_LABEL_ADD_TOOL_NAME, diff --git a/apps/decodex/src/orchestrator/tests/operator/status/history.rs b/apps/decodex/src/orchestrator/tests/operator/status/history.rs index 9aa5a3069..bbca4200a 100644 --- a/apps/decodex/src/orchestrator/tests/operator/status/history.rs +++ b/apps/decodex/src/orchestrator/tests/operator/status/history.rs @@ -629,6 +629,128 @@ fn live_status_counts_terminal_attention_when_current_attention_label_remains() assert!(rendered.contains("History-only terminal attention: 0")); } +#[test] +fn live_status_treats_adopted_ready_to_land_history_attention_as_history_only() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let active_label = tracker::automation_active_label(TEST_SERVICE_ID); + let queue_label = tracker::automation_queue_label(TEST_SERVICE_ID); + let pr_url = "https://github.com/hack-ink/decodex/pull/360"; + let mut issue = sample_issue_with_sort_fields( + "issue-xy-948", + "XY-948", + "In Review", + &[active_label.as_str()], + Some(3), + "2026-06-12T04:20:00Z", + ); + + issue.labels.retain(|label| label.name != queue_label); + + let tracker = FakeTracker::new(vec![issue.clone()]); + + tracker.issue_comments.borrow_mut().insert( + issue.id.clone(), + vec![linear_execution_history_comment( + &issue, + "needs_attention", + "2026-06-12T04:30:00Z", + "manual-attention", + |record| { + record.branch = Some(String::from("y/decodex-xy-948")); + record.worktree_path = Some(String::from(".worktrees/XY-948")); + record.summary = Some(String::from( + "Decodex retained validation-ready partial progress for manual review.", + )); + record.error_class = Some(String::from("partial_progress_retained")); + record.next_action = Some(String::from( + "review the retained worktree diff, then commit/push/PR or mark manual disposition", + )); + record.blockers = Some(vec![String::from( + "lane stopped before review handoff and terminal finalize", + )]); + record.evidence = Some(vec![String::from("cargo make test passed")]); + record.terminal_path = Some(String::from("retained_partial_progress")); + }, + )], + ); + + state_store + .upsert_worktree( + TEST_SERVICE_ID, + &issue.id, + "y/decodex-xy-948", + &config.worktree_root().join(&issue.identifier).display().to_string(), + ) + .expect("worktree should remember previous lane ownership"); + state_store + .record_run_attempt("xy-948-attempt-1-1781248200", &issue.id, 1, "failed") + .expect("failed attempt should record"); + + let mut snapshot = orchestrator::build_live_operator_status_snapshot( + &tracker, + &config, + &workflow, + &state_store, + 10, + ) + .expect("snapshot should build"); + + assert_eq!( + snapshot.projects[0].attention_count, 1, + "active label plus retained history should reproduce the pre-adoption current attention signal" + ); + assert_eq!(snapshot.history_lanes[0].active_label_present, Some(true)); + assert_eq!(snapshot.history_lanes[0].needs_attention_label_present, Some(false)); + assert!( + snapshot + .queued_candidates + .iter() + .all(|candidate| candidate.issue_identifier != "XY-948"), + "the regression should isolate retained history plus post-review ownership, not queue attention" + ); + + let worktree_path = snapshot.worktrees[0].worktree_path.clone(); + + snapshot.post_review_lanes = vec![orchestrator::OperatorPostReviewLaneStatus { + project_id: TEST_SERVICE_ID.to_owned(), + issue_id: issue.id.clone(), + issue_identifier: issue.identifier.clone(), + issue_state: issue.state.name.clone(), + branch_name: String::from("y/decodex-xy-948"), + worktree_path, + classification: String::from("ready_to_land"), + reason: String::from("non_github_review_ready_to_land"), + pr_url: Some(String::from(pr_url)), + pr_head_sha: Some(String::from("1111111111111111111111111111111111111111")), + pr_state: Some(String::from("OPEN")), + review_decision: Some(String::from("APPROVED")), + mergeable: Some(String::from("MERGEABLE")), + check_state: Some(String::from("SUCCESS")), + unresolved_review_threads: Some(0), + readback_warning: None, + readback_root_cause: None, + loop_status: None, + }]; + + let completed_state = workflow.frontmatter().tracker().resolved_completed_state(); + + orchestrator::refresh_worktree_ownership(&mut snapshot, Some(completed_state)); + orchestrator::refresh_operator_project_summary(&mut snapshot, Some(completed_state)); + + let rendered = orchestrator::render_operator_status(&snapshot); + + assert_eq!(snapshot.projects[0].attention_count, 0); + assert_eq!(snapshot.projects[0].post_review_lane_count, 1); + assert_eq!(snapshot.projects[0].retained_worktree_count, 1); + assert_eq!(snapshot.worktrees[0].ownership, "post_review_lane"); + assert_eq!(snapshot.post_review_lanes[0].classification, "ready_to_land"); + assert!(rendered.contains("Current attention: 0")); + assert!(rendered.contains("History-only terminal attention: 1")); + assert!(rendered.contains("classification: ready_to_land")); + assert!(rendered.contains("outcome: needs_attention")); +} + #[test] fn live_status_does_not_count_done_history_attention_without_retained_ownership() { let (_temp_dir, config, workflow) = temp_project_layout(); diff --git a/apps/decodex/src/orchestrator/tests/recovery/reconciliation.rs b/apps/decodex/src/orchestrator/tests/recovery/reconciliation.rs index 90717ad89..8c6578f0b 100644 --- a/apps/decodex/src/orchestrator/tests/recovery/reconciliation.rs +++ b/apps/decodex/src/orchestrator/tests/recovery/reconciliation.rs @@ -647,7 +647,7 @@ fn active_daemon_child_reconciliation_treats_completed_retained_handoff_as_succe #[test] fn stalled_idle_duration_ignores_future_last_activity() { let state_store = StateStore::open_in_memory().expect("state store should open"); - let issue = sample_issue("In Progress", &[]); + let issue = reconciliation_sample_service_owned_issue("In Progress"); let run_id = "run-future-activity"; state_store @@ -999,7 +999,7 @@ fn active_run_reconciliation_keeps_nonterminal_nonactive_worktrees() { } #[test] -fn stalled_run_reconciliation_routes_to_needs_attention_without_cleanup() { +fn stalled_run_reconciliation_schedules_retry_before_attention_budget_exhaustion() { let (_temp_dir, config, workflow) = temp_project_layout(); let tracker = FakeTracker::new(vec![]); let state_store = StateStore::open_in_memory().expect("state store should open"); @@ -1009,6 +1009,8 @@ fn stalled_run_reconciliation_routes_to_needs_attention_without_cleanup() { let run_id = "run-stalled"; let worktree_path = config.worktree_root().join("PUB-101"); + fs::create_dir_all(&worktree_path).expect("worktree path should exist"); + state_store .record_run_attempt(run_id, &issue.id, 1, "running") .expect("run attempt should record"); @@ -1063,33 +1065,115 @@ fn stalled_run_reconciliation_routes_to_needs_attention_without_cleanup() { .status(), "stalled" ); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(tracker.label_removals.borrow().is_empty()); assert!(tracker.comments.borrow().iter().any(|comment| { - comment.contains("stalled_run_detected") - && comment.contains("needs attention") - && comment.contains("clear label `decodex:needs-attention`") + comment.contains("decodex run failed and will retry") + && comment.contains("stalled_run_detected") + && comment.contains("retry the stalled lane automatically") })); assert!( tracker .comments .borrow() .iter() - .all(|comment| !comment.contains("retained partial progress")) + .all(|comment| !comment.contains("clear label `decodex:needs-attention`")) + ); + assert!( + tracker + .comments + .borrow() + .iter() + .all(|comment| records::parse_linear_execution_event_record(comment).is_none()) + ); + + let marker = state::read_run_activity_marker_snapshot(&worktree_path) + .expect("retry marker should load") + .expect("retry marker should exist"); + + assert_eq!(marker.retry_kind(), Some("failure")); + assert!(marker.retry_ready_at_unix_epoch().is_some_and( + |retry_ready_at| retry_ready_at > OffsetDateTime::now_utc().unix_timestamp() + )); +} + +#[test] +fn stalled_run_reconciliation_routes_to_needs_attention_after_retry_budget_exhaustion() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let tracker = FakeTracker::new(vec![]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let worktree_manager = + WorktreeManager::new("pubfi", config.repo_root(), config.worktree_root()); + let issue = reconciliation_sample_service_owned_issue("In Progress"); + let run_id = "run-stalled-exhausted"; + let worktree_path = config.worktree_root().join("PUB-101"); + + fs::create_dir_all(&worktree_path).expect("worktree path should exist"); + state::write_run_retry_budget_attempt_count(&worktree_path, "older-run", 2, 3) + .expect("retry budget marker should write"); + + state_store + .record_run_attempt(run_id, &issue.id, 3, "running") + .expect("run attempt should record"); + state_store + .upsert_lease("pubfi", &issue.id, run_id, "In Progress") + .expect("lease should record"); + state_store + .upsert_worktree( + "pubfi", + &issue.id, + "x/pubfi-pub-101", + &worktree_path.display().to_string(), + ) + .expect("worktree mapping should record"); + + let action = ActiveRunReconciliation { + issue: issue.clone(), + run_attempt: state_store + .run_attempt(run_id) + .expect("run attempt query should succeed") + .expect("run attempt should exist"), + worktree_mapping: state_store + .worktree_for_issue(&issue.id) + .expect("worktree query should succeed"), + disposition: ActiveRunDisposition::Stalled { + idle_for: ACTIVE_RUN_IDLE_TIMEOUT + Duration::from_secs(1), + }, + workflow: workflow.clone(), + }; + + orchestrator::apply_active_run_reconciliation( + &tracker, + &config, + &state_store, + &worktree_manager, + vec![action], + ) + .expect("reconciliation should succeed"); + + assert_eq!( + tracker.label_additions.borrow().as_slice(), + [(issue.id.clone(), vec![String::from("label-needs-attention")])] ); + assert_eq!( + tracker.label_removals.borrow().as_slice(), + [(issue.id.clone(), vec![String::from("label-active")])] + ); + assert!(tracker.comments.borrow().iter().any(|comment| { + comment.contains("stalled_run_detected") + && comment.contains("needs attention") + && comment.contains("clear label `decodex:needs-attention`") + })); let ledger_event = tracker .comments .borrow() .iter() .find_map(|comment| records::parse_linear_execution_event_record(comment)) - .expect("stalled no-progress run should write a Linear execution event"); + .expect("exhausted stalled run should write a Linear execution event"); assert_eq!(ledger_event.event_type, "terminal_failure"); assert_eq!(ledger_event.error_class.as_deref(), Some("stalled_run_detected")); - assert_eq!(ledger_event.terminal_path.as_deref(), None); - assert_eq!( - ledger_event.summary.as_deref(), - Some("Decodex run failed and needs attention.") - ); } #[test] @@ -1230,7 +1314,7 @@ fn assert_dirty_stalled_retained_progress_comments(comments: &[String]) { } #[test] -fn project_reconciliation_routes_orphaned_active_worktree_run_to_needs_attention() { +fn project_reconciliation_schedules_retry_for_orphaned_active_worktree_run() { let (_temp_dir, config, workflow) = temp_project_layout(); let issue = reconciliation_sample_service_owned_issue("In Progress"); let tracker = FakeTracker::new(vec![issue.clone()]); @@ -1281,19 +1365,20 @@ fn project_reconciliation_routes_orphaned_active_worktree_run_to_needs_attention .status(), "stalled" ); - assert_eq!( - tracker.label_additions.borrow().as_slice(), - [(issue.id.clone(), vec![String::from("label-needs-attention")])] - ); - assert_eq!( - tracker.label_removals.borrow().as_slice(), - [(issue.id.clone(), vec![String::from("label-active")])] - ); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(tracker.label_removals.borrow().is_empty()); assert!(tracker.comments.borrow().iter().any(|comment| { comment.contains("stalled_run_detected") - && comment.contains("needs attention") + && comment.contains("decodex run failed and will retry") && comment.contains("run-orphaned-active") })); + + let marker = state::read_run_activity_marker_snapshot(&worktree_path) + .expect("retry marker should load") + .expect("retry marker should exist"); + + assert_eq!(marker.retry_kind(), Some("failure")); + assert!(marker.retry_ready_at_unix_epoch().is_some()); } #[test] diff --git a/apps/decodex/src/orchestrator/tests/recovery/runtime_reentry.rs b/apps/decodex/src/orchestrator/tests/recovery/runtime_reentry.rs index f1d76474c..d4ca19bbc 100644 --- a/apps/decodex/src/orchestrator/tests/recovery/runtime_reentry.rs +++ b/apps/decodex/src/orchestrator/tests/recovery/runtime_reentry.rs @@ -521,7 +521,7 @@ Follow the repository policy. } #[test] -fn materialize_daemon_spawn_state_uses_retained_retry_budget_marker() { +fn materialize_daemon_spawn_state_starts_fresh_budget_for_normal_queue_intake() { let (_temp_dir, config, workflow) = temp_project_layout(); let issue = sample_issue("Todo", &[]); let tracker = FakeTracker::with_refresh_snapshots( @@ -545,10 +545,43 @@ fn materialize_daemon_spawn_state_uses_retained_retry_budget_marker() { orchestrator::materialize_daemon_spawn_state(&config, &workflow, &state_store, &summary) .expect("daemon parent should materialize worktree and retry budget together"); + assert_eq!(daemon_spawn_state.worktree.path, summary.worktree_path); + assert_eq!( + daemon_spawn_state.retry_budget_base, 0, + "normal daemon queue intake should not inherit retry attempts from an old marker" + ); +} + +#[test] +fn materialize_daemon_spawn_state_uses_retained_retry_budget_marker_for_recovered_retry() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let issue = sample_active_issue("In Progress"); + let tracker = FakeTracker::with_refresh_snapshots( + vec![issue.clone()], + vec![vec![issue.clone()], vec![issue.clone()]], + ); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let worktree_manager = + WorktreeManager::new(config.service_id(), config.repo_root(), config.worktree_root()); + let retained_worktree = worktree_manager + .ensure_worktree(&issue.identifier, false) + .expect("retained worktree should exist"); + + state::write_run_retry_budget_attempt_count(&retained_worktree.path, "older-run", 4, 2) + .expect("retry budget marker should write"); + + let summary = orchestrator::run_project_once(&tracker, &config, &workflow, &state_store, true) + .expect("dry-run planning should succeed") + .expect("retained lane should still be selected"); + let daemon_spawn_state = + orchestrator::materialize_daemon_spawn_state(&config, &workflow, &state_store, &summary) + .expect("daemon parent should materialize worktree and retry budget together"); + + assert_eq!(summary.dispatch_mode, orchestrator::IssueDispatchMode::Retry); assert_eq!(daemon_spawn_state.worktree.path, summary.worktree_path); assert_eq!( daemon_spawn_state.retry_budget_base, 2, - "daemon child handoff should preserve retry budget from the retained worktree marker" + "recovered retry handoff should preserve retry budget from the retained worktree marker" ); } diff --git a/apps/decodex/src/orchestrator/tests/recovery/terminal_failures.rs b/apps/decodex/src/orchestrator/tests/recovery/terminal_failures.rs index bffeedfd1..426d1a0f5 100644 --- a/apps/decodex/src/orchestrator/tests/recovery/terminal_failures.rs +++ b/apps/decodex/src/orchestrator/tests/recovery/terminal_failures.rs @@ -237,7 +237,7 @@ fn terminal_failure_with_retained_tracked_changes_records_retained_partial_progr } #[test] -fn retryable_runtime_failure_with_retained_tracked_changes_records_retained_partial_progress() { +fn retryable_runtime_failure_with_retained_tracked_changes_retries_before_attention() { let (_temp_dir, config, workflow) = temp_project_layout(); let active_label = tracker::automation_active_label(TEST_SERVICE_ID); let issue = sample_issue("In Progress", &[active_label.as_str()]); @@ -276,38 +276,27 @@ fn retryable_runtime_failure_with_retained_tracked_changes_records_retained_part .expect("run attempt should record"); orchestrator::handle_failure(&tracker, &config, &workflow, &state_store, &issue_run, &error) - .expect("dirty generic runtime failure should retain partial progress"); + .expect("dirty generic runtime failure should remain retryable"); let comments = tracker.comments.borrow(); + assert!(tracker.state_updates.borrow().is_empty()); + assert!(tracker.label_additions.borrow().is_empty()); assert!(comments.iter().any(|comment| { - comment.contains("decodex retained partial progress and needs attention") - && comment.contains("partial_progress_retained") - && comment.contains("finish validation and PR handoff or reset the patch manually") + comment.contains("decodex run failed and will retry") + && comment.contains("retryable_execution_failure") + && comment.contains("decodex will retry automatically") })); assert!( comments .iter() - .all(|comment| !comment.contains("retryable_execution_failure")), - "retained validation-ready work must not be hidden behind a generic retry comment" + .all(|comment| !comment.contains("decodex retained partial progress and needs attention")), + "retained work must not force manual attention while retry budget remains" ); - - let ledger_event = comments - .iter() - .find_map(|comment| records::parse_linear_execution_event_record(comment)) - .expect("retained generic runtime failure should write a Linear execution event"); - - assert_eq!(ledger_event.event_type, "needs_attention"); - assert_eq!(ledger_event.error_class.as_deref(), Some("partial_progress_retained")); - assert_eq!(ledger_event.terminal_path.as_deref(), Some("retained_partial_progress")); assert!( - ledger_event - .evidence - .as_deref() - .is_some_and(|evidence| evidence - .iter() - .any(|item| item.contains("tracked worktree changes retained"))), - "retained progress evidence should identify the retained tracked patch" + comments.iter().all(|comment| records::parse_linear_execution_event_record(comment) + .is_none()), + "retryable retained work should not write a terminal needs-attention ledger event" ); } @@ -754,209 +743,136 @@ fn app_server_failures_skip_retry_and_require_attention() { "resolve the Codex app-server transport failure manually", ); assert_app_server_failure_requires_attention( - Report::new(AppServerTurnFailure::new( - "thread-1", - Some(String::from("turn-1")), - "failed", - "You've hit your usage limit.", - Some(String::from("usageLimitExceeded")), + Report::new(AppServerTransportFailure::with_phase( + String::from("App-server stdout disconnected unexpectedly."), + "turn/start", + false, )), - "app_server_usage_limit_exceeded", - "inspect Codex account usage", + "app_server_transport_disconnected", + "resolve the Codex app-server transport failure during `turn/start` manually", ); } #[test] -fn dirty_runtime_failures_record_retained_progress_instead_of_terminal_failure() { +fn app_server_preflight_timeouts_retry_before_attention_budget_is_exhausted() { let (_temp_dir, config, workflow) = temp_project_layout(); - let active_label = tracker::automation_active_label(TEST_SERVICE_ID); - let issue = sample_issue("In Progress", &[active_label.as_str()]); - let tracker = FakeTracker::new(vec![issue.clone()]); + let tracker = FakeTracker::new(vec![]); let state_store = StateStore::open_in_memory().expect("state store should open"); - let worktree_path = config.worktree_root().join("PUB-101"); - - git_status_success( - config.repo_root(), - &["worktree", "add", "-b", "x/pubfi-pub-101", ".worktrees/PUB-101", "main"], - ); - - fs::write(worktree_path.join("README.md"), "retained runtime recovery work\n") - .expect("tracked worktree file should change"); - + let issue = sample_issue("In Progress", &[]); let issue_run = IssueRunPlan { issue: issue.clone(), issue_state: issue.state.name.clone(), - initial_issue_state: issue.state.name.clone(), + initial_issue_state: String::from("Todo"), worktree: WorktreeSpec { branch_name: String::from("x/pubfi-pub-101"), issue_identifier: issue.identifier.clone(), - path: worktree_path, - reused_existing: true, + path: config.worktree_root().join("PUB-101"), + reused_existing: false, }, retry_project_slug: issue .project_slug .clone() .expect("sample issue should carry a project slug"), - dispatch_mode: IssueDispatchMode::Retry, - attempt_number: 2, - run_id: String::from("pub-101-attempt-2-123"), - retry_budget_base: 1, + dispatch_mode: IssueDispatchMode::Normal, + attempt_number: 1, + run_id: String::from("pub-101-attempt-1-123"), + retry_budget_base: 0, }; - let error = Report::new(AppServerCapabilityPreflightFailure::blocked_for_test( - "model", - "configured model was not present in model/list.", + let error = Report::new(AppServerCapabilityPreflightFailure::method_timed_out_for_test( + "plugin/list", + String::from("Timed out while waiting for app-server output."), )); + fs::create_dir_all(&issue_run.worktree.path).expect("worktree path should exist"); + state_store .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") .expect("run attempt should record"); orchestrator::handle_failure(&tracker, &config, &workflow, &state_store, &issue_run, &error) - .expect("dirty runtime failure should retain partial progress"); - - let comments = tracker.comments.borrow(); + .expect("preflight timeout should remain retryable"); - assert!(comments.iter().any(|comment| { - comment.contains("decodex retained partial progress and needs attention") - && comment.contains("partial_progress_retained") - && comment.contains("app_server_runtime_preflight_failed") + assert!(tracker.state_updates.borrow().is_empty()); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(tracker.comments.borrow().iter().any(|comment| { + comment.contains("decodex run failed and will retry") + && comment.contains("app_server_plugin_list_timeout") + && comment.contains("retry app-server preflight automatically") })); assert!( - comments + !tracker + .comments + .borrow() .iter() - .all(|comment| !comment.contains("decodex run failed and needs attention")) + .any(|comment| comment.contains("decodex run failed and needs attention")) ); - let ledger_event = comments - .iter() - .find_map(|comment| records::parse_linear_execution_event_record(comment)) - .expect("retained runtime failure should write a Linear execution event"); + let marker = state::read_run_activity_marker_snapshot(&issue_run.worktree.path) + .expect("retry schedule should be readable") + .expect("retry schedule marker should exist"); - assert_eq!(ledger_event.event_type, "needs_attention"); - assert_eq!(ledger_event.error_class.as_deref(), Some("partial_progress_retained")); - assert_eq!(ledger_event.terminal_path.as_deref(), Some("retained_partial_progress")); - assert!( - ledger_event - .evidence - .as_deref() - .is_some_and(|evidence| evidence - .iter() - .any(|item| item.contains("app_server_runtime_preflight_failed"))), - "retained progress evidence should preserve the source runtime error class" - ); + assert_eq!(marker.retry_kind(), Some("failure")); } #[test] -fn explicit_manual_attention_keeps_manual_terminal_path_with_dirty_worktree() { +fn exhausted_app_server_preflight_timeout_retry_budget_requires_attention_with_timeout_class() { let (_temp_dir, config, workflow) = temp_project_layout(); - let active_label = tracker::automation_active_label(TEST_SERVICE_ID); - let issue = sample_issue("In Progress", &[active_label.as_str()]); - let tracker = FakeTracker::new(vec![issue.clone()]); + let tracker = FakeTracker::new(vec![]); let state_store = StateStore::open_in_memory().expect("state store should open"); - let worktree_path = config.worktree_root().join("PUB-101"); - - git_status_success( - config.repo_root(), - &["worktree", "add", "-b", "x/pubfi-pub-101", ".worktrees/PUB-101", "main"], - ); - - fs::write(worktree_path.join("README.md"), "manual attention work\n") - .expect("tracked worktree file should change"); - + let issue = sample_issue("In Progress", &[]); let issue_run = IssueRunPlan { issue: issue.clone(), issue_state: issue.state.name.clone(), - initial_issue_state: issue.state.name.clone(), + initial_issue_state: String::from("Todo"), worktree: WorktreeSpec { branch_name: String::from("x/pubfi-pub-101"), issue_identifier: issue.identifier.clone(), - path: worktree_path, - reused_existing: true, + path: config.worktree_root().join("PUB-101"), + reused_existing: false, }, retry_project_slug: issue .project_slug .clone() .expect("sample issue should carry a project slug"), - dispatch_mode: IssueDispatchMode::Normal, - attempt_number: 1, - run_id: String::from("pub-101-attempt-1-123"), - retry_budget_base: 0, + dispatch_mode: IssueDispatchMode::Retry, + attempt_number: 3, + run_id: String::from("pub-101-attempt-3-123"), + retry_budget_base: 2, }; - let error = Report::new(ManualAttentionRequested { - issue_identifier: issue.identifier.clone(), - label: String::from("decodex:needs-attention"), - run_id: issue_run.run_id.clone(), - error_class: None, - }); + let error = Report::new(AppServerCapabilityPreflightFailure::method_timed_out_for_test( + "plugin/list", + String::from("Timed out while waiting for app-server output."), + )); + + fs::create_dir_all(&issue_run.worktree.path).expect("worktree path should exist"); state_store .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") .expect("run attempt should record"); orchestrator::handle_failure(&tracker, &config, &workflow, &state_store, &issue_run, &error) - .expect("manual attention should keep its terminal path"); - - let ledger_event = tracker - .comments - .borrow() - .iter() - .find_map(|comment| records::parse_linear_execution_event_record(comment)) - .expect("manual attention should write a Linear execution event"); + .expect("exhausted preflight timeout should require attention"); - assert_eq!(ledger_event.event_type, "needs_attention"); - assert_eq!(ledger_event.error_class.as_deref(), Some("human_attention_required")); - assert_eq!(ledger_event.terminal_path.as_deref(), Some("manual_attention")); assert_eq!( - ledger_event.summary.as_deref(), - Some("Decodex run failed and needs attention.") + tracker.state_updates.borrow().last(), + Some(&(issue.id.clone(), String::from("state-todo"))) ); -} - -#[test] -fn prepare_issue_run_clears_terminal_guard_marker_when_new_attempt_starts() { - let (_temp_dir, base_config, workflow) = temp_project_layout(); - let config = service_config_with_github_token_env_var(&base_config, "HOME"); - let issue = sample_issue("Todo", &[]); - let tracker = FakeTracker::with_refresh_snapshots(vec![], vec![vec![issue.clone()]]); - let state_store = StateStore::open_in_memory().expect("state store should open"); - let worktree_manager = - WorktreeManager::new(config.service_id(), config.repo_root(), config.worktree_root()); - let worktree = worktree_manager - .ensure_worktree(&issue.identifier, false) - .expect("worktree should exist before retry guard clearing"); - let marker_path = worktree.path.join(TERMINAL_GUARD_MARKER_FILE); - - fs::write(&marker_path, "stale terminal guard\n").expect("terminal guard marker should write"); - - let issue_run = orchestrator::prepare_issue_run( - PrepareIssueRunContext { - tracker: &tracker, - project: &config, - workflow: &workflow, - state_store: &state_store, - worktree_manager: &worktree_manager, - dry_run: false, - lease_preacquired: false, - dispatch_mode: IssueDispatchMode::Normal, - preferred_issue_state: None, - preferred_initial_issue_state: None, - preferred_run_identity: None, - preferred_retry_budget_base: None, - }, - issue, - ) - .expect("issue preparation should succeed") - .expect("startable issue should produce a run plan"); - - assert_eq!(issue_run.worktree.path, worktree.path); + assert!(tracker.comments.borrow().iter().any(|comment| { + comment.contains("decodex run failed and needs attention") + && comment.contains("app_server_plugin_list_timeout") + && comment.contains("app_server_preflight_failed evidence for the `plugin/list` timeout") + })); assert!( - !marker_path.exists(), - "starting a new attempt should clear stale terminal-guard markers" + !tracker + .comments + .borrow() + .iter() + .any(|comment| comment.contains("decodex run failed and will retry")) ); } #[test] -fn retryable_failures_ignore_prior_continuation_attempts_in_writeback() { +fn phase_goal_terminal_path_missing_retries_before_attention_budget_is_exhausted() { let (_temp_dir, config, workflow) = temp_project_layout(); let tracker = FakeTracker::new(vec![]); let state_store = StateStore::open_in_memory().expect("state store should open"); @@ -964,7 +880,7 @@ fn retryable_failures_ignore_prior_continuation_attempts_in_writeback() { let issue_run = IssueRunPlan { issue: issue.clone(), issue_state: issue.state.name.clone(), - initial_issue_state: issue.state.name.clone(), + initial_issue_state: String::from("Todo"), worktree: WorktreeSpec { branch_name: String::from("x/pubfi-pub-101"), issue_identifier: issue.identifier.clone(), @@ -975,64 +891,1039 @@ fn retryable_failures_ignore_prior_continuation_attempts_in_writeback() { .project_slug .clone() .expect("sample issue should carry a project slug"), - dispatch_mode: IssueDispatchMode::Retry, - attempt_number: 4, - run_id: String::from("pub-101-attempt-4-123"), + dispatch_mode: IssueDispatchMode::Normal, + attempt_number: 1, + run_id: String::from("pub-101-attempt-1-123"), retry_budget_base: 0, }; + let error = Report::new(AppServerPhaseGoalFailure::missing_terminal_path_for_test( + PhaseGoalKind::HandoffEvidence, + )); + + fs::create_dir_all(&issue_run.worktree.path).expect("worktree path should exist"); - state_store - .record_run_attempt("pub-101-attempt-1-123", &issue.id, 1, "succeeded") - .expect("first continuation attempt should record"); - state_store - .record_run_attempt("pub-101-attempt-2-123", &issue.id, 2, "succeeded") - .expect("second continuation attempt should record"); - state_store - .record_run_attempt("pub-101-attempt-3-123", &issue.id, 3, "succeeded") - .expect("third continuation attempt should record"); state_store .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") - .expect("current failed attempt should record"); + .expect("run attempt should record"); - orchestrator::handle_failure( - &tracker, - &config, - &workflow, - &state_store, - &issue_run, - &Report::msg("command failed"), - ) - .expect("retryable failure handling should succeed"); + orchestrator::handle_failure(&tracker, &config, &workflow, &state_store, &issue_run, &error) + .expect("missing terminal path should remain retryable"); assert!(tracker.state_updates.borrow().is_empty()); - assert!(tracker.label_updates.borrow().is_empty()); assert!(tracker.label_additions.borrow().is_empty()); - assert!(tracker.label_removals.borrow().is_empty()); assert!(tracker.comments.borrow().iter().any(|comment| { - comment.contains("retryable_execution_failure") - && comment.contains("- attempt: `4`") - && comment.contains("- retry_budget_attempt: `1` / `3`") - })); - assert!(!tracker.comments.borrow().iter().any(|comment| { - comment.contains("needs attention") || comment.contains("retry_budget_exhausted") + comment.contains("decodex run failed and will retry") + && comment.contains("phase_goal_terminal_path_missing") + && comment.contains("terminal-path recovery automatically") })); + assert!( + !tracker + .comments + .borrow() + .iter() + .any(|comment| comment.contains("decodex run failed and needs attention")) + ); + + let marker = state::read_run_activity_marker_snapshot(&issue_run.worktree.path) + .expect("retry schedule should be readable") + .expect("retry schedule marker should exist"); + + assert_eq!(marker.retry_kind(), Some("failure")); } #[test] -fn manual_attention_failure_overrides_succeeded_run_status() { +fn phase_goal_terminal_path_missing_with_retained_changes_retries_before_attention() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let active_label = tracker::automation_active_label(TEST_SERVICE_ID); + let issue = sample_issue("In Progress", &[active_label.as_str()]); + let tracker = FakeTracker::new(vec![issue.clone()]); let state_store = StateStore::open_in_memory().expect("state store should open"); + let worktree_path = config.worktree_root().join("PUB-103"); - state_store - .record_run_attempt("run-1", "issue-1", 1, "succeeded") - .expect("run attempt should record"); - state_store.update_run_status("run-1", "failed").expect("failed outcome should persist"); + git_status_success( + config.repo_root(), + &["worktree", "add", "-b", "x/pubfi-pub-103", ".worktrees/PUB-103", "main"], + ); - assert_eq!( - state_store - .run_attempt("run-1") + fs::write(worktree_path.join("README.md"), "retained handoff patch\n") + .expect("tracked worktree file should change"); + + let issue_run = IssueRunPlan { + issue: issue.clone(), + issue_state: issue.state.name.clone(), + initial_issue_state: String::from("Todo"), + worktree: WorktreeSpec { + branch_name: String::from("x/pubfi-pub-103"), + issue_identifier: issue.identifier.clone(), + path: worktree_path, + reused_existing: true, + }, + retry_project_slug: issue + .project_slug + .clone() + .expect("sample issue should carry a project slug"), + dispatch_mode: IssueDispatchMode::Normal, + attempt_number: 1, + run_id: String::from("pub-103-attempt-1-123"), + retry_budget_base: 0, + }; + let error = Report::new(AppServerPhaseGoalFailure::missing_terminal_path_for_test( + PhaseGoalKind::HandoffEvidence, + )); + + state_store + .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") + .expect("run attempt should record"); + + orchestrator::handle_failure(&tracker, &config, &workflow, &state_store, &issue_run, &error) + .expect("dirty terminal-path failure should remain retryable"); + + assert!(tracker.state_updates.borrow().is_empty()); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(tracker.comments.borrow().iter().any(|comment| { + comment.contains("decodex run failed and will retry") + && comment.contains("phase_goal_terminal_path_missing") + })); + assert!( + tracker.comments.borrow().iter().all(|comment| { + !comment.contains("decodex retained partial progress and needs attention") + && !comment.contains("decodex run failed and needs attention") + }), + "retained tracked changes must not force manual attention during terminal-path retry" + ); +} + +#[test] +fn retryable_app_server_failures_do_not_write_attention_before_budget_exhaustion() { + let (_temp_dir, config, workflow) = temp_project_layout(); + + assert_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 1, + Report::new(AppServerTransportFailure::with_phase( + String::from("App-server stdout disconnected unexpectedly."), + "thread/start", + true, + )), + "app_server_transport_disconnected", + ); + assert_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 2, + Report::new(AppServerTurnFailure::new( + "thread-1", + Some(String::from("turn-1")), + "failed", + "transient model failure", + None, + )), + "retryable_execution_failure", + ); + assert_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 3, + Report::new(AppServerTurnFailure::new( + "thread-1", + Some(String::from("turn-1")), + "failed", + "You've hit your usage limit.", + Some(String::from("usageLimitExceeded")), + )), + "app_server_usage_limit_exceeded", + ); + assert_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 4, + Report::new(AppServerZeroEvidenceStartFailure::new( + String::from("PUB-104"), + String::from("pub-104-attempt-1-123"), + )), + "app_server_zero_evidence_start_failed", + ); + assert_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 5, + Report::new(AppServerCapabilityPreflightFailure::method_timed_out_for_test( + "plugin/list", + String::from("Timed out while waiting for app-server output."), + )), + "app_server_plugin_list_timeout", + ); + assert_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 6, + Report::new(AppServerPhaseGoalFailure::missing_terminal_path_for_test( + PhaseGoalKind::HandoffEvidence, + )), + "phase_goal_terminal_path_missing", + ); + assert_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 7, + Report::new(AppServerDynamicToolFailure::protocol_for_test( + Some(String::from("issue_comment")), + "dynamic tool declaration was missing input schema", + )), + "app_server_dynamic_tool_protocol_failure", + ); + assert_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 8, + Report::new(AppServerDynamicToolFailure::tool_for_test( + Some(String::from("issue_comment")), + "tool rejected", + )), + "app_server_dynamic_tool_failed", + ); +} + +#[test] +fn retryable_orchestrator_failures_do_not_write_attention_before_budget_exhaustion() { + let (_temp_dir, config, workflow) = temp_project_layout(); + + assert_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 1, + Report::new(RepoGateFailure::new( + RepoGateFailureKind::GitLockContention, + String::from("fatal: Unable to create '.git/index.lock': File exists."), + )), + "repo_gate_git_lock_contention", + ); + assert_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 2, + Report::new(RepoGateFailure::new( + RepoGateFailureKind::VerifyCommandFailed, + String::from("cargo make check failed."), + )), + "repo_gate_verify_failed", + ); + assert_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 3, + Report::new(StalledRunNeedsAttention { + issue_identifier: String::from("PUB-103"), + run_id: String::from("pub-103-attempt-1-123"), + idle_for: ACTIVE_RUN_IDLE_TIMEOUT + Duration::from_secs(1), + }), + "stalled_run_detected", + ); +} + +#[test] +fn dirty_retryable_runtime_failures_keep_automatic_recovery_before_budget_exhaustion() { + let (_temp_dir, config, workflow) = temp_project_layout(); + + assert_dirty_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 1, + Report::new(AppServerTransportFailure::with_phase( + String::from("App-server stdout disconnected unexpectedly."), + "thread/start", + true, + )), + "app_server_transport_disconnected", + ); + assert_dirty_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 2, + Report::new(AppServerCapabilityPreflightFailure::method_timed_out_for_test( + "plugin/list", + String::from("Timed out while waiting for app-server output."), + )), + "app_server_plugin_list_timeout", + ); + assert_dirty_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 3, + Report::new(AppServerPhaseGoalFailure::missing_terminal_path_for_test( + PhaseGoalKind::HandoffEvidence, + )), + "phase_goal_terminal_path_missing", + ); + assert_dirty_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 4, + Report::new(AppServerDynamicToolFailure::protocol_for_test( + Some(String::from("issue_comment")), + "dynamic tool declaration was missing input schema", + )), + "app_server_dynamic_tool_protocol_failure", + ); + assert_dirty_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 5, + Report::new(AppServerDynamicToolFailure::tool_for_test( + Some(String::from("issue_comment")), + "tool rejected", + )), + "app_server_dynamic_tool_failed", + ); + assert_dirty_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 6, + Report::new(RepoGateFailure::new( + RepoGateFailureKind::GitLockContention, + String::from("fatal: Unable to create '.git/index.lock': File exists."), + )), + "repo_gate_git_lock_contention", + ); + assert_dirty_retryable_failure_writeback_does_not_require_attention( + &config, + &workflow, + 7, + Report::new(RepoGateFailure::new( + RepoGateFailureKind::VerifyCommandFailed, + String::from("cargo make check failed."), + )), + "repo_gate_verify_failed", + ); +} + +#[test] +fn startup_transport_failures_retry_before_attention_budget_is_exhausted() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let tracker = FakeTracker::new(vec![]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let issue = sample_issue("In Progress", &[]); + let issue_run = IssueRunPlan { + issue: issue.clone(), + issue_state: issue.state.name.clone(), + initial_issue_state: String::from("Todo"), + worktree: WorktreeSpec { + branch_name: String::from("x/pubfi-pub-101"), + issue_identifier: issue.identifier.clone(), + path: config.worktree_root().join("PUB-101"), + reused_existing: false, + }, + retry_project_slug: issue + .project_slug + .clone() + .expect("sample issue should carry a project slug"), + dispatch_mode: IssueDispatchMode::Normal, + attempt_number: 1, + run_id: String::from("pub-101-attempt-1-123"), + retry_budget_base: 0, + }; + let error = Report::new(AppServerTransportFailure::with_phase( + String::from("App-server stdout disconnected unexpectedly."), + "thread/start", + true, + )); + + fs::create_dir_all(&issue_run.worktree.path).expect("worktree path should exist"); + + state_store + .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") + .expect("run attempt should record"); + + orchestrator::handle_failure(&tracker, &config, &workflow, &state_store, &issue_run, &error) + .expect("startup transport failure should remain retryable"); + + assert!(tracker.state_updates.borrow().is_empty()); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(tracker.comments.borrow().iter().any(|comment| { + comment.contains("decodex run failed and will retry") + && comment.contains("app_server_transport_disconnected") + && comment.contains("thread/start") + && comment.contains("restart the app-server and retry automatically") + })); + assert!( + !tracker + .comments + .borrow() + .iter() + .any(|comment| comment.contains("decodex run failed and needs attention")) + ); +} + +#[test] +fn exhausted_startup_transport_retry_budget_requires_attention_with_transport_class() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let tracker = FakeTracker::new(vec![]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let issue = sample_issue("In Progress", &[]); + let issue_run = IssueRunPlan { + issue: issue.clone(), + issue_state: issue.state.name.clone(), + initial_issue_state: String::from("Todo"), + worktree: WorktreeSpec { + branch_name: String::from("x/pubfi-pub-101"), + issue_identifier: issue.identifier.clone(), + path: config.worktree_root().join("PUB-101"), + reused_existing: false, + }, + retry_project_slug: issue + .project_slug + .clone() + .expect("sample issue should carry a project slug"), + dispatch_mode: IssueDispatchMode::Retry, + attempt_number: 3, + run_id: String::from("pub-101-attempt-3-123"), + retry_budget_base: 2, + }; + let error = Report::new(AppServerTransportFailure::with_phase( + String::from("App-server stdout disconnected unexpectedly."), + "thread/start", + true, + )); + + fs::create_dir_all(&issue_run.worktree.path).expect("worktree path should exist"); + + state_store + .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") + .expect("run attempt should record"); + + orchestrator::handle_failure(&tracker, &config, &workflow, &state_store, &issue_run, &error) + .expect("exhausted startup transport failure should require attention"); + + assert_eq!( + tracker.state_updates.borrow().last(), + Some(&(issue.id.clone(), String::from("state-todo"))) + ); + assert!(tracker.comments.borrow().iter().any(|comment| { + comment.contains("decodex run failed and needs attention") + && comment.contains("app_server_transport_disconnected") + && comment.contains("failure during `thread/start` manually") + })); + assert!( + !tracker + .comments + .borrow() + .iter() + .any(|comment| comment.contains("decodex run failed and will retry")) + ); +} + +#[test] +fn usage_limit_turn_failures_retry_before_attention_budget_is_exhausted() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let tracker = FakeTracker::new(vec![]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let issue = sample_issue("In Progress", &[]); + let issue_run = IssueRunPlan { + issue: issue.clone(), + issue_state: issue.state.name.clone(), + initial_issue_state: String::from("Todo"), + worktree: WorktreeSpec { + branch_name: String::from("x/pubfi-pub-101"), + issue_identifier: issue.identifier.clone(), + path: config.worktree_root().join("PUB-101"), + reused_existing: false, + }, + retry_project_slug: issue + .project_slug + .clone() + .expect("sample issue should carry a project slug"), + dispatch_mode: IssueDispatchMode::Normal, + attempt_number: 1, + run_id: String::from("pub-101-attempt-1-123"), + retry_budget_base: 0, + }; + let error = Report::new(AppServerTurnFailure::new( + "thread-1", + Some(String::from("turn-1")), + "failed", + "You've hit your usage limit.", + Some(String::from("usageLimitExceeded")), + )); + + fs::create_dir_all(&issue_run.worktree.path).expect("worktree path should exist"); + + state_store + .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") + .expect("run attempt should record"); + + orchestrator::handle_failure(&tracker, &config, &workflow, &state_store, &issue_run, &error) + .expect("usage limit failure should remain retryable"); + + assert!(tracker.state_updates.borrow().is_empty()); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(tracker.comments.borrow().iter().any(|comment| { + comment.contains("decodex run failed and will retry") + && comment.contains("app_server_usage_limit_exceeded") + && comment.contains("reselect or refresh the Codex account") + })); + assert!( + !tracker + .comments + .borrow() + .iter() + .any(|comment| comment.contains("decodex run failed and needs attention")) + ); +} + +#[test] +fn usage_limit_turn_failures_with_retained_tracked_changes_retry_before_attention() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let active_label = tracker::automation_active_label(TEST_SERVICE_ID); + let issue = sample_issue("In Progress", &[active_label.as_str()]); + let tracker = FakeTracker::new(vec![issue.clone()]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let worktree_path = config.worktree_root().join("PUB-103"); + + git_status_success( + config.repo_root(), + &["worktree", "add", "-b", "x/pubfi-pub-103", ".worktrees/PUB-103", "main"], + ); + + fs::write(worktree_path.join("README.md"), "retained usage-limit patch\n") + .expect("tracked worktree file should change"); + + let issue_run = IssueRunPlan { + issue: issue.clone(), + issue_state: issue.state.name.clone(), + initial_issue_state: String::from("Todo"), + worktree: WorktreeSpec { + branch_name: String::from("x/pubfi-pub-103"), + issue_identifier: issue.identifier.clone(), + path: worktree_path, + reused_existing: true, + }, + retry_project_slug: issue + .project_slug + .clone() + .expect("sample issue should carry a project slug"), + dispatch_mode: IssueDispatchMode::Normal, + attempt_number: 1, + run_id: String::from("pub-103-attempt-1-123"), + retry_budget_base: 0, + }; + let error = Report::new(AppServerTurnFailure::new( + "thread-1", + Some(String::from("turn-1")), + "failed", + "You've hit your usage limit.", + Some(String::from("usageLimitExceeded")), + )); + + state_store + .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") + .expect("run attempt should record"); + + orchestrator::handle_failure(&tracker, &config, &workflow, &state_store, &issue_run, &error) + .expect("dirty usage-limit failure should remain retryable"); + + assert!(tracker.state_updates.borrow().is_empty()); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(tracker.comments.borrow().iter().any(|comment| { + comment.contains("decodex run failed and will retry") + && comment.contains("app_server_usage_limit_exceeded") + && comment.contains("reselect or refresh the Codex account") + })); + assert!( + tracker.comments.borrow().iter().all(|comment| { + !comment.contains("decodex retained partial progress and needs attention") + && !comment.contains("decodex run failed and needs attention") + }), + "retained tracked changes must not force manual attention while usage-limit retry remains" + ); +} + +#[test] +fn exhausted_usage_limit_retry_budget_requires_attention_with_usage_class() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let tracker = FakeTracker::new(vec![]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let issue = sample_issue("In Progress", &[]); + let issue_run = IssueRunPlan { + issue: issue.clone(), + issue_state: issue.state.name.clone(), + initial_issue_state: String::from("Todo"), + worktree: WorktreeSpec { + branch_name: String::from("x/pubfi-pub-101"), + issue_identifier: issue.identifier.clone(), + path: config.worktree_root().join("PUB-101"), + reused_existing: false, + }, + retry_project_slug: issue + .project_slug + .clone() + .expect("sample issue should carry a project slug"), + dispatch_mode: IssueDispatchMode::Retry, + attempt_number: 3, + run_id: String::from("pub-101-attempt-3-123"), + retry_budget_base: 2, + }; + let error = Report::new(AppServerTurnFailure::new( + "thread-1", + Some(String::from("turn-1")), + "failed", + "You've hit your usage limit.", + Some(String::from("usageLimitExceeded")), + )); + + fs::create_dir_all(&issue_run.worktree.path).expect("worktree path should exist"); + + state_store + .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") + .expect("run attempt should record"); + + orchestrator::handle_failure(&tracker, &config, &workflow, &state_store, &issue_run, &error) + .expect("exhausted usage limit failure should require attention"); + + assert_eq!( + tracker.state_updates.borrow().last(), + Some(&(issue.id.clone(), String::from("state-todo"))) + ); + assert!(tracker.comments.borrow().iter().any(|comment| { + comment.contains("decodex run failed and needs attention") + && comment.contains("app_server_usage_limit_exceeded") + && comment.contains("inspect Codex account usage") + })); + assert!( + !tracker + .comments + .borrow() + .iter() + .any(|comment| comment.contains("decodex run failed and will retry")) + ); +} + +#[test] +fn dirty_runtime_failures_record_retained_progress_instead_of_terminal_failure() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let active_label = tracker::automation_active_label(TEST_SERVICE_ID); + let issue = sample_issue("In Progress", &[active_label.as_str()]); + let tracker = FakeTracker::new(vec![issue.clone()]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let worktree_path = config.worktree_root().join("PUB-101"); + + git_status_success( + config.repo_root(), + &["worktree", "add", "-b", "x/pubfi-pub-101", ".worktrees/PUB-101", "main"], + ); + + fs::write(worktree_path.join("README.md"), "retained runtime recovery work\n") + .expect("tracked worktree file should change"); + + let issue_run = IssueRunPlan { + issue: issue.clone(), + issue_state: issue.state.name.clone(), + initial_issue_state: issue.state.name.clone(), + worktree: WorktreeSpec { + branch_name: String::from("x/pubfi-pub-101"), + issue_identifier: issue.identifier.clone(), + path: worktree_path, + reused_existing: true, + }, + retry_project_slug: issue + .project_slug + .clone() + .expect("sample issue should carry a project slug"), + dispatch_mode: IssueDispatchMode::Retry, + attempt_number: 2, + run_id: String::from("pub-101-attempt-2-123"), + retry_budget_base: 1, + }; + let error = Report::new(AppServerCapabilityPreflightFailure::blocked_for_test( + "model", + "configured model was not present in model/list.", + )); + + state_store + .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") + .expect("run attempt should record"); + + orchestrator::handle_failure(&tracker, &config, &workflow, &state_store, &issue_run, &error) + .expect("dirty runtime failure should retain partial progress"); + + let comments = tracker.comments.borrow(); + + assert!(comments.iter().any(|comment| { + comment.contains("decodex retained partial progress and needs attention") + && comment.contains("partial_progress_retained") + && comment.contains("app_server_runtime_preflight_failed") + })); + assert!( + comments + .iter() + .all(|comment| !comment.contains("decodex run failed and needs attention")) + ); + + let ledger_event = comments + .iter() + .find_map(|comment| records::parse_linear_execution_event_record(comment)) + .expect("retained runtime failure should write a Linear execution event"); + + assert_eq!(ledger_event.event_type, "needs_attention"); + assert_eq!(ledger_event.error_class.as_deref(), Some("partial_progress_retained")); + assert_eq!(ledger_event.terminal_path.as_deref(), Some("retained_partial_progress")); + assert!( + ledger_event + .evidence + .as_deref() + .is_some_and(|evidence| evidence + .iter() + .any(|item| item.contains("app_server_runtime_preflight_failed"))), + "retained progress evidence should preserve the source runtime error class" + ); +} + +#[test] +fn explicit_manual_attention_keeps_manual_terminal_path_with_dirty_worktree() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let active_label = tracker::automation_active_label(TEST_SERVICE_ID); + let issue = sample_issue("In Progress", &[active_label.as_str()]); + let tracker = FakeTracker::new(vec![issue.clone()]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let worktree_path = config.worktree_root().join("PUB-101"); + + git_status_success( + config.repo_root(), + &["worktree", "add", "-b", "x/pubfi-pub-101", ".worktrees/PUB-101", "main"], + ); + + fs::write(worktree_path.join("README.md"), "manual attention work\n") + .expect("tracked worktree file should change"); + + let issue_run = IssueRunPlan { + issue: issue.clone(), + issue_state: issue.state.name.clone(), + initial_issue_state: issue.state.name.clone(), + worktree: WorktreeSpec { + branch_name: String::from("x/pubfi-pub-101"), + issue_identifier: issue.identifier.clone(), + path: worktree_path, + reused_existing: true, + }, + retry_project_slug: issue + .project_slug + .clone() + .expect("sample issue should carry a project slug"), + dispatch_mode: IssueDispatchMode::Normal, + attempt_number: 1, + run_id: String::from("pub-101-attempt-1-123"), + retry_budget_base: 0, + }; + let error = Report::new(ManualAttentionRequested { + issue_identifier: issue.identifier.clone(), + label: String::from("decodex:needs-attention"), + run_id: issue_run.run_id.clone(), + error_class: None, + }); + + state_store + .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") + .expect("run attempt should record"); + + orchestrator::handle_failure(&tracker, &config, &workflow, &state_store, &issue_run, &error) + .expect("manual attention should keep its terminal path"); + + let ledger_event = tracker + .comments + .borrow() + .iter() + .find_map(|comment| records::parse_linear_execution_event_record(comment)) + .expect("manual attention should write a Linear execution event"); + + assert_eq!(ledger_event.event_type, "needs_attention"); + assert_eq!(ledger_event.error_class.as_deref(), Some("human_attention_required")); + assert_eq!(ledger_event.terminal_path.as_deref(), Some("manual_attention")); + assert_eq!( + ledger_event.summary.as_deref(), + Some("Decodex run failed and needs attention.") + ); +} + +#[test] +fn prepare_issue_run_clears_terminal_guard_marker_when_new_attempt_starts() { + let (_temp_dir, base_config, workflow) = temp_project_layout(); + let config = service_config_with_github_token_env_var(&base_config, "HOME"); + let issue = sample_issue("Todo", &[]); + let tracker = FakeTracker::with_refresh_snapshots(vec![], vec![vec![issue.clone()]]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let worktree_manager = + WorktreeManager::new(config.service_id(), config.repo_root(), config.worktree_root()); + let worktree = worktree_manager + .ensure_worktree(&issue.identifier, false) + .expect("worktree should exist before retry guard clearing"); + let marker_path = worktree.path.join(TERMINAL_GUARD_MARKER_FILE); + + fs::write(&marker_path, "stale terminal guard\n").expect("terminal guard marker should write"); + + let issue_run = orchestrator::prepare_issue_run( + PrepareIssueRunContext { + tracker: &tracker, + project: &config, + workflow: &workflow, + state_store: &state_store, + worktree_manager: &worktree_manager, + dry_run: false, + lease_preacquired: false, + dispatch_mode: IssueDispatchMode::Normal, + preferred_issue_state: None, + preferred_initial_issue_state: None, + preferred_run_identity: None, + preferred_retry_budget_base: None, + }, + issue, + ) + .expect("issue preparation should succeed") + .expect("startable issue should produce a run plan"); + + assert_eq!(issue_run.worktree.path, worktree.path); + assert!( + !marker_path.exists(), + "starting a new attempt should clear stale terminal-guard markers" + ); +} + +#[test] +fn retryable_failures_ignore_prior_continuation_attempts_in_writeback() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let tracker = FakeTracker::new(vec![]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let issue = sample_issue("In Progress", &[]); + let issue_run = IssueRunPlan { + issue: issue.clone(), + issue_state: issue.state.name.clone(), + initial_issue_state: issue.state.name.clone(), + worktree: WorktreeSpec { + branch_name: String::from("x/pubfi-pub-101"), + issue_identifier: issue.identifier.clone(), + path: config.worktree_root().join("PUB-101"), + reused_existing: false, + }, + retry_project_slug: issue + .project_slug + .clone() + .expect("sample issue should carry a project slug"), + dispatch_mode: IssueDispatchMode::Retry, + attempt_number: 4, + run_id: String::from("pub-101-attempt-4-123"), + retry_budget_base: 0, + }; + + state_store + .record_run_attempt("pub-101-attempt-1-123", &issue.id, 1, "succeeded") + .expect("first continuation attempt should record"); + state_store + .record_run_attempt("pub-101-attempt-2-123", &issue.id, 2, "succeeded") + .expect("second continuation attempt should record"); + state_store + .record_run_attempt("pub-101-attempt-3-123", &issue.id, 3, "succeeded") + .expect("third continuation attempt should record"); + state_store + .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") + .expect("current failed attempt should record"); + + orchestrator::handle_failure( + &tracker, + &config, + &workflow, + &state_store, + &issue_run, + &Report::msg("command failed"), + ) + .expect("retryable failure handling should succeed"); + + assert!(tracker.state_updates.borrow().is_empty()); + assert!(tracker.label_updates.borrow().is_empty()); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(tracker.label_removals.borrow().is_empty()); + assert!(tracker.comments.borrow().iter().any(|comment| { + comment.contains("retryable_execution_failure") + && comment.contains("- attempt: `4`") + && comment.contains("- retry_budget_attempt: `1` / `3`") + })); + assert!(!tracker.comments.borrow().iter().any(|comment| { + comment.contains("needs attention") || comment.contains("retry_budget_exhausted") + })); +} + +#[test] +fn manual_attention_failure_overrides_succeeded_run_status() { + let state_store = StateStore::open_in_memory().expect("state store should open"); + + state_store + .record_run_attempt("run-1", "issue-1", 1, "succeeded") + .expect("run attempt should record"); + state_store.update_run_status("run-1", "failed").expect("failed outcome should persist"); + + assert_eq!( + state_store + .run_attempt("run-1") .expect("run attempt lookup should succeed") .expect("run attempt should exist") .status(), "failed" ); } + +fn assert_retryable_failure_writeback_does_not_require_attention( + config: &ServiceConfig, + workflow: &WorkflowDocument, + case_number: usize, + error: Report, + expected_error_class: &str, +) { + let tracker = FakeTracker::new(vec![]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let issue_id = format!("issue-{case_number}"); + let issue_identifier = format!("PUB-10{case_number}"); + let issue = sample_issue_with_sort_fields( + &issue_id, + &issue_identifier, + "In Progress", + &[], + Some(3), + "2026-03-13T04:16:17.133Z", + ); + let issue_run = IssueRunPlan { + issue: issue.clone(), + issue_state: issue.state.name.clone(), + initial_issue_state: String::from("Todo"), + worktree: WorktreeSpec { + branch_name: format!("x/pubfi-{}", issue_identifier.to_lowercase()), + issue_identifier: issue.identifier.clone(), + path: config.worktree_root().join(&issue.identifier), + reused_existing: false, + }, + retry_project_slug: issue + .project_slug + .clone() + .expect("sample issue should carry a project slug"), + dispatch_mode: IssueDispatchMode::Normal, + attempt_number: 1, + run_id: format!("pub-10{case_number}-attempt-1-123"), + retry_budget_base: 0, + }; + + fs::create_dir_all(&issue_run.worktree.path).expect("worktree path should exist"); + + state_store + .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") + .expect("run attempt should record"); + + orchestrator::handle_failure(&tracker, config, workflow, &state_store, &issue_run, &error) + .expect("retryable failure handling should succeed"); + + let comments = tracker.comments.borrow(); + + assert!(tracker.state_updates.borrow().is_empty()); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(comments.iter().any(|comment| { + comment.contains("decodex run failed and will retry") + && comment.contains(expected_error_class) + })); + assert!(comments.iter().all(|comment| { + !comment.contains("decodex run failed and needs attention") + && !comment.contains("decodex retained partial progress and needs attention") + })); + assert!(comments.iter().all(|comment| { + records::parse_linear_execution_event_record(comment).is_none() + })); + assert!( + state_store + .list_linear_execution_events(config.service_id(), &issue.id) + .expect("linear execution events should list") + .is_empty() + ); +} + +fn assert_dirty_retryable_failure_writeback_does_not_require_attention( + config: &ServiceConfig, + workflow: &WorkflowDocument, + case_number: usize, + error: Report, + expected_error_class: &str, +) { + let tracker = FakeTracker::new(vec![]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let issue_id = format!("issue-dirty-{case_number}"); + let issue_identifier = format!("PUB-30{case_number}"); + let active_label = tracker::automation_active_label(TEST_SERVICE_ID); + let issue = sample_issue_with_sort_fields( + &issue_id, + &issue_identifier, + "In Progress", + &[active_label.as_str()], + Some(3), + "2026-03-13T04:16:17.133Z", + ); + let branch_name = format!("x/pubfi-{}", issue_identifier.to_lowercase()); + let worktree_rel_path = format!(".worktrees/{issue_identifier}"); + let worktree_path = config.worktree_root().join(&issue_identifier); + + git_status_success( + config.repo_root(), + &["worktree", "add", "-b", &branch_name, &worktree_rel_path, "main"], + ); + + fs::write( + worktree_path.join("README.md"), + format!("dirty retryable recovery case {case_number}\n"), + ) + .expect("tracked worktree file should change"); + + let issue_run = IssueRunPlan { + issue: issue.clone(), + issue_state: issue.state.name.clone(), + initial_issue_state: String::from("Todo"), + worktree: WorktreeSpec { + branch_name, + issue_identifier: issue.identifier.clone(), + path: worktree_path, + reused_existing: true, + }, + retry_project_slug: issue + .project_slug + .clone() + .expect("sample issue should carry a project slug"), + dispatch_mode: IssueDispatchMode::Normal, + attempt_number: 1, + run_id: format!("pub-30{case_number}-attempt-1-123"), + retry_budget_base: 0, + }; + + state_store + .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") + .expect("run attempt should record"); + + orchestrator::handle_failure(&tracker, config, workflow, &state_store, &issue_run, &error) + .expect("dirty retryable failure handling should succeed"); + + let comments = tracker.comments.borrow(); + + assert!(tracker.state_updates.borrow().is_empty()); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(comments.iter().any(|comment| { + comment.contains("decodex run failed and will retry") + && comment.contains(expected_error_class) + })); + assert!( + comments.iter().all(|comment| { + !comment.contains("decodex retained partial progress and needs attention") + && !comment.contains("decodex run failed and needs attention") + }), + "retained tracked changes must not force manual attention for `{expected_error_class}` while retry budget remains" + ); + assert!(comments.iter().all(|comment| { + records::parse_linear_execution_event_record(comment).is_none() + })); + assert!( + state_store + .list_linear_execution_events(config.service_id(), &issue.id) + .expect("linear execution events should list") + .is_empty() + ); +} diff --git a/apps/decodex/src/orchestrator/tests/review_landing/classification_review.rs b/apps/decodex/src/orchestrator/tests/review_landing/classification_review.rs index 7c87db9a7..bdd909c6d 100644 --- a/apps/decodex/src/orchestrator/tests/review_landing/classification_review.rs +++ b/apps/decodex/src/orchestrator/tests/review_landing/classification_review.rs @@ -292,7 +292,7 @@ fn classify_post_review_lane_request_pending_routes_fixable_ci_red_to_repair() { } #[test] -fn classify_post_review_lane_request_pending_blocks_unhandled_ci_red() { +fn classify_post_review_lane_request_pending_repairs_unhandled_ci_red() { let temp_dir = TempDir::new().expect("temp dir should exist"); let state_store = StateStore::open_in_memory().expect("state store should open"); let issue = sample_issue("In Review", &[]); @@ -365,8 +365,89 @@ fn classify_post_review_lane_request_pending_blocks_unhandled_ci_red() { ) .expect("classification should succeed"); - assert_eq!(classification.decision, PostReviewLaneDecision::Block); - assert_eq!(classification.reason, "external_review_request_ci_red_manual_attention",); + assert_eq!(classification.decision, PostReviewLaneDecision::NeedsReviewRepair); + assert_eq!(classification.reason, "external_review_request_ci_red_repair_required",); +} + +#[test] +fn classify_post_review_lane_request_pending_waits_for_unknown_check_state() { + let temp_dir = TempDir::new().expect("temp dir should exist"); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let issue = sample_issue("In Review", &[]); + let head_oid = String::from("08a20f7dfb9526e7421a5f095b1c6adec84e52d6"); + let worktree_path = temp_dir.path().join("lane"); + + fs::create_dir_all(&worktree_path).expect("worktree path should exist"); + + state_store + .upsert_worktree( + "pubfi", + &issue.id, + "x/pubfi-pub-101", + &worktree_path.display().to_string(), + ) + .expect("worktree should record"); + + let worktree = state_store + .list_worktrees("pubfi") + .expect("worktree list should succeed") + .into_iter() + .next() + .expect("worktree should exist"); + let snapshot = PostReviewLaneSnapshot { + issue, + worktree, + review_handoff: Some(sample_review_handoff_marker( + "x/pubfi-pub-101", + "https://github.com/hack-ink/decodex/pull/174", + &head_oid, + )), + local_branch_name: Some(String::from("x/pubfi-pub-101")), + local_head_oid: Some(head_oid.clone()), + }; + + seed_review_orchestration_marker( + &state_store, + TEST_SERVICE_ID, + &snapshot.issue.id, + &ReviewOrchestrationMarker::new( + "run-1", + 1, + "x/pubfi-pub-101", + "https://github.com/hack-ink/decodex/pull/174", + &head_oid, + "request_pending", + None, + None, + None, + 0, + 0, + None, + ), + ); + + let classification = orchestrator::classify_post_review_lane( + &snapshot, + &state_store, + &sample_workflow(), + &FakePullRequestReviewStateInspector::new(vec![Ok(sample_pull_request_review_state( + "https://github.com/hack-ink/decodex/pull/174", + "x/pubfi-pub-101", + &head_oid, + Some("APPROVED"), + "MERGEABLE", + "UNSTABLE", + Some("UNKNOWN_NEW_STATE"), + 0, + ))]), + ) + .expect("classification should succeed"); + + assert_eq!(classification.decision, PostReviewLaneDecision::WaitForReview); + assert_eq!( + classification.reason, + "external_review_request_waiting_for_green_checks", + ); } #[test] diff --git a/apps/decodex/src/orchestrator/tests/review_landing/orchestration.rs b/apps/decodex/src/orchestrator/tests/review_landing/orchestration.rs index 51e059470..eade8c68b 100644 --- a/apps/decodex/src/orchestrator/tests/review_landing/orchestration.rs +++ b/apps/decodex/src/orchestrator/tests/review_landing/orchestration.rs @@ -180,6 +180,87 @@ fn reconcile_post_review_orchestration_uses_matching_handoff_record_for_current_ assert_eq!(marker.pr_url(), pr_url); } +#[test] +fn reconcile_post_review_orchestration_rebinds_stale_head_marker_after_repair_push() { + let (temp_dir, config, workflow) = temp_project_layout(); + let config = service_config_with_github_token_env_var(&config, "PATH"); + let _path_guard = install_fake_post_issue_comment_gh_response( + &temp_dir, + TEST_EXTERNAL_REVIEW_REQUEST_COMMENT_ID, + "2025-11-03T00:00:00Z", + ); + let repo_root = config.repo_root().to_path_buf(); + let issue = post_review_sample_service_owned_issue("In Review"); + let tracker = + FakeTracker::with_refresh_snapshots(vec![issue.clone()], vec![vec![issue.clone()]]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let pr_url = "https://github.com/hack-ink/decodex/pull/173"; + let marker_head_oid = git_output(&repo_root, &["rev-parse", "HEAD"]); + let current_head_oid = + commit_worktree_change(&repo_root, "repair.txt", "repair push\n", "repair push"); + + state_store + .upsert_worktree("pubfi", &issue.id, "main", &repo_root.display().to_string()) + .expect("worktree should record"); + + seed_review_handoff_marker_for_path( + &state_store, + config.service_id(), + &repo_root, + &sample_review_handoff_marker("main", pr_url, &marker_head_oid), + ); + seed_review_orchestration_marker_for_path( + &state_store, + config.service_id(), + &repo_root, + &sample_review_orchestration_marker( + "main", + pr_url, + &marker_head_oid, + "waiting_for_result", + 1, + ), + ); + + let review_state = sample_pull_request_review_state( + pr_url, + "main", + ¤t_head_oid, + Some("APPROVED"), + "MERGEABLE", + "CLEAN", + Some("SUCCESS"), + 0, + ); + + orchestrator::reconcile_post_review_orchestration_with_inspector( + &tracker, + &config, + &workflow, + &state_store, + &FakePullRequestReviewStateInspector::new(vec![Ok(review_state)]), + ) + .expect("post-review orchestration should rebind stale marker without attention"); + + let marker = persisted_review_orchestration_marker_for_path( + &state_store, + config.service_id(), + &repo_root, + ); + + assert_eq!(marker.phase(), "waiting_for_ack"); + assert_eq!(marker.head_sha(), current_head_oid); + assert_eq!( + marker.request_comment_database_id(), + Some(TEST_EXTERNAL_REVIEW_REQUEST_COMMENT_ID) + ); + assert_eq!(marker.external_round_count(), 1); + assert!(tracker.comments.borrow().is_empty()); + assert!(tracker.state_updates.borrow().is_empty()); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(tracker.label_removals.borrow().is_empty()); +} + #[test] fn reconcile_post_review_orchestration_skips_merged_landed_lineage_without_manual_attention() { let (_temp_dir, config, workflow) = temp_project_layout(); @@ -768,6 +849,212 @@ fn reconcile_post_review_orchestration_waits_for_green_checks_before_requesting_ assert!(tracker.label_removals.borrow().is_empty()); } +#[test] +fn reconcile_post_review_orchestration_waits_when_pr_readback_degrades() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let repo_root = config.repo_root().to_path_buf(); + let issue = post_review_sample_service_owned_issue("In Review"); + let tracker = + FakeTracker::with_refresh_snapshots(vec![issue.clone()], vec![vec![issue.clone()]]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let head_oid = String::from_utf8( + Command::new("git") + .arg("-C") + .arg(&repo_root) + .args(["rev-parse", "HEAD"]) + .output() + .expect("git rev-parse should run") + .stdout, + ) + .expect("git output should be utf-8") + .trim() + .to_owned(); + let pr_url = "https://github.com/hack-ink/decodex/pull/173"; + + state_store + .upsert_worktree("pubfi", &issue.id, "main", &repo_root.display().to_string()) + .expect("worktree should record"); + + seed_review_handoff_marker_for_path( + &state_store, + config.service_id(), + &repo_root, + &sample_review_handoff_marker("main", pr_url, &head_oid), + ); + seed_review_orchestration_marker_for_path( + &state_store, + config.service_id(), + &repo_root, + &ReviewOrchestrationMarker::new( + "run-1", + 1, + "main", + pr_url, + &head_oid, + "request_pending", + None, + None, + None, + 0, + 0, + None, + ), + ); + + orchestrator::reconcile_post_review_orchestration_with_inspector( + &tracker, + &config, + &workflow, + &state_store, + &FakePullRequestReviewStateInspector::new(vec![Err(color_eyre::eyre::eyre!( + "gh api failed" + ))]), + ) + .expect("post-review orchestration should tolerate degraded PR readback"); + + let marker = persisted_review_orchestration_marker_for_path( + &state_store, + config.service_id(), + &repo_root, + ); + + assert_eq!(marker.phase(), "request_pending"); + assert!(tracker.comments.borrow().is_empty()); + assert!(tracker.label_updates.borrow().is_empty()); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(tracker.label_removals.borrow().is_empty()); +} + +#[test] +fn reconcile_post_review_orchestration_waits_when_worktree_head_read_fails() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let issue = post_review_sample_service_owned_issue("In Review"); + let tracker = + FakeTracker::with_refresh_snapshots(vec![issue.clone()], vec![vec![issue.clone()]]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let worktree_manager = + WorktreeManager::new(config.service_id(), config.repo_root(), config.worktree_root()); + let worktree = + worktree_manager.ensure_worktree(&issue.identifier, false).expect("worktree should exist"); + let branch_ref_path = + config.repo_root().join(".git").join("refs").join("heads").join(&worktree.branch_name); + let head_oid = git_output(&worktree.path, &["rev-parse", "HEAD"]); + let pr_url = "https://github.com/hack-ink/decodex/pull/173"; + + state_store + .upsert_worktree( + config.service_id(), + &issue.id, + &worktree.branch_name, + &worktree.path.display().to_string(), + ) + .expect("worktree should record"); + + seed_review_handoff_marker_for_path( + &state_store, + config.service_id(), + &worktree.path, + &sample_review_handoff_marker(&worktree.branch_name, pr_url, &head_oid), + ); + seed_review_orchestration_marker_for_path( + &state_store, + config.service_id(), + &worktree.path, + &ReviewOrchestrationMarker::new( + "run-1", + 1, + &worktree.branch_name, + pr_url, + &head_oid, + "request_pending", + None, + None, + None, + 0, + 0, + None, + ), + ); + + fs::remove_file(&branch_ref_path).expect("branch ref should remove"); + orchestrator::reconcile_post_review_orchestration_with_inspector( + &tracker, + &config, + &workflow, + &state_store, + &FakePullRequestReviewStateInspector::new(Vec::new()), + ) + .expect("post-review orchestration should tolerate local worktree readback failure"); + + assert!(tracker.comments.borrow().is_empty()); + assert!(tracker.state_updates.borrow().is_empty()); + assert!(tracker.label_updates.borrow().is_empty()); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(tracker.label_removals.borrow().is_empty()); + assert!( + state_store + .list_linear_execution_events(config.service_id(), &issue.id) + .expect("linear execution events should list") + .is_empty() + ); +} + +#[test] +fn reconcile_post_review_orchestration_waits_when_worktree_branch_read_fails() { + let (temp_dir, config, workflow) = temp_project_layout(); + let issue = post_review_sample_service_owned_issue("In Review"); + let tracker = + FakeTracker::with_refresh_snapshots(vec![issue.clone()], vec![vec![issue.clone()]]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let missing_worktree_path = temp_dir.path().join("missing-retained-worktree"); + let branch_name = "x/pubfi-pub-101"; + let head_oid = git_output(config.repo_root(), &["rev-parse", "HEAD"]); + let pr_url = "https://github.com/hack-ink/decodex/pull/173"; + + fs::create_dir_all(&missing_worktree_path).expect("broken worktree path should exist"); + + state_store + .upsert_worktree( + config.service_id(), + &issue.id, + branch_name, + &missing_worktree_path.display().to_string(), + ) + .expect("worktree should record"); + + seed_review_handoff_marker_value( + &state_store, + config.service_id(), + &issue.id, + &sample_review_handoff_marker(branch_name, pr_url, &head_oid), + ); + + orchestrator::reconcile_post_review_orchestration_with_inspector( + &tracker, + &config, + &workflow, + &state_store, + &FakePullRequestReviewStateInspector::new(Vec::new()), + ) + .expect("post-review orchestration should tolerate local branch readback failure"); + + assert!( + tracker.comments.borrow().is_empty(), + "unexpected tracker comments: {:#?}", + tracker.comments.borrow() + ); + assert!(tracker.state_updates.borrow().is_empty()); + assert!(tracker.label_updates.borrow().is_empty()); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(tracker.label_removals.borrow().is_empty()); + assert!( + state_store + .list_linear_execution_events(config.service_id(), &issue.id) + .expect("linear execution events should list") + .is_empty() + ); +} + #[test] fn reconcile_post_review_orchestration_routes_fixable_ci_red_to_repair_before_requesting_external_review() { @@ -1178,7 +1465,7 @@ fn reconcile_post_review_orchestration_skips_issue_without_service_active_label( } #[test] -fn reconcile_post_review_orchestration_blocks_unhandled_ci_red_before_requesting_external_review() { +fn reconcile_post_review_orchestration_repairs_unhandled_ci_red_before_requesting_external_review() { let (_temp_dir, config, workflow) = temp_project_layout(); let repo_root = config.repo_root().to_path_buf(); let issue = post_review_sample_service_owned_issue("In Review"); @@ -1249,16 +1536,14 @@ fn reconcile_post_review_orchestration_blocks_unhandled_ci_red_before_requesting ) .expect("post-review orchestration should succeed"); - let comments = tracker.comments.borrow(); - let comment = comments.first().expect("manual attention comment should be written"); - let ledger_event = records::parse_linear_execution_event_record(comment) - .expect("manual attention comment should include an execution ledger event"); - - assert_eq!(comments.len(), 1); - assert_eq!( - ledger_event.error_class.as_deref(), - Some("external_review_request_ci_red_manual_attention") + let marker = persisted_review_orchestration_marker_for_path( + &state_store, + config.service_id(), + &repo_root, ); - assert_eq!(tracker.label_additions.borrow().len(), 1); - assert_eq!(tracker.label_removals.borrow().len(), 1); + + assert_eq!(marker.phase(), "repair_required"); + assert!(tracker.comments.borrow().is_empty()); + assert!(tracker.label_additions.borrow().is_empty()); + assert!(tracker.label_removals.borrow().is_empty()); } diff --git a/apps/decodex/src/orchestrator/tests/runtime/failure.rs b/apps/decodex/src/orchestrator/tests/runtime/failure.rs index 096d452a4..c048fe230 100644 --- a/apps/decodex/src/orchestrator/tests/runtime/failure.rs +++ b/apps/decodex/src/orchestrator/tests/runtime/failure.rs @@ -6,6 +6,8 @@ use orchestrator::{ use orchestrator::AppServerZeroEvidenceStartFailure; use orchestrator::LoopGuardrailReason; use orchestrator::LoopGuardrailStopRequested; +use orchestrator::RunFailureWritebackDisposition; +use orchestrator::StalledRunNeedsAttention; fn git_config_value( repo_root: &Path, @@ -104,6 +106,150 @@ fn terminal_failure_comments_surface_actionable_error_classes() { } } +#[test] +fn failure_writeback_disposition_marks_retryable_recovery_classes() { + for (case_name, error, expected_disposition) in [ + ( + "startup transport", + Report::new(AppServerTransportFailure::with_phase( + String::from("App-server stdout disconnected before thread start."), + "thread/start", + true, + )), + RunFailureWritebackDisposition::RetryableStructuredRecovery, + ), + ( + "generic turn failure", + Report::new(AppServerTurnFailure::new( + "thread-1", + Some(String::from("turn-1")), + "failed", + "transient model failure", + None, + )), + RunFailureWritebackDisposition::RetryableStructuredRecovery, + ), + ( + "usage limit turn failure", + Report::new(AppServerTurnFailure::new( + "thread-1", + Some(String::from("turn-1")), + "failed", + "You've hit your usage limit.", + Some(String::from("usageLimitExceeded")), + )), + RunFailureWritebackDisposition::RetryableStructuredRecovery, + ), + ( + "repo gate lock contention", + Report::new(orchestrator::RepoGateFailure::new( + RepoGateFailureKind::GitLockContention, + String::from("fatal: Unable to create '.git/index.lock': File exists."), + )), + RunFailureWritebackDisposition::RetryableStructuredRecovery, + ), + ( + "zero-evidence app-server start failure", + Report::new(AppServerZeroEvidenceStartFailure::new( + String::from("PUB-101"), + String::from("pub-101-attempt-1-123"), + )), + RunFailureWritebackDisposition::RetryableStructuredRecovery, + ), + ( + "app-server capability preflight timeout", + Report::new(AppServerCapabilityPreflightFailure::method_timed_out_for_test( + "plugin/list", + String::from("Timed out while waiting for app-server output."), + )), + RunFailureWritebackDisposition::RetryableStructuredRecovery, + ), + ( + "phase goal terminal path missing", + Report::new(AppServerPhaseGoalFailure::missing_terminal_path_for_test( + PhaseGoalKind::HandoffEvidence, + )), + RunFailureWritebackDisposition::RetryableStructuredRecovery, + ), + ( + "dynamic tool protocol failure", + Report::new(AppServerDynamicToolFailure::protocol_for_test( + Some(String::from("issue_comment")), + "dynamic tool declaration was missing input schema", + )), + RunFailureWritebackDisposition::RetryableStructuredRecovery, + ), + ( + "stalled active run", + Report::new(StalledRunNeedsAttention { + issue_identifier: String::from("PUB-101"), + run_id: String::from("pub-101-attempt-1-123"), + idle_for: ACTIVE_RUN_IDLE_TIMEOUT + Duration::from_secs(1), + }), + RunFailureWritebackDisposition::RetryableStructuredRecovery, + ), + ] { + assert_eq!( + orchestrator::run_failure_writeback_disposition(&error), + expected_disposition, + "{case_name}" + ); + } +} + +#[test] +fn failure_writeback_disposition_marks_terminal_attention_classes() { + for (case_name, error, expected_disposition) in [ + ( + "turn transport", + Report::new(AppServerTransportFailure::with_phase( + String::from("App-server stdout disconnected during turn start."), + "turn/start", + false, + )), + RunFailureWritebackDisposition::TerminalAttention, + ), + ( + "operator attention turn failure", + Report::new(AppServerTurnFailure::new( + "thread-1", + Some(String::from("turn-1")), + "failed", + "operator attention required", + Some(String::from("operatorAttentionRequired")), + )), + RunFailureWritebackDisposition::TerminalAttention, + ), + ( + "app-server capability preflight blocker", + Report::new(AppServerCapabilityPreflightFailure::blocked_for_test( + "model", + "configured model was not present in model/list.", + )), + RunFailureWritebackDisposition::TerminalAttention, + ), + ( + "unsupported phase goal API", + Report::new(AppServerPhaseGoalFailure::unsupported_for_test("thread/goal/set")), + RunFailureWritebackDisposition::TerminalAttention, + ), + ( + "repo gate spawn failure", + Report::new(orchestrator::RepoGateFailure::new( + RepoGateFailureKind::CommandSpawnFailed, + String::from("Failed to spawn repo gate command `cargo make test`: missing tool"), + )), + RunFailureWritebackDisposition::TerminalAttention, + ), + ] { + assert_eq!( + orchestrator::run_failure_writeback_disposition(&error), + expected_disposition, + "{case_name}" + ); + } +} + #[test] fn loop_guardrail_terminal_failure_details_normalize_stop_classes() { let recovery_gate = "clear label `decodex:needs-attention`, then move the issue back to a startable state if another automated run is desired"; @@ -484,6 +630,34 @@ fn repo_gate_lock_contention_retry_comments_preserve_specific_error_class() { assert!(next_action.contains("retry automatically")); } +#[test] +fn stalled_run_retry_comments_preserve_specific_error_class() { + let error = Report::new(StalledRunNeedsAttention { + issue_identifier: String::from("PUB-101"), + run_id: String::from("pub-101-attempt-1-123"), + idle_for: ACTIVE_RUN_IDLE_TIMEOUT + Duration::from_secs(1), + }); + let (error_class, next_action) = orchestrator::retry_comment_details(&error); + + assert_eq!(error_class, "stalled_run_detected"); + assert!(next_action.contains("retry the stalled lane automatically")); + assert!(next_action.contains("retry budget exhausts")); +} + +#[test] +fn app_server_preflight_timeout_retry_comments_preserve_specific_error_class() { + let error = Report::new(AppServerCapabilityPreflightFailure::method_timed_out_for_test( + "plugin/list", + String::from("Timed out while waiting for app-server output."), + )); + let (error_class, next_action) = orchestrator::retry_comment_details(&error); + + assert_eq!(error_class, "app_server_plugin_list_timeout"); + assert!(next_action.contains("retry app-server preflight automatically")); + assert!(next_action.contains("`plugin/list` timeout")); + assert!(next_action.contains("retry budget exhausts")); +} + #[test] fn repo_gate_lock_contention_runtime_retry_writes_specific_retry_schedule_marker() { let (_temp_dir, config, workflow) = temp_project_layout(); @@ -531,6 +705,51 @@ fn repo_gate_lock_contention_runtime_retry_writes_specific_retry_schedule_marker ); } +#[test] +fn app_server_preflight_timeout_runtime_retry_writes_failure_retry_schedule_marker() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let issue = sample_issue("In Progress", &[]); + let worktree_path = config.worktree_root().join("PUB-101"); + let issue_run = IssueRunPlan { + issue: issue.clone(), + issue_state: issue.state.name.clone(), + initial_issue_state: issue.state.name.clone(), + worktree: WorktreeSpec { + branch_name: String::from("x/pubfi-pub-101"), + issue_identifier: issue.identifier.clone(), + path: worktree_path.clone(), + reused_existing: false, + }, + retry_project_slug: issue + .project_slug + .clone() + .expect("sample issue should carry a project slug"), + dispatch_mode: IssueDispatchMode::Normal, + attempt_number: 1, + run_id: String::from("pub-101-attempt-1-123"), + retry_budget_base: 0, + }; + let error = Report::new(AppServerCapabilityPreflightFailure::method_timed_out_for_test( + "plugin/list", + String::from("Timed out while waiting for app-server output."), + )); + + fs::create_dir_all(&worktree_path).expect("worktree path should exist"); + orchestrator::write_retry_schedule_marker_for_runtime_retry(&error, &workflow, &issue_run, 1) + .expect("preflight timeout should write a failure retry marker"); + + let marker = state::read_run_activity_marker_snapshot(&worktree_path) + .expect("retry schedule should remain readable") + .expect("retry marker should exist"); + + assert_eq!(marker.retry_kind(), Some("failure")); + assert!( + marker.retry_ready_at_unix_epoch().is_some_and( + |retry_ready_at| retry_ready_at > OffsetDateTime::now_utc().unix_timestamp() + ) + ); +} + #[test] fn retry_budget_current_failure_does_not_double_count_handed_off_base() { let (_temp_dir, config, _workflow) = temp_project_layout(); @@ -1090,6 +1309,13 @@ fn app_server_terminal_failures_preserve_specific_error_classes() { "app_server_zero_evidence_start_failed", "verify `decodex probe stdio://`", ), + ( + Report::new(AppServerPhaseGoalFailure::missing_terminal_path_for_test( + PhaseGoalKind::HandoffEvidence, + )), + "phase_goal_terminal_path_missing", + "finish validation/review/handoff", + ), ( Report::new(AppServerTurnFailure::new( "thread-1", @@ -1146,7 +1372,7 @@ fn app_server_preflight_terminal_action_surfaces_first_scan_error() { } #[test] -fn zero_evidence_app_server_start_failure_is_promoted_and_records_private_evidence() { +fn zero_evidence_app_server_start_failure_is_promoted_records_private_evidence_and_retries() { let (_temp_dir, config, workflow) = temp_project_layout(); let tracker = FakeTracker::new(vec![]); let state_store = StateStore::open_in_memory().expect("state store should open"); @@ -1190,7 +1416,7 @@ fn zero_evidence_app_server_start_failure_is_promoted_and_records_private_eviden assert!( error.downcast_ref::().is_some(), - "generic no-evidence startup errors should become terminal app-server start failures" + "generic no-evidence startup errors should become typed app-server start failures" ); let events = state_store @@ -1224,20 +1450,219 @@ fn zero_evidence_app_server_start_failure_is_promoted_and_records_private_eviden ); orchestrator::handle_failure(&tracker, &config, &workflow, &state_store, &issue_run, &error) - .expect("terminal failure handling should succeed"); + .expect("retryable zero-evidence failure handling should succeed"); + assert!(tracker.state_updates.borrow().is_empty()); + assert!(tracker.label_additions.borrow().is_empty()); assert!(tracker.comments.borrow().iter().any(|comment| { - comment.contains("app_server_zero_evidence_start_failed") - && comment.contains("verify `decodex probe stdio://`") + comment.contains("decodex run failed and will retry") + && comment.contains("app_server_zero_evidence_start_failed") + && comment.contains("restart the app-server and retry automatically") })); + assert!( + !tracker + .comments + .borrow() + .iter() + .any(|comment| comment.contains("decodex run failed and needs attention")), + "zero-evidence startup failure should not request operator attention before retry budget exhaustion" + ); assert!( !tracker .comments .borrow() .iter() .any(|comment| comment.contains("retryable_execution_failure")), - "zero-evidence startup failure must not burn retry budget as a generic retry" + "zero-evidence startup failure must preserve its typed retry class" + ); +} + +#[test] +fn exhausted_zero_evidence_start_retry_budget_requires_attention_with_typed_class() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let tracker = FakeTracker::new(vec![]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let issue = sample_issue("In Progress", &[]); + let issue_run = IssueRunPlan { + issue: issue.clone(), + issue_state: issue.state.name.clone(), + initial_issue_state: String::from("Todo"), + worktree: WorktreeSpec { + branch_name: String::from("x/pubfi-pub-101"), + issue_identifier: issue.identifier.clone(), + path: config.worktree_root().join("PUB-101"), + reused_existing: false, + }, + retry_project_slug: issue + .project_slug + .clone() + .expect("sample issue should carry a project slug"), + dispatch_mode: IssueDispatchMode::Retry, + attempt_number: 3, + run_id: String::from("pub-101-attempt-3-123"), + retry_budget_base: 2, + }; + let error = Report::new(AppServerZeroEvidenceStartFailure::new( + issue.identifier.clone(), + issue_run.run_id.clone(), + )); + + fs::create_dir_all(&issue_run.worktree.path).expect("worktree path should exist"); + + state_store + .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") + .expect("run attempt should record"); + + orchestrator::handle_failure(&tracker, &config, &workflow, &state_store, &issue_run, &error) + .expect("exhausted zero-evidence failure should require attention"); + + assert_eq!( + tracker.state_updates.borrow().last(), + Some(&(issue.id.clone(), String::from("state-todo"))) + ); + assert!(tracker.comments.borrow().iter().any(|comment| { + comment.contains("decodex run failed and needs attention") + && comment.contains("app_server_zero_evidence_start_failed") + && comment.contains("verify `decodex probe stdio://`") + })); + assert!( + !tracker + .comments + .borrow() + .iter() + .any(|comment| comment.contains("decodex run failed and will retry")), + "exhausted zero-evidence failure should not keep retrying" + ); +} + +#[test] +fn retryable_startup_transport_failure_does_not_promote_to_zero_evidence_attention() { + let (_temp_dir, config, _workflow) = temp_project_layout(); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let issue = sample_issue("In Progress", &[]); + let issue_run = IssueRunPlan { + issue: issue.clone(), + issue_state: issue.state.name.clone(), + initial_issue_state: String::from("Todo"), + worktree: WorktreeSpec { + branch_name: String::from("x/pubfi-pub-101"), + issue_identifier: issue.identifier.clone(), + path: config.worktree_root().join("PUB-101"), + reused_existing: false, + }, + retry_project_slug: issue + .project_slug + .clone() + .expect("sample issue should carry a project slug"), + dispatch_mode: IssueDispatchMode::Normal, + attempt_number: 1, + run_id: String::from("pub-101-attempt-1-123"), + retry_budget_base: 0, + }; + + fs::create_dir_all(&issue_run.worktree.path).expect("worktree path should exist"); + + state_store + .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") + .expect("run attempt should record"); + + let error = orchestrator::promote_zero_evidence_app_server_start_failure( + &config, + &state_store, + &issue_run, + Report::new(AppServerTransportFailure::with_phase( + String::from("App-server stdout disconnected unexpectedly."), + "thread/start", + true, + )), + ); + + assert!( + error.downcast_ref::().is_some(), + "startup transport failures should stay retryable instead of becoming zero-evidence terminal attention" + ); + assert!( + error + .downcast_ref::() + .is_none() ); + + let events = state_store + .list_private_execution_events( + config.service_id(), + &issue.id, + &issue_run.run_id, + issue_run.attempt_number, + ) + .expect("private evidence should list"); + + assert!(events.is_empty()); +} + +#[test] +fn retryable_turn_failure_does_not_promote_to_zero_evidence_attention() { + let (_temp_dir, config, _workflow) = temp_project_layout(); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let issue = sample_issue("In Progress", &[]); + let issue_run = IssueRunPlan { + issue: issue.clone(), + issue_state: issue.state.name.clone(), + initial_issue_state: String::from("Todo"), + worktree: WorktreeSpec { + branch_name: String::from("x/pubfi-pub-101"), + issue_identifier: issue.identifier.clone(), + path: config.worktree_root().join("PUB-101"), + reused_existing: false, + }, + retry_project_slug: issue + .project_slug + .clone() + .expect("sample issue should carry a project slug"), + dispatch_mode: IssueDispatchMode::Normal, + attempt_number: 1, + run_id: String::from("pub-101-attempt-1-123"), + retry_budget_base: 0, + }; + + fs::create_dir_all(&issue_run.worktree.path).expect("worktree path should exist"); + + state_store + .record_run_attempt(&issue_run.run_id, &issue.id, issue_run.attempt_number, "failed") + .expect("run attempt should record"); + + let error = orchestrator::promote_zero_evidence_app_server_start_failure( + &config, + &state_store, + &issue_run, + Report::new(AppServerTurnFailure::new( + "thread-1", + Some(String::from("turn-1")), + "failed", + "You've hit your usage limit.", + Some(String::from("usageLimitExceeded")), + )), + ); + + assert!( + error.downcast_ref::().is_some(), + "structured turn failures should stay retryable instead of becoming zero-evidence terminal attention" + ); + assert!( + error + .downcast_ref::() + .is_none() + ); + + let events = state_store + .list_private_execution_events( + config.service_id(), + &issue.id, + &issue_run.run_id, + issue_run.attempt_number, + ) + .expect("private evidence should list"); + + assert!(events.is_empty()); } #[test] diff --git a/apps/decodex/src/orchestrator/types.rs b/apps/decodex/src/orchestrator/types.rs index b095d205c..bce5a4788 100644 --- a/apps/decodex/src/orchestrator/types.rs +++ b/apps/decodex/src/orchestrator/types.rs @@ -185,6 +185,7 @@ impl PullRequestReadbackRootCause { enum RetainedReviewLaneLoad { Skip, + Wait(String), Ready(Box), Blocked(Box), } @@ -906,7 +907,7 @@ impl Display for StalledRunNeedsAttention { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { write!( f, - "Run `{}` for issue `{}` stalled after {:?} without app-server activity; stop automatic execution and repair manually.", + "Run `{}` for issue `{}` stalled after {:?} without app-server activity; reconcile through the retry budget before requiring operator attention.", self.run_id, self.issue_identifier, self.idle_for ) } diff --git a/docs/reference/operator-control-plane.md b/docs/reference/operator-control-plane.md index 2bdc8cb7b..5a9163061 100644 --- a/docs/reference/operator-control-plane.md +++ b/docs/reference/operator-control-plane.md @@ -178,6 +178,20 @@ failure or child exit, ran the registered repo gate itself, persisted the next p and scheduled continuation instead of writing `decodex:needs-attention`. It is a runtime recovery handoff, not final issue success; the later `handoff_evidence` phase still owns review, push, PR creation, and terminal finalize. +Retry comments with `phase_goal_terminal_path_missing` mean a phase goal reached +`complete` before the required Decodex terminal tool path was recorded. The lane is +still runtime-owned while retry budget remains; the next attempt re-enters the +persisted phase and must record review handoff, closeout, or manual attention before +the issue can leave automation ownership. +Retry comments with `app_server_transport_disconnected` during `initialize`, +`account/login/start`, `thread/start`, or `thread/resume` mean Decodex is restarting +the app-server under the retry budget, not asking for operator attention yet. The +same error class becomes actionable only after retry exhaustion or when the disconnect +occurred after a thread session was attached. +Retry comments with `app_server_usage_limit_exceeded` mean the active Codex account +hit a capacity limit and Decodex will re-run account selection on the next attempt. +They are actionable only after retry exhaustion or when the operator intentionally +pins all new runs to an exhausted fixed account. ## State Ownership @@ -442,9 +456,17 @@ Worktree visibility follows the owning dashboard section: dashboard readback also carry `readback_root_cause` when Decodex can classify the local diagnostic safely, for example `missing_github_cli`, `missing_github_token`, `github_auth_failed`, `github_api_read_failed`, `github_response_parse_failed`, - `pull_request_shape_read_failed`, or `lineage_validation_failed`. These diagnostic + `pull_request_shape_read_failed`, or `lineage_validation_failed`. This warning is a + wait/retry lane, not passive manual attention, unless the post-review classification + decision itself is `Block`. These diagnostic tokens are operator-local and must not include tokens, raw API payloads, or private command output. +- `worktree_checkout_branch_read_failed` and `worktree_head_read_failed` in + `Review & Landing` are degraded local worktree readbacks for a still-bound retained + lane. They may block a fresh classification for this status tick, but they must stay + wait/retry readback conditions and must not add `decodex:needs-attention` unless a + later successful readback proves a hard blocker such as a missing branch, branch + mismatch, missing head, or lineage mismatch. - `pull_request_merge_state_conflict` in `Review & Landing` means one retained post-review readback looked merge-complete but direct PR merge readback did not confirm that the same PR head is merged. Treat it as a readback contradiction, not a @@ -471,7 +493,11 @@ Worktree visibility follows the owning dashboard section: state even when `queued_candidates` is empty and no active or post-review lane currently owns the issue. A terminal Run Ledger attention row without a retained worktree, queued attention row, active or needs-attention tracker label, or blocked - post-review lane is history-only and must not inflate current attention. + post-review lane is history-only and must not inflate current attention. When the + same issue is currently owned by a non-attention `Review & Landing` row such as + `wait_for_review` or `ready_to_land`, that row controls the current action summary; + stale active-label or worktree echoes from an older terminal ledger record stay in + Run Ledger history instead of reappearing as current attention. - If private evidence shows `phase_goal_recovery` followed by a queued continuation, the lane is not a retained-attention worktree even when the preceding child failed. Treat it as Decodex-owned re-entry into the next phase unless a later terminal Run diff --git a/docs/runbook/lane-control-recovery.md b/docs/runbook/lane-control-recovery.md index 8610077f3..4c6a83700 100644 --- a/docs/runbook/lane-control-recovery.md +++ b/docs/runbook/lane-control-recovery.md @@ -214,8 +214,8 @@ Use manual attention when: The valid owned-agent path is: -1. add the configured `decodex:needs-attention` label -2. call `issue_comment` with `kind = "manual_attention"` and structured public fields +1. request the configured `decodex:needs-attention` label through `issue_label_add` +2. call `issue_comment` with `kind = "manual_attention"` and structured public fields so Decodex can validate the blocker and apply the label 3. call `issue_terminal_finalize(path = "manual_attention")` Keep host-local paths, private payloads, raw steer text, process diagnostics, account diff --git a/docs/runbook/self-dogfood-pilot.md b/docs/runbook/self-dogfood-pilot.md index 05eee4884..a5aceb216 100644 --- a/docs/runbook/self-dogfood-pilot.md +++ b/docs/runbook/self-dogfood-pilot.md @@ -369,10 +369,13 @@ wants to observe the self-bootstrap loop without reading source code. pinned to the shared `$HOME/.codex` Codex home and state home; do not repair this by assigning a per-account `CODEX_HOME` or by overriding model, sandbox, approval, or personality settings from project policy. - If the status cause is `app_server_plugin_list_timeout`, inspect the local - `app_server_preflight_failed` evidence for the `plugin/list` timeout, restart - `decodex serve` if the app-server process is stale, and run `decodex probe` until - plugin inventory responds before clearing `decodex:needs-attention`. + If the run reports retryable `app_server_plugin_list_timeout`, leave the issue in + its active retry state and inspect the local `app_server_preflight_failed` + evidence only if the retry budget exhausts or the timeout repeats. If status + reports `attention_cause: app_server_plugin_list_timeout`, inspect the local + evidence for the `plugin/list` timeout, restart `decodex serve` if the app-server + process is stale, and run `decodex probe` until plugin inventory responds before + clearing `decodex:needs-attention`. If preflight evidence shows `skills/list` enabled skills with scan diagnostics, keep the diagnostics as compatibility evidence and do not uninstall official skills solely to clear the scan error. Only missing cwd coverage or zero enabled skills are @@ -667,7 +670,7 @@ The running-lane reconciliation rules are: - terminal issue: stop the lane, mark the run `terminated`, and remove the worktree - non-terminal issue that has left both `In Progress` and any configured startable pre-claim state: stop the lane, mark the run `interrupted`, and keep the worktree - issue still sitting in a startable state during early startup: leave it alone for that tick so the child can finish its initial tracker transition -- stalled lane with no app-server activity through the idle budget: stop the lane, mark the run `stalled`, and move the issue back through the human-attention failure path for manual repair +- stalled lane with no app-server activity through the idle budget: stop the active attempt, mark the run `stalled`, and retry the same owned lane while retry budget remains; use the human-attention failure path only after retry exhaustion, retained tracked partial progress, or another terminal boundary - child already exited before the next tick: still inspect persisted protocol activity so idle-timeout exits converge as `stalled` ## Worktree behavior @@ -793,6 +796,10 @@ repo gate commands. Linear should carry only the coarse team-visible failure sum ## Re-running after failure - If the run is still retryable, leave the issue in `In Progress` and let `decodex` retry. +- If the retryable failure class is `app_server_plugin_list_timeout`, treat it as a + bounded app-server preflight timeout before `thread/start`: leave the active retry + in place and inspect/restart the local serve process only if the timeout repeats or + the retry budget exhausts. - If `execution.max_turns` is greater than `1`, one bounded worker may now reuse the same app-server thread for multiple turns before it yields. - Retryable control-plane retries now split into a short continuation retry after a clean nonterminal worker exit and a capped exponential failure backoff after an abnormal worker exit. - If `status` reports `Git credential preflight failed`, configure the env-var named by `github.token_env_var` for the routed identity before clearing `decodex:needs-attention`; the lane never reached a promptable `git push`. @@ -804,11 +811,12 @@ repo gate commands. Linear should carry only the coarse team-visible failure sum login state; for home mismatches, ensure `HOME` points at the user account that owns the shared `$HOME/.codex` tree. Restart `decodex serve` before clearing `decodex:needs-attention`. -- If status reports `attention_cause: app_server_plugin_list_timeout`, treat it as a - bounded app-server preflight timeout before `thread/start`: inspect the retained - worktree's local preflight evidence for `plugin/list`, restart `decodex serve` if - stale, verify `decodex probe`, then clear `decodex:needs-attention` and move the - issue back to a startable state only when another automated run is desired. +- If status reports `attention_cause: app_server_plugin_list_timeout`, the workflow + retry budget has exhausted for a bounded app-server preflight timeout before + `thread/start`: inspect the retained worktree's local preflight evidence for + `plugin/list`, restart `decodex serve` if stale, verify `decodex probe`, then clear + `decodex:needs-attention` and move the issue back to a startable state only when + another automated run is desired. - If status or Linear terminal failure includes a `skills/list` preflight blocker, inspect the attached `first_error_path` and `first_error` details before changing local plugins or skills. Scan diagnostics with enabled skills are not blockers by diff --git a/docs/spec/app-server.md b/docs/spec/app-server.md index 4a1c1d674..4cfa0104f 100644 --- a/docs/spec/app-server.md +++ b/docs/spec/app-server.md @@ -201,8 +201,11 @@ cwd and at least one enabled skill. Decodex must preserve the scan error count a first error details in local preflight evidence, but it must not block the lane solely because unrelated installed skill metadata failed to scan. Because `plugin/list` is observational and local-marketplace-only, Decodex may retry -one app-server output timeout before failing the lane. If the retry is exhausted, -the terminal failure must remain an app-server preflight failure, report +one app-server output timeout inside the preflight request before failing that request. +If the preflight request retry is exhausted, the lane must still enter structured +runtime retry while workflow retry budget remains. Only after workflow retry budget +exhaustion may the terminal failure become operator-facing attention; when that +happens it must remain an app-server preflight failure, report `app_server_plugin_list_timeout`, and include the `plugin/list` timeout cause in local preflight evidence and operator recovery output rather than looking like a repository implementation failure. @@ -236,6 +239,11 @@ project-scoped fixed account. Successful selection writes `last_selected_at_unix_epoch` back to the JSONL file, and selection holds a pool-local lock so concurrent run dispatches observe the latest selector state instead of all choosing from the same stale snapshot. +If a turn later fails with `codexErrorInfo = "usageLimitExceeded"`, Decodex treats it +as a retryable capacity failure while retry budget remains. The current turn stops +immediately, but the next attempt re-enters normal account selection so the pool can +refresh usage and choose another usable account. Only retry-budget exhaustion makes +that error a human-required `app_server_usage_limit_exceeded` stop. Decodex owns token freshness for injected `chatgptAuthTokens`. It proactively refreshes an account before probing when the access-token JWT `exp` is expired. If no expiration @@ -491,7 +499,17 @@ Rationale: ## Error handling -- JSON-RPC transport failure before `thread/start` succeeds is a retryable startup failure. +- JSON-RPC transport failure before a thread session is attached is a retryable + startup failure. This covers the client waiting on `initialize`, + `account/login/start`, `thread/start`, or `thread/resume`. +- If that startup transport failure exhausts the registered retry budget, the + terminal failure must still preserve `app_server_transport_disconnected`. +- JSON-RPC transport failure after a thread session is attached, including + `turn/start` or turn execution waits, is a human-required transport failure + because blind retry can duplicate turn-side effects. +- Turn failure with `codexErrorInfo = "usageLimitExceeded"` is a retryable capacity + failure until the registered retry budget is exhausted, so account-pool re-selection + can recover without operator attention. - `thread/status/changed` with `systemError` is a failed run. - Turn completion with codex error information must be classified into: - retryable failure diff --git a/docs/spec/lane-control.md b/docs/spec/lane-control.md index 962a0105c..138a04e13 100644 --- a/docs/spec/lane-control.md +++ b/docs/spec/lane-control.md @@ -50,7 +50,7 @@ agent-facing skills must guide responsible use. | Hard interrupt fallback | Explicit fallback only | `decodex lane interrupt --run-id --force` and `POST /api/lane/interrupt` with `"force": true` classify process signaling as `hard_interrupt_fallback` | Use only when soft interrupt is unavailable, timed out, or impossible because the process, app-server boundary, or active lease cannot be reached. A forced interrupt may signal the recorded child process after `active_lease_missing` only when inspection still identifies the same issue, run id, attempt, channel, and live process. Preserve retained worktree evidence and runtime classification. | | Steer active lane | Supported CLI/API control; bottom-layer method stays broad | `decodex lane steer --run-id --expected-turn-id --message `, canonical `POST /api/lane/steer`, legacy alias `POST /api/lane-steer`, local run-control steer request/response files, app-server `turn/steer`, private `control_action` audit events, and protocol activity `turn/steer` summaries | Pass operator-supplied steer text through CLI/API to the current active turn. Require `expectedTurnId`; stale turn ids fail closed. Do not narrow the protocol to a fixed set of task-content categories. Apply policy, audit, privacy, and lifecycle guardrails above the protocol. | | Retained resume/retry | Supported through runtime lifecycle | `decodex run `, retry scheduling, retained worktree recovery, and `thread/resume` for same-thread app-server continuation | Resume only when retained worktree, issue, branch, PR, and runtime evidence still prove the same lane. Treat ambiguous lineage as manual attention. | -| Manual attention | Supported terminal control path | `decodex:needs-attention`, `issue_comment(kind = "manual_attention")`, and `issue_terminal_finalize(path = "manual_attention")` | Stop automation when policy requires a human decision. Explain the blocker through structured public fields and keep private evidence local. | +| Manual attention | Supported terminal control path | `issue_label_add` intent for `decodex:needs-attention`, `issue_comment(kind = "manual_attention")`, and `issue_terminal_finalize(path = "manual_attention")` | Stop automation when policy requires a human decision. Explain the blocker through structured public fields and keep private evidence local. | | Task replacement | Deferred lifecycle work | No supported active-lane replacement command | Do not use steer or raw injection to replace the task silently. Treat replacement as explicit lifecycle work: pause/stop if needed, update or requeue the issue, or create a new issue/lane. | | Raw thread item injection | Unsupported as an operator feature | No Decodex operator path for `thread/inject_items` | Do not expose raw `thread/inject_items` to operators in this rollout. Use `turn/steer` for operator steer. | | Active-lane UI authoring controls | Deferred | Existing dashboard views and low-level handlers are not the CLI/API-first lane-control contract | Do not add dashboard steer, retry, or task-replacement controls in this rollout. Ship CLI/API first, then promote UI controls only after audit and policy behavior is settled. | @@ -233,8 +233,8 @@ contract, authority decision, or retained-progress ownership decision. Agents must not simulate manual attention by editing tracker state directly. The valid agent path is: -1. add the configured `decodex:needs-attention` label -2. call `issue_comment` with `kind = "manual_attention"` and structured public fields +1. request the configured `decodex:needs-attention` label through `issue_label_add` +2. call `issue_comment` with `kind = "manual_attention"` and structured public fields so Decodex can validate the blocker and apply the label 3. call `issue_terminal_finalize(path = "manual_attention")` Operators may later clear the blocker, clear the label, and requeue the lane through diff --git a/docs/spec/linear-execution-ledger.md b/docs/spec/linear-execution-ledger.md index 1015f69e6..1864adf9a 100644 --- a/docs/spec/linear-execution-ledger.md +++ b/docs/spec/linear-execution-ledger.md @@ -241,8 +241,9 @@ Retained partial progress is a `needs_attention` event with `error_class = "partial_progress_retained"` and `terminal_path = "retained_partial_progress"`. It must describe retained tracked worktree changes and must not be emitted as `terminal_failure`. If the retained -disposition absorbs a later runtime failure, the producer should preserve the source -failure class in `evidence` instead of changing the event type or terminal path. +terminal disposition absorbs a later runtime failure, the producer should preserve the +source failure class in `evidence` instead of changing the event type or terminal +path. Loop guardrail observations are private runtime evidence while autonomous architecture recovery is still available. A public Linear record is written only when the guardrail diff --git a/docs/spec/owned-lane-policy.md b/docs/spec/owned-lane-policy.md index 33871d236..2227f9fea 100644 --- a/docs/spec/owned-lane-policy.md +++ b/docs/spec/owned-lane-policy.md @@ -121,7 +121,8 @@ This action is mandatory when: - retry budget is exhausted - the agent explicitly requests human attention -- the lane is stalled +- stalled reconciliation found retained tracked worktree changes that require + operator recovery, or stalled retry budget is exhausted - terminal signaling is missing or contradictory - required labels cannot be applied and the runtime must fall back to a guarded non-startable state - retained worktree, tracker state, or PR state disagree in a way that is not safely self-healing @@ -149,7 +150,9 @@ This action requires: | Running lane remains eligible and activity is current | Current issue state is still active; no interrupting state transition; activity marker or session state still live | `continue` | Not applicable | | Clean worker exit with remaining retry budget | Retry budget remains; issue is still active; no `decodex:needs-attention` label or equivalent human-attention signal | `retry_automatically` | Yes | | Abnormal worker exit with remaining retry budget | Same as above, plus failure is classified as retryable | `retry_automatically` | Yes | -| Retry exhausted, explicit human-attention signal, or stalled lane | Retry budget exhausted, or `decodex:needs-attention`, or stalled-run evidence | `manual_intervention_required` | No | +| App-server capability preflight method timeout with remaining retry budget | App-server initialized; capability preflight timed out; workflow retry budget remains; no hard config/model/provider/skills/plugin/MCP blocker was observed | `retry_automatically` | Yes | +| Stalled active run with no retained tracked changes and remaining retry budget | Issue still has active ownership; retry budget remains; worktree has no retained tracked changes requiring operator recovery | `retry_automatically` | Yes | +| Retry exhausted, explicit human-attention signal, or retained stalled partial progress | Retry budget exhausted, or `decodex:needs-attention`, or stalled-run evidence with retained tracked changes | `manual_intervention_required` | No | | Human-attention label is unavailable but the failure path still must block redispatch | Failure path is human-required; label application failed; guarded retained marker recorded | `manual_intervention_required` | No | | Retained non-terminal lane still matches issue, branch, and owned recovery intent | Retained worktree exists; issue still belongs to the owned lane; recovery signals are consistent | `resume_retained_lane` | Yes | | `In Review` lane has no actionable review yet | PR still belongs to lane; no requested changes that require repair; checks or review are still pending | `wait_for_external_signal` | Yes, when new signal arrives | @@ -212,6 +215,15 @@ default. Human intervention is not complete merely because a human observed the failure. Human intervention is complete only when the blocking signal is materially cleared. +Retained tracked worktree changes do not by themselves convert a retryable abnormal +exit into `manual_intervention_required`. When issue ownership still matches and +retry budget remains, the next retry must resume the same worktree and treat the +patch as recovery context. Retained partial progress becomes human-required only when +the failure is non-retryable, explicit human attention was requested, retry or +loop-recovery budget is exhausted, stalled reconciliation found retained tracked +changes requiring operator recovery, or authoritative ownership signals are +contradictory enough that continuing would require guessing. + Examples of materially cleared signals: - `decodex:needs-attention` is removed and the issue is returned to a startable state diff --git a/docs/spec/post-review-lifecycle.md b/docs/spec/post-review-lifecycle.md index 061b26e93..87958a64d 100644 --- a/docs/spec/post-review-lifecycle.md +++ b/docs/spec/post-review-lifecycle.md @@ -75,12 +75,18 @@ If these signals disagree and the disagreement cannot be resolved without guessi not infer a PR lineage from branch names, current heads, PR titles, or Linear comments, and `decodex run` must not repair this state automatically. -When the retained review handoff marker exists but a direct PR-state read fails, -operator status must degrade the readback instead of replacing the bound lane with a -null-PR blocked state. The status row must keep the issue identifier, retained branch, -marker PR URL, and marker head SHA, and it may expose -`readback_warning = "pull_request_state_read_failed"` until the next successful PR -state refresh. +When the retained review handoff marker exists but a direct PR-state read or local +worktree branch/head read fails, operator status must degrade the readback instead of +replacing the bound lane with a null-PR blocked state. The status row must keep the +issue identifier, retained branch, marker PR URL, and marker head SHA, and it may expose +warnings such as `readback_warning = "pull_request_state_read_failed"`, +`worktree_checkout_branch_read_failed`, or `worktree_head_read_failed` until the next +successful readback. +Retained orchestration must preserve degraded readback as a wait state. It must not +convert `pull_request_state_read_failed`, `worktree_checkout_branch_read_failed`, +`worktree_head_read_failed`, or other `WaitForReview` classifications into passive +manual attention; only classifications whose decision is `Block` may add +`decodex:needs-attention`. When Linear issue metadata readback is degraded by connector backoff, operator status must still keep locally retained handoff rows visible with the marker PR URL and head @@ -213,6 +219,9 @@ While in `review_wait`: - the retained lane remains reserved for the same issue and PR lineage - missing immediate review activity is not, by itself, a failure - review-request acknowledgement probing or bounded resend may happen as orchestration behavior without leaving `review_wait` +- before requesting external review, pending or unknown check readback waits for a + later status tick, and red checks route to `review_repair`; neither case may apply + `decodex:needs-attention` by itself `review_wait` must not trigger code changes on its own. diff --git a/docs/spec/runtime.md b/docs/spec/runtime.md index 957ae552d..a21f8e66f 100644 --- a/docs/spec/runtime.md +++ b/docs/spec/runtime.md @@ -229,8 +229,8 @@ The runtime state machine is local to `decodex`. It is not a replacement for Lin | --- | --- | --- | | `discovered` | The issue was listed from Linear and passed the eligibility filter. | Acquire lease or skip on conflict. | | `leased` | `decodex` created the local lease and reserved the issue for one attempt. | Worktree bootstrap starts or lease fails. | -| `worktree_ready` | The issue lane exists locally and is ready for execution. | `app-server` session starts. | -| `running` | `decodex` has an active `app-server` thread for the issue and may start one or more bounded turns on that thread. | A terminal completion path resolves, the bounded continuation budget is exhausted, the issue becomes non-active, transport fails, or policy violation occurs. | +| `worktree_ready` | The issue lane exists locally and is ready for execution. | `app-server` session starts, or startup transport failure enters the retry budget. | +| `running` | `decodex` has an active `app-server` thread for the issue and may start one or more bounded turns on that thread. | A terminal completion path resolves, the bounded continuation budget is exhausted, the issue becomes non-active, post-thread transport fails, or policy violation occurs. | | `validating` | Agent execution finished and the repo-native gate (`canonicalize_commands`, then `verify_commands`) is running. | The repo gate passes or fails. | | `retry_wait` | The control plane is holding a queued retry entry for the leased lane after a clean continuation exit or a failure with remaining retry budget. | The queued retry revalidates and starts, the queued issue becomes non-active and the claim is released, or operator intervention cancels retries. | | `needs_attention` | Retry budget is exhausted or human intervention is required. | Human updates the issue and it becomes eligible again. | @@ -248,7 +248,9 @@ After each `app-server` turn completes, `decodex` must resolve one continuation - The agent recorded a valid PR-backed review handoff and did not request human attention. - `decodex` proceeds into `validating`, then applies the success writeback if the repo gate passes. - `manual_attention` - - The agent explicitly requested human attention by adding `decodex:needs-attention` and did not also record review handoff. + - The agent explicitly requested human attention with the `decodex:needs-attention` + label intent, left a validated explanatory comment, and did not also record + review handoff. - `decodex` skips success writeback and the post-run repo gate, then enters the human-required failure flow immediately. - invalid completion signaling - If the turn records both signals, or records one terminal path but fails to finalize it explicitly, the attempt is invalid and must fail rather than guessing a completion path. @@ -303,6 +305,22 @@ does not apply to `handoff_evidence`, explicit manual-attention terminal intent, unsupported phase-goal app-server methods, repo-gate human-attention failures, or runs with no current active phase-goal signal. +App-server JSON-RPC transport failures keep the phase where the disconnect occurred. +Disconnects before a durable thread session is attached (`initialize`, +`account/login/start`, `thread/start`, or `thread/resume`) are startup failures and +must flow through the retry budget before any `decodex:needs-attention` writeback. +If the retry budget is exhausted, the terminal failure still uses +`app_server_transport_disconnected`. Disconnects after the thread session boundary, +including `turn/start` and turn execution, remain human-required because automatic +retry can duplicate turn-side effects. + +App-server turn failures with `codexErrorInfo = "usageLimitExceeded"` are runtime +capacity failures, not immediate operator-attention requests. Decodex must stop the +current turn, record a retry with `app_server_usage_limit_exceeded` while retry budget +remains, and let the next attempt re-run account-pool usage probes and account +selection. If the retry budget is exhausted, the same error class becomes a normal +human-required attention stop. + Phase-goal telemetry is local runtime evidence. It must distinguish `goal_complete`, `validation_pass`, `validation_fail`, `active_goal_recovered`, review `clean`, review `findings`, terminal `review_handoff`, and terminal @@ -358,12 +376,12 @@ Before applying success or failure writeback, `decodex` must classify the finish | Disposition | Required agent signal | Forbidden co-signal | Runtime effect | | --- | --- | --- | --- | | `review_handoff` | `issue_review_handoff` plus `issue_terminal_finalize(path = "review_handoff")` | `decodex:needs-attention` | Run the repo-native gate, revalidate PR state, post completion comment, transition to `In Review`. | -| `manual_attention` | `decodex:needs-attention` plus an explanatory public issue summary, then `issue_terminal_finalize(path = "manual_attention")` | `issue_review_handoff` | Skip PR-backed success writeback and the repo-native gate, then treat the run as a human-required failure immediately. | +| `manual_attention` | `issue_label_add` with `decodex:needs-attention` intent, a validated explanatory public issue summary that applies that label, then `issue_terminal_finalize(path = "manual_attention")` | `issue_review_handoff` | Skip PR-backed success writeback and the repo-native gate, then treat the run as a human-required failure immediately. | If neither signal exists, or both signals exist, `decodex` must fail the attempt instead of inferring operator intent. -If the label is recorded without the required explanatory comment, `decodex` must also fail the attempt instead of treating it as a valid `manual_attention` exit. +If the label intent is recorded without the required explanatory comment, `decodex` must also fail the attempt instead of treating it as a valid `manual_attention` exit. If the resolved terminal path is not explicitly finalized through `issue_terminal_finalize`, the app-server wrapper must fail the turn before `decodex` records the attempt as successful. -The explanatory public summary for `manual_attention` must describe the exact observed blocker and should include the failed command plus raw error text only when those values are public-safe, instead of speculating about unverified capability limits. +The explanatory public summary for `manual_attention` must describe the exact observed blocker and should include the failed command plus raw error text only when those values are public-safe, instead of speculating about unverified capability limits. It must not reuse runtime-owned retry or continued-repair `error_class` values such as app-server timeout/turn failures, stalled-run detection, or repo-gate validation failure classes; those remain Decodex-owned retry, continuation, or architecture-recovery signals until the runtime itself reaches a human-required terminal boundary. Execution-state checkpoints are durable progress overlays only. Their phase, focus, next action, blockers, evidence, or verification fields are never a substitute for the explicit terminal-finalization call. After successful completion writeback, Decodex must best-effort archive every locally recorded terminal Codex thread for the issue, including earlier failed retry attempts, so old attempts do not keep the issue visible in the Codex conversation list. @@ -499,6 +517,20 @@ idempotency fields are defined by ### Failure writeback This path applies to retryable failures, retry exhaustion, and explicit `manual_attention` exits. +Before writing a retry comment, transitioning an issue, or applying +`decodex:needs-attention`, Decodex must classify the failure through one writeback +disposition: generic retryable failure, structured retryable recovery, or +human-required terminal attention. Structured retryable recovery includes typed runtime +failures such as zero-evidence app-server startup failures, stalled active-run +reconciliation without retained tracked changes, app-server capability preflight +timeouts, startup transport disconnects, turn failures, dynamic-tool failures, and +retryable repo-gate failures; +those failures must not be reclassified as zero-evidence startup attention merely +because protocol-event persistence lagged. A zero-evidence startup failure may record +private startup diagnostics, but it remains automatic retry work while retry budget +remains and only becomes operator-facing attention after retry exhaustion. +Retry scheduling, terminal writeback, and public `error_class`/`next_action` text must +use that same classification instead of maintaining separate ad hoc failure tables. Retryable failures with remaining budget: @@ -521,7 +553,10 @@ Retry-exhausted or human-required failures: 3. Post a structured failure comment. 4. Finalize the terminal path with `issue_terminal_finalize(path = "manual_attention")`. -If the coding agent explicitly requests human attention by adding `decodex:needs-attention`, `decodex` must stop automatic retries for that attempt, skip PR-backed success writeback, and treat the lane as a human-required failure immediately. +If the coding agent explicitly requests human attention with a +`decodex:needs-attention` label intent and the paired `manual_attention` comment +validates, `decodex` must stop automatic retries for that attempt, skip PR-backed +success writeback, and treat the lane as a human-required failure immediately. The paired explanatory comment must use the issue-scoped `issue_comment` allowlist, currently kind `manual_attention`, so the Linear-visible summary is rendered from structured public fields instead of an arbitrary agent-authored body. Private command @@ -641,7 +676,7 @@ or `stalled` while current marker, active thread, or active work-protocol eviden still identifies the same `run_id` and `attempt_number` as live, operator status must keep the lane visible with the process/protocol liveness details instead of hiding it as only terminal history or cleanup work. -Post-review ownership is stored in the runtime database. Retained handoff rows record the authoritative PR URL, branch lineage, validated PR head OID, run id, and attempt number that completed the `In Review` handoff. Retained orchestration rows record the current post-review phase for that exact handoff identity. If the matching database row is missing, post-review ownership must block as unresolved instead of rebinding from branch-name, current-head, Linear comments, or other heuristics. An explicit operator manual takeover command may adopt a human-owned PR into this same retained database shape only after validating the managed clean worktree, PR repository, default-branch target, exact branch/head match, and green landable PR gates. If the active service label is missing but exists on the issue team, live adopt may restore it after all other invariants pass and must roll that restoration back if the audit write fails. If a retained review marker exists but a stored handoff or orchestration head no longer matches a clean retained worktree and matching PR head, operator status must keep the marker PR URL visible when known and recovery diagnosis must report the concrete mismatched field before any explicit rebind refresh. When retained PR readback degrades but the handoff identity is still safe to preserve, operator-local status may expose a typed `readback_root_cause` diagnostic such as missing GitHub CLI, missing GitHub token, GitHub auth failure, API/read failure, parse/shape failure, or lineage validation failure while keeping public-safe warning reasons such as `pull_request_state_read_failed` stable. +Post-review ownership is stored in the runtime database. Retained handoff rows record the authoritative PR URL, branch lineage, validated PR head OID, run id, and attempt number that completed the `In Review` handoff. Retained orchestration rows record the current post-review phase for that exact handoff identity. If the matching database row is missing, post-review ownership must block as unresolved instead of rebinding from branch-name, current-head, Linear comments, or other heuristics. An explicit operator manual takeover command may adopt a human-owned PR into this same retained database shape only after validating the managed clean worktree, PR repository, default-branch target, exact branch/head match, and green landable PR gates. If the active service label is missing but exists on the issue team, live adopt may restore it after all other invariants pass and must roll that restoration back if the audit write fails. If a retained review marker exists but a stored handoff or orchestration head no longer matches a clean retained worktree and matching PR head, operator status must keep the marker PR URL visible when known and recovery diagnosis must report the concrete mismatched field before any explicit rebind refresh. When the retained handoff still matches the same branch and PR, and PR readback plus local worktree lineage have already accepted the current head as the current PR head, a stale retained orchestration `head_sha` may be rebound to that current head by resetting the orchestration phase to `request_pending`, clearing prior GitHub Review request metadata, and preserving round-count history. Branch, PR, handoff-lineage, or rewritten-history mismatches must continue to block or report for operator recovery instead of being silently rebound. When retained PR readback degrades but the handoff identity is still safe to preserve, operator-local status may expose a typed `readback_root_cause` diagnostic such as missing GitHub CLI, missing GitHub token, GitHub auth failure, API/read failure, parse/shape failure, or lineage validation failure while keeping public-safe warning reasons such as `pull_request_state_read_failed` stable. Retained orchestration must preserve the post-review classification decision when it converts status readback into runtime action: only a `Block` classification may write passive retained manual attention or add `decodex:needs-attention`; degraded readback classified as `WaitForReview` must remain a wait/retry status row and must not be promoted to manual attention by the run-cycle path. The only source-tree runtime artifacts that clean-source checks may ignore are the untracked top-level `.decodex-run-activity` heartbeat marker and `.decodex-run-control/` local control-channel directory. Durable review handoff, orchestration, review-policy checkpoints, retry, phase timing, and retained PR state belong in the Decodex runtime database, not in root-level or worktree-local review marker files. If the heartbeat marker carries similarly named fields for compatibility or operator diagnostics, those breadcrumb values cannot override runtime-store rows. ### Dispatch-slot handoff invariant @@ -734,6 +769,12 @@ After a process restart, recent-run history, active lease ownership, retained po - While the control plane is running an active lane, that child must keep the workflow snapshot it started with; project `WORKFLOW.md` reloads affect later decisions without restarting the in-flight child. - While the control plane is supervising an active child process, stall detection must consult the child-updated `.decodex-run-activity` marker for the current `run_id` plus `attempt_number` and the persisted runtime event journal. A retained marker only proves a live process when its PID is still alive on the current host boot and the process start identity still matches; after power loss, reboot, or same-boot PID reuse, recovery must clear the reconstructed lease and re-enter the retained lane through retry-style dispatch instead of preserving the old running state. - Retry-style recovery prompts must tell the next agent to treat the current worktree, tracker state, runtime-store records, and protocol events as durable truth, use marker files only as diagnostic liveness breadcrumbs, inspect the branch/diff/recent validation evidence first, and continue from partial work rather than assuming prior in-memory model/tool state survived. +- Retained retry-budget markers belong only to the same automatic recovery episode: + retry, review-repair, closeout, and other retry-style dispatch may inherit the + marker so crash/restart recovery does not mint extra attempts. Normal queued intake + starts a new automatic episode after a human has made the issue eligible again, so it + must not inherit an old retained marker's exhausted retry budget unless a caller + supplied an explicit preferred retry-budget base for that new run. - While the control plane owns a queued retry entry, that queued claim must take priority over normal candidate selection for the affected project. - While the control plane evaluates persisted Execution Programs, program-owned ready nodes may receive `decodex:queued:` only when their mapped Linear issue @@ -758,17 +799,33 @@ After a process restart, recent-run history, active lease ownership, retained po - A leased issue that is still in a configured startable state during early control-plane ticks must be treated as a lane that has not finished claiming tracker ownership yet, not as an immediate non-active interruption. - If a running attempt exceeds the app-server idle timeout, `decodex` must treat it as stalled, stop the active run, and mark the attempt `stalled`. - If stalled reconciliation finds tracked changes in the retained worktree, it must classify the lane as retained partial progress directly. This path must write a human-required `needs_attention` ledger record with `error_class = "partial_progress_retained"` and `terminal_path = "retained_partial_progress"` instead of first routing the lane through `stalled_run_detected` or `terminal_failure`. -- If stalled reconciliation finds no tracked changes in the retained worktree, it must converge the issue through the existing human-required failure path with `error_class = "stalled_run_detected"` instead of silently retrying in this phase. +- If stalled reconciliation finds no tracked changes in the retained worktree, it must classify the lane as structured retryable recovery with `error_class = "stalled_run_detected"` while retry budget remains. The retry must keep active ownership, write a failure retry schedule for the same worktree, and must not add `decodex:needs-attention` until retry budget exhaustion or another terminal boundary applies. - If the supervised child already exited before the next control-plane tick, stalled reconciliation must still inspect the just-finished lane using recorded protocol activity and retained worktree state rather than skipping directly to generic failure handling. - Operator status snapshots must expose structured liveness and wait-state fields derived from runtime records plus marker breadcrumbs, including current phase, optional wait reason, current operation, last run/protocol/progress times, idle age, a soft `suspected_stall` signal, optional progress diagnostics, and any queued retry kind plus due time, so operators can distinguish active execution from continuation waits, retry backoff, early stall suspicion, and genuine hard stalls without inferring progress from filesystem churn. `last_progress_at` is meaningful-work progress only: tool calls, file or diff changes, plan/model output, repo validation, PR/review/terminal lifecycle, or other explicit work events may refresh it, but account, rate-limit, phase-goal, passive status, warning, token-usage, heartbeat, or similar non-work protocol traffic must only refresh protocol liveness. When a lane remains in `model_execution` with fresh protocol activity but stale or missing work progress and the recent protocol events are only non-work traffic, status should expose `progress_diagnostic = "protocol_only_activity"` while preserving process and protocol liveness separately. - Operator status snapshots may expose an additive `child_agent_activity` object when app-server protocol events have produced one for the current run. The object must stay machine-readable and dashboard/CLI shared, and should describe dynamic observed buckets rather than a fixed workflow: current child bucket and elapsed time, bucket wall/event/tool counts, current/max/cumulative input tokens, cumulative output tokens, largest tool output, and warnings for repeated large outputs. Missing `child_agent_activity` means no child breakdown was captured; existing JSON consumers must continue to work without it. - If the agent Git credential preflight fails, operator status must report the retained lane as a credential failure requiring operator recovery, not as a still-running lane. - If retry budget or needs-attention recovery finds tracked changes in the retained worktree after active phase-goal recovery has no applicable continuation path, operator status must report retained partial progress rather than only a generic retry-budget hold. Retained progress is the recovery disposition; later runtime, app-server, credential, transport, or repo-gate failure classes must be preserved as source evidence instead of overriding the retained-progress lifecycle path. The failure class may be `partial_progress_retained` when no more specific runtime error class is available. Operators should then inspect the patch, finish validation and PR handoff if it is useful, or reset the retained worktree explicitly. - A retryable runtime or app-server failure that leaves tracked worktree changes must - stop as retained partial progress instead of writing only a generic retry comment. - This does not apply to repo-gate continued-repair failures, because those failures - still authorize bounded automatic repair against the retained patch until retry or - loop-guardrail limits are reached. + keep the owned lane in automatic recovery while retry budget or loop-guardrail + recovery remains. The retained patch is retry context for the same worktree, not by + itself a human-attention signal. When that same failure class exhausts retry budget + or another terminal boundary applies, terminal writeback may classify the retained + patch as `partial_progress_retained` and preserve the runtime, app-server, + credential, transport, or repo-gate class as source evidence. +- If a phase-scoped goal reaches `complete` without the matching Decodex terminal + path, such as handoff evidence finishing without review-handoff, closeout, or + manual-attention finalization, Decodex must treat + `phase_goal_terminal_path_missing` as structured retryable recovery while retry + budget remains. The next attempt re-enters the persisted phase goal and must finish + the required terminal tool path instead of turning goal completion into issue + success. Unsupported app-server goal methods remain hard environment blockers. +- Retained post-review orchestration must treat local branch/head readback failures as + transient wait conditions while the review handoff marker still owns the lane. Status + may report `worktree_checkout_branch_read_failed` or `worktree_head_read_failed`, but + the run-cycle path must not write passive retained manual attention or add + `decodex:needs-attention` for those read failures alone. A later successful readback + may still classify hard blockers such as missing branch, branch mismatch, missing + head, head mismatch, PR mismatch, or another explicit `Block` decision. - If the durable Run Ledger final outcome is `needs_attention` or `terminal_failure`, operator status must count that issue in project-level `attention_count` only when a current attention signal still exists: a retained @@ -778,6 +835,12 @@ After a process restart, recent-run history, active lease ownership, retained po parse `history_lanes` to discover a human-required terminal outcome. A bare terminal Run Ledger attention row with no current owner is history-only ledger evidence; it must remain visible in `history_lanes` without inflating current project attention. + When a non-attention post-review lane currently owns the same issue, such as + `wait_for_review` or `ready_to_land`, that post-review owner controls the current + project attention result; any stale active label or retained worktree echo from the + older terminal ledger must not promote the old Run Ledger outcome back into + `attention_count`. A real `needs_attention_label`, blocked queued row, or blocked + post-review classification still counts as current attention. - If that same issue still carries the service queue label plus the configured `needs_attention_label`, the terminal Run Ledger attention outcome must own the operator projection. Status must not also render the issue as an intake queue @@ -807,6 +870,8 @@ After a process restart, recent-run history, active lease ownership, retained po must run the bounded app-server capability preflight defined in [`app-server.md`](./app-server.md). Missing config/model/provider/skills/plugin/MCP state is a pre-dispatch terminal blocker with an operator-readable error class, not a - promptable agent turn. + promptable agent turn. App-server capability preflight method timeouts are structured + retryable runtime failures while workflow retry budget remains; after retry + exhaustion they may terminalize with their specific app-server preflight error class. - If the local process crashed during a run, `decodex` must recover from the runtime database, current tracker cache or state, and retained worktree inspection. - If Linear shows a non-terminal state but no local lease exists, the issue may become eligible again after reconciliation or may be redispatched through the retained recovered worktree. diff --git a/docs/spec/tracker-tools.md b/docs/spec/tracker-tools.md index b23ef11fc..4d5c77269 100644 --- a/docs/spec/tracker-tools.md +++ b/docs/spec/tracker-tools.md @@ -58,7 +58,9 @@ The follow-up MVP should support these issue-scoped operations: - `issue_review_handoff` - validate and record a PR-backed success handoff for the current issue - `issue_label_add` - - add a label to the current issue when workflow policy requires it + - add an allowlisted immediate label to the current issue when workflow policy + requires it, or record a run-local manual-attention label intent for the + configured `needs_attention_label` - `issue_terminal_finalize` - explicitly finalize the current run's terminal tracker path after the required tracker writes already exist @@ -90,7 +92,9 @@ At turn completion, the issue-scoped tool bridge must leave `decodex` with exact - finalized by `issue_terminal_finalize(path = "review_handoff")` - means the lane is claiming review-ready success - `manual_attention` - - produced by adding the configured `needs_attention_label` and leaving an explanatory comment + - produced by requesting the configured `needs_attention_label`, leaving a + validated explanatory comment, and having Decodex apply the label before writing + the comment - finalized by `issue_terminal_finalize(path = "manual_attention")` - means the lane is explicitly handing the issue back to a human instead of asking for `In Review` @@ -199,18 +203,25 @@ In either invalid case, `decodex` must fail the attempt rather than infer which fails the public-text guard during Decodex-owned writeback, Decodex must use fixed public-safe fallback summary text for the Linear comment and ledger record instead of failing the otherwise valid PR lifecycle transition. -- Adding the configured `needs_attention_label` is an explicit human-required - failure exit for the active lane. In that case the agent must call - `issue_comment` with kind `manual_attention` so Decodex can render the - explanatory `needs_attention` ledger comment, must not also record - `issue_review_handoff`, and `decodex` must stop automatic retries for that - attempt. +- Calling `issue_label_add` with the configured `needs_attention_label` records an + explicit human-required failure intent for the active lane, but it is not an + immediate Linear mutation. In that case the agent must call `issue_comment` with + kind `manual_attention` so Decodex can validate the blocker, apply the + `needs_attention_label`, and render the explanatory `needs_attention` ledger + comment. The agent must not also record `issue_review_handoff`, and `decodex` must + stop automatic retries for that attempt only after the paired manual-attention exit + validates. - Human-attention comments must describe the exact observed blocker through structured public fields: `error_class`, `next_action`, `blockers`, and `evidence`. `failed_command` and `raw_error` may be included only when their values are public-safe. The tool must reject private-looking command or error text before any Linear mutation. The agent must not speculate about capabilities or environment restrictions that it did not directly verify. +- `manual_attention` must not use runtime-owned retry or continued-repair + `error_class` values such as retryable app-server, stalled-run, or repo-gate + validation classes. Those classes remain owned by Decodex retry, continuation, or + architecture-recovery policy until the runtime itself reaches a human-required + terminal boundary. - For authority-boundary stops, `manual_attention` may include a structured `decision_request` object. Its Linear-rendered fields are public-safe only: `decision_request_id`, `reason_code`, `boundary_type`, `proposed_change`, @@ -219,7 +230,11 @@ In either invalid case, `decodex` must fail the attempt rather than infer which worktree evidence, retained diff evidence, and recovery-attempt context, must be written to private runtime evidence before the Linear write and must not be rendered into the public comment. -- The human-attention exit is not complete until the explanatory comment is successfully written after the label request. A label-only signal must be rejected as an invalid completion disposition. +- The human-attention exit is not complete until the requested label and explanatory + comment are successfully written after the comment validates. A label-intent-only + signal must be rejected as an invalid completion disposition, and invalid, + private-looking, unsupported, or runtime-owned retryable comments must not leave a + label-only attention state behind. - The run is not complete until `issue_terminal_finalize` succeeds against the matching terminal path. An execution-state checkpoint or an agent summary message is not a substitute. - Issues that carry the configured `needs_attention_label` must remain ineligible for future automatic selection until a human clears the label. - `issue_review_handoff` and the human-attention exit are mutually exclusive terminal signals for the same turn.