Skip to content

XY-1248: Decodex should close or supersede retained PRs when a repair PR lands the same lane#1073

Open
yvette-carlisle wants to merge 16 commits into
mainfrom
y/decodex-xy-1248
Open

XY-1248: Decodex should close or supersede retained PRs when a repair PR lands the same lane#1073
yvette-carlisle wants to merge 16 commits into
mainfrom
y/decodex-xy-1248

Conversation

@yvette-carlisle

Copy link
Copy Markdown
Member

Summary

  • Add a superseded retained PR closeout recovery command for obsolete retained PRs replaced by a merged successor repair PR.
  • Validate successor lineage, merged successor state, clean retained worktree state, and patch-equivalent obsolete diff before closing/terminalizing.
  • Update Decodex operator docs and OpenWiki coverage for the new recovery path.

Validation

  • cargo make fmt
  • cargo make test

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Subagent review found blockers on attempt 1. Do not land as-is.

Findings:

  1. P1: superseded-closeout can release an active or queued lane. validation/superseded.rs validates opt-out/completed-state availability, but does not reject queue/active/needs-attention labels the way merged closeout does. Apply then transitions issue, removes labels, clears worktree mapping, and marks the run succeeded.
  2. P1: successor issue is not proven to own the successor PR. The validator loads both issues and checks merged/reachable successor PR, but does not prove the successor PR belongs to or has lifecycle authority for the successor issue. A completed unrelated issue plus an unrelated merged PR could satisfy the current checks.
  3. P2: GitHub close/comment mutation happens before durable closeout/writeback succeeds. If later Linear/runtime writes fail, the PR can be closed/commented without matching runtime/tracker reconciliation.

Required repair:

  • Add validation/apply tests for active/queued/needs-attention labels or live ownership on superseded closeout.
  • Prove successor issue-to-PR lineage, or reject unrelated completed successor issues / unrelated merged PRs.
  • Rework mutation order or add idempotent reconciliation so GitHub close cannot be the only durable side effect after later failure.
  • Rerun targeted closeout tests and full cargo make check.

Local validation note: cargo test -p decodex closeout --all-features passed, but cargo make check reached vstyle and failed with workspace-wide style violations, including changed files. The review blockers above remain.

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Addressed the retained repair blockers in commit 3f439c7.

  • Superseded closeout now rejects queued, active, needs-attention, and live-owned obsolete lanes instead of clearing ownership labels.
  • Successor lineage now requires a Decodex execution ledger record tying the successor issue to the exact successor PR head and merge commit.
  • GitHub close/comment happens only after durable tracker closeout records are written, and terminal lifecycle/run success is recorded only after the GitHub close/comment path succeeds.
  • Updated OpenWiki and the Decodex ops skill for the stricter recovery preconditions.

Validation rerun on the repaired head:

  • cargo make fmt
  • cargo test -p decodex closeout --all-features
  • cargo make test

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Second subagent review found one remaining blocker and one P2 regression on repaired commit 3f439c7c. Do not land yet.

Findings:

  1. P1: GitHub close/comment still precedes final lifecycle authority.

    • apps/decodex/src/recovery/closeout/apply.rs posts the PR comment and closes the obsolete PR before record_superseded_closeout_lifecycle_authority.
    • If GitHub succeeds and lifecycle persistence/cleanup fails, the public PR is closed/commented while Decodex lacks final lifecycle authority.
    • Required repair: move durable lifecycle authority/state writes before GitHub side effects, or add explicit idempotent reconciliation that makes this failure mode safe and test-covered.
  2. P2: merged closeout cleanup event text now says Decodex will “close the obsolete PR”.

    • apps/decodex/src/recovery/closeout/events.rs merged closeout text is misleading because merged closeout is for an already-merged same-PR lane and does not close any PR.
    • Required repair: restore accurate merged-closeout wording.

Validated state before this review:

Remaining action: repair the two findings, rerun targeted closeout tests plus cargo make test, then re-review.

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Addressed the remaining retained repair findings in commit c2a9258a.

  • Superseded closeout now records lifecycle authority, run success, and retained worktree cleanup state before posting the GitHub PR comment or closing the obsolete PR.
  • Added focused sequencing tests that prove GitHub side effects are not reached if lifecycle authority persistence fails.
  • Restored merged-closeout cleanup wording so it no longer says Decodex will close an obsolete PR.

