{"schema":"decodex/commit/2","change":"Repair superseded closeout cleanup ordering","authority":"XY-1248","impact":"compatible"}

Author: yvette-carlisle

apps/decodex/src/recovery/closeout/apply.rs

@@ -129,7 +129,7 @@ trait SupersededCloseoutRecoveryOperationsRunner {
 	fn update_issue_state(&mut self) -> Result<()>;
 	fn write_closeout_event(&mut self) -> Result<bool>;
 	fn write_cleanup_event(&mut self) -> Result<bool>;
-	fn record_lifecycle_authority(&mut self) -> Result<()>;
+	fn record_lifecycle_authority(&mut self, cleanup_state: &'static str) -> Result<()>;
 	fn update_run_status(&mut self) -> Result<()>;
 	fn clear_worktree(&mut self) -> Result<()>;
 	fn post_pull_request_comment(&mut self) -> Result<()>;
@@ -140,14 +140,15 @@ fn apply_superseded_closeout_recovery_sequence(
 	operations: &mut impl SupersededCloseoutRecoveryOperationsRunner,
 ) -> Result<(bool, bool, bool)> {
 	operations.ensure_terminalizable()?;
-	operations.update_issue_state()?;
 	let closeout_recorded = operations.write_closeout_event()?;
+	operations.record_lifecycle_authority("pending")?;
+	operations.post_pull_request_comment()?;
+	let pr_closed = operations.close_pull_request_if_open()?;
+	operations.update_issue_state()?;
 	let cleanup_recorded = operations.write_cleanup_event()?;
-	operations.record_lifecycle_authority()?;
+	operations.record_lifecycle_authority("completed")?;
 	operations.update_run_status()?;
 	operations.clear_worktree()?;
-	operations.post_pull_request_comment()?;
-	let pr_closed = operations.close_pull_request_if_open()?;
 
 	Ok((closeout_recorded, cleanup_recorded, pr_closed))
 }
@@ -194,8 +195,8 @@ impl SupersededCloseoutRecoveryOperationsRunner for SupersededCloseoutRecoveryOp
 		)
 	}
 
