Skip to content

fix(ci): complete event-review publication as superseded when the reviewed branch vanished - #977

Merged
steipete merged 1 commit into
mainfrom
fix/vanished-branch-superseded
Jul 31, 2026
Merged

fix(ci): complete event-review publication as superseded when the reviewed branch vanished#977
steipete merged 1 commit into
mainfrom
fix/vanished-branch-superseded

Conversation

@steipete

Copy link
Copy Markdown
Contributor

Problem

Event reviews of clawsweeper's own PRs fail at "Publish review artifacts → Sync before applying artifacts" when the reviewed head branch is force-pushed or deleted mid-review: git pull --rebase errors with "Your configuration specifies to merge with the ref ... but no such ref was fetched" (runs 30646613269, 30646627620 — both on the same contributor PR batch).

The stale review is worthless anyway — the newer head triggers its own review event — so a red run here is pure noise, same family as the #966 head-drift and #968 closed-item cases.

Fix

The sync step captures the pull output; on the vanished-ref signature it emits a notice, sets superseded=true, and exits 0. "Apply review artifacts" (and everything downstream keyed off artifacts_applied) is gated on superseded != 'true'. Any other pull failure still fails the step.

Proof

  • Workflow-shape assertions for the vanished-ref branch and the superseded gate; sweep-workflow suite 80/80.
  • Autoreview (codex, xhigh): clean, "patch is correct (0.98)".

@steipete
steipete requested a review from a team as a code owner July 31, 2026 16:50
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. 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 Jul 31, 2026
@clawsweeper

clawsweeper Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 31, 2026, 12:55 PM ET / 16:55 UTC.

ClawSweeper review

What this changes

The PR treats known missing-ref errors while synchronizing review artifacts as superseded events, then skips applying those stale artifacts.

Merge readiness

Blocked until real behavior proof from a real setup is added - 4 items remain

Keep this PR open. The workflow repair is narrowly scoped and consistent with the merged stale-review resilience work, but it needs removal of the release-owned changelog edit and real after-fix workflow proof before merge.

Priority: P2
Reviewed head: f0f94891312e9f0f69876af55d90be9108d13a8f

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The core workflow change is focused, but release-file churn and missing live workflow proof leave it short of merge-ready quality.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The PR reports workflow-shape tests and autoreview only; before merge, attach redacted action logs or a run that shows the vanished-ref sync notice and that artifact application did not run. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR reports workflow-shape tests and autoreview only; before merge, attach redacted action logs or a run that shows the vanished-ref sync notice and that artifact application did not run. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 4 items Current publication boundary: Current main synchronizes immediately before applying review artifacts, and later record-commit and action-sync paths require artifacts_applied; skipping application therefore prevents stale artifacts from being published.
Focused workflow coverage: The existing focused test locates the sync and apply steps and asserts their primary guards; the proposed additions target the missing-ref signature and superseded gate.
Adjacent merged design precedent: Recent merged changes already complete stale head-drift and closed-item review events as no-op outcomes, establishing the same resilience direction at earlier queue-reservation stages.
Findings 1 actionable finding [P2] Remove the release-owned changelog entry
Security None None.

How this fits together

The sweep workflow produces review artifacts for queued items, synchronizes its publication checkout, and applies the artifacts to canonical records and GitHub. This change sits between artifact download and application, deciding whether a stale review event is published or completed as a no-op.

flowchart LR
  A[Queued review event] --> B[Generate review artifacts]
  B --> C[Download publication artifacts]
  C --> D[Sync publication checkout]
  D -->|Branch available| E[Apply artifacts]
  D -->|Reviewed branch vanished| F[Superseded no-op]
  E --> G[Commit records and sync actions]
  F --> H[Successful workflow completion]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR reports workflow-shape tests and autoreview only; before merge, attach redacted action logs or a run that shows the vanished-ref sync notice and that artifact application did not run. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Remove the release-owned changelog entry (P2) - CHANGELOG.md is release-owned under the repository policy, so this workflow repair should keep its release context in the PR body or commit message instead of modifying that file.
  • Resolve merge risk (P1) - Without an after-fix event run, the exact GitHub Actions checkout behavior and missing-ref output are unproven; an overly broad match could turn a real publication synchronization failure into a successful no-op.
  • Resolve merge risk (P1) - The release-owned CHANGELOG.md change adds unrelated release-process churn to an otherwise focused automation repair.