Validation on the repaired head:

  • cargo make fmt
  • cargo test -p decodex closeout --all-features
  • cargo make test

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Follow-up OpenWiki alignment is included in commit 88451b27, documenting the superseded-closeout apply ordering now enforced by c2a9258a.

Final validation rerun on head 88451b27:

  • cargo make fmt
  • cargo make test

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Third subagent review found two remaining blockers on head 88451b27. Do not land yet.

Findings:

  1. P1: superseded closeout terminalizes Linear before durable Decodex closeout authority.

    • apps/decodex/src/recovery/closeout/apply.rs calls update_issue_state() before closeout ledger writes, lifecycle authority, run status, or cleanup state are durably recorded.
    • If a later step fails, the obsolete lane is public-terminal while Decodex lacks final closeout authority and may not be recoverable by normal automation.
    • Required repair: move public tracker state mutation after durable local authority/ledger writes, or add rollback/idempotent recovery proving this partial state is safe.
  2. P1: retained worktree mapping is cleared before obsolete PR close/comment succeeds.

    • The sequence clears the mapping before the GitHub PR close/comment step.
    • If GitHub fails, local state already says cleanup is complete and the retained mapping is gone; retry then fails the superseded validator because it requires the retained mapping.
    • Required repair: persist close authorization first, perform GitHub comment/close, then mark cleanup complete and clear the mapping only after GitHub succeeds; or explicitly support retry from the durable authority record without requiring the mapping.

Prior concerns status:

  • Active/queued/needs-attention/live-owned guard: mostly fixed, with residual race risk.
  • Successor issue-to-PR lineage proof: fixed for Decodex ledger evidence.
  • GitHub side-effect ordering: fixed narrowly, but unsafe earlier Linear mutation and cleanup-before-GitHub partial states remain.
  • Merged-closeout wording: fixed.

Validation already passing on this head:

  • cargo test -p decodex closeout --all-features --quiet: 88 passed.
  • cargo make test: 1674 passed, 1 skipped.
  • GitHub CodeQL checks: pass.

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Addressed the latest retained repair blockers in commit 1c74ce62.

  • Superseded closeout now records the closeout ledger and pending cleanup lifecycle authority before any public terminal mutation.
  • The obsolete PR comment/close path now completes before Linear completion, cleanup ledger recording, completed lifecycle authority, run success, and retained worktree clearing.
  • Added focused sequencing coverage for lifecycle-authority failure before terminalization and GitHub comment failure before cleanup.
  • Updated OpenWiki and the Decodex ops skill to document the pending-to-completed cleanup ordering.

Validation on head 1c74ce62:

  • cargo make fmt
  • cargo test -p decodex closeout --all-features
  • cargo make test

…seout recovery","authority":"XY-1248","impact":"compatible"}
…iew blockers","authority":"XY-1248","impact":"compatible"}
…e-effect order","authority":"XY-1248","impact":"compatible"}
…pply ordering","authority":"XY-1248","impact":"compatible"}
…anup ordering","authority":"XY-1248","impact":"compatible"}
@yvette-carlisle

Copy link
Copy Markdown
Member Author

Retained repair head refreshed after resolving the merge conflict with current main.

Current repaired head: f4572a28
Base: 90cf9c87

Addressed state:

  • Preserved the superseded-closeout repair ordering: close authorization ledger and pending lifecycle authority before GitHub PR mutation, then Linear completion, cleanup ledger, completed lifecycle authority, run success, and retained worktree clearing only after the obsolete PR comment/close path succeeds.
  • Preserved the prior guards for obsolete lane ownership labels/live ownership and successor issue-to-PR lineage.
  • PR XY-1248: Decodex should close or supersede retained PRs when a repair PR lands the same lane #1073 is now mergeable; remote CodeQL checks are queued on the refreshed head.

Validation rerun on refreshed head:

  • cargo make fmt
  • cargo test -p decodex closeout --all-features
  • cargo make test

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Fourth subagent review found two remaining blockers on head f4572a28. Do not land yet.

