XY-568: Harden Decodex post-review PR-state readback when handoff marker already has a valid PR

apps/decodex/src/agent/tracker_tool_bridge.rs

@@ -604,7 +604,7 @@ impl PullRequestInspector for GhPullRequestInspector {
 		pr_url: &str,
 		github_token: &str,
 	) -> std::result::Result<PullRequestDetails, String> {
-		let mut command = Command::new("gh");
+		let mut command = github::gh_command();
 
 		command.args([
 			"pr",

apps/decodex/src/github.rs

@@ -1,5 +1,7 @@
 use std::{
-	path::Path,
+	env,
+	ffi::OsString,
+	path::{Path, PathBuf},
 	process::{Command, Output},
 	thread,
 	time::{Duration, Instant},
@@ -54,6 +56,9 @@ query($owner: String!, $name: String!, $number: Int!, $reviewThreadsAfter: Strin
   }
 }
 "#;
+const GH_BINARY: &str = "gh";
+const GH_FALLBACK_PATHS: &[&str] =
+	&["/run/current-system/sw/bin/gh", "/opt/homebrew/bin/gh", "/usr/local/bin/gh", "/usr/bin/gh"];
 
 #[derive(Debug)]
 pub(crate) struct PullRequestLocator {
@@ -223,6 +228,14 @@ pub(crate) fn configure_gh_command(command: &mut Command, github_token: &str) {
 		.env("GCM_INTERACTIVE", "never");
 }
 
+pub(crate) fn gh_command() -> Command {
+	Command::new(gh_command_program())
+}
+
+pub(crate) fn gh_command_program() -> PathBuf {
+	gh_command_program_from_env(env::var_os("PATH"), env::var_os("HOME"))
+}
+
 pub(crate) fn parse_pull_request_url(pr_url: &str) -> Result<PullRequestLocator> {
 	let normalized = pr_url.trim().trim_end_matches('/');
 	let suffix = normalized.strip_prefix("https://github.com/").ok_or_else(|| {
@@ -269,7 +282,7 @@ pub(crate) fn post_pull_request_issue_comment(
 	let locator = parse_pull_request_url(pr_url)?;
 	let endpoint =
 		format!("repos/{}/{}/issues/{}/comments", locator.owner, locator.repo, locator.number);
-	let mut command = Command::new("gh");
+	let mut command = gh_command();
 
 	command.args(["api", endpoint.as_str(), "-f", &format!("body={body}")]);
 	command.current_dir(cwd);
@@ -343,7 +356,7 @@ pub(crate) fn inspect_repository_context(
 	cwd: &Path,
 	github_token: &str,
 ) -> Result<RepositoryContext> {
-	let mut command = Command::new("gh");
+	let mut command = gh_command();
 
 	command.args(["repo", "view", "--json", "name,owner,defaultBranchRef,mergeCommitAllowed"]);
 	command.current_dir(cwd);
@@ -402,7 +415,7 @@ pub(crate) fn admin_merge_pull_request(
 	merge_subject: Option<&str>,
 	github_token: &str,
 ) -> Result<()> {
-	let mut command = Command::new("gh");
+	let mut command = gh_command();
 
 	configure_admin_merge_command(&mut command, pr_url, reviewed_head_sha, merge_subject);
 
@@ -472,7 +485,7 @@ pub(crate) fn inspect_commit_subject(
 	github_token: &str,
 ) -> Result<String> {
 	let locator = parse_pull_request_url(pr_url)?;
-	let mut command = Command::new("gh");
+	let mut command = gh_command();
 
 	command
 		.args(["api", &format!("repos/{}/{}/commits/{}", locator.owner, locator.repo, commit_oid)]);
@@ -539,6 +552,41 @@ pub(crate) fn pull_request_is_merged_at_head(
 	Ok(response.state == "MERGED" && response.head_ref_oid.as_deref() == Some(expected_head_sha))
 }
 
+fn gh_command_program_from_env(path_env: Option<OsString>, home: Option<OsString>) -> PathBuf {
+	if let Some(path_env) = path_env {
+		for path_entry in env::split_paths(&path_env) {
+			let candidate = path_entry.join(GH_BINARY);
+
+			if candidate.is_file() {
+				return candidate;
+			}
+		}
+	}
+	if let Some(home) = home {
+		let home = PathBuf::from(home);
+
+		for relative_candidate in [[".local", "bin", GH_BINARY], [".cargo", "bin", GH_BINARY]] {
+			let candidate = relative_candidate
+				.iter()
+				.fold(home.clone(), |path, component| path.join(*component));
+
+			if candidate.is_file() {
+				return candidate;
+			}
+		}
+	}
+
+	for candidate in GH_FALLBACK_PATHS {
+		let candidate = PathBuf::from(candidate);
+
+		if candidate.is_file() {
+			return candidate;
+		}
+	}
+
+	PathBuf::from(GH_BINARY)
+}
+
 fn delete_repository_branch_if_present(
 	cwd: &Path,
 	owner: &str,
@@ -552,7 +600,7 @@ fn delete_repository_branch_if_present(
 
 	let endpoint =
 		format!("repos/{owner}/{repo}/git/refs/heads/{}", github_api_ref_path(branch_name));
-	let mut command = Command::new("gh");
+	let mut command = gh_command();
 
 	command.args(["api", "--method", "DELETE", "--silent", endpoint.as_str()]);
 	command.current_dir(cwd);
@@ -579,7 +627,7 @@ fn inspect_pull_request_merge_response(
 	pr_url: &str,
 	github_token: &str,
 ) -> Result<PullRequestMergeViewResponse> {
-	let mut command = Command::new("gh");
+	let mut command = gh_command();
 
 	command.args(["pr", "view", pr_url, "--json", "state,headRefOid,mergeCommit"]);
 	command.current_dir(cwd);
@@ -650,7 +698,7 @@ fn query_pull_request_landing_state_page(
 	pr_url: &str,
 	github_token: &str,
 ) -> Result<PullRequestLandingStateNode> {
-	let mut command = Command::new("gh");
+	let mut command = gh_command();
 
 	command.args(["api", "graphql", "-f", &format!("query={PULL_REQUEST_LANDING_STATE_QUERY}")]);
 	command.args(["-F", &format!("owner={owner}")]);
@@ -775,7 +823,10 @@ fn commit_subject_wait_error_is_retryable(error: &Report) -> bool {
 
 #[cfg(test)]
 mod tests {
-	use std::ffi::OsStr;
+	use std::{
+		ffi::{OsStr, OsString},
+		fs,
+	};
 
 	use crate::prelude::eyre;
 
@@ -855,6 +906,43 @@ mod tests {
 		);
 	}
 
+	#[test]
+	fn gh_command_program_prefers_path_candidate() {
+		let temp_dir = tempfile::TempDir::new().expect("temp dir should exist");
+		let gh_path = temp_dir.path().join("gh");
+
+		fs::write(&gh_path, "").expect("fake gh should write");
+
+		let resolved = super::gh_command_program_from_env(
+			Some(OsString::from(temp_dir.path().as_os_str())),
+			None,
+		);
+
+		assert_eq!(resolved, gh_path);
+	}
+
+	#[test]
+	fn gh_command_program_falls_back_to_home_local_bin() {
+		let temp_dir = tempfile::TempDir::new().expect("temp dir should exist");
+		let bin_dir = temp_dir.path().join(".local/bin");
+		let gh_path = bin_dir.join("gh");
+
+		fs::create_dir_all(&bin_dir).expect("fake home bin should exist");
+		fs::write(&gh_path, "").expect("fake gh should write");
+
+		let resolved = super::gh_command_program_from_env(
+			Some(OsString::new()),
+			Some(OsString::from(temp_dir.path().as_os_str())),
+		);
+
+		assert_eq!(resolved, gh_path);
+	}
+
+	#[test]
+	fn gh_command_program_knows_nix_profile_fallback() {
+		assert!(super::GH_FALLBACK_PATHS.contains(&"/run/current-system/sw/bin/gh"));
+	}
+
 	#[test]
 	fn merge_commit_wait_retries_only_visibility_errors() {
 		assert!(super::merge_commit_wait_error_is_retryable(&eyre::eyre!(

apps/decodex/src/orchestrator/pull_request_review.rs

@@ -7,7 +7,7 @@ fn query_pull_request_review_state_page(
 	pr_url: &str,
 	github_token: &str,
 ) -> Result<PullRequestReviewStateRepository> {
-	let mut command = Command::new("gh");
+	let mut command = github::gh_command();
 
 	command.args(["api", "graphql", "-f", &format!("query={PULL_REQUEST_REVIEW_STATE_QUERY}")]);
 	command.args(["-F", &format!("owner={owner}")]);
@@ -51,7 +51,7 @@ fn query_pull_request_issue_comments_page(
 	pr_url: &str,
 	github_token: &str,
 ) -> Result<PullRequestIssueCommentsNode> {
-	let mut command = Command::new("gh");
+	let mut command = github::gh_command();
 
 	command.args(["api", "graphql", "-f", &format!("query={PULL_REQUEST_ISSUE_COMMENTS_QUERY}")]);
 	command.args(["-F", &format!("owner={owner}")]);

apps/decodex/src/orchestrator/runtime_validation.rs

@@ -45,7 +45,12 @@ fn validate_daemon_runtime() -> Result<()> {
 }
 
 fn validate_command_available(command: &str, purpose: &str) -> Result<()> {
-	let output = Command::new(command).arg("--version").output().map_err(|error| {
+	let mut command_runner = if command == "gh" {
+		github::gh_command()
+	} else {
+		Command::new(command)
+	};
+	let output = command_runner.arg("--version").output().map_err(|error| {
 		eyre::eyre!("Required command `{command}` is unavailable for {purpose}: {error}")
 	})?;
 

apps/decodex/src/orchestrator/status.rs

@@ -2092,11 +2092,13 @@ fn post_review_lane_status_from_classification(
 		classification: classification.decision.as_str().to_owned(),
 		reason: classification.reason,
 		pr_url: classification.pr_url,
+		pr_head_sha: classification.pr_head_sha,
 		pr_state: classification.pr_state,
 		review_decision: classification.review_decision,
 		mergeable: classification.mergeable,
 		check_state: classification.check_state,
 		unresolved_review_threads: classification.unresolved_review_threads,
+		readback_warning: classification.readback_warning,
 	}
 }
 
@@ -2628,9 +2630,12 @@ where
 		{
 			Ok(review_state) => review_state,
 			Err(_error) => {
-				return Ok(PostReviewLaneStateLoad::Classification(blocked_post_review_lane(
-					"pull_request_state_read_failed",
-				)));
+				return Ok(PostReviewLaneStateLoad::Classification(
+					readback_degraded_post_review_lane_from_handoff(
+						review_handoff,
+						"pull_request_state_read_failed",
+					),
+				));
 			},
 		};
 
@@ -2971,11 +2976,13 @@ fn initial_post_review_lane_classification(
 		decision: PostReviewLaneDecision::WaitForReview,
 		reason: String::from("waiting_for_review_or_checks"),
 		pr_url: Some(review_state.url.clone()),
+		pr_head_sha: Some(review_state.head_ref_oid.clone()),
 		pr_state: Some(review_state.state.clone()),
 		review_decision: review_state.review_decision.clone(),
 		mergeable: Some(review_state.mergeable.clone()),
 		check_state: review_state.status_check_rollup_state.clone(),
 		unresolved_review_threads: Some(review_state.unresolved_review_threads),
+		readback_warning: None,
 	}
 }
 
@@ -2996,11 +3003,13 @@ fn blocked_post_review_lane(reason: &str) -> PostReviewLaneClassification {
 		decision: PostReviewLaneDecision::Block,
 		reason: reason.to_owned(),
 		pr_url: None,
+		pr_head_sha: None,
 		pr_state: None,
 		review_decision: None,
 		mergeable: None,
 		check_state: None,
 		unresolved_review_threads: None,
+		readback_warning: None,
 	}
 }
 
@@ -3011,10 +3020,29 @@ fn blocked_post_review_lane_from_handoff(
 	let mut classification = blocked_post_review_lane(reason);
 
 	classification.pr_url = Some(review_handoff.pr_url().to_owned());
+	classification.pr_head_sha = Some(review_handoff.pr_head_oid().to_owned());
 
 	classification
 }
 
+fn readback_degraded_post_review_lane_from_handoff(
+	review_handoff: &ReviewHandoffMarker,
+	reason: &str,
+) -> PostReviewLaneClassification {
+	PostReviewLaneClassification {
+		decision: PostReviewLaneDecision::WaitForReview,
+		reason: reason.to_owned(),
+		pr_url: Some(review_handoff.pr_url().to_owned()),
+		pr_head_sha: Some(review_handoff.pr_head_oid().to_owned()),
+		pr_state: None,
+		review_decision: None,
+		mergeable: None,
+		check_state: None,
+		unresolved_review_threads: None,
+		readback_warning: Some(reason.to_owned()),
+	}
+}
+
 fn blocked_post_review_lane_status(
 	project: &ServiceConfig,
 	issue: &TrackerIssue,
@@ -3030,11 +3058,13 @@ fn blocked_post_review_lane_status(
 		classification: String::from("blocked"),
 		reason: String::from(reason),
 		pr_url: None,
+		pr_head_sha: None,
 		pr_state: None,
 		review_decision: None,
 		mergeable: None,
 		check_state: None,
 		unresolved_review_threads: None,
+		readback_warning: None,
 	}
 }
 
@@ -4183,7 +4213,7 @@ fn render_operator_status(snapshot: &OperatorStatusSnapshot) -> String {
 	} else {
 		for lane in &snapshot.post_review_lanes {
 			output.push_str(&format!(
-				"- issue_id: {}\n  issue: {}\n  state: {}\n  classification: {}\n  reason: {}\n  branch: {}\n  worktree_path: {}\n  pr_url: {}\n  pr_state: {}\n  review_decision: {}\n  mergeable: {}\n  check_state: {}\n  unresolved_review_threads: {}\n",
+				"- issue_id: {}\n  issue: {}\n  state: {}\n  classification: {}\n  reason: {}\n  branch: {}\n  worktree_path: {}\n  pr_url: {}\n  pr_head_sha: {}\n  pr_state: {}\n  review_decision: {}\n  mergeable: {}\n  check_state: {}\n  unresolved_review_threads: {}\n  readback_warning: {}\n",
 				lane.issue_id,
 				lane.issue_identifier,
 				lane.issue_state,
@@ -4192,13 +4222,15 @@ fn render_operator_status(snapshot: &OperatorStatusSnapshot) -> String {
 				lane.branch_name,
 				lane.worktree_path,
 				lane.pr_url.as_deref().unwrap_or("none"),
+				lane.pr_head_sha.as_deref().unwrap_or("none"),
 				lane.pr_state.as_deref().unwrap_or("none"),
 				lane.review_decision.as_deref().unwrap_or("none"),
 				lane.mergeable.as_deref().unwrap_or("none"),
 				lane.check_state.as_deref().unwrap_or("none"),
 				lane
 					.unresolved_review_threads
-					.map_or_else(|| String::from("none"), |value| value.to_string())
+					.map_or_else(|| String::from("none"), |value| value.to_string()),
+				lane.readback_warning.as_deref().unwrap_or("none")
 			));
 		}
 	}

apps/decodex/src/orchestrator/tests/operator/status/text.rs

@@ -205,11 +205,13 @@ fn operator_status_text_surfaces_cleanup_blocker_pr_url() {
 			classification: String::from("cleanup_blocked"),
 			reason: String::from("retry_budget_exhausted"),
 			pr_url: Some(String::from(pr_url)),
+			pr_head_sha: Some(String::from("08a20f7dfb9526e7421a5f095b1c6adec84e52d6")),
 			pr_state: Some(String::from("MERGED")),
 			review_decision: Some(String::from("APPROVED")),
 			mergeable: Some(String::from("MERGEABLE")),
 			check_state: Some(String::from("SUCCESS")),
 			unresolved_review_threads: Some(0),
+			readback_warning: None,
 		}],
 	};
 	let rendered = orchestrator::render_operator_status(&snapshot);

apps/decodex/src/orchestrator/tests/operator/status_support.rs

@@ -411,11 +411,13 @@ fn operator_status_text_post_review_lanes() -> Vec<OperatorPostReviewLaneStatus>
 		classification: String::from("ready_to_land"),
 		reason: String::from("checks_green"),
 		pr_url: Some(String::from("https://github.com/hack-ink/decodex/pull/103")),
+		pr_head_sha: Some(String::from("08a20f7dfb9526e7421a5f095b1c6adec84e52d6")),
 		pr_state: Some(String::from("OPEN")),
 		review_decision: Some(String::from("APPROVED")),
 		mergeable: Some(String::from("MERGEABLE")),
 		check_state: Some(String::from("SUCCESS")),
 		unresolved_review_threads: Some(0),
+		readback_warning: None,
 	}]
 }