diff --git a/apps/decodex/src/agent/tracker_tool_bridge.rs b/apps/decodex/src/agent/tracker_tool_bridge.rs index ebddbd5a..80965015 100644 --- a/apps/decodex/src/agent/tracker_tool_bridge.rs +++ b/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 { - let mut command = Command::new("gh"); + let mut command = github::gh_command(); command.args([ "pr", diff --git a/apps/decodex/src/github.rs b/apps/decodex/src/github.rs index 4e14ea73..753cd118 100644 --- a/apps/decodex/src/github.rs +++ b/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 { 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 { - 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 { 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, home: Option) -> 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 { - 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 { - 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!( diff --git a/apps/decodex/src/orchestrator/pull_request_review.rs b/apps/decodex/src/orchestrator/pull_request_review.rs index 1abfbf5b..a3eb4ca0 100644 --- a/apps/decodex/src/orchestrator/pull_request_review.rs +++ b/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 { - 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 { - 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}")]); diff --git a/apps/decodex/src/orchestrator/runtime_validation.rs b/apps/decodex/src/orchestrator/runtime_validation.rs index 22528664..e5621581 100644 --- a/apps/decodex/src/orchestrator/runtime_validation.rs +++ b/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}") })?; diff --git a/apps/decodex/src/orchestrator/status.rs b/apps/decodex/src/orchestrator/status.rs index 9cbc2445..854e73d4 100644 --- a/apps/decodex/src/orchestrator/status.rs +++ b/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") )); } } diff --git a/apps/decodex/src/orchestrator/tests/operator/status/text.rs b/apps/decodex/src/orchestrator/tests/operator/status/text.rs index 0451ad4f..d82a8d14 100644 --- a/apps/decodex/src/orchestrator/tests/operator/status/text.rs +++ b/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); diff --git a/apps/decodex/src/orchestrator/tests/operator/status_support.rs b/apps/decodex/src/orchestrator/tests/operator/status_support.rs index e9c4247c..c28e0d3b 100644 --- a/apps/decodex/src/orchestrator/tests/operator/status_support.rs +++ b/apps/decodex/src/orchestrator/tests/operator/status_support.rs @@ -411,11 +411,13 @@ fn operator_status_text_post_review_lanes() -> Vec 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, }] } diff --git a/apps/decodex/src/orchestrator/tests/review_landing/classification_checks.rs b/apps/decodex/src/orchestrator/tests/review_landing/classification_checks.rs index b2bd69b5..9a7e393e 100644 --- a/apps/decodex/src/orchestrator/tests/review_landing/classification_checks.rs +++ b/apps/decodex/src/orchestrator/tests/review_landing/classification_checks.rs @@ -390,7 +390,7 @@ fn classify_post_review_lane_blocks_checkout_branch_mismatch() { } #[test] -fn classify_post_review_lane_blocks_pull_request_state_read_failures() { +fn classify_post_review_lane_degrades_pull_request_state_read_failures_to_handoff_marker() { let temp_dir = TempDir::new().expect("temp dir should exist"); let state_store = StateStore::open_in_memory().expect("state store should open"); let issue = sample_issue("In Review", &[]); @@ -423,7 +423,7 @@ fn classify_post_review_lane_blocks_pull_request_state_read_failures() { &head_oid, )), local_branch_name: Some(String::from("x/pubfi-pub-101")), - local_head_oid: Some(head_oid), + local_head_oid: Some(head_oid.clone()), }; let classification = orchestrator::classify_post_review_lane( &snapshot, @@ -433,25 +433,44 @@ fn classify_post_review_lane_blocks_pull_request_state_read_failures() { "gh api failed" ))]), ) - .expect("classification should degrade to blocked"); + .expect("classification should preserve handoff marker readback"); - assert_eq!(classification.decision, PostReviewLaneDecision::Block); + assert_eq!(classification.decision, PostReviewLaneDecision::WaitForReview); assert_eq!(classification.reason, "pull_request_state_read_failed"); + assert_eq!( + classification.pr_url.as_deref(), + Some("https://github.com/hack-ink/decodex/pull/174") + ); + assert_eq!(classification.pr_head_sha.as_deref(), Some(head_oid.as_str())); + assert_eq!( + classification.readback_warning.as_deref(), + Some("pull_request_state_read_failed") + ); } #[test] -fn classify_post_review_lane_blocks_missing_or_blank_github_token_env_var() { +fn classify_post_review_lane_degrades_missing_or_blank_github_token_env_var() { let missing = classify_post_review_lane_with_github_token_env_var(None); - assert_eq!(missing.decision, PostReviewLaneDecision::Block); + assert_eq!(missing.decision, PostReviewLaneDecision::WaitForReview); assert_eq!(missing.reason, "pull_request_state_read_failed"); + assert_eq!(missing.pr_url.as_deref(), Some("https://github.com/hack-ink/decodex/pull/174")); + assert_eq!( + missing.readback_warning.as_deref(), + Some("pull_request_state_read_failed") + ); let env_var = format!("DECODEX_TEST_BLANK_STATUS_GITHUB_TOKEN_ENV_{}", process::id()); let _env_guard = TestEnvVarGuard::set(&env_var, ""); let blank = classify_post_review_lane_with_github_token_env_var(Some(env_var)); - assert_eq!(blank.decision, PostReviewLaneDecision::Block); + assert_eq!(blank.decision, PostReviewLaneDecision::WaitForReview); assert_eq!(blank.reason, "pull_request_state_read_failed"); + assert_eq!(blank.pr_url.as_deref(), Some("https://github.com/hack-ink/decodex/pull/174")); + assert_eq!( + blank.readback_warning.as_deref(), + Some("pull_request_state_read_failed") + ); } fn classify_post_review_lane_with_github_token_env_var( 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 71119822..5c3e1b2e 100644 --- a/apps/decodex/src/orchestrator/tests/review_landing/status_rows.rs +++ b/apps/decodex/src/orchestrator/tests/review_landing/status_rows.rs @@ -66,6 +66,64 @@ fn build_post_review_lane_statuses_reports_ready_to_land() { assert_eq!(lanes[0].pr_url.as_deref(), Some(pr_url)); } +#[test] +fn build_post_review_lane_statuses_preserves_handoff_marker_when_pr_readback_fails() { + let (_temp_dir, config, workflow) = temp_project_layout(); + let repo_root = config.repo_root().to_path_buf(); + let issue = sample_issue("In Review", &[]); + let tracker = + FakeTracker::with_refresh_snapshots(vec![issue.clone()], vec![vec![issue.clone()]]); + let state_store = StateStore::open_in_memory().expect("state store should open"); + let head_oid = String::from_utf8( + Command::new("git") + .arg("-C") + .arg(&repo_root) + .args(["rev-parse", "HEAD"]) + .output() + .expect("git rev-parse should run") + .stdout, + ) + .expect("git output should be utf-8") + .trim() + .to_owned(); + let pr_url = "https://github.com/hack-ink/decodex/pull/173"; + + state_store + .upsert_worktree("pubfi", &issue.id, "main", &repo_root.display().to_string()) + .expect("worktree should record"); + + seed_review_handoff_marker_for_path( + &state_store, + config.service_id(), + &repo_root, + &sample_review_handoff_marker("main", pr_url, &head_oid), + ); + + let lanes = orchestrator::build_post_review_lane_statuses( + &tracker, + &config, + &workflow, + &state_store, + &FakePullRequestReviewStateInspector::new(vec![Err(color_eyre::eyre::eyre!( + "gh api failed" + ))]), + ) + .expect("post-review lane status build should succeed"); + + assert_eq!(lanes.len(), 1); + assert_eq!(lanes[0].issue_identifier, issue.identifier); + assert_eq!(lanes[0].classification, "wait_for_review"); + assert_eq!(lanes[0].reason, "pull_request_state_read_failed"); + assert_eq!(lanes[0].branch_name, "main"); + assert_eq!(lanes[0].pr_url.as_deref(), Some(pr_url)); + assert_eq!(lanes[0].pr_head_sha.as_deref(), Some(head_oid.as_str())); + assert_eq!( + lanes[0].readback_warning.as_deref(), + Some("pull_request_state_read_failed") + ); + assert_eq!(lanes[0].pr_state, None); +} + #[test] fn build_post_review_lane_statuses_skips_external_review_when_disabled() { let (_temp_dir, config, workflow) = temp_project_layout(); diff --git a/apps/decodex/src/orchestrator/types.rs b/apps/decodex/src/orchestrator/types.rs index 8cc3bc21..de193cfa 100644 --- a/apps/decodex/src/orchestrator/types.rs +++ b/apps/decodex/src/orchestrator/types.rs @@ -883,11 +883,13 @@ struct OperatorPostReviewLaneStatus { classification: String, reason: String, pr_url: Option, + pr_head_sha: Option, pr_state: Option, review_decision: Option, mergeable: Option, check_state: Option, unresolved_review_threads: Option, + readback_warning: Option, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -926,11 +928,13 @@ struct PostReviewLaneClassification { decision: PostReviewLaneDecision, reason: String, pr_url: Option, + pr_head_sha: Option, pr_state: Option, review_decision: Option, mergeable: Option, check_state: Option, unresolved_review_threads: Option, + readback_warning: Option, } struct RetainedReviewLaneBlocked { diff --git a/docs/reference/github-operations.md b/docs/reference/github-operations.md index 4fe6682c..f7d2ba33 100644 --- a/docs/reference/github-operations.md +++ b/docs/reference/github-operations.md @@ -27,6 +27,13 @@ criteria for future simplification. | Remote lane branch cleanup | `gh api --method DELETE repos///git/refs/heads/` in `apps/decodex/src/github.rs` | Replaced custom git plumbing | Decodex only needs idempotent GitHub ref deletion. `gh` removes the prior `git ls-remote` plus `git push --delete` path and the extra askpass helper just for cleanup. | | Default branch sync and local branch/worktree cleanup | Git commands in `apps/decodex/src/default_branch_sync.rs` and `apps/decodex/src/orchestrator/git_ops.rs` | Keep local Git | These steps mutate or inspect the local repository/worktree state. `gh` does not replace the required local checkout synchronization and linked-worktree cleanup. | +Decodex resolves the `gh` executable through the runtime helper before these +operations. The helper checks `PATH`, then common local install locations such as +`$HOME/.local/bin`, `$HOME/.cargo/bin`, `/run/current-system/sw/bin`, +`/opt/homebrew/bin`, and `/usr/local/bin` so a long-running GUI-started control plane +uses the same GitHub CLI binary an operator can run from a shell when validating PR +handoff state. + ## Replacement Criteria Prefer `gh` for GitHub operations when all are true: diff --git a/docs/reference/operator-control-plane.md b/docs/reference/operator-control-plane.md index b42bc3e6..8dd0d272 100644 --- a/docs/reference/operator-control-plane.md +++ b/docs/reference/operator-control-plane.md @@ -234,6 +234,11 @@ Worktree visibility follows the owning dashboard section: marker, and `decodex recover review-handoff diagnose ` reports the stored marker head, orchestration head, PR head, and mismatched field before any explicit rebind refresh. +- `pull_request_state_read_failed` in `Review & Landing` is a degraded PR readback + warning when the retained review handoff marker still exists. `decodex status` + must keep the issue identifier, branch, marker PR URL, and marker PR head SHA visible + so operators can retry status, inspect the PR directly, or run the explicit recovery + path without losing the bound PR identity. - `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/spec/post-review-lifecycle.md b/docs/spec/post-review-lifecycle.md index 8660018a..ab622dbc 100644 --- a/docs/spec/post-review-lifecycle.md +++ b/docs/spec/post-review-lifecycle.md @@ -71,6 +71,13 @@ If these signals disagree and the disagreement cannot be resolved without guessi not infer a PR lineage from branch names, current heads, PR titles, or Linear comments, and `decodex run` must not repair this state automatically. +When the retained review handoff marker exists but a direct PR-state read fails, +operator status must degrade the readback instead of replacing the bound lane with a +null-PR blocked state. The status row must keep the issue identifier, retained branch, +marker PR URL, and marker head SHA, and it may expose +`readback_warning = "pull_request_state_read_failed"` until the next successful PR +state refresh. + The supported operator recovery surface is `decodex recover review-handoff`. This is a 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 diff --git a/docs/spec/runtime.md b/docs/spec/runtime.md index e1f7c449..66055040 100644 --- a/docs/spec/runtime.md +++ b/docs/spec/runtime.md @@ -188,6 +188,11 @@ After each `app-server` turn completes, `decodex` must resolve one continuation - when a retained post-review lane is about to re-enter review repair - when a retained closeout lane is about to validate merged PR state or delete the retained remote branch ref +- GitHub CLI discovery for those boundaries must use the same resolved `gh` command + path as PR inspection, including normal `PATH` lookup and the runtime's known local + install fallbacks. A valid PR that `gh pr view` can inspect with the routed project + token must not fail review handoff solely because the long-running Decodex process + started with a narrower shell path. ## Linear writeback model diff --git a/docs/spec/tracker-tools.md b/docs/spec/tracker-tools.md index c111ff61..69a4fd29 100644 --- a/docs/spec/tracker-tools.md +++ b/docs/spec/tracker-tools.md @@ -170,6 +170,8 @@ In either invalid case, `decodex` must fail the attempt rather than infer which - `decodex` must preflight the local GitHub CLI dependency at the PR-backed review boundary itself: - when a normal lane is about to validate and write back `issue_review_handoff` - when a retained post-review lane is about to re-enter `review_repair` + - using the same resolved `gh` executable path that PR inspection will use, not a + narrower lookup path for preflight than for `issue_review_handoff` - Decodex execution comment bodies should be rendered by Decodex from structured, validated fields. All tool calls must be journaled by `decodex` for recovery and audit.