Findings:

  1. P1: the pre-PR "pending" authority is still terminal closeout authority.

    • apps/decodex/src/recovery/closeout/apply.rs records the pending authority as LifecycleEvidenceKind::CloseoutCompletion + Succeeded.
    • The lifecycle kernel maps that to closed/no-action, and the state adapter reports closeout completed.
    • If the process fails after this pending authority write but before PR comment/close, Linear completion, cleanup ledger, and worktree cleanup, durable state can incorrectly say the lane is closed with no next action while the public PR and cleanup are unfinished.
    • Required repair: make pre-PR authority a non-terminal pending/intent state with a retryable next action, or delay CloseoutCompletion/Succeeded until after PR comment/close and cleanup ledger succeed. Add coverage that pending authority is not closed, not closeout_state=completed, and not no_action.
  2. P1: live runtime ownership validation misses continuation-owned lanes.

    • apps/decodex/src/recovery/closeout/validation/superseded.rs only rejects latest attempts in starting or running.
    • Decodex treats continuation_pending as active/queued ownership elsewhere. Superseded closeout could terminalize/close/clear while a continuation-owned lane still exists.
    • Required repair: reuse the central lane-control/recovery ownership predicate or explicitly reject continuation_pending and equivalent retained retry/live ownership states before terminalization. Add coverage for a continuation_pending runtime state.

Current passing validation on head f4572a28 before this review:

  • cargo test -p decodex closeout --all-features --quiet: 89 passed.
  • cargo make test: 1675 passed, 1 skipped.

Remaining action: repair these two sequencing/ownership issues and re-review.

…ding authority","authority":"XY-1248","impact":"compatible"}
@yvette-carlisle

Copy link
Copy Markdown
Member Author

Addressed the fourth retained repair blockers in commit 6239cf42.

  • Added CloseoutIntent lifecycle authority so superseded-closeout pending authorization records closeout_pending with retryable run_retained_closeout_adapter, not closed, closeout_state=completed, or no_action.
  • Superseded-closeout live ownership validation now rejects any non-terminal latest run attempt through the central run-attempt terminal predicate, including continuation_pending.
  • Updated OpenWiki and the Decodex ops skill to document retryable pending closeout authority and continuation-owned lanes as live ownership.

Validation on pushed head 6239cf42084c47bf76538108d0bf2484c136bd90:

  • cargo make fmt
  • cargo test -p decodex closeout --all-features --quiet (91 passed)
  • cargo make test (1677 passed, 1 skipped)

No inline review threads were present to resolve; the addressed feedback was in PR summary comments.

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Fifth subagent review found one remaining blocker on head 6239cf42. Do not land yet.

Finding:

  1. P1: superseded closeout still misses retry-schedule runtime ownership.
    • apps/decodex/src/recovery/closeout/validation/superseded.rs rejects active shared claims and the latest non-terminal attempt, but it does not reject retained worktree retry-schedule ownership.
    • A lane with a terminal-looking failed/interrupted latest attempt plus a future retry marker is still queued runtime ownership. Current code could terminalize/close/clear it.
    • This conflicts with the contract that queued retry entries remain runtime claims until fired, cancelled, or ineligible; existing status coverage projects this shape as retry_backoff/waiting ownership.
    • Required repair: extend the superseded terminalizable guard to inspect retained worktree runtime retry schedule ownership for the matching issue/run/attempt and reject it. Add a focused test with a terminal latest attempt plus write_run_retry_schedule(...) proving superseded closeout refuses the lane.

Current passing validation on 6239cf42 before this review:

  • cargo test -p decodex closeout --all-features --quiet: 91 passed.
  • cargo make test: 1677 passed, 1 skipped.

Remaining action: repair retry-schedule ownership guard, rerun validation, and re-review.

…ry ownership","authority":"XY-1248","impact":"compatible"}
@yvette-carlisle

Copy link
Copy Markdown
Member Author

Addressed the fifth retained repair blocker in commit 17612e52.

  • Superseded closeout now rejects obsolete lanes that still have a retained retry schedule for the latest terminal run attempt, so retry-backed runtime ownership cannot be terminalized as superseded.
  • Added focused coverage for a failed latest attempt with a pending retry schedule.
  • Updated OpenWiki and the Decodex ops skill to document retry schedules as live ownership for superseded closeout.

Validation on pushed head 17612e52708533a3cd79bb24bc5f4006b496fc0d:

  • cargo make fmt
  • cargo test -p decodex closeout --all-features --quiet (92 passed)
  • cargo make test (1678 passed, 1 skipped)

No inline review threads were present to resolve. CodeQL checks are running on the refreshed head.

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Sixth subagent review found one remaining blocker on head 17612e52. Do not land yet.

