Skip to content

fix(apply): prevent one-sided paired closes after live drift - #1042

Draft
vincentkoc wants to merge 1 commit into
mainfrom
fix/clawsweeper-close-guard-pairs
Draft

fix(apply): prevent one-sided paired closes after live drift#1042
vincentkoc wants to merge 1 commit into
mainfrom
fix/clawsweeper-close-guard-pairs

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Aug 6, 2026

Copy link
Copy Markdown
Member

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

  • Frozen base: 0588bda948653c59a60b65c01d9ff3ce1f780df4.
  • Reviewed head: 1c23b39f3174cdd2a0f90ba9e2c1d7882cce454d.
  • Direct TypeScript build passed: ./node_modules/.bin/tsc -p tsconfig.json.
  • Focused runtime/policy proof passed: 112 tests, 112 passed, 0 failed.
  • Exact-head Codex review found no actionable defects; its direct build, lint,
    and 85 affected tests passed.
  • Independent committed-range ClawSweeper review found no code or security
    blocker against the exact frozen base/head.
  • Type-aware oxlint passed for all changed production files with warnings denied.
  • oxfmt and git diff --check passed.
  • Production TypeScript delta: +43 net lines.
  • Test delta: +215 net lines.
  • The merged fix(workflow): scope comment-only reconciliation #1040 touches workflow
    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

  • A counterpart is closed during initial context hydration, then reopens before
    terminal pair validation.
  • A counterpart is closed during initial hydration, then its terminal GitHub
    refresh fails.
  • stale_version_bug is disabled for the counterpart.
  • stale_insufficient_info gains recent human activity.
  • The counterpart becomes locked, closed, or unknown after admission.
  • Current-item stale-version and obsolete-fix policy changes occur after the
    close-time comment/lease step.
  • A legacy ClawSweeper-owned close note is updated before the final guards.

Command and environment

./node_modules/.bin/tsc -p tsconfig.json
node --test --test-concurrency=2 \
  test/apply-close-policy-guards.test.ts \
  test/apply-same-author-pair-close.test.ts \
  test/apply-stale-version-bug-policy.test.ts \
  test/apply-obsolete-fix-pr-policy.test.ts \
  test/apply-label-sync.test.ts \
  test/close-reasons.test.ts

Executed on macOS with Node 26.5.0 from head
1c23b39f3174cdd2a0f90ba9e2c1d7882cce454d, based on
0588bda948653c59a60b65c01d9ff3ce1f780df4.

Observed result

  • Both closed-to-open and closed-to-refresh-error counterparts produced
    skipped_same_author_pair; no close command ran.
  • Counterpart policy blockers prevented pair admission before queue mutation.
  • A confirmed-closed counterpart was queued for the existing archive path.
  • Final stale-version, obsolete-fix, pair, and lease changes prevented close
    after earlier candidate approval.
  • The close-time note says execution is conditional on final live guards.
  • An owned legacy note was patched before the final guard and close sequence.

Artifact or trace

The executable assertions are in:

  • test/apply-close-policy-guards.test.ts
  • test/apply-same-author-pair-close.test.ts
  • test/apply-label-sync.test.ts
  • test/close-reasons.test.ts

Limits

  • Blacksmith Testbox could not start because the repository has no matching
    workflow configuration.
  • AWS Crabbox hydration rejected pnpm/action-setup@v6.0.9; the no-hydrate
    fallback then hit coordinator error 1101 before a lease was created.
  • The full local coverage command remains blocked by the existing package mirror
    miss for pnpm@10.33.0; focused changed-surface proof passed.
  • The committed-range reviewer classified the current proof as controlled
    mock/fixture evidence and requested a redacted production-path apply trace
    before merge. This draft intentionally stops before that final merge gate.
  • No live GitHub item was applied, commented, or closed.
  • No npm or release publishing was performed.

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

  • Root cause: pair and liveness facts were cached at candidate time and reused
    after later mutations.
  • Owner: apply close-policy evaluation and terminal apply sequencing.
  • Canonical fix: one policy evaluator, live terminal revalidation, and
    fail-closed relation refresh.
  • Removed paths: pair-start memoization and final reliance on cached
    stale-version/obsolete-fix candidate results.
  • Sibling coverage: issue/PR pair direction, stale-version, obsolete-fix,
    stale-insufficient-info, locked/closed/unknown state, legacy note migration,
    and dry-run sequencing.

@vincentkoc vincentkoc self-assigned this Aug 6, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Aug 6, 2026
@clawsweeper

clawsweeper Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed August 29, 2026, 2:04 AM ET / 06:04 UTC.

ClawSweeper review

What this changes

The 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
Reviewed head: 1c23b39f3174cdd2a0f90ba9e2c1d7882cce454d

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) PR readiness rating was derived from proof quality, review findings, security review, and reviewer confidence.
Proof confidence 🌊 off-meta tidepool Not applicable: Real behavior proof is not required for maintainer- or bot-authored pull requests.
Patch quality 🧂 unranked krab (1/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: Real behavior proof is not required for maintainer- or bot-authored pull requests.
Evidence reviewed 5 items Introduced terminal guard: The PR rechecks pair state immediately before the current item closes, but does not make the later counterpart mutation part of that terminal operation.
Sequential execution: The workflow calls close execution for the current item and records it closed before the queued counterpart is processed in a later loop iteration.
Current-main drift: Current main substantially rewrites close execution, including paired mutation-lease handling, so this branch needs an integration refresh rather than an endpoint-based landing decision.
Findings 2 actionable findings [P1] Keep the first pair item open until the counterpart is terminal
[P2] Remove the release-owned changelog entry
Security None None.

How this fits together

ClawSweeper’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]
Loading

