From 582f79b9078e67edd04d6e4620fce9ee4870d38b Mon Sep 17 00:00:00 2001 From: Yvette Carlisle Date: Wed, 17 Jun 2026 12:43:49 +0800 Subject: [PATCH 1/2] {"schema":"decodex/commit/1","summary":"Replace retained review markers with lifecycle record","authority":"XY-972"} --- apps/decodex/src/cli.rs | 8 +- apps/decodex/src/maintenance.rs | 87 ++-- apps/decodex/src/recovery.rs | 48 +- apps/decodex/src/state/internal.rs | 583 ++++++++++++++--------- apps/decodex/src/state/models.rs | 128 ++++- apps/decodex/src/state/store.rs | 305 ++++++++---- apps/decodex/src/state/tests.rs | 375 +++++++++++++-- docs/reference/operator-control-plane.md | 33 +- docs/reference/test-suite.md | 8 +- docs/runbook/index.md | 2 +- docs/runbook/lane-control-recovery.md | 4 +- docs/runbook/recover-review-handoff.md | 65 +-- docs/spec/lane-control.md | 6 +- docs/spec/linear-execution-ledger.md | 14 +- docs/spec/post-review-lifecycle.md | 118 +++-- docs/spec/runtime.md | 76 ++- 16 files changed, 1290 insertions(+), 570 deletions(-) diff --git a/apps/decodex/src/cli.rs b/apps/decodex/src/cli.rs index 3a811f0e..bb95a11a 100644 --- a/apps/decodex/src/cli.rs +++ b/apps/decodex/src/cli.rs @@ -317,7 +317,7 @@ struct LandCommand { #[arg(long, conflicts_with = "authority", requires = "pr")] manual_authority: bool, /// Pull request URL to land. Required with `--manual-authority`; otherwise defaults to the - /// current review handoff marker. + /// current review lifecycle record. #[arg(long, value_name = "URL")] pr: Option, /// Additional related issues for the landed change record. @@ -1009,7 +1009,7 @@ struct ReviewHandoffRebindCommand { /// Pull request URL to bind after validation. #[arg(long, value_name = "URL")] pr: String, - /// Validate only; do not write runtime markers or tracker audit comments. + /// Validate only; do not write runtime lifecycle state or tracker audit comments. #[arg(long)] dry_run: bool, } @@ -1022,7 +1022,7 @@ struct ReviewHandoffAdoptCommand { /// Pull request URL to adopt after validation. #[arg(long, value_name = "URL")] pr: String, - /// Validate only; do not write runtime markers or tracker audit comments. + /// Validate only; do not write runtime lifecycle state or tracker audit comments. #[arg(long)] dry_run: bool, } @@ -1833,7 +1833,7 @@ enum ResearchOutcomeArg { #[derive(Debug, Subcommand)] enum RecoverSubcommand { - /// Recover retained review lanes whose handoff marker is missing. + /// Recover retained review lanes whose lifecycle record is missing. ReviewHandoff(ReviewHandoffRecoveryCommand), /// Record an audited fallback closeout for a legacy cleanup-only worktree. LegacyCloseout(LegacyCloseoutRecoveryCommand), diff --git a/apps/decodex/src/maintenance.rs b/apps/decodex/src/maintenance.rs index 4f4db6bb..b76fc7bc 100644 --- a/apps/decodex/src/maintenance.rs +++ b/apps/decodex/src/maintenance.rs @@ -556,7 +556,7 @@ fn maintain_runtime_protocol_events( event_count: candidate.event_count, last_event_at: candidate.last_event_at.clone(), reason: format!( - "terminal run has no run lease, retained worktree, or review marker and its latest protocol event is older than {} days", + "terminal run has no run lease, retained worktree, or review lifecycle record and its latest protocol event is older than {} days", policy.protocol_event_retention_days ), }); @@ -633,9 +633,8 @@ fn protocol_event_compaction_candidates( AND last.sequence_number = totals.last_sequence_number LEFT JOIN leases run_lease ON run_lease.issue_id = attempts.issue_id LEFT JOIN worktrees retained_worktree ON retained_worktree.issue_id = attempts.issue_id - LEFT JOIN review_handoffs review_handoff ON review_handoff.issue_id = attempts.issue_id - LEFT JOIN review_orchestrations review_orchestration - ON review_orchestration.issue_id = attempts.issue_id + LEFT JOIN review_lifecycle_records review_lifecycle + ON review_lifecycle.issue_id = attempts.issue_id LEFT JOIN ( SELECT issue_id, @@ -650,8 +649,7 @@ fn protocol_event_compaction_candidates( AND totals.last_created_at_unix < ?1 AND run_lease.issue_id IS NULL AND retained_worktree.issue_id IS NULL - AND review_handoff.issue_id IS NULL - AND review_orchestration.issue_id IS NULL + AND review_lifecycle.issue_id IS NULL AND human_stop_event.run_id IS NULL ORDER BY totals.last_created_at_unix ASC, attempts.run_id ASC", )?; @@ -683,9 +681,8 @@ fn protected_protocol_run_count(connection: &Connection) -> Result { JOIN protocol_events events ON events.run_id = attempts.run_id LEFT JOIN leases run_lease ON run_lease.issue_id = attempts.issue_id LEFT JOIN worktrees retained_worktree ON retained_worktree.issue_id = attempts.issue_id - LEFT JOIN review_handoffs review_handoff ON review_handoff.issue_id = attempts.issue_id - LEFT JOIN review_orchestrations review_orchestration - ON review_orchestration.issue_id = attempts.issue_id + LEFT JOIN review_lifecycle_records review_lifecycle + ON review_lifecycle.issue_id = attempts.issue_id LEFT JOIN ( SELECT issue_id, @@ -698,8 +695,7 @@ fn protected_protocol_run_count(connection: &Connection) -> Result { AND human_stop_event.run_id = attempts.run_id WHERE run_lease.issue_id IS NOT NULL OR retained_worktree.issue_id IS NOT NULL - OR review_handoff.issue_id IS NOT NULL - OR review_orchestration.issue_id IS NOT NULL + OR review_lifecycle.issue_id IS NOT NULL OR human_stop_event.run_id IS NOT NULL OR attempts.status NOT IN ('succeeded', 'failed', 'interrupted', 'terminated')", [], @@ -996,7 +992,7 @@ mod tests { recorded_at TEXT NOT NULL, recorded_at_unix INTEGER NOT NULL ); - CREATE TABLE review_handoffs ( + CREATE TABLE review_lifecycle_records ( project_id TEXT NOT NULL, issue_id TEXT NOT NULL, branch_name TEXT NOT NULL, @@ -1006,17 +1002,6 @@ mod tests { target_base_ref_name TEXT, pr_head_ref_name TEXT NOT NULL, pr_head_oid TEXT NOT NULL, - updated_at TEXT NOT NULL, - updated_at_unix INTEGER NOT NULL, - PRIMARY KEY (project_id, issue_id, branch_name) - ); - CREATE TABLE review_orchestrations ( - project_id TEXT NOT NULL, - issue_id TEXT NOT NULL, - branch_name TEXT NOT NULL, - run_id TEXT NOT NULL, - attempt_number INTEGER NOT NULL, - pr_url TEXT NOT NULL, head_sha TEXT NOT NULL, phase TEXT NOT NULL, request_comment_database_id INTEGER, @@ -1025,9 +1010,14 @@ mod tests { request_retry_count INTEGER NOT NULL, external_round_count INTEGER NOT NULL, auto_merge_enabled_at_unix_epoch INTEGER, + landing_state TEXT NOT NULL DEFAULT 'not_started', + closeout_state TEXT NOT NULL DEFAULT 'not_started', + repair_attempt_count INTEGER NOT NULL DEFAULT 0, + evidence_json TEXT NOT NULL DEFAULT '{}', + next_action TEXT NOT NULL DEFAULT '', updated_at TEXT NOT NULL, updated_at_unix INTEGER NOT NULL, - PRIMARY KEY (project_id, issue_id, branch_name, run_id, attempt_number) + PRIMARY KEY (project_id, issue_id, branch_name) );"; #[test] @@ -1069,10 +1059,20 @@ mod tests { insert_attempt(&connection, "review-handoff-run", "review-issue", "succeeded"); insert_event(&connection, "review-handoff-run", 1, old); - insert_review_handoff(&connection, "review-issue", "review-handoff-run"); + insert_review_lifecycle( + &connection, + "review-issue", + "review-handoff-run", + "request_pending", + ); insert_attempt(&connection, "cleanup-blocked-run", "cleanup-issue", "succeeded"); insert_event(&connection, "cleanup-blocked-run", 1, old); - insert_review_orchestration(&connection, "cleanup-issue", "cleanup-blocked-run"); + insert_review_lifecycle( + &connection, + "cleanup-issue", + "cleanup-blocked-run", + "cleanup_blocked", + ); insert_attempt(&connection, "attention-run", "attention-issue", "failed"); insert_event(&connection, "attention-run", 1, old); insert_linear_execution_event( @@ -1342,41 +1342,28 @@ mod tests { .expect("event should insert"); } - fn insert_review_handoff(connection: &Connection, issue_id: &str, run_id: &str) { + fn insert_review_lifecycle(connection: &Connection, issue_id: &str, run_id: &str, phase: &str) { connection .execute( - "INSERT INTO review_handoffs ( + "INSERT INTO review_lifecycle_records ( project_id, issue_id, branch_name, run_id, attempt_number, pr_url, - target_base_ref_name, pr_head_ref_name, pr_head_oid, updated_at, - updated_at_unix - ) VALUES ( - 'decodex', ?1, 'y/decodex-test', ?2, 1, - 'https://github.com/hack-ink/decodex/pull/1', 'main', - 'y/decodex-test', 'abc123', '2026-05-01T00:00:00Z', 0 - )", - rusqlite::params![issue_id, run_id], - ) - .expect("review handoff should insert"); - } - - fn insert_review_orchestration(connection: &Connection, issue_id: &str, run_id: &str) { - connection - .execute( - "INSERT INTO review_orchestrations ( - project_id, issue_id, branch_name, run_id, attempt_number, pr_url, - head_sha, phase, request_comment_database_id, + target_base_ref_name, pr_head_ref_name, pr_head_oid, head_sha, phase, + request_comment_database_id, request_created_at_unix_epoch, request_description_thumbs_up_count, request_retry_count, external_round_count, auto_merge_enabled_at_unix_epoch, + landing_state, closeout_state, repair_attempt_count, evidence_json, + next_action, updated_at, updated_at_unix ) VALUES ( 'decodex', ?1, 'y/decodex-test', ?2, 1, - 'https://github.com/hack-ink/decodex/pull/1', 'abc123', - 'cleanup_blocked', NULL, NULL, NULL, 0, 0, NULL, + 'https://github.com/hack-ink/decodex/pull/1', 'main', + 'y/decodex-test', 'abc123', 'abc123', ?3, NULL, NULL, NULL, 0, 0, NULL, + 'not_started', 'not_started', 0, '{}', '', '2026-05-01T00:00:00Z', 0 )", - rusqlite::params![issue_id, run_id], + rusqlite::params![issue_id, run_id, phase], ) - .expect("review orchestration should insert"); + .expect("review lifecycle should insert"); } fn insert_linear_execution_event( diff --git a/apps/decodex/src/recovery.rs b/apps/decodex/src/recovery.rs index f09d9224..8d8bec53 100644 --- a/apps/decodex/src/recovery.rs +++ b/apps/decodex/src/recovery.rs @@ -63,7 +63,7 @@ pub(crate) struct ReviewHandoffRebindRequest { pub(crate) issue: String, /// Pull request URL to bind. pub(crate) pr_url: String, - /// Validate without writing markers or tracker audit comments. + /// Validate without writing a lifecycle record or tracker audit comments. pub(crate) dry_run: bool, } @@ -74,7 +74,7 @@ pub(crate) struct ReviewHandoffAdoptRequest { pub(crate) issue: String, /// Pull request URL to adopt. pub(crate) pr_url: String, - /// Validate without writing runtime markers or tracker audit comments. + /// Validate without writing runtime lifecycle state or tracker audit comments. pub(crate) dry_run: bool, } @@ -124,8 +124,8 @@ struct ReviewHandoffDiagnostic { local_head_oid: Option, worktree_clean: Option, existing_pr_url: Option, - existing_marker_head_oid: Option, - existing_orchestration_head_oid: Option, + existing_lifecycle_handoff_head_oid: Option, + existing_lifecycle_phase_head_oid: Option, pr_base_ref: Option, pr_head_oid: Option, mismatched_field: Option, @@ -279,8 +279,8 @@ impl RebindMode { fn summary_action(self) -> &'static str { match self { - Self::RestoreMissingHandoff => "restored retained review handoff marker", - Self::RefreshExistingHandoff => "refreshed retained review handoff marker", + Self::RestoreMissingHandoff => "restored retained review lifecycle record", + Self::RefreshExistingHandoff => "refreshed retained review lifecycle record", Self::CompleteExistingHandoffState => "completed retained review handoff state", } } @@ -744,10 +744,10 @@ fn diagnose_issue_worktree( local_head_oid, worktree_clean, existing_pr_url: existing_handoff.as_ref().map(|handoff| handoff.pr_url().to_owned()), - existing_marker_head_oid: existing_handoff + existing_lifecycle_handoff_head_oid: existing_handoff .as_ref() .map(|handoff| handoff.pr_head_oid().to_owned()), - existing_orchestration_head_oid: existing_orchestration + existing_lifecycle_phase_head_oid: existing_orchestration .as_ref() .map(|orchestration| orchestration.head_sha().to_owned()), pr_base_ref: binding.pr_base_ref, @@ -1060,7 +1060,7 @@ fn inspect_handoff_next_action(issue_identifier: &str, pr_url: &str) -> String { fn rebind_refresh_next_action(issue_identifier: &str, pr_url: &str) -> String { format!( - "Run `decodex recover review-handoff rebind {issue_identifier} --pr {pr_url} --dry-run`, then rerun without `--dry-run` to refresh the retained marker if validation passes." + "Run `decodex recover review-handoff rebind {issue_identifier} --pr {pr_url} --dry-run`, then rerun without `--dry-run` to refresh the retained lifecycle record if validation passes." ) } @@ -1088,7 +1088,7 @@ fn render_review_handoff_recovery_report(report: &ReviewHandoffRecoveryReport) - for diagnostic in &report.diagnostics { output.push_str(&format!( - "- issue: {}\n state: {}\n classification: {}\n reason: {}\n branch: {}\n worktree_path: {}\n local_branch: {}\n local_head: {}\n worktree_clean: {}\n existing_pr_url: {}\n existing_marker_head: {}\n existing_orchestration_head: {}\n pr_base_ref: {}\n pr_head: {}\n mismatched_field: {}\n active_label_present: {}\n next_action: {}\n", + "- issue: {}\n state: {}\n classification: {}\n reason: {}\n branch: {}\n worktree_path: {}\n local_branch: {}\n local_head: {}\n worktree_clean: {}\n existing_pr_url: {}\n existing_lifecycle_handoff_head: {}\n existing_lifecycle_phase_head: {}\n pr_base_ref: {}\n pr_head: {}\n mismatched_field: {}\n active_label_present: {}\n next_action: {}\n", diagnostic.issue_identifier, diagnostic.issue_state, diagnostic.classification, @@ -1099,8 +1099,8 @@ fn render_review_handoff_recovery_report(report: &ReviewHandoffRecoveryReport) - optional_text(diagnostic.local_head_oid.as_deref()), diagnostic.worktree_clean.map_or_else(|| String::from("unknown"), |clean| clean.to_string()), optional_text(diagnostic.existing_pr_url.as_deref()), - optional_text(diagnostic.existing_marker_head_oid.as_deref()), - optional_text(diagnostic.existing_orchestration_head_oid.as_deref()), + optional_text(diagnostic.existing_lifecycle_handoff_head_oid.as_deref()), + optional_text(diagnostic.existing_lifecycle_phase_head_oid.as_deref()), optional_text(diagnostic.pr_base_ref.as_deref()), optional_text(diagnostic.pr_head_oid.as_deref()), optional_text(diagnostic.mismatched_field.as_deref()), @@ -1615,7 +1615,7 @@ fn validate_existing_handoff_refresh( ) -> Result<(String, i64, RebindMode)> { if existing_handoff.pr_url() != landing_url(landing_state) { eyre::bail!( - "Issue `{}` already has review handoff marker for branch `{}` and PR `{}`; refusing to rebind it to `{}`.", + "Issue `{}` already has a review lifecycle record for branch `{}` and PR `{}`; refusing to rebind it to `{}`.", issue.identifier, worktree.branch_name(), existing_handoff.pr_url(), @@ -1641,7 +1641,7 @@ fn validate_existing_handoff_refresh( } eyre::bail!( - "Issue `{}` already has a review handoff marker for branch `{}` and PR `{}` at head `{local_head_oid}`; no rebind is needed.", + "Issue `{}` already has a review lifecycle record for branch `{}` and PR `{}` at head `{local_head_oid}`; no rebind is needed.", issue.identifier, worktree.branch_name(), existing_handoff.pr_url() @@ -2275,7 +2275,7 @@ fn validate_adopt_absent_handoff_marker( .is_some() { eyre::bail!( - "Issue `{}` already has a retained review handoff marker for branch `{branch}`; use `decodex land` or `decodex recover review-handoff rebind` instead.", + "Issue `{}` already has a retained review lifecycle record for branch `{branch}`; use `decodex land` or `decodex recover review-handoff rebind` instead.", issue.identifier ); } @@ -2327,7 +2327,7 @@ fn apply_review_handoff_rebind( if let Err(error) = write_rebind_audit(context, validation, &event) .and_then(|()| context.state_store.record_linear_execution_event(&event)) { - context.state_store.clear_review_markers_for_handoff( + context.state_store.clear_review_lifecycle_for_handoff( context.config.service_id(), &validation.issue.id, &handoff_marker, @@ -2393,7 +2393,7 @@ fn apply_review_handoff_adopt( if let Err(error) = local_state_write { mark_adopt_attempt_failed(context, validation); - context.state_store.clear_review_markers_for_handoff( + context.state_store.clear_review_lifecycle_for_handoff( context.config.service_id(), &validation.issue.id, &handoff_marker, @@ -2410,7 +2410,7 @@ fn apply_review_handoff_adopt( Err(error) => { mark_adopt_attempt_failed(context, validation); - context.state_store.clear_review_markers_for_handoff( + context.state_store.clear_review_lifecycle_for_handoff( context.config.service_id(), &validation.issue.id, &handoff_marker, @@ -2429,7 +2429,7 @@ fn apply_review_handoff_adopt( { mark_adopt_attempt_failed(context, validation); - context.state_store.clear_review_markers_for_handoff( + context.state_store.clear_review_lifecycle_for_handoff( context.config.service_id(), &validation.issue.id, &handoff_marker, @@ -2667,7 +2667,7 @@ fn review_handoff_rebind_event( format!("branch={}", validation.worktree.branch_name()), format!("pr_url={pr_url}"), format!("pr_head_sha={}", validation.local_head_oid), - format!("existing_review_handoff_marker={}", validation.mode.evidence_value()), + format!("existing_review_lifecycle_record={}", validation.mode.evidence_value()), format!("needs_attention_label_repair={}", validation.clear_needs_attention_label), ]); event.next_action = Some(String::from("continue retained post-review lifecycle")); @@ -2722,7 +2722,7 @@ fn review_handoff_adopt_event( "existing_retained_worktree_mapping={}", validation.previous_worktree_mapping.is_some() ), - String::from("existing_review_handoff_marker=false"), + String::from("existing_review_lifecycle_record=false"), ]); event.next_action = Some(String::from("continue retained post-review lifecycle")); @@ -4252,8 +4252,8 @@ Test workflow. record.pr_base_ref = Some(String::from("main")); record.commit_sha = Some(String::from("0123456789abcdef0123456789abcdef01234567")); record.validation_result = Some(String::from("passed")); - record.summary = Some(String::from("Explicit operator rebind restored marker.")); - record.evidence = Some(vec![String::from("existing_review_handoff_marker=absent")]); + record.summary = Some(String::from("Explicit operator rebind restored lifecycle record.")); + record.evidence = Some(vec![String::from("existing_review_lifecycle_record=absent")]); records::validate_linear_execution_event_record(&record) .expect("rebind event should validate"); @@ -4365,7 +4365,7 @@ Test workflow. record.pr_base_ref = Some(String::from("main")); record.commit_sha = Some(String::from("0123456789abcdef0123456789abcdef01234567")); record.validation_result = Some(String::from("passed")); - record.summary = Some(String::from("Explicit operator rebind restored marker.")); + record.summary = Some(String::from("Explicit operator rebind restored lifecycle record.")); let error = records::validate_linear_execution_event_record(&record) .expect_err("rebind event without evidence should fail"); diff --git a/apps/decodex/src/state/internal.rs b/apps/decodex/src/state/internal.rs index 4834bebf..6f31499b 100644 --- a/apps/decodex/src/state/internal.rs +++ b/apps/decodex/src/state/internal.rs @@ -17,6 +17,53 @@ use libc::{ }; use rusqlite::{self, Row, types::Type}; +const REVIEW_LIFECYCLE_SCHEMA_SQL: &str = r#" +CREATE TABLE IF NOT EXISTS review_lifecycle_records ( + project_id TEXT NOT NULL, + issue_id TEXT NOT NULL, + branch_name TEXT NOT NULL, + run_id TEXT NOT NULL, + attempt_number INTEGER NOT NULL, + pr_url TEXT NOT NULL, + target_base_ref_name TEXT, + pr_head_ref_name TEXT NOT NULL, + pr_head_oid TEXT NOT NULL, + head_sha TEXT NOT NULL, + phase TEXT NOT NULL, + request_comment_database_id INTEGER, + request_created_at_unix_epoch INTEGER, + request_description_thumbs_up_count INTEGER, + request_retry_count INTEGER NOT NULL, + external_round_count INTEGER NOT NULL, + auto_merge_enabled_at_unix_epoch INTEGER, + landing_state TEXT NOT NULL DEFAULT 'not_started', + closeout_state TEXT NOT NULL DEFAULT 'not_started', + repair_attempt_count INTEGER NOT NULL DEFAULT 0, + evidence_json TEXT NOT NULL DEFAULT '{}', + next_action TEXT NOT NULL DEFAULT '', + updated_at TEXT NOT NULL, + updated_at_unix INTEGER NOT NULL, + PRIMARY KEY (project_id, issue_id, branch_name) +); +CREATE TABLE IF NOT EXISTS review_policy_checkpoints ( + project_id TEXT NOT NULL, + issue_id TEXT NOT NULL, + run_id TEXT NOT NULL, + attempt_number INTEGER NOT NULL, + phase TEXT NOT NULL, + status TEXT NOT NULL, + head_sha TEXT NOT NULL, + nonclean_rounds INTEGER NOT NULL, + details_json TEXT NOT NULL DEFAULT '{}', + updated_at TEXT NOT NULL, + updated_at_unix INTEGER NOT NULL, + PRIMARY KEY (project_id, issue_id, run_id, attempt_number, phase) +); +"#; +const DROP_LEGACY_REVIEW_MARKER_TABLES_SQL: &str = r#" +DROP TABLE IF EXISTS review_handoffs; +DROP TABLE IF EXISTS review_orchestrations; +"#; static RUN_ACTIVITY_MARKER_WRITE_SEQUENCE: AtomicU64 = AtomicU64::new(0); pub(crate) struct EffectiveRuntimeMarker<'a> { @@ -134,8 +181,7 @@ struct StateData { execution_programs: HashMap, program_intake_plans: HashMap, program_issue_mappings: HashMap, - review_handoffs: HashMap, - review_orchestrations: HashMap, + review_lifecycle_records: HashMap, review_policy_checkpoints: HashMap, loop_guardrail_checkpoints: HashMap, connector_backoffs: HashMap<(String, String), ConnectorBackoff>, @@ -159,8 +205,7 @@ impl StateData { self.execution_programs = loaded.execution_programs; self.program_intake_plans = loaded.program_intake_plans; self.program_issue_mappings = loaded.program_issue_mappings; - self.review_handoffs = loaded.review_handoffs; - self.review_orchestrations = loaded.review_orchestrations; + self.review_lifecycle_records = loaded.review_lifecycle_records; self.review_policy_checkpoints = loaded.review_policy_checkpoints; self.loop_guardrail_checkpoints = loaded.loop_guardrail_checkpoints; self.connector_backoffs = loaded.connector_backoffs; @@ -177,6 +222,8 @@ impl StateData { fn replace_project_loop_evidence_state(&mut self, project_id: &str, loaded: Self) { self.private_execution_events.retain(|record| record.project_id != project_id); self.private_execution_events.extend(loaded.private_execution_events); + self.review_lifecycle_records.retain(|key, _record| key.project_id != project_id); + self.review_lifecycle_records.extend(loaded.review_lifecycle_records); self.review_policy_checkpoints.retain(|key, _record| key.project_id != project_id); self.review_policy_checkpoints.extend(loaded.review_policy_checkpoints); } @@ -450,57 +497,22 @@ ON linear_execution_events (service_id, issue_id, event_unix, recorded_at_unix); } fn bootstrap_review_schema(&self) -> Result<()> { - self.connection.execute_batch( - r#" -CREATE TABLE IF NOT EXISTS review_handoffs ( - project_id TEXT NOT NULL, - issue_id TEXT NOT NULL, - branch_name TEXT NOT NULL, - run_id TEXT NOT NULL, - attempt_number INTEGER NOT NULL, - pr_url TEXT NOT NULL, - target_base_ref_name TEXT, - pr_head_ref_name TEXT NOT NULL, - pr_head_oid TEXT NOT NULL, - updated_at TEXT NOT NULL, - updated_at_unix INTEGER NOT NULL, - PRIMARY KEY (project_id, issue_id, branch_name) -); -CREATE TABLE IF NOT EXISTS review_orchestrations ( - project_id TEXT NOT NULL, - issue_id TEXT NOT NULL, - branch_name TEXT NOT NULL, - run_id TEXT NOT NULL, - attempt_number INTEGER NOT NULL, - pr_url TEXT NOT NULL, - head_sha TEXT NOT NULL, - phase TEXT NOT NULL, - request_comment_database_id INTEGER, - request_created_at_unix_epoch INTEGER, - request_description_thumbs_up_count INTEGER, - request_retry_count INTEGER NOT NULL, - external_round_count INTEGER NOT NULL, - auto_merge_enabled_at_unix_epoch INTEGER, - updated_at TEXT NOT NULL, - updated_at_unix INTEGER NOT NULL, - PRIMARY KEY (project_id, issue_id, branch_name, run_id, attempt_number) -); -CREATE TABLE IF NOT EXISTS review_policy_checkpoints ( - project_id TEXT NOT NULL, - issue_id TEXT NOT NULL, - run_id TEXT NOT NULL, - attempt_number INTEGER NOT NULL, - phase TEXT NOT NULL, - status TEXT NOT NULL, - head_sha TEXT NOT NULL, - nonclean_rounds INTEGER NOT NULL, - details_json TEXT NOT NULL DEFAULT '{}', - updated_at TEXT NOT NULL, - updated_at_unix INTEGER NOT NULL, - PRIMARY KEY (project_id, issue_id, run_id, attempt_number, phase) -); -"#, - )?; + let legacy_handoffs_exist = self.table_exists("review_handoffs")?; + let legacy_orchestrations_exist = self.table_exists("review_orchestrations")?; + + self.connection.execute_batch(REVIEW_LIFECYCLE_SCHEMA_SQL)?; + + match (legacy_handoffs_exist, legacy_orchestrations_exist) { + (true, true) => { + self.migrate_legacy_review_handoffs_with_orchestrations()?; + self.migrate_legacy_review_orchestration_orphans()?; + }, + (true, false) => self.migrate_legacy_review_handoffs()?, + (false, true) => self.migrate_legacy_review_orchestrations()?, + (false, false) => {}, + } + + self.connection.execute_batch(DROP_LEGACY_REVIEW_MARKER_TABLES_SQL)?; self.ensure_column( "review_policy_checkpoints", "details_json", @@ -510,6 +522,126 @@ CREATE TABLE IF NOT EXISTS review_policy_checkpoints ( Ok(()) } + fn table_exists(&self, table: &str) -> Result { + let count: i64 = self.connection.query_row( + "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = ?1", + params![table], + |row| row.get(0), + )?; + + Ok(count > 0) + } + + fn migrate_legacy_review_handoffs(&self) -> Result<()> { + self.connection.execute( + "INSERT OR IGNORE INTO review_lifecycle_records ( + project_id, issue_id, branch_name, run_id, attempt_number, pr_url, + target_base_ref_name, pr_head_ref_name, pr_head_oid, head_sha, phase, + request_comment_database_id, request_created_at_unix_epoch, + request_description_thumbs_up_count, request_retry_count, external_round_count, + auto_merge_enabled_at_unix_epoch, landing_state, closeout_state, + repair_attempt_count, evidence_json, next_action, updated_at, updated_at_unix + ) + SELECT project_id, issue_id, branch_name, run_id, attempt_number, pr_url, + target_base_ref_name, pr_head_ref_name, pr_head_oid, pr_head_oid, + 'request_pending', NULL, NULL, NULL, 0, 0, NULL, 'not_started', + 'not_started', 0, '{}', '', updated_at, updated_at_unix + FROM review_handoffs + ORDER BY updated_at_unix DESC, updated_at DESC", + [], + )?; + + Ok(()) + } + + fn migrate_legacy_review_orchestrations(&self) -> Result<()> { + self.connection.execute( + "INSERT OR IGNORE INTO review_lifecycle_records ( + project_id, issue_id, branch_name, run_id, attempt_number, pr_url, + target_base_ref_name, pr_head_ref_name, pr_head_oid, head_sha, phase, + request_comment_database_id, request_created_at_unix_epoch, + request_description_thumbs_up_count, request_retry_count, external_round_count, + auto_merge_enabled_at_unix_epoch, landing_state, closeout_state, + repair_attempt_count, evidence_json, next_action, updated_at, updated_at_unix + ) + SELECT project_id, issue_id, branch_name, run_id, attempt_number, pr_url, + NULL, branch_name, head_sha, head_sha, phase, + request_comment_database_id, request_created_at_unix_epoch, + request_description_thumbs_up_count, request_retry_count, external_round_count, + auto_merge_enabled_at_unix_epoch, 'not_started', 'not_started', + 0, '{}', '', updated_at, updated_at_unix + FROM review_orchestrations + ORDER BY updated_at_unix DESC, updated_at DESC", + [], + )?; + + Ok(()) + } + + fn migrate_legacy_review_handoffs_with_orchestrations(&self) -> Result<()> { + self.connection.execute( + "INSERT OR IGNORE INTO review_lifecycle_records ( + project_id, issue_id, branch_name, run_id, attempt_number, pr_url, + target_base_ref_name, pr_head_ref_name, pr_head_oid, head_sha, phase, + request_comment_database_id, request_created_at_unix_epoch, + request_description_thumbs_up_count, request_retry_count, external_round_count, + auto_merge_enabled_at_unix_epoch, landing_state, closeout_state, + repair_attempt_count, evidence_json, next_action, updated_at, updated_at_unix + ) + SELECT h.project_id, h.issue_id, h.branch_name, h.run_id, h.attempt_number, h.pr_url, + h.target_base_ref_name, h.pr_head_ref_name, h.pr_head_oid, + COALESCE(o.head_sha, h.pr_head_oid), COALESCE(o.phase, 'request_pending'), + o.request_comment_database_id, o.request_created_at_unix_epoch, + o.request_description_thumbs_up_count, COALESCE(o.request_retry_count, 0), + COALESCE(o.external_round_count, 0), o.auto_merge_enabled_at_unix_epoch, + 'not_started', 'not_started', 0, '{}', '', + COALESCE(o.updated_at, h.updated_at), + COALESCE(o.updated_at_unix, h.updated_at_unix) + FROM review_handoffs AS h + LEFT JOIN review_orchestrations AS o + ON o.project_id = h.project_id + AND o.issue_id = h.issue_id + AND o.branch_name = h.branch_name + AND o.run_id = h.run_id + AND o.attempt_number = h.attempt_number + ORDER BY h.updated_at_unix DESC, h.updated_at DESC", + [], + )?; + + Ok(()) + } + + fn migrate_legacy_review_orchestration_orphans(&self) -> Result<()> { + self.connection.execute( + "INSERT OR IGNORE INTO review_lifecycle_records ( + project_id, issue_id, branch_name, run_id, attempt_number, pr_url, + target_base_ref_name, pr_head_ref_name, pr_head_oid, head_sha, phase, + request_comment_database_id, request_created_at_unix_epoch, + request_description_thumbs_up_count, request_retry_count, external_round_count, + auto_merge_enabled_at_unix_epoch, landing_state, closeout_state, + repair_attempt_count, evidence_json, next_action, updated_at, updated_at_unix + ) + SELECT o.project_id, o.issue_id, o.branch_name, o.run_id, o.attempt_number, o.pr_url, + NULL, o.branch_name, o.head_sha, o.head_sha, o.phase, + o.request_comment_database_id, o.request_created_at_unix_epoch, + o.request_description_thumbs_up_count, o.request_retry_count, + o.external_round_count, o.auto_merge_enabled_at_unix_epoch, + 'not_started', 'not_started', 0, '{}', '', o.updated_at, o.updated_at_unix + FROM review_orchestrations AS o + WHERE NOT EXISTS ( + SELECT 1 + FROM review_handoffs AS h + WHERE h.project_id = o.project_id + AND h.issue_id = o.issue_id + AND h.branch_name = o.branch_name + ) + ORDER BY o.updated_at_unix DESC, o.updated_at DESC", + [], + )?; + + Ok(()) + } + fn ensure_column(&self, table: &str, column: &str, add_column_sql: &str) -> Result<()> { let mut statement = self.connection.prepare(&format!("PRAGMA table_info({table})"))?; let column_names = statement.query_map([], |row| row.get::<_, String>(1))?; @@ -853,8 +985,7 @@ ON CONFLICT(key) DO UPDATE SET value = excluded.value; self.load_decision_contracts(&mut state)?; self.load_execution_programs(&mut state)?; self.load_program_intake_state(&mut state)?; - self.load_review_handoffs(&mut state)?; - self.load_review_orchestrations(&mut state)?; + self.load_review_lifecycle_records(&mut state)?; self.load_review_policy_checkpoints(&mut state)?; self.load_loop_guardrail_checkpoints(&mut state)?; self.load_connector_backoffs(&mut state)?; @@ -878,6 +1009,7 @@ ON CONFLICT(key) DO UPDATE SET value = excluded.value; let mut state = StateData::default(); self.load_private_execution_events_for_project(&mut state, project_id)?; + self.load_review_lifecycle_records_for_project(&mut state, project_id)?; self.load_review_policy_checkpoints_for_project(&mut state, project_id)?; Ok(state) @@ -906,8 +1038,7 @@ ON CONFLICT(key) DO UPDATE SET value = excluded.value; persist_decision_contracts(&transaction, state)?; persist_execution_programs(&transaction, state)?; persist_program_intake_state(&transaction, state)?; - persist_review_handoffs(&transaction, state)?; - persist_review_orchestrations(&transaction, state)?; + persist_review_lifecycle_records(&transaction, state)?; persist_review_policy_checkpoints(&transaction, state)?; persist_loop_guardrail_checkpoints(&transaction, state)?; persist_connector_backoffs(&transaction, state)?; @@ -1361,35 +1492,25 @@ ON CONFLICT(key) DO UPDATE SET value = excluded.value; params![previous_issue_id], )?; transaction.execute( - "INSERT OR IGNORE INTO review_handoffs ( + "INSERT OR IGNORE INTO review_lifecycle_records ( project_id, issue_id, branch_name, run_id, attempt_number, pr_url, - target_base_ref_name, pr_head_ref_name, pr_head_oid, updated_at, updated_at_unix - ) - SELECT project_id, ?2, branch_name, run_id, attempt_number, pr_url, - target_base_ref_name, pr_head_ref_name, pr_head_oid, updated_at, updated_at_unix - FROM review_handoffs WHERE issue_id = ?1", - params![previous_issue_id, canonical_issue_id], - )?; - transaction.execute( - "DELETE FROM review_handoffs WHERE issue_id = ?1", - params![previous_issue_id], - )?; - transaction.execute( - "INSERT OR IGNORE INTO review_orchestrations ( - project_id, issue_id, branch_name, run_id, attempt_number, pr_url, head_sha, - phase, request_comment_database_id, request_created_at_unix_epoch, + target_base_ref_name, pr_head_ref_name, pr_head_oid, head_sha, phase, + request_comment_database_id, request_created_at_unix_epoch, request_description_thumbs_up_count, request_retry_count, external_round_count, - auto_merge_enabled_at_unix_epoch, updated_at, updated_at_unix + auto_merge_enabled_at_unix_epoch, landing_state, closeout_state, + repair_attempt_count, evidence_json, next_action, updated_at, updated_at_unix ) - SELECT project_id, ?2, branch_name, run_id, attempt_number, pr_url, head_sha, - phase, request_comment_database_id, request_created_at_unix_epoch, + SELECT project_id, ?2, branch_name, run_id, attempt_number, pr_url, + target_base_ref_name, pr_head_ref_name, pr_head_oid, head_sha, phase, + request_comment_database_id, request_created_at_unix_epoch, request_description_thumbs_up_count, request_retry_count, external_round_count, - auto_merge_enabled_at_unix_epoch, updated_at, updated_at_unix - FROM review_orchestrations WHERE issue_id = ?1", + auto_merge_enabled_at_unix_epoch, landing_state, closeout_state, + repair_attempt_count, evidence_json, next_action, updated_at, updated_at_unix + FROM review_lifecycle_records WHERE issue_id = ?1", params![previous_issue_id, canonical_issue_id], )?; transaction.execute( - "DELETE FROM review_orchestrations WHERE issue_id = ?1", + "DELETE FROM review_lifecycle_records WHERE issue_id = ?1", params![previous_issue_id], )?; transaction.commit()?; @@ -1397,13 +1518,12 @@ ON CONFLICT(key) DO UPDATE SET value = excluded.value; Ok(()) } - fn delete_worktree_and_review_markers(&mut self, issue_id: &str) -> Result<()> { + fn delete_worktree_and_review_lifecycle(&mut self, issue_id: &str) -> Result<()> { let transaction = self.connection.transaction()?; transaction.execute("DELETE FROM worktrees WHERE issue_id = ?1", params![issue_id])?; - transaction.execute("DELETE FROM review_handoffs WHERE issue_id = ?1", params![issue_id])?; transaction.execute( - "DELETE FROM review_orchestrations WHERE issue_id = ?1", + "DELETE FROM review_lifecycle_records WHERE issue_id = ?1", params![issue_id], )?; transaction.execute( @@ -1430,12 +1550,7 @@ ON CONFLICT(key) DO UPDATE SET value = excluded.value; let transaction = self.connection.transaction()?; transaction.execute( - "DELETE FROM review_handoffs - WHERE project_id = ?1 AND issue_id = ?2 AND branch_name = ?3", - params![project_id, issue_id, branch_name], - )?; - transaction.execute( - "DELETE FROM review_orchestrations + "DELETE FROM review_lifecycle_records WHERE project_id = ?1 AND issue_id = ?2 AND branch_name = ?3 AND run_id = ?4 AND attempt_number = ?5", params![project_id, issue_id, branch_name, run_id, attempt_number], @@ -2452,35 +2567,53 @@ ON CONFLICT(key) DO UPDATE SET value = excluded.value; Ok(records) } - fn load_review_handoffs(&self, state: &mut StateData) -> Result<()> { + fn load_review_lifecycle_records(&self, state: &mut StateData) -> Result<()> { let mut statement = self.connection.prepare( "SELECT project_id, issue_id, branch_name, run_id, attempt_number, pr_url, \ - target_base_ref_name, pr_head_ref_name, pr_head_oid, updated_at, updated_at_unix \ - FROM review_handoffs", + target_base_ref_name, pr_head_ref_name, pr_head_oid, head_sha, phase, \ + request_comment_database_id, request_created_at_unix_epoch, \ + request_description_thumbs_up_count, request_retry_count, external_round_count, \ + auto_merge_enabled_at_unix_epoch, landing_state, closeout_state, \ + repair_attempt_count, evidence_json, next_action, updated_at, updated_at_unix \ + FROM review_lifecycle_records", )?; let rows = statement.query_map([], |row| { let project_id: String = row.get(0)?; let issue_id: String = row.get(1)?; let branch_name: String = row.get(2)?; - let marker = ReviewHandoffMarker { - run_id: row.get(3)?, - attempt_number: row.get(4)?, - branch_name: branch_name.clone(), - pr_url: row.get(5)?, - target_base_ref_name: row.get(6)?, - pr_head_ref_name: row.get(7)?, - pr_head_oid: row.get(8)?, - }; + let run_id: String = row.get(3)?; + let attempt_number: i64 = row.get(4)?; + let request_description_thumbs_up_count = row + .get::<_, Option>(13)? + .and_then(|count| usize::try_from(count).ok()); Ok(( - ReviewMarkerKey::new(&project_id, &issue_id, &branch_name), - ReviewHandoffRuntimeRecord { + ReviewLifecycleKey::new(&project_id, &issue_id, &branch_name), + ReviewLifecycleRuntimeRecord { project_id, issue_id, branch_name, - marker, - updated_at: row.get(9)?, - updated_at_unix: row.get(10)?, + run_id, + attempt_number, + pr_url: row.get(5)?, + target_base_ref_name: row.get(6)?, + pr_head_ref_name: row.get(7)?, + pr_head_oid: row.get(8)?, + head_sha: row.get(9)?, + phase: row.get(10)?, + request_comment_database_id: row.get(11)?, + request_created_at_unix_epoch: row.get(12)?, + request_description_thumbs_up_count, + request_retry_count: row.get(14)?, + external_round_count: row.get(15)?, + auto_merge_enabled_at_unix_epoch: row.get(16)?, + landing_state: row.get(17)?, + closeout_state: row.get(18)?, + repair_attempt_count: row.get(19)?, + evidence_json: row.get(20)?, + next_action: row.get(21)?, + updated_at: row.get(22)?, + updated_at_unix: row.get(23)?, }, )) })?; @@ -2488,61 +2621,63 @@ ON CONFLICT(key) DO UPDATE SET value = excluded.value; for row in rows { let (key, record) = row?; - state.review_handoffs.insert(key, record); + state.review_lifecycle_records.insert(key, record); } Ok(()) } - fn load_review_orchestrations(&self, state: &mut StateData) -> Result<()> { + fn load_review_lifecycle_records_for_project( + &self, + state: &mut StateData, + project_id: &str, + ) -> Result<()> { let mut statement = self.connection.prepare( - "SELECT project_id, issue_id, branch_name, run_id, attempt_number, pr_url, head_sha, \ - phase, request_comment_database_id, request_created_at_unix_epoch, \ + "SELECT project_id, issue_id, branch_name, run_id, attempt_number, pr_url, \ + target_base_ref_name, pr_head_ref_name, pr_head_oid, head_sha, phase, \ + request_comment_database_id, request_created_at_unix_epoch, \ request_description_thumbs_up_count, request_retry_count, external_round_count, \ - auto_merge_enabled_at_unix_epoch, updated_at, updated_at_unix \ - FROM review_orchestrations", + auto_merge_enabled_at_unix_epoch, landing_state, closeout_state, \ + repair_attempt_count, evidence_json, next_action, updated_at, updated_at_unix \ + FROM review_lifecycle_records WHERE project_id = ?1", )?; - let rows = statement.query_map([], |row| { + let rows = statement.query_map(params![project_id], |row| { let project_id: String = row.get(0)?; let issue_id: String = row.get(1)?; let branch_name: String = row.get(2)?; let run_id: String = row.get(3)?; let attempt_number: i64 = row.get(4)?; let request_description_thumbs_up_count = row - .get::<_, Option>(10)? + .get::<_, Option>(13)? .and_then(|count| usize::try_from(count).ok()); - let marker = ReviewOrchestrationMarker::new( - run_id.clone(), - attempt_number, - branch_name.clone(), - row.get::<_, String>(5)?, - row.get::<_, String>(6)?, - row.get::<_, String>(7)?, - row.get(8)?, - row.get(9)?, - request_description_thumbs_up_count, - row.get(11)?, - row.get(12)?, - row.get(13)?, - ); Ok(( - ReviewOrchestrationKey::new( - &project_id, - &issue_id, - &branch_name, - &run_id, - attempt_number, - ), - ReviewOrchestrationRuntimeRecord { + ReviewLifecycleKey::new(&project_id, &issue_id, &branch_name), + ReviewLifecycleRuntimeRecord { project_id, issue_id, branch_name, run_id, attempt_number, - marker, - updated_at: row.get(14)?, - updated_at_unix: row.get(15)?, + pr_url: row.get(5)?, + target_base_ref_name: row.get(6)?, + pr_head_ref_name: row.get(7)?, + pr_head_oid: row.get(8)?, + head_sha: row.get(9)?, + phase: row.get(10)?, + request_comment_database_id: row.get(11)?, + request_created_at_unix_epoch: row.get(12)?, + request_description_thumbs_up_count, + request_retry_count: row.get(14)?, + external_round_count: row.get(15)?, + auto_merge_enabled_at_unix_epoch: row.get(16)?, + landing_state: row.get(17)?, + closeout_state: row.get(18)?, + repair_attempt_count: row.get(19)?, + evidence_json: row.get(20)?, + next_action: row.get(21)?, + updated_at: row.get(22)?, + updated_at_unix: row.get(23)?, }, )) })?; @@ -2550,7 +2685,7 @@ ON CONFLICT(key) DO UPDATE SET value = excluded.value; for row in rows { let (key, record) = row?; - state.review_orchestrations.insert(key, record); + state.review_lifecycle_records.insert(key, record); } Ok(()) @@ -2994,12 +3129,12 @@ impl WorktreeMappingRecord { } #[derive(Clone, Debug, Eq, Hash, PartialEq)] -struct ReviewMarkerKey { +struct ReviewLifecycleKey { project_id: String, issue_id: String, branch_name: String, } -impl ReviewMarkerKey { +impl ReviewLifecycleKey { fn new(project_id: &str, issue_id: &str, branch_name: &str) -> Self { Self { project_id: project_id.to_owned(), @@ -3009,32 +3144,6 @@ impl ReviewMarkerKey { } } -#[derive(Clone, Debug, Eq, Hash, PartialEq)] -struct ReviewOrchestrationKey { - project_id: String, - issue_id: String, - branch_name: String, - run_id: String, - attempt_number: i64, -} -impl ReviewOrchestrationKey { - fn new( - project_id: &str, - issue_id: &str, - branch_name: &str, - run_id: &str, - attempt_number: i64, - ) -> Self { - Self { - project_id: project_id.to_owned(), - issue_id: issue_id.to_owned(), - branch_name: branch_name.to_owned(), - run_id: run_id.to_owned(), - attempt_number, - } - } -} - #[derive(Clone, Debug, Eq, Hash, PartialEq)] struct ReviewPolicyKey { project_id: String, @@ -3062,26 +3171,69 @@ impl ReviewPolicyKey { } #[derive(Clone, Debug)] -struct ReviewHandoffRuntimeRecord { - project_id: String, - issue_id: String, - branch_name: String, - marker: ReviewHandoffMarker, - updated_at: String, - updated_at_unix: i64, -} - -#[derive(Clone, Debug)] -struct ReviewOrchestrationRuntimeRecord { +struct ReviewLifecycleRuntimeRecord { project_id: String, issue_id: String, branch_name: String, run_id: String, attempt_number: i64, - marker: ReviewOrchestrationMarker, + pr_url: String, + target_base_ref_name: Option, + pr_head_ref_name: String, + pr_head_oid: String, + head_sha: String, + phase: String, + request_comment_database_id: Option, + request_created_at_unix_epoch: Option, + request_description_thumbs_up_count: Option, + request_retry_count: i64, + external_round_count: i64, + auto_merge_enabled_at_unix_epoch: Option, + landing_state: String, + closeout_state: String, + repair_attempt_count: i64, + evidence_json: String, + next_action: String, updated_at: String, updated_at_unix: i64, } +impl ReviewLifecycleRuntimeRecord { + fn as_public(&self) -> ReviewLifecycleRecord { + ReviewLifecycleRecord { + project_id: self.project_id.clone(), + issue_id: self.issue_id.clone(), + branch_name: self.branch_name.clone(), + run_id: self.run_id.clone(), + attempt_number: self.attempt_number, + pr_url: self.pr_url.clone(), + target_base_ref_name: self.target_base_ref_name.clone(), + pr_head_ref_name: self.pr_head_ref_name.clone(), + pr_head_oid: self.pr_head_oid.clone(), + head_sha: self.head_sha.clone(), + phase: self.phase.clone(), + request_comment_database_id: self.request_comment_database_id, + request_created_at_unix_epoch: self.request_created_at_unix_epoch, + request_description_thumbs_up_count: self.request_description_thumbs_up_count, + request_retry_count: self.request_retry_count, + external_round_count: self.external_round_count, + auto_merge_enabled_at_unix_epoch: self.auto_merge_enabled_at_unix_epoch, + landing_state: self.landing_state.clone(), + closeout_state: self.closeout_state.clone(), + repair_attempt_count: self.repair_attempt_count, + evidence_json: self.evidence_json.clone(), + next_action: self.next_action.clone(), + updated_at: self.updated_at.clone(), + updated_at_unix: self.updated_at_unix, + } + } + + fn matches_handoff_identity(&self, handoff: &ReviewHandoffMarker) -> bool { + self.run_id == handoff.run_id() + && self.attempt_number == handoff.attempt_number() + && self.branch_name == handoff.branch_name() + && self.pr_url == handoff.pr_url() + } +} #[derive(Clone, Debug)] struct ReviewPolicyRuntimeRecord { @@ -4307,59 +4459,48 @@ fn insert_program_intake_state( Ok(()) } -fn persist_review_handoffs(transaction: &Transaction<'_>, state: &StateData) -> Result<()> { - for record in state.review_handoffs.values() { +fn persist_review_lifecycle_records( + transaction: &Transaction<'_>, + state: &StateData, +) -> Result<()> { + for record in state.review_lifecycle_records.values() { transaction.execute( - "INSERT OR REPLACE INTO review_handoffs ( + "INSERT OR REPLACE INTO review_lifecycle_records ( project_id, issue_id, branch_name, run_id, attempt_number, pr_url, - target_base_ref_name, pr_head_ref_name, pr_head_oid, updated_at, updated_at_unix - ) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11)", - params![ - record.project_id, - record.issue_id, - record.branch_name, - record.marker.run_id, - record.marker.attempt_number, - record.marker.pr_url, - record.marker.target_base_ref_name, - record.marker.pr_head_ref_name, - record.marker.pr_head_oid, - record.updated_at, - record.updated_at_unix, - ], - )?; - } - - Ok(()) -} - -fn persist_review_orchestrations(transaction: &Transaction<'_>, state: &StateData) -> Result<()> { - for record in state.review_orchestrations.values() { - transaction.execute( - "INSERT OR REPLACE INTO review_orchestrations ( - project_id, issue_id, branch_name, run_id, attempt_number, pr_url, head_sha, + target_base_ref_name, pr_head_ref_name, pr_head_oid, head_sha, phase, request_comment_database_id, request_created_at_unix_epoch, request_description_thumbs_up_count, request_retry_count, external_round_count, - auto_merge_enabled_at_unix_epoch, updated_at, updated_at_unix - ) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16)", + auto_merge_enabled_at_unix_epoch, landing_state, closeout_state, + repair_attempt_count, evidence_json, next_action, updated_at, updated_at_unix + ) VALUES ( + ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, + ?13, ?14, ?15, ?16, ?17, ?18, ?19, ?20, ?21, ?22, ?23, ?24 + )", params![ record.project_id, record.issue_id, record.branch_name, record.run_id, record.attempt_number, - record.marker.pr_url, - record.marker.head_sha, - record.marker.phase, - record.marker.request_comment_database_id, - record.marker.request_created_at_unix_epoch, + record.pr_url, + record.target_base_ref_name, + record.pr_head_ref_name, + record.pr_head_oid, + record.head_sha, + record.phase, + record.request_comment_database_id, + record.request_created_at_unix_epoch, record - .marker .request_description_thumbs_up_count .and_then(|count| i64::try_from(count).ok()), - record.marker.request_retry_count, - record.marker.external_round_count, - record.marker.auto_merge_enabled_at_unix_epoch, + record.request_retry_count, + record.external_round_count, + record.auto_merge_enabled_at_unix_epoch, + record.landing_state, + record.closeout_state, + record.repair_attempt_count, + record.evidence_json, + record.next_action, record.updated_at, record.updated_at_unix, ], diff --git a/apps/decodex/src/state/models.rs b/apps/decodex/src/state/models.rs index e5cec685..0cb5c1dd 100644 --- a/apps/decodex/src/state/models.rs +++ b/apps/decodex/src/state/models.rs @@ -1555,7 +1555,6 @@ impl ReviewOrchestrationMarker { self.request_created_at_unix_epoch } - #[cfg(test)] pub(crate) fn request_description_thumbs_up_count(&self) -> Option { self.request_description_thumbs_up_count } @@ -1573,6 +1572,133 @@ impl ReviewOrchestrationMarker { } } +/// Runtime-owned review lifecycle record for one retained PR-backed lane. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) struct ReviewLifecycleRecord { + project_id: String, + issue_id: String, + branch_name: String, + run_id: String, + attempt_number: i64, + pr_url: String, + target_base_ref_name: Option, + pr_head_ref_name: String, + pr_head_oid: String, + head_sha: String, + phase: String, + request_comment_database_id: Option, + request_created_at_unix_epoch: Option, + request_description_thumbs_up_count: Option, + request_retry_count: i64, + external_round_count: i64, + auto_merge_enabled_at_unix_epoch: Option, + landing_state: String, + closeout_state: String, + repair_attempt_count: i64, + evidence_json: String, + next_action: String, + updated_at: String, + updated_at_unix: i64, +} +#[allow(dead_code)] +impl ReviewLifecycleRecord { + pub(crate) fn project_id(&self) -> &str { + &self.project_id + } + + pub(crate) fn issue_id(&self) -> &str { + &self.issue_id + } + + pub(crate) fn branch_name(&self) -> &str { + &self.branch_name + } + + pub(crate) fn run_id(&self) -> &str { + &self.run_id + } + + pub(crate) fn attempt_number(&self) -> i64 { + self.attempt_number + } + + pub(crate) fn pr_url(&self) -> &str { + &self.pr_url + } + + pub(crate) fn target_base_ref_name(&self) -> Option<&str> { + self.target_base_ref_name.as_deref() + } + + pub(crate) fn pr_head_ref_name(&self) -> &str { + &self.pr_head_ref_name + } + + pub(crate) fn pr_head_oid(&self) -> &str { + &self.pr_head_oid + } + + pub(crate) fn head_sha(&self) -> &str { + &self.head_sha + } + + pub(crate) fn phase(&self) -> &str { + &self.phase + } + + pub(crate) fn request_comment_database_id(&self) -> Option { + self.request_comment_database_id + } + + pub(crate) fn request_created_at_unix_epoch(&self) -> Option { + self.request_created_at_unix_epoch + } + + pub(crate) fn request_description_thumbs_up_count(&self) -> Option { + self.request_description_thumbs_up_count + } + + pub(crate) fn request_retry_count(&self) -> i64 { + self.request_retry_count + } + + pub(crate) fn external_round_count(&self) -> i64 { + self.external_round_count + } + + pub(crate) fn auto_merge_enabled_at_unix_epoch(&self) -> Option { + self.auto_merge_enabled_at_unix_epoch + } + + pub(crate) fn landing_state(&self) -> &str { + &self.landing_state + } + + pub(crate) fn closeout_state(&self) -> &str { + &self.closeout_state + } + + pub(crate) fn repair_attempt_count(&self) -> i64 { + self.repair_attempt_count + } + + pub(crate) fn evidence_json(&self) -> &str { + &self.evidence_json + } + + pub(crate) fn next_action(&self) -> &str { + &self.next_action + } + + pub(crate) fn updated_at(&self) -> &str { + &self.updated_at + } + + pub(crate) fn updated_at_unix(&self) -> i64 { + self.updated_at_unix + } +} + pub(crate) fn worktree_provenance( source: impl Into, created_at_unix: Option, diff --git a/apps/decodex/src/state/store.rs b/apps/decodex/src/state/store.rs index 1737e641..5b9f48e9 100644 --- a/apps/decodex/src/state/store.rs +++ b/apps/decodex/src/state/store.rs @@ -28,6 +28,7 @@ pub(crate) struct ReviewPolicyCheckpointInput<'a> { #[derive(Clone, Debug, Default)] pub(crate) struct ProjectLoopEvidenceSnapshot { private_events: HashMap<(String, String, i64), Vec>, + review_lifecycle_records: HashMap<(String, String), ReviewLifecycleRecord>, review_checkpoints: HashMap<(String, String, i64, String), ReviewPolicyCheckpoint>, } impl ProjectLoopEvidenceSnapshot { @@ -42,6 +43,13 @@ impl ProjectLoopEvidenceSnapshot { .push(event); } + fn insert_review_lifecycle_record(&mut self, record: ReviewLifecycleRecord) { + self.review_lifecycle_records.insert( + (record.issue_id().to_owned(), record.branch_name().to_owned()), + record, + ); + } + fn insert_review_checkpoint(&mut self, checkpoint: ReviewPolicyCheckpoint) { self.review_checkpoints.insert( ( @@ -76,6 +84,15 @@ impl ProjectLoopEvidenceSnapshot { .unwrap_or(&[]) } + #[cfg_attr(not(test), allow(dead_code))] + pub(crate) fn review_lifecycle_record( + &self, + issue_id: &str, + branch_name: &str, + ) -> Option<&ReviewLifecycleRecord> { + self.review_lifecycle_records.get(&(issue_id.to_owned(), branch_name.to_owned())) + } + pub(crate) fn private_events_for_issue(&self, issue_id: &str) -> Vec<&PrivateExecutionEvent> { let mut events = self .private_events @@ -324,13 +341,8 @@ impl StateStore { state.worktrees.entry(canonical_issue_id.to_owned()).or_insert(mapping); } - retarget_review_handoff_issue( - &mut state.review_handoffs, - previous_issue_id, - canonical_issue_id, - ); - retarget_review_orchestration_issue( - &mut state.review_orchestrations, + retarget_review_lifecycle_issue( + &mut state.review_lifecycle_records, previous_issue_id, canonical_issue_id, ); @@ -1883,6 +1895,13 @@ impl StateStore { { snapshot.insert_private_event(record.as_public()); } + for record in state + .review_lifecycle_records + .values() + .filter(|record| record.project_id == project_id) + { + snapshot.insert_review_lifecycle_record(record.as_public()); + } for record in state .review_policy_checkpoints .values() @@ -2389,7 +2408,7 @@ impl StateStore { self.upsert_worktree_and_remember_run_project_locked(&mapping) } - /// Create or replace the retained review handoff marker for one issue lane. + /// Create or replace the retained review handoff projection for one issue lane. pub(crate) fn upsert_review_handoff_marker( &self, project_id: &str, @@ -2397,38 +2416,108 @@ impl StateStore { marker: &ReviewHandoffMarker, ) -> Result<()> { let now = timestamp_parts(); - let key = ReviewMarkerKey::new(project_id, issue_id, marker.branch_name()); + let key = ReviewLifecycleKey::new(project_id, issue_id, marker.branch_name()); let mut state = self.lock()?; + let record = + state.review_lifecycle_records.entry(key).or_insert_with(|| { + ReviewLifecycleRuntimeRecord { + project_id: project_id.to_owned(), + issue_id: issue_id.to_owned(), + branch_name: marker.branch_name().to_owned(), + run_id: marker.run_id().to_owned(), + attempt_number: marker.attempt_number(), + pr_url: marker.pr_url().to_owned(), + target_base_ref_name: marker.target_base_ref_name().map(str::to_owned), + pr_head_ref_name: marker.pr_head_ref_name().to_owned(), + pr_head_oid: marker.pr_head_oid().to_owned(), + head_sha: marker.pr_head_oid().to_owned(), + phase: String::from("request_pending"), + request_comment_database_id: None, + request_created_at_unix_epoch: None, + request_description_thumbs_up_count: None, + request_retry_count: 0, + external_round_count: 0, + auto_merge_enabled_at_unix_epoch: None, + landing_state: String::from("not_started"), + closeout_state: String::from("not_started"), + repair_attempt_count: 0, + evidence_json: String::from("{}"), + next_action: String::new(), + updated_at: now.text.clone(), + updated_at_unix: now.unix, + } + }); + let same_handoff_projection = record.run_id == marker.run_id() + && record.attempt_number == marker.attempt_number() + && record.pr_url == marker.pr_url() + && record.target_base_ref_name.as_deref() == marker.target_base_ref_name() + && record.pr_head_ref_name == marker.pr_head_ref_name() + && record.pr_head_oid == marker.pr_head_oid(); + + record.run_id = marker.run_id().to_owned(); + record.attempt_number = marker.attempt_number(); + record.pr_url = marker.pr_url().to_owned(); + record.target_base_ref_name = marker.target_base_ref_name().map(str::to_owned); + record.pr_head_ref_name = marker.pr_head_ref_name().to_owned(); + record.pr_head_oid = marker.pr_head_oid().to_owned(); + + if !same_handoff_projection { + record.head_sha = marker.pr_head_oid().to_owned(); + record.phase = String::from("request_pending"); + record.request_comment_database_id = None; + record.request_created_at_unix_epoch = None; + record.request_description_thumbs_up_count = None; + record.request_retry_count = 0; + record.external_round_count = 0; + record.auto_merge_enabled_at_unix_epoch = None; + record.landing_state = String::from("not_started"); + record.closeout_state = String::from("not_started"); + record.repair_attempt_count = 0; + record.evidence_json = String::from("{}"); + + record.next_action.clear(); + } - state.review_handoffs.insert( - key, - ReviewHandoffRuntimeRecord { - project_id: project_id.to_owned(), - issue_id: issue_id.to_owned(), - branch_name: marker.branch_name().to_owned(), - marker: marker.clone(), - updated_at: now.text, - updated_at_unix: now.unix, - }, - ); + record.updated_at = now.text; + record.updated_at_unix = now.unix; self.persist_runtime_state_locked(&state) } - /// Read the retained review handoff marker for one issue branch from the runtime DB. + /// Read the retained review handoff projection for one issue branch. pub(crate) fn review_handoff_marker( &self, project_id: &str, issue_id: &str, branch_name: &str, ) -> Result> { + Ok(self + .review_lifecycle_record(project_id, issue_id, branch_name)? + .map(|record| ReviewHandoffMarker { + run_id: record.run_id().to_owned(), + attempt_number: record.attempt_number(), + branch_name: record.branch_name().to_owned(), + pr_url: record.pr_url().to_owned(), + target_base_ref_name: record.target_base_ref_name().map(str::to_owned), + pr_head_ref_name: record.pr_head_ref_name().to_owned(), + pr_head_oid: record.pr_head_oid().to_owned(), + })) + } + + /// Read the runtime-owned review lifecycle record for one retained issue branch. + pub(crate) fn review_lifecycle_record( + &self, + project_id: &str, + issue_id: &str, + branch_name: &str, + ) -> Result> { let state = self.lock()?; - let key = ReviewMarkerKey::new(project_id, issue_id, branch_name); + let key = ReviewLifecycleKey::new(project_id, issue_id, branch_name); - Ok(state.review_handoffs.get(&key).map(|record| record.marker.clone())) + Ok(state.review_lifecycle_records.get(&key).map(ReviewLifecycleRuntimeRecord::as_public)) } - /// Create or replace the retained review orchestration marker for one issue lane. + /// Create or replace the retained review orchestration projection for one issue lane. pub(crate) fn upsert_review_orchestration_marker( &self, project_id: &str, @@ -2436,28 +2525,52 @@ impl StateStore { marker: &ReviewOrchestrationMarker, ) -> Result<()> { let now = timestamp_parts(); - let key = ReviewOrchestrationKey::new( - project_id, - issue_id, - marker.branch_name(), - marker.run_id(), - marker.attempt_number(), - ); + let key = ReviewLifecycleKey::new(project_id, issue_id, marker.branch_name()); let mut state = self.lock()?; + let record = + state.review_lifecycle_records.entry(key).or_insert_with(|| { + ReviewLifecycleRuntimeRecord { + project_id: project_id.to_owned(), + issue_id: issue_id.to_owned(), + branch_name: marker.branch_name().to_owned(), + run_id: marker.run_id().to_owned(), + attempt_number: marker.attempt_number(), + pr_url: marker.pr_url().to_owned(), + target_base_ref_name: None, + pr_head_ref_name: marker.branch_name().to_owned(), + pr_head_oid: marker.head_sha().to_owned(), + head_sha: marker.head_sha().to_owned(), + phase: marker.phase().to_owned(), + request_comment_database_id: None, + request_created_at_unix_epoch: None, + request_description_thumbs_up_count: None, + request_retry_count: 0, + external_round_count: 0, + auto_merge_enabled_at_unix_epoch: None, + landing_state: String::from("not_started"), + closeout_state: String::from("not_started"), + repair_attempt_count: 0, + evidence_json: String::from("{}"), + next_action: String::new(), + updated_at: now.text.clone(), + updated_at_unix: now.unix, + } + }); - state.review_orchestrations.insert( - key, - ReviewOrchestrationRuntimeRecord { - project_id: project_id.to_owned(), - issue_id: issue_id.to_owned(), - branch_name: marker.branch_name().to_owned(), - run_id: marker.run_id().to_owned(), - attempt_number: marker.attempt_number(), - marker: marker.clone(), - updated_at: now.text, - updated_at_unix: now.unix, - }, - ); + record.run_id = marker.run_id().to_owned(); + record.attempt_number = marker.attempt_number(); + record.pr_url = marker.pr_url().to_owned(); + record.head_sha = marker.head_sha().to_owned(); + record.phase = marker.phase().to_owned(); + record.request_comment_database_id = marker.request_comment_database_id(); + record.request_created_at_unix_epoch = marker.request_created_at_unix_epoch(); + record.request_description_thumbs_up_count = + marker.request_description_thumbs_up_count(); + record.request_retry_count = marker.request_retry_count(); + record.external_round_count = marker.external_round_count(); + record.auto_merge_enabled_at_unix_epoch = marker.auto_merge_enabled_at_unix_epoch(); + record.updated_at = now.text; + record.updated_at_unix = now.unix; self.persist_runtime_state_locked(&state) } @@ -2469,16 +2582,34 @@ impl StateStore { issue_id: &str, review_handoff: &ReviewHandoffMarker, ) -> Result> { - let state = self.lock()?; - let key = ReviewOrchestrationKey::new( - project_id, - issue_id, - review_handoff.branch_name(), - review_handoff.run_id(), - review_handoff.attempt_number(), - ); + let Some(record) = + self.review_lifecycle_record(project_id, issue_id, review_handoff.branch_name())? + else { + return Ok(None); + }; - Ok(state.review_orchestrations.get(&key).map(|record| record.marker.clone())) + if record.run_id() != review_handoff.run_id() + || record.attempt_number() != review_handoff.attempt_number() + || record.branch_name() != review_handoff.branch_name() + || record.pr_url() != review_handoff.pr_url() + { + return Ok(None); + } + + Ok(Some(ReviewOrchestrationMarker::new( + record.run_id().to_owned(), + record.attempt_number(), + record.branch_name().to_owned(), + record.pr_url().to_owned(), + record.head_sha().to_owned(), + record.phase().to_owned(), + record.request_comment_database_id(), + record.request_created_at_unix_epoch(), + record.request_description_thumbs_up_count(), + record.request_retry_count(), + record.external_round_count(), + record.auto_merge_enabled_at_unix_epoch(), + ))) } /// Create or replace the latest review-policy checkpoint for one run phase. @@ -2634,26 +2765,25 @@ impl StateStore { self.delete_loop_guardrail_checkpoint_locked(project_id, issue_id, reason) } - /// Remove the exact retained review markers created for one handoff identity. - pub(crate) fn clear_review_markers_for_handoff( + /// Remove the exact review lifecycle record created for one handoff identity. + pub(crate) fn clear_review_lifecycle_for_handoff( &self, project_id: &str, issue_id: &str, handoff_marker: &ReviewHandoffMarker, orchestration_marker: &ReviewOrchestrationMarker, ) -> Result<()> { - let handoff_key = ReviewMarkerKey::new(project_id, issue_id, handoff_marker.branch_name()); - let orchestration_key = ReviewOrchestrationKey::new( - project_id, - issue_id, - orchestration_marker.branch_name(), - orchestration_marker.run_id(), - orchestration_marker.attempt_number(), - ); + let lifecycle_key = ReviewLifecycleKey::new(project_id, issue_id, handoff_marker.branch_name()); let mut state = self.lock()?; - state.review_handoffs.remove(&handoff_key); - state.review_orchestrations.remove(&orchestration_key); + if state + .review_lifecycle_records + .get(&lifecycle_key) + .is_some_and(|record| record.matches_handoff_identity(handoff_marker)) + { + state.review_lifecycle_records.remove(&lifecycle_key); + } + state.review_policy_checkpoints.retain(|key, _record| { key.project_id != project_id || key.issue_id != issue_id @@ -2709,16 +2839,15 @@ impl StateStore { let mut state = self.lock()?; state.worktrees.remove(issue_id); - state.review_handoffs.retain(|key, _record| key.issue_id != issue_id); state - .review_orchestrations + .review_lifecycle_records .retain(|key, _record| key.issue_id != issue_id); state .review_policy_checkpoints .retain(|key, _record| key.issue_id != issue_id); self.persist_runtime_state_locked(&state)?; - self.delete_worktree_and_review_markers_locked(issue_id) + self.delete_worktree_and_review_lifecycle_locked(issue_id) } fn lock_without_refresh(&self) -> Result> { @@ -3054,7 +3183,7 @@ impl StateStore { sqlite.retarget_issue_identity(previous_issue_id, canonical_issue_id) } - fn delete_worktree_and_review_markers_locked(&self, issue_id: &str) -> Result<()> { + fn delete_worktree_and_review_lifecycle_locked(&self, issue_id: &str) -> Result<()> { let Some(sqlite) = self.sqlite.as_ref() else { return Ok(()); }; @@ -3062,7 +3191,7 @@ impl StateStore { .lock() .map_err(|_| eyre::eyre!("StateStore SQLite mutex is poisoned."))?; - sqlite.delete_worktree_and_review_markers(issue_id) + sqlite.delete_worktree_and_review_lifecycle(issue_id) } fn delete_review_marker_identity_locked( @@ -3728,8 +3857,8 @@ fn timestamp_text_from_unix(unix_epoch: i64) -> String { .unwrap_or_else(|| timestamp_parts().text) } -fn retarget_review_handoff_issue( - records: &mut HashMap, +fn retarget_review_lifecycle_issue( + records: &mut HashMap, previous_issue_id: &str, canonical_issue_id: &str, ) { @@ -3747,7 +3876,7 @@ fn retarget_review_handoff_issue( record.issue_id = canonical_issue_id.to_owned(); records - .entry(ReviewMarkerKey::new(&key.project_id, canonical_issue_id, &key.branch_name)) + .entry(ReviewLifecycleKey::new(&key.project_id, canonical_issue_id, &key.branch_name)) .or_insert(record); } } @@ -4111,33 +4240,3 @@ fn run_control_action_failure_class( Some("run_control_action_failed") } - -fn retarget_review_orchestration_issue( - records: &mut HashMap, - previous_issue_id: &str, - canonical_issue_id: &str, -) { - let previous_keys = records - .keys() - .filter(|key| key.issue_id == previous_issue_id) - .cloned() - .collect::>(); - - for key in previous_keys { - let Some(mut record) = records.remove(&key) else { - continue; - }; - - record.issue_id = canonical_issue_id.to_owned(); - - records - .entry(ReviewOrchestrationKey::new( - &key.project_id, - canonical_issue_id, - &key.branch_name, - &key.run_id, - key.attempt_number, - )) - .or_insert(record); - } -} diff --git a/apps/decodex/src/state/tests.rs b/apps/decodex/src/state/tests.rs index e31479f3..29ba7bea 100644 --- a/apps/decodex/src/state/tests.rs +++ b/apps/decodex/src/state/tests.rs @@ -35,6 +35,92 @@ use crate::{ }; const IN_PROGRESS_STATE: &str = "In Progress"; +const REVIEW_LIFECYCLE_MIGRATION_FIXTURE: &str = r#" +CREATE TABLE review_handoffs ( + project_id TEXT NOT NULL, + issue_id TEXT NOT NULL, + branch_name TEXT NOT NULL, + run_id TEXT NOT NULL, + attempt_number INTEGER NOT NULL, + pr_url TEXT NOT NULL, + target_base_ref_name TEXT, + pr_head_ref_name TEXT NOT NULL, + pr_head_oid TEXT NOT NULL, + updated_at TEXT NOT NULL, + updated_at_unix INTEGER NOT NULL, + PRIMARY KEY (project_id, issue_id, branch_name) +); +CREATE TABLE review_orchestrations ( + project_id TEXT NOT NULL, + issue_id TEXT NOT NULL, + branch_name TEXT NOT NULL, + run_id TEXT NOT NULL, + attempt_number INTEGER NOT NULL, + pr_url TEXT NOT NULL, + head_sha TEXT NOT NULL, + phase TEXT NOT NULL, + request_comment_database_id INTEGER, + request_created_at_unix_epoch INTEGER, + request_description_thumbs_up_count INTEGER, + request_retry_count INTEGER NOT NULL, + external_round_count INTEGER NOT NULL, + auto_merge_enabled_at_unix_epoch INTEGER, + updated_at TEXT NOT NULL, + updated_at_unix INTEGER NOT NULL, + PRIMARY KEY (project_id, issue_id, branch_name, run_id, attempt_number) +); +INSERT INTO review_handoffs ( + project_id, issue_id, branch_name, run_id, attempt_number, pr_url, + target_base_ref_name, pr_head_ref_name, pr_head_oid, updated_at, updated_at_unix +) VALUES ( + 'pubfi', 'PUB-101', 'x/decodex-pub-101', 'run-1', 2, + 'https://github.com/hack-ink/decodex/pull/101', 'main', 'x/decodex-pub-101', + '08a20f7dfb9526e7421a5f095b1c6adec84e52d6', '2026-06-17T01:00:00Z', + 1771290000 +); +INSERT INTO review_orchestrations ( + project_id, issue_id, branch_name, run_id, attempt_number, pr_url, head_sha, + phase, request_comment_database_id, request_created_at_unix_epoch, + request_description_thumbs_up_count, request_retry_count, external_round_count, + auto_merge_enabled_at_unix_epoch, updated_at, updated_at_unix +) VALUES ( + 'pubfi', 'PUB-101', 'x/decodex-pub-101', 'run-1', 2, + 'https://github.com/hack-ink/decodex/pull/101', + '19b20f7dfb9526e7421a5f095b1c6adec84e52d7', 'waiting_for_ack', 1234, + 1771290030, 4, 1, 3, 1771290060, '2026-06-17T01:01:00Z', 1771290060 +); +INSERT INTO review_orchestrations ( + project_id, issue_id, branch_name, run_id, attempt_number, pr_url, head_sha, + phase, request_comment_database_id, request_created_at_unix_epoch, + request_description_thumbs_up_count, request_retry_count, external_round_count, + auto_merge_enabled_at_unix_epoch, updated_at, updated_at_unix +) VALUES ( + 'pubfi', 'PUB-202', 'x/decodex-pub-202', 'run-2', 1, + 'https://github.com/hack-ink/decodex/pull/202', + '28c20f7dfb9526e7421a5f095b1c6adec84e52d8', 'request_pending', NULL, + NULL, NULL, 0, 1, NULL, '2026-06-17T01:02:00Z', 1771290120 +); +INSERT INTO review_handoffs ( + project_id, issue_id, branch_name, run_id, attempt_number, pr_url, + target_base_ref_name, pr_head_ref_name, pr_head_oid, updated_at, updated_at_unix +) VALUES ( + 'pubfi', 'PUB-303', 'x/decodex-pub-303', 'run-2', 1, + 'https://github.com/hack-ink/decodex/pull/303', 'main', 'x/decodex-pub-303', + '38c20f7dfb9526e7421a5f095b1c6adec84e52d8', '2026-06-17T01:03:00Z', + 1771290180 +); +INSERT INTO review_orchestrations ( + project_id, issue_id, branch_name, run_id, attempt_number, pr_url, head_sha, + phase, request_comment_database_id, request_created_at_unix_epoch, + request_description_thumbs_up_count, request_retry_count, external_round_count, + auto_merge_enabled_at_unix_epoch, updated_at, updated_at_unix +) VALUES ( + 'pubfi', 'PUB-303', 'x/decodex-pub-303', 'run-1', 1, + 'https://github.com/hack-ink/decodex/pull/303', + '39c20f7dfb9526e7421a5f095b1c6adec84e52d9', 'waiting_for_ack', 4321, + 1771290240, 5, 2, 4, 1771290300, '2026-06-17T01:04:00Z', 1771290240 +); +"#; #[cfg(unix)] fn fd_has_close_on_exec(fd: i32) -> bool { @@ -131,6 +217,14 @@ fn assert_decision_contract_retargeted(reopened: &StateStore) { ); } +fn seed_review_lifecycle_migration_fixture(state_path: &Path) { + let connection = Connection::open(state_path).expect("fixture db should open"); + + connection + .execute_batch(REVIEW_LIFECYCLE_MIGRATION_FIXTURE) + .expect("review lifecycle migration fixture should seed"); +} + fn upsert_handoff_review_policy_checkpoint( store: &StateStore, issue_id: &str, @@ -241,7 +335,7 @@ fn loop_guardrail_checkpoints_track_fingerprints_and_retarget_issue() { } #[test] -fn review_markers_roundtrip_preserve_required_fields() { +fn review_lifecycle_record_roundtrip_preserves_required_fields_and_projection() { let store = StateStore::open_in_memory().expect("state store should open"); let handoff = ReviewHandoffMarker::new( "run-1", @@ -255,12 +349,12 @@ fn review_markers_roundtrip_preserve_required_fields() { store .upsert_review_handoff_marker("pubfi", "PUB-101", &handoff) - .expect("review handoff marker should persist"); + .expect("review handoff projection should persist"); let restored_handoff = store .review_handoff_marker("pubfi", "PUB-101", "x/decodex-pub-101") - .expect("review handoff marker should read") - .expect("review handoff marker should exist"); + .expect("review handoff projection should read") + .expect("review handoff projection should exist"); assert_eq!(restored_handoff, handoff); @@ -281,14 +375,231 @@ fn review_markers_roundtrip_preserve_required_fields() { store .upsert_review_orchestration_marker("pubfi", "PUB-101", &orchestration) - .expect("review orchestration marker should persist"); + .expect("review orchestration projection should persist"); + + let lifecycle = store + .review_lifecycle_record("pubfi", "PUB-101", "x/decodex-pub-101") + .expect("review lifecycle record should read") + .expect("review lifecycle record should exist"); + + assert_eq!(lifecycle.project_id(), "pubfi"); + assert_eq!(lifecycle.issue_id(), "PUB-101"); + assert_eq!(lifecycle.branch_name(), "x/decodex-pub-101"); + assert_eq!(lifecycle.run_id(), "run-1"); + assert_eq!(lifecycle.attempt_number(), 2); + assert_eq!(lifecycle.pr_url(), "https://github.com/hack-ink/decodex/pull/101"); + assert_eq!(lifecycle.target_base_ref_name(), Some("main")); + assert_eq!(lifecycle.pr_head_ref_name(), "x/decodex-pub-101"); + assert_eq!(lifecycle.pr_head_oid(), "08a20f7dfb9526e7421a5f095b1c6adec84e52d6"); + assert_eq!(lifecycle.head_sha(), "08a20f7dfb9526e7421a5f095b1c6adec84e52d6"); + assert_eq!(lifecycle.phase(), "waiting_for_ack"); + assert_eq!(lifecycle.request_comment_database_id(), Some(1_234)); + assert_eq!(lifecycle.request_created_at_unix_epoch(), Some(1_775_200_000)); + assert_eq!(lifecycle.request_description_thumbs_up_count(), Some(3)); + assert_eq!(lifecycle.request_retry_count(), 1); + assert_eq!(lifecycle.external_round_count(), 2); + assert_eq!(lifecycle.auto_merge_enabled_at_unix_epoch(), Some(1_775_200_900)); + assert_eq!(lifecycle.landing_state(), "not_started"); + assert_eq!(lifecycle.closeout_state(), "not_started"); + assert_eq!(lifecycle.repair_attempt_count(), 0); + assert_eq!(lifecycle.evidence_json(), "{}"); + assert_eq!(lifecycle.next_action(), ""); + assert!(!lifecycle.updated_at().is_empty()); + assert!(lifecycle.updated_at_unix() > 0); + + store + .upsert_review_handoff_marker("pubfi", "PUB-101", &handoff) + .expect("same handoff projection should persist without resetting lifecycle state"); + + let lifecycle = store + .review_lifecycle_record("pubfi", "PUB-101", "x/decodex-pub-101") + .expect("review lifecycle record should read after same handoff") + .expect("review lifecycle record should exist after same handoff"); + + assert_eq!(lifecycle.phase(), "waiting_for_ack"); + assert_eq!(lifecycle.request_comment_database_id(), Some(1_234)); let restored_orchestration = store .review_orchestration_marker("pubfi", "PUB-101", &handoff) - .expect("review orchestration marker should read") - .expect("review orchestration marker should exist"); + .expect("review orchestration projection should read") + .expect("review orchestration projection should exist"); assert_eq!(restored_orchestration, orchestration); + + let snapshot = store + .project_loop_evidence_snapshot("pubfi") + .expect("project loop evidence snapshot should read"); + let snapshot_lifecycle = snapshot + .review_lifecycle_record("PUB-101", "x/decodex-pub-101") + .expect("snapshot review lifecycle should exist"); + + assert_eq!(snapshot_lifecycle, &lifecycle); +} + +#[test] +fn changed_review_handoff_projection_resets_lifecycle_phase_fields() { + let store = StateStore::open_in_memory().expect("state store should open"); + let old_handoff = ReviewHandoffMarker::new( + "run-1", + 1, + "x/decodex-pub-101", + "https://github.com/hack-ink/decodex/pull/101", + "main", + "x/decodex-pub-101", + "08a20f7dfb9526e7421a5f095b1c6adec84e52d6", + ); + let old_orchestration = ReviewOrchestrationMarker::new( + "run-1", + 1, + "x/decodex-pub-101", + "https://github.com/hack-ink/decodex/pull/101", + "19b20f7dfb9526e7421a5f095b1c6adec84e52d7", + "waiting_for_ack", + Some(1_234), + Some(1_775_200_000), + Some(3), + 2, + 4, + Some(1_775_200_900), + ); + let new_handoff = ReviewHandoffMarker::new( + "run-2", + 1, + "x/decodex-pub-101", + "https://github.com/hack-ink/decodex/pull/101", + "main", + "x/decodex-pub-101", + "28c20f7dfb9526e7421a5f095b1c6adec84e52d8", + ); + + store + .upsert_review_handoff_marker("pubfi", "PUB-101", &old_handoff) + .expect("old handoff projection should persist"); + store + .upsert_review_orchestration_marker("pubfi", "PUB-101", &old_orchestration) + .expect("old orchestration projection should persist"); + store + .upsert_review_handoff_marker("pubfi", "PUB-101", &new_handoff) + .expect("changed handoff projection should persist"); + + let lifecycle = store + .review_lifecycle_record("pubfi", "PUB-101", "x/decodex-pub-101") + .expect("review lifecycle record should read") + .expect("review lifecycle record should exist"); + + assert_eq!(lifecycle.run_id(), "run-2"); + assert_eq!(lifecycle.pr_head_oid(), "28c20f7dfb9526e7421a5f095b1c6adec84e52d8"); + assert_eq!(lifecycle.head_sha(), "28c20f7dfb9526e7421a5f095b1c6adec84e52d8"); + assert_eq!(lifecycle.phase(), "request_pending"); + assert_eq!(lifecycle.request_comment_database_id(), None); + assert_eq!(lifecycle.request_created_at_unix_epoch(), None); + assert_eq!(lifecycle.request_description_thumbs_up_count(), None); + assert_eq!(lifecycle.request_retry_count(), 0); + assert_eq!(lifecycle.external_round_count(), 0); + assert_eq!(lifecycle.auto_merge_enabled_at_unix_epoch(), None); + assert_eq!(lifecycle.landing_state(), "not_started"); + assert_eq!(lifecycle.closeout_state(), "not_started"); + assert_eq!(lifecycle.repair_attempt_count(), 0); + assert_eq!(lifecycle.evidence_json(), "{}"); + assert_eq!(lifecycle.next_action(), ""); + + let orchestration = store + .review_orchestration_marker("pubfi", "PUB-101", &new_handoff) + .expect("new orchestration projection should read") + .expect("new orchestration projection should exist"); + + assert_eq!(orchestration.run_id(), "run-2"); + assert_eq!(orchestration.head_sha(), "28c20f7dfb9526e7421a5f095b1c6adec84e52d8"); + assert_eq!(orchestration.phase(), "request_pending"); + assert_eq!(orchestration.request_retry_count(), 0); + assert_eq!(orchestration.external_round_count(), 0); +} + +#[test] +fn historical_review_marker_tables_migrate_into_lifecycle_record() { + let temp_dir = TempDir::new().expect("tempdir should create"); + let state_path = temp_dir.path().join("runtime.sqlite3"); + + seed_review_lifecycle_migration_fixture(&state_path); + + let store = + StateStore::open(&state_path).expect("state store should migrate historical markers"); + let handoff = store + .review_handoff_marker("pubfi", "PUB-101", "x/decodex-pub-101") + .expect("migrated handoff projection should read") + .expect("migrated handoff projection should exist"); + + assert_eq!(handoff.run_id(), "run-1"); + assert_eq!(handoff.attempt_number(), 2); + assert_eq!(handoff.pr_head_oid(), "08a20f7dfb9526e7421a5f095b1c6adec84e52d6"); + + let lifecycle = store + .review_lifecycle_record("pubfi", "PUB-101", "x/decodex-pub-101") + .expect("migrated lifecycle record should read") + .expect("migrated lifecycle record should exist"); + + assert_eq!(lifecycle.pr_head_oid(), "08a20f7dfb9526e7421a5f095b1c6adec84e52d6"); + assert_eq!(lifecycle.head_sha(), "19b20f7dfb9526e7421a5f095b1c6adec84e52d7"); + assert_eq!(lifecycle.phase(), "waiting_for_ack"); + assert_eq!(lifecycle.request_comment_database_id(), Some(1_234)); + assert_eq!(lifecycle.request_created_at_unix_epoch(), Some(1_771_290_030)); + assert_eq!(lifecycle.request_description_thumbs_up_count(), Some(4)); + assert_eq!(lifecycle.request_retry_count(), 1); + assert_eq!(lifecycle.external_round_count(), 3); + assert_eq!(lifecycle.auto_merge_enabled_at_unix_epoch(), Some(1_771_290_060)); + + let orphan_lifecycle = store + .review_lifecycle_record("pubfi", "PUB-202", "x/decodex-pub-202") + .expect("orphan historical orchestration should read") + .expect("orphan historical orchestration should migrate"); + + assert_eq!(orphan_lifecycle.target_base_ref_name(), None); + assert_eq!(orphan_lifecycle.pr_head_ref_name(), "x/decodex-pub-202"); + assert_eq!(orphan_lifecycle.pr_head_oid(), "28c20f7dfb9526e7421a5f095b1c6adec84e52d8"); + assert_eq!(orphan_lifecycle.head_sha(), "28c20f7dfb9526e7421a5f095b1c6adec84e52d8"); + + let stale_orchestration_lifecycle = store + .review_lifecycle_record("pubfi", "PUB-303", "x/decodex-pub-303") + .expect("stale historical orchestration should read") + .expect("current historical handoff should migrate"); + + assert_eq!(stale_orchestration_lifecycle.run_id(), "run-2"); + assert_eq!(stale_orchestration_lifecycle.attempt_number(), 1); + assert_eq!( + stale_orchestration_lifecycle.pr_head_oid(), + "38c20f7dfb9526e7421a5f095b1c6adec84e52d8" + ); + assert_eq!( + stale_orchestration_lifecycle.head_sha(), + "38c20f7dfb9526e7421a5f095b1c6adec84e52d8" + ); + assert_eq!(stale_orchestration_lifecycle.phase(), "request_pending"); + assert_eq!(stale_orchestration_lifecycle.request_comment_database_id(), None); + assert_eq!(stale_orchestration_lifecycle.request_created_at_unix_epoch(), None); + assert_eq!(stale_orchestration_lifecycle.request_description_thumbs_up_count(), None); + assert_eq!(stale_orchestration_lifecycle.request_retry_count(), 0); + assert_eq!(stale_orchestration_lifecycle.external_round_count(), 0); + assert_eq!(stale_orchestration_lifecycle.auto_merge_enabled_at_unix_epoch(), None); + + drop(store); + + let connection = Connection::open(&state_path).expect("migrated db should open"); + let legacy_table_count: i64 = connection + .query_row( + "SELECT COUNT(*) FROM sqlite_master \ + WHERE type = 'table' AND name IN ('review_handoffs', 'review_orchestrations')", + [], + |row| row.get(0), + ) + .expect("legacy marker tables should query"); + + assert_eq!(legacy_table_count, 0); + + let lifecycle_count: i64 = connection + .query_row("SELECT COUNT(*) FROM review_lifecycle_records", [], |row| row.get(0)) + .expect("review lifecycle rows should query"); + + assert_eq!(lifecycle_count, 3); } #[test] @@ -336,7 +647,7 @@ fn connector_backoff_roundtrip_and_clear_from_runtime_store() { } #[test] -fn clear_review_markers_for_handoff_preserves_other_branches() { +fn clear_review_lifecycle_for_handoff_preserves_other_branches() { let temp_dir = TempDir::new().expect("tempdir should create"); let state_path = temp_dir.path().join("runtime.sqlite3"); let store = StateStore::open(&state_path).expect("state store should open"); @@ -368,10 +679,10 @@ fn clear_review_markers_for_handoff_preserves_other_branches() { store .upsert_review_handoff_marker("pubfi", "PUB-101", &removed_handoff) - .expect("removed handoff marker should persist"); + .expect("removed handoff projection should persist"); store .upsert_review_orchestration_marker("pubfi", "PUB-101", &removed_orchestration) - .expect("removed orchestration marker should persist"); + .expect("removed orchestration projection should persist"); upsert_handoff_review_policy_checkpoint( &store, @@ -384,10 +695,10 @@ fn clear_review_markers_for_handoff_preserves_other_branches() { store .upsert_review_handoff_marker("pubfi", "PUB-101", &kept_handoff) - .expect("kept handoff marker should persist"); + .expect("kept handoff projection should persist"); store .upsert_review_orchestration_marker("pubfi", "PUB-101", &kept_orchestration) - .expect("kept orchestration marker should persist"); + .expect("kept orchestration projection should persist"); upsert_handoff_review_policy_checkpoint( &store, @@ -399,32 +710,32 @@ fn clear_review_markers_for_handoff_preserves_other_branches() { ); store - .clear_review_markers_for_handoff( + .clear_review_lifecycle_for_handoff( "pubfi", "PUB-101", &removed_handoff, &removed_orchestration, ) - .expect("exact review markers should clear"); + .expect("exact review lifecycle should clear"); let reopened = StateStore::open(&state_path).expect("reopened state store should open"); assert!( reopened .review_handoff_marker("pubfi", "PUB-101", "x/decodex-pub-101") - .expect("removed handoff marker should read") + .expect("removed handoff projection should read") .is_none() ); assert_eq!( reopened .review_handoff_marker("pubfi", "PUB-101", "x/decodex-pub-101-review") - .expect("kept handoff marker should read"), + .expect("kept handoff projection should read"), Some(kept_handoff.clone()) ); assert_eq!( reopened .review_orchestration_marker("pubfi", "PUB-101", &kept_handoff) - .expect("kept orchestration marker should read"), + .expect("kept orchestration projection should read"), Some(kept_orchestration) ); assert!( @@ -444,7 +755,7 @@ fn clear_review_markers_for_handoff_preserves_other_branches() { } #[test] -fn missing_review_markers_return_absent() { +fn missing_review_lifecycle_projections_return_absent() { let store = StateStore::open_in_memory().expect("state store should open"); let handoff = ReviewHandoffMarker::new( "run-1", @@ -459,13 +770,13 @@ fn missing_review_markers_return_absent() { assert!( store .review_handoff_marker("pubfi", "PUB-101", "x/decodex-pub-101") - .expect("review handoff marker should read") + .expect("review handoff projection should read") .is_none() ); assert!( store .review_orchestration_marker("pubfi", "PUB-101", &handoff) - .expect("review orchestration marker should read") + .expect("review orchestration projection should read") .is_none() ); } @@ -524,7 +835,7 @@ fn review_policy_checkpoints_persist_reload_and_clear_for_run_attempt() { } #[test] -fn persistent_review_markers_survive_stale_store_persist_and_are_visible() { +fn persistent_review_lifecycle_survives_stale_store_persist_and_is_visible() { let temp_dir = TempDir::new().expect("tempdir should create"); let state_path = temp_dir.path().join("runtime.sqlite3"); let observer = StateStore::open(&state_path).expect("observer state store should open"); @@ -555,15 +866,15 @@ fn persistent_review_markers_survive_stale_store_persist_and_are_visible() { writer .upsert_review_handoff_marker("pubfi", "PUB-101", &handoff) - .expect("handoff marker should persist"); + .expect("handoff projection should persist"); writer .upsert_review_orchestration_marker("pubfi", "PUB-101", &orchestration) - .expect("orchestration marker should persist"); + .expect("orchestration projection should persist"); let observed_handoff = observer .review_handoff_marker("pubfi", "PUB-101", "x/decodex-pub-101") - .expect("observer should read handoff marker") - .expect("observer should see marker written by another store"); + .expect("observer should read handoff projection") + .expect("observer should see lifecycle written by another store"); assert_eq!(observed_handoff, handoff); @@ -576,13 +887,13 @@ fn persistent_review_markers_survive_stale_store_persist_and_are_visible() { assert_eq!( reopened .review_handoff_marker("pubfi", "PUB-101", "x/decodex-pub-101") - .expect("reopened store should read handoff marker"), + .expect("reopened store should read handoff projection"), Some(handoff.clone()) ); assert_eq!( reopened .review_orchestration_marker("pubfi", "PUB-101", &handoff) - .expect("reopened store should read orchestration marker"), + .expect("reopened store should read orchestration projection"), Some(orchestration) ); assert!( @@ -2564,7 +2875,7 @@ fn opens_legacy_worktree_rows_with_unknown_provenance() { } #[test] -fn persistent_clear_worktree_deletes_review_markers() { +fn persistent_clear_worktree_deletes_review_lifecycle() { let temp_dir = TempDir::new().expect("tempdir should create"); let state_path = temp_dir.path().join("runtime.sqlite3"); let store = StateStore::open(&state_path).expect("state store should open"); @@ -2597,10 +2908,10 @@ fn persistent_clear_worktree_deletes_review_markers() { .expect("worktree mapping should be recorded"); store .upsert_review_handoff_marker("pubfi", "PUB-101", &handoff) - .expect("handoff marker should persist"); + .expect("handoff projection should persist"); store .upsert_review_orchestration_marker("pubfi", "PUB-101", &orchestration) - .expect("orchestration marker should persist"); + .expect("orchestration projection should persist"); store.clear_worktree("PUB-101").expect("worktree cleanup should persist"); let reopened = StateStore::open(&state_path).expect("reopened store should open"); @@ -2655,10 +2966,10 @@ fn canonicalize_issue_identity_retargets_persistent_rows_without_cache_refresh() .expect("decision contract should persist"); writer .upsert_review_handoff_marker("pubfi", "PUB-101", &handoff) - .expect("handoff marker should persist"); + .expect("handoff projection should persist"); writer .upsert_review_orchestration_marker("pubfi", "PUB-101", &orchestration) - .expect("orchestration marker should persist"); + .expect("orchestration projection should persist"); upsert_handoff_review_policy_checkpoint( &writer, diff --git a/docs/reference/operator-control-plane.md b/docs/reference/operator-control-plane.md index 989169a4..51bf8852 100644 --- a/docs/reference/operator-control-plane.md +++ b/docs/reference/operator-control-plane.md @@ -8,7 +8,7 @@ owner: docs tags: [reference] code_refs: [apps/decodex/src/orchestrator/status.rs, apps/decodex/src/orchestrator/run_cycle.rs] drift_watch: [decodex serve, control_plane_snapshot, operator dashboard, runtime.sqlite3, project.toml, WORKFLOW.md] -last_verified: 2026-06-16 +last_verified: 2026-06-17 --- # Operator Control Plane @@ -489,21 +489,21 @@ Worktree visibility follows the owning dashboard section: - `Review & Landing` means a retained PR lane still owns the path for review repair, landing, closeout, or retained-lane cleanup. - `missing_review_handoff_record` in `Review & Landing` means Decodex found a retained - review worktree but cannot find the authoritative runtime DB handoff marker. Treat - this as an orphaned retained review lane: inspect it with + review worktree but cannot find the authoritative runtime DB review lifecycle + record. Treat this as an orphaned retained review lane: inspect it with `decodex recover review-handoff diagnose `, then use the explicit rebind path only after the PR URL and retained worktree lineage match exactly. -- Review handoff or orchestration head mismatch reasons mean Decodex found a retained - marker but one stored field no longer matches the clean retained worktree and PR - head. `decodex status` keeps the bound PR URL visible when it can identify the - marker, and `decodex recover review-handoff diagnose ` reports the stored - marker head, orchestration head, PR head, and mismatched field before any explicit - rebind refresh. +- Review lifecycle handoff or phase head mismatch reasons mean Decodex found a + retained lifecycle record but one stored field no longer matches the clean retained + worktree and PR head. `decodex status` keeps the bound PR URL visible when it can + identify the lifecycle record, and `decodex recover review-handoff diagnose ` + reports the stored handoff head, phase head, PR head, and mismatched field before + any explicit rebind refresh. - `pull_request_state_read_failed` in `Review & Landing` is a degraded PR readback - warning when the retained review handoff marker still exists. `decodex status` - must keep the issue identifier, branch, marker PR URL, and marker PR head SHA visible - so operators can retry status, inspect the PR directly, or run the explicit recovery - path without losing the bound PR identity. Local status JSON, text output, and + warning when the retained review lifecycle record still exists. `decodex status` + must keep the issue identifier, branch, lifecycle PR URL, and lifecycle PR head SHA + visible so operators can retry status, inspect the PR directly, or run the explicit + recovery path without losing the bound PR identity. Local status JSON, text output, and dashboard readback also carry `readback_root_cause` when Decodex can classify the local diagnostic safely, for example `missing_github_cli`, `missing_github_token`, `github_auth_failed`, `github_api_read_failed`, `github_response_parse_failed`, @@ -532,8 +532,9 @@ Worktree visibility follows the owning dashboard section: - `linear_active_label_present` in `Intake Queue` means the issue still carries service active ownership while it is also queued, but local status could not prove a matching run lease. Treat it as a recovery/attention row, not ready work. If its - attention cause is `evidence_missing`, use the retained marker, worktree, and public - Linear state as the available recovery evidence before retrying or cleaning labels. + attention cause is `evidence_missing`, use the retained runtime records, worktree, + and public Linear state as the available recovery evidence before retrying or + cleaning labels. - `Recovery Worktrees` means the path is retained local state after the authoritative runtime owner is gone or cannot explain it as active, review/landing, or queued work. @@ -563,7 +564,7 @@ cleanup, and cleanup-only local retention. Runtime-recorded mappings report `provenance.source = "runtime_recorded"` with created and refreshed Unix timestamps. Deterministically rebuilt mappings report `provenance.source = "runtime_recovered"` when tracker, -retained marker, or closeout evidence proves a current owner after local state was +retained lifecycle record, or closeout evidence proves a current owner after local state was missing. Filesystem-only scans use scan-specific provenance such as `filesystem_scan` or `git_hygiene_scan`. Rows migrated from older runtime stores that had no provenance report `provenance.source = "legacy_unknown"` and may set diff --git a/docs/reference/test-suite.md b/docs/reference/test-suite.md index 03f2cacb..b6685244 100644 --- a/docs/reference/test-suite.md +++ b/docs/reference/test-suite.md @@ -6,7 +6,7 @@ status: active authority: current_state owner: docs tags: [reference] -last_verified: 2026-06-16 +last_verified: 2026-06-17 --- # Test Suite @@ -103,7 +103,7 @@ large catch-all test file unless the behavior crosses several of these stages. | `apps/decodex/src/orchestrator/tests/review_landing/status_support.rs` | 0 | Shared Review & Landing status fixtures | | `apps/decodex/src/orchestrator/tests/review_landing/status_rows.rs` | 18 | Review & Landing status rows and handoff lineage | | `apps/decodex/src/orchestrator/tests/review_landing/orchestration.rs` | 17 | Review orchestration, admin merge, and repair routing | -| `apps/decodex/src/orchestrator/tests/review_landing/status_markers.rs` | 1 | Review orchestration marker handling and recovered targeted visibility | +| `apps/decodex/src/orchestrator/tests/review_landing/status_markers.rs` | 1 | Review lifecycle readback handling and recovered targeted visibility | | `apps/decodex/src/orchestrator/tests/review_landing/classification_review.rs` | 12 | Review repair, request-pending, stale handoff, merged PR classification | | `apps/decodex/src/orchestrator/tests/review_landing/classification_checks.rs` | 16 | Required checks, GitHub token gates, GraphQL pagination/query shape | | `apps/decodex/src/orchestrator/tests/review_landing/review_state.rs` | 2 | Pull-request review-state conversion from GitHub GraphQL nodes | @@ -126,8 +126,8 @@ Keep separate tests when the case protects a different observable contract: Git commands, runtime database state, or app-server protocol payloads. - Different state-machine outcome, especially blocked versus ineligible, retryable versus terminal, repair versus closeout, or queued versus retained. -- Different persisted marker semantics, such as review handoff lineage, retry schedule - marker, cleanup handoff marker, or closeout identity reuse. +- Different persisted lifecycle semantics, such as review handoff lineage, retry + schedule state, cleanup handoff state, or closeout identity reuse. - Different authority boundary, such as GitHub token routing, Linear tracker writes, repo-local Git config, or runtime-only state. - Different process or concurrency boundary, such as active child reconciliation, lock diff --git a/docs/runbook/index.md b/docs/runbook/index.md index 91282135..df3e65b8 100644 --- a/docs/runbook/index.md +++ b/docs/runbook/index.md @@ -43,7 +43,7 @@ Question this index answers: "which sequence should I execute?" - [`social-publishing-workflow.md`](./social-publishing-workflow.md) for turning Radar evidence into low-frequency `@decodexspace` X posts or blocked publication records. - [`recover-review-handoff.md`](./recover-review-handoff.md) for diagnosing retained - review lanes, explicitly rebinding missing or stale runtime DB handoff markers, and + review lanes, explicitly rebinding missing or stale runtime DB lifecycle records, and adopting verified human-owned PRs into the normal Decodex landing lifecycle. - [`review-config-migration.md`](./review-config-migration.md) for one-time migration from historical review config keys to `[codex].review` levels. diff --git a/docs/runbook/lane-control-recovery.md b/docs/runbook/lane-control-recovery.md index 59798c6b..61bfa759 100644 --- a/docs/runbook/lane-control-recovery.md +++ b/docs/runbook/lane-control-recovery.md @@ -6,7 +6,7 @@ status: active authority: procedural owner: automation tags: [runbook] -last_verified: 2026-06-16 +last_verified: 2026-06-17 --- # Lane-Control Recovery @@ -96,7 +96,7 @@ or clean labels. | Forced interrupt after `run_lease_missing` reports no signalable process. | Treat force as non-mutating for the child process. | Inspect retained worktree and private evidence; do not claim the interrupt succeeded or clear attention labels. | | Hard fallback reports `hard_interrupt_fallback`. | Treat it as an interrupted runtime event, not a graceful completion. | Inspect retained worktree and evidence; resume only if lineage is exact. | | Retained worktree has useful local changes and lineage matches issue, branch, runtime evidence, and PR when present. | Resume or repair the same lane. | Use `decodex run ` when the registered workflow makes it eligible, or use the specific retained recovery runbook. | -| Review handoff marker is missing or stale but the retained PR lane appears recoverable. | Diagnose before rebind. | Run `decodex recover review-handoff diagnose ` and follow [`recover-review-handoff.md`](./recover-review-handoff.md). | +| Review lifecycle record is missing or stale but the retained PR lane appears recoverable. | Diagnose before rebind. | Run `decodex recover review-handoff diagnose ` and follow [`recover-review-handoff.md`](./recover-review-handoff.md). | | Queue label or tracker state was changed and the scheduler should observe it before the next poll. | Request a refresh, not a retry. | `POST /api/linear-scan` with `projectId`, or no body for all enabled projects. | | Queue label should be added, removed, or interpreted. | Use service-scoped label policy. | Follow the `labels` skill; do not guess `` or clear `needs-attention` before fixing the blocker. | | Broad steer materially changes the objective or acceptance contract. | Preserve audit and resolve lifecycle explicitly. | Update and requeue the same issue, create a new issue/lane, or route the owned run to manual attention. | diff --git a/docs/runbook/recover-review-handoff.md b/docs/runbook/recover-review-handoff.md index 65870040..58a7f5a6 100644 --- a/docs/runbook/recover-review-handoff.md +++ b/docs/runbook/recover-review-handoff.md @@ -1,21 +1,22 @@ --- type: "Runbook" title: "Recover Review Handoff" -description: "Diagnose and explicitly repair retained review lanes that are blocked by a missing or stale runtime DB review-handoff marker." +description: "Diagnose and explicitly repair retained review lanes that are blocked by a missing or stale runtime DB review lifecycle record." status: active authority: procedural owner: automation tags: [runbook] -last_verified: 2026-06-16 +last_verified: 2026-06-17 --- # Recover Review Handoff Purpose: Diagnose and explicitly repair retained review lanes that are blocked by a -missing or stale runtime DB review-handoff marker. +missing or stale runtime DB review lifecycle record. Use this when: `decodex status` or the dashboard shows a `Review & Landing` lane -blocked with `missing_review_handoff_record`, a review handoff/orchestration head -mismatch, or a similar retained review marker mismatch after manual repair or rebase. +blocked with `missing_review_handoff_record`, a review lifecycle head or phase +mismatch, or a similar retained review lifecycle mismatch after manual repair or +rebase. Do not use this for: healthy PR handoffs, review repair, landing, closeout, cleanup-only worktrees, or manual PR landing. @@ -36,9 +37,9 @@ Use `--json` for a structured report. The diagnostic is read-only. It reports the issue, tracker state, branch, worktree, local branch, local head, worktree cleanliness, existing PR URL when one is already -bound, stored handoff head, stored orchestration head, PR base/head when readable, -the mismatched field when one is known, active automation label presence, and the -suggested next command. +bound, stored lifecycle handoff head, stored lifecycle phase head, PR base/head when +readable, the mismatched field when one is known, active automation label presence, +and the suggested next command. ## Explicit Rebind @@ -47,8 +48,8 @@ This is a break-glass path. Healthy lanes should keep using operators should not use rebind just because a normal review handoff is still in progress. -Only rebind when the diagnosis says the review handoff marker is absent, says the -existing same-branch same-PR marker must be refreshed after the retained worktree and +Only rebind when the diagnosis says the review lifecycle record is absent, says the +existing same-branch same-PR record must be refreshed after the retained worktree and PR head have been checked, or reports `review_handoff_state_transition_pending`. ```sh @@ -56,28 +57,28 @@ decodex recover review-handoff rebind --pr --dry-run decodex recover review-handoff rebind --pr ``` -The non-dry-run command writes runtime DB handoff/orchestration markers and records a -`review_handoff_rebind` audit event on the tracker issue. For a stale existing marker, +The non-dry-run command writes the runtime DB review lifecycle record and records a +`review_handoff_rebind` audit event on the tracker issue. For a stale existing record, it refreshes only the same branch and same PR after validating the clean retained -worktree head matches the PR head. For a partial normal handoff where the marker is -missing, or where an already-current marker exists but the issue state was not +worktree head matches the PR head. For a partial normal handoff where the record is +missing, or where an already-current record exists but the issue state was not advanced, the command may also move the issue from the workflow `tracker.in_progress_state` to `tracker.success_state` after the rebind audit -succeeds. If stale failure writeback already moved an already-current marker lane to +succeeds. If stale failure writeback already moved an already-current record lane to `tracker.failure_state` and added `tracker.needs_attention_label`, rebind may clear that label and move the issue to `tracker.success_state`; this is only for current same-PR -same-head markers, not for missing or stale marker recovery. It does not merge the PR, +same-head records, not for missing or stale record recovery. It does not merge the PR, queue follow-up issues, or clean worktrees. The command rejects the rebind unless all of these are true: - the issue is in the workflow `tracker.success_state`, still in `tracker.in_progress_state` from a partial normal handoff with a missing or - already-current marker, or in `tracker.failure_state` only when an already-current - marker proves that stale failure writeback caused tracker state drift + already-current lifecycle record, or in `tracker.failure_state` only when an + already-current record proves that stale failure writeback caused tracker state drift - the issue does not have the opt-out label - the issue does not have the needs-attention label, except for the already-current - marker plus `tracker.failure_state` drift case where rebind clears it after recording + record plus `tracker.failure_state` drift case where rebind clears it after recording the audit - the issue still has `decodex:active:` ownership - the retained worktree branch matches the runtime DB worktree mapping @@ -87,8 +88,8 @@ The command rejects the rebind unless all of these are true: - the PR targets the configured default branch - the PR is open and non-draft - the PR head branch and head SHA match the retained worktree -- no review handoff marker already exists for the issue/branch, or the existing marker - is for the same branch and PR and needs a head/orchestration refresh or pending issue +- no review lifecycle record already exists for the issue/branch, or the existing + record is for the same branch and PR and needs a head/phase refresh or pending issue state transition After a successful rebind, run: @@ -110,8 +111,8 @@ closeout path. If that issue already has a worktree mapping, adopt accepts it on the mapping points at the current managed checkout; a mapping for a different checkout is a fail-closed mismatch. Adopt rewrites that mapping to the current PR branch only after every dry-run/live validation passes. Do not use adopt for lanes that already -have a review handoff marker; those belong to `rebind`, normal `decodex land`, or the -retained post-review scheduler. +have a review lifecycle record; those belong to `rebind`, normal `decodex land`, or +the retained post-review scheduler. Run it from the lane worktree, not from the repo root: @@ -129,8 +130,8 @@ The command rejects the adopt unless all of these are true: `tracker.success_state` - no conflicting retained worktree mapping exists; an existing mapping is allowed only when it points at the current managed checkout -- no review handoff marker already exists for the issue's current branch or previously - mapped branch +- no review lifecycle record already exists for the issue's current branch or + previously mapped branch - the current checkout is a managed worktree under the configured `worktree_root` - the current worktree is clean except top-level Decodex runtime artifacts such as `.decodex-run-activity` and `.decodex-run-control/` @@ -140,7 +141,7 @@ The command rejects the adopt unless all of these are true: - the PR head branch and head SHA exactly match the current worktree branch and `HEAD` The non-dry-run command writes a runtime worktree mapping, a local takeover run -attempt, review handoff/orchestration markers, and a `review_handoff_adopt` audit +attempt, a review lifecycle record, and a `review_handoff_adopt` audit event. If the active service label was missing, live adopt restores it after local handoff state is written and before the audit event is recorded; if the audit write fails, the label restoration is rolled back. If the issue was still in the workflow @@ -183,12 +184,12 @@ plus explicit rebind before any manual cleanup. If diagnosis reports `classification: review_handoff_ownership_drift`, `reason: active_ownership_label_missing`, and `active_label_present: false`, do not run -rebind just to restore ownership. If the lane has no retained handoff marker because a -human PR needs manual takeover, run `recover review-handoff adopt --dry-run`; the dry -run reports `would_restore_active_label=true` when live adopt can restore the active -service label after validating the issue, managed worktree, PR branch, PR head, and -landability gates. If the lane already has a retained handoff marker, use the ordinary -diagnosis/rebind or post-review path instead of hand-adding labels. If the issue still +rebind just to restore ownership. If the lane has no retained lifecycle record because +a human PR needs manual takeover, run `recover review-handoff adopt --dry-run`; the +dry run reports `would_restore_active_label=true` when live adopt can restore the +active service label after validating the issue, managed worktree, PR branch, PR head, +and landability gates. If the lane already has a retained lifecycle record, use the +ordinary diagnosis/rebind or post-review path instead of hand-adding labels. If the issue still has `decodex:needs-attention`, clear that label only after the recorded blocker has been repaired or an explicit recovery command says it will clear the label itself. diff --git a/docs/spec/lane-control.md b/docs/spec/lane-control.md index 41802ddd..985d52d9 100644 --- a/docs/spec/lane-control.md +++ b/docs/spec/lane-control.md @@ -6,7 +6,7 @@ status: active authority: normative owner: runtime tags: [spec] -last_verified: 2026-06-16 +last_verified: 2026-06-17 --- # Lane-Control Specification @@ -244,8 +244,8 @@ When status or failure writeback reports `validation_repeat`, `no_effective_diff `remaining_delta_unchanged`, `review_churn`, `review_handoff_state_drift`, `dependency_program_stale`, `uncovered_direction`, or `ambiguous_retained_progress`, operators must inspect the retained worktree, private evidence, blocker state, -recovery packet, boundary check, review findings, or retained handoff marker named by -that reason before clearing `decodex:needs-attention`. +recovery packet, boundary check, review findings, or retained lifecycle record named +by that reason before clearing `decodex:needs-attention`. Do not use steer, retry, label cleanup, or hard interrupt to bypass the guardrail without changing the underlying repair strategy, dependency readiness, research contract, authority decision, or retained-progress ownership decision. diff --git a/docs/spec/linear-execution-ledger.md b/docs/spec/linear-execution-ledger.md index 250d0475..a345d102 100644 --- a/docs/spec/linear-execution-ledger.md +++ b/docs/spec/linear-execution-ledger.md @@ -6,7 +6,7 @@ status: active authority: normative owner: runtime tags: [spec] -last_verified: 2026-06-16 +last_verified: 2026-06-17 --- # Linear Execution Ledger @@ -281,12 +281,12 @@ When the exact failed command or raw error contains private information, produce omit it and use public `error_class`, `next_action`, `blockers`, and `evidence` instead. -`review_handoff_rebind` is only for an explicit operator recovery command that restores a -missing or stale runtime DB review handoff marker after validating the retained worktree -and PR lineage. `review_handoff_adopt` is only for the explicit manual takeover command -that adopts a human-owned PR into the retained review handoff shape after validating the -managed clean worktree, active service ownership, exact PR head, and green landable PR -gates. Neither event is a normal agent terminal signal, neither implies +`review_handoff_rebind` is only for an explicit operator recovery command that +restores a missing or stale runtime DB review lifecycle record after validating the +retained worktree and PR lineage. `review_handoff_adopt` is only for the explicit +manual takeover command that adopts a human-owned PR into the retained review +lifecycle shape after validating the managed clean worktree, active service ownership, +exact PR head, and green landable PR gates. Neither event is a normal agent terminal signal, neither implies `issue_terminal_finalize` ran, and neither must be emitted automatically from `decodex run`. diff --git a/docs/spec/post-review-lifecycle.md b/docs/spec/post-review-lifecycle.md index 088231b9..e758a4db 100644 --- a/docs/spec/post-review-lifecycle.md +++ b/docs/spec/post-review-lifecycle.md @@ -6,7 +6,7 @@ status: active authority: normative owner: runtime tags: [spec] -last_verified: 2026-06-16 +last_verified: 2026-06-17 --- # Post-Review Lifecycle @@ -75,7 +75,20 @@ Post-`In Review` classification may use only these signal groups: - whether closeout already ran - whether deterministic local cleanup remains pending -In the current runtime, the retained lane persists its validated review handoff in the Decodex runtime database and uses that row as the authoritative post-review lineage record for repair, landing, closeout, and cleanup. When that exact database handoff is missing, post-review ownership must block as unresolved instead of rebinding from branch-name, current-head-only heuristics, or Linear comments. +In the current runtime, the retained lane persists one `review_lifecycle_records` row +in the Decodex runtime database and uses that row as the authoritative post-review +lineage record for repair, landing, closeout, and cleanup. The record stores the PR +URL, base/head branch, validated PR head OID, run id, attempt number, current +post-review phase, review-request metadata, landing/closeout/repair state, evidence, +and next action. Handoff and orchestration tool-boundary shapes are projections of this +record; they are not separate durable authority. When that exact lifecycle record is +missing, post-review ownership must +block as unresolved instead of rebinding from branch-name, current-head-only +heuristics, or Linear comments. +Historical `review_handoffs` and `review_orchestrations` tables are one-time runtime +bootstrap migration input into `review_lifecycle_records`; after that migration, they +are dropped and must not be used as readback authority. Operators must use explicit +diagnose, rebind, or adopt recovery to create or refresh a current lifecycle record. If these signals disagree and the disagreement cannot be resolved without guessing operator intent, the runtime must use `manual_intervention_required`. @@ -86,47 +99,49 @@ not infer a PR lineage from branch names, current heads, PR titles, or Linear co and `decodex run` must not repair this state automatically. Failure writeback must also respect this post-review boundary. If an execution failure -arrives after a retained review handoff marker already binds the current issue, -branch, and local HEAD lineage, Decodex may self-heal state drift by rebuilding the -review orchestration marker, clearing loop guardrail checkpoints for the issue, and -moving the tracker issue back to `tracker.success_state` when the issue had drifted to -`tracker.in_progress_state` or `tracker.failure_state`. This is not implementation -repair and must happen before retry/no-diff loop guardrails run. If the retained marker -is absent, unverified, or diverged, Decodex must stop with +arrives after a retained review lifecycle record already binds the current issue, +branch, PR, and local HEAD lineage, Decodex may self-heal state drift by rebinding the +phase fields in that lifecycle record, clearing loop guardrail checkpoints for the +issue, and moving the tracker issue back to `tracker.success_state` when the issue had +drifted to `tracker.in_progress_state` or `tracker.failure_state`. This is not +implementation repair and must happen before retry/no-diff loop guardrails run. If the +retained lifecycle record is absent, unverified, or diverged, Decodex must stop with `review_handoff_state_drift` or the existing `missing_review_handoff_record` posture and require explicit recovery evidence rather than guessing a PR lineage. -When the retained review handoff marker exists but a direct PR-state read or local +When the retained review lifecycle record exists but a direct PR-state read or local worktree branch/head read fails, operator status must degrade the readback instead of replacing the bound lane with a null-PR blocked state. The status row must keep the -issue identifier, retained branch, marker PR URL, and marker head SHA, and it may expose -warnings such as `readback_warning = "pull_request_state_read_failed"`, +issue identifier, retained branch, lifecycle PR URL, and lifecycle head SHA, and it may +expose warnings such as `readback_warning = "pull_request_state_read_failed"`, `worktree_checkout_branch_read_failed`, or `worktree_head_read_failed` until the next successful readback. -Retained orchestration must preserve degraded readback as a wait state. It must not +Retained orchestration must preserve degraded readback as a wait state in the +lifecycle record. It must not convert `pull_request_state_read_failed`, `worktree_checkout_branch_read_failed`, `worktree_head_read_failed`, or other `WaitForReview` classifications into passive manual attention; only classifications whose decision is `Block` may add `decodex:needs-attention`. When Linear issue metadata readback is degraded by connector backoff, operator status -must still keep locally retained handoff rows visible with the marker PR URL and head -SHA and must mark the row as tracker-readback degraded instead of presenting the PR or -code state as failed. If the handoff is bound but `decodex:active:` is +must still keep locally retained lifecycle rows visible with the lifecycle PR URL and +head SHA and must mark the row as tracker-readback degraded instead of presenting the +PR or code state as failed. If the lifecycle record is bound but +`decodex:active:` is missing, recovery diagnosis must classify the state as ownership drift and give the single label repair action instead of recommending rebind. The supported operator recovery surface is `decodex recover review-handoff`. This is a -break-glass recovery path for orphaned retained review lanes and stale retained marker +break-glass recovery path for orphaned retained review lanes and stale lifecycle heads after explicit manual repair or rebase. It is not part of the normal automation success path. - `diagnose` is read-only. It reports the project, issue, branch, worktree, local head, - active automation label, existing PR URL when present, stored handoff head, stored - orchestration head, PR base/head when readable, and the missing or mismatched marker - reason. A diagnostic may report a bound marker, active ownership drift, a missing - marker, a pending issue-state transition, an unverified PR read, or a concrete field - mismatch that requires explicit rebind. + active automation label, existing PR URL when present, lifecycle handoff head, + lifecycle phase head, PR base/head when readable, and the missing or mismatched + lifecycle reason. A diagnostic may report a bound lifecycle record, active ownership + drift, a missing lifecycle record, a pending issue-state transition, an unverified PR + read, or a concrete field mismatch that requires explicit rebind. - `adopt` is mutating and requires an explicit issue identifier plus PR URL. It is the supported manual takeover path for a human-owned PR that was created outside a runtime-retained lane but should now enter Decodex's normal retained review/landing @@ -138,50 +153,51 @@ success path. be open, non-draft, mergeable, green, free of pending review requests, and free of unresolved review threads. It may reuse an existing worktree mapping only when that mapping points at the current managed checkout; it must reject mappings to a - different checkout and must reject any existing review handoff marker for the current - or previously mapped branch. Adopt rewrites the mapping to the current PR branch - only after validation succeeds. Those already-bound marker lanes belong to `rebind` + different checkout and must reject any existing review lifecycle record for the + current or previously mapped branch. Adopt rewrites the mapping to the current PR + branch only after validation succeeds. Those already-bound lifecycle lanes belong to `rebind` or normal landing. - `rebind` is mutating and requires an explicit issue identifier plus PR URL. It must validate the configured project, tracker issue state, active automation ownership, retained worktree branch, clean worktree, PR repository, PR base, PR head branch, PR - head SHA, and open non-draft PR state before writing markers. Existing-marker refresh + head SHA, and open non-draft PR state before writing the lifecycle record. + Existing-record refresh requires the workflow `tracker.success_state`. Partial normal handoff recovery may - also accept the workflow `tracker.in_progress_state` when the marker is missing, or - when an already-current marker exists but the issue state was not advanced, and the - validated PR plus retained worktree prove the handoff lineage. If stale failure - writeback already moved that already-current marker lane back to + also accept the workflow `tracker.in_progress_state` when the lifecycle record is + missing, or when an already-current lifecycle record exists but the issue state was + not advanced, and the validated PR plus retained worktree prove the handoff lineage. + If stale failure writeback already moved that already-current lifecycle lane back to `tracker.failure_state` and applied `tracker.needs_attention_label`, explicit rebind may clear that label and move the issue to `tracker.success_state` after the rebind audit succeeds. Decodex must reject this failure-state recovery for missing or stale - markers because those still need explicit PR-lineage repair before tracker state can - be trusted. -- If no review handoff marker exists, `rebind` restores the missing handoff and - orchestration markers from the validated PR and retained worktree. If a marker already - exists for the same branch and PR but its stored handoff or orchestration head is - stale, `rebind` may refresh that marker to the validated PR head. It must reject an - existing marker for a different PR, and it must reject a current same-branch same-PR - marker as a no-op unless the issue is still in `tracker.in_progress_state` and only - the success-state transition remains. + lifecycle records because those still need explicit PR-lineage repair before tracker + state can be trusted. +- If no review lifecycle record exists, `rebind` restores the missing lifecycle record + from the validated PR and retained worktree. If a record already exists for the same + branch and PR but its stored handoff head or phase head is stale, `rebind` may + refresh that record to the validated PR head. It must reject an existing record for a + different PR, and it must reject a current same-branch same-PR record as a no-op + unless the issue is still in `tracker.in_progress_state` and only the success-state + transition remains. - A successful adopt writes a runtime worktree mapping for the current managed checkout, creates a local run attempt identity for the takeover, writes the same runtime DB - handoff and orchestration marker shapes as normal `issue_review_handoff` needs, + review lifecycle record as normal `issue_review_handoff` needs, records a `review_handoff_adopt` audit event, and may move the issue from `tracker.in_progress_state` to `tracker.success_state` after the audit succeeds. It does not land the PR, queue follow-up work, or clear needs-attention. A subsequent `decodex land --authority --pr ` owns merge, tracker closeout, and cleanup through the normal issue-authority path. -- A successful rebind writes the same runtime DB handoff and orchestration marker shapes - as normal `issue_review_handoff` needs, and records a `review_handoff_rebind` audit +- A successful rebind writes the same runtime DB review lifecycle record as normal + `issue_review_handoff` needs, and records a `review_handoff_rebind` audit event. It does not land the PR, queue follow-up work, or substitute for healthy lanes' normal `issue_review_handoff` plus `issue_terminal_finalize(path = "review_handoff")` - path. If any audit write fails after marker creation, the command must clear the new - markers and report failure instead of leaving a silently rebound lane. -- Once a rebind or equivalent current handoff marker exists for a retained lane, stale + path. If any audit write fails after lifecycle record creation, the command must + clear the new record and report failure instead of leaving a silently rebound lane. +- Once a rebind or equivalent current lifecycle record exists for a retained lane, stale passive failure handling for the earlier `missing_review_handoff_record` observation must not move the issue back to the failure state or add `decodex:needs-attention`. - The next scheduler/status pass must reclassify from the current marker instead of - applying the obsolete missing-marker writeback. + The next scheduler/status pass must reclassify from the current lifecycle record + instead of applying the obsolete missing-record writeback. `cleanup_only` rows are outside this rebind surface. When operator status reports a cleanup-only worktree with `provenance_source = "legacy_unknown"`, Decodex has only an @@ -191,10 +207,10 @@ checkout for local-only changes, run `decodex recover legacy-closeout --pr --dry-run`, rerun with `--manual-authority` only after validation passes, and only then remove the worktree. That fallback must stay rarer than normal closeout, explicit rebind, or deterministic -legacy reconstruction from authoritative markers. Runtime recovery may classify a -retained worktree as `runtime_recovered` only after tracker, retained marker, or -closeout evidence proves a current owner; it must not silently upgrade a terminal -cleanup-only `legacy_unknown` row. +legacy reconstruction from authoritative lifecycle records. Runtime recovery may +classify a retained worktree as `runtime_recovered` only after tracker, retained +lifecycle record, or closeout evidence proves a current owner; it must not silently +upgrade a terminal cleanup-only `legacy_unknown` row. ## Phase model diff --git a/docs/spec/runtime.md b/docs/spec/runtime.md index 7d8386f1..0d954c34 100644 --- a/docs/spec/runtime.md +++ b/docs/spec/runtime.md @@ -115,7 +115,8 @@ mirror: | Runtime SQLite `program_intake_plans` | Queryable local projection of `decodex.program_intake_plan/1` metadata, including intake kind, source contract when present, authority fingerprint, and public-safe summary. | | Runtime SQLite `program_issue_mappings` | Queryable local projection of each internal program node's mapped Linear issue, tracker state, dispatch intent, active/manual/attention facts, and dispatch-briefing fact. | | Runtime SQLite `run_control_channels` | Local control capability metadata for active run attempts. It records the project, issue, run id, attempt, transport, local channel path, channel status, and publish/update timestamps needed to route future control requests without bypassing run lease ownership. | -| Runtime SQLite `review_policy_checkpoints` | Latest bounded-review checkpoint state for one project, issue, run, attempt, and phase, including structured independent-review detail. This row is the authority for review handoff and retained repair gating. | +| Runtime SQLite `review_lifecycle_records` | Single authoritative post-review record for one retained PR-backed lane. It stores handoff identity, PR URL, base/head branch, validated head OID, current post-review phase, review-request metadata, landing/closeout/repair state, evidence, and next action. Handoff and orchestration tool-boundary shapes are projections of this record, not separate durable authority. Historical `review_handoffs` and `review_orchestrations` tables are one-time bootstrap migration input, then dropped and never used as readback authority. | +| Runtime SQLite `review_policy_checkpoints` | Latest bounded-review checkpoint state for one project, issue, run, attempt, and phase, including structured independent-review detail. This row is the authority for review handoff and retained repair gating before a post-review lifecycle record exists. | | Runtime SQLite `loop_guardrail_checkpoints` | Latest convergence checkpoint for one project, issue, and guardrail reason. It stores the fingerprint, consecutive count, run id, attempt number, and structured detail used to stop non-converging loops without replaying Linear comments. | | Agent evidence under `~/.codex/decodex/agent-evidence//` | Derived local handoff view for repair agents. It may reference private evidence readback commands and compact run capsules, but it is not scheduling authority and is not a public mirror. | | Logs under `~/.codex/decodex/logs/` and `.decodex-run-activity` | Diagnostic process and liveness signals. They may explain what a local process did, but they are not the structured execution ledger and must not be replayed as tracker state. | @@ -143,7 +144,8 @@ The following facts are local runtime truth and must not be rebuilt from Linear observation count, source run, and structured stop detail - retry and backoff state: queued retry kind, due time, retry budget, and connector backoff - phase timing and operator activity summaries -- retained worktree mappings, retained PR handoff identity, post-review phase, and cleanup or repair ownership +- retained worktree mappings, review lifecycle records, retained PR handoff identity, + post-review phase, and cleanup or repair ownership Linear issue fields and Linear execution ledger comments are the team-visible tracker mirror for low-frequency lifecycle records. They may enrich completed run history when the connector is available, but they must not become the live source for run leases, dispatch ownership, retry/backoff state, phase timing, retained worktree ownership, or operator snapshot continuity. @@ -360,7 +362,7 @@ projections. - Before starting a live run, the service must reconcile stale local leases, terminal worktree mappings, and runtime-provenance worktree mappings whose checkout path no longer exists and has no active lease, active service label, - needs-attention label, shared claim, running attempt, or review-handoff marker. + needs-attention label, shared claim, running attempt, or review lifecycle record. - Generic live dispatch must not require GitHub CLI authority before the lane actually attempts PR-backed review handoff. - Generic live dispatch must resolve `github.token_env_var` before launching the agent app-server so lane-owned `git push` and `gh pr create` commands inherit noninteractive GitHub credentials. Missing or blank GitHub credentials must fail the run through the human-required path instead of retrying or leaving a promptable lane running. - Project configs may set `[github].command_path` to make one expected GitHub CLI binary authoritative for project-scoped GitHub operations. When it is configured, review handoff validation, retained review readback, landing inspection, GitHub comments, admin merge, merge readback, and remote branch cleanup must invoke that path instead of silently rediscovering another `gh` binary. @@ -474,12 +476,13 @@ remains inspectable instead of being deleted, hidden, or mislabeled as an empty- retry. Review handoff is a lifecycle boundary for loop guardrails. If a retryable failure -occurs after Decodex has a retained review handoff marker for the current issue, -branch, and local HEAD lineage, failure handling must recover the post-review -orchestration marker and return the lane to the review lifecycle before recording a -new `no_effective_diff` checkpoint or terminalizing the run. If the worktree has a -clean handoff checkpoint for the current head but the retained handoff marker is -missing or has unverified/diverged lineage, Decodex must classify the failure as +occurs after Decodex has a retained review lifecycle record for the current issue, +branch, PR, and local HEAD lineage, failure handling must recover the post-review +phase in that lifecycle record and return the lane to the review lifecycle before +recording a new `no_effective_diff` checkpoint or terminalizing the run. If the +worktree has a clean handoff checkpoint for the current head but the retained +lifecycle record is missing or has unverified/diverged lineage, Decodex must classify +the failure as `review_handoff_state_drift` and require explicit handoff recovery evidence. It must not send this condition through implementation architecture recovery and must not mislabel it as ordinary no-effective-diff repair churn. @@ -714,7 +717,6 @@ or `stalled` while current marker, active thread, or active work-protocol eviden still identifies the same `run_id` and `attempt_number` as live, operator status must keep the lane visible with the process/protocol liveness details instead of hiding it as only terminal history or cleanup work. - Operator status lifecycle reconstruction may use recorded run attempts plus local evidence that is already scoped to the same project, issue, run id, and attempt: runtime run leases, run-control channels, protocol activity summaries, private @@ -728,8 +730,43 @@ source evidence and any missing-evidence gaps so operators and agents can invest restart or manual-recovery failures without replaying Linear comments as runtime state. Evidence that cannot be bound to a local project, issue, run id, and attempt is diagnostic context only, not a synthesized lifecycle attempt. -Post-review ownership is stored in the runtime database. Retained handoff rows record the authoritative PR URL, branch lineage, validated PR head OID, run id, and attempt number that completed the `In Review` handoff. Retained orchestration rows record the current post-review phase for that exact handoff identity. If the matching database row is missing, post-review ownership must block as unresolved instead of rebinding from branch-name, current-head, Linear comments, or other heuristics. An explicit operator manual takeover command may adopt a human-owned PR into this same retained database shape only after validating the managed clean worktree, PR repository, default-branch target, exact branch/head match, and green landable PR gates. If the active service label is missing but exists on the issue team, live adopt may restore it after all other invariants pass and must roll that restoration back if the audit write fails. If a retained review marker exists but a stored handoff or orchestration head no longer matches a clean retained worktree and matching PR head, operator status must keep the marker PR URL visible when known and recovery diagnosis must report the concrete mismatched field before any explicit rebind refresh. When the retained handoff still matches the same branch and PR, and PR readback plus local worktree lineage have already accepted the current head as the current PR head, a stale retained orchestration `head_sha` may be rebound to that current head by resetting the orchestration phase to `request_pending`, clearing prior GitHub Review request metadata, and preserving round-count history. Branch, PR, handoff-lineage, or rewritten-history mismatches must continue to block or report for operator recovery instead of being silently rebound. When a fresh active run owns the same issue, operator status must project that active execution as the current lane state and mark the retained post-review lane as shadowed instead of letting stale PR readback drive current project counts. When retained PR readback degrades but the handoff identity is still safe to preserve, operator-local status may expose a typed `readback_root_cause` diagnostic such as missing GitHub CLI, missing GitHub token, GitHub auth failure, API/read failure, parse/shape failure, or lineage validation failure while keeping public-safe warning reasons such as `pull_request_state_read_failed` stable. Retained orchestration must preserve the post-review classification decision when it converts status readback into runtime action: only a non-shadowed `Block` classification may write passive retained manual attention or add `decodex:needs-attention`; degraded readback classified as `WaitForReview` must remain a wait/retry status row and must not be promoted to manual attention by the run-cycle path. -The only source-tree runtime artifacts that clean-source checks may ignore are the untracked top-level `.decodex-run-activity` heartbeat marker and `.decodex-run-control/` local control-channel directory. Durable review handoff, orchestration, review-policy checkpoints, retry, phase timing, and retained PR state belong in the Decodex runtime database, not in root-level or worktree-local review marker files. If the heartbeat marker carries similarly named fields for compatibility or operator diagnostics, those breadcrumb values cannot override runtime-store rows. + +Post-review ownership is stored in the runtime database. One +`review_lifecycle_records` row records the authoritative PR URL, branch lineage, +validated PR head OID, run id, attempt number, current post-review phase, +review-request metadata, landing/closeout/repair state, evidence, and next action for +the retained lane. If the matching database row is missing, post-review ownership must +block as unresolved instead of rebinding from branch-name, current-head, Linear +comments, or other heuristics. An explicit operator manual takeover command may adopt +a human-owned PR into this same retained database shape only after validating the +managed clean worktree, PR repository, default-branch target, exact branch/head match, +and green landable PR gates. If the active service label is missing but exists on the +issue team, live adopt may restore it after all other invariants pass and must roll +that restoration back if the audit write fails. If a retained lifecycle record exists +but a stored handoff or phase head no longer matches a clean retained worktree and +matching PR head, operator status must keep the lifecycle PR URL visible when known +and recovery diagnosis must report the concrete mismatched field before any explicit +rebind refresh. When the retained lifecycle record still matches the same branch and +PR, and PR readback plus local worktree lineage have already accepted the current head +as the current PR head, a stale lifecycle `head_sha` may be rebound to that current +head by resetting the phase to `request_pending`, clearing prior GitHub Review request +metadata, and preserving round-count history. Branch, PR, handoff-lineage, or +rewritten-history mismatches must continue to block or report for operator recovery +instead of being silently rebound. When a fresh active run owns the same issue, +operator status must project that active execution as the current lane state and mark +the retained post-review lane as shadowed instead of letting stale PR readback drive +current project counts. When retained PR readback degrades but the lifecycle identity +is still safe to preserve, operator-local status may expose a typed +`readback_root_cause` diagnostic such as missing GitHub CLI, missing GitHub token, +GitHub auth failure, API/read failure, parse/shape failure, or lineage validation +failure while keeping public-safe warning reasons such as +`pull_request_state_read_failed` stable. The retained lifecycle controller must +preserve the post-review classification decision when it converts status readback into +runtime action: only a non-shadowed `Block` classification may write passive retained +manual attention or add `decodex:needs-attention`; degraded readback classified as +`WaitForReview` must remain a wait/retry status row and must not be promoted to +manual attention by the run-cycle path. +The only source-tree runtime artifacts that clean-source checks may ignore are the untracked top-level `.decodex-run-activity` heartbeat marker and `.decodex-run-control/` local control-channel directory. Durable review lifecycle records, review-policy checkpoints, retry, phase timing, and retained PR state belong in the Decodex runtime database, not in root-level or worktree-local review marker files. If the heartbeat marker carries similarly named fields for compatibility or operator diagnostics, those breadcrumb values cannot override runtime-store rows. ### Dispatch-slot handoff invariant @@ -803,8 +840,9 @@ After a process restart, recent-run history, run lease ownership, retained post- - Worktrees: retain while the issue is non-terminal, and also retain terminal owned lanes while authoritative post-merge closeout or deterministic cleanup is still incomplete. - Worktree mappings must carry durable local provenance. New runtime-recorded mappings use `provenance_source = "runtime_recorded"` with created and updated Unix - timestamps. Mappings reconstructed from retained tracker/worktree/marker evidence - after local runtime state is missing use `provenance_source = "runtime_recovered"`; + timestamps. Mappings reconstructed from retained tracker, worktree, + lifecycle-record, or activity-marker evidence after local runtime state is missing + use `provenance_source = "runtime_recovered"`; they are recoverable runtime state, but not proof that the original runtime row was still present. Rows migrated from older runtime stores that lack this information must remain readable but must be classified as `provenance_source = @@ -821,7 +859,7 @@ After a process restart, recent-run history, run lease ownership, retained post- ## Recovery rules -- On service startup, `decodex` must inspect deterministic `.worktrees/` paths together with tracker issue ids already known from local leases or worktree mappings to rebuild retained worktree mappings before starting new work. This recovery must only write a recovered worktree mapping after tracker, retained marker, or closeout evidence proves that retry, active-lane, or post-review closeout state owns the worktree; a terminal cleanup-only legacy row must keep `provenance_source = "legacy_unknown"` until the operator runs the explicit legacy closeout audit path. +- On service startup, `decodex` must inspect deterministic `.worktrees/` paths together with tracker issue ids already known from local leases or worktree mappings to rebuild retained worktree mappings before starting new work. This recovery must only write a recovered worktree mapping after tracker, retained lifecycle record, or closeout evidence proves that retry, active-lane, or post-review closeout state owns the worktree; a terminal cleanup-only legacy row must keep `provenance_source = "legacy_unknown"` until the operator runs the explicit legacy closeout audit path. - If Linear still shows a non-terminal `In Progress` issue and its retained worktree exists locally, `decodex` must treat that lane as a retry-style recovery candidate before selecting fresh `Todo` work. - Retry recovery must bind retained lanes to issue identity and local runtime state rather than to Linear project membership. - While the control plane is running an active lane, every poll tick must refresh cached tracker state for the leased issue before considering any new selection. @@ -847,11 +885,11 @@ After a process restart, recent-run history, run lease ownership, retained post- - If the leased issue becomes terminal during a control-plane tick, `decodex` must stop the active run, mark the attempt `terminated`, clear the lease, and then retain or clean the worktree according to the retention rules above. - If the leased issue becomes non-terminal and leaves both the `In Progress` lane state and any configured startable pre-claim state, `decodex` must stop the active run, mark the attempt `interrupted`, clear the lease, and keep the worktree for inspection. - If a recovered lease is already in `tracker.success_state` and its retained - review-handoff marker matches the same `run_id` and `attempt_number`, reconciliation + review lifecycle record matches the same `run_id` and `attempt_number`, reconciliation must mark the local attempt `succeeded` and clear only the lease so deterministic retained closeout can reuse the handoff identity. - Deterministic retained closeout must take its `run_id` and `attempt_number` from - the durable review-handoff marker or equivalent tracker record, not from a later + the durable review lifecycle record or equivalent tracker record, not from a later same-process re-entry summary. Later local attempts that did not consume retry budget must not force a synthetic closeout attempt number. - A leased issue that is still in a configured startable state during early control-plane ticks must be treated as a lane that has not finished claiming tracker ownership yet, not as an immediate non-active interruption. @@ -888,7 +926,7 @@ After a process restart, recent-run history, run lease ownership, retained post- the required terminal tool path instead of turning goal completion into issue success. Unsupported app-server goal methods remain hard environment blockers. - Retained post-review orchestration must treat local branch/head readback failures as - transient wait conditions while the review handoff marker still owns the lane. Status + transient wait conditions while the review lifecycle record still owns the lane. Status may report `worktree_checkout_branch_read_failed` or `worktree_head_read_failed`, but the run-cycle path must not write passive retained manual attention or add `decodex:needs-attention` for those read failures alone. A later successful readback @@ -914,7 +952,7 @@ After a process restart, recent-run history, run lease ownership, retained post- operator projection. Status must not also render the issue as an intake queue candidate, because the queue label is then a stale echo of the retained terminal lane rather than dispatchable backlog. -- If Linear still has `decodex:active:` on an issue that also remains queued, but the local runtime cannot prove a matching run lease, status must classify the queued row as blocked with reason `linear_active_label_present`; it must not treat the issue as ready intake. If the retained marker or private execution event rows for that run are missing, status must surface `evidence_missing` in the recovery details. If the retained worktree has tracked changes, that dirty worktree remains owned by queued recovery/attention instead of being hidden as cleanup-only state. +- If Linear still has `decodex:active:` on an issue that also remains queued, but the local runtime cannot prove a matching run lease, status must classify the queued row as blocked with reason `linear_active_label_present`; it must not treat the issue as ready intake. If the retained runtime record or private execution event rows for that run are missing, status must surface `evidence_missing` in the recovery details. If the retained worktree has tracked changes, that dirty worktree remains owned by queued recovery/attention instead of being hidden as cleanup-only state. - Operator status snapshots must expose worktree provenance in both JSON and human text output. A cleanup-only worktree with `provenance_source = "legacy_unknown"` must set `audit_required = true` and provide a `decodex recover legacy-closeout` next action; From 3d264c110a08fb4c516c6fff2a5470b696976787 Mon Sep 17 00:00:00 2001 From: Yvette Carlisle Date: Wed, 17 Jun 2026 13:28:45 +0800 Subject: [PATCH 2/2] {"schema":"decodex/commit/1","summary":"Remove retained review marker migration path","authority":"XY-972"} --- apps/decodex/src/state/internal.rs | 136 +---------------------------- apps/decodex/src/state/tests.rs | 91 ++++++------------- docs/spec/post-review-lifecycle.md | 8 +- docs/spec/runtime.md | 2 +- 4 files changed, 34 insertions(+), 203 deletions(-) diff --git a/apps/decodex/src/state/internal.rs b/apps/decodex/src/state/internal.rs index 6f31499b..d57aea90 100644 --- a/apps/decodex/src/state/internal.rs +++ b/apps/decodex/src/state/internal.rs @@ -497,22 +497,8 @@ ON linear_execution_events (service_id, issue_id, event_unix, recorded_at_unix); } fn bootstrap_review_schema(&self) -> Result<()> { - let legacy_handoffs_exist = self.table_exists("review_handoffs")?; - let legacy_orchestrations_exist = self.table_exists("review_orchestrations")?; - - self.connection.execute_batch(REVIEW_LIFECYCLE_SCHEMA_SQL)?; - - match (legacy_handoffs_exist, legacy_orchestrations_exist) { - (true, true) => { - self.migrate_legacy_review_handoffs_with_orchestrations()?; - self.migrate_legacy_review_orchestration_orphans()?; - }, - (true, false) => self.migrate_legacy_review_handoffs()?, - (false, true) => self.migrate_legacy_review_orchestrations()?, - (false, false) => {}, - } - self.connection.execute_batch(DROP_LEGACY_REVIEW_MARKER_TABLES_SQL)?; + self.connection.execute_batch(REVIEW_LIFECYCLE_SCHEMA_SQL)?; self.ensure_column( "review_policy_checkpoints", "details_json", @@ -522,126 +508,6 @@ ON linear_execution_events (service_id, issue_id, event_unix, recorded_at_unix); Ok(()) } - fn table_exists(&self, table: &str) -> Result { - let count: i64 = self.connection.query_row( - "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = ?1", - params![table], - |row| row.get(0), - )?; - - Ok(count > 0) - } - - fn migrate_legacy_review_handoffs(&self) -> Result<()> { - self.connection.execute( - "INSERT OR IGNORE INTO review_lifecycle_records ( - project_id, issue_id, branch_name, run_id, attempt_number, pr_url, - target_base_ref_name, pr_head_ref_name, pr_head_oid, head_sha, phase, - request_comment_database_id, request_created_at_unix_epoch, - request_description_thumbs_up_count, request_retry_count, external_round_count, - auto_merge_enabled_at_unix_epoch, landing_state, closeout_state, - repair_attempt_count, evidence_json, next_action, updated_at, updated_at_unix - ) - SELECT project_id, issue_id, branch_name, run_id, attempt_number, pr_url, - target_base_ref_name, pr_head_ref_name, pr_head_oid, pr_head_oid, - 'request_pending', NULL, NULL, NULL, 0, 0, NULL, 'not_started', - 'not_started', 0, '{}', '', updated_at, updated_at_unix - FROM review_handoffs - ORDER BY updated_at_unix DESC, updated_at DESC", - [], - )?; - - Ok(()) - } - - fn migrate_legacy_review_orchestrations(&self) -> Result<()> { - self.connection.execute( - "INSERT OR IGNORE INTO review_lifecycle_records ( - project_id, issue_id, branch_name, run_id, attempt_number, pr_url, - target_base_ref_name, pr_head_ref_name, pr_head_oid, head_sha, phase, - request_comment_database_id, request_created_at_unix_epoch, - request_description_thumbs_up_count, request_retry_count, external_round_count, - auto_merge_enabled_at_unix_epoch, landing_state, closeout_state, - repair_attempt_count, evidence_json, next_action, updated_at, updated_at_unix - ) - SELECT project_id, issue_id, branch_name, run_id, attempt_number, pr_url, - NULL, branch_name, head_sha, head_sha, phase, - request_comment_database_id, request_created_at_unix_epoch, - request_description_thumbs_up_count, request_retry_count, external_round_count, - auto_merge_enabled_at_unix_epoch, 'not_started', 'not_started', - 0, '{}', '', updated_at, updated_at_unix - FROM review_orchestrations - ORDER BY updated_at_unix DESC, updated_at DESC", - [], - )?; - - Ok(()) - } - - fn migrate_legacy_review_handoffs_with_orchestrations(&self) -> Result<()> { - self.connection.execute( - "INSERT OR IGNORE INTO review_lifecycle_records ( - project_id, issue_id, branch_name, run_id, attempt_number, pr_url, - target_base_ref_name, pr_head_ref_name, pr_head_oid, head_sha, phase, - request_comment_database_id, request_created_at_unix_epoch, - request_description_thumbs_up_count, request_retry_count, external_round_count, - auto_merge_enabled_at_unix_epoch, landing_state, closeout_state, - repair_attempt_count, evidence_json, next_action, updated_at, updated_at_unix - ) - SELECT h.project_id, h.issue_id, h.branch_name, h.run_id, h.attempt_number, h.pr_url, - h.target_base_ref_name, h.pr_head_ref_name, h.pr_head_oid, - COALESCE(o.head_sha, h.pr_head_oid), COALESCE(o.phase, 'request_pending'), - o.request_comment_database_id, o.request_created_at_unix_epoch, - o.request_description_thumbs_up_count, COALESCE(o.request_retry_count, 0), - COALESCE(o.external_round_count, 0), o.auto_merge_enabled_at_unix_epoch, - 'not_started', 'not_started', 0, '{}', '', - COALESCE(o.updated_at, h.updated_at), - COALESCE(o.updated_at_unix, h.updated_at_unix) - FROM review_handoffs AS h - LEFT JOIN review_orchestrations AS o - ON o.project_id = h.project_id - AND o.issue_id = h.issue_id - AND o.branch_name = h.branch_name - AND o.run_id = h.run_id - AND o.attempt_number = h.attempt_number - ORDER BY h.updated_at_unix DESC, h.updated_at DESC", - [], - )?; - - Ok(()) - } - - fn migrate_legacy_review_orchestration_orphans(&self) -> Result<()> { - self.connection.execute( - "INSERT OR IGNORE INTO review_lifecycle_records ( - project_id, issue_id, branch_name, run_id, attempt_number, pr_url, - target_base_ref_name, pr_head_ref_name, pr_head_oid, head_sha, phase, - request_comment_database_id, request_created_at_unix_epoch, - request_description_thumbs_up_count, request_retry_count, external_round_count, - auto_merge_enabled_at_unix_epoch, landing_state, closeout_state, - repair_attempt_count, evidence_json, next_action, updated_at, updated_at_unix - ) - SELECT o.project_id, o.issue_id, o.branch_name, o.run_id, o.attempt_number, o.pr_url, - NULL, o.branch_name, o.head_sha, o.head_sha, o.phase, - o.request_comment_database_id, o.request_created_at_unix_epoch, - o.request_description_thumbs_up_count, o.request_retry_count, - o.external_round_count, o.auto_merge_enabled_at_unix_epoch, - 'not_started', 'not_started', 0, '{}', '', o.updated_at, o.updated_at_unix - FROM review_orchestrations AS o - WHERE NOT EXISTS ( - SELECT 1 - FROM review_handoffs AS h - WHERE h.project_id = o.project_id - AND h.issue_id = o.issue_id - AND h.branch_name = o.branch_name - ) - ORDER BY o.updated_at_unix DESC, o.updated_at DESC", - [], - )?; - - Ok(()) - } - fn ensure_column(&self, table: &str, column: &str, add_column_sql: &str) -> Result<()> { let mut statement = self.connection.prepare(&format!("PRAGMA table_info({table})"))?; let column_names = statement.query_map([], |row| row.get::<_, String>(1))?; diff --git a/apps/decodex/src/state/tests.rs b/apps/decodex/src/state/tests.rs index 29ba7bea..858b160b 100644 --- a/apps/decodex/src/state/tests.rs +++ b/apps/decodex/src/state/tests.rs @@ -35,7 +35,7 @@ use crate::{ }; const IN_PROGRESS_STATE: &str = "In Progress"; -const REVIEW_LIFECYCLE_MIGRATION_FIXTURE: &str = r#" +const DROPPED_REVIEW_MARKER_TABLES_FIXTURE: &str = r#" CREATE TABLE review_handoffs ( project_id TEXT NOT NULL, issue_id TEXT NOT NULL, @@ -217,12 +217,12 @@ fn assert_decision_contract_retargeted(reopened: &StateStore) { ); } -fn seed_review_lifecycle_migration_fixture(state_path: &Path) { +fn seed_dropped_review_marker_tables(state_path: &Path) { let connection = Connection::open(state_path).expect("fixture db should open"); connection - .execute_batch(REVIEW_LIFECYCLE_MIGRATION_FIXTURE) - .expect("review lifecycle migration fixture should seed"); + .execute_batch(DROPPED_REVIEW_MARKER_TABLES_FIXTURE) + .expect("dropped review marker tables should seed"); } fn upsert_handoff_review_policy_checkpoint( @@ -516,74 +516,39 @@ fn changed_review_handoff_projection_resets_lifecycle_phase_fields() { } #[test] -fn historical_review_marker_tables_migrate_into_lifecycle_record() { +fn historical_review_marker_tables_drop_without_lifecycle_migration() { let temp_dir = TempDir::new().expect("tempdir should create"); let state_path = temp_dir.path().join("runtime.sqlite3"); - seed_review_lifecycle_migration_fixture(&state_path); + seed_dropped_review_marker_tables(&state_path); - let store = - StateStore::open(&state_path).expect("state store should migrate historical markers"); - let handoff = store - .review_handoff_marker("pubfi", "PUB-101", "x/decodex-pub-101") - .expect("migrated handoff projection should read") - .expect("migrated handoff projection should exist"); - - assert_eq!(handoff.run_id(), "run-1"); - assert_eq!(handoff.attempt_number(), 2); - assert_eq!(handoff.pr_head_oid(), "08a20f7dfb9526e7421a5f095b1c6adec84e52d6"); - - let lifecycle = store - .review_lifecycle_record("pubfi", "PUB-101", "x/decodex-pub-101") - .expect("migrated lifecycle record should read") - .expect("migrated lifecycle record should exist"); + let store = StateStore::open(&state_path).expect("state store should drop historical markers"); - assert_eq!(lifecycle.pr_head_oid(), "08a20f7dfb9526e7421a5f095b1c6adec84e52d6"); - assert_eq!(lifecycle.head_sha(), "19b20f7dfb9526e7421a5f095b1c6adec84e52d7"); - assert_eq!(lifecycle.phase(), "waiting_for_ack"); - assert_eq!(lifecycle.request_comment_database_id(), Some(1_234)); - assert_eq!(lifecycle.request_created_at_unix_epoch(), Some(1_771_290_030)); - assert_eq!(lifecycle.request_description_thumbs_up_count(), Some(4)); - assert_eq!(lifecycle.request_retry_count(), 1); - assert_eq!(lifecycle.external_round_count(), 3); - assert_eq!(lifecycle.auto_merge_enabled_at_unix_epoch(), Some(1_771_290_060)); - - let orphan_lifecycle = store - .review_lifecycle_record("pubfi", "PUB-202", "x/decodex-pub-202") - .expect("orphan historical orchestration should read") - .expect("orphan historical orchestration should migrate"); - - assert_eq!(orphan_lifecycle.target_base_ref_name(), None); - assert_eq!(orphan_lifecycle.pr_head_ref_name(), "x/decodex-pub-202"); - assert_eq!(orphan_lifecycle.pr_head_oid(), "28c20f7dfb9526e7421a5f095b1c6adec84e52d8"); - assert_eq!(orphan_lifecycle.head_sha(), "28c20f7dfb9526e7421a5f095b1c6adec84e52d8"); - - let stale_orchestration_lifecycle = store - .review_lifecycle_record("pubfi", "PUB-303", "x/decodex-pub-303") - .expect("stale historical orchestration should read") - .expect("current historical handoff should migrate"); - - assert_eq!(stale_orchestration_lifecycle.run_id(), "run-2"); - assert_eq!(stale_orchestration_lifecycle.attempt_number(), 1); - assert_eq!( - stale_orchestration_lifecycle.pr_head_oid(), - "38c20f7dfb9526e7421a5f095b1c6adec84e52d8" + assert!( + store + .review_handoff_marker("pubfi", "PUB-101", "x/decodex-pub-101") + .expect("handoff projection should read") + .is_none(), + "historical review_handoffs rows must not become lifecycle records" ); - assert_eq!( - stale_orchestration_lifecycle.head_sha(), - "38c20f7dfb9526e7421a5f095b1c6adec84e52d8" + assert!( + store + .review_lifecycle_record("pubfi", "PUB-202", "x/decodex-pub-202") + .expect("orchestration-only lifecycle should read") + .is_none(), + "historical review_orchestrations rows must not become lifecycle records" + ); + assert!( + store + .review_lifecycle_record("pubfi", "PUB-303", "x/decodex-pub-303") + .expect("stale historical lifecycle should read") + .is_none(), + "historical mixed review rows must not become lifecycle records" ); - assert_eq!(stale_orchestration_lifecycle.phase(), "request_pending"); - assert_eq!(stale_orchestration_lifecycle.request_comment_database_id(), None); - assert_eq!(stale_orchestration_lifecycle.request_created_at_unix_epoch(), None); - assert_eq!(stale_orchestration_lifecycle.request_description_thumbs_up_count(), None); - assert_eq!(stale_orchestration_lifecycle.request_retry_count(), 0); - assert_eq!(stale_orchestration_lifecycle.external_round_count(), 0); - assert_eq!(stale_orchestration_lifecycle.auto_merge_enabled_at_unix_epoch(), None); drop(store); - let connection = Connection::open(&state_path).expect("migrated db should open"); + let connection = Connection::open(&state_path).expect("bootstrapped db should open"); let legacy_table_count: i64 = connection .query_row( "SELECT COUNT(*) FROM sqlite_master \ @@ -599,7 +564,7 @@ fn historical_review_marker_tables_migrate_into_lifecycle_record() { .query_row("SELECT COUNT(*) FROM review_lifecycle_records", [], |row| row.get(0)) .expect("review lifecycle rows should query"); - assert_eq!(lifecycle_count, 3); + assert_eq!(lifecycle_count, 0); } #[test] diff --git a/docs/spec/post-review-lifecycle.md b/docs/spec/post-review-lifecycle.md index e758a4db..54b7dcfa 100644 --- a/docs/spec/post-review-lifecycle.md +++ b/docs/spec/post-review-lifecycle.md @@ -85,10 +85,10 @@ record; they are not separate durable authority. When that exact lifecycle recor missing, post-review ownership must block as unresolved instead of rebinding from branch-name, current-head-only heuristics, or Linear comments. -Historical `review_handoffs` and `review_orchestrations` tables are one-time runtime -bootstrap migration input into `review_lifecycle_records`; after that migration, they -are dropped and must not be used as readback authority. Operators must use explicit -diagnose, rebind, or adopt recovery to create or refresh a current lifecycle record. +Historical `review_handoffs` and `review_orchestrations` tables are dropped during +runtime bootstrap without copying their rows, and must not be used as readback +authority. Operators must use explicit diagnose, rebind, or adopt recovery to create or +refresh a current lifecycle record. If these signals disagree and the disagreement cannot be resolved without guessing operator intent, the runtime must use `manual_intervention_required`. diff --git a/docs/spec/runtime.md b/docs/spec/runtime.md index 0d954c34..aa08e4ad 100644 --- a/docs/spec/runtime.md +++ b/docs/spec/runtime.md @@ -115,7 +115,7 @@ mirror: | Runtime SQLite `program_intake_plans` | Queryable local projection of `decodex.program_intake_plan/1` metadata, including intake kind, source contract when present, authority fingerprint, and public-safe summary. | | Runtime SQLite `program_issue_mappings` | Queryable local projection of each internal program node's mapped Linear issue, tracker state, dispatch intent, active/manual/attention facts, and dispatch-briefing fact. | | Runtime SQLite `run_control_channels` | Local control capability metadata for active run attempts. It records the project, issue, run id, attempt, transport, local channel path, channel status, and publish/update timestamps needed to route future control requests without bypassing run lease ownership. | -| Runtime SQLite `review_lifecycle_records` | Single authoritative post-review record for one retained PR-backed lane. It stores handoff identity, PR URL, base/head branch, validated head OID, current post-review phase, review-request metadata, landing/closeout/repair state, evidence, and next action. Handoff and orchestration tool-boundary shapes are projections of this record, not separate durable authority. Historical `review_handoffs` and `review_orchestrations` tables are one-time bootstrap migration input, then dropped and never used as readback authority. | +| Runtime SQLite `review_lifecycle_records` | Single authoritative post-review record for one retained PR-backed lane. It stores handoff identity, PR URL, base/head branch, validated head OID, current post-review phase, review-request metadata, landing/closeout/repair state, evidence, and next action. Handoff and orchestration tool-boundary shapes are projections of this record, not separate durable authority. Historical `review_handoffs` and `review_orchestrations` tables are dropped during bootstrap, not migrated or used as readback authority. | | Runtime SQLite `review_policy_checkpoints` | Latest bounded-review checkpoint state for one project, issue, run, attempt, and phase, including structured independent-review detail. This row is the authority for review handoff and retained repair gating before a post-review lifecycle record exists. | | Runtime SQLite `loop_guardrail_checkpoints` | Latest convergence checkpoint for one project, issue, and guardrail reason. It stores the fingerprint, consecutive count, run id, attempt number, and structured detail used to stop non-converging loops without replaying Linear comments. | | Agent evidence under `~/.codex/decodex/agent-evidence//` | Derived local handoff view for repair agents. It may reference private evidence readback commands and compact run capsules, but it is not scheduling authority and is not a public mirror. |