Finding:

  1. P1: superseded closeout still misses live retained app-server/activity ownership when the latest persisted attempt is terminal.
    • apps/decodex/src/recovery/closeout/validation/superseded.rs rejects leases, non-terminal attempt status, and retry markers, but it does not reject a live app-server/process/protocol marker for a retained worktree when the persisted latest attempt already looks terminal.
    • That can allow superseded closeout to terminalize/comment/close/clear a lane that still has live runtime ownership.
    • Required repair: when a retained worktree marker exists for the issue/run, reject live marker/process/protocol ownership using the existing process-liveness/activity helpers, not just retry_kind. Add focused coverage for a terminal latest attempt plus current live marker/process/protocol activity that must fail superseded closeout validation.

Current passing validation on 17612e52 before this review:

  • cargo test -p decodex closeout --all-features --quiet: 92 passed.
  • cargo make test: 1678 passed, 1 skipped.

Remaining action: repair live retained marker/process ownership guard, rerun validation, and re-review.

…n superseded closeout","authority":"XY-1248","impact":"compatible"}
@yvette-carlisle

Copy link
Copy Markdown
Member Author

Addressed the sixth retained repair blocker in commit 14490a83.

  • Superseded closeout now rejects matching retained worktree markers for the latest terminal attempt when they still show live process ownership, active thread ownership, or live protocol/activity evidence.
  • The guard reuses the existing stale-active process/thread liveness helpers and still keeps retry-schedule detection scoped to the matching run/attempt.
  • Added focused coverage for a terminal latest attempt plus live retained protocol/process marker.
  • Updated OpenWiki and the Decodex ops skill to document live retained markers as superseded-closeout ownership blockers.

Validation on pushed head 14490a833b2a4486222e7cb89177e0f32deeae8d:

  • cargo make fmt
  • cargo test -p decodex closeout --all-features --quiet (93 passed)
  • cargo make test (1679 passed, 1 skipped)

No inline review threads were present to resolve. CodeQL checks are running on the refreshed head.

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Seventh subagent review found two remaining blockers on head 14490a83. Do not land yet.

Findings:

  1. P1: live retained marker/retry ownership is still tied too narrowly to the latest attempt row.

    • apps/decodex/src/recovery/closeout/validation/superseded.rs checks live retained marker/retry ownership only for latest_run_attempt_for_issue, and returns early when the marker run/attempt does not match.
    • If the retained worktree has a live marker or retry schedule but no latest attempt row, or a mismatched older live marker, superseded closeout can terminalize a live/retry-scheduled retained lane.
    • Required repair: inspect retained worktree markers independently of the latest attempt, reject any live/thread/protocol/retry marker for the retained lane unless proven stale/inert, and scan/reject all non-terminal attempts for both issue id and identifier.
  2. P1: public Linear closeout/cleanup projection can still precede durable lifecycle authority.

    • apps/decodex/src/recovery/closeout/apply.rs publishes the Linear closeout ledger comment before record_lifecycle_authority("pending"), and publishes cleanup before completed lifecycle authority.
    • If lifecycle persistence fails after the public write, Linear can claim closeout/cleanup authority that the runtime authority record does not durably own.
    • Required repair: record retryable pending lifecycle authority before any public Linear/GitHub projection, ensure cleanup completion has retry-safe durable state before public cleanup projection, and add sequence tests that fail lifecycle persistence and assert no public Linear write occurred.

Current passing validation on 14490a83 before this review:

  • cargo test -p decodex closeout --all-features --quiet: 93 passed.
  • cargo make test: 1679 passed, 1 skipped.

Remaining action: repair retained marker scanning and Linear projection ordering, rerun validation, and re-review.

…ut ownership","authority":"XY-1248","impact":"compatible"}
@yvette-carlisle

Copy link
Copy Markdown
Member Author

Addressed the seventh retained repair blockers in commit 07a2c3f1.

  • Superseded closeout now scans every retained run attempt recorded for both issue id and identifier, and rejects any non-terminal attempt instead of only checking the latest row.
  • Retained worktree marker ownership is now checked independently of the latest attempt row, including retry markers, live process/thread evidence, and protocol/activity markers with no matching or mismatched attempt row.
  • Superseded closeout now records retryable pending lifecycle authority before public Linear closeout/GitHub projections, and records completed lifecycle authority before public cleanup projection.
  • Added focused coverage for marker-only retry ownership, mismatched retry ownership, marker-only/mismatched live protocol ownership, identifier-keyed non-terminal attempts, and lifecycle projection ordering failures.
  • Updated OpenWiki and the Decodex ops skill to document the stricter recovery preconditions and ordering.