Before merge

  • Keep the first pair item open until the counterpart is terminal (P1) - The new pre-close pair check only proves the counterpart is eligible before this PR calls closeItem. The counterpart is processed separately and can then fail its own final policy, lease, or close operation, leaving this PR closed alone. Use a pair-level terminal protocol or compensate the first mutation, and cover this sequence.
  • Remove the release-owned changelog entry (P2) - Repository policy reserves CHANGELOG.md for release ownership. Remove this entry and keep release context in the PR body and commit message.
  • Resolve merge risk (P1) - The first item of a same-author pair can still close while the counterpart later fails its final policy, lease, or mutation step.
  • Resolve merge risk (P1) - Substantial current-main drift means the branch requires a refreshed integration review.
  • Complete next step (P2) - Member authorship prevents auto-close; the branch needs the two identified repairs and a current-main rebase before re-review.
  • Improve patch quality - Address the highest-priority review finding and re-run the changed-surface validation.

Findings

  • [P1] Keep the first pair item open until the counterpart is terminal — src/clawsweeper-apply-close-execution.ts:309-310
  • [P2] Remove the release-owned changelog entry — CHANGELOG.md:179
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed surface 9 production files, 1 changelog file, 4 test files The diff spans close admission, execution, policy evaluation, context refresh, and comment publication.
Line delta production +43 net, tests +215 net The tests do not cover a successful first close followed by a failed second terminal step.

Merge-risk options

Maintainer options:

  1. Make paired closure terminally safe (recommended)
    Validate and close both pair members through a protocol that cannot finalize the first item if the second final guard or mutation fails.
  2. Pause the branch
    Keep this branch unmerged if the current-main paired-close redesign is the preferred landing path.

Technical review

Best 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:

  • [P1] Keep the first pair item open until the counterpart is terminal — src/clawsweeper-apply-close-execution.ts:309-310
    The new pre-close pair check only proves the counterpart is eligible before this PR calls closeItem. The counterpart is processed separately and can then fail its own final policy, lease, or close operation, leaving this PR closed alone. Use a pair-level terminal protocol or compensate the first mutation, and cover this sequence.
    Confidence: 0.98
  • [P2] Remove the release-owned changelog entry — CHANGELOG.md:179
    Repository policy reserves CHANGELOG.md for release ownership. Remove this entry and keep release context in the PR body and commit message.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against b1d6b8513271.

Labels

Label justifications:

  • P2: This is a bounded but consequential defect in internal GitHub close automation.
  • merge-risk: 🚨 automation: The PR changes live policy checks and close-mutation sequencing in the apply workflow.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: Real behavior proof is not required for maintainer- or bot-authored pull requests.

Evidence

What I checked:

Likely related people:

  • Peter Steinberger: Blame attributes the existing close sequence to the apply-phase refactor. (role: original close-flow refactor author; confidence: high; commits: 857498060344; files: src/clawsweeper-apply-close-execution.ts)
  • Vincent Koc: The introduced terminal pair checks are attributed to this PR commit. (role: introduced current pair-recheck change; confidence: high; commits: 1c23b39f3174; files: src/clawsweeper-apply-close-execution.ts, src/clawsweeper-apply-close-guards.ts)
  • Milton: Recent current-main history touched the close execution path, though the commit subject concerns validation. (role: recent area contributor; confidence: medium; commits: 1db170c5bf7a; files: src/clawsweeper-apply-close-execution.ts)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (40 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-09T18:28:56.452Z sha 1c23b39 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-09T19:59:55.614Z sha 1c23b39 :: found issues before merge. :: [P2] Revalidate the pair inside the close mutation guard
  • reviewed 2026-08-09T21:04:35.013Z sha 1c23b39 :: needs changes before merge. :: [P2] Revalidate the pair in the close mutation guard | [P2] Remove the release-owned changelog entry
  • reviewed 2026-08-09T22:48:06.668Z sha 1c23b39 :: found issues before merge. :: [P2] Remove the release-owned changelog entry
  • reviewed 2026-08-09T23:58:43.858Z sha 1c23b39 :: found issues before merge. :: [P1] Keep the first pair item open until the counterpart is terminal | [P2] Remove the release-owned changelog entry
  • reviewed 2026-08-11T12:13:45.026Z sha 1c23b39 :: found issues before merge. :: [P1] Prevent the first close until the paired item is terminal | [P2] Remove the release-owned changelog entry
  • reviewed 2026-08-12T06:24:14.110Z sha 1c23b39 :: found issues before merge. :: [P1] Prevent a split close after the first pair mutation | [P2] Remove the release-owned changelog entry
  • reviewed 2026-08-22T02:18:36.040Z sha 1c23b39 :: found issues before merge. :: [P1] Keep the first pair item open until the counterpart is terminal

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 7, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant