Fix review performance and quality issues

apps/decodex/src/agent/app_server.rs

@@ -3346,9 +3346,7 @@ fn normalized_home_path(path: &Path) -> PathBuf {
 fn thread_resume_error_allows_fallback(error: &Report) -> bool {
 	let message = error.to_string().to_lowercase();
 
-	message.contains("no rollout found for thread id")
-		|| message.contains("thread not found")
-		|| message.contains("failed to load rollout")
+	message.contains("no rollout found for thread id") || message.contains("thread not found")
 }
 
 fn handle_dynamic_tool_call(

apps/decodex/src/agent/app_server/tests.rs

@@ -791,6 +791,9 @@ fn app_server_turn_failure_classifies_operator_attention() {
 fn thread_resume_fallback_only_allows_missing_thread_errors() {
 	assert!(super::thread_resume_error_allows_fallback(&eyre::eyre!("thread not found")));
 	assert!(super::thread_resume_error_allows_fallback(&eyre::eyre!(
+		"no rollout found for thread id thread-1"
+	)));
+	assert!(!super::thread_resume_error_allows_fallback(&eyre::eyre!(
 		"failed to load rollout from disk"
 	)));
 	assert!(!super::thread_resume_error_allows_fallback(&eyre::eyre!(

apps/decodex/src/git_credentials.rs

@@ -1,6 +1,6 @@
 #[cfg(unix)] use std::os::unix::fs::PermissionsExt as _;
 use std::{
-	fs,
+	env, fs,
 	io::ErrorKind,
 	path::{Path, PathBuf},
 	process::{self, Command},
@@ -18,6 +18,8 @@ const GITHUB_SSH_URL_PREFIXES: &[&str] = &[
 	"ssh://git@github.com-x/",
 	"ssh://git@github.com-y/",
 ];
+const GIT_CONFIG_ENV_REMOVE_FLOOR: usize = 64;
+
 static NEXT_ASKPASS_ID: AtomicU64 = AtomicU64::new(0);
 
 #[derive(Clone, Copy)]
@@ -47,44 +49,6 @@ impl<'a> GitCredentialSource<'a> {
 	}
 }
 
-#[derive(Clone, Default, Eq, PartialEq)]
-pub(crate) enum GitSigningConfig {
-	#[default]
-	Preserve,
-	DisableInherited,
-	SigningKey(String),
-}
-impl GitSigningConfig {
-	pub(crate) fn from_local_git_config(repo_root: &Path) -> Result<Self> {
-		let output = Command::new("git")
-			.arg("-C")
-			.arg(repo_root)
-			.args(["config", "--local", "--includes", "--get", "user.signingkey"])
-			.output()?;
-
-		if output.status.success() {
-			let signing_key = String::from_utf8_lossy(&output.stdout).trim().to_owned();
-
-			return if signing_key.is_empty() {
-				Ok(Self::DisableInherited)
-			} else {
-				Ok(Self::SigningKey(signing_key))
-			};
-		}
-		if output.status.code() == Some(1) {
-			return Ok(Self::Preserve);
-		}
-
-		let stderr = String::from_utf8_lossy(&output.stderr);
-
-		eyre::bail!(
-			"Failed to inspect local Git signing key in `{}`: {}",
-			repo_root.display(),
-			stderr.trim()
-		);
-	}
-}
-
 #[derive(Clone, Default, Eq, PartialEq)]
 pub(crate) struct GitCredentialEnvironment {
 	github_token_env_var: Option<String>,
@@ -121,6 +85,8 @@ impl GitCredentialEnvironment {
 	}
 
 	pub(crate) fn apply_to(&self, command: &mut Command) {
+		clear_injected_git_config(command);
+
 		command
 			.env("GH_PROMPT_DISABLED", "1")
 			.env("GIT_TERMINAL_PROMPT", "0")
@@ -183,6 +149,7 @@ impl GitAskpassGuard {
 		Ok(Self { path })
 	}
 }
+
 impl Drop for GitAskpassGuard {
 	fn drop(&mut self) {
 		if let Err(error) = fs::remove_file(&self.path)
@@ -197,6 +164,59 @@ impl Drop for GitAskpassGuard {
 	}
 }
 
+#[derive(Clone, Default, Eq, PartialEq)]
+pub(crate) enum GitSigningConfig {
+	#[default]
+	Preserve,
+	DisableInherited,
+	SigningKey(String),
+}
+impl GitSigningConfig {
+	pub(crate) fn from_local_git_config(repo_root: &Path) -> Result<Self> {
+		let output = Command::new("git")
+			.arg("-C")
+			.arg(repo_root)
+			.args(["config", "--local", "--includes", "--get", "user.signingkey"])
+			.output()?;
+
+		if output.status.success() {
+			let signing_key = String::from_utf8_lossy(&output.stdout).trim().to_owned();
+
+			return if signing_key.is_empty() {
+				Ok(Self::DisableInherited)
+			} else {
+				Ok(Self::SigningKey(signing_key))
+			};
+		}
+		if output.status.code() == Some(1) {
+			return Ok(Self::Preserve);
+		}
+
+		let stderr = String::from_utf8_lossy(&output.stderr);
+
+		eyre::bail!(
+			"Failed to inspect local Git signing key in `{}`: {}",
+			repo_root.display(),
+			stderr.trim()
+		);
+	}
+}
+
+pub(crate) fn clear_injected_git_config(command: &mut Command) {
+	let config_count = env::var("GIT_CONFIG_COUNT")
+		.ok()
+		.and_then(|value| value.parse::<usize>().ok())
+		.unwrap_or(0);
+
+	command.env_remove("GIT_CONFIG_COUNT");
+	command.env_remove("GIT_CONFIG_PARAMETERS");
+
+	for index in 0..config_count.max(GIT_CONFIG_ENV_REMOVE_FLOOR) {
+		command.env_remove(format!("GIT_CONFIG_KEY_{index}"));
+		command.env_remove(format!("GIT_CONFIG_VALUE_{index}"));
+	}
+}
+
 pub(crate) fn scoped_github_askpass_path(root: &Path, label: &str) -> PathBuf {
 	let safe_label = sanitize_path_component(label);
 	let id = NEXT_ASKPASS_ID.fetch_add(1, Ordering::Relaxed);
@@ -240,3 +260,37 @@ fn sanitize_path_component(value: &str) -> String {
 
 	if sanitized.is_empty() { String::from("git") } else { sanitized }
 }
+
+#[cfg(test)]
+mod tests {
+	use std::{ffi::OsStr, process::Command};
+
+	use crate::git_credentials::GitCredentialEnvironment;
+
+	#[test]
+	fn apply_to_scrubs_inherited_git_config_injection() {
+		let mut command = Command::new("git");
+
+		command
+			.env("GIT_CONFIG_PARAMETERS", "commit.gpgsign=true")
+			.env("GIT_CONFIG_COUNT", "1")
+			.env("GIT_CONFIG_KEY_0", "commit.gpgsign")
+			.env("GIT_CONFIG_VALUE_0", "true");
+
+		GitCredentialEnvironment::default().apply_to(&mut command);
+
+		assert_env_removed(&command, "GIT_CONFIG_PARAMETERS");
+		assert_env_removed(&command, "GIT_CONFIG_COUNT");
+		assert_env_removed(&command, "GIT_CONFIG_KEY_0");
+		assert_env_removed(&command, "GIT_CONFIG_VALUE_0");
+	}
+
+	fn assert_env_removed(command: &Command, name: &str) {
+		let target = OsStr::new(name);
+
+		assert!(
+			command.get_envs().any(|(key, value)| key == target && value.is_none()),
+			"`{name}` should be explicitly removed from child environment"
+		);
+	}
+}

apps/decodex/src/github.rs

@@ -5,10 +5,12 @@ use std::{
 	time::{Duration, Instant},
 };
 
+use color_eyre::Report;
 use serde::Deserialize;
 use time::{OffsetDateTime, format_description::well_known::Rfc3339};
 
 use crate::{
+	git_credentials,
 	prelude::{Result, eyre},
 	pull_request::PullRequestLandingState,
 };
@@ -211,6 +213,8 @@ struct CommitViewCommit {
 }
 
 pub(crate) fn configure_gh_command(command: &mut Command, github_token: &str) {
+	git_credentials::clear_injected_git_config(command);
+
 	command
 		.env("GH_TOKEN", github_token)
 		.env("GITHUB_TOKEN", github_token)
@@ -453,7 +457,8 @@ pub(crate) fn wait_for_pull_request_merge_commit(
 		match inspect_pull_request_merge_commit(cwd, pr_url, github_token) {
 			Ok(merge_commit) => return Ok(merge_commit),
 			Err(error) if Instant::now() >= deadline => return Err(error),
-			Err(_error) => {},
+			Err(error) if merge_commit_wait_error_is_retryable(&error) => {},
+			Err(error) => return Err(error),
 		};
 
 		thread::sleep(Duration::from_secs(1));
@@ -515,7 +520,8 @@ pub(crate) fn wait_for_commit_subject(
 		match inspect_commit_subject(cwd, pr_url, commit_oid, github_token) {
 			Ok(subject) => return Ok(subject),
 			Err(error) if Instant::now() >= deadline => return Err(error),
-			Err(_error) => {},
+			Err(error) if commit_subject_wait_error_is_retryable(&error) => {},
+			Err(error) => return Err(error),
 		};
 
 		thread::sleep(Duration::from_secs(1));
@@ -753,10 +759,26 @@ fn next_pull_request_review_threads_cursor(
 		})
 }
 
+fn merge_commit_wait_error_is_retryable(error: &Report) -> bool {
+	let message = error.to_string();
+
+	message.contains("did not reach `MERGED` state after landing")
+		|| message.contains("does not expose a merge commit after merge")
+}
+
+fn commit_subject_wait_error_is_retryable(error: &Report) -> bool {
+	let message = error.to_string().to_ascii_lowercase();
+
+	message.contains("failed to inspect merge commit")
+		&& (message.contains("not found") || message.contains("http 404"))
+}
+
 #[cfg(test)]
 mod tests {
 	use std::ffi::OsStr;
 
+	use crate::prelude::eyre;
+
 	#[test]
 	fn parses_pull_request_url() {
 		let locator = super::parse_pull_request_url("https://github.com/hack-ink/decodex/pull/20")
@@ -833,6 +855,26 @@ mod tests {
 		);
 	}
 
+	#[test]
+	fn merge_commit_wait_retries_only_visibility_errors() {
+		assert!(super::merge_commit_wait_error_is_retryable(&eyre::eyre!(
+			"Pull request `https://github.com/hack-ink/decodex/pull/1` does not expose a merge commit after merge."
+		)));
+		assert!(!super::merge_commit_wait_error_is_retryable(&eyre::eyre!(
+			"Failed to inspect merge result for `https://github.com/hack-ink/decodex/pull/1`: HTTP 401"
+		)));
+	}
+
+	#[test]
+	fn commit_subject_wait_retries_only_not_found_visibility_errors() {
+		assert!(super::commit_subject_wait_error_is_retryable(&eyre::eyre!(
+			"Failed to inspect merge commit `abc` for `https://github.com/hack-ink/decodex/pull/1`: HTTP 404 Not Found"
+		)));
+		assert!(!super::commit_subject_wait_error_is_retryable(&eyre::eyre!(
+			"Failed to inspect merge commit `abc` for `https://github.com/hack-ink/decodex/pull/1`: HTTP 401 Unauthorized"
+		)));
+	}
+
 	#[test]
 	fn repository_match_rejects_foreign_pull_request_url() {
 		let repository = super::RepositoryContext {

apps/decodex/src/orchestrator/daemon.rs

@@ -791,38 +791,11 @@ where
 	T: IssueTracker,
 {
 	let now = Instant::now();
+	let Some(first_entry) = retry_queue.next_entry().cloned() else {
+		return Ok(RetryDispatchDecision::Continue);
+	};
 
-	loop {
-		let Some(first_entry) = retry_queue.next_entry().cloned() else {
-			return Ok(RetryDispatchDecision::Continue);
-		};
-
-		if now >= first_entry.ready_at {
-			break;
-		}
-
-		let Some(issue) = refresh_issue(tracker, &first_entry.issue_id)? else {
-			clear_retry_schedule_and_release(retry_queue, state_store, &first_entry.issue_id)?;
-
-			continue;
-		};
-
-			if matches!(
-				evaluate_retry_entry_retention_policy(
-					tracker,
-					&issue,
-					project,
-					workflow,
-				state_store,
-				&first_entry,
-			)?,
-			RetryEntryRetentionDecision::Drop
-		) {
-			clear_retry_schedule_and_release(retry_queue, state_store, &first_entry.issue_id)?;
-
-			continue;
-		}
-
+	if now < first_entry.ready_at {
 		tracing::debug!(
 			issue_id = first_entry.issue_id,
 			retry_kind = ?first_entry.kind,

apps/decodex/src/orchestrator/execution.rs

@@ -204,9 +204,15 @@ where
 {
 	let body = format!("Decodex execution event: {}", record.event_type);
 
-	tracker::create_linear_execution_event_comment(tracker, issue_id, &body, record)?;
+	if state_store.record_linear_execution_event(record)?
+		&& let Err(error) = tracker::create_linear_execution_event_comment_without_remote_scan(
+			tracker, issue_id, &body, record,
+		)
+	{
+		state_store.forget_linear_execution_event(&record.idempotency_key)?;
 
-	state_store.record_linear_execution_event(record)?;
+		return Err(error);
+	}
 
 	Ok(())
 }
@@ -1186,15 +1192,26 @@ where
 		},
 	);
 
-	tracker::create_linear_execution_event_comment(
-		tracker,
-		&issue_run.issue.id,
-		&comment,
-		&event,
-	)?;
-
 	if let Some(state_store) = runtime.state_store {
-		state_store.record_linear_execution_event(&event)?;
+		if state_store.record_linear_execution_event(&event)?
+			&& let Err(error) = tracker::create_linear_execution_event_comment_without_remote_scan(
+				tracker,
+				&issue_run.issue.id,
+				&comment,
+				&event,
+			)
+		{
+			state_store.forget_linear_execution_event(&event.idempotency_key)?;
+
+			return Err(error);
+		}
+	} else {
+		tracker::create_linear_execution_event_comment(
+			tracker,
+			&issue_run.issue.id,
+			&comment,
+			&event,
+		)?;
 	}
 
 	Ok(TerminalFailureOutcome {