Validation on pushed head 07a2c3f1:

  • cargo make fmt
  • cargo test -p decodex closeout --all-features --quiet (101 passed)
  • cargo make test (1687 passed, 1 skipped)

No inline review threads were present to resolve.

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Eighth subagent review found two remaining P1 blockers on head 07a2c3f1. Do not land yet.

Findings:

  1. P1: merged closeout recovery still publishes public Linear closeout/cleanup projections before durable lifecycle authority.

    • apps/decodex/src/recovery/closeout/apply.rs writes public Linear closeout/cleanup projections in apply_merged_closeout_recovery before recording lifecycle authority.
    • If lifecycle persistence fails, Linear can already say closeout/cleanup completed without durable Decodex authority.
    • Required repair: record durable lifecycle authority before any public Linear projection, and only clear/update run state after required projections are confirmed or explicitly retry-safe.
  2. P1: superseded closeout duplicate handling can skip required public Linear projection.

    • write_superseded_closeout_event treats a local ledger duplicate as success and skips the Linear comment.
    • A crash after local record_linear_execution_event but before create_linear_execution_event_comment_direct leaves the public projection missing; retry can then close PR, record completed authority/run success, and clear the worktree without publishing the required record.
    • Required repair: make projection retry check the remote Linear ledger or persist explicit projection_posted state. Only return duplicate success when the corresponding Linear comment exists.

Current passing validation on 07a2c3f1 before this review:

  • cargo test -p decodex closeout --all-features --quiet: 101 passed.
  • cargo make test: 1687 passed, 1 skipped.

Remaining action: repair projection ordering and idempotent Linear projection tracking, rerun validation, and re-review.

…ering","authority":"XY-1248","impact":"compatible"}
@yvette-carlisle

Copy link
Copy Markdown
Member Author

Addressed the eighth retained repair blockers in commit 9d8cd03c.

  • Merged closeout recovery now records durable lifecycle authority before publishing public Linear closeout/cleanup projections, and only clears retained worktree/run state after projections are handled.
  • Closeout ledger projection writes now verify the matching remote Linear comment before treating a local duplicate as complete; retries repost a missing public comment instead of silently continuing from the local row.
  • Added focused sequencing and duplicate-projection coverage, and updated OpenWiki plus the Decodex ops skill for the new recovery ordering.

Validation on pushed head 9d8cd03c:

  • cargo make fmt
  • cargo test -p decodex closeout --all-features --quiet (106 passed)
  • cargo make test (1692 passed, 1 skipped)

No inline review threads were present to resolve.

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Subagent review on head 9d8cd03cb4295bd396a73fdeecbde083da0ef6ff found one remaining P1 blocker:

apps/decodex/src/recovery/closeout/validation/superseded.rs:303

ensure_marker_has_no_live_runtime_evidence only blocks protocol/activity marker evidence when marker_liveness != NotAlive. A retained worktree marker for an older or mismatched run with a dead process can still carry last_progress, last_protocol_activity, event_count, last_event_type, child_agent_activity, or protocol_activity and pass terminalization. That violates the superseded-closeout guardrail that no stale or mismatched retained lane may be terminalized from retained activity evidence.

Required repair:

  • Fail closed on stale or mismatched retained markers with any protocol/activity/progress evidence unless a separate explicit stale-lane recovery has cleared or classified that marker.
  • Add a regression test where the latest attempt is terminal, the marker run/attempt is different or older, process liveness resolves NotAlive, and protocol/activity fields are present; superseded terminalization must reject it.

… closeout","authority":"XY-1248","impact":"compatible"}
@yvette-carlisle

Copy link
Copy Markdown
Member Author

Addressed the retained activity-marker blocker in commit 46b6562.

  • Superseded closeout now rejects protocol, activity, or progress evidence even when the retained marker process is dead or the marker belongs to an older or mismatched run.
  • Added a regression test that proves a NotAlive mismatched marker with protocol activity cannot terminalize the obsolete lane.
  • Updated OpenWiki and the Decodex ops skill to require explicit stale-lane recovery before superseded terminalization.

Validation on pushed head 46b6562:

  • cargo make fmt
  • cargo test -p decodex closeout --all-features (107 passed)
  • cargo make test (1693 passed, 1 skipped)