Findings

  • [P2] Remove the release-owned changelog entry — CHANGELOG.md:21
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope 26 added, 2 removed across 3 files The diff is concentrated in one workflow branch, its focused assertion, and a release-file entry that should not be part of the PR.

Merge-risk options

Maintainer options:

  1. Prove and narrow the workflow repair (recommended)
    Remove the release-owned changelog edit and attach a redacted deleted- or force-pushed-branch run showing the superseded notice and skipped artifact application before merge.
  2. Accept the remaining workflow uncertainty
    Maintainers may land the narrowly matched change on structural-test evidence if they explicitly accept that the live branch-vanish path has not been demonstrated.

Technical review

Best possible solution:

Retain the narrowly matched superseded path, remove the release-owned changelog entry, and attach redacted action logs showing that a vanished reviewed branch emits the notice while artifact application is skipped.

Do we have a high-confidence way to reproduce the issue?

No — the PR identifies two failed runs and current source confirms the exact sync/apply boundary, but no local or attached after-fix run demonstrates the vanished-ref path.

Is this the best way to solve the issue?

Yes, after the cleanup and proof: recognizing only known missing-ref signatures while preserving failure for other pull errors is the narrowest maintainable solution.

Full review comments:

  • [P2] Remove the release-owned changelog entry — CHANGELOG.md:21
    CHANGELOG.md is release-owned under the repository policy, so this workflow repair should keep its release context in the PR body or commit message instead of modifying that file.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add P2: The PR addresses noisy but bounded review-publication failures in shared automation.
  • add merge-risk: 🚨 automation: It changes success/failure handling and downstream gating in the live review-publication workflow.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR reports workflow-shape tests and autoreview only; before merge, attach redacted action logs or a run that shows the vanished-ref sync notice and that artifact application did not run. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: The PR addresses noisy but bounded review-publication failures in shared automation.
  • merge-risk: 🚨 automation: It changes success/failure handling and downstream gating in the live review-publication workflow.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR reports workflow-shape tests and autoreview only; before merge, attach redacted action logs or a run that shows the vanished-ref sync notice and that artifact application did not run. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Current publication boundary: Current main synchronizes immediately before applying review artifacts, and later record-commit and action-sync paths require artifacts_applied; skipping application therefore prevents stale artifacts from being published. (.github/workflows/sweep.yml:3627, 0ee212e35c1a)
  • Focused workflow coverage: The existing focused test locates the sync and apply steps and asserts their primary guards; the proposed additions target the missing-ref signature and superseded gate. (test/sweep-workflow.test.ts:195, 0ee212e35c1a)
  • Adjacent merged design precedent: Recent merged changes already complete stale head-drift and closed-item review events as no-op outcomes, establishing the same resilience direction at earlier queue-reservation stages. (.github/workflows/sweep.yml:3627, b4d1edea05cc)
  • Repository policy: Repository policy keeps CHANGELOG.md release-owned and requires real behavior proof for code-bearing workflow changes; the PR body provides structural tests and autoreview, not an after-fix workflow run. (AGENTS.md:34, 0ee212e35c1a)

Likely related people:

  • Peter Steinberger: Authored the recent merged stale-review resilience fixes in the same sweep workflow area. (role: recent automation contributor; confidence: high; commits: 3dc70e67f287, b4d1edea05cc; files: .github/workflows/sweep.yml, test/sweep-workflow.test.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Remove the CHANGELOG.md entry.
  • Attach redacted after-fix workflow output for the deleted- or force-pushed-branch scenario.
  • If the PR-body update does not trigger a fresh review, ask a maintainer to comment @clawsweeper re-review.

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.

@steipete
steipete merged commit 197d807 into main Jul 31, 2026
17 checks passed
@steipete
steipete deleted the fix/vanished-branch-superseded branch July 31, 2026 16:58
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: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant