diff --git a/apps/decodex/src/agent/tracker_tool_bridge.rs b/apps/decodex/src/agent/tracker_tool_bridge.rs index e8020d7f4..a9254d460 100644 --- a/apps/decodex/src/agent/tracker_tool_bridge.rs +++ b/apps/decodex/src/agent/tracker_tool_bridge.rs @@ -19,7 +19,7 @@ use time::{OffsetDateTime, format_description::well_known::Rfc3339}; #[cfg(test)] use crate::tracker::privacy_classifier::DISABLED_PUBLIC_PROJECTION_PRIVACY_CLASSIFIER; use crate::{ - config::InternalReviewMode, + config::ReviewLevel, github, prelude::eyre, state::StateStore, @@ -491,13 +491,13 @@ pub(crate) struct ReviewHandoffContext { pub(crate) cwd: PathBuf, pub(crate) github_token_env_var: Option, pub(crate) github_command_path: Option, - pub(crate) internal_review_mode: InternalReviewMode, + pub(crate) review_level: ReviewLevel, pub(crate) mode: ReviewExecutionMode, pub(crate) recorded_pr_url: Option, } impl ReviewHandoffContext { - pub(crate) fn internal_review_checkpoint_enabled(&self) -> bool { - self.internal_review_mode.requires_review_checkpoint() + pub(crate) fn decodex_review_checkpoint_enabled(&self) -> bool { + self.review_level.requires_review_checkpoint() } } diff --git a/apps/decodex/src/agent/tracker_tool_bridge/review.rs b/apps/decodex/src/agent/tracker_tool_bridge/review.rs index 66d01a0ef..1b9077e98 100644 --- a/apps/decodex/src/agent/tracker_tool_bridge/review.rs +++ b/apps/decodex/src/agent/tracker_tool_bridge/review.rs @@ -1014,7 +1014,7 @@ impl<'a> TrackerToolBridge<'a> { &self, review_context: &ReviewHandoffContext, ) -> std::result::Result<(), String> { - if !review_context.internal_review_checkpoint_enabled() { + if !review_context.decodex_review_checkpoint_enabled() { return Ok(()); } @@ -1046,7 +1046,7 @@ impl<'a> TrackerToolBridge<'a> { &self, review_context: &ReviewHandoffContext, ) -> crate::prelude::Result> { - if !review_context.internal_review_checkpoint_enabled() { + if !review_context.decodex_review_checkpoint_enabled() { return Ok(None); } diff --git a/apps/decodex/src/agent/tracker_tool_bridge/tests.rs b/apps/decodex/src/agent/tracker_tool_bridge/tests.rs index 268bc8c5d..c611c3904 100644 --- a/apps/decodex/src/agent/tracker_tool_bridge/tests.rs +++ b/apps/decodex/src/agent/tracker_tool_bridge/tests.rs @@ -23,7 +23,7 @@ use crate::{ ReviewPolicyStopRequested, RunCompletionDisposition, TrackerToolBridge, TurnCompletionStatus, }, - config::InternalReviewMode, + config::ReviewLevel, prelude::eyre, state::{ self, ReviewHandoffMarker, ReviewOrchestrationMarker, ReviewPolicyCheckpoint, StateStore, @@ -43,7 +43,7 @@ include!("tests/mutation/dispatch.rs"); include!("tests/mutation/continuation.rs"); include!("tests/mutation/progress.rs"); -// Review handoff, repair, closeout, and internal-review policy. +// Review handoff, repair, closeout, and Decodex Review policy. include!("tests/review/policy.rs"); include!("tests/review/handoff.rs"); @@ -447,7 +447,7 @@ fn sample_review_context() -> ReviewHandoffContext { cwd: PathBuf::from("/tmp/PUB-618"), github_token_env_var: Some(String::from("HOME")), github_command_path: None, - internal_review_mode: InternalReviewMode::Loop, + review_level: ReviewLevel::Standard, mode: ReviewExecutionMode::Handoff, recorded_pr_url: None, } @@ -475,7 +475,7 @@ fn sample_review_context_in(cwd: &Path) -> ReviewHandoffContext { cwd: cwd.to_path_buf(), github_token_env_var: Some(String::from("HOME")), github_command_path: None, - internal_review_mode: InternalReviewMode::Loop, + review_level: ReviewLevel::Standard, mode: ReviewExecutionMode::Handoff, recorded_pr_url: None, } @@ -491,7 +491,7 @@ fn sample_review_repair_context_in(cwd: &Path, pr_url: &str) -> ReviewHandoffCon cwd: cwd.to_path_buf(), github_token_env_var: Some(String::from("HOME")), github_command_path: None, - internal_review_mode: InternalReviewMode::Loop, + review_level: ReviewLevel::Standard, mode: ReviewExecutionMode::Repair, recorded_pr_url: Some(pr_url.to_owned()), } @@ -507,7 +507,7 @@ fn sample_closeout_context_in(cwd: &Path, pr_url: &str) -> ReviewHandoffContext cwd: cwd.to_path_buf(), github_token_env_var: Some(String::from("HOME")), github_command_path: None, - internal_review_mode: InternalReviewMode::Loop, + review_level: ReviewLevel::Standard, mode: ReviewExecutionMode::Closeout, recorded_pr_url: Some(pr_url.to_owned()), } diff --git a/apps/decodex/src/agent/tracker_tool_bridge/tests/mutation/dispatch.rs b/apps/decodex/src/agent/tracker_tool_bridge/tests/mutation/dispatch.rs index 9c42c42fb..ff0ec464b 100644 --- a/apps/decodex/src/agent/tracker_tool_bridge/tests/mutation/dispatch.rs +++ b/apps/decodex/src/agent/tracker_tool_bridge/tests/mutation/dispatch.rs @@ -506,12 +506,16 @@ fn review_handoff_inspection_uses_configured_github_token() { let tracker = FakeTracker::new(); let issue = sample_issue(); let workflow = sample_workflow(); + let token_env_var = "DECODEX_TEST_REVIEW_HANDOFF_GITHUB_TOKEN"; + let _env_guard = TestEnvVarGuard::set(token_env_var, "configured-review-token"); let inspector = GitHubTokenAssertingPullRequestInspector { - expected_token: env::var("HOME").expect("HOME should exist"), + expected_token: String::from("configured-review-token"), response: sample_pull_request(), }; let local_repo_inspector = FakeLocalRepoInspector::new(vec![Ok(sample_local_repo())]); - let review_context = sample_review_context_in(temp_dir.path()); + let mut review_context = sample_review_context_in(temp_dir.path()); + + review_context.github_token_env_var = Some(String::from(token_env_var)); write_clean_review_checkpoint(&review_context); diff --git a/apps/decodex/src/agent/tracker_tool_bridge/tests/review/policy.rs b/apps/decodex/src/agent/tracker_tool_bridge/tests/review/policy.rs index 69c0c061a..4301c8146 100644 --- a/apps/decodex/src/agent/tracker_tool_bridge/tests/review/policy.rs +++ b/apps/decodex/src/agent/tracker_tool_bridge/tests/review/policy.rs @@ -343,7 +343,7 @@ fn review_checkpoint_tool_surface_excludes_closeout() { } #[test] -fn review_checkpoint_tool_surface_respects_internal_review_config() { +fn review_checkpoint_tool_surface_respects_review_level() { let tracker = FakeTracker::new(); let issue = sample_issue(); let review_issue = sample_review_issue(); @@ -357,8 +357,8 @@ fn review_checkpoint_tool_surface_respects_internal_review_config() { "https://github.com/hack-ink/decodex/pull/242", ); - review_context.internal_review_mode = InternalReviewMode::Off; - repair_context.internal_review_mode = InternalReviewMode::Off; + review_context.review_level = ReviewLevel::Off; + repair_context.review_level = ReviewLevel::Off; let bridge = TrackerToolBridge::with_review_handoff_for_test( &tracker, @@ -402,12 +402,12 @@ fn review_checkpoint_tool_surface_respects_internal_review_config() { assert!(matches!( checkpoint_response.content_items.as_slice(), [DynamicToolContentItem::InputText{ text }] - if text.contains("codex.internal_review_mode = \"off\"") + if text.contains("[codex].review = \"off\"") )); } #[test] -fn prompt_only_internal_review_mode_does_not_expose_checkpoint_tool() { +fn basic_review_level_does_not_expose_checkpoint_tool() { let tracker = FakeTracker::new(); let issue = sample_issue(); let workflow = sample_workflow(); @@ -416,7 +416,7 @@ fn prompt_only_internal_review_mode_does_not_expose_checkpoint_tool() { let local_repo_inspector = FakeLocalRepoInspector::new(Vec::new()); let mut review_context = sample_review_context_in(temp_dir.path()); - review_context.internal_review_mode = InternalReviewMode::Prompt; + review_context.review_level = ReviewLevel::Basic; let bridge = TrackerToolBridge::with_review_handoff_for_test( &tracker, @@ -446,7 +446,7 @@ fn prompt_only_internal_review_mode_does_not_expose_checkpoint_tool() { assert!(matches!( checkpoint_response.content_items.as_slice(), [DynamicToolContentItem::InputText{ text }] - if text.contains("codex.internal_review_mode = \"prompt\"") + if text.contains("[codex].review = \"basic\"") )); } @@ -1198,7 +1198,7 @@ fn review_handoff_requires_a_clean_checkpoint() { } #[test] -fn review_completion_skips_clean_checkpoint_when_internal_review_disabled() { +fn review_completion_skips_clean_checkpoint_when_review_gate_disabled() { for completion_path in ["handoff", "repair"] { let temp_dir = TempDir::new().expect("tempdir should create"); let tracker = FakeTracker::new(); @@ -1210,7 +1210,7 @@ fn review_completion_skips_clean_checkpoint_when_internal_review_disabled() { let local_repo_inspector = FakeLocalRepoInspector::new(vec![Ok(sample_local_repo())]); let mut review_context = sample_review_context_in(temp_dir.path()); - review_context.internal_review_mode = InternalReviewMode::Off; + review_context.review_level = ReviewLevel::Off; let bridge = TrackerToolBridge::with_review_handoff_for_test( &tracker, @@ -1237,7 +1237,7 @@ fn review_completion_skips_clean_checkpoint_when_internal_review_disabled() { sample_review_repair_apply_inspectors(pr_url); let mut review_context = sample_review_repair_context_in(temp_dir.path(), pr_url); - review_context.internal_review_mode = InternalReviewMode::Off; + review_context.review_level = ReviewLevel::Off; let bridge = TrackerToolBridge::with_review_repair_for_test( &tracker, @@ -1262,7 +1262,7 @@ fn review_completion_skips_clean_checkpoint_when_internal_review_disabled() { } #[test] -fn disabled_internal_review_ignores_stale_review_policy_stop_state() { +fn disabled_review_gate_ignores_stale_review_policy_stop_state() { let temp_dir = TempDir::new().expect("tempdir should create"); let tracker = FakeTracker::new(); let issue = sample_issue(); @@ -1271,7 +1271,7 @@ fn disabled_internal_review_ignores_stale_review_policy_stop_state() { let local_repo_inspector = FakeLocalRepoInspector::new(Vec::new()); let mut review_context = sample_review_context_in(temp_dir.path()); - review_context.internal_review_mode = InternalReviewMode::Off; + review_context.review_level = ReviewLevel::Off; write_review_policy_checkpoint( &review_context, @@ -1290,7 +1290,7 @@ fn disabled_internal_review_ignores_stale_review_policy_stop_state() { &local_repo_inspector, ); let completion_status = DynamicToolHandler::classify_turn_completion(&bridge, "done") - .expect("disabled internal review should ignore stale review stop state"); + .expect("disabled review gate should ignore stale review stop state"); assert_eq!(completion_status, TurnCompletionStatus::Continue); } diff --git a/apps/decodex/src/agent/tracker_tool_bridge/tools.rs b/apps/decodex/src/agent/tracker_tool_bridge/tools.rs index ede9897bf..d9fed23d5 100644 --- a/apps/decodex/src/agent/tracker_tool_bridge/tools.rs +++ b/apps/decodex/src/agent/tracker_tool_bridge/tools.rs @@ -53,7 +53,7 @@ impl<'a> TrackerToolBridge<'a> { if self .review_context .as_ref() - .is_some_and(ReviewHandoffContext::internal_review_checkpoint_enabled) + .is_some_and(ReviewHandoffContext::decodex_review_checkpoint_enabled) { tool_specs.extend(self.review_checkpoint_tool_specs()); } @@ -67,7 +67,7 @@ impl<'a> TrackerToolBridge<'a> { if self .review_context .as_ref() - .is_some_and(ReviewHandoffContext::internal_review_checkpoint_enabled) + .is_some_and(ReviewHandoffContext::decodex_review_checkpoint_enabled) { tool_specs.extend(self.review_checkpoint_tool_specs()); } @@ -934,10 +934,10 @@ impl<'a> TrackerToolBridge<'a> { )); }; - if !review_context.internal_review_checkpoint_enabled() { + if !review_context.decodex_review_checkpoint_enabled() { return DynamicToolCallResponse::failure(format!( - "`issue_review_checkpoint` is disabled because `codex.internal_review_mode = \"{}\"` for this run.", - review_context.internal_review_mode.as_str() + "`issue_review_checkpoint` is disabled because `[codex].review = \"{}\"` for this run.", + review_context.review_level.as_str() )); } @@ -1141,7 +1141,7 @@ impl<'a> TrackerToolBridge<'a> { self.issue.identifier ) })?; - } else if review_context.internal_review_checkpoint_enabled() { + } else if review_context.decodex_review_checkpoint_enabled() { return Err(format!( "Runtime state store is required to clear review policy state for issue `{}` after recording `{tool_name}`.", self.issue.identifier diff --git a/apps/decodex/src/config.rs b/apps/decodex/src/config.rs index 4994206ca..a9d6bb6eb 100644 --- a/apps/decodex/src/config.rs +++ b/apps/decodex/src/config.rs @@ -185,24 +185,18 @@ impl ProjectGitHubConfig { /// Project-level Codex defaults from service configuration. #[derive(Clone, Debug, Eq, PartialEq, Deserialize)] #[serde(deny_unknown_fields)] +#[derive(Default)] pub struct ProjectCodexConfig { - #[serde(default = "default_internal_review_mode")] - internal_review_mode: InternalReviewMode, - #[serde(default = "default_external_review_enabled")] - external_review_enabled: bool, + #[serde(default = "default_review_level")] + review: ReviewLevel, #[serde(default = "default_goal_support_mode")] goal_support: ProjectCodexGoalSupportMode, accounts: Option, } impl ProjectCodexConfig { - /// Internal review behavior Decodex should request for agent runs. - pub fn internal_review_mode(&self) -> InternalReviewMode { - self.internal_review_mode - } - - /// Whether Decodex should drive the retained external `@codex review` loop. - pub fn external_review_enabled(&self) -> bool { - self.external_review_enabled + /// Review level Decodex should apply for agent runs. + pub fn review_level(&self) -> ReviewLevel { + self.review } /// Whether app-server phase-scoped goal support is enabled for lane runs. @@ -234,17 +228,6 @@ impl ProjectCodexConfig { } } -impl Default for ProjectCodexConfig { - fn default() -> Self { - Self { - internal_review_mode: default_internal_review_mode(), - external_review_enabled: default_external_review_enabled(), - goal_support: default_goal_support_mode(), - accounts: None, - } - } -} - /// Optional local-only classifier for public Linear projection text. #[derive(Clone, Debug, Eq, PartialEq, Deserialize)] #[serde(deny_unknown_fields)] @@ -402,36 +385,49 @@ impl ServiceConfigDocument { } } -/// Internal review mode for agent runs. +/// Review level for agent runs. #[derive(Clone, Copy, Debug, Eq, PartialEq, Deserialize)] #[serde(rename_all = "snake_case")] -pub enum InternalReviewMode { - /// Use the runtime-owned independent review checkpoint loop. - Loop, - /// Add a prompt-only self-review instruction without the checkpoint loop. - Prompt, - /// Disable internal review behavior. +pub enum ReviewLevel { + /// Disable review gates. Off, + /// Require implementation self-check only. + Basic, + /// Require self-check plus the Decodex Review checkpoint gate. + Standard, + /// Require standard review plus the GitHub Review path. + Strict, } -impl InternalReviewMode { - /// Config string for this mode. +impl ReviewLevel { + /// Config string for this level. pub const fn as_str(self) -> &'static str { match self { - Self::Loop => "loop", - Self::Prompt => "prompt", Self::Off => "off", + Self::Basic => "basic", + Self::Standard => "standard", + Self::Strict => "strict", } } - /// Whether this mode uses the structured checkpoint loop. + /// Whether this level prompts the implementation self-check. + pub const fn uses_self_check(self) -> bool { + !matches!(self, Self::Off) + } + + /// Whether this level uses the structured Decodex Review checkpoint gate. pub const fn requires_review_checkpoint(self) -> bool { - matches!(self, Self::Loop) + matches!(self, Self::Standard | Self::Strict) + } + + /// Whether this level uses the GitHub `@codex review` path. + pub const fn uses_github_review(self) -> bool { + matches!(self, Self::Strict) } } -impl Default for InternalReviewMode { +impl Default for ReviewLevel { fn default() -> Self { - default_internal_review_mode() + default_review_level() } } @@ -500,12 +496,8 @@ pub fn checkouts_share_repository(a: &Path, b: &Path) -> Result { Ok(a_common_dir.is_some() && a_common_dir == b_common_dir) } -const fn default_external_review_enabled() -> bool { - true -} - -const fn default_internal_review_mode() -> InternalReviewMode { - InternalReviewMode::Loop +const fn default_review_level() -> ReviewLevel { + ReviewLevel::Strict } const fn default_goal_support_mode() -> ProjectCodexGoalSupportMode { @@ -994,7 +986,7 @@ mod tests { use tempfile::TempDir; use crate::{ - config::{self, InternalReviewMode, ProjectCodexGoalSupportMode, ServiceConfig}, + config::{self, ProjectCodexGoalSupportMode, ReviewLevel, ServiceConfig}, worktree::WorktreeManager, }; @@ -1067,8 +1059,7 @@ mod tests { assert_eq!(config.workflow_path(), canonical_root.join("WORKFLOW.md")); assert_eq!(config.github().token_env_var(), "HOME"); assert_eq!(config.github().command_path(), Some(canonical_root.join("bin/gh").as_path())); - assert_eq!(config.codex().internal_review_mode(), InternalReviewMode::Loop); - assert!(config.codex().external_review_enabled()); + assert_eq!(config.codex().review_level(), ReviewLevel::Strict); } #[test] @@ -1221,23 +1212,13 @@ mod tests { } #[test] - fn parses_codex_review_settings() { - for (case_name, codex_body, expected_mode, expected_external_review) in [ - ( - "explicit off mode and disabled external review", - r#" - internal_review_mode = "off" - external_review_enabled = false"#, - InternalReviewMode::Off, - false, - ), - ( - "prompt mode keeps external review default enabled", - r#" - internal_review_mode = "prompt""#, - InternalReviewMode::Prompt, - true, - ), + fn parses_codex_review_levels() { + for (case_name, codex_body, expected_level) in [ + ("default strict level", "", ReviewLevel::Strict), + ("explicit off level", r#"review = "off""#, ReviewLevel::Off), + ("explicit basic level", r#"review = "basic""#, ReviewLevel::Basic), + ("explicit standard level", r#"review = "standard""#, ReviewLevel::Standard), + ("explicit strict level", r#"review = "strict""#, ReviewLevel::Strict), ] { let temp_dir = TempDir::new().expect("temp dir should exist"); let config_path = write_config_file( @@ -1259,8 +1240,7 @@ mod tests { ); let config = ServiceConfig::from_path(&config_path).expect(case_name); - assert_eq!(config.codex().internal_review_mode(), expected_mode); - assert_eq!(config.codex().external_review_enabled(), expected_external_review); + assert_eq!(config.codex().review_level(), expected_level); } } @@ -1445,7 +1425,7 @@ mod tests { } #[test] - fn rejects_unknown_codex_internal_review_mode() { + fn rejects_unknown_codex_review_level() { let temp_dir = TempDir::new().expect("temp dir should exist"); let config_path = write_config_file( temp_dir.path(), @@ -1459,11 +1439,11 @@ mod tests { token_env_var = "HOME" [codex] - internal_review_mode = "prompt_only" + review = "prompt_only" "#, ); - let error = ServiceConfig::from_path(&config_path) - .expect_err("unknown internal review mode should fail"); + let error = + ServiceConfig::from_path(&config_path).expect_err("unknown review level should fail"); assert!(error.to_string().contains("prompt_only")); } diff --git a/apps/decodex/src/orchestrator.rs b/apps/decodex/src/orchestrator.rs index 53db7a7fb..14c69ab51 100644 --- a/apps/decodex/src/orchestrator.rs +++ b/apps/decodex/src/orchestrator.rs @@ -40,7 +40,7 @@ use time::{OffsetDateTime, format_description::well_known::Rfc3339}; use crate::{agent, default_branch_sync, git_credentials, maintenance, state}; #[rustfmt::skip] -use crate::{agent::{ACTIVE_RUN_IDLE_TIMEOUT, AppServerCapabilityPreflightFailure, AppServerDynamicToolFailure, AppServerHomePreflightFailure, AppServerPhaseGoalFailure, AppServerProcessEnv, AppServerRunRequest, AppServerRunResult, AppServerTransportFailure, AppServerTurnFailure, ISSUE_DELIVERY_CLOSEOUT_COMPLETE_TOOL_NAME, ISSUE_LABEL_ADD_TOOL_NAME, ISSUE_PROGRESS_CHECKPOINT_TOOL_NAME, ISSUE_REVIEW_CHECKPOINT_TOOL_NAME, ISSUE_REVIEW_HANDOFF_TOOL_NAME, ISSUE_REVIEW_REPAIR_COMPLETE_TOOL_NAME, ISSUE_TERMINAL_FINALIZE_TOOL_NAME, ISSUE_TRANSITION_TOOL_NAME, DecodexRunContext, DecodexToolBridge, PhaseGoalController, PhaseGoalKind, PhaseGoalSpec, PhaseGoalTransition, ReviewExecutionMode, ReviewHandoffContext, ReviewHandoffWritebackFailed, ReviewPolicyStopReason, ReviewPolicyStopRequested, RunCompletionDisposition, TrackerToolBridge, TurnContinuationGuard}, config::{InternalReviewMode, ServiceConfig}, execution_program::{ExecutionProgramOperatorSummary, ExecutionProgramReadinessContext, ExecutionWorkflowPolicy}, git_credentials::GitCredentialSource, github, prelude::{Result, eyre}, state::{ChildAgentActivityBucket, ChildAgentActivitySummary, CodexAccountActivitySummary, ExecutionProgramRecord, LoopGuardrailCheckpoint, LoopGuardrailCheckpointInput, ProjectRegistration, ProjectRunStatus, ProtocolActivitySummary, RUN_OPERATION_AGENT_RUN, RUN_OPERATION_APP_SERVER_PREFLIGHT, RUN_OPERATION_GIT_CREDENTIALS, RUN_OPERATION_IDLE, RUN_OPERATION_RECONCILIATION, RUN_OPERATION_REPO_GATE, RUN_OPERATION_REVIEW_WRITEBACK, RUN_OPERATION_WAITING_EXTERNAL, ReviewHandoffMarker, ReviewOrchestrationMarker, RunActivityMarker, RunAttempt, StateStore, WorktreeMapping}, tracker::{IssueTracker, TrackerComment, TrackerIssue, linear::LinearClient, records}, workflow::{WorkflowDocument, WorkflowExecution}, worktree::{WorktreeManager, WorktreeSpec}}; +use crate::{agent::{ACTIVE_RUN_IDLE_TIMEOUT, AppServerCapabilityPreflightFailure, AppServerDynamicToolFailure, AppServerHomePreflightFailure, AppServerPhaseGoalFailure, AppServerProcessEnv, AppServerRunRequest, AppServerRunResult, AppServerTransportFailure, AppServerTurnFailure, ISSUE_DELIVERY_CLOSEOUT_COMPLETE_TOOL_NAME, ISSUE_LABEL_ADD_TOOL_NAME, ISSUE_PROGRESS_CHECKPOINT_TOOL_NAME, ISSUE_REVIEW_CHECKPOINT_TOOL_NAME, ISSUE_REVIEW_HANDOFF_TOOL_NAME, ISSUE_REVIEW_REPAIR_COMPLETE_TOOL_NAME, ISSUE_TERMINAL_FINALIZE_TOOL_NAME, ISSUE_TRANSITION_TOOL_NAME, DecodexRunContext, DecodexToolBridge, PhaseGoalController, PhaseGoalKind, PhaseGoalSpec, PhaseGoalTransition, ReviewExecutionMode, ReviewHandoffContext, ReviewHandoffWritebackFailed, ReviewPolicyStopReason, ReviewPolicyStopRequested, RunCompletionDisposition, TrackerToolBridge, TurnContinuationGuard}, config::{ReviewLevel, ServiceConfig}, execution_program::{ExecutionProgramOperatorSummary, ExecutionProgramReadinessContext, ExecutionWorkflowPolicy}, git_credentials::GitCredentialSource, github, prelude::{Result, eyre}, state::{ChildAgentActivityBucket, ChildAgentActivitySummary, CodexAccountActivitySummary, ExecutionProgramRecord, LoopGuardrailCheckpoint, LoopGuardrailCheckpointInput, ProjectRegistration, ProjectRunStatus, ProtocolActivitySummary, RUN_OPERATION_AGENT_RUN, RUN_OPERATION_APP_SERVER_PREFLIGHT, RUN_OPERATION_GIT_CREDENTIALS, RUN_OPERATION_IDLE, RUN_OPERATION_RECONCILIATION, RUN_OPERATION_REPO_GATE, RUN_OPERATION_REVIEW_WRITEBACK, RUN_OPERATION_WAITING_EXTERNAL, ReviewHandoffMarker, ReviewOrchestrationMarker, RunActivityMarker, RunAttempt, StateStore, WorktreeMapping}, tracker::{IssueTracker, TrackerComment, TrackerIssue, linear::LinearClient, records}, workflow::{WorkflowDocument, WorkflowExecution}, worktree::{WorktreeManager, WorktreeSpec}}; use harness_improvement::{ HarnessImprovementCandidateSummary, HarnessOutcomeKind, harness_improvement_candidates_from_private_events, record_harness_outcome_best_effort, diff --git a/apps/decodex/src/orchestrator/execution.rs b/apps/decodex/src/orchestrator/execution.rs index 4fc7dbfe3..c2217ede5 100644 --- a/apps/decodex/src/orchestrator/execution.rs +++ b/apps/decodex/src/orchestrator/execution.rs @@ -239,7 +239,7 @@ impl RepoGatePhaseGoalController<'_> { detail.unwrap_or("Run the registered canonicalize and verify commands before completing this phase.") ), PhaseGoalKind::RepairAcceptedReviewFindings => format!( - "Decodex phase: {}\nRepair accepted review findings for {} on the retained PR head without widening issue scope. Do not request fresh external review before Decodex validation.", + "Decodex phase: {}\nRepair accepted review findings for {} on the retained PR head without widening issue scope. Do not request GitHub Review before Decodex validation.", phase.as_str(), self.issue_run.issue.identifier ), @@ -902,7 +902,7 @@ fn build_issue_run_continuation_user_input( issue_run.dispatch_mode, review_context.recorded_pr_url.as_deref(), workflow.frontmatter().tracker().success_state(), - project.codex().internal_review_mode(), + project.codex().review_level(), ) } diff --git a/apps/decodex/src/orchestrator/prompting.rs b/apps/decodex/src/orchestrator/prompting.rs index 07469cb09..d52af966f 100644 --- a/apps/decodex/src/orchestrator/prompting.rs +++ b/apps/decodex/src/orchestrator/prompting.rs @@ -1,7 +1,7 @@ pub(crate) const TRACKER_PUBLIC_TEXT_BOUNDARY_INSTRUCTION: &str = "Tracker public text boundary\n- Linear tracker text is public/team-visible. Do not include local host paths, routed identity details, account details, credential-like names, private config paths, tokens, or secrets in issue comments, progress checkpoints, review summaries, closeout summaries, blockers, evidence, verification, failed commands, or raw errors.\n- `issue_comment` accepts only allowlisted public comment kinds. For manual attention, call it with `kind: \"manual_attention\"` and structured public fields; do not send arbitrary comment bodies.\n- Use public collaboration identifiers when needed: PR URLs, issue identifiers, branch names, commit SHAs, and repository-relative paths.\n- Decodex may apply a local-only secondary privacy classifier to rendered public projections, but that classifier is not the privacy boundary; keep private evidence out of public fields before tool calls."; -const PROMPT_ONLY_INTERNAL_REVIEW_INSTRUCTION: &str = +const SELF_CHECK_INSTRUCTION: &str = "Review your work repeatedly and fix any logic bugs until no new issues are found."; fn build_retry_recovery_context(dispatch_mode: IssueDispatchMode) -> Option { @@ -78,15 +78,14 @@ where .frontmatter() .tracker() .resolved_completed_state(); - let internal_review_mode = project.codex().internal_review_mode(); + let review_level = project.codex().review_level(); let tracker_contract = match issue_run.dispatch_mode { IssueDispatchMode::ReviewRepair => format!( - "Tracker tool contract\n- You own issue-scoped tracker writes for `{issue}` on retained PR `{pr_url}`.\n- This run resumes an existing `{success}` lane. Do not move the issue back to `{in_progress}` and do not call `{review_handoff_tool}`.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n{internal_review_guidance}- For each actionable review item on `{pr_url}`, including non-thread review summaries, validate the claim against the codebase, tests, and requirements before changing code, and keep pushback or clarification threads open until the repaired head is ready.\n- If this run was triggered by retained landing fallback, handle only the implementation-shaped blocker such as branch sync, conflict resolution, ambiguous mergeability, or repository-specific recovery. Do not merge or land the PR yourself.\n{repair_architecture_guidance}- Repair the current PR head on branch `{branch}`, run the repository validation needed to justify the repaired head, and push the repaired head.\n- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- Do not request fresh external review yourself. `decodex` will post the next runtime-owned external review request after `{review_repair_tool}` succeeds.\n- After the repaired head is pushed, reply in-thread for every addressed comment and resolve only the GitHub review threads whose fixes landed and verified on the repaired head.\n{completion_guidance}- If you determine the issue needs human attention, add label `{needs_attention}` with `{label_tool}`, then call `issue_comment` with kind `manual_attention` and structured public fields (`error_class`, `next_action`, `blockers`, `evidence`; include `failed_command` and `raw_error` only when public-safe), and then call `{terminal_finalize_tool}` with path `manual_attention`. Do not speculate about capabilities you did not directly verify.\n- Keep the tracker issue in `{success}`. `decodex` will handle the later external review request or clean-path runtime landing, closeout, and cleanup lifecycle.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}\n- Never write to any other issue.", + "Tracker tool contract\n- You own issue-scoped tracker writes for `{issue}` on retained PR `{pr_url}`.\n- This run resumes an existing `{success}` lane. Do not move the issue back to `{in_progress}` and do not call `{review_handoff_tool}`.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n{decodex_review_guidance}- For each actionable review item on `{pr_url}`, including non-thread review summaries, validate the claim against the codebase, tests, and requirements before changing code, and keep pushback or clarification threads open until the repaired head is ready.\n- If this run was triggered by retained landing fallback, handle only the implementation-shaped blocker such as branch sync, conflict resolution, ambiguous mergeability, or repository-specific recovery. Do not merge or land the PR yourself.\n{repair_architecture_guidance}- Repair the current PR head on branch `{branch}`, run the repository validation needed to justify the repaired head, and push the repaired head.\n- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n{github_review_guidance}- After the repaired head is pushed, reply in-thread for every addressed comment and resolve only the GitHub review threads whose fixes landed and verified on the repaired head.\n{completion_guidance}- If you determine the issue needs human attention, add label `{needs_attention}` with `{label_tool}`, then call `issue_comment` with kind `manual_attention` and structured public fields (`error_class`, `next_action`, `blockers`, `evidence`; include `failed_command` and `raw_error` only when public-safe), and then call `{terminal_finalize_tool}` with path `manual_attention`. Do not speculate about capabilities you did not directly verify.\n{retained_tail_guidance}- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}\n- Never write to any other issue.", issue = issue_run.issue.identifier, pr_url = recorded_pr_url.unwrap_or("(missing review handoff marker)"), progress_checkpoint_tool = ISSUE_PROGRESS_CHECKPOINT_TOOL_NAME, review_handoff_tool = ISSUE_REVIEW_HANDOFF_TOOL_NAME, - review_repair_tool = ISSUE_REVIEW_REPAIR_COMPLETE_TOOL_NAME, terminal_finalize_tool = ISSUE_TERMINAL_FINALIZE_TOOL_NAME, in_progress = workflow.frontmatter().tracker().in_progress_state(), success = workflow.frontmatter().tracker().success_state(), @@ -95,8 +94,10 @@ where label_tool = ISSUE_LABEL_ADD_TOOL_NAME, continuation_guidance = continuation_guidance, repair_architecture_guidance = repair_architecture_guidance, - internal_review_guidance = build_repair_internal_review_guidance(internal_review_mode), - completion_guidance = build_repair_completion_guidance(internal_review_mode), + decodex_review_guidance = build_repair_review_guidance(review_level), + github_review_guidance = build_repair_github_review_guidance(review_level, ISSUE_REVIEW_REPAIR_COMPLETE_TOOL_NAME), + retained_tail_guidance = build_repair_retained_tail_guidance(review_level, workflow.frontmatter().tracker().success_state()), + completion_guidance = build_repair_completion_guidance(review_level), ), IssueDispatchMode::Closeout => format!( "Tracker tool contract\n- You own issue-scoped tracker writes for `{issue}` on retained PR `{pr_url}`.\n- This run resumes a merged post-review lane for the same PR lineage. The tracker issue may still be in `{success}` or may already be in `{completed}` while deterministic closeout tail work remains. Do not move the issue back to `{in_progress}` and do not call `{review_handoff_tool}` or `{review_repair_tool}`.\n- Treat retained closeout as a short deterministic tail. Reuse the existing merged PR evidence instead of restarting broad discovery, and only rerun the minimum validation needed to justify `Done` plus cleanup.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n- If you call `{progress_checkpoint_tool}` during closeout, either omit `head_sha` and let `decodex` record the exact current lane HEAD automatically, or pass the exact full current `HEAD` SHA. Do not send an abbreviated SHA that differs from the live lane head.\n- Merge is already authoritative for `{pr_url}` before this run starts. Do not land, merge, or request review from this closeout run.\n- If the issue is still in `{success}`, transition it once to `{completed}` with `{transition_tool}` before `{closeout_tool}`. If it is already in `{completed}`, leave it there.\n- Finish the remaining Linear closeout tail work for this same merged PR lineage, then call `{closeout_tool}` with PR `{pr_url}` and a short result summary, then call `{terminal_finalize_tool}` with path `closeout`.\n- Do not end the turn without either `{closeout_tool}` plus `{terminal_finalize_tool}`, or the manual-attention path.\n- If you determine the issue needs human attention, add label `{needs_attention}` with `{label_tool}`, then call `issue_comment` with kind `manual_attention` and structured public fields (`error_class`, `next_action`, `blockers`, `evidence`; include `failed_command` and `raw_error` only when public-safe), and then call `{terminal_finalize_tool}` with path `manual_attention`. Do not speculate about capabilities you did not directly verify.\n- Keep all tracker and PR writes scoped to this retained lane. `decodex` will validate the merged PR lineage, the resolved completed state, and the later cleanup boundary.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}\n- Never write to any other issue.", @@ -116,7 +117,7 @@ where continuation_guidance = continuation_guidance, ), _ => format!( - "Tracker tool contract\n- You own issue-scoped tracker writes for `{issue}`.\n- At the start of execution, call `{transition_tool}` to move the issue to `{in_progress}`. Decodex already records the run-start Linear ledger, so do not add a separate start comment.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n{internal_review_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- When the implementation is ready, commit the lane, push branch `{branch}`, and create or update a non-draft PR titled `{pr_title}` for that branch.\n{completion_guidance}- If you determine the issue needs human attention, add label `{needs_attention}` with `{label_tool}`, then call `issue_comment` with kind `manual_attention` and structured public fields (`error_class`, `next_action`, `blockers`, `evidence`; include `failed_command` and `raw_error` only when public-safe), and then call `{terminal_finalize_tool}` with path `manual_attention`. Do not speculate about capabilities you did not directly verify. Do not call `{review_handoff_tool}` in that case; `decodex` will stop the lane as a human-required failure without automatic retry.\n- Do not move the issue directly to `{success}` with `{transition_tool}`. `decodex` will complete the success writeback only after its own validation passes.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}\n- Never write to any other issue.", + "Tracker tool contract\n- You own issue-scoped tracker writes for `{issue}`.\n- At the start of execution, call `{transition_tool}` to move the issue to `{in_progress}`. Decodex already records the run-start Linear ledger, so do not add a separate start comment.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n{decodex_review_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- When the implementation is ready, commit the lane, push branch `{branch}`, and create or update a non-draft PR titled `{pr_title}` for that branch.\n{completion_guidance}- If you determine the issue needs human attention, add label `{needs_attention}` with `{label_tool}`, then call `issue_comment` with kind `manual_attention` and structured public fields (`error_class`, `next_action`, `blockers`, `evidence`; include `failed_command` and `raw_error` only when public-safe), and then call `{terminal_finalize_tool}` with path `manual_attention`. Do not speculate about capabilities you did not directly verify. Do not call `{review_handoff_tool}` in that case; `decodex` will stop the lane as a human-required failure without automatic retry.\n- Do not move the issue directly to `{success}` with `{transition_tool}`. `decodex` will complete the success writeback only after its own validation passes.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}\n- Never write to any other issue.", issue = issue_run.issue.identifier, transition_tool = ISSUE_TRANSITION_TOOL_NAME, label_tool = ISSUE_LABEL_ADD_TOOL_NAME, @@ -129,10 +130,10 @@ where needs_attention = workflow.frontmatter().tracker().needs_attention_label(), continuation_guidance = continuation_guidance, pr_title = review_pull_request_title(&issue_run.issue), - internal_review_guidance = build_handoff_internal_review_guidance( - internal_review_mode + decodex_review_guidance = build_handoff_review_guidance( + review_level ), - completion_guidance = build_handoff_completion_guidance(internal_review_mode), + completion_guidance = build_handoff_completion_guidance(review_level), ), }; @@ -165,14 +166,14 @@ where .frontmatter() .tracker() .resolved_completed_state(); - let internal_review_mode = project.codex().internal_review_mode(); + let review_level = project.codex().review_level(); let recovery_context = build_retry_recovery_context(issue_run.dispatch_mode) .map(|section| format!("{section}\n\n")) .unwrap_or_default(); match issue_run.dispatch_mode { IssueDispatchMode::ReviewRepair => format!( - "Continue retained review repair for Linear issue {identifier}: {title}\n\nDescription:\n{description}\n\nCurrent PR:\n- `{pr_url}`\n\nExecution checklist:\n- Resume from the current branch and PR state in this worktree. Do not move the issue back to `{in_progress}`.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n{internal_review_guidance}- Read the current review feedback on `{pr_url}`, including non-thread review summaries, validate each actionable claim against the codebase, tests, and requirements, fix only the verified issues on branch `{branch}`, and keep scope limited to the outstanding retained repair.\n- If the lane is here because retained landing was not a deterministic clean path, handle only the branch sync, conflict resolution, ambiguous mergeability, or repository-specific recovery needed to make the PR clean again. Do not merge or land the PR yourself.\n- Leave pushback or clarification threads open until the repaired head is ready.\n{repair_architecture_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- Run the repository validation needed to justify the repaired head.\n- Commit the repair and push the same branch. Do not request fresh external review yourself; `decodex` will post the next runtime-owned external review request after `{review_repair_tool}` succeeds.\n- After the repaired head is pushed, reply in-thread for every addressed comment and resolve only the GitHub review threads whose fixes landed and verified on the repaired head.\n{completion_guidance}- If the issue needs manual attention, add label `{needs_attention}` with `{label_tool}`, call `issue_comment` with kind `manual_attention` and structured public fields, and then call `{terminal_finalize_tool}` with path `manual_attention`.\n- Keep the issue in `{success}` and do not treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}", + "Continue retained review repair for Linear issue {identifier}: {title}\n\nDescription:\n{description}\n\nCurrent PR:\n- `{pr_url}`\n\nExecution checklist:\n- Resume from the current branch and PR state in this worktree. Do not move the issue back to `{in_progress}`.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n{decodex_review_guidance}- Read the current review feedback on `{pr_url}`, including non-thread review summaries, validate each actionable claim against the codebase, tests, and requirements, fix only the verified issues on branch `{branch}`, and keep scope limited to the outstanding retained repair.\n- If the lane is here because retained landing was not a deterministic clean path, handle only the branch sync, conflict resolution, ambiguous mergeability, or repository-specific recovery needed to make the PR clean again. Do not merge or land the PR yourself.\n- Leave pushback or clarification threads open until the repaired head is ready.\n{repair_architecture_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- Run the repository validation needed to justify the repaired head.\n- Commit the repair and push the same branch.\n{github_review_guidance}- After the repaired head is pushed, reply in-thread for every addressed comment and resolve only the GitHub review threads whose fixes landed and verified on the repaired head.\n{completion_guidance}- If the issue needs manual attention, add label `{needs_attention}` with `{label_tool}`, call `issue_comment` with kind `manual_attention` and structured public fields, and then call `{terminal_finalize_tool}` with path `manual_attention`.\n- Keep the issue in `{success}` and do not treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}", identifier = issue.identifier, title = issue.title, description = description, @@ -180,15 +181,15 @@ where in_progress = workflow.frontmatter().tracker().in_progress_state(), branch = issue_run.worktree.branch_name, progress_checkpoint_tool = ISSUE_PROGRESS_CHECKPOINT_TOOL_NAME, - review_repair_tool = ISSUE_REVIEW_REPAIR_COMPLETE_TOOL_NAME, terminal_finalize_tool = ISSUE_TERMINAL_FINALIZE_TOOL_NAME, needs_attention = workflow.frontmatter().tracker().needs_attention_label(), label_tool = ISSUE_LABEL_ADD_TOOL_NAME, success = workflow.frontmatter().tracker().success_state(), continuation_guidance = continuation_guidance, repair_architecture_guidance = repair_architecture_guidance, - internal_review_guidance = build_repair_internal_review_guidance(internal_review_mode), - completion_guidance = build_repair_completion_guidance(internal_review_mode), + decodex_review_guidance = build_repair_review_guidance(review_level), + github_review_guidance = build_repair_github_review_guidance(review_level, ISSUE_REVIEW_REPAIR_COMPLETE_TOOL_NAME), + completion_guidance = build_repair_completion_guidance(review_level), ), IssueDispatchMode::Closeout => format!( "Continue retained closeout for Linear issue {identifier}: {title}\n\nDescription:\n{description}\n\nCurrent PR:\n- `{pr_url}`\n\nExecution checklist:\n- Resume from the current branch and merged PR lineage in this worktree. Do not move the issue back to `{in_progress}`.\n- Treat retained closeout as a short deterministic tail. Reuse the existing merged PR evidence instead of restarting broad discovery, and only rerun the minimum validation needed to justify `Done` plus cleanup.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n- If you call `{progress_checkpoint_tool}` during closeout, either omit `head_sha` and let `decodex` record the exact current lane HEAD automatically, or pass the exact full current `HEAD` SHA.\n- Merge is already authoritative for `{pr_url}` before this run starts. Do not land, merge, or request review from this closeout run.\n- The tracker issue may already be in `{completed}` while this deterministic tail work remains pending.\n- If the issue is still in `{success}`, move it once to `{completed}` with `{transition_tool}` before `{closeout_tool}`.\n- Call `{closeout_tool}` with `{pr_url}` and a short result summary, then call `{terminal_finalize_tool}` with path `closeout`.\n- Do not end the turn without either `{closeout_tool}` plus `{terminal_finalize_tool}`, or the manual-attention path.\n- If the issue needs manual attention, add label `{needs_attention}` with `{label_tool}`, call `issue_comment` with kind `manual_attention` and structured public fields, and then call `{terminal_finalize_tool}` with path `manual_attention`.\n- Keep the lane scoped to this retained post-review work and do not treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}", @@ -208,7 +209,7 @@ where continuation_guidance = continuation_guidance, ), _ => format!( - "Resolve Linear issue {identifier}: {title}\n\nDescription:\n{description}\n\n{recovery_context}Execution checklist:\n- Move the issue to `{in_progress}` with `{transition_tool}`. Decodex already records the run-start Linear ledger, so do not leave a separate start comment.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n- Keep discovery bounded to the minimal implementation files needed for this issue; defer broader docs or upstream reading unless a concrete ambiguity blocks the change.\n- Implement the fix in the current worktree.\n{internal_review_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- Run the repository validation needed to justify a reviewable PR.\n- Commit the lane, push branch `{branch}`, and create or update a non-draft PR titled `{pr_title}` for that branch.\n{completion_guidance}- If the issue needs manual attention, add label `{needs_attention}` with `{label_tool}`, call `issue_comment` with kind `manual_attention` and structured public fields, and then call `{terminal_finalize_tool}` with path `manual_attention`. Do not call `{review_handoff_tool}` in that case; `decodex` will stop the lane as a human-required failure without automatic retry.\n- Do not move the issue directly to `{success}` with `{transition_tool}`; `decodex` will finish that writeback after its own validation passes.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}", + "Resolve Linear issue {identifier}: {title}\n\nDescription:\n{description}\n\n{recovery_context}Execution checklist:\n- Move the issue to `{in_progress}` with `{transition_tool}`. Decodex already records the run-start Linear ledger, so do not leave a separate start comment.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n- Keep discovery bounded to the minimal implementation files needed for this issue; defer broader docs or upstream reading unless a concrete ambiguity blocks the change.\n- Implement the fix in the current worktree.\n{decodex_review_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- Run the repository validation needed to justify a reviewable PR.\n- Commit the lane, push branch `{branch}`, and create or update a non-draft PR titled `{pr_title}` for that branch.\n{completion_guidance}- If the issue needs manual attention, add label `{needs_attention}` with `{label_tool}`, call `issue_comment` with kind `manual_attention` and structured public fields, and then call `{terminal_finalize_tool}` with path `manual_attention`. Do not call `{review_handoff_tool}` in that case; `decodex` will stop the lane as a human-required failure without automatic retry.\n- Do not move the issue directly to `{success}` with `{transition_tool}`; `decodex` will finish that writeback after its own validation passes.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}", identifier = issue.identifier, title = issue.title, description = description, @@ -224,10 +225,10 @@ where needs_attention = workflow.frontmatter().tracker().needs_attention_label(), continuation_guidance = continuation_guidance, pr_title = review_pull_request_title(issue), - internal_review_guidance = build_handoff_internal_review_guidance( - internal_review_mode + decodex_review_guidance = build_handoff_review_guidance( + review_level ), - completion_guidance = build_handoff_completion_guidance(internal_review_mode), + completion_guidance = build_handoff_completion_guidance(review_level), ), } } @@ -238,7 +239,7 @@ fn build_continuation_user_input( dispatch_mode: IssueDispatchMode, recorded_pr_url: Option<&str>, success_state: &str, - internal_review_mode: InternalReviewMode, + review_level: ReviewLevel, ) -> String { let completed_state = workflow .frontmatter() @@ -247,16 +248,16 @@ fn build_continuation_user_input( match dispatch_mode { IssueDispatchMode::ReviewRepair => format!( - "Continue retained review repair for Linear issue {identifier} in the current thread and worktree.\n\nContinuation checklist:\n- Resume from the current repository state and outstanding review feedback or retained landing fallback on `{pr_url}`.\n- Keep changes scoped to the same retained review lane and do not move the issue out of `{success}`.\n{internal_review_guidance}- Validate each actionable review claim against the codebase, tests, and requirements before changing code, and keep pushback or clarification threads open until the repaired head is ready.\n- If the blocker is landing fallback, repair only the branch sync, conflict, ambiguous mergeability, or repository-specific recovery issue; do not merge or land the PR yourself.\n- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- If the repaired head is ready, push it. Do not request fresh external review yourself; Decodex will post the next runtime-owned external review request after `{review_repair_tool}` succeeds.\n- After the repaired head is pushed, reply in-thread for every addressed comment and resolve only the GitHub review threads whose fixes landed and verified on the repaired head.\n{completion_guidance}- If the issue requires manual attention, add the needs-attention label, call `issue_comment` with kind `manual_attention` and structured public fields, then finalize path `manual_attention` before ending the turn.\n- If more work still remains after this turn, you may end the turn without terminal finalization and Decodex will decide whether to continue.", + "Continue retained review repair for Linear issue {identifier} in the current thread and worktree.\n\nContinuation checklist:\n- Resume from the current repository state and outstanding review feedback or retained landing fallback on `{pr_url}`.\n- Keep changes scoped to the same retained review lane and do not move the issue out of `{success}`.\n{decodex_review_guidance}- Validate each actionable review claim against the codebase, tests, and requirements before changing code, and keep pushback or clarification threads open until the repaired head is ready.\n- If the blocker is landing fallback, repair only the branch sync, conflict, ambiguous mergeability, or repository-specific recovery issue; do not merge or land the PR yourself.\n- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- If the repaired head is ready, push it.\n{github_review_guidance}- After the repaired head is pushed, reply in-thread for every addressed comment and resolve only the GitHub review threads whose fixes landed and verified on the repaired head.\n{completion_guidance}- If the issue requires manual attention, add the needs-attention label, call `issue_comment` with kind `manual_attention` and structured public fields, then finalize path `manual_attention` before ending the turn.\n- If more work still remains after this turn, you may end the turn without terminal finalization and Decodex will decide whether to continue.", identifier = issue.identifier, pr_url = recorded_pr_url.unwrap_or("(missing review handoff marker)"), success = success_state, - review_repair_tool = ISSUE_REVIEW_REPAIR_COMPLETE_TOOL_NAME, - internal_review_guidance = build_repair_continuation_review_guidance( - internal_review_mode + github_review_guidance = build_repair_github_review_guidance(review_level, ISSUE_REVIEW_REPAIR_COMPLETE_TOOL_NAME), + decodex_review_guidance = build_repair_continuation_review_guidance( + review_level ), completion_guidance = build_repair_continuation_completion_guidance( - internal_review_mode + review_level ), ), IssueDispatchMode::Closeout => format!( @@ -271,139 +272,161 @@ fn build_continuation_user_input( terminal_finalize_tool = ISSUE_TERMINAL_FINALIZE_TOOL_NAME, ), _ => format!( - "Continue working on Linear issue {identifier} in the current thread and worktree.\n\nContinuation checklist:\n- Resume from the current repository state instead of restarting broad discovery.\n- Keep changes scoped to the same issue lane.\n{internal_review_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n{completion_guidance}- If the issue requires manual attention, add the needs-attention label, call `issue_comment` with kind `manual_attention` and structured public fields, then finalize path `manual_attention` before ending the turn.\n- If more work still remains after this turn, you may end the turn without terminal finalization and Decodex will decide whether to continue.", + "Continue working on Linear issue {identifier} in the current thread and worktree.\n\nContinuation checklist:\n- Resume from the current repository state instead of restarting broad discovery.\n- Keep changes scoped to the same issue lane.\n{decodex_review_guidance}- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n{completion_guidance}- If the issue requires manual attention, add the needs-attention label, call `issue_comment` with kind `manual_attention` and structured public fields, then finalize path `manual_attention` before ending the turn.\n- If more work still remains after this turn, you may end the turn without terminal finalization and Decodex will decide whether to continue.", identifier = issue.identifier, - internal_review_guidance = build_handoff_continuation_review_guidance( - internal_review_mode + decodex_review_guidance = build_handoff_continuation_review_guidance( + review_level ), completion_guidance = build_handoff_continuation_completion_guidance( - internal_review_mode, + review_level, &review_pull_request_title(issue), ), ), } } -fn build_handoff_internal_review_guidance(internal_review_mode: InternalReviewMode) -> String { - match internal_review_mode { - InternalReviewMode::Loop => format!( - "- Request an independent fresh-context read-only review pass for the actual current diff and branch state. The reviewer must not edit files, push, land, or mutate tracker state.\n- Use the repo-native bounded review method from `WORKFLOW.md`: run the requirements pass and the adversarial reviewer pass against the current `HEAD`, including regression risk, missing tests, docs/config drift, migration fallout, operator-facing fallout, and mismatch with the accepted Loop/Decision Contract.\n- Validate reviewer comments before repair. Record accepted findings separately from rejected or non-actionable comments, fix only the smallest coherent owned batch, rerun verification, and re-read `HEAD` before deciding the normalized review status.\n- Every time the independent review pass produces a result for the current head, call `{}` with reviewer `independent_fresh_context`, that normalized status, the exact current `HEAD` SHA, concise evidence, checklist notes, and structured accepted/rejected findings.\n", +fn build_handoff_review_guidance(review_level: ReviewLevel) -> String { + match review_level { + ReviewLevel::Off => format!( + "- `[codex].review = \"off\"` for this project, so skip Self Check and Decodex Review, and do not call `{}`.\n", ISSUE_REVIEW_CHECKPOINT_TOOL_NAME ), - InternalReviewMode::Prompt => format!("- {PROMPT_ONLY_INTERNAL_REVIEW_INSTRUCTION}\n"), - InternalReviewMode::Off => format!( - "- `codex.internal_review_mode = \"off\"` for this project, so skip internal review and do not call `{}`.\n", + ReviewLevel::Basic => format!("- Self Check: {SELF_CHECK_INSTRUCTION}\n"), + ReviewLevel::Standard | ReviewLevel::Strict => format!( + "- Self Check: {SELF_CHECK_INSTRUCTION}\n- Decodex Review: request an independent fresh-context read-only review pass for the actual current diff and branch state. The reviewer must not edit files, push, land, or mutate tracker state.\n- Use the repo-native bounded review method from `WORKFLOW.md`: run the requirements pass and the adversarial reviewer pass against the current `HEAD`, including regression risk, missing tests, docs/config drift, migration fallout, operator-facing fallout, and mismatch with the accepted Loop/Decision Contract.\n- Validate reviewer comments before repair. Record accepted findings separately from rejected or non-actionable comments, fix only the smallest coherent owned batch, rerun verification, and re-read `HEAD` before deciding the normalized review status.\n- Every time the Decodex Review pass produces a result for the current head, call `{}` with reviewer `independent_fresh_context`, that normalized status, the exact current `HEAD` SHA, concise evidence, checklist notes, and structured accepted/rejected findings.\n", ISSUE_REVIEW_CHECKPOINT_TOOL_NAME ), } } -fn build_repair_internal_review_guidance(internal_review_mode: InternalReviewMode) -> String { - match internal_review_mode { - InternalReviewMode::Loop => format!( - "- Request an independent fresh-context read-only review pass for the actual repaired branch state. The reviewer must not edit files, push, land, or mutate tracker state.\n- Use the repo-native bounded review method from `WORKFLOW.md`: run the requirements pass and the adversarial reviewer pass against the current repaired `HEAD`, including regression risk, missing tests, docs/config drift, migration fallout, operator-facing fallout, and mismatch with the accepted Loop/Decision Contract.\n- Validate reviewer comments before repair. Record accepted findings separately from rejected or non-actionable comments, fix only the smallest coherent owned batch, rerun verification, and re-read `HEAD` before deciding the normalized review status.\n- Every time the independent review pass produces a result for the current repaired head, call `{}` with reviewer `independent_fresh_context`, that normalized status, the exact current `HEAD` SHA, concise evidence, checklist notes, and structured accepted/rejected findings.\n", +fn build_repair_review_guidance(review_level: ReviewLevel) -> String { + match review_level { + ReviewLevel::Off => format!( + "- `[codex].review = \"off\"` for this project, so skip Self Check and Decodex Review, and do not call `{}`.\n", ISSUE_REVIEW_CHECKPOINT_TOOL_NAME ), - InternalReviewMode::Prompt => format!("- {PROMPT_ONLY_INTERNAL_REVIEW_INSTRUCTION}\n"), - InternalReviewMode::Off => format!( - "- `codex.internal_review_mode = \"off\"` for this project, so skip internal review and do not call `{}`.\n", + ReviewLevel::Basic => format!("- Self Check: {SELF_CHECK_INSTRUCTION}\n"), + ReviewLevel::Standard | ReviewLevel::Strict => format!( + "- Self Check: {SELF_CHECK_INSTRUCTION}\n- Decodex Review: request an independent fresh-context read-only review pass for the actual repaired branch state. The reviewer must not edit files, push, land, or mutate tracker state.\n- Use the repo-native bounded review method from `WORKFLOW.md`: run the requirements pass and the adversarial reviewer pass against the current repaired `HEAD`, including regression risk, missing tests, docs/config drift, migration fallout, operator-facing fallout, and mismatch with the accepted Loop/Decision Contract.\n- Validate reviewer comments before repair. Record accepted findings separately from rejected or non-actionable comments, fix only the smallest coherent owned batch, rerun verification, and re-read `HEAD` before deciding the normalized review status.\n- Every time the Decodex Review pass produces a result for the current repaired head, call `{}` with reviewer `independent_fresh_context`, that normalized status, the exact current `HEAD` SHA, concise evidence, checklist notes, and structured accepted/rejected findings.\n", ISSUE_REVIEW_CHECKPOINT_TOOL_NAME ), } } -fn build_handoff_completion_guidance(internal_review_mode: InternalReviewMode) -> String { - match internal_review_mode { - InternalReviewMode::Loop => format!( +fn build_handoff_completion_guidance(review_level: ReviewLevel) -> String { + match review_level { + ReviewLevel::Standard | ReviewLevel::Strict => format!( "- Call `{}` only after the latest `{}` for this handoff phase and current `HEAD` is `clean`. Then call `{}` with path `review_handoff`.\n", ISSUE_REVIEW_HANDOFF_TOOL_NAME, ISSUE_REVIEW_CHECKPOINT_TOOL_NAME, ISSUE_TERMINAL_FINALIZE_TOOL_NAME ), - InternalReviewMode::Prompt | InternalReviewMode::Off => format!( + ReviewLevel::Off | ReviewLevel::Basic => format!( "- Call `{}` after the branch is pushed, the non-draft PR is ready, and required validation has passed. Then call `{}` with path `review_handoff`.\n", ISSUE_REVIEW_HANDOFF_TOOL_NAME, ISSUE_TERMINAL_FINALIZE_TOOL_NAME ), } } -fn build_repair_completion_guidance(internal_review_mode: InternalReviewMode) -> String { - match internal_review_mode { - InternalReviewMode::Loop => format!( +fn build_repair_completion_guidance(review_level: ReviewLevel) -> String { + match review_level { + ReviewLevel::Standard | ReviewLevel::Strict => format!( "- Call `{}` only after the latest `{}` for this repair phase and current `HEAD` is `clean`. Then call `{}` with path `review_repair`.\n", ISSUE_REVIEW_REPAIR_COMPLETE_TOOL_NAME, ISSUE_REVIEW_CHECKPOINT_TOOL_NAME, ISSUE_TERMINAL_FINALIZE_TOOL_NAME ), - InternalReviewMode::Prompt | InternalReviewMode::Off => format!( + ReviewLevel::Off | ReviewLevel::Basic => format!( "- Call `{}` after the repaired head is pushed and required validation has passed. Then call `{}` with path `review_repair`.\n", ISSUE_REVIEW_REPAIR_COMPLETE_TOOL_NAME, ISSUE_TERMINAL_FINALIZE_TOOL_NAME ), } } -fn build_handoff_continuation_review_guidance( - internal_review_mode: InternalReviewMode, -) -> String { - match internal_review_mode { - InternalReviewMode::Loop => format!( - "- Resume by requesting an independent fresh-context read-only review pass for the actual current diff and branch state; the reviewer must not edit files, push, land, or mutate tracker state.\n- Apply the repo-native bounded review method from `WORKFLOW.md`, validate comments before repair, record accepted findings separately from rejected or non-actionable comments, fix only the smallest coherent owned batch, rerun verification, and re-read `HEAD` before deciding the normalized review status.\n- After each independent review result for the current head, call `{}` with reviewer `independent_fresh_context`, the normalized status, current `HEAD` SHA, checklist notes, and structured accepted/rejected findings.\n", +fn build_handoff_continuation_review_guidance(review_level: ReviewLevel) -> String { + match review_level { + ReviewLevel::Off => format!( + "- `[codex].review = \"off\"` for this project, so continue without Self Check or Decodex Review and do not call `{}`.\n", ISSUE_REVIEW_CHECKPOINT_TOOL_NAME ), - InternalReviewMode::Prompt => format!("- {PROMPT_ONLY_INTERNAL_REVIEW_INSTRUCTION}\n"), - InternalReviewMode::Off => format!( - "- `codex.internal_review_mode = \"off\"` for this project, so continue without internal review and do not call `{}`.\n", + ReviewLevel::Basic => format!("- Self Check: {SELF_CHECK_INSTRUCTION}\n"), + ReviewLevel::Standard | ReviewLevel::Strict => format!( + "- Resume by requesting a Decodex Review pass for the actual current diff and branch state; the reviewer must not edit files, push, land, or mutate tracker state.\n- Apply the repo-native bounded review method from `WORKFLOW.md`, validate comments before repair, record accepted findings separately from rejected or non-actionable comments, fix only the smallest coherent owned batch, rerun verification, and re-read `HEAD` before deciding the normalized review status.\n- After each Decodex Review result for the current head, call `{}` with reviewer `independent_fresh_context`, the normalized status, current `HEAD` SHA, checklist notes, and structured accepted/rejected findings.\n", ISSUE_REVIEW_CHECKPOINT_TOOL_NAME ), } } -fn build_repair_continuation_review_guidance(internal_review_mode: InternalReviewMode) -> String { - match internal_review_mode { - InternalReviewMode::Loop => format!( - "- Resume by requesting an independent fresh-context read-only review pass for the actual repaired branch state; the reviewer must not edit files, push, land, or mutate tracker state.\n- Apply the repo-native bounded review method from `WORKFLOW.md`, validate comments before repair, record accepted findings separately from rejected or non-actionable comments, fix only the smallest coherent owned batch, rerun verification, and re-read `HEAD` before deciding the normalized review status.\n- After each independent review result for the repaired head, call `{}` with reviewer `independent_fresh_context`, the normalized status, current `HEAD` SHA, checklist notes, and structured accepted/rejected findings.\n", +fn build_repair_continuation_review_guidance(review_level: ReviewLevel) -> String { + match review_level { + ReviewLevel::Off => format!( + "- `[codex].review = \"off\"` for this project, so continue without Self Check or Decodex Review and do not call `{}`.\n", ISSUE_REVIEW_CHECKPOINT_TOOL_NAME ), - InternalReviewMode::Prompt => format!("- {PROMPT_ONLY_INTERNAL_REVIEW_INSTRUCTION}\n"), - InternalReviewMode::Off => format!( - "- `codex.internal_review_mode = \"off\"` for this project, so continue without internal review and do not call `{}`.\n", + ReviewLevel::Basic => format!("- Self Check: {SELF_CHECK_INSTRUCTION}\n"), + ReviewLevel::Standard | ReviewLevel::Strict => format!( + "- Resume by requesting a Decodex Review pass for the actual repaired branch state; the reviewer must not edit files, push, land, or mutate tracker state.\n- Apply the repo-native bounded review method from `WORKFLOW.md`, validate comments before repair, record accepted findings separately from rejected or non-actionable comments, fix only the smallest coherent owned batch, rerun verification, and re-read `HEAD` before deciding the normalized review status.\n- After each Decodex Review result for the repaired head, call `{}` with reviewer `independent_fresh_context`, the normalized status, current `HEAD` SHA, checklist notes, and structured accepted/rejected findings.\n", ISSUE_REVIEW_CHECKPOINT_TOOL_NAME ), } } fn build_handoff_continuation_completion_guidance( - internal_review_mode: InternalReviewMode, + review_level: ReviewLevel, pr_title: &str, ) -> String { - match internal_review_mode { - InternalReviewMode::Loop => format!( + match review_level { + ReviewLevel::Standard | ReviewLevel::Strict => format!( "- If the implementation is review-ready, ensure the non-draft PR title is `{pr_title}` and finish the PR-backed tracker handoff only after the latest `{}` for the current `HEAD` is `clean`.\n", ISSUE_REVIEW_CHECKPOINT_TOOL_NAME, ), - InternalReviewMode::Prompt | InternalReviewMode::Off => format!( + ReviewLevel::Off | ReviewLevel::Basic => format!( "- If the implementation is review-ready, ensure the non-draft PR title is `{pr_title}` and finish the PR-backed tracker handoff after required validation has passed.\n", ), } } fn build_repair_continuation_completion_guidance( - internal_review_mode: InternalReviewMode, + review_level: ReviewLevel, ) -> String { - match internal_review_mode { - InternalReviewMode::Loop => format!( + match review_level { + ReviewLevel::Standard | ReviewLevel::Strict => format!( "- Call `{}` only after the latest `{}` for the current `HEAD` is `clean`, and then call `{}` with path `review_repair`.\n", ISSUE_REVIEW_REPAIR_COMPLETE_TOOL_NAME, ISSUE_REVIEW_CHECKPOINT_TOOL_NAME, ISSUE_TERMINAL_FINALIZE_TOOL_NAME ), - InternalReviewMode::Prompt | InternalReviewMode::Off => format!( + ReviewLevel::Off | ReviewLevel::Basic => format!( "- Call `{}` after the repaired head is pushed and required validation has passed, and then call `{}` with path `review_repair`.\n", ISSUE_REVIEW_REPAIR_COMPLETE_TOOL_NAME, ISSUE_TERMINAL_FINALIZE_TOOL_NAME ), } } +fn build_repair_github_review_guidance(review_level: ReviewLevel, repair_tool_name: &str) -> String { + if review_level.uses_github_review() { + return format!( + "- Do not request GitHub Review yourself. Decodex will post the next runtime-owned GitHub Review request after `{repair_tool_name}` succeeds.\n", + ); + } + + String::from( + "- Do not request GitHub Review from this run; the configured review level does not use the runtime-owned GitHub Review path.\n", + ) +} + +fn build_repair_retained_tail_guidance(review_level: ReviewLevel, success_state: &str) -> String { + if review_level.uses_github_review() { + return format!( + "- Keep the tracker issue in `{success_state}`. Decodex will handle the later GitHub Review request or clean-path runtime landing, closeout, and cleanup lifecycle.\n", + ); + } + + format!( + "- Keep the tracker issue in `{success_state}`. Decodex will handle the clean-path runtime landing, closeout, and cleanup lifecycle.\n", + ) +} + fn allows_clean_continuation( workflow: &WorkflowDocument, dispatch_mode: IssueDispatchMode, @@ -462,7 +485,7 @@ fn build_external_repair_architecture_guidance( } format!( - "- This retained repair is external review round {}. Before another patch-only cycle, decide whether the repeated churn points to an architectural or root-cause defect that local patching will not converge.\n- If it is architectural, take the manual-attention path instead of continuing patch-on-patch repair.\n- If it is not architectural and the findings are still normal retained review work, continue this repair normally; a successful `{}` will reset the external review-round budget.\n", + "- This retained repair is GitHub Review round {}. Before another patch-only cycle, decide whether the repeated churn points to an architectural or root-cause defect that local patching will not converge.\n- If it is architectural, take the manual-attention path instead of continuing patch-on-patch repair.\n- If it is not architectural and the findings are still normal retained review work, continue this repair normally; a successful `{}` will reset the GitHub Review round budget.\n", marker.external_round_count(), ISSUE_REVIEW_REPAIR_COMPLETE_TOOL_NAME ) @@ -496,7 +519,7 @@ fn build_review_run_context( cwd: issue_run.worktree.path.clone(), github_token_env_var: Some(project.github().token_env_var().to_owned()), github_command_path: project.github().command_path().map(Path::to_path_buf), - internal_review_mode: project.codex().internal_review_mode(), + review_level: project.codex().review_level(), mode: ReviewExecutionMode::Repair, recorded_pr_url: Some(review_handoff.pr_url().to_owned()), }) @@ -522,7 +545,7 @@ fn build_review_run_context( cwd: issue_run.worktree.path.clone(), github_token_env_var: Some(project.github().token_env_var().to_owned()), github_command_path: project.github().command_path().map(Path::to_path_buf), - internal_review_mode: project.codex().internal_review_mode(), + review_level: project.codex().review_level(), mode: ReviewExecutionMode::Closeout, recorded_pr_url: Some(review_handoff.pr_url().to_owned()), }) @@ -536,7 +559,7 @@ fn build_review_run_context( cwd: issue_run.worktree.path.clone(), github_token_env_var: Some(project.github().token_env_var().to_owned()), github_command_path: project.github().command_path().map(Path::to_path_buf), - internal_review_mode: project.codex().internal_review_mode(), + review_level: project.codex().review_level(), mode: ReviewExecutionMode::Handoff, recorded_pr_url: None, }), diff --git a/apps/decodex/src/orchestrator/run_cycle.rs b/apps/decodex/src/orchestrator/run_cycle.rs index 3db4213a7..464d280df 100644 --- a/apps/decodex/src/orchestrator/run_cycle.rs +++ b/apps/decodex/src/orchestrator/run_cycle.rs @@ -470,8 +470,8 @@ fn reconcile_retained_review_lane( where T: IssueTracker, { - if !project.codex().external_review_enabled() { - return handle_internal_review_only_lane( + if !project.codex().review_level().uses_github_review() { + return handle_non_github_review_lane( tracker, project, workflow, @@ -529,7 +529,7 @@ where } } -fn handle_internal_review_only_lane( +fn handle_non_github_review_lane( tracker: &T, project: &ServiceConfig, workflow: &WorkflowDocument, @@ -552,7 +552,7 @@ where state_store, lane, now_unix_epoch, - "internal_review_only_merge_visibility_timeout", + "non_github_review_merge_visibility_timeout", ); } if external_review_requires_repair(&lane.review_state, &lane.orchestration_marker) @@ -598,8 +598,8 @@ where &mut runtime, lane, RetainedAdminMergeReasons { - admin_merge_unavailable: "internal_review_only_admin_merge_unavailable", - admin_merge_failed: "internal_review_only_admin_merge_failed", + admin_merge_unavailable: "non_github_review_admin_merge_unavailable", + admin_merge_failed: "non_github_review_admin_merge_failed", }, ) } @@ -2321,7 +2321,7 @@ where github_command_path: project.github().command_path().map(Path::to_path_buf), }; - if let Some(retained_summary) = drain_internal_review_only_retained_tail_with_inspector( + if let Some(retained_summary) = drain_non_github_review_retained_tail_with_inspector( tracker, project, workflow, @@ -2371,7 +2371,7 @@ where }) } -fn drain_internal_review_only_retained_tail_with_inspector( +fn drain_non_github_review_retained_tail_with_inspector( tracker: &T, project: &ServiceConfig, workflow: &WorkflowDocument, @@ -2385,7 +2385,7 @@ where I: PullRequestReviewStateInspector, F: FnMut(&RunSummary) -> Result>, { - if project.codex().external_review_enabled() + if project.codex().review_level().uses_github_review() || summary.continuation_pending || !matches!( summary.dispatch_mode, @@ -2426,7 +2426,7 @@ where return Ok(None); } - if lane.reason != "internal_review_only_waiting_for_merge" + if lane.reason != "non_github_review_waiting_for_merge" || pass + 1 == INTERNAL_RETAINED_DRAIN_MAX_PASSES { return Ok(None); diff --git a/apps/decodex/src/orchestrator/selection.rs b/apps/decodex/src/orchestrator/selection.rs index 1bc487b91..2879d6138 100644 --- a/apps/decodex/src/orchestrator/selection.rs +++ b/apps/decodex/src/orchestrator/selection.rs @@ -312,11 +312,11 @@ fn retained_review_needs_attention_error_class(reason: &str) -> &'static str { "external_review_pass_signal_missing" => "external_review_pass_signal_missing", "external_review_request_ci_red_manual_attention" => "external_review_request_ci_red_manual_attention", - "internal_review_only_admin_merge_failed" => "internal_review_only_admin_merge_failed", - "internal_review_only_admin_merge_unavailable" => - "internal_review_only_admin_merge_unavailable", - "internal_review_only_merge_visibility_timeout" => - "internal_review_only_merge_visibility_timeout", + "non_github_review_admin_merge_failed" => "non_github_review_admin_merge_failed", + "non_github_review_admin_merge_unavailable" => + "non_github_review_admin_merge_unavailable", + "non_github_review_merge_visibility_timeout" => + "non_github_review_merge_visibility_timeout", "pull_request_is_draft" => "pull_request_is_draft", "pull_request_merge_commit_lineage_check_failed" => "pull_request_merge_commit_lineage_check_failed", diff --git a/apps/decodex/src/orchestrator/status.rs b/apps/decodex/src/orchestrator/status.rs index 2b85c53f3..37ff01b6f 100644 --- a/apps/decodex/src/orchestrator/status.rs +++ b/apps/decodex/src/orchestrator/status.rs @@ -3044,7 +3044,7 @@ where snapshot, workflow, review_state_inspector, - project.codex().external_review_enabled(), + project.codex().review_level().uses_github_review(), Some((state_store, project.service_id())), ) } @@ -3053,7 +3053,7 @@ fn classify_post_review_lane_with_external_review( snapshot: &PostReviewLaneSnapshot, workflow: &WorkflowDocument, review_state_inspector: &I, - external_review_enabled: bool, + github_review_enabled: bool, runtime_state: Option<(&StateStore, &str)>, ) -> crate::prelude::Result where @@ -3073,7 +3073,7 @@ where ) { return Ok(classification); } - if !external_review_enabled { + if !github_review_enabled { let orchestration_marker = load_post_review_orchestration_marker( snapshot, &review_state, @@ -3085,7 +3085,7 @@ where return Ok(classification); } - apply_internal_review_only_post_review_classification( + apply_non_github_review_post_review_classification( &mut classification, &review_state, orchestration_marker.as_ref(), @@ -3302,7 +3302,7 @@ fn apply_pre_orchestration_post_review_classification( false } -fn apply_internal_review_only_post_review_classification( +fn apply_non_github_review_post_review_classification( classification: &mut PostReviewLaneClassification, review_state: &PullRequestReviewState, orchestration_marker: Option<&ReviewOrchestrationMarker>, @@ -3322,10 +3322,10 @@ fn apply_internal_review_only_post_review_classification( { *classification = blocked_post_review_lane_from_state( review_state, - "internal_review_only_merge_visibility_timeout", + "non_github_review_merge_visibility_timeout", ); } else { - classification.reason = String::from("internal_review_only_waiting_for_merge"); + classification.reason = String::from("non_github_review_waiting_for_merge"); } return Ok(()); @@ -3335,7 +3335,7 @@ fn apply_internal_review_only_post_review_classification( classification.reason = if review_state_landing_requires_agent_fallback(review_state) { String::from("retained_landing_agent_fallback_required") } else { - String::from("internal_review_only_repair_required") + String::from("non_github_review_repair_required") }; return Ok(()); @@ -3344,12 +3344,12 @@ fn apply_internal_review_only_post_review_classification( if review_state_clean_path_landing_gates_satisfied(review_state) { classification.decision = PostReviewLaneDecision::ReadyToLand; - classification.reason = String::from("internal_review_only_ready_to_land"); + classification.reason = String::from("non_github_review_ready_to_land"); } else if review_state_landing_requires_agent_fallback(review_state) { classification.decision = PostReviewLaneDecision::NeedsReviewRepair; classification.reason = String::from("retained_landing_agent_fallback_required"); } else { - classification.reason = String::from("internal_review_only_waiting_gates"); + classification.reason = String::from("non_github_review_waiting_gates"); } Ok(()) diff --git a/apps/decodex/src/orchestrator/tests.rs b/apps/decodex/src/orchestrator/tests.rs index 6ca2a8722..891ec5ddc 100644 --- a/apps/decodex/src/orchestrator/tests.rs +++ b/apps/decodex/src/orchestrator/tests.rs @@ -30,7 +30,7 @@ use crate::{orchestrator::RepoGatePhaseGoalController, tracker::records}; TrackerToolBridge, TurnContinuationGuard, }; #[rustfmt::skip] -use crate::config::{InternalReviewMode, ServiceConfig}; +use crate::config::{ReviewLevel, ServiceConfig}; #[rustfmt::skip] use crate::github; #[rustfmt::skip] @@ -1107,8 +1107,7 @@ fn sample_service_config_toml( tracker_api_key_env_var: &str, github_token_env_var: &str, worktree_root: Option<&Path>, - internal_review_mode: InternalReviewMode, - external_review_enabled: bool, + review_level: ReviewLevel, ) -> String { let mut toml = format!( r#"service_id = "{service_id}" @@ -1121,18 +1120,9 @@ token_env_var = "{github_token_env_var}" "# ); - if internal_review_mode != InternalReviewMode::Loop || !external_review_enabled { + if review_level != ReviewLevel::Strict { toml.push_str("\n\n[codex]\n"); - - if internal_review_mode != InternalReviewMode::Loop { - toml.push_str(&format!( - "internal_review_mode = \"{}\"\n", - internal_review_mode.as_str() - )); - } - if !external_review_enabled { - toml.push_str("external_review_enabled = false\n"); - } + toml.push_str(&format!("review = \"{}\"\n", review_level.as_str())); } toml.push_str( @@ -1153,8 +1143,7 @@ repo_root = "." fn service_config_toml_for_config( config: &ServiceConfig, github_token_env_var: &str, - internal_review_mode: InternalReviewMode, - external_review_enabled: bool, + review_level: ReviewLevel, ) -> String { let default_worktree_root = config.repo_root().join(".worktrees"); let worktree_root = @@ -1165,8 +1154,7 @@ fn service_config_toml_for_config( config.tracker().api_key_env_var(), github_token_env_var, worktree_root, - internal_review_mode, - external_review_enabled, + review_level, ) } @@ -1176,46 +1164,19 @@ fn service_config_with_github_token_env_var( ) -> ServiceConfig { write_service_config( config.repo_root(), - &service_config_toml_for_config( - config, - token_env_var, - config.codex().internal_review_mode(), - config.codex().external_review_enabled(), - ), + &service_config_toml_for_config(config, token_env_var, config.codex().review_level()), ); load_service_config(config.repo_root()) } -fn service_config_with_external_review_enabled( +fn service_config_with_review_level( config: &ServiceConfig, - external_review_enabled: bool, + review_level: ReviewLevel, ) -> ServiceConfig { write_service_config( config.repo_root(), - &service_config_toml_for_config( - config, - config.github().token_env_var(), - config.codex().internal_review_mode(), - external_review_enabled, - ), - ); - - load_service_config(config.repo_root()) -} - -fn service_config_with_internal_review_mode( - config: &ServiceConfig, - internal_review_mode: InternalReviewMode, -) -> ServiceConfig { - write_service_config( - config.repo_root(), - &service_config_toml_for_config( - config, - config.github().token_env_var(), - internal_review_mode, - config.codex().external_review_enabled(), - ), + &service_config_toml_for_config(config, config.github().token_env_var(), review_level), ); load_service_config(config.repo_root()) @@ -1300,7 +1261,7 @@ fn temp_project_layout_with_tracker_project_slug_max_turns_and_read_first( write_service_config( &repo_root, - &sample_service_config_toml("pubfi", "HOME", "HOME", None, InternalReviewMode::Loop, true), + &sample_service_config_toml("pubfi", "HOME", "HOME", None, ReviewLevel::Strict), ); git_status_success(&repo_root, &["init", "-b", "main"]); git_status_success(&repo_root, &["config", "user.name", "Decodex Tests"]); @@ -1330,7 +1291,7 @@ fn temp_project_layout_with_workflow_markdown( write_service_config( &repo_root, - &sample_service_config_toml("pubfi", "HOME", "HOME", None, InternalReviewMode::Loop, true), + &sample_service_config_toml("pubfi", "HOME", "HOME", None, ReviewLevel::Strict), ); git_status_success(&repo_root, &["init", "-b", "main"]); git_status_success(&repo_root, &["config", "user.name", "Decodex Tests"]); diff --git a/apps/decodex/src/orchestrator/tests/intake/candidate_selection.rs b/apps/decodex/src/orchestrator/tests/intake/candidate_selection.rs index 9d30208fa..9d569d2a7 100644 --- a/apps/decodex/src/orchestrator/tests/intake/candidate_selection.rs +++ b/apps/decodex/src/orchestrator/tests/intake/candidate_selection.rs @@ -794,12 +794,12 @@ fn retained_closeout_identity_reuse_respects_attempt_history() { } #[test] -fn internal_review_only_retained_drain_handles_same_issue_closeout_after_merge_visibility() { +fn non_github_review_retained_drain_handles_same_issue_closeout_after_merge_visibility() { for closeout_available in [true, false] { let (temp_dir, config, workflow) = temp_project_layout(); - let config = service_config_with_external_review_enabled( + let config = service_config_with_review_level( &service_config_with_github_token_env_var(&config, "PATH"), - false, + ReviewLevel::Standard, ); let (_path_guard, invocation_log_path) = install_fake_admin_merge_gh_response(&temp_dir); let state_store = StateStore::open_in_memory().expect("state store should open"); @@ -857,7 +857,7 @@ fn internal_review_only_retained_drain_handles_same_issue_closeout_after_merge_v ..handoff_summary.clone() }; let closeout_dispatches = RefCell::new(Vec::new()); - let drained = orchestrator::drain_internal_review_only_retained_tail_with_inspector( + let drained = orchestrator::drain_non_github_review_retained_tail_with_inspector( &tracker, &config, &workflow, @@ -878,7 +878,7 @@ fn internal_review_only_retained_drain_handles_same_issue_closeout_after_merge_v if closeout_available { Ok(Some(closeout_summary.clone())) } else { Ok(None) } }, ) - .expect("internal-review-only retained drain should succeed"); + .expect("non-GitHub-review retained drain should succeed"); if closeout_available { assert_eq!( @@ -942,9 +942,9 @@ fn assert_admin_merge_invocation( } #[test] -fn internal_review_only_retained_drain_stops_cleanly_when_checks_are_pending() { +fn non_github_review_retained_drain_stops_cleanly_when_checks_are_pending() { let (_temp_dir, config, workflow) = temp_project_layout(); - let config = service_config_with_external_review_enabled(&config, false); + let config = service_config_with_review_level(&config, ReviewLevel::Standard); let state_store = StateStore::open_in_memory().expect("state store should open"); let issue = candidate_selection_service_owned_issue("In Review"); let tracker = FakeTracker::with_refresh_snapshots( @@ -968,7 +968,7 @@ fn internal_review_only_retained_drain_stops_cleanly_when_checks_are_pending() { let handoff_summary = sample_handoff_summary(&issue, &repo_root); let closeout_dispatches = RefCell::new(Vec::new()); - let drained = orchestrator::drain_internal_review_only_retained_tail_with_inspector( + let drained = orchestrator::drain_non_github_review_retained_tail_with_inspector( &tracker, &config, &workflow, diff --git a/apps/decodex/src/orchestrator/tests/intake/run_and_prompting.rs b/apps/decodex/src/orchestrator/tests/intake/run_and_prompting.rs index 62bbc81c9..45ba7cce1 100644 --- a/apps/decodex/src/orchestrator/tests/intake/run_and_prompting.rs +++ b/apps/decodex/src/orchestrator/tests/intake/run_and_prompting.rs @@ -79,7 +79,7 @@ fn build_normal_prompt_surfaces( IssueDispatchMode::Normal, None, workflow.frontmatter().tracker().success_state(), - config.codex().internal_review_mode(), + config.codex().review_level(), ); PromptSurfaces { developer_instructions, user_input, continuation_input } @@ -609,7 +609,7 @@ fn normal_prompts_require_issue_prefixed_pull_request_title() { IssueDispatchMode::Normal, None, workflow.frontmatter().tracker().success_state(), - config.codex().internal_review_mode(), + config.codex().review_level(), ); let expected_title = "XY-381: Ensure Decodex-created PR titles include issue authority prefix"; let create_or_update_instruction = @@ -673,21 +673,21 @@ fn retry_prompts_include_recovery_context() { } #[test] -fn normal_prompts_respect_non_loop_internal_review_modes() { +fn normal_prompts_respect_non_standard_review_levels() { for (mode, expected, forbidden_checkpoint) in [ ( - InternalReviewMode::Off, - "do not call `issue_review_checkpoint`", + ReviewLevel::Off, + "[codex].review = \"off\"", None, ), ( - InternalReviewMode::Prompt, - "Review your work repeatedly and fix any logic bugs until no new issues are found.", + ReviewLevel::Basic, + "Self Check: Review your work repeatedly and fix any logic bugs until no new issues are found.", Some(ISSUE_REVIEW_CHECKPOINT_TOOL_NAME), ), ] { let (_temp_dir, config, workflow) = temp_project_layout(); - let config = service_config_with_internal_review_mode(&config, mode); + let config = service_config_with_review_level(&config, mode); let prompts = build_normal_prompt_surfaces(&config, &workflow); for prompt in prompts.all() { @@ -747,7 +747,7 @@ fn multi_turn_prompts_allow_nonterminal_yield_boundary() { IssueDispatchMode::Normal, None, workflow.frontmatter().tracker().success_state(), - config.codex().internal_review_mode(), + config.codex().review_level(), ); assert!(user_input.contains("you may end the turn without")); @@ -801,7 +801,7 @@ fn closeout_prompts_forbid_clean_continuation_boundaries() { IssueDispatchMode::Closeout, Some(pr_url), workflow.frontmatter().tracker().success_state(), - config.codex().internal_review_mode(), + config.codex().review_level(), ); for prompt in [&developer_instructions, &user_input, &continuation_input] { @@ -857,17 +857,14 @@ fn review_repair_prompts_require_same_pr_repair_completion() { IssueDispatchMode::ReviewRepair, Some(pr_url), workflow.frontmatter().tracker().success_state(), - config.codex().internal_review_mode(), + config.codex().review_level(), ); assert!(developer_instructions.contains(ISSUE_REVIEW_REPAIR_COMPLETE_TOOL_NAME)); assert!(developer_instructions.contains(ISSUE_REVIEW_CHECKPOINT_TOOL_NAME)); assert!(developer_instructions.contains("Do not move the issue back to `In Progress`")); assert!(developer_instructions.contains("do not call `issue_review_handoff`")); - assert!( - developer_instructions - .contains("Request an independent fresh-context read-only review pass") - ); + assert!(developer_instructions.contains("Decodex Review: request an independent fresh-context read-only review pass")); assert!(developer_instructions.contains("structured accepted/rejected findings")); assert!(developer_instructions.contains( "including non-thread review summaries, validate the claim against the codebase, tests, and requirements" @@ -879,7 +876,7 @@ fn review_repair_prompts_require_same_pr_repair_completion() { assert!(developer_instructions.contains("Do not merge or land the PR yourself")); assert!(user_input.contains(pr_url)); assert!(user_input.contains(ISSUE_REVIEW_CHECKPOINT_TOOL_NAME)); - assert!(user_input.contains("Request an independent fresh-context read-only review pass")); + assert!(user_input.contains("Decodex Review: request an independent fresh-context read-only review pass")); assert!(user_input.contains("structured accepted/rejected findings")); assert!(user_input.contains( "Read the current review feedback on `https://github.com/hack-ink/decodex/pull/77`, including non-thread review summaries" @@ -896,10 +893,7 @@ fn review_repair_prompts_require_same_pr_repair_completion() { "resolve only the GitHub review threads whose fixes landed and verified on the repaired head" )); assert!(continuation_input.contains(ISSUE_REVIEW_CHECKPOINT_TOOL_NAME)); - assert!( - continuation_input - .contains("Resume by requesting an independent fresh-context read-only review pass") - ); + assert!(continuation_input.contains("Resume by requesting a Decodex Review pass")); assert!(continuation_input.contains("structured accepted/rejected findings")); assert!(continuation_input.contains( "Validate each actionable review claim against the codebase, tests, and requirements before changing code" @@ -911,7 +905,7 @@ fn review_repair_prompts_require_same_pr_repair_completion() { ); assert!(continuation_input.contains("retained landing fallback")); assert!(continuation_input.contains("do not merge or land the PR yourself")); - assert!(continuation_input.contains("Do not request fresh external review yourself")); + assert!(continuation_input.contains("Do not request GitHub Review yourself")); assert!(continuation_input.contains("In Review")); assert!(continuation_input.contains("review_repair")); @@ -930,9 +924,9 @@ fn review_repair_prompts_require_same_pr_repair_completion() { } #[test] -fn review_repair_prompts_skip_internal_review_checkpoint_when_disabled() { +fn review_repair_prompts_skip_decodex_review_checkpoint_when_off() { let (_temp_dir, config, workflow) = temp_project_layout_with_max_turns(4); - let config = service_config_with_internal_review_mode(&config, InternalReviewMode::Off); + let config = service_config_with_review_level(&config, ReviewLevel::Off); let issue = sample_issue("In Review", &[]); let tracker = FakeTracker::new(vec![issue.clone()]); let issue_run = orchestrator::IssueRunPlan { @@ -976,11 +970,11 @@ fn review_repair_prompts_skip_internal_review_checkpoint_when_disabled() { IssueDispatchMode::ReviewRepair, Some(pr_url), workflow.frontmatter().tracker().success_state(), - config.codex().internal_review_mode(), + config.codex().review_level(), ); for prompt in [&developer_instructions, &user_input, &continuation_input] { - assert!(prompt.contains("codex.internal_review_mode = \"off\"")); + assert!(prompt.contains("[codex].review = \"off\"")); assert!(prompt.contains("do not call `issue_review_checkpoint`")); assert!(!prompt.contains("Follow the repo-native bounded review method")); assert!(!prompt.contains("only after the latest `issue_review_checkpoint`")); @@ -994,7 +988,7 @@ fn review_repair_prompts_skip_internal_review_checkpoint_when_disabled() { assert!(user_input.contains("required validation has passed")); assert!(continuation_input.contains("required validation has passed")); assert!(user_input.contains("validate each actionable claim against the codebase")); - assert!(continuation_input.contains("Do not request fresh external review yourself")); + assert!(continuation_input.contains("Do not request GitHub Review from this run")); } #[test] @@ -1047,7 +1041,7 @@ Custom workflow. IssueDispatchMode::ReviewRepair, Some("https://github.com/hack-ink/decodex/pull/77"), workflow.frontmatter().tracker().success_state(), - InternalReviewMode::Loop, + ReviewLevel::Standard, ); assert!(continuation_input.contains("Ready For QA")); @@ -1128,11 +1122,11 @@ fn review_repair_prompts_surface_architecture_check_on_fourth_external_round() { Some(pr_url), ); - assert!(developer_instructions.contains("external review round 4")); + assert!(developer_instructions.contains("GitHub Review round 4")); assert!(developer_instructions.contains("architectural or root-cause defect")); - assert!(developer_instructions.contains("reset the external review-round budget")); - assert!(user_input.contains("external review round 4")); - assert!(user_input.contains("Do not request fresh external review yourself")); + assert!(developer_instructions.contains("reset the GitHub Review round budget")); + assert!(user_input.contains("GitHub Review round 4")); + assert!(user_input.contains("Do not request GitHub Review yourself")); } #[test] @@ -1241,7 +1235,7 @@ fn review_repair_prompts_ignore_newer_unrelated_branch_orchestration_records() { ) .expect("review repair developer instructions should build"); - assert!(!developer_instructions.contains("external review round 4")); + assert!(!developer_instructions.contains("GitHub Review round 4")); assert!(!developer_instructions.contains("architectural or root-cause defect")); } @@ -1291,7 +1285,7 @@ fn closeout_prompts_require_retained_pr_closeout_completion() { IssueDispatchMode::Closeout, Some(pr_url), workflow.frontmatter().tracker().success_state(), - config.codex().internal_review_mode(), + config.codex().review_level(), ); assert!(developer_instructions.contains(ISSUE_DELIVERY_CLOSEOUT_COMPLETE_TOOL_NAME)); @@ -1700,7 +1694,7 @@ fn continuation_guard_allows_closeout_continuation_after_issue_reaches_completed cwd: worktree.path.clone(), github_token_env_var: None, github_command_path: None, - internal_review_mode: InternalReviewMode::Loop, + review_level: ReviewLevel::Strict, mode: ReviewExecutionMode::Closeout, recorded_pr_url: Some(String::from(pr_url)), }, @@ -1784,7 +1778,7 @@ fn continuation_guard_blocks_closeout_continuation_when_completed_issue_pr_is_op cwd: worktree.path.clone(), github_token_env_var: None, github_command_path: None, - internal_review_mode: InternalReviewMode::Loop, + review_level: ReviewLevel::Strict, mode: ReviewExecutionMode::Closeout, recorded_pr_url: Some(String::from(pr_url)), }, @@ -1867,7 +1861,7 @@ fn continuation_guard_errors_when_completed_issue_pr_state_cannot_be_read() { cwd: worktree.path.clone(), github_token_env_var: None, github_command_path: None, - internal_review_mode: InternalReviewMode::Loop, + review_level: ReviewLevel::Strict, mode: ReviewExecutionMode::Closeout, recorded_pr_url: Some(String::from(pr_url)), }, diff --git a/apps/decodex/src/orchestrator/tests/intake/workflow_reload.rs b/apps/decodex/src/orchestrator/tests/intake/workflow_reload.rs index 4df0a7810..c2f8f8dbc 100644 --- a/apps/decodex/src/orchestrator/tests/intake/workflow_reload.rs +++ b/apps/decodex/src/orchestrator/tests/intake/workflow_reload.rs @@ -178,7 +178,7 @@ fn expected_developer_instructions( )); sections.push(format!( - "Tracker tool contract\n- You own issue-scoped tracker writes for `{issue}`.\n- At the start of execution, call `{transition_tool}` to move the issue to `{in_progress}`. Decodex already records the run-start Linear ledger, so do not add a separate start comment.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n- Request an independent fresh-context read-only review pass for the actual current diff and branch state. The reviewer must not edit files, push, land, or mutate tracker state.\n- Use the repo-native bounded review method from `WORKFLOW.md`: run the requirements pass and the adversarial reviewer pass against the current `HEAD`, including regression risk, missing tests, docs/config drift, migration fallout, operator-facing fallout, and mismatch with the accepted Loop/Decision Contract.\n- Validate reviewer comments before repair. Record accepted findings separately from rejected or non-actionable comments, fix only the smallest coherent owned batch, rerun verification, and re-read `HEAD` before deciding the normalized review status.\n- Every time the independent review pass produces a result for the current head, call `{review_checkpoint_tool}` with reviewer `independent_fresh_context`, that normalized status, the exact current `HEAD` SHA, concise evidence, checklist notes, and structured accepted/rejected findings.\n- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- When the implementation is ready, commit the lane, push branch `{branch}`, and create or update a non-draft PR titled `{pr_title}` for that branch.\n- Call `{review_handoff_tool}` only after the latest `{review_checkpoint_tool}` for this handoff phase and current `HEAD` is `clean`. Then call `{terminal_finalize_tool}` with path `review_handoff`.\n- If you determine the issue needs human attention, add label `{needs_attention}` with `{label_tool}`, then call `issue_comment` with kind `manual_attention` and structured public fields (`error_class`, `next_action`, `blockers`, `evidence`; include `failed_command` and `raw_error` only when public-safe), and then call `{terminal_finalize_tool}` with path `manual_attention`. Do not speculate about capabilities you did not directly verify. Do not call `{review_handoff_tool}` in that case; `decodex` will stop the lane as a human-required failure without automatic retry.\n- Do not move the issue directly to `{success}` with `{transition_tool}`. `decodex` will complete the success writeback only after its own validation passes.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}\n- Never write to any other issue.", + "Tracker tool contract\n- You own issue-scoped tracker writes for `{issue}`.\n- At the start of execution, call `{transition_tool}` to move the issue to `{in_progress}`. Decodex already records the run-start Linear ledger, so do not add a separate start comment.\n- Update `{progress_checkpoint_tool}` whenever the execution phase, focus, next action, blockers, evidence, or verification state changes materially.\n- Self Check: Review your work repeatedly and fix any logic bugs until no new issues are found.\n- Decodex Review: request an independent fresh-context read-only review pass for the actual current diff and branch state. The reviewer must not edit files, push, land, or mutate tracker state.\n- Use the repo-native bounded review method from `WORKFLOW.md`: run the requirements pass and the adversarial reviewer pass against the current `HEAD`, including regression risk, missing tests, docs/config drift, migration fallout, operator-facing fallout, and mismatch with the accepted Loop/Decision Contract.\n- Validate reviewer comments before repair. Record accepted findings separately from rejected or non-actionable comments, fix only the smallest coherent owned batch, rerun verification, and re-read `HEAD` before deciding the normalized review status.\n- Every time the Decodex Review pass produces a result for the current head, call `{review_checkpoint_tool}` with reviewer `independent_fresh_context`, that normalized status, the exact current `HEAD` SHA, concise evidence, checklist notes, and structured accepted/rejected findings.\n- Treat failures from repo-native `canonicalize_commands`, `verify_commands`, or tracked rewrites left by that repo gate as continued repair by default: keep fixing the lane and rerun the gate instead of taking `manual_attention` unless the blocker is clearly toolchain, environment, or operator-owned.\n- When the implementation is ready, commit the lane, push branch `{branch}`, and create or update a non-draft PR titled `{pr_title}` for that branch.\n- Call `{review_handoff_tool}` only after the latest `{review_checkpoint_tool}` for this handoff phase and current `HEAD` is `clean`. Then call `{terminal_finalize_tool}` with path `review_handoff`.\n- If you determine the issue needs human attention, add label `{needs_attention}` with `{label_tool}`, then call `issue_comment` with kind `manual_attention` and structured public fields (`error_class`, `next_action`, `blockers`, `evidence`; include `failed_command` and `raw_error` only when public-safe), and then call `{terminal_finalize_tool}` with path `manual_attention`. Do not speculate about capabilities you did not directly verify. Do not call `{review_handoff_tool}` in that case; `decodex` will stop the lane as a human-required failure without automatic retry.\n- Do not move the issue directly to `{success}` with `{transition_tool}`. `decodex` will complete the success writeback only after its own validation passes.\n- Do not report the run as complete or treat `{progress_checkpoint_tool}` as terminal completion until `{terminal_finalize_tool}` succeeds.{continuation_guidance}\n- Never write to any other issue.", issue = issue_run.issue.identifier, transition_tool = ISSUE_TRANSITION_TOOL_NAME, label_tool = ISSUE_LABEL_ADD_TOOL_NAME, diff --git a/apps/decodex/src/orchestrator/tests/operator/status/control_plane.rs b/apps/decodex/src/orchestrator/tests/operator/status/control_plane.rs index 76fcc029f..feecb1f8a 100644 --- a/apps/decodex/src/orchestrator/tests/operator/status/control_plane.rs +++ b/apps/decodex/src/orchestrator/tests/operator/status/control_plane.rs @@ -96,8 +96,7 @@ fn control_plane_context_failure_includes_project_warning_detail() { missing_env_var, base_config.github().token_env_var(), None, - base_config.codex().internal_review_mode(), - base_config.codex().external_review_enabled(), + base_config.codex().review_level(), ), ); @@ -353,7 +352,7 @@ fn control_plane_snapshot_aggregates_top_level_lanes_for_all_registered_projects write_service_config( idle_base_config.repo_root(), - &sample_service_config_toml("rsnap", "HOME", "HOME", None, InternalReviewMode::Loop, true), + &sample_service_config_toml("rsnap", "HOME", "HOME", None, ReviewLevel::Strict), ); let idle_config = load_service_config(idle_base_config.repo_root()); @@ -518,8 +517,7 @@ fn service_scoped_project_registration( "HOME", "HOME", None, - InternalReviewMode::Loop, - true, + ReviewLevel::Strict, ), ); diff --git a/apps/decodex/src/orchestrator/tests/operator/status/running_lanes.rs b/apps/decodex/src/orchestrator/tests/operator/status/running_lanes.rs index 1684e9740..325bced22 100644 --- a/apps/decodex/src/orchestrator/tests/operator/status/running_lanes.rs +++ b/apps/decodex/src/orchestrator/tests/operator/status/running_lanes.rs @@ -432,8 +432,7 @@ fn idle_operator_status_snapshot_includes_configured_codex_accounts() { let mut config_toml = service_config_toml_for_config( &base_config, base_config.github().token_env_var(), - base_config.codex().internal_review_mode(), - base_config.codex().external_review_enabled(), + base_config.codex().review_level(), ); config_toml.push_str(&format!( @@ -491,8 +490,7 @@ fn status_command_snapshot_does_not_probe_configured_codex_accounts() { let mut config_toml = service_config_toml_for_config( &base_config, base_config.github().token_env_var(), - base_config.codex().internal_review_mode(), - base_config.codex().external_review_enabled(), + base_config.codex().review_level(), ); config_toml.push_str( diff --git a/apps/decodex/src/orchestrator/tests/recovery/runtime_reentry.rs b/apps/decodex/src/orchestrator/tests/recovery/runtime_reentry.rs index 174e0e990..f1d76474c 100644 --- a/apps/decodex/src/orchestrator/tests/recovery/runtime_reentry.rs +++ b/apps/decodex/src/orchestrator/tests/recovery/runtime_reentry.rs @@ -341,9 +341,9 @@ fn run_project_once_recovers_retained_worktree_from_issue_identifier() { #[test] fn run_project_once_recovers_ready_post_review_lane_before_landing() { let (temp_dir, base_config, workflow) = temp_project_layout(); - let config = service_config_with_external_review_enabled( + let config = service_config_with_review_level( &service_config_with_github_token_env_var(&base_config, "PATH"), - false, + ReviewLevel::Standard, ); let issue = sample_active_issue("In Review"); let tracker = FakeTracker::with_refresh_snapshots( diff --git a/apps/decodex/src/orchestrator/tests/retry/scheduling.rs b/apps/decodex/src/orchestrator/tests/retry/scheduling.rs index 0459d5aec..201931e16 100644 --- a/apps/decodex/src/orchestrator/tests/retry/scheduling.rs +++ b/apps/decodex/src/orchestrator/tests/retry/scheduling.rs @@ -1287,9 +1287,9 @@ fn spawn_sleeping_daemon_child( #[test] fn daemon_tick_reconciles_ready_retained_review_lane_before_dry_run_planning() { let (temp_dir, base_config, workflow) = temp_project_layout(); - let config = service_config_with_external_review_enabled( + let config = service_config_with_review_level( &service_config_with_github_token_env_var(&base_config, "PATH"), - false, + ReviewLevel::Standard, ); let (_path_guard, invocation_log_path) = install_fake_admin_merge_gh_response(&temp_dir); let state_store = StateStore::open_in_memory().expect("state store should open"); @@ -1397,9 +1397,9 @@ fn daemon_tick_reconciles_ready_retained_review_lane_before_dry_run_planning() { #[test] fn daemon_tick_clears_terminal_mapping_without_worktree_before_retained_land() { let (temp_dir, base_config, workflow) = temp_project_layout(); - let config = service_config_with_external_review_enabled( + let config = service_config_with_review_level( &service_config_with_github_token_env_var(&base_config, "PATH"), - false, + ReviewLevel::Standard, ); let (_path_guard, invocation_log_path) = install_fake_admin_merge_gh_response(&temp_dir); let state_store = StateStore::open_in_memory().expect("state store should open"); diff --git a/apps/decodex/src/orchestrator/tests/review_landing/orchestration.rs b/apps/decodex/src/orchestrator/tests/review_landing/orchestration.rs index ea9b5a4cb..51e059470 100644 --- a/apps/decodex/src/orchestrator/tests/review_landing/orchestration.rs +++ b/apps/decodex/src/orchestrator/tests/review_landing/orchestration.rs @@ -426,25 +426,24 @@ fn reconcile_post_review_orchestration_routes_non_clean_landing_to_agent_fallbac #[test] fn reconcile_post_review_orchestration_runs_admin_merge_without_external_review_when_disabled() { assert_reconcile_post_review_orchestration_runs_admin_merge_without_external_review( - InternalReviewMode::Loop, + ReviewLevel::Standard, ); } #[test] -fn reconcile_post_review_orchestration_runs_admin_merge_in_prompt_internal_review_mode() { +fn reconcile_post_review_orchestration_runs_admin_merge_in_basic_review_level() { assert_reconcile_post_review_orchestration_runs_admin_merge_without_external_review( - InternalReviewMode::Prompt, + ReviewLevel::Basic, ); } fn assert_reconcile_post_review_orchestration_runs_admin_merge_without_external_review( - internal_review_mode: InternalReviewMode, + review_level: ReviewLevel, ) { let (temp_dir, config, workflow) = temp_project_layout(); - let config = service_config_with_internal_review_mode(&config, internal_review_mode); - let config = service_config_with_external_review_enabled( + let config = service_config_with_review_level( &service_config_with_github_token_env_var(&config, "PATH"), - false, + review_level, ); let (_path_guard, invocation_log_path) = install_fake_admin_merge_gh_response(&temp_dir); let repo_root = config.repo_root().to_path_buf(); @@ -524,12 +523,12 @@ fn assert_reconcile_post_review_orchestration_runs_admin_merge_without_external_ } #[test] -fn reconcile_post_review_orchestration_routes_internal_review_only_non_clean_landing_to_agent_fallback( +fn reconcile_post_review_orchestration_routes_non_github_review_non_clean_landing_to_agent_fallback( ) { let (temp_dir, config, workflow) = temp_project_layout(); - let config = service_config_with_external_review_enabled( + let config = service_config_with_review_level( &service_config_with_github_token_env_var(&config, "PATH"), - false, + ReviewLevel::Standard, ); let (_path_guard, invocation_log_path) = install_fake_admin_merge_gh_response(&temp_dir); let repo_root = config.repo_root().to_path_buf(); @@ -581,7 +580,7 @@ fn reconcile_post_review_orchestration_routes_internal_review_only_non_clean_lan assert_eq!(marker.phase(), "repair_required"); assert!( !invocation_log_path.exists(), - "non-clean internal-review-only retained landing must not invoke runtime admin merge" + "non-clean non-GitHub-review retained landing must not invoke runtime admin merge" ); assert!(tracker.comments.borrow().is_empty()); assert!(tracker.label_additions.borrow().is_empty()); 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 f4467ca0a..db0061a5b 100644 --- a/apps/decodex/src/orchestrator/tests/review_landing/status_rows.rs +++ b/apps/decodex/src/orchestrator/tests/review_landing/status_rows.rs @@ -135,7 +135,7 @@ fn build_post_review_lane_statuses_preserves_handoff_marker_when_pr_readback_fai #[test] fn build_post_review_lane_statuses_skips_external_review_when_disabled() { let (_temp_dir, config, workflow) = temp_project_layout(); - let config = service_config_with_external_review_enabled(&config, false); + let config = service_config_with_review_level(&config, ReviewLevel::Standard); let repo_root = config.repo_root().to_path_buf(); let issue = sample_issue("In Review", &[]); let tracker = @@ -187,7 +187,7 @@ fn build_post_review_lane_statuses_skips_external_review_when_disabled() { assert_eq!(lanes.len(), 1); assert_eq!(lanes[0].classification, "ready_to_land"); - assert_eq!(lanes[0].reason, "internal_review_only_ready_to_land"); + assert_eq!(lanes[0].reason, "non_github_review_ready_to_land"); assert_eq!(lanes[0].pr_url.as_deref(), Some(pr_url)); } diff --git a/decodex.example.toml b/decodex.example.toml index 67366fb7c..c7e5cfa8f 100644 --- a/decodex.example.toml +++ b/decodex.example.toml @@ -10,10 +10,9 @@ token_env_var = "GITHUB_TOKEN" # command_path = "/opt/homebrew/bin/gh" # Optional Codex-specific runtime policy. -# Omit this block to use `internal_review_mode = "loop"` and `external_review_enabled = true`. +# Omit this block to use `review = "strict"`. [codex] -external_review_enabled = true -internal_review_mode = "loop" +review = "strict" # Phase-scoped app-server goals: "auto" falls back when unsupported, "required" # fails dispatch when goal methods are unavailable, and "off" disables them. goal_support = "auto" diff --git a/docs/runbook/index.md b/docs/runbook/index.md index f2b6d93ea..e7583d98a 100644 --- a/docs/runbook/index.md +++ b/docs/runbook/index.md @@ -46,6 +46,8 @@ Question this index answers: "which sequence should I execute?" explicitly rebinding retained review lanes blocked by a missing or stale runtime DB handoff marker. +- [`review-config-migration.md`](./review-config-migration.md) for one-time migration + from historical review config keys to `[codex].review` levels. - [`research-to-execution-loop.md`](./research-to-execution-loop.md) for compiling latent research contracts, promoting accepted results, inspecting Execution Program queue shaping, and following validation, review, guardrail, and harness feedback. diff --git a/docs/runbook/review-config-migration.md b/docs/runbook/review-config-migration.md new file mode 100644 index 000000000..262d60588 --- /dev/null +++ b/docs/runbook/review-config-migration.md @@ -0,0 +1,79 @@ +# Review Config Migration + +Purpose: One-time manual migration from the historical Decodex review config keys to +the v0.2.0 review-level model. +Use this when: An agent or operator is updating a registered Decodex project +`project.toml` for the Loop Engineering release. +Do not use this for: Changing review behavior in code, adding compatibility parsing, +or mutating operator-local dogfood config before the release flip issue says to do so. +Governing specs: [`../spec/review-orchestration.md`](../spec/review-orchestration.md) +and [`../spec/runtime.md`](../spec/runtime.md). + +## Target Shape + +Every migrated project config uses only this review key: + +```toml +[codex] +review = "off" # or "basic", "standard", "strict" +``` + +The levels mean: + +- `off`: no review gate. +- `basic`: Self Check only. +- `standard`: Self Check plus Decodex Review through `issue_review_checkpoint`. +- `strict`: Standard plus GitHub Review through the existing `@codex review` path. + +## Historical Mapping + +Use the old fields only as migration input: + +| Old fields | New level | +| --- | --- | +| `internal_review_mode = "off"` and `external_review_enabled = false` | `review = "off"` | +| `internal_review_mode = "prompt"` and `external_review_enabled = false` | `review = "basic"` | +| `internal_review_mode = "loop"` and `external_review_enabled = false` | `review = "standard"` | +| `internal_review_mode = "loop"` and `external_review_enabled = true` | `review = "strict"` | + +If an old config combined prompt-only Self Check with GitHub Review, choose the new +level by intent: use `basic` to keep Self Check only, or `strict` to keep the GitHub +Review path. The new model intentionally does not preserve every historical +cross-product. + +## Files To Inspect + +- Registered project config: `~/.codex/decodex/projects//project.toml` +- Registered project workflow: `~/.codex/decodex/projects//WORKFLOW.md` +- Checked-in example: `decodex.example.toml` +- Review specs: `docs/spec/review-orchestration.md`, + `docs/spec/runtime.md`, and `docs/spec/tracker-tools.md` +- Prompt and tracker behavior if changing code: + `apps/decodex/src/orchestrator/prompting.rs` and + `apps/decodex/src/agent/tracker_tool_bridge/` + +## No-Compat Rule + +Do not add or rely on fallback parsing for the old keys. After this migration, the +project config parser should reject unknown historical review fields through the +normal strict TOML schema. + +Do not mutate the operator-local Decodex dogfood config until the new code is merged +and the final dogfood or release issue is ready to flip it. That config currently +belongs to the operator environment, not this source-lane migration. + +## Validation + +After code or checked-in docs change, run: + +```sh +cargo test -p decodex review --all-features +cargo test -p decodex config --all-features +cargo test -p decodex run_and_prompting --all-features +cargo make fmt +cargo make lint-fix +cargo make test +``` + +Before pushing a PR head, the registered project gate is the canonicalize commands +followed by the verify commands from `WORKFLOW.md`. diff --git a/docs/runbook/self-dogfood-pilot.md b/docs/runbook/self-dogfood-pilot.md index eea166647..f0831e19a 100644 --- a/docs/runbook/self-dogfood-pilot.md +++ b/docs/runbook/self-dogfood-pilot.md @@ -108,8 +108,7 @@ api_key_env_var = "LINEAR_API_KEY" token_env_var = "GITHUB_TOKEN" [codex] -internal_review_mode = "prompt" -external_review_enabled = false +review = "basic" goal_support = "auto" # Optional secondary public-projection privacy guard. @@ -130,13 +129,18 @@ Notes: - Decodex does not expose repo-local model or reasoning overrides. `codex app-server` inherits those defaults from `~/.codex/config.toml`. - `api_key_env_var` is required and must name the environment variable that stores the Linear API token. - `github.token_env_var` is required for PR-backed review handoff validation and post-review PR-state inspection and must name the environment variable that stores the GitHub token. -- For the self-dogfood pilot, use `codex.internal_review_mode = "loop"` for the runtime-owned self-review checkpoint loop, `"prompt"` to add only `Review your work repeatedly and fix any logic bugs until no new issues are found.`, or `"off"` to skip internal self-review. If omitted, the default is `"loop"`. -- Keep `codex.external_review_enabled = false` when the retained lane should skip the runtime-owned `@codex review` request and rely on the PR-backed handoff plus the normal PR landing checks. +- For the self-dogfood pilot, use `[codex].review = "basic"` when the lane should + use only Self Check, `"standard"` when it should also require Decodex Review, and + `"strict"` when it should additionally request GitHub Review after PR handoff. + `"off"` skips review gates. If omitted, the default is `"strict"`. - Use `codex.goal_support = "auto"` to let retained lanes use phase-scoped app-server goals when the selected Codex build supports them and fall back safely when it does not. Use `"required"` only for a project that must fail fast when goal methods are absent, and `"off"` to disable goal handling. -- With `codex.external_review_enabled = false`, a one-shot `decodex run` may continue draining the same retained lane after review handoff if the retained landing gates are already satisfied. If those gates are still pending, the run exits cleanly at the retained waiting boundary instead of spinning. +- With non-strict review levels, a one-shot `decodex run` may continue draining the + same retained lane after review handoff if the retained landing gates are already + satisfied. If those gates are still pending, the run exits cleanly at the retained + waiting boundary instead of spinning. - `[privacy_classifier]` is optional and disabled when omitted. If enabled, `endpoint` must be an operator-managed loopback HTTP service; Decodex sends only rendered public projection text fields to it, never private runtime evidence. diff --git a/docs/spec/index.md b/docs/spec/index.md index 75e6aa418..1a8cd82ad 100644 --- a/docs/spec/index.md +++ b/docs/spec/index.md @@ -100,7 +100,7 @@ Then keep the body explicit: - [`owned-lane-policy.md`](./owned-lane-policy.md) defines the fallback policy for Decodex-owned lanes, including manual-intervention and automatic-recovery decisions. - [`review-orchestration.md`](./review-orchestration.md) defines the shared - internal/external review loop, strict external-review signals, round accounting, and + Self Check, Decodex Review, and GitHub Review loop, strict GitHub Review signals, round accounting, and direct landing entry rules. - [`post-review-lifecycle.md`](./post-review-lifecycle.md) defines the normative post- `In Review` lane phases, transitions, and ownership boundaries through landing, diff --git a/docs/spec/installable-agent-policy.md b/docs/spec/installable-agent-policy.md index 395a04582..13cf43cd8 100644 --- a/docs/spec/installable-agent-policy.md +++ b/docs/spec/installable-agent-policy.md @@ -42,7 +42,7 @@ policy. | Working context | Use an isolated task context when the repository or toolchain declares one. | Hardcoded `.worktrees/` layouts or Decodex lane cleanup rules. | | Identity | Derive external-service identity from repo, project, or tool configuration; stop when required identity is missing or contradictory. | Person-to-token maps, workspace names, fallback identities, and exact `GITHUB_*` or `LINEAR_*` variables. | | Validation | Run the repository-declared gate before review handoff, PR-head refresh, or branch-state mutation. | `WORKFLOW.md` frontmatter keys, command lists, or gate-profile selection semantics. | -| Review | Review the current head before handoff or merge and repair verified findings. | Decodex bounded review checkpoint tools, review-round accounting, external-review signals, or landing-entry rules. | +| Review | Review the current head before handoff or merge and repair verified findings. | Decodex bounded review checkpoint tools, review-round accounting, GitHub Review signals, or landing-entry rules. | | Commit messages | Follow the repository's declared commit-message contract. | The `decodex/commit/1` schema when the target repository does not declare it. | | Change control | Do not overwrite unrelated local changes; stop when ownership is unclear. | Retained-lane reconciliation, recovery worktree classification, or closeout cleanup policy. | @@ -54,7 +54,7 @@ policy. | Project execution gates, canonicalization and verification commands, gate profiles, and workspace hooks | [`workflow-file.md`](./workflow-file.md) plus the registered project `WORKFLOW.md` | | Service identity, repo root, worktree root, and tracker or GitHub credential environment-variable names | Centralized project `project.toml`; see the operator surface map in [`../reference/operator-control-plane.md`](../reference/operator-control-plane.md) | | Automatic intake labels, active ownership, retry behavior, and retained lane planning | [`runtime.md`](./runtime.md) and [`owned-lane-policy.md`](./owned-lane-policy.md) | -| Review handoff, bounded independent review, external-review pass signals, repair rounds, and architecture escalation | [`review-orchestration.md`](./review-orchestration.md) and the registered project `WORKFLOW.md` bounded review method | +| Review handoff, bounded independent review, GitHub Review pass signals, repair rounds, and architecture escalation | [`review-orchestration.md`](./review-orchestration.md) and the registered project `WORKFLOW.md` bounded review method | | Post-`In Review` waiting, repair, landing, closeout, cleanup, and manual-intervention phases | [`post-review-lifecycle.md`](./post-review-lifecycle.md) | | Local commit-message schema for Decodex-managed history | [`commit-messages.md`](./commit-messages.md) | | Operator procedures, pilot setup, and live validation steps | [`../runbook/index.md`](../runbook/index.md) and the specific runbook for the procedure | diff --git a/docs/spec/loop-runtime.md b/docs/spec/loop-runtime.md index 84e4bd9df..36855b1d8 100644 --- a/docs/spec/loop-runtime.md +++ b/docs/spec/loop-runtime.md @@ -275,8 +275,8 @@ read-only review pass. This pass is distinct from in-thread self-review: Contract - candidate findings must be validated before repair work changes the lane -The review orchestration contract, including internal/external review modes and -review-stop classes, is defined by [`review-orchestration.md`](./review-orchestration.md). +The review orchestration contract, including review levels and review-stop classes, +is defined by [`review-orchestration.md`](./review-orchestration.md). ## Unattended Execution diff --git a/docs/spec/post-review-lifecycle.md b/docs/spec/post-review-lifecycle.md index a2aeeaf42..e6d43c201 100644 --- a/docs/spec/post-review-lifecycle.md +++ b/docs/spec/post-review-lifecycle.md @@ -23,7 +23,11 @@ Defines: Post-`In Review` lane phases, phase-to-action-class mapping, authoritat Linear comment event-ledger schema used by post-review handoff, repair, landing, closeout, and cleanup records. - [`owned-lane-policy.md`](./owned-lane-policy.md) defines the allowed action classes and the fallback policy for waiting, repair re-entry, landing readiness, automatic recovery, and manual intervention. -- [`review-orchestration.md`](./review-orchestration.md) defines the shared internal/external review loop, the optional service-level external-review toggle, strict external-review request and pass signals when that loop is enabled, round accounting, and the rule that external pass flows into Decodex-directed admin merge instead of a separate manual landing request. +- [`review-orchestration.md`](./review-orchestration.md) defines the shared Self + Check, Decodex Review, and GitHub Review loop, strict GitHub Review request and + pass signals when that level is enabled, round accounting, and the rule that + GitHub Review pass flows into Decodex-directed admin merge instead of a separate + manual landing request. - This document narrows those action classes into the specific post-`In Review` lane phases and transitions that Decodex must honor after review handoff succeeds. ## Core invariants @@ -174,12 +178,14 @@ While in `review_repair`: - the runtime must reuse the retained lane when it is still valid - repair work must stay bound to the same issue, branch lineage, and PR -- the runtime must validate each external-review claim against the codebase, tests, and requirements before changing code +- the runtime must validate each GitHub Review claim against the codebase, tests, and requirements before changing code - when a fresh-context `issue_review_checkpoint` exists for the repair phase, repair work must operate on accepted findings from that checkpoint; rejected or non-actionable comments remain evidence, not repair scope - the repaired head must pass the local pre-review gate before being pushed -- when `codex.internal_review_mode = "loop"`, every repaired-head bounded-review result must first be recorded through `issue_review_checkpoint` +- when `[codex].review` is `"standard"` or `"strict"`, every repaired-head + bounded Decodex Review result must first be recorded through + `issue_review_checkpoint` - every addressed review thread must receive an in-thread reply for the repaired head - only threads whose landed fix is verified on that repaired head may be resolved; pushback or clarification threads stay open - once a new head is pushed and fresh review is requested on the same PR, the lane returns to `review_wait` for that new head @@ -191,14 +197,15 @@ In the current XY-174 slice, a retained repair run finishes by recording an expl the `review_repair` terminal path. Applying that completion refreshes the local runtime handoff row to the repaired PR head while keeping the tracker issue in `In Review`; it does not re-run the original `issue_review_handoff` state transition. -When `codex.internal_review_mode = "loop"`, `issue_review_repair_complete` is valid only -when the latest retained repair checkpoint is `clean` for the current repaired head. If -repeated repair checkpoints stay in `findings` for three consecutive rounds, or the -checkpoint reports `needs_architecture_review` / `blocked`, the runtime must stop for -human intervention instead of patch-on-patch churn. When -`codex.internal_review_mode = "prompt"` or `"off"`, retained repair completion skips that -independent-review checkpoint requirement but still requires the repaired head to be pushed and -the configured repository validation gate to pass. +When `[codex].review` is `"standard"` or `"strict"`, +`issue_review_repair_complete` is valid only when the latest retained repair +checkpoint is `clean` for the current repaired head. If repeated repair checkpoints +stay in `findings` for three consecutive rounds, or the checkpoint reports +`needs_architecture_review` / `blocked`, the runtime must stop for human +intervention instead of patch-on-patch churn. When `[codex].review` is `"off"` or +`"basic"`, retained repair completion skips that Decodex Review checkpoint +requirement but still requires the repaired head to be pushed and the configured +repository validation gate to pass. The same completion also requires that the PR still belongs to the retained lane, points at the repaired lane HEAD, and remains open and ready for fresh review. diff --git a/docs/spec/review-orchestration.md b/docs/spec/review-orchestration.md index 98ac2f988..465b5ec59 100644 --- a/docs/spec/review-orchestration.md +++ b/docs/spec/review-orchestration.md @@ -2,9 +2,9 @@ Purpose: Define the normative review orchestration contract that sits above runtime-native review handoff, retained review repair, and landing. Status: normative -Read this when: You are implementing or reviewing how a Decodex-owned lane requests internal or external review, counts review rounds, reacts to review results, or transitions from review pass into handoff or landing. +Read this when: You are implementing or reviewing how a Decodex-owned lane requests Self Check, Decodex Review, or GitHub Review, counts review rounds, reacts to review results, or transitions from review pass into handoff or landing. Not this document: The low-level app-server protocol, the post-`In Review` lane phase model, the tracker tool schema, or local skill payloads. -Defines: Shared review-loop semantics, reviewer-source-specific rules, strict external-review adapter signals, review-round accounting, architecture-escalation rules, landing entry requirements, and manual-intervention boundaries. +Defines: Shared review-loop semantics, reviewer-source-specific rules, strict GitHub Review adapter signals, review-round accounting, architecture-escalation rules, landing entry requirements, and manual-intervention boundaries. ## Implementation note @@ -29,28 +29,32 @@ There is one shared review loop for Decodex-owned lanes: 3. validate and repair the actionable findings for the current lane head 4. request review again until the lane passes or stops for escalation -Internal review behavior is selected per service through the registered project config -`[codex].internal_review_mode`. The supported modes are `"loop"`, `"prompt"`, and -`"off"`. -External GitHub review may be enabled or disabled per service through the registered project -config `[codex].external_review_enabled`. When enabled, each review source participates in the -retained review loop. When external review is disabled, Decodex skips the runtime-owned `@codex -review` request and may land directly from the retained lane once the normal landing gates are -satisfied and the PR is already on the deterministic clean merge path. +Review behavior is selected per service through the registered project config +`[codex].review`. The supported levels are `"off"`, `"basic"`, `"standard"`, and +`"strict"`. -When external review is enabled, the difference is reviewer source: +The levels map to review sources as follows: -- Internal review in `"loop"` mode uses a runtime-controlled independent - fresh-context read-only Codex review checkpoint for the current lane head. -- Internal self-review in `"prompt"` mode injects only the prompt `Review your work repeatedly and fix any logic bugs until no new issues are found.` and does not expose or require the checkpoint tool. -- External review uses a GitHub PR comment request by posting `@codex review` on the current PR. +- `off`: no review gate. +- `basic`: Self Check only. The agent reviews its own work repeatedly and fixes + obvious issues before handoff. +- `standard`: Self Check plus Decodex Review. Decodex exposes and requires the + runtime-owned independent fresh-context `issue_review_checkpoint` gate. +- `strict`: Standard plus GitHub Review. After Decodex Review and PR-backed + handoff, Decodex uses the runtime-owned GitHub `@codex review` path where the + existing adapter supports it. + +When the configured level does not include GitHub Review, Decodex skips the +runtime-owned `@codex review` request and may land directly from the retained lane +once the normal landing gates are satisfied and the PR is already on the +deterministic clean merge path. ## Shared invariants 1. Review always applies to the current lane head, not an older remembered implementation state. 2. While a review request is outstanding, the lane itself must not push unrelated new commits. 3. If PR state, branch lineage, or retained-lane ownership changes externally while review is pending, the lane must stop for `manual_intervention_required` instead of trying to recover automatically. -4. Internal and external review rounds are counted independently. +4. Decodex Review and GitHub Review rounds are counted independently. 5. Review pass is fail-closed. If the expected pass signals are missing, contradictory, or ambiguous, the lane must stop for manual intervention instead of guessing success. ## Shared repair rules @@ -61,7 +65,7 @@ After any review arrives: - repair only the verified issues - keep the repair batch scoped to the smallest coherent owned change set - rerun the repository validation required for the current head before the next review request -- when `codex.internal_review_mode = "loop"`, record the normalized bounded-review +- when `[codex].review` is `"standard"` or `"strict"`, record the normalized Decodex Review result for the exact current `HEAD` through `issue_review_checkpoint`, including the explicit independent reviewer source, checklist notes, accepted findings, rejected findings, non-empty evidence, and repair guidance @@ -81,51 +85,59 @@ Rules: - A resend caused by missing acknowledgement is a retry of the current request, not a new review round. - A review round does not complete until the lane either requests the next review or stops for escalation. -- The first three review results in the same review mode consume the normal convergence budget. -- When the fourth review result arrives in the same review mode, the lane must request an architecture check: +- The first three review results from the same review source consume the normal convergence budget. +- When the fourth review result arrives from the same review source, the lane must request an architecture check: - if the repeated churn is rooted in an architectural defect or root-cause issue that local patching will not converge, stop for `manual_intervention_required` - - if the findings are normal and not rooted in architectural churn, continue and reset that review mode's three-round budget + - if the findings are normal and not rooted in architectural churn, continue and reset that review source's three-round budget -## Internal review modes +## Review levels -Internal review mode is service-controlled. +Review level is service-controlled. Rules: -- `codex.internal_review_mode = "loop"` uses the runtime-owned independent - fresh-context read-only review checkpoint loop. Decodex exposes +- `[codex].review = "off"` skips Self Check, Decodex Review, and GitHub Review. + Decodex does not expose `issue_review_checkpoint`, does not require a clean + checkpoint before handoff or repair completion, and ignores stale review-policy + checkpoint state for turn-stop classification. +- `[codex].review = "basic"` injects the Self Check instruction and does not expose + or require the checkpoint tool. It does not use GitHub Review. +- `[codex].review = "standard"` uses Self Check plus the runtime-owned independent + fresh-context read-only Decodex Review checkpoint loop. Decodex exposes `issue_review_checkpoint`, requires a current `clean` checkpoint before `issue_review_handoff` or `issue_review_repair_complete`, stores structured accepted/rejected finding evidence, and applies the review-policy stop rules to - stale or non-clean checkpoint state. -- `codex.internal_review_mode = "prompt"` injects exactly the prompt `Review your work repeatedly and fix any logic bugs until no new issues are found.` into the agent instructions. Decodex does not expose `issue_review_checkpoint`, does not require a clean checkpoint before handoff or repair completion, and ignores stale review-policy checkpoint state for turn-stop classification. -- `codex.internal_review_mode = "off"` skips internal review. Decodex does not expose `issue_review_checkpoint`, does not require a clean checkpoint before handoff or repair completion, and ignores stale review-policy checkpoint state for turn-stop classification. -- Omitted `codex.internal_review_mode` defaults to `"loop"`. `codex.internal_review_enabled` is rejected. -- In `"loop"` mode, the runtime may choose the exact local transport or + stale or non-clean checkpoint state. It does not use GitHub Review. +- `[codex].review = "strict"` uses the standard requirements and then participates + in the GitHub Review loop. +- Omitted `[codex].review` defaults to `"strict"`. +- In `"standard"` and `"strict"` levels, the runtime may choose the exact local transport or child-conversation mechanism, but it must remain a fully runtime-controlled read-only review request. The reviewer must not edit files, push, land, or mutate tracker state. -- In `"loop"` mode, internal review must use the same bounded review method and normalized review outcomes as any other review pass. -- In `"loop"` mode, a `findings` checkpoint requires at least one accepted finding; +- In `"standard"` and `"strict"` levels, Decodex Review must use the same bounded review method and normalized review outcomes as any other review pass. +- In `"standard"` and `"strict"` levels, a `findings` checkpoint requires at least one accepted finding; rejected or non-actionable reviewer comments may be recorded with a `clean` checkpoint and must not become repair input. -- If `"loop"` mode internal review returns an ambiguous or contradictory result that the runtime cannot classify without guessing, stop for `manual_intervention_required`. -- Internal review pass transitions into the normal PR-backed review handoff flow, not directly into landing. -- Prompt-only self-review is not sufficient when the loop-runtime risk policy - requires independent review. Use the loop-mode independent checkpoint boundary +- If Decodex Review returns an ambiguous or contradictory result that the runtime + cannot classify without guessing, stop for `manual_intervention_required`. +- Decodex Review pass transitions into the normal PR-backed review handoff flow, not + directly into landing. +- Self Check is not sufficient when the loop-runtime risk policy + requires independent review. Use the Decodex Review checkpoint boundary before treating the lane as ready for handoff or landing. -## External GitHub review +## GitHub Review -External review is adapter-driven and uses strict observable GitHub signals. +GitHub Review is adapter-driven and uses strict observable GitHub signals. ### Request and acknowledgement -The external review request is made by posting `@codex review` on the current PR. +The GitHub Review request is made by posting `@codex review` on the current PR. The request is accepted only when that exact request comment receives an `eyes` reaction from the `codex` reviewer actor. -Before every external review request, the current PR head must already have green required CI or +Before every GitHub Review request, the current PR head must already have green required CI or required checks. Rules: @@ -133,18 +145,18 @@ Rules: - If required CI or required checks are still pending, do not post a review request yet. Stay in the retained lane and wait for green. - If required CI is red in a retained-repair class the runtime already knows how to handle, return - to retained repair first and request external review only after the repaired head becomes green. + to retained repair first and request GitHub Review only after the repaired head becomes green. - If required CI is red in a way the runtime cannot classify or repair without guessing, stop for `manual_intervention_required` instead of posting the review request anyway. -- If no `eyes` reaction appears within one minute, resend the external review request exactly once. +- If no `eyes` reaction appears within one minute, resend the GitHub Review request exactly once. - That resend is a retry of the same request, not a new review round. -- Treat the lane as having only one outstanding external review request at a time. +- Treat the lane as having only one outstanding GitHub Review request at a time. - If the resent request still does not receive `eyes` within one minute, stop for `manual_intervention_required`. -- Once the `eyes` signal is observed, poll until external review arrives or manual intervention becomes required. +- Once the `eyes` signal is observed, poll until GitHub Review arrives or manual intervention becomes required. ### Read surface -External review processing must read all relevant GitHub review surfaces, not only inline threads: +GitHub Review processing must read all relevant GitHub review surfaces, not only inline threads: - review summaries and overall review body - inline review comments and review threads @@ -154,16 +166,16 @@ External review processing must read all relevant GitHub review surfaces, not on ### Strict pass signal -External review passes only when both of these exact signals are present: +GitHub Review passes only when both of these exact signals are present: - review content authored by the `codex` reviewer actor is exactly the standalone text `Didn't find any major issues.` after trimming surrounding whitespace - the PR description currently has a `thumbs-up` reaction from the `codex` reviewer actor -No fuzzy fallback, semantic-equivalence check, or alternative wording is allowed. If those signals do not appear exactly, external review does not pass automatically and the lane must stop for `manual_intervention_required`. +No fuzzy fallback, semantic-equivalence check, or alternative wording is allowed. If those signals do not appear exactly, GitHub Review does not pass automatically and the lane must stop for `manual_intervention_required`. -## External repair and thread resolution +## GitHub Review repair and thread resolution -After external review returns findings: +After GitHub Review returns findings: - validate the findings against the current lane head before changing code - repair only the verified issues @@ -181,18 +193,26 @@ Pushback or clarification threads stay open. The next step after review pass depends on reviewer source. -### After internal review pass +### After Decodex Review pass - continue the normal PR-backed review handoff flow - create or refresh the non-draft PR for the current lane head if needed -- when `codex.internal_review_mode = "loop"`, record `issue_review_handoff` only after the latest bounded-review result for that handoff phase and current `HEAD` is `clean` -- when `codex.internal_review_mode = "prompt"` or `"off"`, record `issue_review_handoff` after the branch is pushed, the non-draft PR is ready, and required validation has passed -- if `codex.external_review_enabled = false`, treat that PR-backed handoff as sufficient review input for retained landing and do not post `@codex review` -- after a successful external-review-disabled handoff, the same top-level `decodex run` may keep draining the same retained lane through retained landing, closeout, and deterministic cleanup until the lane reaches a stable waiting state or finishes that tail work +- when `[codex].review` is `"standard"` or `"strict"`, record `issue_review_handoff` + only after the latest bounded-review result for that handoff phase and current + `HEAD` is `clean` +- when `[codex].review` is `"off"` or `"basic"`, record `issue_review_handoff` + after the branch is pushed, the non-draft PR is ready, and required validation has + passed +- if `[codex].review` is not `"strict"`, treat that PR-backed handoff as sufficient + review input for retained landing and do not post `@codex review` +- after a successful non-strict handoff, the same top-level `decodex run` may keep + draining the same retained lane through retained landing, closeout, and + deterministic cleanup until the lane reaches a stable waiting state or finishes + that tail work - direct runtime merge is limited to the clean path; if branch sync, conflict resolution, ambiguous mergeability, or repository-specific recovery is still required, re-enter the retained agent path first - if retained checks are still pending or merge visibility is not yet authoritative, stop the same run cleanly at that waiting boundary instead of busy-waiting indefinitely -### After external review pass +### After GitHub Review pass - execute the same PR's GitHub admin merge directly only on the deterministic clean path - do not require a separate human merge step first when the clean-path preconditions are already satisfied @@ -233,10 +253,10 @@ Exact ownership boundaries for closeout and cleanup remain governed by [`post-re The lane must stop for `manual_intervention_required` when any of these occur: -- external PR, branch, or lineage changes while review is pending -- internal review returns an ambiguous result -- external review acknowledgement never appears within the allowed resend budget -- external pass signals do not match the strict required pair exactly +- GitHub PR, branch, or lineage changes while review is pending +- Decodex Review returns an ambiguous result +- GitHub Review acknowledgement never appears within the allowed resend budget +- GitHub Review pass signals do not match the strict required pair exactly - admin merge is unsupported for the repository - merged PR visibility does not arrive within the configured polling ceiling - architecture check concludes that repeated review churn is rooted in a non-converging architectural defect @@ -301,7 +321,7 @@ structured architecture or convergence stop. 1. Post `@codex review` on the current PR. 2. Observe `eyes` on that exact comment within one minute. -3. Wait for the external review result. +3. Wait for the GitHub Review result. 4. Confirm the review content is exactly `Didn't find any major issues.` after trimming surrounding whitespace. 5. Confirm the PR description currently has a `thumbs-up` from the `codex` reviewer actor. 6. Run admin merge after landing gates are satisfied. diff --git a/docs/spec/runtime.md b/docs/spec/runtime.md index 5e122178a..c78a1524b 100644 --- a/docs/spec/runtime.md +++ b/docs/spec/runtime.md @@ -354,7 +354,9 @@ with no changing tracked delta record `no_effective_diff`. Fingerprints use the HEAD plus the tracked worktree status and diff against `HEAD`, so retained partial progress remains inspectable instead of being deleted or hidden. -When `codex.internal_review_mode = "loop"`, handoff and retained review-repair runs also consume the latest structured `issue_review_checkpoint` state for the current phase and current lane head from the owned lane: +When `[codex].review` is `"standard"` or `"strict"`, handoff and retained +review-repair runs also consume the latest structured `issue_review_checkpoint` +state for the current phase and current lane head from the owned lane: - no checkpoint and no terminal path: allow a clean continuation boundary - latest checkpoint `clean` and no terminal path: allow continuation so the agent can finish handoff or repair completion @@ -380,7 +382,7 @@ always wins over stale marker values. Legacy marker recovery seeds an empty `details_json` payload because old markers did not carry structured finding evidence. Recording `issue_review_handoff` or `issue_review_repair_complete` clears the runtime checkpoint for that run attempt and only best-effort clears any legacy marker fields. -When `codex.internal_review_mode = "prompt"` or `"off"`, Decodex does not expose +When `[codex].review` is `"off"` or `"basic"`, Decodex does not expose `issue_review_checkpoint`, does not require a clean checkpoint before review handoff or repair completion, and ignores stale review-policy state while classifying clean turn boundaries. @@ -492,7 +494,7 @@ mutations, or duplicate comment for that logical event. The local runtime store is the global Decodex SQLite database for one local installation. It lives at `~/.codex/decodex/runtime.sqlite3`, not inside any registered project checkout or worktree. Every row that belongs to a repo is scoped by `project_id`. Decodex logs live beside that database under `~/.codex/decodex/logs/`, the optional shared Codex account pool lives at `~/.codex/decodex/accounts.jsonl`, global operator config lives at `~/.codex/decodex/config.toml`, bounded local account usage estimates live at `~/.codex/decodex/account-usage-history.jsonl`, and agent-readable derived evidence lives under `~/.codex/decodex/agent-evidence//`; vendor-qualified app-data directories and per-project runtime databases are not part of the runtime contract. Global operator config owns account-pool routing and shared account display-name offsets. Account usage history owns local seven-day display estimates and non-secret account capacity weights only; it does not contain token material and does not decide scheduling. UI-only preferences such as theme, table sorting, and local privacy visibility are not runtime state. -Project contracts live outside registered repositories under `~/.codex/decodex/projects//`. Each project directory must contain `project.toml` and `WORKFLOW.md`; arbitrary project file names such as `.toml` are not part of the contract. `project.toml` must set `[paths].repo_root` so the project contract is explicit. The `[github]` table owns the routed token environment variable and may also set `command_path` when the expected `gh` binary should be explicit for GUI-launched runs. The `[codex]` table owns app-server-adjacent runtime policy such as `internal_review_mode`, `external_review_enabled`, and `goal_support`. `goal_support` accepts `"auto"`, `"required"`, and `"off"` and defaults to `"auto"` when omitted. Project registration stores the centralized `config_path`, target `repo_root`, `worktree_root`, and workflow path in the global runtime database. Commands that start inside a registered checkout or lane worktree resolve the project through that registry; they do not discover or trust worktree-local config files. Project config refreshes preserve an existing enabled or disabled registry toggle; only explicit operator commands such as `decodex project add `, `decodex project enable `, and `decodex project disable ` may change that toggle. `decodex serve` schedules and polls enabled registered projects from the global runtime database; the operator and App projections must still expose active runtime DB-backed attempts for disabled projects because pause is a future-dispatch control, not a visibility or ownership deletion. It must not scan `.codex` history, repo-local config files, or currently open worktrees to infer additional projects. +Project contracts live outside registered repositories under `~/.codex/decodex/projects//`. Each project directory must contain `project.toml` and `WORKFLOW.md`; arbitrary project file names such as `.toml` are not part of the contract. `project.toml` must set `[paths].repo_root` so the project contract is explicit. The `[github]` table owns the routed token environment variable and may also set `command_path` when the expected `gh` binary should be explicit for GUI-launched runs. The `[codex]` table owns app-server-adjacent runtime policy such as `review` and `goal_support`. `review` accepts `"off"`, `"basic"`, `"standard"`, and `"strict"` and defaults to `"strict"` when omitted. `goal_support` accepts `"auto"`, `"required"`, and `"off"` and defaults to `"auto"` when omitted. Project registration stores the centralized `config_path`, target `repo_root`, `worktree_root`, and workflow path in the global runtime database. Commands that start inside a registered checkout or lane worktree resolve the project through that registry; they do not discover or trust worktree-local config files. Project config refreshes preserve an existing enabled or disabled registry toggle; only explicit operator commands such as `decodex project add `, `decodex project enable `, and `decodex project disable ` may change that toggle. `decodex serve` schedules and polls enabled registered projects from the global runtime database; the operator and App projections must still expose active runtime DB-backed attempts for disabled projects because pause is a future-dispatch control, not a visibility or ownership deletion. It must not scan `.codex` history, repo-local config files, or currently open worktrees to infer additional projects. `project.toml` may also configure `[privacy_classifier]` with a loopback HTTP `endpoint` and bounded `timeout_ms` for an operator-managed local classifier runtime. diff --git a/docs/spec/tracker-tools.md b/docs/spec/tracker-tools.md index d61dc9b14..c3f45eaed 100644 --- a/docs/spec/tracker-tools.md +++ b/docs/spec/tracker-tools.md @@ -142,7 +142,9 @@ In either invalid case, `decodex` must fail the attempt rather than infer which public lifecycle signal changes materially, such as the normalized phase or public branch/PR projection anchor changing. Repeated private evidence updates inside the same public signal must append private runtime events without adding Linear comments. -- `issue_review_checkpoint` is available only when `codex.internal_review_mode = "loop"`, and only during the pre-PR handoff phase and retained review-repair runs; `closeout` does not expose it. +- `issue_review_checkpoint` is available only when `[codex].review` is `"standard"` + or `"strict"`, and only during the pre-PR handoff phase and retained + review-repair runs; `closeout` does not expose it. - `issue_review_checkpoint` must accept only these normalized statuses: `clean`, `findings`, `needs_architecture_review`, `blocked`. - `issue_review_checkpoint` must bind every checkpoint to an explicit `head_sha` @@ -162,9 +164,17 @@ In either invalid case, `decodex` must fail the attempt rather than infer which non-empty evidence, file and line references when possible, and concrete repair guidance. Rejected findings must include severity, non-empty evidence, and the rejection reason. -- When `codex.internal_review_mode = "loop"`, `decodex` treats `issue_review_checkpoint` as the only authoritative structured review-policy signal. Skill prose or wrapper-local result words must not replace it. -- When `codex.internal_review_mode = "loop"`, `issue_review_handoff` and `issue_review_repair_complete` must require the latest `clean` checkpoint for the current phase and current lane head, not merely any older clean checkpoint from the same lane. -- When `codex.internal_review_mode = "prompt"` or `"off"`, `issue_review_handoff` and `issue_review_repair_complete` must not require `issue_review_checkpoint`; they still must pass PR validation, branch/head checks, and the configured repository validation gate before writeback. +- When `[codex].review` is `"standard"` or `"strict"`, `decodex` treats + `issue_review_checkpoint` as the only authoritative structured review-policy + signal. Skill prose or wrapper-local result words must not replace it. +- When `[codex].review` is `"standard"` or `"strict"`, `issue_review_handoff` and + `issue_review_repair_complete` must require the latest `clean` checkpoint for the + current phase and current lane head, not merely any older clean checkpoint from the + same lane. +- When `[codex].review` is `"off"` or `"basic"`, `issue_review_handoff` and + `issue_review_repair_complete` must not require `issue_review_checkpoint`; they + still must pass PR validation, branch/head checks, and the configured repository + validation gate before writeback. - `issue_review_handoff` must validate that the supplied PR belongs to the current repository and lane branch, points at the validated lane HEAD, is open, and is ready for review before `decodex` accepts the handoff. - `issue_review_repair_complete` must validate that the supplied PR belongs to the current repository and retained lane branch, points at the validated lane HEAD, is open, and is ready for fresh review before `decodex` accepts retained repair completion. - `issue_review_handoff` records the success metadata during the turn, but `decodex` owns the final completion comment and `In Review` transition after service-side validation succeeds. @@ -229,7 +239,11 @@ In either invalid case, `decodex` must fail the attempt rather than infer which - If the agent never reaches a tracker write, `decodex` may perform a minimal fallback write during reconciliation or terminal failure handling. - If a tracker tool call fails transiently, the failure should be surfaced to the run journal so retry logic can reason about it. - If a tracker tool call fails because it targeted the wrong issue or an unsupported operation, treat that as a policy violation, not as a retryable transport error. -- When `codex.internal_review_mode = "loop"`, if the latest `issue_review_checkpoint` reports `findings` for the third consecutive non-clean round on the same phase, or reports `needs_architecture_review` / `blocked`, `decodex` must stop the lane through the human-required failure path instead of retrying automatically. +- When `[codex].review` is `"standard"` or `"strict"`, if the latest + `issue_review_checkpoint` reports `findings` for the third consecutive non-clean + round on the same phase, or reports `needs_architecture_review` / `blocked`, + `decodex` must stop the lane through the human-required failure path instead of + retrying automatically. - Review-policy stops do not dispatch research directly. `decodex` may surface operator guidance for a bounded research follow-up, but future runtime-owned research escalation is valid only after a separate adapter contract can verify the current diff --git a/docs/spec/workflow-file.md b/docs/spec/workflow-file.md index 49cffd432..16325e015 100644 --- a/docs/spec/workflow-file.md +++ b/docs/spec/workflow-file.md @@ -120,10 +120,10 @@ Removed fields: Child-run execution policy is not part of the project-owned workflow contract. `decodex` must let `codex app-server` inherit sandbox and approval behavior from the active Codex runtime instead of declaring repo-local overrides in `WORKFLOW.md`. -Codex app-server-adjacent runtime settings such as `codex.goal_support`, -`codex.internal_review_mode`, and `codex.external_review_enabled` belong to the -centralized project `project.toml`, not to `WORKFLOW.md`. `WORKFLOW.md` still owns -the bounded turn budget and repo gate used after a phase goal completes. +Codex app-server-adjacent runtime settings such as `codex.goal_support` and +`codex.review` belong to the centralized project `project.toml`, not to +`WORKFLOW.md`. `WORKFLOW.md` still owns the bounded turn budget and repo gate used +after a phase goal completes. ## `[execution]`