Independent adversarial review outcome: clean. No inline review threads were present to resolve.

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Subagent review on head 46b6562f3c3357e765fdcb99b240c5f13b48b2a1 found one remaining P1 blocker:

apps/decodex/src/recovery/closeout/validation/superseded.rs:303

The stale/mismatched retained-marker guard now rejects progress/protocol fields, event counts, and activity summaries, but it still does not reject activity-only evidence: last_activity_unix_epoch and current_operation. A dead marker written by apps/decodex/src/state/run_activity_marker/write.rs:73 can have last_activity_unix_epoch even when process liveness resolves NotAlive, so superseded closeout can still terminalize stale retained ownership with unresolved activity evidence.

The current regression test around superseded.rs:785 includes protocol evidence after the dead process marker, so it does not prove the activity-only case is blocked.

Required repair:

  • Treat stale/mismatched retained markers with last_activity_unix_epoch or current_operation as retained activity evidence and fail closed unless a separate explicit stale-lane recovery has cleared/classified the marker.
  • Add an activity-only regression test: latest attempt terminal, marker run/attempt older or mismatched, process liveness NotAlive, no protocol/progress/event-count fields, but last_activity_unix_epoch and/or current_operation present; superseded closeout must reject it.

…-only markers","authority":"XY-1248","impact":"compatible"}
@yvette-carlisle

Copy link
Copy Markdown
Member Author

Addressed the retained activity-only marker blocker in commit 31aa56c6.

  • Superseded closeout now rejects stale or mismatched retained markers containing only last_activity_unix_epoch or only current_operation.
  • Added isolated regressions for a dead mismatched activity-only marker and a mismatched operation-only marker.
  • Existing OpenWiki already states the fail-closed activity-evidence contract, so no documentation change was needed.

Validation on pushed head 31aa56c684d92258474fd73f38683f1d80ddb308:

  • cargo make fmt
  • cargo test -p decodex closeout --all-features (109 passed)
  • cargo make test (1695 passed, 1 skipped)

Independent adversarial review outcome: clean. No inline review threads were present to resolve.

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Subagent review on head 31aa56c684d92258474fd73f38683f1d80ddb308 found one remaining P1 blocker:

apps/decodex/src/recovery/closeout/validation/superseded.rs:216

Superseded closeout still checks active shared claims, non-terminal run attempts, retry markers, and retained worktree activity markers, but it does not check durable SQLite execution evidence for the obsolete issue/run: private execution events, protocol events, or persisted run activity summaries. A terminal obsolete attempt with retained issue_progress_checkpoint, protocol events, or run_activity_summaries but no live marker could still pass and be terminalized.

That violates the closeout safety rule that retained protocol/activity/progress evidence must be explicitly cleared or classified before superseded terminalization. Stale-active recovery already treats this evidence class as ownership/progress evidence; superseded closeout needs equivalent fail-closed coverage.

The previous activity-only marker finding appears fixed: last_activity_unix_epoch, current_operation, progress/protocol marker fields, child-agent activity, and protocol summaries now block even when process liveness is NotAlive, and tests cover dead activity-only and operation-only markers.

Required repair:

  • Reject superseded closeout when durable private/protocol/run-activity evidence exists for the obsolete issue/run unless a separate explicit stale-lane recovery has cleared/classified it.
  • Add regression tests for otherwise-terminal obsolete lanes with private progress evidence, protocol event evidence, and run activity summaries but no live marker.

…able evidence","authority":"XY-1248","impact":"compatible"}
@yvette-carlisle

Copy link
Copy Markdown
Member Author

Addressed the durable SQLite evidence blocker in commit ae560d8a.

  • Superseded closeout now rejects terminal obsolete attempts that retain canonical progress_checkpoint evidence, protocol journal evidence, or persisted child/protocol run-activity summaries even when no worktree marker exists.
  • Added persistent-SQLite regressions for all three evidence classes; each test reopens the state store before validation, including the previously stale-active-allowlisted probing checkpoint shape.
  • Existing OpenWiki and operator guidance already states that retained protocol, activity, or progress evidence blocks superseded terminalization, so no documentation change was required.

Validation on pushed head ae560d8a6e912d5dc1d868c474a67925294d088c:

  • cargo make fmt
  • cargo test -p decodex terminalizable_guard_rejects_durable --all-features --quiet (3 passed)
  • cargo make test (1698 passed, 1 skipped)

