diff --git a/apps/decodex/src/cli.rs b/apps/decodex/src/cli.rs index b29539355..5f77d4a45 100644 --- a/apps/decodex/src/cli.rs +++ b/apps/decodex/src/cli.rs @@ -35,8 +35,8 @@ use crate::{ RadarValidateRequest, }, recovery::{ - self, LegacyCloseoutRecoveryRequest, ReviewHandoffAdoptRequest, - ReviewHandoffDiagnoseRequest, ReviewHandoffRebindRequest, + self, LegacyCloseoutRecoveryRequest, MergedCloseoutRecoveryRequest, + ReviewHandoffAdoptRequest, ReviewHandoffDiagnoseRequest, ReviewHandoffRebindRequest, }, research_design::{ self, ResearchDesignCompileRequest, ResearchDesignOutcome, ResearchDesignPromoteRequest, @@ -863,6 +863,15 @@ impl RecoverCommand { manual_authority: args.manual_authority, }, ), + RecoverSubcommand::MergedCloseout(args) => recovery::run_merged_closeout( + self.project_config.as_path(), + &MergedCloseoutRecoveryRequest { + issue: args.issue.clone(), + pr_url: args.pr.clone(), + dry_run: args.dry_run, + manual_authority: args.manual_authority, + }, + ), } } } @@ -952,6 +961,22 @@ struct LegacyCloseoutRecoveryCommand { manual_authority: bool, } +#[derive(Debug, Args)] +struct MergedCloseoutRecoveryCommand { + /// Issue identifier for the already-merged retained lane. + #[arg(value_name = "ISSUE")] + issue: String, + /// Merged pull request URL that proves the lane's terminal code lineage. + #[arg(long, value_name = "PR_URL")] + pr: String, + /// Validate without writing closeout or cleanup ledger events. + #[arg(long)] + dry_run: bool, + /// Required for non-dry-run merged closeout reconciliation. + #[arg(long)] + manual_authority: bool, +} + #[derive(Debug, Args)] struct ArchiveLinearCommand { #[command(flatten)] @@ -1702,6 +1727,8 @@ enum RecoverSubcommand { ReviewHandoff(ReviewHandoffRecoveryCommand), /// Record an audited fallback closeout for a legacy cleanup-only worktree. LegacyCloseout(LegacyCloseoutRecoveryCommand), + /// Reconcile stale retained attention after a PR is already merged and cleaned up. + MergedCloseout(MergedCloseoutRecoveryCommand), } #[derive(Debug, Subcommand)] @@ -1846,16 +1873,17 @@ mod tests { CommitCommand, DiagnoseCommand, EvidenceCommand, IntakeCommand, IntakeGoalCommand, IntakeIssuesCommand, IntakeSubcommand, LandCommand, LaneCommand, LaneInspectCommand, LaneInterruptCommand, LaneSteerCommand, LaneSubcommand, LegacyCloseoutRecoveryCommand, - ProbeCommand, ProjectCommand, ProjectConfigArgs, ProjectSubcommand, - RadarBackfillReleaseRangeCommand, RadarBundleBuildCommand, RadarBundleCommand, - RadarBundleSubcommand, RadarBundleValidateCommand, RadarCommand, RadarLedgerCommand, - RadarLedgerIngestExistingCommand, RadarLedgerSubcommand, RadarLedgerSummaryCommand, - RadarRefreshReleaseDeltaCommand, RadarRefreshUpstreamQueueCommand, - RadarRenderSignalCommand, RadarSubcommand, RadarValidateCommand, RecoverCommand, - RecoverSubcommand, ResearchCommand, ResearchCompileCommand, ResearchOutcomeArg, - ResearchPromoteCommand, ResearchSubcommand, ReviewHandoffAdoptCommand, - ReviewHandoffDiagnoseCommand, ReviewHandoffRebindCommand, ReviewHandoffRecoveryCommand, - ReviewHandoffRecoverySubcommand, RunCommand, ServeCommand, StatusCommand, + MergedCloseoutRecoveryCommand, ProbeCommand, ProjectCommand, ProjectConfigArgs, + ProjectSubcommand, RadarBackfillReleaseRangeCommand, RadarBundleBuildCommand, + RadarBundleCommand, RadarBundleSubcommand, RadarBundleValidateCommand, RadarCommand, + RadarLedgerCommand, RadarLedgerIngestExistingCommand, RadarLedgerSubcommand, + RadarLedgerSummaryCommand, RadarRefreshReleaseDeltaCommand, + RadarRefreshUpstreamQueueCommand, RadarRenderSignalCommand, RadarSubcommand, + RadarValidateCommand, RecoverCommand, RecoverSubcommand, ResearchCommand, + ResearchCompileCommand, ResearchOutcomeArg, ResearchPromoteCommand, ResearchSubcommand, + ReviewHandoffAdoptCommand, ReviewHandoffDiagnoseCommand, ReviewHandoffRebindCommand, + ReviewHandoffRecoveryCommand, ReviewHandoffRecoverySubcommand, RunCommand, ServeCommand, + StatusCommand, }; #[test] @@ -2928,4 +2956,31 @@ mod tests { && pr == "https://github.com/hack-ink/pubfi-mono-v2/pull/14" )); } + + #[test] + fn parses_merged_closeout_manual_authority() { + let cli = Cli::parse_from([ + "decodex", + "recover", + "merged-closeout", + "PUB-1549", + "--pr", + "https://github.com/helixbox/pubfi-mono/pull/309", + "--manual-authority", + ]); + + assert!(matches!( + cli.command, + Command::Recover(RecoverCommand { + command: RecoverSubcommand::MergedCloseout(MergedCloseoutRecoveryCommand { + issue, + pr, + dry_run: false, + manual_authority: true, + }), + .. + }) if issue == "PUB-1549" + && pr == "https://github.com/helixbox/pubfi-mono/pull/309" + )); + } } diff --git a/apps/decodex/src/orchestrator/daemon.rs b/apps/decodex/src/orchestrator/daemon.rs index d746b2468..520545482 100644 --- a/apps/decodex/src/orchestrator/daemon.rs +++ b/apps/decodex/src/orchestrator/daemon.rs @@ -251,7 +251,7 @@ where add_operator_snapshot_warning(&mut snapshot, "external_observer_status_skipped"); } - refresh_operator_project_summary(&mut snapshot); + refresh_operator_project_summary(&mut snapshot, None); Ok(snapshot) } diff --git a/apps/decodex/src/orchestrator/entrypoints.rs b/apps/decodex/src/orchestrator/entrypoints.rs index e0680ec81..0aab49b06 100644 --- a/apps/decodex/src/orchestrator/entrypoints.rs +++ b/apps/decodex/src/orchestrator/entrypoints.rs @@ -346,7 +346,10 @@ pub(crate) fn run_diagnose(request: DiagnoseRequest<'_>) -> Result<()> { }, }?; - refresh_operator_project_summary(&mut snapshot); + refresh_operator_project_summary( + &mut snapshot, + Some(workflow.frontmatter().tracker().resolved_completed_state()), + ); let results = write_agent_evidence_snapshot( &snapshot, @@ -598,7 +601,7 @@ fn project_status_snapshot_from_operator_cache( project_snapshot.warnings = project_warnings_from_details(&project_snapshot.warning_details); truncate_status_snapshot_to_limit(&mut project_snapshot, limit); - refresh_operator_project_summary(&mut project_snapshot); + refresh_operator_project_summary(&mut project_snapshot, None); Ok(project_snapshot) } @@ -613,7 +616,7 @@ fn mark_cached_status_snapshot( snapshot.snapshot_age_seconds = Some(snapshot_age_seconds); truncate_status_snapshot_to_limit(snapshot, limit); - refresh_operator_project_summary(snapshot); + refresh_operator_project_summary(snapshot, None); } fn truncate_status_snapshot_to_limit(snapshot: &mut OperatorStatusSnapshot, limit: usize) { @@ -741,7 +744,10 @@ fn build_live_status_command_snapshot( add_operator_snapshot_warning(&mut snapshot, &warning); } - refresh_operator_project_summary(&mut snapshot); + refresh_operator_project_summary( + &mut snapshot, + Some(workflow.frontmatter().tracker().resolved_completed_state()), + ); if !snapshot .connector_backoffs @@ -959,7 +965,7 @@ fn build_operator_status_snapshot_for_tracker_backoff( add_operator_snapshot_warning(&mut snapshot, "external_observer_status_skipped"); apply_terminal_history_ledger_outcomes(&mut snapshot); - refresh_operator_project_summary(&mut snapshot); + refresh_operator_project_summary(&mut snapshot, None); Ok(snapshot) } @@ -1638,7 +1644,10 @@ fn build_operator_state_snapshot_without_live_observers( &mut snapshot, Some(workflow.frontmatter().tracker().resolved_completed_state()), ); - refresh_operator_project_summary(&mut snapshot); + refresh_operator_project_summary( + &mut snapshot, + Some(workflow.frontmatter().tracker().resolved_completed_state()), + ); Ok(snapshot) } diff --git a/apps/decodex/src/orchestrator/status.rs b/apps/decodex/src/orchestrator/status.rs index e0cc3ee72..849992c99 100644 --- a/apps/decodex/src/orchestrator/status.rs +++ b/apps/decodex/src/orchestrator/status.rs @@ -262,6 +262,9 @@ struct OperatorIssueDisplayMetadata { issue_identifier: String, title: Option, author: Option, + issue_state: Option, + active_label_present: Option, + needs_attention_label_present: Option, } struct WorktreeOwnership { @@ -373,7 +376,7 @@ fn build_operator_status_snapshot_with_account_mode( }; refresh_worktree_ownership(&mut snapshot, None); - refresh_operator_project_summary(&mut snapshot); + refresh_operator_project_summary(&mut snapshot, None); Ok(snapshot) } @@ -658,7 +661,10 @@ where &mut snapshot, Some(workflow.frontmatter().tracker().resolved_completed_state()), ); - refresh_operator_project_summary(&mut snapshot); + refresh_operator_project_summary( + &mut snapshot, + Some(workflow.frontmatter().tracker().resolved_completed_state()), + ); Ok(snapshot) } @@ -797,6 +803,7 @@ where hydrate_operator_run_rows_from_tracker( context.tracker, context.project, + context.workflow, snapshot, context.run_issue_metadata_hydration, ), @@ -1348,7 +1355,10 @@ fn legacy_cleanup_next_action(worktree: &OperatorWorktreeStatus) -> String { ) } -fn refresh_operator_project_summary(snapshot: &mut OperatorStatusSnapshot) { +fn refresh_operator_project_summary( + snapshot: &mut OperatorStatusSnapshot, + completed_state: Option<&str>, +) { let active_run_count = snapshot.active_runs.len(); let running_lane_count = snapshot.active_runs.iter().filter(|run| operator_run_counts_as_running(run)).count(); @@ -1360,7 +1370,7 @@ fn refresh_operator_project_summary(snapshot: &mut OperatorStatusSnapshot) { let post_review_lane_count = snapshot.post_review_lanes.len(); let retained_worktree_count = rendered_recovery_worktrees(snapshot).len(); let waiting_lane_count = project_waiting_lane_count(snapshot); - let attention_count = project_attention_count(snapshot); + let attention_count = project_attention_count(snapshot, completed_state); let cleanup_blocked_count = project_cleanup_blocked_count(snapshot); let cleanup_pending_count = project_cleanup_pending_count(snapshot); let connector_state = project_connector_state(snapshot); @@ -1420,7 +1430,10 @@ fn queued_candidate_counts_as_waiting_intake(candidate: &OperatorQueuedIssueStat !matches!(candidate.classification.as_str(), "claimed" | "closed") } -fn project_attention_count(snapshot: &OperatorStatusSnapshot) -> usize { +fn project_attention_count( + snapshot: &OperatorStatusSnapshot, + completed_state: Option<&str>, +) -> usize { let mut attention_keys = HashSet::new(); for run in snapshot @@ -1455,7 +1468,7 @@ fn project_attention_count(snapshot: &OperatorStatusSnapshot) -> usize { for lane in snapshot .history_lanes .iter() - .filter(|lane| history_ledger_outcome_requires_attention(&lane.ledger_outcome)) + .filter(|lane| history_lane_has_unresolved_attention(snapshot, lane, completed_state)) { attention_keys.insert(history_lane_group_key(lane)); } @@ -1463,6 +1476,56 @@ fn project_attention_count(snapshot: &OperatorStatusSnapshot) -> usize { attention_keys.len() } +fn history_lane_has_unresolved_attention( + snapshot: &OperatorStatusSnapshot, + lane: &OperatorHistoryLaneStatus, + completed_state: Option<&str>, +) -> bool { + if !history_ledger_outcome_requires_attention(&lane.ledger_outcome) { + return false; + } + + !history_lane_attention_is_resolved_tracker_echo(snapshot, lane, completed_state) +} + +fn history_lane_attention_is_resolved_tracker_echo( + snapshot: &OperatorStatusSnapshot, + lane: &OperatorHistoryLaneStatus, + completed_state: Option<&str>, +) -> bool { + let Some(completed_state) = completed_state else { + return false; + }; + + if lane.issue_state.as_deref() != Some(completed_state) { + return false; + } + if lane.active_label_present != Some(false) + || lane.needs_attention_label_present != Some(false) + { + return false; + } + + let issue_key = history_lane_group_key(lane); + + !snapshot.worktrees.iter().any(|worktree| { + operator_issue_attention_key(&worktree.issue_id, worktree.issue_identifier.as_deref()) + == issue_key + }) && !snapshot.post_review_lanes.iter().any(|post_review_lane| { + operator_issue_attention_key( + &post_review_lane.issue_id, + Some(&post_review_lane.issue_identifier), + ) == issue_key + }) && !snapshot.queued_candidates.iter().any(|candidate| { + if candidate.classification == "closed" && candidate.attention.is_none() { + return false; + } + + operator_issue_attention_key(&candidate.issue_id, Some(&candidate.issue_identifier)) + == issue_key + }) +} + fn operator_issue_attention_key(issue_id: &str, issue_identifier: Option<&str>) -> String { let issue_id = issue_id.trim(); @@ -1925,6 +1988,7 @@ fn codex_account_activity_summaries( fn hydrate_operator_run_rows_from_tracker( tracker: &T, project: &ServiceConfig, + workflow: &WorkflowDocument, snapshot: &mut OperatorStatusSnapshot, hydration: RunIssueMetadataHydration, ) -> TrackerObserverOutcome @@ -1939,15 +2003,24 @@ where match tracker.refresh_issues(&issue_ids) { Ok(issues) => { + let active_label = crate::tracker::automation_active_label(project.service_id()); + let needs_attention_label = + workflow.frontmatter().tracker().needs_attention_label().to_owned(); let metadata_by_issue_id = issues .into_iter() .map(|issue| { + let active_label_present = issue.has_label(&active_label); + let needs_attention_label_present = issue.has_label(&needs_attention_label); + ( issue.id, OperatorIssueDisplayMetadata { issue_identifier: issue.identifier, title: Some(issue.title), author: issue.author, + issue_state: Some(issue.state.name), + active_label_present: Some(active_label_present), + needs_attention_label_present: Some(needs_attention_label_present), }, ) }) @@ -2061,6 +2134,19 @@ fn apply_history_lane_issue_metadata( if let Some(author) = metadata.author.as_ref().filter(|author| !author.trim().is_empty()) { lane.author = Some(author.clone()); } + if let Some(issue_state) = metadata + .issue_state + .as_ref() + .filter(|issue_state| !issue_state.trim().is_empty()) + { + lane.issue_state = Some(issue_state.clone()); + } + if let Some(active_label_present) = metadata.active_label_present { + lane.active_label_present = Some(active_label_present); + } + if let Some(needs_attention_label_present) = metadata.needs_attention_label_present { + lane.needs_attention_label_present = Some(needs_attention_label_present); + } } fn apply_run_issue_metadata( @@ -2190,6 +2276,9 @@ fn hydrate_history_lane_from_ledger_records( issue_identifier: record.record.issue_identifier.clone(), title: None, author: None, + issue_state: None, + active_label_present: None, + needs_attention_label_present: None, }; fill_missing_history_lane_issue_metadata(lane, &metadata); @@ -7062,6 +7151,9 @@ fn operator_history_lanes( issue_identifier: run.issue_identifier.clone(), title: run.title.clone(), author: run.author.clone(), + issue_state: None, + active_label_present: None, + needs_attention_label_present: None, issue_key: operator_run_issue_key(run), attempt_count: 1, ledger_outcome: not_loaded_history_ledger_outcome(), diff --git a/apps/decodex/src/orchestrator/tests/operator/status/history.rs b/apps/decodex/src/orchestrator/tests/operator/status/history.rs index ef9e19e88..6d4ca7ca2 100644 --- a/apps/decodex/src/orchestrator/tests/operator/status/history.rs +++ b/apps/decodex/src/orchestrator/tests/operator/status/history.rs @@ -477,6 +477,59 @@ fn local_status_summary_counts_terminal_history_needs_attention_without_queue_ca assert!(rendered.contains("role: retained_attention")); } +#[test] +fn live_status_does_not_count_done_history_attention_without_retained_ownership() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let queue_label = tracker::automation_queue_label(TEST_SERVICE_ID); + let mut issue = sample_issue_with_sort_fields( + "issue-pub-1549", + "PUB-1549", + "Done", + &[], + Some(3), + "2026-06-12T01:56:00Z", + ); + + issue.labels.retain(|label| label.name != queue_label); + + let tracker = FakeTracker::new(vec![issue.clone()]); + let comments = retained_partial_progress_linear_execution_history_comments(&issue); + + state_store + .upsert_worktree( + TEST_SERVICE_ID, + &issue.id, + "x/pubfi-mono-pub-1549", + &config.worktree_root().join(&issue.identifier).display().to_string(), + ) + .expect("worktree should remember previous lane ownership"); + state_store + .record_run_attempt("pub-1549-attempt-1-1781240781", &issue.id, 1, "failed") + .expect("failed attempt should record"); + state_store.clear_worktree(&issue.id).expect("completed lane cleanup should clear local worktree"); + tracker.issue_comments.borrow_mut().insert(issue.id.clone(), comments); + + let snapshot = orchestrator::build_live_operator_status_snapshot( + &tracker, + &config, + &workflow, + &state_store, + 10, + ) + .expect("snapshot should build"); + let lane = snapshot.history_lanes.first().expect("history lane should exist"); + + assert_eq!(snapshot.projects[0].attention_count, 0); + assert_eq!(snapshot.projects[0].retained_worktree_count, 0); + assert!(snapshot.queued_candidates.is_empty()); + assert_eq!(lane.issue_state.as_deref(), Some("Done")); + assert_eq!(lane.active_label_present, Some(false)); + assert_eq!(lane.needs_attention_label_present, Some(false)); + assert_eq!(lane.ledger_outcome.final_outcome, "needs_attention"); + assert_eq!(lane.latest_run.status, "needs_attention"); +} + #[test] fn live_operator_history_lanes_require_linear_execution_ledger_records() { let (_temp_dir, config, workflow) = temp_project_layout(); diff --git a/apps/decodex/src/orchestrator/types.rs b/apps/decodex/src/orchestrator/types.rs index d3e9eb16a..b095d205c 100644 --- a/apps/decodex/src/orchestrator/types.rs +++ b/apps/decodex/src/orchestrator/types.rs @@ -1439,6 +1439,12 @@ struct OperatorHistoryLaneStatus { issue_identifier: Option, title: Option, author: Option, + #[serde(skip_serializing_if = "Option::is_none")] + issue_state: Option, + #[serde(skip_serializing_if = "Option::is_none")] + active_label_present: Option, + #[serde(skip_serializing_if = "Option::is_none")] + needs_attention_label_present: Option, issue_key: String, attempt_count: usize, ledger_outcome: OperatorHistoryLedgerOutcome, diff --git a/apps/decodex/src/recovery.rs b/apps/decodex/src/recovery.rs index f18a29e95..aa73f9d39 100644 --- a/apps/decodex/src/recovery.rs +++ b/apps/decodex/src/recovery.rs @@ -9,7 +9,7 @@ use std::{ use color_eyre::{Report, eyre::WrapErr}; use serde::Serialize; -use time::{OffsetDateTime, format_description::well_known::Rfc3339}; +use time::{Duration, OffsetDateTime, format_description::well_known::Rfc3339}; use crate::{ config::ServiceConfig, @@ -41,6 +41,8 @@ const REVIEW_HANDOFF_REBIND_EVENT: &str = "review_handoff_rebind"; const REVIEW_HANDOFF_ADOPT_EVENT: &str = "review_handoff_adopt"; const LEGACY_MANUAL_CLOSEOUT_EVENT: &str = "closeout"; const LEGACY_MANUAL_CLOSEOUT_ANCHOR: &str = "legacy_manual_closeout"; +const MERGED_CLOSEOUT_CLOSEOUT_ANCHOR: &str = "merged_closeout"; +const MERGED_CLOSEOUT_CLEANUP_ANCHOR: &str = "merged_closeout_cleanup"; const REBOUND_ORCHESTRATION_PHASE: &str = "request_pending"; const LINEAR_CONNECTOR_BACKOFF_WARNING: &str = "tracker_rate_limited"; const LINEAR_CONNECTOR_BACKOFF_SECS: i64 = 15 * 60; @@ -89,6 +91,19 @@ pub(crate) struct LegacyCloseoutRecoveryRequest { pub(crate) manual_authority: bool, } +/// Explicit merged PR closeout reconciliation for stale retained attention. +#[derive(Debug)] +pub(crate) struct MergedCloseoutRecoveryRequest { + /// Issue identifier to reconcile. + pub(crate) issue: String, + /// Merged pull request URL that proves terminal code lineage. + pub(crate) pr_url: String, + /// Validate without writing runtime or tracker ledger events. + pub(crate) dry_run: bool, + /// Required for non-dry-run mutation. + pub(crate) manual_authority: bool, +} + #[derive(Serialize)] struct ReviewHandoffRecoveryReport { project_id: String, @@ -187,6 +202,11 @@ struct AdoptValidation { success_state_transition: Option, previous_worktree_mapping: Option, } +impl AdoptValidation { + fn should_restore_active_label(&self) -> bool { + !self.active_label_present + } +} struct LegacyCloseoutValidation { issue: TrackerIssue, @@ -197,6 +217,24 @@ struct LegacyCloseoutValidation { worktree_path_for_event: Option, } +struct MergedCloseoutValidation { + issue: TrackerIssue, + branch_name: String, + worktree_path_for_event: String, + run_id: String, + attempt_number: i64, + landing_state: PullRequestLandingState, + merge_commit: String, + worktree_mapping: Option, +} + +struct MergedCloseoutRetainedContext { + branch_name: String, + worktree_path: String, + run_id: String, + attempt_number: i64, +} + #[derive(Debug)] struct RebindSuccessStateTransition { state_name: String, @@ -350,9 +388,10 @@ pub(crate) fn run_review_handoff_adopt( .success_state_transition .as_ref() .map_or("none", |transition| transition.state_name.as_str()); + let active_label = tracker::automation_active_label(context.config.service_id()); println!( - "dry run: review handoff adopt validated for project={} issue={} branch={} pr={} head={} run_id={} attempt={} active_label_present={} state_transition={}", + "dry run: review handoff adopt validated for project={} issue={} branch={} pr={} head={} run_id={} attempt={} active_label={} active_label_present={} would_restore_active_label={} state_transition={}", context.config.service_id(), validation.issue.identifier, validation.branch_name, @@ -360,7 +399,9 @@ pub(crate) fn run_review_handoff_adopt( validation.local_head_oid, validation.run_id, validation.attempt_number, + active_label, validation.active_label_present, + validation.should_restore_active_label(), state_transition ); @@ -427,6 +468,55 @@ pub(crate) fn run_legacy_closeout( Ok(()) } +/// Run an explicit merged PR closeout reconciliation for stale retained attention. +pub(crate) fn run_merged_closeout( + config_path: Option<&Path>, + request: &MergedCloseoutRecoveryRequest, +) -> Result<()> { + let context = load_recovery_context(config_path)?; + let validation = validate_merged_closeout_request(&context, request)?; + + if request.dry_run { + println!( + "dry run: merged closeout validated for project={} issue={} branch={} worktree_path={} pr={} head={} merge_commit={} run_id={} attempt={}", + context.config.service_id(), + validation.issue.identifier, + validation.branch_name, + validation.worktree_path_for_event, + landing_url(&validation.landing_state), + validation.landing_state.head_ref_oid, + validation.merge_commit, + validation.run_id, + validation.attempt_number + ); + + return Ok(()); + } + if !request.manual_authority { + eyre::bail!( + "`recover merged-closeout` writes closeout and cleanup ledger records and requires --manual-authority outside dry-run mode." + ); + } + + let (closeout_recorded, cleanup_recorded) = + apply_merged_closeout_recovery(&context, &validation)?; + + println!( + "merged closeout recovery ok: project={} issue={} branch={} worktree_path={} pr={} head={} merge_commit={} closeout_recorded={} cleanup_recorded={}", + context.config.service_id(), + validation.issue.identifier, + validation.branch_name, + validation.worktree_path_for_event, + landing_url(&validation.landing_state), + validation.landing_state.head_ref_oid, + validation.merge_commit, + closeout_recorded, + cleanup_recorded + ); + + Ok(()) +} + fn load_recovery_context(config_path: Option<&Path>) -> Result { let state_store = runtime::open_runtime_store()?; let config_path = resolve_recovery_config_path(config_path, &state_store)?; @@ -1177,6 +1267,59 @@ fn validate_legacy_closeout_request( }) } +fn validate_merged_closeout_request( + context: &RecoveryContext, + request: &MergedCloseoutRecoveryRequest, +) -> Result { + let issue = load_issue_by_identifier(&context.tracker, &request.issue)?; + + validate_merged_closeout_issue_context(context, &issue)?; + + let (landing_state, default_branch) = inspect_project_pull_request(context, &request.pr_url)?; + + validate_merged_closeout_pull_request(context, &landing_state, &default_branch)?; + + let merge_commit = inspect_project_pull_request_merge_commit(context, &request.pr_url)?; + + ensure_merge_commit_reachable_from_remote_default_branch( + context.config.repo_root(), + &request.pr_url, + &merge_commit, + &default_branch, + )?; + + let worktree_mapping = retained_worktree_mapping_for_issue(context, &issue)?; + let retained_context = + merged_closeout_retained_context(context, &issue, worktree_mapping.as_ref())?; + + if landing_state.head_ref_name != retained_context.branch_name { + eyre::bail!( + "Pull request `{}` points at branch `{}`, but retained lane branch is `{}`.", + landing_url(&landing_state), + landing_state.head_ref_name, + retained_context.branch_name + ); + } + + validate_merged_closeout_worktree_mapping( + context, + &issue, + worktree_mapping.as_ref(), + &landing_state, + )?; + + Ok(MergedCloseoutValidation { + issue, + branch_name: retained_context.branch_name, + worktree_path_for_event: retained_context.worktree_path, + run_id: retained_context.run_id, + attempt_number: retained_context.attempt_number, + landing_state, + merge_commit, + worktree_mapping, + }) +} + fn validate_legacy_closeout_issue_state( tracker_policy: &WorkflowTracker, issue: &TrackerIssue, @@ -1207,6 +1350,125 @@ fn legacy_closeout_worktree( eyre::bail!("Issue `{}` has no retained worktree mapping.", issue.identifier) } +fn validate_merged_closeout_issue_context( + context: &RecoveryContext, + issue: &TrackerIssue, +) -> Result<()> { + let tracker_policy = context.workflow.frontmatter().tracker(); + let completed_state = tracker_policy.resolved_completed_state(); + + if issue.state.name != completed_state { + eyre::bail!( + "Issue `{}` is in `{}`, but merged closeout recovery requires `{completed_state}`.", + issue.identifier, + issue.state.name + ); + } + if issue.has_label(tracker_policy.opt_out_label()) { + eyre::bail!( + "Issue `{}` has opt-out label `{}`.", + issue.identifier, + tracker_policy.opt_out_label() + ); + } + + for label in [ + tracker::automation_queue_label(context.config.service_id()), + tracker::automation_active_label(context.config.service_id()), + tracker_policy.needs_attention_label().to_owned(), + ] { + if tracker::issue_has_label_with_server_confirmation(&context.tracker, issue, &label)? { + eyre::bail!( + "Issue `{}` still has Linear label `{label}`; merged closeout recovery requires queue, active, and needs-attention labels to be absent.", + issue.identifier + ); + } + } + + Ok(()) +} + +fn retained_worktree_mapping_for_issue( + context: &RecoveryContext, + issue: &TrackerIssue, +) -> Result> { + if let Some(worktree) = context.state_store.worktree_for_issue(&issue.id)? { + return Ok(Some(worktree)); + } + + context.state_store.worktree_for_issue(&issue.identifier) +} + +fn merged_closeout_retained_context( + context: &RecoveryContext, + issue: &TrackerIssue, + worktree_mapping: Option<&WorktreeMapping>, +) -> Result { + let latest_record = latest_merged_closeout_source_record(context, issue)?; + let branch_name = worktree_mapping + .map(|mapping| mapping.branch_name().to_owned()) + .or_else(|| latest_record.as_ref().and_then(|record| record.branch.clone())) + .ok_or_else(|| { + eyre::eyre!( + "Issue `{}` has no retained branch in runtime state or execution ledger.", + issue.identifier + ) + })?; + let worktree_path = worktree_mapping + .and_then(|mapping| relative_worktree_path_for_recovery(context, mapping.worktree_path())) + .or_else(|| latest_record.as_ref().and_then(|record| record.worktree_path.clone())) + .unwrap_or_else(|| format!(".worktrees/{}", issue.identifier)); + let (run_id, attempt_number) = if let Some(record) = latest_record + .as_ref() + .filter(|record| !record.run_id.trim().is_empty() && record.attempt_number >= 1) + { + (record.run_id.clone(), record.attempt_number) + } else if let Some(attempt) = context.state_store.latest_run_attempt_for_issue(&issue.id)? { + (attempt.run_id().to_owned(), attempt.attempt_number()) + } else { + (format!("merged-closeout-{}", issue.identifier.to_ascii_lowercase()), 1) + }; + + Ok(MergedCloseoutRetainedContext { branch_name, worktree_path, run_id, attempt_number }) +} + +fn latest_merged_closeout_source_record( + context: &RecoveryContext, + issue: &TrackerIssue, +) -> Result> { + let mut records = + context.state_store.list_linear_execution_events(context.config.service_id(), &issue.id)?; + + if issue.identifier != issue.id { + records.extend( + context + .state_store + .list_linear_execution_events(context.config.service_id(), &issue.identifier)?, + ); + } + + let comments = context.tracker.list_comments(&issue.id)?; + + records.extend( + comments + .iter() + .filter_map(|comment| records::parse_linear_execution_event_record(&comment.body)) + .filter(|record| { + record.service_id == context.config.service_id() + && (record.issue_id == issue.id || record.issue_identifier == issue.identifier) + }), + ); + + Ok(records + .into_iter() + .filter(|record| record.branch.as_ref().is_some_and(|branch| !branch.trim().is_empty())) + .max_by(|left, right| { + left.event_timestamp + .cmp(&right.event_timestamp) + .then_with(|| left.idempotency_key.cmp(&right.idempotency_key)) + })) +} + fn load_issue_by_identifier(tracker: &T, issue_identifier: &str) -> Result where T: IssueTracker + ?Sized, @@ -1456,6 +1718,157 @@ fn inspect_project_pull_request_merge_commit( ) } +fn validate_merged_closeout_pull_request( + context: &RecoveryContext, + landing_state: &PullRequestLandingState, + default_branch: &str, +) -> Result<()> { + if landing_state.base_ref_name != default_branch { + eyre::bail!( + "Pull request `{}` targets `{}`, but configured default branch is `{default_branch}`.", + landing_url(landing_state), + landing_state.base_ref_name + ); + } + if landing_state.state != "MERGED" { + eyre::bail!( + "Pull request `{}` is `{}`; merged closeout recovery requires `MERGED`.", + landing_url(landing_state), + landing_state.state + ); + } + if landing_state.head_ref_name.trim().is_empty() { + eyre::bail!( + "Pull request `{}` does not expose the merged head branch required for retained lane reconciliation.", + landing_url(landing_state) + ); + } + if landing_state.head_ref_name == default_branch { + eyre::bail!( + "Pull request `{}` uses default branch `{default_branch}` as its head; merged closeout recovery cannot prove retained lane identity.", + landing_url(landing_state) + ); + } + + let remote_ref = format!("refs/remotes/origin/{default_branch}"); + let output = Command::new("git") + .arg("-C") + .arg(context.config.repo_root()) + .args(["rev-parse", "--verify", remote_ref.as_str()]) + .output()?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + + eyre::bail!( + "Configured repo root `{}` does not expose `{remote_ref}`; sync the default branch before merged closeout recovery: {}", + context.config.repo_root().display(), + stderr.trim() + ); + } + + Ok(()) +} + +fn ensure_merge_commit_reachable_from_remote_default_branch( + repo_root: &Path, + pr_url: &str, + merge_commit: &str, + default_branch: &str, +) -> Result<()> { + let remote_ref = format!("refs/remotes/origin/{default_branch}"); + let status = Command::new("git") + .arg("-C") + .arg(repo_root) + .args(["merge-base", "--is-ancestor", merge_commit, remote_ref.as_str()]) + .status()?; + + if status.success() { + return Ok(()); + } + if status.code() == Some(1) { + eyre::bail!( + "Configured repo root `{}` remote `{remote_ref}` does not contain merge commit `{merge_commit}` for `{pr_url}`.", + repo_root.display() + ); + } + + eyre::bail!( + "`git merge-base --is-ancestor {merge_commit} {remote_ref}` failed in `{}` with status `{status}`.", + repo_root.display() + ) +} + +fn validate_merged_closeout_worktree_mapping( + context: &RecoveryContext, + issue: &TrackerIssue, + worktree_mapping: Option<&WorktreeMapping>, + landing_state: &PullRequestLandingState, +) -> Result<()> { + if let Some(mapping) = worktree_mapping { + if mapping.branch_name() != landing_state.head_ref_name { + eyre::bail!( + "Issue `{}` retained worktree branch is `{}`, but merged PR head branch is `{}`.", + issue.identifier, + mapping.branch_name(), + landing_state.head_ref_name + ); + } + + return validate_merged_closeout_worktree_path(mapping.worktree_path(), landing_state); + } + + let Some(relative_path) = latest_merged_closeout_source_record(context, issue)? + .and_then(|record| record.worktree_path) + else { + return Ok(()); + }; + let worktree_path = context.config.repo_root().join(relative_path); + + validate_merged_closeout_worktree_path(&worktree_path, landing_state) +} + +fn validate_merged_closeout_worktree_path( + worktree_path: &Path, + landing_state: &PullRequestLandingState, +) -> Result<()> { + if !worktree_path.exists() { + return Ok(()); + } + if !worktree_is_clean(worktree_path)? { + eyre::bail!( + "Retained worktree `{}` still has local changes; merged closeout recovery will not mark it cleanup-complete.", + worktree_path.display() + ); + } + + let local_branch = worktree_checkout_branch_name(worktree_path)?.ok_or_else(|| { + eyre::eyre!("Retained worktree `{}` is detached.", worktree_path.display()) + })?; + + if local_branch != landing_state.head_ref_name { + eyre::bail!( + "Retained worktree `{}` is on branch `{local_branch}`, but merged PR head branch is `{}`.", + worktree_path.display(), + landing_state.head_ref_name + ); + } + + let local_head = worktree_head_oid(worktree_path)?.ok_or_else(|| { + eyre::eyre!("Retained worktree `{}` has no readable HEAD.", worktree_path.display()) + })?; + + if local_head != landing_state.head_ref_oid { + eyre::bail!( + "Retained worktree `{}` HEAD is `{local_head}`, but merged PR head is `{}`.", + worktree_path.display(), + landing_state.head_ref_oid + ); + } + + Ok(()) +} + fn validate_rebind_worktree( worktree: &WorktreeMapping, landing_state: &PullRequestLandingState, @@ -1590,9 +2003,16 @@ fn validate_adopt_issue_context( let active_label_present = tracker::issue_has_label_with_server_confirmation(&context.tracker, issue, &active_label)?; - if !active_label_present { + if !active_label_present + && tracker::issue_team_label_id_with_server_confirmation( + &context.tracker, + issue, + &active_label, + )? + .is_none() + { eyre::bail!( - "Issue `{}` is missing active automation label `{active_label}`. Restore explicit lane ownership before manual takeover adopt.", + "Issue `{}` is missing active automation label `{active_label}`, and that label was not found on the team.", issue.identifier ); } @@ -1952,7 +2372,6 @@ fn apply_review_handoff_adopt( 0, None, ); - let event = review_handoff_adopt_event(context, validation); let worktree_path = validation.worktree_path.to_string_lossy().to_string(); let local_state_write = context .state_store @@ -1999,6 +2418,26 @@ fn apply_review_handoff_adopt( return Err(error); } + + let active_label_restored = match restore_adopt_active_label(context, validation) { + Ok(active_label_restored) => active_label_restored, + Err(error) => { + mark_adopt_attempt_failed(context, validation); + + context.state_store.clear_review_markers_for_handoff( + context.config.service_id(), + &validation.issue.id, + &handoff_marker, + &orchestration_marker, + )?; + + rollback_adopt_worktree_mapping(context, validation)?; + + return Err(error); + }, + }; + let event = review_handoff_adopt_event(context, validation, active_label_restored); + if let Err(error) = write_adopt_audit(context, validation, &event) .and_then(|()| context.state_store.record_linear_execution_event(&event)) { @@ -2011,6 +2450,7 @@ fn apply_review_handoff_adopt( &orchestration_marker, )?; + rollback_adopt_active_label_restoration(context, validation, active_label_restored)?; rollback_adopt_worktree_mapping(context, validation)?; return Err(error); @@ -2022,6 +2462,35 @@ fn apply_review_handoff_adopt( Ok(()) } +fn restore_adopt_active_label( + context: &RecoveryContext, + validation: &AdoptValidation, +) -> Result { + if !validation.should_restore_active_label() { + return Ok(false); + } + + let active_label = tracker::automation_active_label(context.config.service_id()); + + tracker::set_issue_label_presence(&context.tracker, &validation.issue, &active_label, true) +} + +fn rollback_adopt_active_label_restoration( + context: &RecoveryContext, + validation: &AdoptValidation, + active_label_restored: bool, +) -> Result<()> { + if !active_label_restored { + return Ok(()); + } + + let active_label = tracker::automation_active_label(context.config.service_id()); + + tracker::set_issue_label_presence(&context.tracker, &validation.issue, &active_label, false)?; + + Ok(()) +} + fn rollback_adopt_worktree_mapping( context: &RecoveryContext, validation: &AdoptValidation, @@ -2101,6 +2570,7 @@ fn review_handoff_rebind_event( fn review_handoff_adopt_event( context: &RecoveryContext, validation: &AdoptValidation, + active_label_restored: bool, ) -> LinearExecutionEventRecord { let pr_url = landing_url(&validation.landing_state); let stable_anchor = records::stable_event_anchor(&[ @@ -2138,6 +2608,7 @@ fn review_handoff_adopt_event( format!("pr_url={pr_url}"), format!("pr_head_sha={}", validation.local_head_oid), format!("active_label_present={}", validation.active_label_present), + format!("active_label_restored={active_label_restored}"), String::from("manual_takeover_adopt=true"), format!( "existing_retained_worktree_mapping={}", @@ -2290,6 +2761,175 @@ fn write_legacy_closeout_audit( Ok(true) } +fn apply_merged_closeout_recovery( + context: &RecoveryContext, + validation: &MergedCloseoutValidation, +) -> Result<(bool, bool)> { + let closeout_event = merged_closeout_event(context, validation); + let cleanup_event = merged_closeout_cleanup_event(context, validation); + let closeout_recorded = write_merged_closeout_event( + context, + validation, + &closeout_event, + "Decodex merged closeout recovery: verified the PR was merged into the current default branch and reconciled the stale retained attention closeout ledger.", + )?; + let cleanup_recorded = match write_merged_closeout_event( + context, + validation, + &cleanup_event, + "Decodex merged closeout recovery: verified retained lane cleanup is already complete and recorded cleanup_complete.", + ) { + Ok(cleanup_recorded) => cleanup_recorded, + Err(error) => { + if closeout_recorded { + context + .state_store + .forget_linear_execution_event(&closeout_event.idempotency_key)?; + } + + return Err(error); + }, + }; + + if validation.worktree_mapping.is_some() { + context.state_store.clear_worktree(&validation.issue.id)?; + + if validation.issue.identifier != validation.issue.id { + context.state_store.clear_worktree(&validation.issue.identifier)?; + } + } + + Ok((closeout_recorded, cleanup_recorded)) +} + +fn write_merged_closeout_event( + context: &RecoveryContext, + validation: &MergedCloseoutValidation, + event: &LinearExecutionEventRecord, + body: &str, +) -> Result { + let privacy_classifier = ConfiguredPublicProjectionPrivacyClassifier::from_config( + context.config.privacy_classifier(), + )?; + let projection = + tracker::prepare_linear_execution_event_comment(body, event, &privacy_classifier)?; + let recorded = context.state_store.record_linear_execution_event(&projection.record)?; + + if !recorded { + return Ok(false); + } + + if let Err(error) = tracker::create_prepared_linear_execution_event_comment_without_remote_scan( + &context.tracker, + &validation.issue.id, + &projection, + ) { + context.state_store.forget_linear_execution_event(&projection.record.idempotency_key)?; + + return Err(error); + } + + Ok(true) +} + +fn merged_closeout_event( + context: &RecoveryContext, + validation: &MergedCloseoutValidation, +) -> LinearExecutionEventRecord { + let pr_url = landing_url(&validation.landing_state); + let stable_anchor = records::stable_event_anchor(&[ + pr_url, + &validation.merge_commit, + MERGED_CLOSEOUT_CLOSEOUT_ANCHOR, + ]); + let mut event = LinearExecutionEventRecord::new( + LinearExecutionEventIdentity { + service_id: context.config.service_id(), + issue_id: &validation.issue.id, + issue_identifier: &validation.issue.identifier, + run_id: &validation.run_id, + attempt_number: validation.attempt_number, + }, + LEGACY_MANUAL_CLOSEOUT_EVENT, + current_timestamp(), + &stable_anchor, + ); + + event.branch = Some(validation.branch_name.clone()); + event.worktree_path = Some(validation.worktree_path_for_event.clone()); + event.pr_url = Some(pr_url.to_owned()); + event.pr_head_sha = Some(validation.landing_state.head_ref_oid.clone()); + event.pr_base_ref = Some(validation.landing_state.base_ref_name.clone()); + event.commit_sha = Some(validation.merge_commit.clone()); + event.validation_result = Some(String::from("passed")); + event.target_state = Some(validation.issue.state.name.clone()); + event.summary = Some(format!( + "Merged closeout recovery recorded for {} after PR {} was already merged.", + validation.issue.identifier, pr_url + )); + event.evidence = Some(vec![ + format!("issue_state={}", validation.issue.state.name), + format!("branch={}", validation.branch_name), + format!("pr_url={pr_url}"), + format!("pr_head_sha={}", validation.landing_state.head_ref_oid), + format!("merge_commit={}", validation.merge_commit), + String::from("origin_default_contains_merge_commit=true"), + ]); + event.next_action = Some(String::from( + "Decodex will record cleanup_complete for the already-merged retained lane.", + )); + + event +} + +fn merged_closeout_cleanup_event( + context: &RecoveryContext, + validation: &MergedCloseoutValidation, +) -> LinearExecutionEventRecord { + let pr_url = landing_url(&validation.landing_state); + let stable_anchor = records::stable_event_anchor(&[ + &validation.branch_name, + &validation.worktree_path_for_event, + &validation.merge_commit, + MERGED_CLOSEOUT_CLEANUP_ANCHOR, + ]); + let mut event = LinearExecutionEventRecord::new( + LinearExecutionEventIdentity { + service_id: context.config.service_id(), + issue_id: &validation.issue.id, + issue_identifier: &validation.issue.identifier, + run_id: &validation.run_id, + attempt_number: validation.attempt_number, + }, + "cleanup_complete", + timestamp_after_seconds(1), + &stable_anchor, + ); + + event.branch = Some(validation.branch_name.clone()); + event.worktree_path = Some(validation.worktree_path_for_event.clone()); + event.pr_url = Some(pr_url.to_owned()); + event.pr_head_sha = Some(validation.landing_state.head_ref_oid.clone()); + event.pr_base_ref = Some(validation.landing_state.base_ref_name.clone()); + event.commit_sha = Some(validation.merge_commit.clone()); + event.cleanup_status = Some(String::from("merged_closeout_reconciled")); + event.target_state = Some(validation.issue.state.name.clone()); + event.summary = Some(format!( + "Merged closeout recovery marked stale retained lane {} cleanup complete.", + validation.issue.identifier + )); + event.evidence = Some(vec![ + format!("issue_state={}", validation.issue.state.name), + format!("branch={}", validation.branch_name), + format!("worktree_path={}", validation.worktree_path_for_event), + String::from("linear_queue_active_attention_labels_absent=true"), + String::from("retained_worktree_has_no_uncommitted_changes=true"), + ]); + event.next_action = Some(String::from("No Decodex runtime action remains for this lane.")); + + event +} + fn landing_url(landing_state: &PullRequestLandingState) -> &str { &landing_state.url } @@ -2308,6 +2948,12 @@ fn current_timestamp() -> String { OffsetDateTime::now_utc().format(&Rfc3339).expect("timestamp formatting should succeed") } +fn timestamp_after_seconds(seconds: i64) -> String { + (OffsetDateTime::now_utc() + Duration::seconds(seconds)) + .format(&Rfc3339) + .expect("timestamp formatting should succeed") +} + fn git_toplevel_path(cwd: &Path) -> Result { let output = Command::new("git").arg("-C").arg(cwd).args(["rev-parse", "--show-toplevel"]).output()?; @@ -2439,6 +3085,18 @@ fn repository_relative_path(repo_root: &Path, path: &Path) -> Option { Some(relative.to_string_lossy().to_string()) } +fn relative_worktree_path_for_recovery( + context: &RecoveryContext, + worktree_path: &Path, +) -> Option { + repository_relative_path(context.config.repo_root(), worktree_path).or_else(|| { + worktree_path + .strip_prefix(context.config.repo_root()) + .ok() + .map(|relative| relative.to_string_lossy().to_string()) + }) +} + #[cfg(test)] mod tests { use std::{fs, path::Path}; @@ -3408,6 +4066,61 @@ Test workflow. .expect("adopt event should validate"); } + #[test] + fn merged_closeout_recovery_events_validate() { + let mut closeout = LinearExecutionEventRecord::new( + LinearExecutionEventIdentity { + service_id: "pubfi-mono", + issue_id: "issue-id", + issue_identifier: "PUB-1549", + run_id: "pub-1549-attempt-1-1781240781", + attempt_number: 1, + }, + super::LEGACY_MANUAL_CLOSEOUT_EVENT, + super::current_timestamp(), + "anchor-closeout", + ); + + closeout.branch = Some(String::from("x/pubfi-mono-pub-1549")); + closeout.worktree_path = Some(String::from(".worktrees/PUB-1549")); + closeout.pr_url = Some(String::from("https://github.com/helixbox/pubfi-mono/pull/309")); + closeout.pr_head_sha = Some(String::from("0123456789abcdef0123456789abcdef01234567")); + closeout.pr_base_ref = Some(String::from("main")); + closeout.commit_sha = Some(String::from("1123456789abcdef0123456789abcdef01234567")); + closeout.validation_result = Some(String::from("passed")); + closeout.target_state = Some(String::from("Done")); + closeout.summary = Some(String::from("Merged closeout recovery recorded.")); + + records::validate_linear_execution_event_record(&closeout) + .expect("merged closeout event should validate"); + + let mut cleanup = LinearExecutionEventRecord::new( + LinearExecutionEventIdentity { + service_id: "pubfi-mono", + issue_id: "issue-id", + issue_identifier: "PUB-1549", + run_id: "pub-1549-attempt-1-1781240781", + attempt_number: 1, + }, + "cleanup_complete", + super::timestamp_after_seconds(1), + "anchor-cleanup", + ); + + cleanup.branch = Some(String::from("x/pubfi-mono-pub-1549")); + cleanup.worktree_path = Some(String::from(".worktrees/PUB-1549")); + cleanup.pr_url = Some(String::from("https://github.com/helixbox/pubfi-mono/pull/309")); + cleanup.pr_head_sha = Some(String::from("0123456789abcdef0123456789abcdef01234567")); + cleanup.pr_base_ref = Some(String::from("main")); + cleanup.commit_sha = Some(String::from("1123456789abcdef0123456789abcdef01234567")); + cleanup.cleanup_status = Some(String::from("merged_closeout_reconciled")); + cleanup.target_state = Some(String::from("Done")); + cleanup.summary = Some(String::from("Merged closeout recovery marked cleanup complete.")); + + records::validate_linear_execution_event_record(&cleanup) + .expect("merged closeout cleanup event should validate"); + } + #[test] fn review_handoff_rebind_event_requires_evidence() { let mut record = LinearExecutionEventRecord::new( diff --git a/docs/reference/operator-control-plane.md b/docs/reference/operator-control-plane.md index 886c03642..6ede39ee3 100644 --- a/docs/reference/operator-control-plane.md +++ b/docs/reference/operator-control-plane.md @@ -502,6 +502,16 @@ and PR terminal state, inspect the checkout, run `--manual-authority` only after validation passes, and then remove the local worktree only after that evidence is understood. +If the row is not a real retained checkout and the issue is already completed because +a human merged the PR outside Decodex, use the formal stale-attention reconciliation +path instead of rerunning the lane: verify the PR URL, run +`decodex recover merged-closeout --pr --dry-run`, then rerun with +`--manual-authority` only after it proves the issue is completed, queue/active/attention +labels are absent, the PR head branch matches the retained branch, and the merge commit +is reachable from the current local `origin/`. Successful recovery +writes `closeout` plus `cleanup_complete` ledger records and should remove the false +project attention count. + The expected operator path for a cleanup-only row is short: 1. Verify the tracker issue and any associated PR are merged, done, or otherwise diff --git a/docs/runbook/recover-review-handoff.md b/docs/runbook/recover-review-handoff.md index d2021c1be..0a8323092 100644 --- a/docs/runbook/recover-review-handoff.md +++ b/docs/runbook/recover-review-handoff.md @@ -112,8 +112,9 @@ decodex recover review-handoff adopt --pr The command rejects the adopt unless all of these are true: -- the issue has `decodex:active:` and does not have the opt-out or - needs-attention labels +- the issue either has `decodex:active:` or the service active label + exists on the issue team and can be restored by live adopt after all other checks + pass; the issue must not have the opt-out or needs-attention labels - the issue is in the workflow `tracker.in_progress_state` or already in `tracker.success_state` - no conflicting retained worktree mapping exists; an existing mapping is allowed only @@ -130,8 +131,11 @@ The command rejects the adopt unless all of these are true: The non-dry-run command writes a runtime worktree mapping, a local takeover run attempt, review handoff/orchestration markers, and a `review_handoff_adopt` audit -event. If the issue was still in the workflow `tracker.in_progress_state`, the command -moves it to `tracker.success_state` after the audit succeeds. It does not merge the PR. +event. If the active service label was missing, live adopt restores it after local +handoff state is written and before the audit event is recorded; if the audit write +fails, the label restoration is rolled back. If the issue was still in the workflow +`tracker.in_progress_state`, the command moves it to `tracker.success_state` after the +audit succeeds. It does not merge the PR. After a successful adopt, land through the normal issue-authority path: @@ -169,13 +173,16 @@ plus explicit rebind before any manual cleanup. If diagnosis reports `classification: review_handoff_ownership_drift`, `reason: active_ownership_label_missing`, and `active_label_present: false`, do not run -rebind just to restore ownership. First verify the issue is still meant to continue the -retained post-review lifecycle for this service, then restore the issue to the workflow -success state and add `decodex:active:`. If the issue still has -`decodex:needs-attention`, clear that label only after the recorded blocker has been -repaired. +rebind just to restore ownership. If the lane has no retained handoff marker because a +human PR needs manual takeover, run `recover review-handoff adopt --dry-run`; the dry +run reports `would_restore_active_label=true` when live adopt can restore the active +service label after validating the issue, managed worktree, PR branch, PR head, and +landability gates. If the lane already has a retained handoff marker, use the ordinary +diagnosis/rebind or post-review path instead of hand-adding labels. If the issue still +has `decodex:needs-attention`, clear that label only after the recorded blocker has +been repaired or an explicit recovery command says it will clear the label itself. -After restoring explicit ownership, rerun: +After an explicit recovery restores or confirms ownership, rerun: ```sh decodex recover review-handoff diagnose @@ -185,3 +192,40 @@ decodex status Continue with `decodex land` or the normal retained post-review lifecycle only when the diagnosis remains bound and status reports a landable or otherwise concrete post-review state. + +## Merged Closeout Reconciliation + +Use `recover merged-closeout` when Decodex retained a terminal +`needs_attention`/`partial_progress_retained` ledger outcome, but a human already +merged the PR, the tracker issue is Done, and no useful retained patch remains. This +path reconciles Decodex lifecycle state only; it does not change business code, rerun +the lane, merge a PR, or delete local files. + +Run dry-run first: + +```sh +decodex recover merged-closeout --pr --dry-run +decodex recover merged-closeout --pr --manual-authority +``` + +The live command writes idempotent `closeout` and `cleanup_complete` Linear execution +ledger records, records them in the local runtime store, and clears any stale runtime +worktree mapping for the issue only after both ledger writes succeed. + +The command rejects the reconciliation unless all of these are true: + +- the tracker issue is in the workflow completed state +- the issue does not have the queue, active, opt-out, or needs-attention labels +- the PR belongs to the configured repository, targets the configured default branch, + and is `MERGED` +- the PR head branch matches the retained branch from runtime worktree mapping or the + existing execution ledger +- the PR merge commit is reachable from the current local `origin/` +- any retained worktree path that still exists is clean, on the PR head branch, and at + the PR head SHA +- the retained branch can be proven from the runtime mapping or existing execution + ledger + +After a successful reconciliation, `decodex status --live` should no longer count the +old terminal attention as current project attention; the Run Ledger should show +`cleanup_complete` as the final lifecycle outcome. diff --git a/docs/spec/linear-execution-ledger.md b/docs/spec/linear-execution-ledger.md index 58142c456..1015f69e6 100644 --- a/docs/spec/linear-execution-ledger.md +++ b/docs/spec/linear-execution-ledger.md @@ -222,6 +222,15 @@ Every event requires the record envelope. Additional required fields are listed | `terminal_failure` | `error_class`, `next_action`, `blockers`, `evidence` | `branch`, `worktree_path`, `pr_url`, `commit_sha`, `failed_command`, `raw_error`, `summary` | | `cleanup_complete` | `branch`, `worktree_path`, `cleanup_status`, `summary` | `pr_url`, `commit_sha` | +`recover merged-closeout` does not introduce a separate event type. After validating +that the tracker issue is completed, labels are clear, the PR is merged into the +configured repository default branch, the PR head branch matches the retained branch, +and the merge commit is reachable from local `origin/`, it writes an +idempotent `closeout` event followed by an idempotent `cleanup_complete` event. The +`cleanup_complete` record uses `cleanup_status = "merged_closeout_reconciled"` and +must sort after the companion `closeout` record so status consumers see +`cleanup_complete` as the final lifecycle outcome. + `terminal_path` values must match the runtime-owned terminal path for the tool or phase that writes the event. For normal review handoff this is `review_handoff`; for retained repair completion this is `review_repair`; for explicit human-required exits this is diff --git a/docs/spec/runtime.md b/docs/spec/runtime.md index e0e2c5062..30882e34a 100644 --- a/docs/spec/runtime.md +++ b/docs/spec/runtime.md @@ -628,7 +628,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 active service label, managed clean worktree, PR repository, default-branch target, exact branch/head match, and green landable PR gates. 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 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. 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 @@ -761,7 +761,13 @@ After a process restart, recent-run history, active lease ownership, retained po `attention_count` even when no active run, queued candidate, or post-review lane currently owns it. A retained worktree for that same issue must be projected as retained attention, not neutral cleanup-only hygiene, so monitors do not need to - parse `history_lanes` to discover a human-required terminal outcome. + parse `history_lanes` to discover a human-required terminal outcome. When live + tracker readback proves the issue is in the configured completed state, the service + queue, active, and needs-attention labels are absent, no retained worktree or + post-review lane owns the issue, and an explicit `recover merged-closeout` path has + or can write `closeout` plus `cleanup_complete` records after validating the merged + PR lineage, that stale terminal attention is a resolved history echo rather than + current project 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 diff --git a/plugins/decodex/skills/automation/SKILL.md b/plugins/decodex/skills/automation/SKILL.md index c4b2f6ce1..3346bc702 100644 --- a/plugins/decodex/skills/automation/SKILL.md +++ b/plugins/decodex/skills/automation/SKILL.md @@ -77,8 +77,15 @@ cargo run -p decodex --bin decodex -- serve not an automation retry path. Use it only when a verified human-owned PR from a managed worktree should be adopted into the retained review/landing lifecycle. It may reuse an existing worktree mapping only when that mapping points at the same current - checkout; a stale mapping branch name is repaired by the successful adopt write. It - must not clear needs-attention or replace normal retained-lane rebind. + checkout; a stale mapping branch name is repaired by the successful adopt write. If + the active service label is missing, dry-run must report whether live adopt will + restore it after validation. Adopt must not clear needs-attention or replace normal + retained-lane rebind. +- `recover merged-closeout` is the explicit stale retained-attention reconciliation + path after a PR was already merged and the issue is already completed. Use dry-run + first; live recovery requires manual authority and writes `closeout` plus + `cleanup_complete` only after PR lineage, origin/default containment, labels, and + retained worktree safety checks pass. - A lane is terminal only after exactly one terminal path is finalized: `review_handoff` or `manual_attention`. - `phase = terminal_pending` means the agent already called terminal finalize and diff --git a/plugins/decodex/skills/labels/SKILL.md b/plugins/decodex/skills/labels/SKILL.md index ebab9cd0f..70c8a9cbc 100644 --- a/plugins/decodex/skills/labels/SKILL.md +++ b/plugins/decodex/skills/labels/SKILL.md @@ -48,10 +48,14 @@ If a project-scoped command supplies `--config `, read that project only when a current same-PR same-head handoff marker proves stale failure-state drift. - Humans should not normally add or clear `decodex:active:` unless doing explicit recovery and the runtime-owned state has been verified. -- For `recover review-handoff adopt`, restore `decodex:active:` only after - verifying the issue, service ID, managed worktree, PR branch, and PR head belong to - the same manual takeover lane. Adopt must then validate the label again before it - writes runtime handoff state. +- For `recover review-handoff adopt`, do not hand-add + `decodex:active:` just to satisfy the command. The dry-run reports + missing active ownership and whether live adopt will restore it. The live command + restores the active label only after validating the issue, service ID, managed + worktree, PR branch, and PR head belong to the same manual takeover lane. +- For `recover merged-closeout`, queue, active, and needs-attention labels must already + be absent. The command reconciles stale runtime/ledger attention after merged PR + proof; it does not clear labels as a shortcut. ## Queue an Issue diff --git a/plugins/decodex/skills/land/SKILL.md b/plugins/decodex/skills/land/SKILL.md index fc8e17ca0..27ff8844d 100644 --- a/plugins/decodex/skills/land/SKILL.md +++ b/plugins/decodex/skills/land/SKILL.md @@ -65,8 +65,9 @@ but intentionally skips tracker closeout and active-label ownership checks. - Use `recover review-handoff adopt` only when no retained review handoff marker exists, any existing worktree mapping points at the same current managed checkout, and the current clean worktree exactly matches the PR head branch and SHA. A stale mapping - branch name is repaired by adopt; if a retained marker exists or the mapping points - elsewhere, use normal land or `recover review-handoff rebind` instead. + branch name is repaired by adopt; if the active service label is missing, live adopt + may restore it after all other validation passes. If a retained marker exists or the + mapping points elsewhere, use normal land or `recover review-handoff rebind` instead. - Do not substitute `gh pr merge`, GitHub UI, merge queue, raw `git`, direct GitHub API mutation, or a hand-assembled merge for a failed or unavailable `decodex land`. - If GitHub merge already happened but `decodex land` stopped during closeout or diff --git a/plugins/decodex/skills/manual-cli/SKILL.md b/plugins/decodex/skills/manual-cli/SKILL.md index eb3b81a1e..d7034c50c 100644 --- a/plugins/decodex/skills/manual-cli/SKILL.md +++ b/plugins/decodex/skills/manual-cli/SKILL.md @@ -64,11 +64,20 @@ registered project's `WORKFLOW.md`. - Use `recover review-handoff adopt` for a verified human-owned PR that was created from a managed Decodex worktree and should enter normal `decodex land --authority` closeout. Run dry-run first from the lane worktree, then rerun live only after it - confirms the active service label, clean worktree, exact PR branch/head match, and - green landable PR gates. Adopt may reuse an existing worktree mapping only when it - points at the same current managed checkout; a stale mapping branch name is repaired - during the successful adopt write. Do not use adopt when a retained review handoff - marker already exists; use rebind or normal land there. + confirms active-label state, clean worktree, exact PR branch/head match, and green + landable PR gates. If the active service label is missing, live adopt may restore it + after all other validation passes and will report that in dry-run. Adopt may reuse an + existing worktree mapping only when it points at the same current managed checkout; a + stale mapping branch name is repaired during the successful adopt write. Do not use + adopt when a retained review handoff marker already exists; use rebind or normal land + there. +- Use `recover merged-closeout` when Decodex still reports stale retained attention + but the PR was already merged, the tracker issue is completed, and no real retained + patch remains. Run dry-run first with the merged PR URL; live recovery requires + `--manual-authority` and writes `closeout` plus `cleanup_complete` only after it + validates completed issue state, absent queue/active/attention labels, matching PR + head branch, merge commit containment in `origin/`, and clean or + absent retained worktree state. - Use `probe stdio://` before relying on the Codex app-server boundary. - Use `POST /api/linear-scan` after label or issue-state changes when the scheduler should refresh before its next 5-minute Linear poll.