diff --git a/Makefile.toml b/Makefile.toml index 414c8f9f..ae833c6c 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -141,6 +141,7 @@ args = [ # | task | type | cwd | # | ---------------- | --------- | --- | # | lint | composite | | +# | lint-fix | extend | | # | lint-rust | command | | # | lint-vstyle | composite | | # | lint-vstyle-rust | command | | @@ -152,6 +153,9 @@ dependencies = [ "lint-vstyle", ] +[tasks.lint-fix] +extend = "lint" + [tasks.lint-rust] workspace = false command = "cargo" diff --git a/apps/decodex/src/orchestrator/status.rs b/apps/decodex/src/orchestrator/status.rs index 90b73ff1..35079377 100644 --- a/apps/decodex/src/orchestrator/status.rs +++ b/apps/decodex/src/orchestrator/status.rs @@ -2572,9 +2572,9 @@ where let local_head_oid = match validate_post_review_lane_worktree(snapshot, review_handoff) { Ok(local_head_oid) => local_head_oid, Err(reason) => { - return Ok(PostReviewLaneStateLoad::Classification(blocked_post_review_lane( - reason, - ))); + return Ok(PostReviewLaneStateLoad::Classification( + blocked_post_review_lane_from_handoff(review_handoff, reason), + )); }, }; let review_state = match review_state_inspector @@ -2958,6 +2958,17 @@ fn blocked_post_review_lane(reason: &str) -> PostReviewLaneClassification { } } +fn blocked_post_review_lane_from_handoff( + review_handoff: &ReviewHandoffMarker, + reason: &str, +) -> PostReviewLaneClassification { + let mut classification = blocked_post_review_lane(reason); + + classification.pr_url = Some(review_handoff.pr_url().to_owned()); + + classification +} + fn blocked_post_review_lane_status( project: &ServiceConfig, issue: &TrackerIssue, diff --git a/apps/decodex/src/orchestrator/tests/review_landing/status_rows.rs b/apps/decodex/src/orchestrator/tests/review_landing/status_rows.rs index 34814b9c..71119822 100644 --- a/apps/decodex/src/orchestrator/tests/review_landing/status_rows.rs +++ b/apps/decodex/src/orchestrator/tests/review_landing/status_rows.rs @@ -861,6 +861,7 @@ fn build_post_review_lane_statuses_blocks_review_handoff_lineage_rewrite() { assert_eq!(lanes.len(), 1); assert_eq!(lanes[0].classification, "blocked"); assert_eq!(lanes[0].reason, "review_handoff_lineage_mismatch"); + assert_eq!(lanes[0].pr_url.as_deref(), Some(pr_url)); } #[test] diff --git a/apps/decodex/src/recovery.rs b/apps/decodex/src/recovery.rs index 447e8d5b..ea0d16fe 100644 --- a/apps/decodex/src/recovery.rs +++ b/apps/decodex/src/recovery.rs @@ -17,8 +17,8 @@ use crate::{ pull_request::PullRequestLandingState, runtime, state::{ - RUN_ACTIVITY_MARKER_FILE, ReviewHandoffMarker, ReviewOrchestrationMarker, RunAttempt, - StateStore, WorktreeMapping, + RUN_ACTIVITY_MARKER_FILE, ReviewHandoffMarker, ReviewOrchestrationMarker, StateStore, + WorktreeMapping, }, tracker::{ self, IssueTracker, TrackerIssue, @@ -30,6 +30,10 @@ use crate::{ const MISSING_HANDOFF_REASON: &str = "missing_review_handoff_record"; const ORPHANED_REVIEW_HANDOFF_CLASSIFICATION: &str = "orphaned_review_handoff"; +const REVIEW_HANDOFF_BOUND_CLASSIFICATION: &str = "review_handoff_bound"; +const REVIEW_HANDOFF_REBIND_REQUIRED_CLASSIFICATION: &str = "review_handoff_rebind_required"; +const REVIEW_HANDOFF_UNVERIFIED_CLASSIFICATION: &str = "review_handoff_unverified"; +const REVIEW_HANDOFF_MISMATCH_CLASSIFICATION: &str = "review_handoff_mismatch"; const REVIEW_HANDOFF_REBIND_EVENT: &str = "review_handoff_rebind"; const REBOUND_ORCHESTRATION_PHASE: &str = "request_pending"; @@ -73,10 +77,47 @@ struct ReviewHandoffDiagnostic { local_head_oid: Option, worktree_clean: Option, existing_pr_url: Option, + existing_marker_head_oid: Option, + existing_orchestration_head_oid: Option, + pr_base_ref: Option, + pr_head_oid: Option, + mismatched_field: Option, active_label_present: Option, next_action: String, } +struct HandoffBindingDiagnostic { + classification: String, + reason: String, + pr_base_ref: Option, + pr_head_oid: Option, + mismatched_field: Option, + next_action: String, +} + +struct HandoffDiagnosticRequest<'a> { + service_id: &'a str, + issue_identifier: &'a str, + worktree: &'a WorktreeMapping, + existing_handoff: Option<&'a ReviewHandoffMarker>, + existing_orchestration: Option<&'a ReviewOrchestrationMarker>, + local_branch_name: Option<&'a str>, + local_head_oid: Option<&'a str>, + worktree_clean: Option, + pr_inspection: Option<&'a PullRequestLandingState>, + active_label_present: Option, +} + +struct HandoffDiagnosticContext<'a> { + issue_identifier: &'a str, + worktree: &'a WorktreeMapping, + existing_handoff: &'a ReviewHandoffMarker, + existing_orchestration: Option<&'a ReviewOrchestrationMarker>, + local_branch_name: Option<&'a str>, + local_head_oid: Option<&'a str>, + worktree_clean: Option, +} + struct RecoveryContext { config: ServiceConfig, workflow: WorkflowDocument, @@ -87,11 +128,40 @@ struct RecoveryContext { struct RebindValidation { issue: TrackerIssue, worktree: WorktreeMapping, - attempt: RunAttempt, + run_id: String, + attempt_number: i64, landing_state: PullRequestLandingState, local_head_oid: String, worktree_path_for_event: Option, active_label_present: bool, + mode: RebindMode, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum RebindMode { + RestoreMissingHandoff, + RefreshExistingHandoff, +} +impl RebindMode { + fn evidence_value(self) -> &'static str { + match self { + Self::RestoreMissingHandoff => "absent", + Self::RefreshExistingHandoff => "refreshed", + } + } + + fn summary_verb(self) -> &'static str { + match self { + Self::RestoreMissingHandoff => "restored", + Self::RefreshExistingHandoff => "refreshed", + } + } +} + +enum ReviewHandoffLineage { + Descends, + Diverged, + Unknown, } /// Run a read-only retained review handoff diagnostic. @@ -128,12 +198,13 @@ pub(crate) fn run_review_handoff_rebind( if request.dry_run { println!( - "dry run: review handoff rebind validated for project={} issue={} branch={} pr={} head={} active_label_present={}", + "dry run: review handoff rebind validated for project={} issue={} branch={} pr={} head={} mode={} active_label_present={}", context.config.service_id(), validation.issue.identifier, validation.worktree.branch_name(), landing_url(&validation.landing_state), validation.local_head_oid, + validation.mode.evidence_value(), validation.active_label_present ); @@ -143,12 +214,13 @@ pub(crate) fn run_review_handoff_rebind( apply_review_handoff_rebind(&context, &validation)?; println!( - "rebind ok: project={} issue={} branch={} pr={} head={}", + "rebind ok: project={} issue={} branch={} pr={} head={} mode={}", context.config.service_id(), validation.issue.identifier, validation.worktree.branch_name(), landing_url(&validation.landing_state), - validation.local_head_oid + validation.local_head_oid, + validation.mode.evidence_value() ); Ok(()) @@ -235,6 +307,21 @@ fn diagnose_issue_worktree( &issue.id, worktree.branch_name(), )?; + let existing_orchestration = existing_handoff + .as_ref() + .map(|handoff| { + context.state_store.review_orchestration_marker( + context.config.service_id(), + &issue.id, + handoff, + ) + }) + .transpose()? + .flatten(); + let pr_inspection = existing_handoff + .as_ref() + .and_then(|handoff| inspect_project_pull_request(context, handoff.pr_url()).ok()) + .map(|(landing_state, _default_branch)| landing_state); let local_branch_name = worktree_checkout_branch_name(worktree.worktree_path()).ok().flatten(); let local_head_oid = worktree_head_oid(worktree.worktree_path()).ok().flatten(); let worktree_clean = worktree_is_clean(worktree.worktree_path()).ok(); @@ -245,55 +332,281 @@ fn diagnose_issue_worktree( &active_label_name, ) .ok(); - let missing_handoff = existing_handoff.is_none(); + let binding = diagnostic_binding(HandoffDiagnosticRequest { + service_id: context.config.service_id(), + issue_identifier: &issue.identifier, + worktree: &worktree, + existing_handoff: existing_handoff.as_ref(), + existing_orchestration: existing_orchestration.as_ref(), + local_branch_name: local_branch_name.as_deref(), + local_head_oid: local_head_oid.as_deref(), + worktree_clean, + pr_inspection: pr_inspection.as_ref(), + active_label_present, + }); Ok(ReviewHandoffDiagnostic { project_id: context.config.service_id().to_owned(), issue_id: issue.id.clone(), issue_identifier: issue.identifier.clone(), issue_state: issue.state.name.clone(), - classification: diagnostic_classification(existing_handoff.as_ref()), - reason: diagnostic_reason(existing_handoff.as_ref()), + classification: binding.classification, + reason: binding.reason, branch_name: worktree.branch_name().to_owned(), worktree_path: worktree.worktree_path().display().to_string(), local_branch_name, local_head_oid, worktree_clean, - existing_pr_url: existing_handoff.map(|handoff| handoff.pr_url().to_owned()), + existing_pr_url: existing_handoff.as_ref().map(|handoff| handoff.pr_url().to_owned()), + existing_marker_head_oid: existing_handoff + .as_ref() + .map(|handoff| handoff.pr_head_oid().to_owned()), + existing_orchestration_head_oid: existing_orchestration + .as_ref() + .map(|orchestration| orchestration.head_sha().to_owned()), + pr_base_ref: binding.pr_base_ref, + pr_head_oid: binding.pr_head_oid, + mismatched_field: binding.mismatched_field, active_label_present, - next_action: diagnostic_next_action( - context.config.service_id(), - &issue.identifier, - missing_handoff, - ), + next_action: binding.next_action, }) } -fn diagnostic_classification(existing_handoff: Option<&ReviewHandoffMarker>) -> String { - if existing_handoff.is_some() { - return String::from("review_handoff_bound"); +fn diagnostic_binding(request: HandoffDiagnosticRequest<'_>) -> HandoffBindingDiagnostic { + let Some(existing_handoff) = request.existing_handoff else { + return HandoffBindingDiagnostic { + classification: String::from(ORPHANED_REVIEW_HANDOFF_CLASSIFICATION), + reason: String::from(MISSING_HANDOFF_REASON), + pr_base_ref: None, + pr_head_oid: None, + mismatched_field: None, + next_action: missing_handoff_next_action(request.service_id, request.issue_identifier), + }; + }; + let context = HandoffDiagnosticContext { + issue_identifier: request.issue_identifier, + worktree: request.worktree, + existing_handoff, + existing_orchestration: request.existing_orchestration, + local_branch_name: request.local_branch_name, + local_head_oid: request.local_head_oid, + worktree_clean: request.worktree_clean, + }; + let pr_base_ref = request.pr_inspection.map(|pr| pr.base_ref_name.clone()); + let pr_head_oid = request.pr_inspection.map(|pr| pr.head_ref_oid.clone()); + + if let Some(diagnostic) = worktree_binding_mismatch(&context, &pr_base_ref, &pr_head_oid) { + return diagnostic; } - String::from(ORPHANED_REVIEW_HANDOFF_CLASSIFICATION) + let Some(local_head_oid) = request.local_head_oid else { + return mismatched_handoff_diagnostic( + "worktree_head_missing", + "worktree.local_head", + pr_base_ref, + pr_head_oid, + inspect_handoff_next_action(request.issue_identifier, existing_handoff.pr_url()), + ); + }; + let Some(pr_inspection) = request.pr_inspection else { + return HandoffBindingDiagnostic { + classification: String::from(REVIEW_HANDOFF_UNVERIFIED_CLASSIFICATION), + reason: String::from("pull_request_state_read_failed"), + pr_base_ref, + pr_head_oid, + mismatched_field: Some(String::from("pr_url")), + next_action: inspect_handoff_next_action( + request.issue_identifier, + existing_handoff.pr_url(), + ), + }; + }; + + if let Some(diagnostic) = + pull_request_binding_mismatch(&context, pr_inspection, &pr_base_ref, &pr_head_oid) + { + return diagnostic; + } + if let Some(diagnostic) = + marker_head_binding_mismatch(&context, local_head_oid, &pr_base_ref, &pr_head_oid) + { + return diagnostic; + } + + HandoffBindingDiagnostic { + classification: String::from(REVIEW_HANDOFF_BOUND_CLASSIFICATION), + reason: String::from("review_handoff_record_present"), + pr_base_ref, + pr_head_oid, + mismatched_field: None, + next_action: bound_handoff_next_action(request.service_id, request.active_label_present), + } +} + +fn worktree_binding_mismatch( + context: &HandoffDiagnosticContext<'_>, + pr_base_ref: &Option, + pr_head_oid: &Option, +) -> Option { + let mismatch = if context.existing_handoff.branch_name() != context.worktree.branch_name() { + Some(("review_handoff_branch_mismatch", "review_handoff.branch_name")) + } else if context.local_branch_name.is_none() { + Some(("worktree_checkout_branch_missing", "worktree.local_branch")) + } else if context.local_branch_name != Some(context.worktree.branch_name()) { + Some(("worktree_checkout_branch_mismatch", "worktree.local_branch")) + } else if context.worktree_clean == Some(false) { + Some(("worktree_dirty", "worktree.clean")) + } else if context.local_head_oid.is_none() { + Some(("worktree_head_missing", "worktree.local_head")) + } else { + None + }; + + mismatch.map(|(reason, field)| { + mismatched_handoff_diagnostic( + reason, + field, + pr_base_ref.clone(), + pr_head_oid.clone(), + inspect_handoff_next_action( + context.issue_identifier, + context.existing_handoff.pr_url(), + ), + ) + }) +} + +fn pull_request_binding_mismatch( + context: &HandoffDiagnosticContext<'_>, + pr_inspection: &PullRequestLandingState, + pr_base_ref: &Option, + pr_head_oid: &Option, +) -> Option { + if context + .existing_handoff + .target_base_ref_name() + .is_some_and(|base_ref| base_ref != pr_inspection.base_ref_name.as_str()) + { + return Some(rebind_required_diagnostic( + "review_handoff_base_mismatch", + "review_handoff.target_base_ref_name", + pr_base_ref.clone(), + pr_head_oid.clone(), + context.issue_identifier, + context.existing_handoff.pr_url(), + )); + } + + let mismatch = if pr_inspection.head_ref_name != context.worktree.branch_name() { + Some(("pull_request_branch_mismatch", "pull_request.head_ref_name")) + } else if context.local_head_oid != Some(pr_inspection.head_ref_oid.as_str()) { + Some(("pull_request_head_mismatch", "pull_request.head_ref_oid")) + } else { + None + }; + + mismatch.map(|(reason, field)| { + mismatched_handoff_diagnostic( + reason, + field, + pr_base_ref.clone(), + pr_head_oid.clone(), + inspect_handoff_next_action( + context.issue_identifier, + context.existing_handoff.pr_url(), + ), + ) + }) +} + +fn marker_head_binding_mismatch( + context: &HandoffDiagnosticContext<'_>, + local_head_oid: &str, + pr_base_ref: &Option, + pr_head_oid: &Option, +) -> Option { + let mismatch = if context.existing_handoff.pr_head_oid() != local_head_oid { + match worktree_head_descends_from_review_handoff( + context.worktree.worktree_path(), + context.existing_handoff.pr_head_oid(), + local_head_oid, + ) { + ReviewHandoffLineage::Descends => None, + ReviewHandoffLineage::Diverged => + Some(("review_handoff_lineage_mismatch", "review_handoff.pr_head_oid")), + ReviewHandoffLineage::Unknown => + Some(("review_handoff_lineage_check_failed", "review_handoff.pr_head_oid")), + } + } else if let Some(orchestration) = context.existing_orchestration { + orchestration_binding_mismatch(context, orchestration, local_head_oid) + } else { + None + }; + + mismatch.map(|(reason, field)| { + rebind_required_diagnostic( + reason, + field, + pr_base_ref.clone(), + pr_head_oid.clone(), + context.issue_identifier, + context.existing_handoff.pr_url(), + ) + }) } -fn diagnostic_reason(existing_handoff: Option<&ReviewHandoffMarker>) -> String { - if existing_handoff.is_some() { - return String::from("review_handoff_record_present"); +fn orchestration_binding_mismatch( + context: &HandoffDiagnosticContext<'_>, + orchestration: &ReviewOrchestrationMarker, + local_head_oid: &str, +) -> Option<(&'static str, &'static str)> { + if orchestration.branch_name() != context.worktree.branch_name() { + Some(("review_orchestration_branch_mismatch", "review_orchestration.branch_name")) + } else if orchestration.pr_url() != context.existing_handoff.pr_url() { + Some(("review_orchestration_pr_mismatch", "review_orchestration.pr_url")) + } else if orchestration.head_sha() != local_head_oid { + Some(("review_orchestration_head_mismatch", "review_orchestration.head_sha")) + } else { + None } +} - String::from(MISSING_HANDOFF_REASON) +fn mismatched_handoff_diagnostic( + reason: &str, + mismatched_field: &str, + pr_base_ref: Option, + pr_head_oid: Option, + next_action: String, +) -> HandoffBindingDiagnostic { + HandoffBindingDiagnostic { + classification: String::from(REVIEW_HANDOFF_MISMATCH_CLASSIFICATION), + reason: reason.to_owned(), + pr_base_ref, + pr_head_oid, + mismatched_field: Some(mismatched_field.to_owned()), + next_action, + } } -fn diagnostic_next_action( - service_id: &str, +fn rebind_required_diagnostic( + reason: &str, + mismatched_field: &str, + pr_base_ref: Option, + pr_head_oid: Option, issue_identifier: &str, - missing_handoff: bool, -) -> String { - if !missing_handoff { - return String::from("Continue the existing post-review lifecycle; no rebind is needed."); + pr_url: &str, +) -> HandoffBindingDiagnostic { + HandoffBindingDiagnostic { + classification: String::from(REVIEW_HANDOFF_REBIND_REQUIRED_CLASSIFICATION), + reason: reason.to_owned(), + pr_base_ref, + pr_head_oid, + mismatched_field: Some(mismatched_field.to_owned()), + next_action: rebind_refresh_next_action(issue_identifier, pr_url), } +} +fn missing_handoff_next_action(service_id: &str, issue_identifier: &str) -> String { format!( "Inspect PR lineage, ensure label `{}` is present, then run `decodex recover review-handoff rebind {} --pr ` if the PR exactly matches this retained lane.", tracker::automation_active_label(service_id), @@ -301,6 +614,29 @@ fn diagnostic_next_action( ) } +fn bound_handoff_next_action(service_id: &str, active_label_present: Option) -> String { + if active_label_present == Some(false) { + return format!( + "Restore explicit lane ownership with label `{}`, then continue the existing post-review lifecycle.", + tracker::automation_active_label(service_id) + ); + } + + String::from("Continue the existing post-review lifecycle; no rebind is needed.") +} + +fn inspect_handoff_next_action(issue_identifier: &str, pr_url: &str) -> String { + format!( + "Inspect the retained worktree and PR `{pr_url}`; run `decodex recover review-handoff rebind {issue_identifier} --pr {pr_url}` only after the mismatch is repaired." + ) +} + +fn rebind_refresh_next_action(issue_identifier: &str, pr_url: &str) -> String { + format!( + "Run `decodex recover review-handoff rebind {issue_identifier} --pr {pr_url} --dry-run`, then rerun without `--dry-run` to refresh the retained marker if validation passes." + ) +} + fn render_review_handoff_recovery_report(report: &ReviewHandoffRecoveryReport) -> String { let mut output = format!("Review handoff recovery diagnostics for project {}\n", report.project_id); @@ -313,7 +649,7 @@ fn render_review_handoff_recovery_report(report: &ReviewHandoffRecoveryReport) - for diagnostic in &report.diagnostics { output.push_str(&format!( - "- issue: {}\n state: {}\n classification: {}\n reason: {}\n branch: {}\n worktree_path: {}\n local_branch: {}\n local_head: {}\n worktree_clean: {}\n existing_pr_url: {}\n active_label_present: {}\n next_action: {}\n", + "- issue: {}\n state: {}\n classification: {}\n reason: {}\n branch: {}\n worktree_path: {}\n local_branch: {}\n local_head: {}\n worktree_clean: {}\n existing_pr_url: {}\n existing_marker_head: {}\n existing_orchestration_head: {}\n pr_base_ref: {}\n pr_head: {}\n mismatched_field: {}\n active_label_present: {}\n next_action: {}\n", diagnostic.issue_identifier, diagnostic.issue_state, diagnostic.classification, @@ -324,6 +660,11 @@ fn render_review_handoff_recovery_report(report: &ReviewHandoffRecoveryReport) - optional_text(diagnostic.local_head_oid.as_deref()), diagnostic.worktree_clean.map_or_else(|| String::from("unknown"), |clean| clean.to_string()), optional_text(diagnostic.existing_pr_url.as_deref()), + optional_text(diagnostic.existing_marker_head_oid.as_deref()), + optional_text(diagnostic.existing_orchestration_head_oid.as_deref()), + optional_text(diagnostic.pr_base_ref.as_deref()), + optional_text(diagnostic.pr_head_oid.as_deref()), + optional_text(diagnostic.mismatched_field.as_deref()), diagnostic.active_label_present.map_or_else(|| String::from("unknown"), |present| present.to_string()), diagnostic.next_action, )); @@ -342,12 +683,33 @@ fn validate_rebind_request( ) -> Result { let issue = load_issue_by_identifier(&context.tracker, &request.issue)?; let worktree = validate_rebind_issue_context(context, &issue)?; - let attempt = - context.state_store.latest_run_attempt_for_issue(&issue.id)?.ok_or_else(|| { - eyre::eyre!("Issue `{}` has no recorded run attempt to rebind.", issue.identifier) - })?; + let existing_handoff = context.state_store.review_handoff_marker( + context.config.service_id(), + &issue.id, + worktree.branch_name(), + )?; let landing_state = inspect_rebind_pull_request(context, &request.pr_url)?; let local_head_oid = validate_rebind_worktree(&worktree, &landing_state)?; + let existing_orchestration = existing_handoff + .as_ref() + .map(|handoff| { + context.state_store.review_orchestration_marker( + context.config.service_id(), + &issue.id, + handoff, + ) + }) + .transpose()? + .flatten(); + let (run_id, attempt_number, mode) = validate_rebind_existing_handoff( + context, + &issue, + &worktree, + existing_handoff.as_ref(), + existing_orchestration.as_ref(), + &landing_state, + &local_head_oid, + )?; let active_label_present = validate_rebind_tracker_labels(context, &issue)?; let worktree_path_for_event = repository_relative_path(context.config.repo_root(), worktree.worktree_path()); @@ -355,11 +717,13 @@ fn validate_rebind_request( Ok(RebindValidation { issue, worktree, - attempt, + run_id, + attempt_number, landing_state, local_head_oid, worktree_path_for_event, active_label_present, + mode, }) } @@ -404,52 +768,94 @@ fn validate_rebind_issue_context( let worktree = context.state_store.worktree_for_issue(&issue.id)?.ok_or_else(|| { eyre::eyre!("Issue `{}` has no retained worktree mapping.", issue.identifier) })?; - let existing_handoff = context.state_store.review_handoff_marker( - context.config.service_id(), - &issue.id, - worktree.branch_name(), - )?; - if let Some(existing_handoff) = existing_handoff { + Ok(worktree) +} + +fn validate_rebind_existing_handoff( + context: &RecoveryContext, + issue: &TrackerIssue, + worktree: &WorktreeMapping, + existing_handoff: Option<&ReviewHandoffMarker>, + existing_orchestration: Option<&ReviewOrchestrationMarker>, + landing_state: &PullRequestLandingState, + local_head_oid: &str, +) -> Result<(String, i64, RebindMode)> { + let Some(existing_handoff) = existing_handoff else { + let attempt = + context.state_store.latest_run_attempt_for_issue(&issue.id)?.ok_or_else(|| { + eyre::eyre!("Issue `{}` has no recorded run attempt to rebind.", issue.identifier) + })?; + + return Ok(( + attempt.run_id().to_owned(), + attempt.attempt_number(), + RebindMode::RestoreMissingHandoff, + )); + }; + + validate_existing_handoff_refresh( + &issue.identifier, + worktree, + existing_handoff, + existing_orchestration, + landing_state, + local_head_oid, + ) +} + +fn validate_existing_handoff_refresh( + issue_identifier: &str, + worktree: &WorktreeMapping, + existing_handoff: &ReviewHandoffMarker, + existing_orchestration: Option<&ReviewOrchestrationMarker>, + landing_state: &PullRequestLandingState, + local_head_oid: &str, +) -> Result<(String, i64, RebindMode)> { + if existing_handoff.pr_url() != landing_url(landing_state) { eyre::bail!( - "Issue `{}` already has review handoff marker for branch `{}` and PR `{}`.", - issue.identifier, + "Issue `{}` already has review handoff marker for branch `{}` and PR `{}`; refusing to rebind it to `{}`.", + issue_identifier, + worktree.branch_name(), + existing_handoff.pr_url(), + landing_url(landing_state) + ); + } + + let orchestration_is_current = existing_orchestration.is_none_or(|marker| { + marker.branch_name() == worktree.branch_name() + && marker.pr_url() == landing_url(landing_state) + && marker.head_sha() == local_head_oid + }); + + if existing_handoff.pr_head_oid() == local_head_oid && orchestration_is_current { + eyre::bail!( + "Issue `{}` already has a review handoff marker for branch `{}` and PR `{}` at head `{local_head_oid}`; no rebind is needed.", + issue_identifier, worktree.branch_name(), existing_handoff.pr_url() ); } - Ok(worktree) + Ok(( + existing_handoff.run_id().to_owned(), + existing_handoff.attempt_number(), + RebindMode::RefreshExistingHandoff, + )) } fn inspect_rebind_pull_request( context: &RecoveryContext, pr_url: &str, ) -> Result { - let github_token = context.config.github().resolve_token()?; - let repository = github::inspect_repository_context(context.config.repo_root(), &github_token)?; + let (landing_state, default_branch) = inspect_project_pull_request(context, pr_url)?; - if !github::pull_request_matches_repository(pr_url, &repository)? { - eyre::bail!( - "Pull request `{}` does not belong to configured repository `{}/{}`.", - pr_url, - repository.owner, - repository.name - ); - } - - let landing_state = github::inspect_pull_request_landing_state( - context.config.repo_root(), - pr_url, - &github_token, - )?; - - if landing_state.base_ref_name != repository.default_branch { + if landing_state.base_ref_name != default_branch { eyre::bail!( "Pull request `{}` targets `{}`, but configured default branch is `{}`.", pr_url, landing_state.base_ref_name, - repository.default_branch + default_branch ); } if landing_state.state != "OPEN" { @@ -465,6 +871,31 @@ fn inspect_rebind_pull_request( Ok(landing_state) } +fn inspect_project_pull_request( + context: &RecoveryContext, + pr_url: &str, +) -> Result<(PullRequestLandingState, String)> { + let github_token = context.config.github().resolve_token()?; + let repository = github::inspect_repository_context(context.config.repo_root(), &github_token)?; + + if !github::pull_request_matches_repository(pr_url, &repository)? { + eyre::bail!( + "Pull request `{}` does not belong to configured repository `{}/{}`.", + pr_url, + repository.owner, + repository.name + ); + } + + let landing_state = github::inspect_pull_request_landing_state( + context.config.repo_root(), + pr_url, + &github_token, + )?; + + Ok((landing_state, repository.default_branch)) +} + fn validate_rebind_worktree( worktree: &WorktreeMapping, landing_state: &PullRequestLandingState, @@ -527,8 +958,8 @@ fn apply_review_handoff_rebind( validation: &RebindValidation, ) -> Result<()> { let handoff_marker = ReviewHandoffMarker::new( - validation.attempt.run_id(), - validation.attempt.attempt_number(), + validation.run_id.clone(), + validation.attempt_number, validation.worktree.branch_name(), landing_url(&validation.landing_state), validation.landing_state.base_ref_name.clone(), @@ -536,8 +967,8 @@ fn apply_review_handoff_rebind( validation.local_head_oid.clone(), ); let orchestration_marker = ReviewOrchestrationMarker::new( - validation.attempt.run_id(), - validation.attempt.attempt_number(), + validation.run_id.clone(), + validation.attempt_number, validation.worktree.branch_name(), landing_url(&validation.landing_state), validation.local_head_oid.clone(), @@ -593,8 +1024,8 @@ fn review_handoff_rebind_event( service_id: context.config.service_id(), issue_id: &validation.issue.id, issue_identifier: &validation.issue.identifier, - run_id: validation.attempt.run_id(), - attempt_number: validation.attempt.attempt_number(), + run_id: &validation.run_id, + attempt_number: validation.attempt_number, }, REVIEW_HANDOFF_REBIND_EVENT, current_timestamp(), @@ -609,15 +1040,16 @@ fn review_handoff_rebind_event( event.commit_sha = Some(validation.local_head_oid.clone()); event.validation_result = Some(String::from("passed")); event.summary = Some(format!( - "Explicit operator rebind restored retained review handoff marker for {}.", - validation.issue.identifier + "Explicit operator rebind {} retained review handoff marker for {}.", + validation.mode.summary_verb(), + validation.issue.identifier, )); event.evidence = Some(vec![ format!("issue_state={}", validation.issue.state.name), format!("branch={}", validation.worktree.branch_name()), format!("pr_url={pr_url}"), format!("pr_head_sha={}", validation.local_head_oid), - String::from("existing_review_handoff_marker=absent"), + format!("existing_review_handoff_marker={}", validation.mode.evidence_value()), ]); event.next_action = Some(String::from("continue retained post-review lifecycle")); @@ -630,7 +1062,8 @@ fn write_rebind_audit( event: &LinearExecutionEventRecord, ) -> Result<()> { let body = format!( - "Decodex operator recovery: rebound retained review handoff marker for `{}` to `{}`. This does not land the pull request.", + "Decodex operator recovery: {} retained review handoff marker for `{}` to `{}`. This does not land the pull request.", + validation.mode.summary_verb(), validation.issue.identifier, landing_url(&validation.landing_state) ); @@ -699,6 +1132,31 @@ fn worktree_head_oid(worktree_path: &Path) -> Result> { ) } +fn worktree_head_descends_from_review_handoff( + worktree_path: &Path, + recorded_head_oid: &str, + local_head_oid: &str, +) -> ReviewHandoffLineage { + if recorded_head_oid == local_head_oid { + return ReviewHandoffLineage::Descends; + } + + let Ok(output) = Command::new("git") + .arg("-C") + .arg(worktree_path) + .args(["merge-base", "--is-ancestor", recorded_head_oid, local_head_oid]) + .output() + else { + return ReviewHandoffLineage::Unknown; + }; + + match output.status.code() { + Some(0) => ReviewHandoffLineage::Descends, + Some(1) => ReviewHandoffLineage::Diverged, + _ => ReviewHandoffLineage::Unknown, + } +} + fn worktree_is_clean(worktree_path: &Path) -> Result { Ok(worktree_blocking_status_lines(worktree_path)?.is_empty()) } @@ -748,11 +1206,358 @@ fn repository_relative_path(repo_root: &Path, path: &Path) -> Option { #[cfg(test)] mod tests { + use std::{fs, path::Path}; + + use tempfile::TempDir; + use crate::{ - recovery::REVIEW_HANDOFF_REBIND_EVENT, + pull_request::PullRequestLandingState, + recovery::{ + REVIEW_HANDOFF_BOUND_CLASSIFICATION, REVIEW_HANDOFF_REBIND_EVENT, + REVIEW_HANDOFF_REBIND_REQUIRED_CLASSIFICATION, + }, + state::{ReviewHandoffMarker, ReviewOrchestrationMarker, StateStore, WorktreeMapping}, tracker::records::{self, LinearExecutionEventIdentity, LinearExecutionEventRecord}, }; + fn sample_worktree(branch_name: &str) -> WorktreeMapping { + sample_worktree_at(branch_name, Path::new("/tmp/PUB-718")) + } + + fn sample_worktree_at(branch_name: &str, worktree_path: &Path) -> WorktreeMapping { + let store = StateStore::open_in_memory().expect("state store should open"); + let worktree_path = worktree_path.to_string_lossy(); + + store + .upsert_worktree("pubfi", "issue-id", branch_name, &worktree_path) + .expect("worktree should persist"); + + store + .worktree_for_issue("issue-id") + .expect("worktree should read") + .expect("worktree should exist") + } + + fn sample_landing_state( + pr_url: &str, + branch_name: &str, + head_oid: &str, + ) -> PullRequestLandingState { + PullRequestLandingState { + url: pr_url.to_owned(), + state: String::from("OPEN"), + is_draft: false, + review_decision: Some(String::from("APPROVED")), + base_ref_name: String::from("main"), + pending_review_requests: 0, + mergeable: String::from("MERGEABLE"), + merge_state_status: String::from("CLEAN"), + head_ref_name: branch_name.to_owned(), + head_ref_oid: head_oid.to_owned(), + status_check_rollup_state: Some(String::from("SUCCESS")), + unresolved_review_threads: 0, + } + } + + fn run_git(repo: &Path, args: &[&str]) -> String { + let output = std::process::Command::new("git") + .arg("-C") + .arg(repo) + .args(args) + .output() + .expect("git command should run"); + + assert!( + output.status.success(), + "git {:?} failed: {}", + args, + String::from_utf8_lossy(&output.stderr) + ); + + String::from_utf8(output.stdout).expect("git stdout should be utf8").trim().to_owned() + } + + fn commit_file(repo: &Path, contents: &str) -> String { + fs::write(repo.join("tracked.txt"), contents).expect("tracked file should write"); + + run_git(repo, &["add", "tracked.txt"]); + run_git(repo, &["commit", "-m", "test commit"]); + + run_git(repo, &["rev-parse", "HEAD"]) + } + + fn temp_git_worktree(branch_name: &str) -> (TempDir, String, String) { + let temp_dir = TempDir::new().expect("temp git repo should exist"); + let repo = temp_dir.path(); + + run_git(repo, &["init"]); + run_git(repo, &["config", "user.email", "decodex@example.invalid"]); + run_git(repo, &["config", "user.name", "Decodex Test"]); + run_git(repo, &["checkout", "-b", branch_name]); + + let first_head = commit_file(repo, "first\n"); + let second_head = commit_file(repo, "second\n"); + + (temp_dir, first_head, second_head) + } + + fn temp_rebased_git_worktree(branch_name: &str) -> (TempDir, String, String) { + let (temp_dir, first_head, _) = temp_git_worktree(branch_name); + let repo = temp_dir.path(); + + run_git(repo, &["checkout", "--orphan", "rebased"]); + run_git(repo, &["rm", "-rf", "."]); + + let rebased_head = commit_file(repo, "rebased\n"); + + run_git(repo, &["branch", "-D", branch_name]); + run_git(repo, &["branch", "-m", branch_name]); + + (temp_dir, first_head, rebased_head) + } + + #[test] + fn diagnostic_treats_descendant_handoff_head_as_bound() { + let branch_name = "x/pubfi-pub-718"; + let pr_url = "https://github.com/hack-ink/pubfi-mono-v2/pull/14"; + let (temp_dir, original_head, current_head) = temp_git_worktree(branch_name); + let worktree = sample_worktree_at(branch_name, temp_dir.path()); + let handoff = ReviewHandoffMarker::new( + "pub-718-attempt-1", + 1, + branch_name, + pr_url, + "main", + branch_name, + original_head, + ); + let landing_state = sample_landing_state(pr_url, branch_name, ¤t_head); + let diagnostic = super::diagnostic_binding(super::HandoffDiagnosticRequest { + service_id: "pubfi", + issue_identifier: "PUB-718", + worktree: &worktree, + existing_handoff: Some(&handoff), + existing_orchestration: None, + local_branch_name: Some(branch_name), + local_head_oid: Some(¤t_head), + worktree_clean: Some(true), + pr_inspection: Some(&landing_state), + active_label_present: Some(true), + }); + + assert_eq!(diagnostic.classification, REVIEW_HANDOFF_BOUND_CLASSIFICATION); + assert_eq!(diagnostic.reason, "review_handoff_record_present"); + assert_eq!(diagnostic.mismatched_field, None); + } + + #[test] + fn diagnostic_requires_refresh_when_handoff_head_is_stale() { + let branch_name = "x/pubfi-pub-718"; + let pr_url = "https://github.com/hack-ink/pubfi-mono-v2/pull/14"; + let (temp_dir, original_head, rebased_head) = temp_rebased_git_worktree(branch_name); + let worktree = sample_worktree_at(branch_name, temp_dir.path()); + let handoff = ReviewHandoffMarker::new( + "pub-718-attempt-1", + 1, + branch_name, + pr_url, + "main", + branch_name, + original_head, + ); + let landing_state = sample_landing_state(pr_url, branch_name, &rebased_head); + let diagnostic = super::diagnostic_binding(super::HandoffDiagnosticRequest { + service_id: "pubfi", + issue_identifier: "PUB-718", + worktree: &worktree, + existing_handoff: Some(&handoff), + existing_orchestration: None, + local_branch_name: Some(branch_name), + local_head_oid: Some(&rebased_head), + worktree_clean: Some(true), + pr_inspection: Some(&landing_state), + active_label_present: Some(true), + }); + + assert_eq!(diagnostic.classification, REVIEW_HANDOFF_REBIND_REQUIRED_CLASSIFICATION); + assert_eq!(diagnostic.reason, "review_handoff_lineage_mismatch"); + assert_eq!(diagnostic.pr_head_oid.as_deref(), Some(rebased_head.as_str())); + assert_eq!(diagnostic.mismatched_field.as_deref(), Some("review_handoff.pr_head_oid")); + assert!(diagnostic.next_action.contains("rebind PUB-718")); + assert!(diagnostic.next_action.contains("--dry-run")); + } + + #[test] + fn diagnostic_requires_refresh_when_orchestration_head_is_stale() { + let branch_name = "x/pubfi-pub-718"; + let pr_url = "https://github.com/hack-ink/pubfi-mono-v2/pull/14"; + let head_oid = "1123456789abcdef0123456789abcdef01234567"; + let worktree = sample_worktree(branch_name); + let handoff = ReviewHandoffMarker::new( + "pub-718-attempt-1", + 1, + branch_name, + pr_url, + "main", + branch_name, + head_oid, + ); + let orchestration = ReviewOrchestrationMarker::new( + "pub-718-attempt-1", + 1, + branch_name, + pr_url, + "0123456789abcdef0123456789abcdef01234567", + "waiting_for_result", + None, + None, + None, + 0, + 0, + None, + ); + let landing_state = sample_landing_state(pr_url, branch_name, head_oid); + let diagnostic = super::diagnostic_binding(super::HandoffDiagnosticRequest { + service_id: "pubfi", + issue_identifier: "PUB-718", + worktree: &worktree, + existing_handoff: Some(&handoff), + existing_orchestration: Some(&orchestration), + local_branch_name: Some(branch_name), + local_head_oid: Some(head_oid), + worktree_clean: Some(true), + pr_inspection: Some(&landing_state), + active_label_present: Some(true), + }); + + assert_eq!(diagnostic.classification, REVIEW_HANDOFF_REBIND_REQUIRED_CLASSIFICATION); + assert_eq!(diagnostic.reason, "review_orchestration_head_mismatch"); + assert_eq!(diagnostic.mismatched_field.as_deref(), Some("review_orchestration.head_sha")); + } + + #[test] + fn diagnostic_bound_handoff_reports_missing_active_ownership_recovery() { + let branch_name = "x/pubfi-pub-718"; + let pr_url = "https://github.com/hack-ink/pubfi-mono-v2/pull/14"; + let head_oid = "1123456789abcdef0123456789abcdef01234567"; + let worktree = sample_worktree(branch_name); + let handoff = ReviewHandoffMarker::new( + "pub-718-attempt-1", + 1, + branch_name, + pr_url, + "main", + branch_name, + head_oid, + ); + let orchestration = ReviewOrchestrationMarker::new( + "pub-718-attempt-1", + 1, + branch_name, + pr_url, + head_oid, + "request_pending", + None, + None, + None, + 0, + 0, + None, + ); + let landing_state = sample_landing_state(pr_url, branch_name, head_oid); + let diagnostic = super::diagnostic_binding(super::HandoffDiagnosticRequest { + service_id: "pubfi", + issue_identifier: "PUB-718", + worktree: &worktree, + existing_handoff: Some(&handoff), + existing_orchestration: Some(&orchestration), + local_branch_name: Some(branch_name), + local_head_oid: Some(head_oid), + worktree_clean: Some(true), + pr_inspection: Some(&landing_state), + active_label_present: Some(false), + }); + + assert_eq!(diagnostic.classification, REVIEW_HANDOFF_BOUND_CLASSIFICATION); + assert_eq!(diagnostic.reason, "review_handoff_record_present"); + assert!(diagnostic.next_action.contains("decodex:active:pubfi")); + assert!(diagnostic.next_action.contains("Restore explicit lane ownership")); + } + + #[test] + fn rebind_validation_refreshes_existing_same_branch_pr_marker() { + let branch_name = "x/pubfi-pub-718"; + let pr_url = "https://github.com/hack-ink/pubfi-mono-v2/pull/14"; + let worktree = sample_worktree(branch_name); + let handoff = ReviewHandoffMarker::new( + "pub-718-attempt-1", + 1, + branch_name, + pr_url, + "main", + branch_name, + "0123456789abcdef0123456789abcdef01234567", + ); + let landing_state = + sample_landing_state(pr_url, branch_name, "1123456789abcdef0123456789abcdef01234567"); + let (run_id, attempt_number, mode) = super::validate_existing_handoff_refresh( + "PUB-718", + &worktree, + &handoff, + None, + &landing_state, + "1123456789abcdef0123456789abcdef01234567", + ) + .expect("stale existing marker should be refreshable"); + + assert_eq!(run_id, "pub-718-attempt-1"); + assert_eq!(attempt_number, 1); + assert_eq!(mode, super::RebindMode::RefreshExistingHandoff); + } + + #[test] + fn rebind_validation_rejects_current_existing_marker_as_noop() { + let branch_name = "x/pubfi-pub-718"; + let pr_url = "https://github.com/hack-ink/pubfi-mono-v2/pull/14"; + let head_oid = "1123456789abcdef0123456789abcdef01234567"; + let worktree = sample_worktree(branch_name); + let handoff = ReviewHandoffMarker::new( + "pub-718-attempt-1", + 1, + branch_name, + pr_url, + "main", + branch_name, + head_oid, + ); + let orchestration = ReviewOrchestrationMarker::new( + "pub-718-attempt-1", + 1, + branch_name, + pr_url, + head_oid, + "request_pending", + None, + None, + None, + 0, + 0, + None, + ); + let landing_state = sample_landing_state(pr_url, branch_name, head_oid); + let error = super::validate_existing_handoff_refresh( + "PUB-718", + &worktree, + &handoff, + Some(&orchestration), + &landing_state, + head_oid, + ) + .expect_err("current existing marker should not be rebound"); + + assert!(error.to_string().contains("no rebind is needed")); + } + #[test] fn review_handoff_rebind_event_validation_accepts_required_fields() { let mut record = LinearExecutionEventRecord::new( diff --git a/docs/reference/operator-control-plane.md b/docs/reference/operator-control-plane.md index 9b156d8e..118cf20a 100644 --- a/docs/reference/operator-control-plane.md +++ b/docs/reference/operator-control-plane.md @@ -153,6 +153,12 @@ Worktree visibility follows the owning dashboard section: this as an orphaned retained review lane: inspect it with `decodex recover review-handoff diagnose `, then use the explicit rebind path only after the PR URL and retained worktree lineage match exactly. +- Review handoff or orchestration head mismatch reasons mean Decodex found a retained + marker but one stored field no longer matches the clean retained worktree and PR + head. `decodex status` keeps the bound PR URL visible when it can identify the + marker, and `decodex recover review-handoff diagnose ` reports the stored + marker head, orchestration head, PR head, and mismatched field before any explicit + rebind refresh. - `Intake Queue` means queued attention still owns the path, including partial retained progress after retries. - `Recovery Worktrees` means the path is retained local state after the authoritative diff --git a/docs/runbook/index.md b/docs/runbook/index.md index ac428447..08069522 100644 --- a/docs/runbook/index.md +++ b/docs/runbook/index.md @@ -39,7 +39,8 @@ Question this index answers: "which sequence should I execute?" - [`social-publishing-workflow.md`](./social-publishing-workflow.md) for turning Radar evidence into reviewed `@decodexspace` X drafts and recording publication evidence. - [`recover-review-handoff.md`](./recover-review-handoff.md) for diagnosing and - explicitly rebinding retained review lanes blocked by a missing runtime DB handoff + explicitly rebinding retained review lanes blocked by a missing or stale runtime DB + handoff marker. - [`self-dogfood-pilot.md`](./self-dogfood-pilot.md) for the retained-lane pilot run against `decodex` itself and the bounded live-operation sequence. diff --git a/docs/runbook/recover-review-handoff.md b/docs/runbook/recover-review-handoff.md index 58c20424..45963689 100644 --- a/docs/runbook/recover-review-handoff.md +++ b/docs/runbook/recover-review-handoff.md @@ -1,10 +1,11 @@ -# Recover Missing Review Handoff +# Recover Review Handoff Purpose: Diagnose and explicitly repair retained review lanes that are blocked by a -missing runtime DB handoff marker. +missing or stale runtime DB review-handoff marker. -Use this when: `decodex status` or the dashboard shows a `Review & Landing` -lane blocked with `missing_review_handoff_record`. +Use this when: `decodex status` or the dashboard shows a `Review & Landing` lane +blocked with `missing_review_handoff_record`, a review handoff/orchestration head +mismatch, or a similar retained review marker mismatch after manual repair or rebase. Do not use this for: healthy PR handoffs, review repair, landing, closeout, cleanup-only worktrees, or manual PR landing. @@ -25,7 +26,9 @@ Use `--json` for a structured report. The diagnostic is read-only. It reports the issue, tracker state, branch, worktree, local branch, local head, worktree cleanliness, existing PR URL when one is already -bound, active automation label presence, and the suggested next command. +bound, stored handoff head, stored orchestration head, PR base/head when readable, +the mismatched field when one is known, active automation label presence, and the +suggested next command. ## Explicit Rebind @@ -34,8 +37,9 @@ This is a break-glass path. Healthy lanes should keep using operators should not use rebind just because a normal review handoff is still in progress. -Only rebind when the diagnosis says the review handoff marker is absent and the PR -lineage has been checked. +Only rebind when the diagnosis says the review handoff marker is absent, or says the +existing same-branch same-PR marker must be refreshed after the retained worktree and +PR head have been checked. ```sh decodex recover review-handoff rebind --pr --dry-run @@ -43,8 +47,10 @@ decodex recover review-handoff rebind --pr ``` The non-dry-run command writes runtime DB handoff/orchestration markers and records a -`review_handoff_rebind` audit event on the tracker issue. It does not merge the PR, -change issue state, queue follow-up issues, or clean worktrees. +`review_handoff_rebind` audit event on the tracker issue. For a stale existing marker, +it refreshes only the same branch and same PR after validating the clean retained +worktree head matches the PR head. It does not merge the PR, change issue state, queue +follow-up issues, or clean worktrees. The command rejects the rebind unless all of these are true: @@ -57,7 +63,8 @@ The command rejects the rebind unless all of these are true: - the PR targets the configured default branch - the PR is open and non-draft - the PR head branch and head SHA match the retained worktree -- no review handoff marker already exists for the issue/branch +- no review handoff marker already exists for the issue/branch, or the existing marker + is for the same branch and PR and needs a head/orchestration refresh After a successful rebind, run: @@ -69,3 +76,23 @@ decodex run --dry-run The lane should leave `missing_review_handoff_record` and return to the existing post-review lifecycle classification such as waiting for review, ready to land, review repair required, or blocked for a different concrete reason. + +## Active Ownership Recovery + +If diagnosis reports `classification: review_handoff_bound` but +`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. + +After restoring explicit ownership, rerun: + +```sh +decodex recover review-handoff diagnose +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. diff --git a/docs/spec/post-review-lifecycle.md b/docs/spec/post-review-lifecycle.md index 09482b64..8660018a 100644 --- a/docs/spec/post-review-lifecycle.md +++ b/docs/spec/post-review-lifecycle.md @@ -72,15 +72,25 @@ not infer a PR lineage from branch names, current heads, PR titles, or Linear co and `decodex run` must not repair this state automatically. The supported operator recovery surface is `decodex recover review-handoff`. This is a -break-glass recovery path for orphaned retained review lanes, not part of the normal -automation success path. +break-glass recovery path for orphaned retained review lanes and stale retained marker +heads after explicit manual repair or rebase. It is not part of the normal automation +success path. - `diagnose` is read-only. It reports the project, issue, branch, worktree, local head, - active automation label, existing PR URL when present, and the missing marker reason. + active automation label, existing PR URL when present, stored handoff head, stored + orchestration head, PR base/head when readable, and the missing or mismatched marker + reason. A diagnostic may report a bound marker, a missing marker, an unverified PR + read, or a concrete field mismatch that requires explicit rebind. - `rebind` is mutating and requires an explicit issue identifier plus PR URL. It must validate the configured project, tracker issue, success-state compatibility, active automation ownership, retained worktree branch, clean worktree, PR repository, PR base, PR head branch, PR head SHA, and open non-draft PR state before writing markers. +- If no review handoff marker exists, `rebind` restores the missing handoff and + orchestration markers from the validated PR and retained worktree. If a marker already + exists for the same branch and PR but its stored handoff or orchestration head is + stale, `rebind` may refresh that marker to the validated PR head. It must reject an + existing marker for a different PR, and it must reject a current same-branch same-PR + marker as a no-op. - A successful rebind writes the same runtime DB handoff and orchestration marker shapes as normal `issue_review_handoff` needs, and records a `review_handoff_rebind` audit event. It does not land the PR, queue follow-up work, or substitute for healthy lanes' diff --git a/docs/spec/runtime.md b/docs/spec/runtime.md index f8f3ccb9..08fa583c 100644 --- a/docs/spec/runtime.md +++ b/docs/spec/runtime.md @@ -314,7 +314,7 @@ The runtime database stores at least: - typed connector health and external API backoff For child supervision, the active lane may also carry a short-lived worktree heartbeat marker at `.worktrees//.decodex-run-activity`. That marker is advisory, keyed to the current `run_id` plus `attempt_number`, and exists so the control plane can observe child activity across process boundaries, surface active thread and protocol progress in operator status, and keep high-frequency telemetry out of Linear. When the marker records process liveness, it must pair `process_id` with both host boot identity (`host_boot_id`) and process start identity (`process_start_identity`). A marker from a previous boot, a marker missing either identity, a marker whose process start identity no longer matches the live PID, or a marker observed while Decodex cannot read the current host or process identity must not be treated as a live process even if that PID currently exists. Operator snapshots expose `process_liveness_reason` so operators can distinguish stopped processes, previous-boot markers, and same-boot PID reuse from genuine live execution. The marker may also carry an additive `child_agent_activity` JSON summary for the current attempt; that summary is diagnostic state for operator snapshots, not durable scheduling authority. Operator snapshots must keep queue ownership separate from execution liveness: `active_lease` and `queue_lease_state` describe the local queue lease, while `execution_liveness` describes the observed process, app-server thread, or protocol marker that keeps an active lane visible. If a raw attempt is still `starting` after app-server thread, model, or protocol activity is observed, operator-facing `status` must report `running` and preserve the raw value in `attempt_status`. High-frequency heartbeat, child-agent buckets, token counts, idle ages, and other transient liveness details stay local/operator-only under the boundary defined by [`linear-execution-ledger.md`](./linear-execution-ledger.md). -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. +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. 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. The only source-tree marker that clean-source checks may ignore is the untracked `.decodex-run-activity` heartbeat marker. Review handoff, orchestration, retry, phase timing, and retained PR state belong in the Decodex runtime database, not in root-level or worktree-local review marker files. ### Dispatch-slot handoff invariant