Implementation and adversarial review outcome: clean. No inline review threads were present to resolve. Runtime-owned CodeQL checks are in progress.

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Subagent review on head ae560d8a6e912d5dc1d868c474a67925294d088c found remaining blockers:

P1: apps/decodex/src/recovery/closeout/validation/superseded.rs:227

Superseded closeout does not check active run_control_channels rows before terminalization. The guard checks shared claims, terminal run-attempt status, durable protocol/activity evidence, and retained worktree markers, but an active run-control channel for a terminal-looking attempt can survive without a lease or marker. That remains runtime control/ownership evidence and is treated as a blocker by other recovery paths. Counterexample: terminal failed attempt, lease gone, no marker, active control-channel row remains; validation can pass and apply can close the obsolete PR/clear the worktree instead of requiring explicit stale-lane recovery.

P1: apps/decodex/src/recovery/closeout/validation/superseded.rs:238

The durable private-evidence check is currently dependent on list_run_attempts_for_issue. Private execution events are issue-scoped and can exist without a matching run-attempt row; append APIs do not enforce a run-attempt FK. If a private event exists without an attempt row, the loop never calls the durable-evidence guard and markerless obsolete-issue private evidence can still pass.

P2: apps/decodex/src/recovery/closeout/validation/superseded.rs:268

The private-event check inherits the stale-active release allowlist. That may be appropriate after explicit stale-active diagnosis, but superseded closeout is a destructive obsolete PR/issue terminalization path. Private events such as control_channel_published, phase_goal_set, no-diff guardrail, and phase-goal failure telemetry should be considered for fail-closed handling or explicitly justified with tests.

Required repair:

  • Reject superseded closeout when active run-control channel rows exist for the obsolete issue/run unless explicit stale-lane recovery has cleared/classified them.
  • Reject obsolete issue private execution events even when no matching run-attempt row exists.
  • Add regression tests for active run-control channel evidence, private evidence with no attempt row, and the remaining allowlisted private-event behavior or a documented/tested allowlist rationale.

…loseout ownership","authority":"XY-1248","impact":"compatible"}
@yvette-carlisle

Copy link
Copy Markdown
Member Author

Addressed the latest retained repair blockers in commit efad45a4.

  • Superseded closeout now rejects durable active run-control channel rows for both canonical issue ids and identifiers, including terminal-looking attempts after lease release.
  • Issue-scoped private execution events without matching run-attempt rows now fail closed.
  • Historical control_channel_published and phase_goal_set telemetry is accepted only with canonical producer payload shapes after the independent ownership and progress guards pass; malformed telemetry fails closed.
  • Added persistent SQLite regressions for both issue identity forms plus positive and negative telemetry-shape coverage.
  • Updated OpenWiki and Decodex operator guidance for the stricter authority boundary.

Validation on pushed head efad45a4a9a076ddbc83fad2346bb2041cea678c:

  • cargo make fmt
  • cargo make test (1702 passed, 1 skipped)

Implementation and adversarial review outcome: clean. The PR remote head matches the pushed commit and remains mergeable. No inline review threads are present to resolve.

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Subagent review on head efad45a4a9a076ddbc83fad2346bb2041cea678c found remaining blockers:

P1: apps/decodex/src/recovery/closeout/apply.rs:181

Superseded closeout can self-poison retries. The apply path records pending lifecycle authority before Linear/GitHub side effects. That lifecycle write persists a private lifecycle_event. If a later side effect fails, the next run re-enters ensure_terminalizable, which rejects non-allowlisted private evidence; the allowlist does not include lifecycle_event. This contradicts the intended retryable pending closeout authority and can leave the lane stuck after a transient GitHub/Linear failure.

P1: apps/decodex/src/recovery/closeout/validation/superseded.rs:107

The fallback superseded-closeout-... run id can create private evidence without a matching run-attempt row. When no obsolete issue attempt exists, validation invents a run id instead of creating/requiring a run attempt. Apply later records lifecycle private evidence under that run id, while update_run_status is a no-op if the run row is absent. A retry then hits the new orphan private evidence and fails the private-evidence-without-attempt guard. This means the prior private-evidence fix rejects preexisting orphans but can generate a new one.

P2: apps/decodex/src/recovery/closeout/apply.rs:186

The obsolete Linear issue is moved to completed before completed closeout authority is recorded. If completed authority write fails, public tracker state is terminal while local lifecycle authority remains pending. This compounds the retry blocker above.

