fix(apply): prevent one-sided paired closes after live drift - #1042
fix(apply): prevent one-sided paired closes after live drift#1042vincentkoc wants to merge 1 commit into
Conversation
|
Codex review: found issues before merge. Reviewed August 29, 2026, 2:04 AM ET / 06:04 UTC. ClawSweeper reviewWhat this changesThe PR adds live close-policy rechecks for same-author issue/PR pairs and makes pre-close PR comments conditional on final guards. Merge readiness⛔ Blocked by patch quality or review findings - 6 items remain Keep open: this member-authored PR still permits a split same-author pair close because it finalizes the first item before the queued counterpart reaches its own terminal guard. It also needs refresh against substantial current-main changes in the same execution path. Priority: P2 Review scores
Verification
How this fits togetherClawSweeper’s apply lane reads durable review records, refreshes live GitHub state, and performs guarded comment and close operations. Same-author issue/PR pairs are admitted together but their close operations are executed separately. flowchart LR
A[Reviewed pair records] --> B[Pair admission]
B --> C[Live policy checks]
C --> D[First GitHub close]
D --> E[Counterpart final guard]
E --> F[Counterpart close or skip]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Rebase onto current main, remove the release-owned changelog edit, and use one paired mutation protocol that leaves both items open if either final counterpart step fails. Do we have a high-confidence way to reproduce the issue? Yes: admit a paired PR and issue, let the PR pass its new pre-close check and close, then make the queued issue fail its own final guard or mutation; no two-item terminal protocol or compensation exists. Is this the best way to solve the issue? No: the new checks improve admission safety but do not prevent a split result after the first close. A pair-level terminal protocol is the narrower solution. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against b1d6b8513271. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (40 earlier review cycles; latest 8 shown)
|
What Problem This Solves
Fixes an issue where ClawSweeper could approve a same-author issue/PR pair from
stale candidate context, then close one side after the counterpart changed,
became policy-ineligible, or could no longer be revalidated.
It also fixes close-time notes that described a PR as already closed before the
final live guards and close command had completed.
Why This Change Was Made
The apply workflow now uses one mutation-free close-reason policy evaluator for
both the current item and its paired counterpart. Candidate results remain a
cheap prefilter; the final path rechecks live stale-version, obsolete-fix,
comment-activity, lease, and pair state before the close-time note and again
immediately before closing.
Related-item refreshes preserve prior identity plus refresh errors, and a known
same-author pair fails closed when its live relation cannot be revalidated.
Confirmed-closed counterparts still enter the normal archive path. The
close-time PR note is status-neutral, and legacy ClawSweeper-owned notes are
updated before the final guards.
No release publishing, npm publishing, or live apply/close operation is part of
this change.
User Impact
Operators can expect paired issue/PR closes to remain atomic from the apply
workflow's perspective: one side is not closed when the other side has reopened,
become blocked by current policy, or cannot be refreshed safely.
PR comments no longer claim that a close already happened while a final live
guard can still keep the PR open.
Evidence
0588bda948653c59a60b65c01d9ff3ce1f780df4.1c23b39f3174cdd2a0f90ba9e2c1d7882cce454d../node_modules/.bin/tsc -p tsconfig.json.and 85 affected tests passed.
blocker against the exact frozen base/head.
oxfmtandgit diff --checkpassed.+43net lines.+215net lines.reconciliation only; it has no overlapping files or runtime ownership with
this apply-close change.
Real Behavior Proof
Claim
ClawSweeper does not close one side of a same-author issue/PR pair when the
counterpart reopens, fails a current close policy, or cannot be revalidated.
No public close note claims success before final live guards pass.
Exercised surface
The proof executes the compiled apply-decision workflow with controlled GitHub
CLI fixtures, the real pair admission/terminal guard path, the real close-reason
policy evaluator, the close-time comment mutation path, and final lease/policy
sequencing.
Scenario or fixture
terminal pair validation.
refresh fails.
stale_version_bugis disabled for the counterpart.stale_insufficient_infogains recent human activity.close-time comment/lease step.
Command and environment
Executed on macOS with Node 26.5.0 from head
1c23b39f3174cdd2a0f90ba9e2c1d7882cce454d, based on0588bda948653c59a60b65c01d9ff3ce1f780df4.Observed result
skipped_same_author_pair; no close command ran.after earlier candidate approval.
Artifact or trace
The executable assertions are in:
test/apply-close-policy-guards.test.tstest/apply-same-author-pair-close.test.tstest/apply-label-sync.test.tstest/close-reasons.test.tsLimits
workflow configuration.
pnpm/action-setup@v6.0.9; the no-hydratefallback then hit coordinator error 1101 before a lease was created.
miss for
pnpm@10.33.0; focused changed-surface proof passed.mock/fixture evidence and requested a redacted production-path apply trace
before merge. This draft intentionally stops before that final merge gate.
OpenClaw Bay Impact
None. This changes internal apply-close policy and mutation sequencing only; it
does not change dashboard, observer, public status, or data-contract surfaces.
Architecture Notes
after later mutations.
fail-closed relation refresh.
stale-version/obsolete-fix candidate results.
stale-insufficient-info, locked/closed/unknown state, legacy note migration,
and dry-run sequencing.