ci(repo): stale-base squash merge hygiene guard (#2691) - #2799
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Claude finished @kyle-sexton's task in 53s —— View job CI Security Review
No security issues found. Reviewed the security-relevant surface of this PR:
Supply-chain and pin-related concerns (action pinning, trigger patterns, permissions) are left to zizmor's static advisory lane per this skill's scope. |
|
Last security-reviewed head: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95cc4ef46f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
cb0c40f to
7cb2fd5
Compare
Add check-stale-base-overlap as a PR CI tripwire, document the babysit and pull-request never-merge-while-behind rule, and point at github-iac strict as the durable prevention for silent squash reverts. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
… sides Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
7cb2fd5 to
63d878f
Compare
…ll docs Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
|
Claude finished @cursor[bot]'s task in 3m 8s —— View job CI Code Review
Review skipped: this PR is already merged. Per
Nothing further to review or post. |
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
…trict recommendation (#2830) ## What Wording-only correction to two defects in how `scripts/check-stale-base-overlap.sh` (shipped in #2799) describes itself, and to the same framing where it had propagated. The gate itself is unchanged and stays required. It correctly detects the stale-**base** class. ## Defect 1 — false coverage citation The header cited `claude-code-plugins#2691: #2635 then #2639 lost inside ten minutes` as its motivating incident. Those incidents are a different class: ``` git merge-base --is-ancestor f603880 refs/pull/2641/head # exit 0 — #2639 WAS in the ancestry git grep -c "read-only supporting allowlist" refs/pull/2641/head # zero — the tree lacked the work ``` The bases were fresh; the trees were stale. Run against those exact branches this gate exits 0 ("fresh"). A required check was naming incidents it does not detect. The class that covers them is `scripts/check-silent-revert.sh` (#2808). The two cover **disjoint** classes; neither subsumes the other. Every touched surface now says that explicitly. ## Defect 2 — a recommendation barred by an accepted ADR The header called GitHub's `requiredStatusChecks.strict` "the durable prevention". That is wrong twice over: - It is barred. `melodic-software/github-iac` → `docs/adr/0001-relax-strict-required-status-checks.md` is ACCEPTED and implemented as a non-strict ruleset; its revisit trigger prescribes per-repo granularity, not a global flip. - It is false. The measurement above shows strict — and a merge queue — would have **passed** all three recorded incidents. Replaced with a neutral statement of scope. Nothing here endorses the setting, softly or otherwise, and nothing recommends flipping it. `.github/workflows/silent-revert-canary.yml` and `scripts/check-silent-revert.sh` already carried the accurate framing and were deliberately left untouched. ## Files | File | Change | | --- | --- | | `scripts/check-stale-base-overlap.sh` | Header "Why" rewritten with an explicit SCOPE paragraph; the "Durable prevention" stderr line replaced with a neutral scope statement | | `.github/workflows/ci.yml` | Job comment only — no logic, wiring, or `needs` touched | | `plugins/source-control/skills/babysit-prs/reference/freshness.md` | Dropped the "#2635 then #2639 ... inside ten minutes" citation and the "when the durable `strict` setting is not yet on" framing | | `plugins/source-control/skills/pull-request/reference/merge.md` | Dropped "the durable fix is `strict: true`" | | `plugins/source-control/CHANGELOG.md` | New 0.54.9 entry; 0.54.7's trailing clause amended because it actively recommended the barred setting rather than merely recording a release | | `plugins/source-control/.claude-plugin/plugin.json` | 0.54.8 → 0.54.9 (changelog-parity) | ## Verification - `bash -n scripts/check-stale-base-overlap.sh` — OK. The header sits directly above `set -uo pipefail`; parsing is intact. - `bash scripts/check-stale-base-overlap.test.sh` — **ALL PASS**, exit 0 (all five cases). - No behavioral change. The only non-comment lines in the diff are stderr prose inside two `echo` strings. - `plugins/disk-hygiene/CHANGELOG.md:83` describes the incidents as "squash-merged from stale bases" — the same wrong premise, but it is another plugin's release history and makes no claim about this gate's coverage. Left alone deliberately. ## Related No linked issue. This corrects wording on surfaces shipped by an already-merged PR; nothing here closes a GitHub issue. - #2799 — merged the gate and the incorrect header wording this PR corrects. - #2808 — merged `scripts/check-silent-revert.sh`, the detector that actually covers the disjoint class. - #2691 — the audit whose incidents were miscited. CLOSED as COMPLETED and deliberately left closed; referenced for context only, with no closing keyword. - `melodic-software/github-iac` → `docs/adr/0001-relax-strict-required-status-checks.md` — the ACCEPTED ADR that bars the recommendation removed here. Not touched by this PR. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
<!-- CURSOR_AGENT_PR_BODY_BEGIN --> No linked issue ## Summary Every push to `main` since #2799 has failed the required `ci-status` check because `stale-base-overlap-gate` skipped on non-PR events and the aggregator rejects any `needs.*.result` other than `success`. ## Fix Remove the job-level `if: github.event_name == 'pull_request'`. The detector self-test now runs on every event (including push to `main`). Fetch-base and the overlap check stay PR-only. Checkout uses `github.event.pull_request.head.sha || github.sha` so `ref` is never empty on push. The aggregator is unchanged: skipped required lanes stay fail-closed. ## Verification - `bash scripts/check-stale-base-overlap.test.sh`: ALL PASS (usage, fresh base, overlapping behind-base fails, disjoint behind-base, missing ref). - This PR's `stale-base-overlap-gate` ran (not skipped) and passed; `ci-status` passed. - After merge, a `main` push should report `stale-base-overlap-gate` as `success` (self-test only) so `ci-status` can go green. ## Related Refs #2691 (the stale-base gate this job implements). The skip-on-push failure is the same never-skip contract documented on `plugin-gate` / `skill-quality-gate` / `hygiene`. <!-- CURSOR_AGENT_PR_BODY_END --> <div><a href="https://cursor.com/agents/bc-e8b0bda9-a6d8-4815-a457-6a4adfc9d54b?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a> <a href="https://cursor.com/background-agent?bcId=bc-e8b0bda9-a6d8-4815-a457-6a4adfc9d54b&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> </div> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Summary
Adds a PR CI gate (
scripts/check-stale-base-overlap.sh) that fails when the head is behind its base on overlapping paths — the failure mode that silently reverted #2635/#2639 with CI green. Documents the babysit/pull-request rule: never squash-merge while behind, even ifmergeStateStatusis CLEAN under a non-strict ruleset. Points atrequiredStatusChecks.strictinmelodic-software/github-iacas the durable prevention.Test plan
bash scripts/check-stale-base-overlap.test.sh(fresh / overlap / disjoint / bad-ref)stale-base-overlap-gateon this PRRelated
Closes #2691