Required repair:

  • Make pending superseded-closeout lifecycle/private evidence retry-safe, or bypass/reconcile it explicitly when it matches the same closeout authority.
  • Avoid generating orphan private evidence from fallback run ids; create/require a matching run-attempt row or use an authority/evidence path that will not trip the no-attempt private-evidence guard on retry.
  • Reorder or harden completed-closeout authority vs public terminal issue transition so local lifecycle authority cannot lag behind terminal public tracker state.
  • Add retry/reentry tests for failed side effects after pending authority and for the no-run-attempt fallback path.

…es authority-safe","authority":"XY-1248","impact":"compatible"}
@yvette-carlisle

Copy link
Copy Markdown
Member Author

Addressed the latest retained repair findings in commit 937a8b23.

  • Exact pending/completed superseded-closeout lifecycle envelopes are now retryable only when project, issue, run/attempt, branch, obsolete PR/head, successor merge, actor, and idempotency lineage all match.
  • No-attempt recovery now records a matching terminal synthetic attempt before lifecycle evidence, while deterministic run-ID collisions fail closed during dry-run validation and live apply.
  • Completed lifecycle authority is persisted before the obsolete Linear issue moves to its terminal state.
  • Added focused retry, orphan-evidence, lineage-mismatch, sequencing, and collision coverage; updated OpenWiki and operator guidance.

Validation on pushed head 937a8b23a852f88d6ae55633012b5f7dba09ec15:

  • cargo make fmt
  • cargo test -p decodex closeout --all-features --quiet (124 passed)
  • cargo make test (1710 passed, 1 skipped)

Implementation review: clean. Independent adversarial review: clean. No inline review threads were present to resolve.

@yvette-carlisle

Copy link
Copy Markdown
Member Author

Subagent review on head 937a8b23a852f88d6ae55633012b5f7dba09ec15 found two remaining blockers. Do not land until repaired and re-reviewed.

  • P1 apps/decodex/src/recovery/closeout/apply.rs:184: superseded-closeout validates the obsolete PR head once, then later comments/closes the PR using cached validation.obsolete_landing_state without re-reading PR head/base/state or using a head precondition. The safety proof that obsolete PR has no unique unlanded patch is made in validation/superseded.rs:76, but mutating close path at apply.rs:263 and apply.rs:285 trusts the stale readback. If the obsolete PR branch moves after validation, the command can close and terminalize a PR that now contains unique work. Re-read immediately before GitHub/Linear terminal side effects and fail unless PR URL, base, state, and head SHA still match validated head.

  • P2 apps/decodex/src/recovery/closeout/validation/superseded.rs:675: successor lineage is only “successor issue has a ledger record for successor PR/head/merge” plus “obsolete head has no unique patch on default.” It never proves the successor issue/PR is actually the accepted repair for the obsolete issue. A completed unrelated successor PR can satisfy the ledger check while a third path put the obsolete patch on default, and the command will still publish “successor PR landed the accepted repair” and terminalize the obsolete issue. Add an explicit predecessor/successor relation check or stronger lineage record binding the obsolete issue/PR to the successor closeout.

Review also noted missing regression coverage for both cases: a TOCTOU obsolete PR head movement case and a negative unrelated-successor lineage case.

…eage","authority":"XY-1248","impact":"compatible"}
@yvette-carlisle

yvette-carlisle commented Jul 10, 2026

Copy link
Copy Markdown
Member Author

Addressed the current retained review findings in commit 2377fb44.

  • Superseded closeout now re-reads the obsolete PR URL, base ref/OID, state, branch, and head before each Linear/GitHub terminal mutation, then confirms the same lineage remains closed before completed authority, issue terminalization, cleanup projection, and worktree clearing.
  • Predecessor and successor issues must now have an explicit Linear relation; the existing retained mapping and successor execution ledger continue to bind each issue to its exact PR/head/merge lineage.
  • Added regression coverage for moved PR heads/bases, reopened PRs at every terminal boundary, and unrelated successor issues. Direct and inverse relation pagination is fail-closed, and the new query shape was checked against the live Linear GraphQL schema.
  • Updated the owning OpenWiki contracts/workflow pages and Decodex operator skill.

Fresh validation on pushed head 2377fb44a3c89b7ef8aa46714472d972191bd522:

  • cargo make fmt
  • cargo make test (1,719 passed, 1 skipped)

Implementation review: clean. Independent adversarial review: clean. No inline review threads were present to resolve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant