Skip to content

revert(pr-merge): drop the wake's unmoved-refusal skip (#13107), which never fired in 24 h - #13453

Merged
kyle-sexton merged 4 commits into
mainfrom
revert/13107-wake-unmoved-skip
Nov 5, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
revert/13107-wake-unmoved-skip

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Nov 5, 2026 •

Copy link
Copy Markdown
Member

Refs: #12445

Summary

#13107 set its own 24 h gate: revert if probe-only wakes x 9 calls > skipped wakes x 64 calls. Measured from 2026-11-04T06:16Z to 2026-11-05T06:16Z, with the GitHub outages 09:54-11:44Z, 16:00-16:54Z and 17:45-18:16Z left out, over 62 pr-merge workflow_dispatch wakes:

  • 0 report=last outcome=skipped: the skip never fired;
  • 29 probe-only (11 fresh check refused, 18 fresh check skipped);
  • 29 armed;
  • 4 report=none.

29 x 9 = 261 > 0 x 64 = 0, so the gate fails and the skip comes out.

Fix

Removed (the refusal-stands skip only):

  • pr-merge.yml keep-report: the refusal-stands probe, its ::notice, the refusal-stands= output and the variables it used; the wake-outcome step's REFUSAL_STANDS env and its skipped: the last sweep's report still refuses ... outcome.
  • babysit_verify_evidence.py refusal-stands (_refusal_stands).
  • babysit_postmerge.py: CONTENT_REFUSALS, content_refusal and refusal_moved. running is dropped from _verifying_runs_unchanged and idle_runs_refusal because refusal_moved was its only caller.
  • Their tests in test_babysit_lane_room.py and workflow.test.mjs, and the matching sentence in docs/conventions/pr-pipeline/README.md.

Kept:

New change fragments for source-control (patch, Removed) and pr-pipeline (none). #13107's own fragments already shipped.

Verification

  • pytest plugins/source-control/skills/babysit-prs/scripts/tests: 4110 passed.
  • The new IdleRefusal test fails when judged=_judged_runs(report) is deleted from idle_runs_refusal ('runs-changed' is not None) and passes with it.
  • node --test .github/scripts/pr-merge/workflow.test.mjs: 108 of 108 pass, after merging origin/main.
  • ruff check on the babysit scripts, scripts/sync-hold-floor.py --check and scripts/check-changelog-fragments.sh --check all pass.
  • Per-wake call delta: a wake goes back to having no probe. That saves about 9 calls on each wake whose last report refused and did not stand (29 of 62 wakes in the window). No wake loses a skip, because none was taken.
  • At the ready flip, on head ac52bc1 (main merged in): test_babysit_lane_room.py and test_babysit_postmerge*.py 618 passed; workflow.test.mjs 108 of 108.

Security review at the ready flip (non-blocking notes)

An independent security review of the diff found nothing blocking: no secret exposure, no permission or trigger change, no billing change. The arm step's condition (pr-merge.yml:2596) still reads only keep-report.outputs.stands, and nothing reads refusal-stands any more. A wake the skip used to refuse now runs the fresh check and arms only when that check and settle --fresh both pass, the same rule every other wake follows. Two non-blocking notes:

  • pr-merge.yml:2552-2585: a wake the skip refused can now arm, but only where the probe's "nothing moved" prediction was wrong. That is the intended effect, not a gate bypass.
  • workflow.test.mjs:694: the test asserts only the first call before the fresh check (run.calls[0]), plus the total call count. It could pin the exact check count per outcome, but the count assertion already catches an extra call. Coverage only; left as is.

Remaining

What stays open on #12445 after this PR:

Done-when 1-3 are delivered:

Related

🤖 Generated with Claude Code

kyle-sexton and others added 4 commits November 5, 2026 01:28
…h never fired in 24 h

This reverts the refusal-stands skip from commit
55f8a62. Its own 24 h gate failed: in
62 wakes, 0 skipped on the probe and 29 paid it for nothing.

Removed: the keep-report step's refusal-stands probe, its notice and
output, the REFUSAL_STANDS outcome, babysit_verify_evidence.py
refusal-stands, babysit_postmerge.py content_refusal, refusal_moved and
CONTENT_REFUSALS, the running option of _verifying_runs_unchanged and
idle_runs_refusal (only refusal_moved passed it), their tests and the
pr-pipeline README sentence.