-	fn record_lifecycle_authority(&mut self) -> Result<()> {
-		record_superseded_closeout_lifecycle_authority(self.context, self.validation)
+	fn record_lifecycle_authority(&mut self, cleanup_state: &'static str) -> Result<()> {
+		record_superseded_closeout_lifecycle_authority(self.context, self.validation, cleanup_state)
 	}
 
 	fn update_run_status(&mut self) -> Result<()> {
@@ -302,8 +303,12 @@ mod tests {
 			Ok(true)
 		}
 
-		fn record_lifecycle_authority(&mut self) -> Result<()> {
-			self.record("record_lifecycle_authority")
+		fn record_lifecycle_authority(&mut self, cleanup_state: &'static str) -> Result<()> {
+			match cleanup_state {
+				"pending" => self.record("record_lifecycle_authority_pending"),
+				"completed" => self.record("record_lifecycle_authority_completed"),
+				_ => unreachable!("unsupported test cleanup state"),
+			}
 		}
 
 		fn update_run_status(&mut self) -> Result<()> {
@@ -337,37 +342,58 @@ mod tests {
 			operations.steps.into_inner(),
 			vec![
 				"ensure_terminalizable",
-				"update_issue_state",
 				"write_closeout_event",
+				"record_lifecycle_authority_pending",
+				"post_pull_request_comment",
+				"close_pull_request_if_open",
+				"update_issue_state",
 				"write_cleanup_event",
-				"record_lifecycle_authority",
+				"record_lifecycle_authority_completed",
 				"update_run_status",
 				"clear_worktree",
-				"post_pull_request_comment",
-				"close_pull_request_if_open",
 			]
 		);
 	}
 
 	#[test]
-	fn superseded_closeout_does_not_touch_github_when_lifecycle_authority_fails() {
+	fn superseded_closeout_does_not_terminalize_issue_when_lifecycle_authority_fails() {
 		let mut operations = RecordingSupersededCloseoutOperations {
-			fail_at: Some("record_lifecycle_authority"),
+			fail_at: Some("record_lifecycle_authority_pending"),
 			..RecordingSupersededCloseoutOperations::default()
 		};
 
 		let error = apply_superseded_closeout_recovery_sequence(&mut operations)
 			.expect_err("lifecycle authority failure should stop recovery");
 
-		assert!(error.to_string().contains("record_lifecycle_authority failed"));
+		assert!(error.to_string().contains("record_lifecycle_authority_pending failed"));
 		assert_eq!(
 			operations.steps.into_inner(),
 			vec![
 				"ensure_terminalizable",
-				"update_issue_state",
 				"write_closeout_event",
-				"write_cleanup_event",
-				"record_lifecycle_authority",
+				"record_lifecycle_authority_pending",
+			]
+		);
+	}
+
+	#[test]
+	fn superseded_closeout_keeps_cleanup_retryable_when_github_comment_fails() {
+		let mut operations = RecordingSupersededCloseoutOperations {
+			fail_at: Some("post_pull_request_comment"),
+			..RecordingSupersededCloseoutOperations::default()
+		};
+
+		let error = apply_superseded_closeout_recovery_sequence(&mut operations)
+			.expect_err("GitHub comment failure should stop cleanup");
+
+		assert!(error.to_string().contains("post_pull_request_comment failed"));
+		assert_eq!(
+			operations.steps.into_inner(),
+			vec![
+				"ensure_terminalizable",
+				"write_closeout_event",
+				"record_lifecycle_authority_pending",
+				"post_pull_request_comment",
 			]
 		);
 	}
@@ -554,6 +580,7 @@ fn record_merged_closeout_lifecycle_decision(
 fn record_superseded_closeout_lifecycle_authority(
 	context: &RecoveryContext,
 	validation: &SupersededCloseoutValidation,
+	cleanup_state: &'static str,
 ) -> Result<()> {
 	let review_level = context.config.codex().review_level();
 	let checkpoint = runtime_review_checkpoint_status_for_head(
@@ -588,25 +615,31 @@ fn record_superseded_closeout_lifecycle_authority(
 		next_state: record.next_state(),
 	});
 	let idempotency_key = format!(
-		"{}:{}:{}:{}",
+		"{}:{}:{}:{}:{}",
 		context.config.service_id(),
 		validation.issue.id,
 		validation.successor_merge_commit,
-		"superseded_closeout_recovery"
+		"superseded_closeout_recovery",
+		cleanup_state
 	);
+	let causation_id = match cleanup_state {
+		"pending" => "superseded_closeout_recovery_pr_close_authorized",
+		"completed" => "superseded_closeout_recovery_closeout_complete",
+		_ => "superseded_closeout_recovery_closeout_state",
+	};
 	let decided_at = current_timestamp();
 	let decision = decide_lifecycle_transition(LifecycleDecisionInput {
 		facts: &facts,
 		previous,
 		evidence_kind: LifecycleEvidenceKind::CloseoutCompletion,
 		outcome: LifecycleOutcome::Succeeded,
 		merge_commit: Some(&validation.successor_merge_commit),
-		cleanup_state: Some("completed"),
+		cleanup_state: Some(cleanup_state),
 		authority: "issue_authority",
 		actor: "superseded_closeout_recovery",
 		idempotency_key: &idempotency_key,
 		correlation_id: &validation.run_id,
-		causation_id: Some("superseded_closeout_recovery_closeout_complete"),
+		causation_id: Some(causation_id),
 		decided_at: &decided_at,
 	});
 

openwiki/specs/contracts-and-data.md

@@ -139,7 +139,7 @@ After PR-backed handoff, retained review/landing/closeout lifecycle authority is
 
 The pure lifecycle kernel decides from normalized facts; adapters perform side effects and persist projections. Linear comments, manual receipts, local branch names, PR titles, and current head heuristics are not final lifecycle authority.
 
-Fail-closed rule: if a retained lane lacks an exact lifecycle authority projection, normal/program/retry dispatch must not guess a lineage. Use explicit `decodex recover review-handoff diagnose`, `adopt`, or `rebind` paths depending on evidence. If a later issue and successor PR already landed the accepted repair, use `decodex recover superseded-closeout` instead of rebinding the obsolete PR; it requires explicit predecessor/successor issue and PR lineage, a successor issue execution ledger record for the exact merged successor PR head and merge commit, merged successor readback, default-branch reachability, absent queue/active/needs-attention labels and live runtime ownership for the obsolete issue, clean retained worktree evidence, no patch-unique obsolete PR commits relative to the default branch, public-safe closeout ledger records, and an issue-authority lifecycle closeout record.
+Fail-closed rule: if a retained lane lacks an exact lifecycle authority projection, normal/program/retry dispatch must not guess a lineage. Use explicit `decodex recover review-handoff diagnose`, `adopt`, or `rebind` paths depending on evidence. If a later issue and successor PR already landed the accepted repair, use `decodex recover superseded-closeout` instead of rebinding the obsolete PR; it requires explicit predecessor/successor issue and PR lineage, a successor issue execution ledger record for the exact merged successor PR head and merge commit, merged successor readback, default-branch reachability, absent queue/active/needs-attention labels and live runtime ownership for the obsolete issue, clean retained worktree evidence, no patch-unique obsolete PR commits relative to the default branch, public-safe closeout ledger records, and issue-authority lifecycle closeout records that move from pending cleanup before PR mutation to completed cleanup after the obsolete PR is commented/closed.
 
 ## Commit message contract
 

openwiki/workflows/runtime-operator-workflows.md

@@ -135,7 +135,7 @@ decodex recover superseded-closeout ...
 ```
 
 Use recovery when normal runtime status reports retained lane drift, missing lifecycle authority, ghost lanes, stale active ownership, or already-merged closeout gaps. The post-review lifecycle spec is strict: missing lifecycle records are fail-closed and must not be reconstructed from branch names, PR titles, Linear comments, or current head alone (`openwiki/specs/contracts-and-data.md`).
-Use `superseded-closeout` when an obsolete retained PR should not be rebound because a distinct successor issue and merged successor PR already landed the accepted repair. The command validates the obsolete and successor issues, same configured repository/default branch, successor issue ledger lineage for the exact merged successor PR head and merge commit, merged successor PR reachability from `origin/<default>`, absent queue/active/needs-attention labels and live runtime ownership for the obsolete issue, clean retained worktree, and absence of patch-unique obsolete PR commits relative to the default branch. Apply records the public-safe closeout ledger, lifecycle authority, run success, and retained cleanup state before commenting on or closing the obsolete GitHub PR, so the PR mutation is never the only durable side effect.
+Use `superseded-closeout` when an obsolete retained PR should not be rebound because a distinct successor issue and merged successor PR already landed the accepted repair. The command validates the obsolete and successor issues, same configured repository/default branch, successor issue ledger lineage for the exact merged successor PR head and merge commit, merged successor PR reachability from `origin/<default>`, absent queue/active/needs-attention labels and live runtime ownership for the obsolete issue, clean retained worktree, and absence of patch-unique obsolete PR commits relative to the default branch. Apply records the public-safe close authorization ledger and pending-cleanup lifecycle authority before terminalizing the issue or mutating the obsolete GitHub PR, then records cleanup completion and clears retained worktree state only after the obsolete PR comment/close path succeeds.
 
 Future agents changing recovery must preserve explicit evidence requirements and add tests under the matching `apps/decodex/src/orchestrator/tests/recovery_*` or `apps/decodex/src/recovery/tests/` area.
 

plugins/decodex/skills/decodex-ops/SKILL.md

@@ -47,7 +47,9 @@ service labels, recovery, or lane-control details matter.
   before live superseded closeout. The obsolete issue must have no queue, active,
   or needs-attention labels or live runtime ownership, and the successor issue
   must expose a Decodex ledger record for the exact successor PR head and merge
-  commit.
+  commit. Live superseded closeout records close authorization before the obsolete
+  issue/PR terminal mutation and records cleanup completion only after the PR
+  comment/close path succeeds.
 - Do not infer PR lineage from branch names, PR titles, Linear comments, status
   summaries, or stale snapshots.