{"schema":"decodex/commit/1","summary":"Land Fix retained handoff reconciliation","authority":"manual"}

Author: yvette-carlisle

apps/decodex/src/agent/tracker_tool_bridge/review.rs

@@ -486,10 +486,6 @@ impl<'a> TrackerToolBridge<'a> {
 		self.persist_review_orchestration_marker(review_context, &orchestration_marker)?;
 
 		if let Err(error) = self.tracker.update_issue_state(&self.issue.id, success_state_id) {
-			if let Some(state_store) = self.state_store {
-				state_store.clear_review_markers(&self.issue.id)?;
-			}
-
 			return Err(Report::new(ReviewHandoffWritebackFailed {
 				issue_identifier: self.issue.identifier.clone(),
 				run_id: review_context.run_id.clone(),

apps/decodex/src/agent/tracker_tool_bridge/tests/review/handoff.rs

@@ -387,7 +387,7 @@ fn review_handoff_apply_does_not_duplicate_existing_ledger_event() {
 }
 
 #[test]
-fn reports_partial_review_handoff_when_state_transition_fails_after_tracker_record_write() {
+fn keeps_review_handoff_marker_when_state_transition_fails_after_tracker_record_write() {
 	let temp_dir = TempDir::new().expect("tempdir should create");
 	let tracker = FakeTracker::with_state_update_error("tracker state write failed");
 	let issue = sample_issue();
@@ -445,11 +445,13 @@ fn reports_partial_review_handoff_when_state_transition_fails_after_tracker_reco
 	assert!(error.to_string().contains("tracker state write failed"));
 	assert_eq!(tracker.comments.borrow().len(), 1);
 	assert!(tracker.state_updates.borrow().is_empty());
-	assert!(
+	assert_eq!(
 		bridge_state_store(&bridge)
 			.review_handoff_marker(TEST_SERVICE_ID, &issue.id, "x/decodex-pub-618")
 			.expect("runtime handoff marker read should succeed")
-			.is_none()
+			.expect("partial review handoff should keep the retained marker")
+			.pr_url(),
+		"https://github.com/hack-ink/decodex/pull/49"
 	);
 }
 

apps/decodex/src/manual.rs

@@ -209,7 +209,7 @@ pub(crate) fn run_land(config_path: Option<&Path>, request: &ManualLandRequest)
 	}
 	if context.prepared_closeout.is_some() && context.review_handoff.is_none() {
 		eyre::bail!(
-			"`decodex land` issue closeout requires a retained review handoff marker so it can write deterministic Linear execution ledger events. Run `decodex recovery review-handoff rebind` for `{}` before retrying.",
+			"`decodex land` issue closeout requires a retained review handoff marker so it can write deterministic Linear execution ledger events. Run `decodex recover review-handoff rebind` for `{}` before retrying.",
 			context.current_branch
 		);
 	}
@@ -534,7 +534,11 @@ fn finalize_land_closeout(
 			eyre::eyre!("`decodex land` issue cleanup requires a retained review handoff marker.")
 		})?;
 
-		clear_manual_closeout_runtime_state(state_store, &prepared_closeout.issue.id)?;
+		clear_manual_closeout_runtime_state(
+			state_store,
+			&prepared_closeout.issue.id,
+			handoff.run_id(),
+		)?;
 		clear_manual_closeout_issue_scope(
 			&prepared_closeout.tracker,
 			&prepared_closeout.issue,
@@ -1578,10 +1582,17 @@ where
 	Ok(())
 }
 
-fn clear_manual_closeout_runtime_state(state_store: &StateStore, issue_id: &str) -> Result<()> {
+fn clear_manual_closeout_runtime_state(
+	state_store: &StateStore,
+	issue_id: &str,
+	handoff_run_id: &str,
+) -> Result<()> {
 	state_store.succeed_active_run_attempts_for_issue(issue_id).wrap_err_with(|| {
 		format!("Failed to finalize active runtime attempts for issue `{issue_id}`.")
 	})?;
+
+	succeed_manual_land_handoff_attempt(state_store, issue_id, handoff_run_id)?;
+
 	state_store
 		.clear_lease(issue_id)
 		.wrap_err_with(|| format!("Failed to clear runtime lease for issue `{issue_id}`."))?;
@@ -1592,6 +1603,28 @@ fn clear_manual_closeout_runtime_state(state_store: &StateStore, issue_id: &str)
 	Ok(())
 }
 
+fn succeed_manual_land_handoff_attempt(
+	state_store: &StateStore,
+	issue_id: &str,
+	handoff_run_id: &str,
+) -> Result<()> {
+	let Some(attempt) = state_store.run_attempt(handoff_run_id)? else {
+		return Ok(());
+	};
+
+	if attempt.issue_id() != issue_id {
+		eyre::bail!(
+			"Manual land handoff run `{handoff_run_id}` belongs to issue `{}`, not `{issue_id}`.",
+			attempt.issue_id()
+		);
+	}
+	if attempt.status() != "succeeded" {
+		state_store.update_run_status(handoff_run_id, "succeeded")?;
+	}
+
+	Ok(())
+}
+
 fn linear_label_not_on_issue_error(error: &Report) -> bool {
 	error
 		.chain()
@@ -2749,7 +2782,7 @@ exit 1\n",
 		let issue = sample_issue("issue-1", "XY-225", true, &["decodex:active:pubfi"]);
 		let other_issue = sample_issue("issue-2", "XY-226", true, &["decodex:active:pubfi"]);
 		let handoff = state::ReviewHandoffMarker::new(
-			"run-1",
+			"run-1-failed",
 			1,
 			"y/decodex-xy-225",
 			"https://github.com/hack-ink/decodex/pull/67",
@@ -2783,7 +2816,7 @@ exit 1\n",
 			.record_run_attempt("run-2", &other_issue.id, 1, "running")
 			.expect("other issue running attempt should persist");
 
-		manual::clear_manual_closeout_runtime_state(&state_store, &issue.id)
+		manual::clear_manual_closeout_runtime_state(&state_store, &issue.id, handoff.run_id())
 			.expect("manual closeout runtime state should clear");
 
 		assert!(
@@ -2834,7 +2867,7 @@ exit 1\n",
 				.expect("run attempt lookup should succeed")
 				.expect("run attempt should remain")
 				.status(),
-			"failed"
+			"succeeded"
 		);
 		assert_eq!(
 			state_store