Kept: the judgedRuns reading added later (judged, moved(),
_judged_runs), and the same commit's separate run-cache and attempts
wiring (BABYSIT_RUN_CACHE in keep-report, attempts.json kept beside the
fresh report, the arm step's BABYSIT_POSTMERGE_ATTEMPTS) with its tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The revert removed the only test that reached idle_runs_refusal's
judged= argument, which sat in the refusal_moved class. The new
IdleRefusal case shows a completed run the report recorded at the same
updated_at counts as unmoved, while the same run without the record, or
re-run since, counts as moved. It fails when the argument is deleted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 5, 2026 06:40
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-05T06:42:44.779792Z ac52bc1 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

claude-security-review reviewed ac52bc12ff84f68de60fa138b9e7ceac3eb517ec: read 8 of 8 changed file(s); 0 finding(s), 0 as inline comments.

Security review: no security issues found

This was a full review: I read every hunk of all 8 changed files. There were no earlier heads, so the review was not limited to blocking findings.

This pull request reverts #13107's "refusal-stands" skip in pr-merge's PR-scoped wake. Three files match the trusted-path list and got full scrutiny: .github/workflows/pr-merge.yml, babysit_postmerge.py and babysit_verify_evidence.py.

Merge hold check. The removed skip could only stop a wake from arming. When it fired, it deleted the last report so the arm step stayed skipped. With it gone, a wake that used to stop there now runs the fresh post-merge check, which every other wake already runs. The arm step's condition is unchanged:

  • It still requires steps.keep-report.outputs.stands == 'true' for a PR-scoped dispatch (pr-merge.yml#L2596).
  • stands becomes true only when one of these passes:
    • the last report has exit 0 or 5 and settle --idle-tick keeps it (#L2546);
    • the fresh check exits 0 or 5 and settle --idle-tick --fresh keeps it (#L2573-L2574).

So the hold is not widened. A report the skip used to stop on still fails the same idle-tick checks unless something it read has actually changed.

Removed code. I searched the tree: no callers remain for refusal_moved, content_refusal in postmerge, the refusal-stands subcommand, or the REFUSAL_STANDS output. Dropping running= from _verifying_runs_unchanged and idle_runs_refusal leaves their default behaviour the same, and the only caller that passed running=True was removed. last_code and last_sha are still assigned before use at #L2544-L2545, so set -u is unaffected.

Other checks.

  • No changes to permissions, triggers, tokens, secrets, trusted actors, the kill switch or plugin manifests.
  • No new untrusted input reaches a step, and no outputs are newly interpolated: the refusal-stands output and the REFUSAL_STANDS env var were removed.
  • The pull request text contains no embedded instructions to the lane.

Spend. The change uses no paid resources. It only changes how many GitHub API calls a wake makes, which the description puts at about 9 fewer on a wake whose last report refused.

critical-findings: 0

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

claude-review reviewed ac52bc12ff84f68de60fa138b9e7ceac3eb517ec: read 8 of 8 changed file(s); 0 finding(s), 0 as inline comments.

Code review: PR #13453, a revert of #13107's unmoved-refusal skip

No findings. I read every hunk of all 8 changed files in the full diff. This is the first review of this pull request (0 earlier heads), so all findings were in scope, not only blocking ones.

What I checked

  • pr-merge.yml keep-report step. The refusal-stands branch is gone, and so are the refusal= / standing= variables and the refusal-stands= output. last_code and last_sha are no longer set up front. They are still assigned and read only inside the if [ -f … ] block (pr-merge.yml#L2543-L2549), so set -u cannot trip on them. The arm step's condition still reads only keep-report.outputs.stands (#L2596). The wake-outcome step no longer reads REFUSAL_STANDS.
  • Leftover references. A repo-wide grep for refusal-stands, REFUSAL_STANDS, refusal_moved, CONTENT_REFUSALS, _refusal_stands and running= in babysit code turns up only the new change fragment and an older shipped CHANGELOG.md entry. babysit_dependabot.content_refusal is a different function and is untouched.
  • babysit_postmerge.py. With running gone, moved() counts only completed runs. The judged check still compares the run's current updated_at against judgedRuns, so fix(source-control): count a verifying run as moved only when the post-merge check did not read it #13241's behaviour is kept for completed runs, which matches the PR description.
  • Tests.
    • The new IdleRefusal.test_a_run_the_report_judged_finished_moves_nothing covers the judged=_judged_runs(report) wiring in idle_runs_refusal: unrecorded counts as moved, recorded at the same updated_at counts for nothing, and a later updated_at counts again. This replaces the coverage lost when the RefusalStands class was removed.
    • In workflow.test.mjs, the call-order assertion is now if (check > 0) assert.match(run.calls[0], SETTLE(...)) plus the exact calls.length. That still pins the order for the cases kept.
  • Docs and fragments. The README sentence and the step comment now describe the remaining behaviour. The source-control fragment is patch / Removed, which suits the removal of an internal subcommand. The pr-pipeline fragment is bump: none. No plugin.json or marketplace.json changed, so there are no manifest lines.

Notes

  • Not independently verified. The pytest, node --test and ruff results in the PR description are the author's claims; I could not run them in this lane.
  • Spend. The change spends no new prepaid credit. It removes about 9 GitHub API calls per wake whose last report refused, by the author's count.
  • Closing issues. The PR only refs pr-pipeline: wake pr-merge per PR on pr-verify completion and make the cron a backstop #12445 and closes no issue, so I had no acceptance criteria to check.
  • Embedded instructions. I found no instructions to the review lane in the PR context.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

The pr-verify verifier failed head ac52bc12ff84f68de60fa138b9e7ceac3eb517ec. What it wrote, as data:

Summary: Refs #12445, so this PR is judged as a partial delivery. The part it delivers is in scope
  and correct. The issue's AFTER v5 comment set a 24 h net-calls gate on #13107's unmoved-refusal
  skip: revert it if probe-only wakes x 9 > skipped wakes x 64. This PR reverts exactly that skip.
  It removes the `refusal-stands` probe, its output, its notice and its outcome text from
  pr-merge.yml. It removes `_refusal_stands`, `content_refusal`, `refusal_moved` and
  `CONTENT_REFUSALS`. It drops the `running` option, whose only caller was `refusal_moved`. It keeps
  #13107's Change B and #13241's `judgedRuns`, with a replacement IdleRefusal test that covers the
  `judged=` wiring. The workflow tests, Python tests, docs and change fragments are updated to
  match. Nothing in the diff weakens a guard: the arm step still needs `stands`. Every CI job on the
  judged head concluded success, including test-node
  (https://github.com/melodic-software/claude-code-plugins/actions/runs/41981982121/job/127094113260,
  workflow.test.mjs) and test-python (0)/(1)
  (https://github.com/melodic-software/claude-code-plugins/actions/runs/41981982121/job/127094113161).
  It still fails on one rule. A `refs` PR's body must name what remains of the issue for later work,
  and this body names only the delivered part and the commits it keeps. It says nothing about what
  remains of #12445, such as done-when 4's 24 h calls-per-merged-PR measure.
- PR #13453 body: this PR uses `Refs: #12445` but never says what remains of #12445. Add a line
  naming the open work, such as the amended done-when 4: pr-merge github-token calls per merged PR
  at or below 56.5 over the 24 h window, with no breaker trip that pr-merge's share explains, still
  to be measured and posted on the issue.

This pull request leaves the merge queue, and ci-status holds it out while it carries pr-verify-failed. A pr-verify pass removes the label, at a new commit or at this same head: once a trusted person edits the body, and only trusted people have edited it since the fail, this head is judged again, so a fail on the body or the issues it links needs only the body fixed, no new commit.

@github-actions github-actions Bot added pr-verify-failed pr-verify failed this head; a pass at a new commit removes it. and removed pr-verify-failed pr-verify failed this head; a pass at a new commit removes it. labels Nov 5, 2026
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 5, 2026
Merged via the queue into main with commit 5b550fa Nov 5, 2026
103 checks passed
@kyle-sexton
kyle-sexton deleted the revert/13107-wake-unmoved-skip branch November 5, 2026 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant