Repository navigation
fix: do not close or announce a merge that GitHub has not confirmed #339
Description
Activity
- added
on Sep 27, 2026 Codex review: this still needs some work. Reviewed October 3, 2026, 7:49 PM ET / 23:49 UTC.
Summary
Both source-proven defects remain on current main. The open fixing PR #341 owns the repair and preserves the reporter’s contribution; this issue remains necessary until that work lands.Reproducibility: yes. source establishes a precise trigger: a successful merge command followed by GitHub metadata without a merge timestamp still produces executed status in both paths. No live merge-queue reproduction or test execution was performed in this read-only review.
Next step
Review and land or resolve #341; its explicit closing link rules out a competing fix PR.Review details
Best possible solution:
Both merge paths should require confirmed merge metadata for the reviewed head and retain unconfirmed submissions as pending without closeouts or success announcements.
Do we have a high-confidence way to reproduce the issue?
Yes, source establishes a precise trigger: a successful merge command followed by GitHub metadata without a merge timestamp still produces executed status in both paths. No live merge-queue reproduction or test execution was performed in this read-only review.
Is this the best way to solve the issue?
Yes, applying the existing applicator confirmation contract to both paths is a focused repair; pending submissions must remain replayable until GitHub confirms completion.
AGENTS.md: found and applied where relevant.
Codex review notes: model internal, reasoning medium; reviewed against 85936b40ec88.
Label changes
Label changes:
No label changes.
Label justifications:
P1: Current merge automation can close unresolved issues and announce success before GitHub confirms a merge.impact:other: The issue affects the correctness of repository automation, closeouts, and execution records.
Evidence reviewed
What I checked:
- Post-flight still permits premature closeout: After the merge command succeeds, the finalizer unconditionally returns executed while allowing null merge timestamp and commit SHA. The caller invokes linked-item closeouts whenever that status is executed. (
scripts/post-flight.mjs:227, 85936b40ec88) - Automerge still fabricates confirmation: The router returns executed after a zero exit, substitutes the current time for absent mergedAt, and accepts a null merge commit. Its caller proceeds to post a response and mark the command executed. (
scripts/comment-router.mjs:1130, 85936b40ec88) - Existing confirmation contract: The applicator already blocks a merge result unless it contains a merge timestamp and a full 40-character merge commit SHA, demonstrating an existing safety contract rather than a new capability. (
scripts/apply-result.mjs:1089, 85936b40ec88) - Live fixing PR remains open: GitHub confirms fix(merge): require confirmed merges and passing preflight checks #341 is open, merged=false, and merged_at=null at head 5b3e6bc. Its body explicitly closes this issue and carries forward SebTardif’s diagnosis and patches with co-author credit. A populated mergeCommitSha in the supplied context is not evidence of a completed merge. (5b3e6bc4895b)
- Feature-history routing: Path history lists recent work by Peter Steinberger and repeated merge-safety changes by Vincent Koc. Exact-line blame and historical patch inspection could not complete because required historical objects were unavailable and their retrieval returned HTTP 403; no introduction attribution is claimed. (
scripts/post-flight.mjs, 85936b40ec88) - Current-main and release check: The checkout matches the fetched main SHA and still contains both defects. GitHub’s latest-release endpoint returned 404, so no shipped fix was established. (85936b40ec88)
Likely related people:
- Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
- Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
How this review workflow works
- 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-reviewcomments, 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-reviewor@clawsweeper re-runon an open PR or issue to request a fresh review only. - Maintainers can also comment
@clawsweeper reviewto 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 explainto ask for more context, or@clawsweeper stopto stop active automation.
🦞🔧
ClawSweeper is automatically building a fix for this issue.The issue review finished and no active implementation pull request was found.
Automatic implementation progress:
- State: Queued
- Detail: Review complete; no active implementation PR was found. The Codex implementation worker is queued.
- Run: https://github.com/openclaw/clawsweeper/actions/runs/36368606158
- Updated: 2026-09-28T02:09:59.130Z
- Opt out: add
clawsweeper:manual-onlyorclawsweeper:human-review.
- added and removed
on Oct 2, 2026 - added a commit that references this issue
on Oct 10, 2026 Fixed in #345 (commit add5308), preserving your original contribution. Post-flight and the router now require GitHub confirmation of the reviewed head before announcing success or closing linked issues. Pending submissions survive retries and later-command failures; a replacement head needs a valid review before closeout.
Production-CLI regressions cover queued, incomplete, replaced-head, and replay outcomes with synthetic GitHub responses. All 814 tests and 7,019 job specifications passed on AWS with zero skips, and exact-head hosted validation passed: https://github.com/openclaw/clownfish/actions/runs/38030256963 . Thank you for the diagnosis and patch.
gh pr mergecan exit 0 before GitHub has a merged pull request. A merge-queue enqueue is one case. Post-flight and automerge both treated that zero exit as a completed merge.From reading
scripts/post-flight.mjsandscripts/comment-router.mjs, and fromnode --test test/unconfirmed-merge.test.mjs test/lib.test.mjs. This was not reproduced against a live merge queue.Post-flight. After
gh pr merge --squashexits 0,finalizeFixPrrecordedexecutedeven when the follow-up fetch had nomerged_at.finalizePostMergeCloseoutsthen commented on and closed linked issues.apply-result.mjsalready blocks this case with "merge command returned without a verified merged pull request".Automerge.
executeAutomergerecordedexecuted, posted that Clownfish merged the PR, and storednew Date()whenmergedAtwas missing. The ledger then skipped that comment on later passes.The fork branch requires
merged_atand a 40-character merge commit SHA before either path reports success. Post-flight stays blocked and skips closeout. Automerge returnswaiting, so the pass is not ledgered as executed.Upstream pull requests are limited to collaborators (
pull_request_creation_policyiscollaborators_only), so the patch is on the fork:f6716f70a5aac39496041bd7c206754c81ab835bSebTardif:fix/require-verified-merge