diff --git a/apps/decodex/src/cli.rs b/apps/decodex/src/cli.rs index 8634943b3..091ca7b5e 100644 --- a/apps/decodex/src/cli.rs +++ b/apps/decodex/src/cli.rs @@ -35,8 +35,8 @@ use crate::{ RadarValidateRequest, }, recovery::{ - self, LegacyCloseoutRecoveryRequest, ReviewHandoffDiagnoseRequest, - ReviewHandoffRebindRequest, + self, LegacyCloseoutRecoveryRequest, ReviewHandoffAdoptRequest, + ReviewHandoffDiagnoseRequest, ReviewHandoffRebindRequest, }, research_design::{ self, ResearchDesignCompileRequest, ResearchDesignOutcome, ResearchDesignPromoteRequest, @@ -888,6 +888,14 @@ impl ReviewHandoffRecoveryCommand { dry_run: args.dry_run, }, ), + ReviewHandoffRecoverySubcommand::Adopt(args) => recovery::run_review_handoff_adopt( + config_path, + &ReviewHandoffAdoptRequest { + issue: args.issue.clone(), + pr_url: args.pr.clone(), + dry_run: args.dry_run, + }, + ), } } } @@ -915,6 +923,19 @@ struct ReviewHandoffRebindCommand { dry_run: bool, } +#[derive(Debug, Args)] +struct ReviewHandoffAdoptCommand { + /// Issue identifier for the human-owned review lane. + #[arg(value_name = "ISSUE")] + issue: String, + /// Pull request URL to adopt after validation. + #[arg(long, value_name = "URL")] + pr: String, + /// Validate only; do not write runtime markers or tracker audit comments. + #[arg(long)] + dry_run: bool, +} + #[derive(Debug, Args)] struct LegacyCloseoutRecoveryCommand { /// Issue identifier for the legacy cleanup-only worktree. @@ -1689,6 +1710,8 @@ enum ReviewHandoffRecoverySubcommand { Diagnose(ReviewHandoffDiagnoseCommand), /// Explicitly bind a validated PR URL to one retained review lane. Rebind(ReviewHandoffRebindCommand), + /// Adopt a verified human-owned PR into the retained review lifecycle. + Adopt(ReviewHandoffAdoptCommand), } #[derive(Debug, Subcommand)] @@ -1830,9 +1853,9 @@ mod tests { RadarRefreshReleaseDeltaCommand, RadarRefreshUpstreamQueueCommand, RadarRenderSignalCommand, RadarSubcommand, RadarValidateCommand, RecoverCommand, RecoverSubcommand, ResearchCommand, ResearchCompileCommand, ResearchOutcomeArg, - ResearchPromoteCommand, ResearchSubcommand, ReviewHandoffDiagnoseCommand, - ReviewHandoffRebindCommand, ReviewHandoffRecoveryCommand, ReviewHandoffRecoverySubcommand, - RunCommand, ServeCommand, StatusCommand, + ResearchPromoteCommand, ResearchSubcommand, ReviewHandoffAdoptCommand, + ReviewHandoffDiagnoseCommand, ReviewHandoffRebindCommand, ReviewHandoffRecoveryCommand, + ReviewHandoffRecoverySubcommand, RunCommand, ServeCommand, StatusCommand, }; #[test] @@ -2797,6 +2820,33 @@ mod tests { )); } + #[test] + fn parses_review_handoff_adopt_dry_run() { + let cli = Cli::parse_from([ + "decodex", + "recover", + "review-handoff", + "adopt", + "XY-944", + "--pr", + "https://github.com/hack-ink/decodex/pull/344", + "--dry-run", + ]); + + assert!(matches!( + cli.command, + Command::Recover(RecoverCommand { + command: RecoverSubcommand::ReviewHandoff(ReviewHandoffRecoveryCommand { + command: ReviewHandoffRecoverySubcommand::Adopt( + ReviewHandoffAdoptCommand { issue, pr, dry_run: true } + ) + }), + .. + }) if issue == "XY-944" + && pr == "https://github.com/hack-ink/decodex/pull/344" + )); + } + #[test] fn parses_legacy_closeout_manual_authority() { let cli = Cli::parse_from([ diff --git a/apps/decodex/src/recovery.rs b/apps/decodex/src/recovery.rs index 2971f0608..f18a29e95 100644 --- a/apps/decodex/src/recovery.rs +++ b/apps/decodex/src/recovery.rs @@ -7,7 +7,7 @@ use std::{ process::Command, }; -use color_eyre::Report; +use color_eyre::{Report, eyre::WrapErr}; use serde::Serialize; use time::{OffsetDateTime, format_description::well_known::Rfc3339}; @@ -15,7 +15,7 @@ use crate::{ config::ServiceConfig, github, prelude::{Result, eyre}, - pull_request::PullRequestLandingState, + pull_request::{self, PullRequestLandingState}, runtime, state::{ self, ConnectorBackoffInput, ReviewHandoffMarker, ReviewOrchestrationMarker, StateStore, @@ -38,6 +38,7 @@ const REVIEW_HANDOFF_REBIND_REQUIRED_CLASSIFICATION: &str = "review_handoff_rebi 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 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 REBOUND_ORCHESTRATION_PHASE: &str = "request_pending"; @@ -64,6 +65,17 @@ pub(crate) struct ReviewHandoffRebindRequest { pub(crate) dry_run: bool, } +/// Explicit manual PR takeover into retained review handoff state. +#[derive(Debug)] +pub(crate) struct ReviewHandoffAdoptRequest { + /// Issue identifier to adopt. + pub(crate) issue: String, + /// Pull request URL to adopt. + pub(crate) pr_url: String, + /// Validate without writing runtime markers or tracker audit comments. + pub(crate) dry_run: bool, +} + /// Explicit legacy closeout audit request. #[derive(Debug)] pub(crate) struct LegacyCloseoutRecoveryRequest { @@ -162,6 +174,20 @@ struct RebindValidation { clear_needs_attention_label: bool, } +struct AdoptValidation { + issue: TrackerIssue, + branch_name: String, + worktree_path: PathBuf, + run_id: String, + attempt_number: i64, + landing_state: PullRequestLandingState, + local_head_oid: String, + worktree_path_for_event: Option, + active_label_present: bool, + success_state_transition: Option, + previous_worktree_mapping: Option, +} + struct LegacyCloseoutValidation { issue: TrackerIssue, worktree: WorktreeMapping, @@ -311,6 +337,52 @@ pub(crate) fn run_review_handoff_rebind( Ok(()) } +/// Run an explicit manual PR takeover into retained review handoff state. +pub(crate) fn run_review_handoff_adopt( + config_path: Option<&Path>, + request: &ReviewHandoffAdoptRequest, +) -> Result<()> { + let context = load_recovery_context(config_path)?; + let validation = validate_adopt_request(&context, request)?; + + if request.dry_run { + let state_transition = validation + .success_state_transition + .as_ref() + .map_or("none", |transition| transition.state_name.as_str()); + + println!( + "dry run: review handoff adopt validated for project={} issue={} branch={} pr={} head={} run_id={} attempt={} active_label_present={} state_transition={}", + context.config.service_id(), + validation.issue.identifier, + validation.branch_name, + landing_url(&validation.landing_state), + validation.local_head_oid, + validation.run_id, + validation.attempt_number, + validation.active_label_present, + state_transition + ); + + return Ok(()); + } + + apply_review_handoff_adopt(&context, &validation)?; + + println!( + "adopt ok: project={} issue={} branch={} pr={} head={} run_id={} attempt={}", + context.config.service_id(), + validation.issue.identifier, + validation.branch_name, + landing_url(&validation.landing_state), + validation.local_head_oid, + validation.run_id, + validation.attempt_number + ); + + Ok(()) +} + /// Run an explicit audited legacy closeout fallback. pub(crate) fn run_legacy_closeout( config_path: Option<&Path>, @@ -864,8 +936,9 @@ fn rebind_required_diagnostic( 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.", + "Inspect PR lineage and ensure label `{}` is present. Use `decodex recover review-handoff rebind {} --pr ` for a retained lane PR, or `decodex recover review-handoff adopt {} --pr ` from the managed worktree for a human-owned PR takeover.", tracker::automation_active_label(service_id), + issue_identifier, issue_identifier ) } @@ -998,6 +1071,61 @@ fn validate_rebind_request( }) } +fn validate_adopt_request( + context: &RecoveryContext, + request: &ReviewHandoffAdoptRequest, +) -> Result { + let issue = load_issue_by_identifier(&context.tracker, &request.issue)?; + let label_validation = validate_adopt_issue_context(context, &issue)?; + let landing_state = inspect_rebind_pull_request(context, &request.pr_url)?; + let existing_worktree_mapping = context.state_store.worktree_for_issue(&issue.id)?; + + validate_adopt_landing_state(&landing_state)?; + + let cwd = env::current_dir()?; + let worktree_path = validate_adopt_current_worktree( + context, + &issue, + &landing_state, + &cwd, + existing_worktree_mapping.as_ref(), + )?; + let branch_name = worktree_checkout_branch_name(&worktree_path)? + .ok_or_else(|| eyre::eyre!("Manual takeover worktree is detached."))?; + let local_head_oid = worktree_head_oid(&worktree_path)? + .ok_or_else(|| eyre::eyre!("Manual takeover worktree has no readable HEAD."))?; + + validate_adopt_absent_handoff_marker( + context, + &issue, + &branch_name, + existing_worktree_mapping.as_ref(), + )?; + + let success_state_transition = validate_adopt_issue_state(context, &issue)?; + let attempt_number = context + .state_store + .latest_run_attempt_for_issue(&issue.id)? + .map_or(1, |attempt| attempt.attempt_number().saturating_add(1)); + let run_id = manual_adopt_run_id(&issue.identifier, attempt_number, &local_head_oid); + let worktree_path_for_event = + repository_relative_path(context.config.repo_root(), &worktree_path); + + Ok(AdoptValidation { + issue, + branch_name, + worktree_path, + run_id, + attempt_number, + landing_state, + local_head_oid, + worktree_path_for_event, + active_label_present: label_validation.active_label_present, + success_state_transition, + previous_worktree_mapping: existing_worktree_mapping, + }) +} + fn validate_legacy_closeout_request( context: &RecoveryContext, request: &LegacyCloseoutRecoveryRequest, @@ -1444,6 +1572,290 @@ fn validate_rebind_tracker_labels( ) } +fn validate_adopt_issue_context( + context: &RecoveryContext, + issue: &TrackerIssue, +) -> Result { + let tracker_policy = context.workflow.frontmatter().tracker(); + + if issue.has_label(tracker_policy.opt_out_label()) { + eyre::bail!( + "Issue `{}` has opt-out label `{}`.", + issue.identifier, + tracker_policy.opt_out_label() + ); + } + + let active_label = tracker::automation_active_label(context.config.service_id()); + let active_label_present = + tracker::issue_has_label_with_server_confirmation(&context.tracker, issue, &active_label)?; + + if !active_label_present { + eyre::bail!( + "Issue `{}` is missing active automation label `{active_label}`. Restore explicit lane ownership before manual takeover adopt.", + issue.identifier + ); + } + + let needs_attention_label = tracker_policy.needs_attention_label(); + let needs_attention_present = tracker::issue_has_label_with_server_confirmation( + &context.tracker, + issue, + needs_attention_label, + )?; + + if needs_attention_present { + eyre::bail!( + "Issue `{}` has needs-attention label `{needs_attention_label}`; manual takeover adopt will not bypass a human-required stop.", + issue.identifier + ); + } + + Ok(RebindLabelValidation { active_label_present, clear_needs_attention_label: false }) +} + +fn validate_adopt_issue_state( + context: &RecoveryContext, + issue: &TrackerIssue, +) -> Result> { + validate_adopt_issue_state_for_policy(context.workflow.frontmatter().tracker(), issue) +} + +fn validate_adopt_issue_state_for_policy( + tracker_policy: &WorkflowTracker, + issue: &TrackerIssue, +) -> Result> { + let success_state = tracker_policy.success_state(); + + if issue.state.name == success_state { + return Ok(None); + } + if issue.state.name == tracker_policy.in_progress_state() { + let state_id = issue.state_id_for_name(success_state).ok_or_else(|| { + eyre::eyre!("State `{success_state}` was not found for issue `{}`.", issue.identifier) + })?; + + return Ok(Some(RebindSuccessStateTransition { + state_name: success_state.to_owned(), + state_id: state_id.to_owned(), + })); + } + + eyre::bail!( + "Issue `{}` is in `{}`, but manual takeover adopt requires `{}` or `{}`.", + issue.identifier, + issue.state.name, + tracker_policy.in_progress_state(), + success_state + ) +} + +fn validate_adopt_landing_state(landing_state: &PullRequestLandingState) -> Result<()> { + let pr_url = landing_url(landing_state); + let gate_view = landing_state.gate_view(); + + if pull_request::manual_landing_gates_satisfied(gate_view) { + return Ok(()); + } + if gate_view.state != "OPEN" { + eyre::bail!("Pull request `{pr_url}` is `{}`; adopt requires `OPEN`.", gate_view.state); + } + if gate_view.is_draft { + eyre::bail!("Pull request `{pr_url}` is still draft."); + } + if gate_view.pending_review_requests > 0 { + eyre::bail!( + "Pull request `{pr_url}` still has {} pending review request(s).", + gate_view.pending_review_requests + ); + } + if gate_view.unresolved_review_threads > 0 { + eyre::bail!( + "Pull request `{pr_url}` still has {} unresolved review thread(s).", + gate_view.unresolved_review_threads + ); + } + if gate_view.review_decision == Some("CHANGES_REQUESTED") { + eyre::bail!("Pull request `{pr_url}` still has active change requests."); + } + + if let Some(reason) = pull_request::merge_state_requires_review_repair( + gate_view.mergeable, + gate_view.merge_state_status, + ) { + eyre::bail!("Pull request `{pr_url}` requires review repair: {reason}."); + } + + if pull_request::failed_checks_require_repair( + gate_view.status_check_rollup_state, + gate_view.merge_state_status, + ) { + eyre::bail!("Pull request `{pr_url}` has failed required checks that need repair."); + } + + if let Some(other) = gate_view.status_check_rollup_state + && pull_request::checks_require_wait(Some(other)) + { + eyre::bail!( + "Pull request `{pr_url}` is still waiting on checks: statusCheckRollup=`{other}`." + ); + } + + if pull_request::mergeability_unknown(gate_view) { + eyre::bail!("Pull request `{pr_url}` mergeability is still unknown."); + } + if !pull_request::merge_state_allows_ready_to_land(gate_view.merge_state_status) { + eyre::bail!( + "Pull request `{pr_url}` is not ready to adopt: mergeStateStatus=`{}`.", + gate_view.merge_state_status + ); + } + if gate_view.mergeable != "MERGEABLE" { + eyre::bail!( + "Pull request `{pr_url}` is not mergeable: mergeable=`{}`.", + gate_view.mergeable + ); + } + + match gate_view.status_check_rollup_state { + Some("SUCCESS") | None => Ok(()), + Some(other) => eyre::bail!( + "Pull request `{pr_url}` still has non-green checks: statusCheckRollup=`{other}`." + ), + } +} + +fn validate_adopt_current_worktree( + context: &RecoveryContext, + issue: &TrackerIssue, + landing_state: &PullRequestLandingState, + cwd: &Path, + existing_worktree_mapping: Option<&WorktreeMapping>, +) -> Result { + let worktree_path = git_toplevel_path(cwd)?; + let canonical_worktree = fs::canonicalize(&worktree_path).wrap_err_with(|| { + format!("Failed to canonicalize current worktree `{}`.", worktree_path.display()) + })?; + let canonical_root = fs::canonicalize(context.config.worktree_root()).wrap_err_with(|| { + format!( + "Failed to canonicalize configured worktree root `{}`.", + context.config.worktree_root().display() + ) + })?; + + if !canonical_worktree.starts_with(&canonical_root) || canonical_worktree == canonical_root { + eyre::bail!( + "Manual takeover adopt for issue `{}` must run from a managed lane under worktree_root `{}`.", + issue.identifier, + context.config.worktree_root().display() + ); + } + + let local_branch = worktree_checkout_branch_name(&canonical_worktree)? + .ok_or_else(|| eyre::eyre!("Manual takeover worktree is detached."))?; + + if let Some(mapping) = existing_worktree_mapping { + validate_adopt_existing_worktree_mapping( + context.config.service_id(), + issue, + mapping, + &canonical_worktree, + )?; + } + + if local_branch != landing_state.head_ref_name { + eyre::bail!( + "Pull request `{}` points at branch `{}`, but current worktree branch is `{local_branch}`.", + landing_url(landing_state), + landing_state.head_ref_name + ); + } + if !worktree_is_clean(&canonical_worktree)? { + eyre::bail!( + "Manual takeover worktree `{}` has local changes; adopt requires a clean lane checkout.", + canonical_worktree.display() + ); + } + + let local_head = worktree_head_oid(&canonical_worktree)? + .ok_or_else(|| eyre::eyre!("Manual takeover worktree has no readable HEAD."))?; + + if landing_state.head_ref_oid != local_head { + eyre::bail!( + "Pull request `{}` points at head `{}`, but current worktree HEAD is `{local_head}`.", + landing_url(landing_state), + landing_state.head_ref_oid + ); + } + + Ok(canonical_worktree) +} + +fn validate_adopt_existing_worktree_mapping( + service_id: &str, + issue: &TrackerIssue, + mapping: &WorktreeMapping, + canonical_worktree: &Path, +) -> Result<()> { + if mapping.project_id() != service_id { + eyre::bail!( + "Issue `{}` already has a retained worktree mapping for project `{}`, not `{}`.", + issue.identifier, + mapping.project_id(), + service_id + ); + } + + let canonical_mapping = fs::canonicalize(mapping.worktree_path()).wrap_err_with(|| { + format!( + "Failed to canonicalize retained worktree mapping `{}` for issue `{}`.", + mapping.worktree_path().display(), + issue.identifier + ) + })?; + + if canonical_mapping != canonical_worktree { + eyre::bail!( + "Issue `{}` already has a retained worktree mapping at `{}`, but manual takeover adopt is running from `{}`.", + issue.identifier, + mapping.worktree_path().display(), + canonical_worktree.display() + ); + } + + Ok(()) +} + +fn validate_adopt_absent_handoff_marker( + context: &RecoveryContext, + issue: &TrackerIssue, + branch_name: &str, + existing_worktree_mapping: Option<&WorktreeMapping>, +) -> Result<()> { + let mut branches = vec![branch_name.to_owned()]; + + if let Some(mapping) = existing_worktree_mapping + && mapping.branch_name() != branch_name + { + branches.push(mapping.branch_name().to_owned()); + } + + for branch in branches { + if context + .state_store + .review_handoff_marker(context.config.service_id(), &issue.id, &branch)? + .is_some() + { + eyre::bail!( + "Issue `{}` already has a retained review handoff marker for branch `{branch}`; use `decodex land` or `decodex recover review-handoff rebind` instead.", + issue.identifier + ); + } + } + + Ok(()) +} + fn apply_review_handoff_rebind( context: &RecoveryContext, validation: &RebindValidation, @@ -1513,6 +1925,131 @@ fn apply_review_handoff_rebind( Ok(()) } +fn apply_review_handoff_adopt( + context: &RecoveryContext, + validation: &AdoptValidation, +) -> Result<()> { + let handoff_marker = ReviewHandoffMarker::new( + validation.run_id.clone(), + validation.attempt_number, + validation.branch_name.clone(), + landing_url(&validation.landing_state), + validation.landing_state.base_ref_name.clone(), + validation.landing_state.head_ref_name.clone(), + validation.local_head_oid.clone(), + ); + let orchestration_marker = ReviewOrchestrationMarker::new( + validation.run_id.clone(), + validation.attempt_number, + validation.branch_name.clone(), + landing_url(&validation.landing_state), + validation.local_head_oid.clone(), + REBOUND_ORCHESTRATION_PHASE, + None, + None, + None, + 0, + 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 + .upsert_worktree( + context.config.service_id(), + &validation.issue.id, + &validation.branch_name, + &worktree_path, + ) + .and_then(|()| { + context.state_store.record_run_attempt( + &validation.run_id, + &validation.issue.id, + validation.attempt_number, + "starting", + ) + }) + .and_then(|()| { + context.state_store.upsert_review_handoff_marker( + context.config.service_id(), + &validation.issue.id, + &handoff_marker, + ) + }) + .and_then(|()| { + context.state_store.upsert_review_orchestration_marker( + context.config.service_id(), + &validation.issue.id, + &orchestration_marker, + ) + }); + + if let Err(error) = local_state_write { + 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); + } + if let Err(error) = write_adopt_audit(context, validation, &event) + .and_then(|()| context.state_store.record_linear_execution_event(&event)) + { + 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); + } + if let Some(transition) = validation.success_state_transition.as_ref() { + context.tracker.update_issue_state(&validation.issue.id, &transition.state_id)?; + } + + Ok(()) +} + +fn rollback_adopt_worktree_mapping( + context: &RecoveryContext, + validation: &AdoptValidation, +) -> Result<()> { + if let Some(mapping) = validation.previous_worktree_mapping.as_ref() { + let worktree_path = mapping.worktree_path().to_string_lossy(); + + return context.state_store.upsert_worktree( + mapping.project_id(), + mapping.issue_id(), + mapping.branch_name(), + &worktree_path, + ); + } + + context.state_store.clear_worktree(&validation.issue.id) +} + +fn mark_adopt_attempt_failed(context: &RecoveryContext, validation: &AdoptValidation) { + if let Err(error) = context.state_store.update_run_status(&validation.run_id, "failed") { + tracing::warn!( + ?error, + run_id = %validation.run_id, + "Failed to mark manual takeover adopt attempt failed." + ); + } +} + fn review_handoff_rebind_event( context: &RecoveryContext, validation: &RebindValidation, @@ -1561,6 +2098,58 @@ fn review_handoff_rebind_event( event } +fn review_handoff_adopt_event( + context: &RecoveryContext, + validation: &AdoptValidation, +) -> LinearExecutionEventRecord { + let pr_url = landing_url(&validation.landing_state); + let stable_anchor = records::stable_event_anchor(&[ + pr_url, + &validation.local_head_oid, + REVIEW_HANDOFF_ADOPT_EVENT, + ]); + 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, + }, + REVIEW_HANDOFF_ADOPT_EVENT, + current_timestamp(), + &stable_anchor, + ); + + event.branch = Some(validation.branch_name.clone()); + event.worktree_path = validation.worktree_path_for_event.clone(); + event.pr_url = Some(pr_url.to_owned()); + event.pr_head_sha = Some(validation.local_head_oid.clone()); + event.pr_base_ref = Some(validation.landing_state.base_ref_name.clone()); + event.commit_sha = Some(validation.local_head_oid.clone()); + event.validation_result = Some(String::from("passed")); + event.summary = Some(format!( + "Explicit operator manual takeover adopted review handoff for {}.", + validation.issue.identifier, + )); + 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.local_head_oid), + format!("active_label_present={}", validation.active_label_present), + String::from("manual_takeover_adopt=true"), + format!( + "existing_retained_worktree_mapping={}", + validation.previous_worktree_mapping.is_some() + ), + String::from("existing_review_handoff_marker=false"), + ]); + event.next_action = Some(String::from("continue retained post-review lifecycle")); + + event +} + fn write_rebind_audit( context: &RecoveryContext, validation: &RebindValidation, @@ -1587,6 +2176,31 @@ fn write_rebind_audit( Ok(()) } +fn write_adopt_audit( + context: &RecoveryContext, + validation: &AdoptValidation, + event: &LinearExecutionEventRecord, +) -> Result<()> { + let body = format!( + "Decodex operator recovery: adopted human-owned PR `{}` for `{}` into retained review handoff state. This does not land the pull request.", + landing_url(&validation.landing_state), + validation.issue.identifier, + ); + let privacy_classifier = ConfiguredPublicProjectionPrivacyClassifier::from_config( + context.config.privacy_classifier(), + )?; + let projection = + tracker::prepare_linear_execution_event_comment(&body, event, &privacy_classifier)?; + + tracker::create_prepared_linear_execution_event_comment( + &context.tracker, + &validation.issue.id, + &projection, + )?; + + Ok(()) +} + fn legacy_closeout_event( context: &RecoveryContext, validation: &LegacyCloseoutValidation, @@ -1680,10 +2294,37 @@ fn landing_url(landing_state: &PullRequestLandingState) -> &str { &landing_state.url } +fn manual_adopt_run_id(issue_identifier: &str, attempt_number: i64, head_oid: &str) -> String { + let normalized_issue = issue_identifier + .chars() + .map(|ch| if ch.is_ascii_alphanumeric() { ch.to_ascii_lowercase() } else { '-' }) + .collect::(); + let head_prefix = head_oid.chars().take(12).collect::(); + + format!("{normalized_issue}-manual-adopt-{attempt_number}-{head_prefix}") +} + fn current_timestamp() -> String { OffsetDateTime::now_utc().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()?; + + if output.status.success() { + return Ok(PathBuf::from(trimmed_stdout(&output.stdout)?)); + } + + let stderr = String::from_utf8_lossy(&output.stderr); + + eyre::bail!( + "Failed to inspect current Git worktree root from `{}`: {}", + cwd.display(), + stderr.trim() + ) +} + fn worktree_checkout_branch_name(worktree_path: &Path) -> Result> { let output = Command::new("git") .arg("-C") @@ -1807,8 +2448,9 @@ mod tests { use crate::{ pull_request::PullRequestLandingState, recovery::{ - REVIEW_HANDOFF_BOUND_CLASSIFICATION, REVIEW_HANDOFF_OWNERSHIP_DRIFT_CLASSIFICATION, - REVIEW_HANDOFF_REBIND_EVENT, REVIEW_HANDOFF_REBIND_REQUIRED_CLASSIFICATION, + REVIEW_HANDOFF_ADOPT_EVENT, REVIEW_HANDOFF_BOUND_CLASSIFICATION, + REVIEW_HANDOFF_OWNERSHIP_DRIFT_CLASSIFICATION, REVIEW_HANDOFF_REBIND_EVENT, + REVIEW_HANDOFF_REBIND_REQUIRED_CLASSIFICATION, }, state::{ReviewHandoffMarker, ReviewOrchestrationMarker, StateStore, WorktreeMapping}, tracker::{ @@ -2100,6 +2742,162 @@ Test workflow. assert!(!error.to_string().contains("partial missing-marker")); } + #[test] + fn adopt_state_allows_in_progress_or_review_only() { + let workflow = sample_workflow(); + let in_progress = sample_issue("In Progress"); + let transition = super::validate_adopt_issue_state_for_policy( + workflow.frontmatter().tracker(), + &in_progress, + ) + .expect("in-progress issue should be adoptable") + .expect("in-progress issue should transition to review"); + + assert_eq!(transition.state_name, "In Review"); + assert_eq!(transition.state_id, "state-review"); + + let in_review = sample_issue("In Review"); + let no_transition = super::validate_adopt_issue_state_for_policy( + workflow.frontmatter().tracker(), + &in_review, + ) + .expect("in-review issue should remain adoptable"); + + assert!(no_transition.is_none()); + + let todo = sample_issue("Todo"); + let error = + super::validate_adopt_issue_state_for_policy(workflow.frontmatter().tracker(), &todo) + .expect_err("manual takeover should not bypass failure/start states"); + + assert!(error.to_string().contains("manual takeover adopt requires")); + } + + #[test] + fn adopt_landing_state_rejects_pending_checks() { + let mut landing_state = sample_landing_state( + "https://github.com/hack-ink/decodex/pull/344", + "xy/xy-944-manual-takeover-adopt", + "1123456789abcdef0123456789abcdef01234567", + ); + + landing_state.status_check_rollup_state = Some(String::from("PENDING")); + + let error = super::validate_adopt_landing_state(&landing_state) + .expect_err("manual takeover must not adopt pending checks"); + + assert!(error.to_string().contains("still waiting on checks")); + } + + #[test] + fn adopt_landing_state_rejects_closed_or_draft_prs() { + let mut closed = sample_landing_state( + "https://github.com/hack-ink/decodex/pull/344", + "xy/xy-944-manual-takeover-adopt", + "1123456789abcdef0123456789abcdef01234567", + ); + + closed.state = String::from("CLOSED"); + + let error = super::validate_adopt_landing_state(&closed) + .expect_err("manual takeover must reject closed PRs"); + + assert!(error.to_string().contains("adopt requires `OPEN`")); + + let mut draft = sample_landing_state( + "https://github.com/hack-ink/decodex/pull/344", + "xy/xy-944-manual-takeover-adopt", + "1123456789abcdef0123456789abcdef01234567", + ); + + draft.is_draft = true; + + let error = super::validate_adopt_landing_state(&draft) + .expect_err("manual takeover must reject draft PRs"); + + assert!(error.to_string().contains("is still draft")); + } + + #[test] + fn adopt_landing_state_rejects_failed_required_checks() { + let mut landing_state = sample_landing_state( + "https://github.com/hack-ink/decodex/pull/344", + "xy/xy-944-manual-takeover-adopt", + "1123456789abcdef0123456789abcdef01234567", + ); + + landing_state.status_check_rollup_state = Some(String::from("FAILURE")); + landing_state.merge_state_status = String::from("BLOCKED"); + + let error = super::validate_adopt_landing_state(&landing_state) + .expect_err("manual takeover must reject failed required checks"); + + assert!(error.to_string().contains("failed required checks")); + } + + #[test] + fn adopt_existing_worktree_mapping_accepts_same_project_and_path() { + let temp_dir = TempDir::new().expect("temp worktree should exist"); + let branch_name = "x/pubfi-pub-718"; + let issue = sample_issue("In Progress"); + let mapping = sample_worktree_at(branch_name, temp_dir.path()); + let canonical_worktree = + fs::canonicalize(temp_dir.path()).expect("temp worktree should canonicalize"); + + super::validate_adopt_existing_worktree_mapping( + "pubfi", + &issue, + &mapping, + &canonical_worktree, + ) + .expect("matching mapping should be accepted"); + } + + #[test] + fn adopt_existing_worktree_mapping_accepts_stale_branch_for_same_path() { + let retained_dir = TempDir::new().expect("retained worktree should exist"); + let issue = sample_issue("In Progress"); + let mapping = sample_worktree_at("x/pubfi-pub-718-old", retained_dir.path()); + let retained_worktree = + fs::canonicalize(retained_dir.path()).expect("retained worktree should canonicalize"); + + super::validate_adopt_existing_worktree_mapping( + "pubfi", + &issue, + &mapping, + &retained_worktree, + ) + .expect("stale mapping branch should be adopted when path matches"); + } + + #[test] + fn adopt_existing_worktree_mapping_rejects_stale_path() { + let retained_dir = TempDir::new().expect("retained worktree should exist"); + let current_dir = TempDir::new().expect("current worktree should exist"); + let issue = sample_issue("In Progress"); + let mapping = sample_worktree_at("x/pubfi-pub-718", retained_dir.path()); + let current_worktree = + fs::canonicalize(current_dir.path()).expect("current worktree should canonicalize"); + let error = super::validate_adopt_existing_worktree_mapping( + "pubfi", + &issue, + &mapping, + ¤t_worktree, + ) + .expect_err("stale mapping path must be rejected"); + + assert!(error.to_string().contains("already has a retained worktree mapping at")); + } + + #[test] + fn manual_adopt_run_id_is_stable_for_head() { + let head_oid = "0123456789abcdef0123456789abcdef01234567"; + let run_id = super::manual_adopt_run_id("XY-944", 2, head_oid); + + assert_eq!(run_id, "xy-944-manual-adopt-2-0123456789ab"); + assert_eq!(run_id, super::manual_adopt_run_id("XY-944", 2, head_oid)); + } + #[test] fn diagnostic_treats_descendant_handoff_head_as_bound() { let branch_name = "x/pubfi-pub-718"; @@ -2580,6 +3378,36 @@ Test workflow. .expect("rebind event should validate"); } + #[test] + fn review_handoff_adopt_event_validation_accepts_required_fields() { + let mut record = LinearExecutionEventRecord::new( + LinearExecutionEventIdentity { + service_id: "decodex", + issue_id: "issue-id", + issue_identifier: "XY-944", + run_id: "xy-944-manual-adopt-1", + attempt_number: 1, + }, + REVIEW_HANDOFF_ADOPT_EVENT, + super::current_timestamp(), + "anchor", + ); + + record.branch = Some(String::from("xy/xy-944-manual-takeover-adopt")); + record.worktree_path = Some(String::from(".worktrees/XY-944")); + record.pr_url = Some(String::from("https://github.com/hack-ink/decodex/pull/344")); + record.pr_head_sha = Some(String::from("0123456789abcdef0123456789abcdef01234567")); + record.pr_base_ref = Some(String::from("main")); + record.commit_sha = Some(String::from("0123456789abcdef0123456789abcdef01234567")); + record.validation_result = Some(String::from("passed")); + record.summary = + Some(String::from("Explicit operator manual takeover adopted review handoff.")); + record.evidence = Some(vec![String::from("manual_takeover_adopt=true")]); + + records::validate_linear_execution_event_record(&record) + .expect("adopt event should validate"); + } + #[test] fn review_handoff_rebind_event_requires_evidence() { let mut record = LinearExecutionEventRecord::new( diff --git a/apps/decodex/src/tracker/records.rs b/apps/decodex/src/tracker/records.rs index 72bc51b57..1dcd28ce1 100644 --- a/apps/decodex/src/tracker/records.rs +++ b/apps/decodex/src/tracker/records.rs @@ -483,6 +483,7 @@ fn event_requires_summary(event_type: &str) -> bool { | "review_handoff" | "repair_handoff" | "review_handoff_rebind" + | "review_handoff_adopt" | "landed" | "closeout" | "cleanup_complete" @@ -496,8 +497,13 @@ fn event_requires_next_action(event_type: &str) -> bool { fn event_requires_items(event_type: &str, field_name: &str) -> bool { matches!( (event_type, field_name), - ("needs_attention" | "terminal_failure" | "review_handoff_rebind", "evidence") - | ("needs_attention" | "terminal_failure", "blockers") + ( + "needs_attention" + | "terminal_failure" + | "review_handoff_rebind" + | "review_handoff_adopt", + "evidence", + ) | ("needs_attention" | "terminal_failure", "blockers") ) } @@ -611,7 +617,7 @@ fn validate_linear_execution_event_fields( require_string(record.terminal_path.as_deref(), "terminal_path") }, - "review_handoff_rebind" => { + "review_handoff_rebind" | "review_handoff_adopt" => { validate_pr_event_fields(record)?; require_string(record.validation_result.as_deref(), "validation_result")?; require_string(record.summary.as_deref(), "summary")?; diff --git a/docs/runbook/index.md b/docs/runbook/index.md index 09c79f094..43a08d725 100644 --- a/docs/runbook/index.md +++ b/docs/runbook/index.md @@ -42,10 +42,9 @@ Question this index answers: "which sequence should I execute?" release-asset recovery manifests checked in. - [`social-publishing-workflow.md`](./social-publishing-workflow.md) for turning Radar evidence into low-frequency `@decodexspace` X posts or blocked publication records. -- [`recover-review-handoff.md`](./recover-review-handoff.md) for diagnosing and - explicitly rebinding retained review lanes blocked by a missing or stale runtime DB - handoff - marker. +- [`recover-review-handoff.md`](./recover-review-handoff.md) for diagnosing retained + review lanes, explicitly rebinding missing or stale runtime DB handoff markers, and + adopting verified human-owned PRs into the normal Decodex landing lifecycle. - [`review-config-migration.md`](./review-config-migration.md) for one-time migration from historical review config keys to `[codex].review` levels. - [`release-readiness.md`](./release-readiness.md) for the v0.2.0 Loop Engineering diff --git a/docs/runbook/recover-review-handoff.md b/docs/runbook/recover-review-handoff.md index 6e682dc5a..d2021c1be 100644 --- a/docs/runbook/recover-review-handoff.md +++ b/docs/runbook/recover-review-handoff.md @@ -92,6 +92,53 @@ 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. +## Manual PR Takeover + +Use `adopt` when a human-owned PR was created from a managed Decodex worktree and the +operator wants Decodex to take over the normal issue-authority landing and tracker +closeout path. If that issue already has a worktree mapping, adopt accepts it only when +the mapping points at the current managed checkout; a mapping for a different checkout +is a fail-closed mismatch. Adopt rewrites that mapping to the current PR branch only +after every dry-run/live validation passes. Do not use adopt for lanes that already +have a review handoff marker; those belong to `rebind`, normal `decodex land`, or the +retained post-review scheduler. + +Run it from the lane worktree, not from the repo root: + +```sh +decodex recover review-handoff adopt --pr --dry-run +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 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 + when it points at the current managed checkout +- no review handoff marker already exists for the issue's current branch or previously + mapped branch +- the current checkout is a managed worktree under the configured `worktree_root` +- the current worktree is clean except top-level Decodex runtime artifacts such as + `.decodex-run-activity` and `.decodex-run-control/` +- the PR belongs to the configured GitHub repository, targets the configured default + branch, is open and non-draft, has no pending review requests or unresolved review + threads, and has green landable checks +- the PR head branch and head SHA exactly match the current worktree branch and `HEAD` + +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. + +After a successful adopt, land through the normal issue-authority path: + +```sh +decodex land --authority --pr "" +``` + ## Legacy Cleanup-Only Rows Do not use `recover review-handoff rebind` for a row that appears only under diff --git a/docs/spec/linear-execution-ledger.md b/docs/spec/linear-execution-ledger.md index 801b03667..58142c456 100644 --- a/docs/spec/linear-execution-ledger.md +++ b/docs/spec/linear-execution-ledger.md @@ -184,6 +184,7 @@ The event type set is intentionally small and low-frequency: - `pr_updated` - `review_handoff` - `review_handoff_rebind` +- `review_handoff_adopt` - `repair_handoff` - `landed` - `closeout` @@ -213,6 +214,7 @@ Every event requires the record envelope. Additional required fields are listed | `pr_updated` | `branch`, `pr_url`, `pr_head_sha`, `pr_base_ref`, `commit_sha` | `worktree_path`, `summary` | | `review_handoff` | `branch`, `worktree_path`, `pr_url`, `pr_head_sha`, `pr_base_ref`, `commit_sha`, `validation_result`, `summary`, `terminal_path` | `verification` | | `review_handoff_rebind` | `branch`, `pr_url`, `pr_head_sha`, `pr_base_ref`, `commit_sha`, `validation_result`, `summary`, `evidence` | `worktree_path`, `next_action` | +| `review_handoff_adopt` | `branch`, `pr_url`, `pr_head_sha`, `pr_base_ref`, `commit_sha`, `validation_result`, `summary`, `evidence` | `worktree_path`, `next_action` | | `repair_handoff` | `branch`, `worktree_path`, `pr_url`, `pr_head_sha`, `pr_base_ref`, `commit_sha`, `validation_result`, `summary`, `terminal_path` | `verification` | | `landed` | `branch`, `pr_url`, `pr_head_sha`, `pr_base_ref`, `commit_sha`, `summary` | `worktree_path` | | `closeout` | `pr_url`, `commit_sha`, `summary` | `branch`, `worktree_path`, `validation_result`, `target_state` | @@ -253,9 +255,13 @@ omit it and use public `error_class`, `next_action`, `blockers`, and `evidence` instead. `review_handoff_rebind` is only for an explicit operator recovery command that restores a -missing runtime DB review handoff marker after validating the retained worktree and PR -lineage. It is not a normal agent terminal signal, does not imply `issue_terminal_finalize` -ran, and must not be emitted automatically from `decodex run`. +missing or stale runtime DB review handoff marker after validating the retained worktree +and PR lineage. `review_handoff_adopt` is only for the explicit manual takeover command +that adopts a human-owned PR into the retained review handoff shape after validating the +managed clean worktree, active service ownership, exact PR head, and green landable PR +gates. Neither event is a normal agent terminal signal, neither implies +`issue_terminal_finalize` ran, and neither must be emitted automatically from +`decodex run`. ## Progress checkpoint records diff --git a/docs/spec/post-review-lifecycle.md b/docs/spec/post-review-lifecycle.md index bd6b3254d..061b26e93 100644 --- a/docs/spec/post-review-lifecycle.md +++ b/docs/spec/post-review-lifecycle.md @@ -100,6 +100,21 @@ success path. reason. A diagnostic may report a bound marker, active ownership drift, a missing marker, a pending issue-state transition, an unverified PR read, or a concrete field mismatch that requires explicit rebind. +- `adopt` is mutating and requires an explicit issue identifier plus PR URL. It is the + supported manual takeover path for a human-owned PR that was created outside a + runtime-retained lane but should now enter Decodex's normal retained review/landing + lifecycle. It must run from the current managed lane worktree under the configured + `worktree_root`, validate active automation ownership, reject opt-out and + needs-attention stops, require the issue to be in `tracker.in_progress_state` or + already in `tracker.success_state`, require a clean current checkout, require the + current branch and `HEAD` to match the PR head branch and SHA, and require the PR to + be open, non-draft, mergeable, green, free of pending review requests, and free of + unresolved review threads. It may reuse an existing worktree mapping only when that + mapping points at the current managed checkout; it must reject mappings to a + different checkout and must reject any existing review handoff marker for the current + or previously mapped branch. Adopt rewrites the mapping to the current PR branch + only after validation succeeds. Those already-bound marker lanes belong to `rebind` + or normal landing. - `rebind` is mutating and requires an explicit issue identifier plus PR URL. It must validate the configured project, tracker issue state, active automation ownership, retained worktree branch, clean worktree, PR repository, PR base, PR head branch, PR @@ -121,6 +136,14 @@ success path. existing marker for a different PR, and it must reject a current same-branch same-PR marker as a no-op unless the issue is still in `tracker.in_progress_state` and only the success-state transition remains. +- A successful adopt writes a runtime worktree mapping for the current managed checkout, + creates a local run attempt identity for the takeover, writes the same runtime DB + handoff and orchestration marker shapes as normal `issue_review_handoff` needs, + records a `review_handoff_adopt` audit event, and may move the issue from + `tracker.in_progress_state` to `tracker.success_state` after the audit succeeds. It + does not land the PR, queue follow-up work, or clear needs-attention. A subsequent + `decodex land --authority --pr ` owns merge, tracker closeout, and + cleanup through the normal issue-authority path. - 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 dbe6c9db3..cac50d612 100644 --- a/docs/spec/runtime.md +++ b/docs/spec/runtime.md @@ -621,7 +621,7 @@ or `stalled` while current marker or protocol evidence 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. 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 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. 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 diff --git a/plugins/decodex/skills/automation/SKILL.md b/plugins/decodex/skills/automation/SKILL.md index 976912f21..c4b2f6ce1 100644 --- a/plugins/decodex/skills/automation/SKILL.md +++ b/plugins/decodex/skills/automation/SKILL.md @@ -73,6 +73,12 @@ cargo run -p decodex --bin decodex -- serve - `decodex:needs-attention` is a human-required stop that automation must not silently retry. The only runtime-owned clear path is the explicit review-handoff rebind recovery where a current same-PR same-head marker proves stale failure-state drift. +- `recover review-handoff adopt` is an explicit human/operator manual takeover path, + 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. - 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 441585f55..ebab9cd0f 100644 --- a/plugins/decodex/skills/labels/SKILL.md +++ b/plugins/decodex/skills/labels/SKILL.md @@ -48,6 +48,10 @@ 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. ## Queue an Issue @@ -83,6 +87,8 @@ If a project-scoped command supplies `--config `, read that project - Do not use `decodex:active:` to mean "please start work". - Do not clear `decodex:needs-attention` just to silence a failed lane. - Do not add a service-scoped label for the wrong registered service. +- Do not add `decodex:active:` just to make `decodex land` pass; use it + only for verified retained-lane recovery or manual PR takeover adopt. - Do not ask ordinary users to apply queue labels, mention DAG/goal mechanics, or manage internal readiness state just to move from research to execution; route that through promotion, planning, and automation policy. diff --git a/plugins/decodex/skills/land/SKILL.md b/plugins/decodex/skills/land/SKILL.md index 08bd44ba6..fc8e17ca0 100644 --- a/plugins/decodex/skills/land/SKILL.md +++ b/plugins/decodex/skills/land/SKILL.md @@ -15,6 +15,8 @@ with GitHub UI, `gh pr merge`, merge queue actions, raw `git`, or direct API mut - The user explicitly asks to land a PR through Decodex. - A PR already exists and the intended merge path is `decodex land`. - The task asks whether another merge path is acceptable for a Decodex-owned landing. +- A human-owned PR should be landed with issue authority after first being adopted into + Decodex's retained review handoff state. - The lane is deliberate `--manual-authority` work with no authoritative tracker issue but still needs Decodex-owned PR landing. @@ -31,9 +33,14 @@ with GitHub UI, `gh pr merge`, merge queue actions, raw `git`, or direct API mut 1. Confirm the PR exists, the intended base and head are the ones being landed, required checks are green, and the repository expects Decodex-owned landing. 2. Run `decodex land ""`. -3. For a deliberate non-issue lane, run +3. If issue-authority land reports missing retained handoff state for a human-owned PR + created from a managed lane worktree, run + `decodex recover review-handoff adopt --pr --dry-run` from that + worktree, then rerun it live only after validation passes. Retry + `decodex land --authority --pr ""` after the adopt succeeds. +4. For a deliberate non-issue lane, run `decodex land --manual-authority --pr ""`. -4. If `decodex land` succeeds and the repo-root default branch is current, finish the +5. If `decodex land` succeeds and the repo-root default branch is current, finish the cleanup tail: remove merged linked worktrees and local/remote lane branches when no retained automation state still owns them. @@ -55,6 +62,11 @@ but intentionally skips tracker closeout and active-label ownership checks. - If `decodex land` reports that checks are still pending or expected, treat that as a wait condition: keep the tracker issue in its retained review state, keep the active ownership label in place, wait for CI, and retry `decodex land`. +- 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. - 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 a8202f123..eb3b81a1e 100644 --- a/plugins/decodex/skills/manual-cli/SKILL.md +++ b/plugins/decodex/skills/manual-cli/SKILL.md @@ -61,6 +61,14 @@ registered project's `WORKFLOW.md`. - Use `recover review-handoff diagnose` and then `recover review-handoff rebind` for retained PR handoff state drift; the live rebind owns marker refresh plus the narrow current-marker failure-state label/state repair described in the runbook. +- 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. - 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.