{"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

@@ -132,7 +132,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<()>;
@@ -143,14 +143,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))
 }
@@ -197,8 +198,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<()> {
@@ -305,8 +306,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<()> {
@@ -340,37 +345,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",
 			]
 		);
 	}
@@ -558,6 +584,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 = orchestrator::runtime_review_checkpoint_status_for_head(
@@ -592,25 +619,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 = self::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

@@ -150,7 +150,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. The persisted lifecycle authority projection records issue/project identity, phase, transition, previous/next state, next action, review gate state, PR URL, base/head branches, validated head, worktree path, merge/cleanup state, source evidence refs, idempotency/correlation ids, actor, and decision time. Historical handoff/orchestration tables and public ledger comments are not a substitute for this projection.
 
-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.
 
 Recovery boundaries are explicit. Startup/current-lane recovery may rebuild retained worktree mappings only from deterministic paths plus tracker/runtime/lifecycle evidence; missing lifecycle records, mismatched stored handoff heads, stale PID markers, and unscoped logs are diagnostic inputs, not ownership. Retained tracked changes after crash or stall flow through retry, phase-goal recovery, repo-gate recovery, or human-attention classification according to runtime evidence; they must not be rebound from branch names or PR titles.
 

openwiki/workflows/runtime-operator-workflows.md

@@ -148,7 +148,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.
 
 Recovery is dry-run-first unless the command is read-only. `review-handoff diagnose` is the read-only entrypoint; `rebind` repairs retained Decodex PR lanes only when the retained worktree and PR prove exact issue/branch/head authority, while `adopt` is limited to human-created PRs from a managed clean Decodex worktree and must not take over lanes that already have lifecycle records. `stale-active diagnose` applies to tracker-present active-label ownership with no safe live/progress owner; `ghost-lane diagnose` applies to missing-issue local runtime state. Stop instead of mutating when the report names live process state, run leases/shared claims, needs-attention labels, non-runtime worktree changes, unmerged commits, unavailable default-branch proof, private progress evidence, review-policy checkpoints, PR/review lineage, mixed private evidence, or unreadable worktrees.
 

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.