Skip to content

fix(source-control): hold a CLEAN babysit head that is behind a loose base - #6165

Merged
cursor[bot] merged 4 commits into
mainfrom
cursor/babysit-behind-base-e44b
Oct 4, 2026
Merged

cursor[bot] merged 4 commits into
mainfrom
cursor/babysit-behind-base-e44b

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Under loose required status checks GitHub reports a head that is behind its base as CLEAN. The babysit merge gate made no base compare, and the queue snapshot compared only a BLOCKED head, so the gate could squash-merge a behind head and drop base commits, even though freshness.md calls behind a hard stop.

Fix

  • When the merge gate runs on a PR that is otherwise ready (or held only by running checks), it compares the head against the live base. It holds the PR if the head is behind or the compare cannot be read, and reports baseFreshness. This happens at gate time, including when the gate arms --auto; an already-armed auto-merge is not re-checked if the base moves later, a race that predates this PR.
  • The gate makes no compare on a base whose rulesets require a merge queue or up-to-date branches. Up-to-date means a strict required_status_checks rule that lists at least one check, since GitHub's strict setting "will not take effect unless at least one status check is enabled" (rules API). Classic branch protection is not read; such a base pays one extra compare, never a wrong merge.
  • The queue snapshot compares every BLOCKED, CLEAN or HAS_HOOKS PR on every cycle, and reports a behind CLEAN/HAS_HOOKS head as branch_freshness.state == "behind" so the guarded refresh clears the hold.
    • For that head it also reads the base's rules, and flags it behind only when the read succeeds and shows no merge queue.
    • On a failed read the head stays not behind for that cycle, so a transient failure cannot start a refresh on a queue base (a refresh disarms auto-merge and reruns CI and the AI reviews). The gate still compares and holds, and the next snapshot re-reads the rules.
    • A compare that keeps failing on a loose base holds until a human acts.
  • The review-request candidate skips a head the snapshot reports behind, matching the live re-check in request_review.py.
  • Merge-queue base: unchanged. A queue tests the PR against the latest base itself (managing a merge queue). This repository's main has a queue, so the gate's behaviour here does not change.
  • Docs: reference/freshness.md (with a dated source record) and reference/safety.md describe the hold, its timing and its cost. source-control goes to 0.79.8 with a CHANGELOG entry.

Verification

  • Babysit pytest suite: 927 passed, 370 subtests passed. The new tests fail on the old code: a CLEAN/HAS_HOOKS head behind a loose base is not held, auto-merge arms over a behind head, the snapshot does not report a behind CLEAN head, a strict rule with no checks skips the compare, a failed rules read marks the head behind, and the review-request candidate picks a behind head.
  • Unchanged cases are pinned: an up-to-date head is ready; a queue base or a ruleset-strict base makes no gate compare; BLOCKED keeps its fallback; a PR already held makes no compare.
  • engine.test.sh: 11 PASS, 0 FAIL. The other affected suites (babysit-wrapper-help, nesting-invariant-ssot, check-contract-clause-coverage) pass.
  • run-ruff.sh check, typos, markdownlint-cli2, gitleaks and all four changelog-parity checks pass.
  • An independent verifier checked every fix(source-control): babysit gate does not hold a CLEAN head that is behind a non-strict base #5955 criterion, traced that a held head is always refreshable (no wedge), confirmed the ruleset field and compare semantics against the GitHub REST schema and a live read of this repo's rules, and checked this repo's own main is unaffected.

Related

Closes #5955
Builds on #5935 and #6148 (merge-queue handling).

Open in Web Open in Cursor 

cursoragent and others added 3 commits October 4, 2026 03:30
… base

Under loose required status checks GitHub reports a behind head CLEAN, and
the gate compared the head against its base only on BLOCKED, so it could
squash-merge a behind head. The merge gate now compares an otherwise-ready
head against the live base on a base with neither strict checks nor a merge
queue, and holds it while behind or unreadable. The snapshot reports the same
head behind so the guarded refresh can clear the hold; a queue base is left
as before.

Closes #5955

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…-base-e44b

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…ds and docs

Count a strict required_status_checks rule only when it lists a check,
since GitHub's strict setting takes no effect without one. Leave the
merge-queue flag unknown on a failed rules read so the snapshot reports
a CLEAN head not behind for that cycle instead of refreshing a possible
queue base. Skip a behind head as a review-request candidate. State that
the gate checks freshness at gate and arm time only, reads rulesets but
not classic branch protection, and holds on a persistently failing
compare until a human acts; correct the cost wording and a broken reflow.

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 4, 2026 03:53
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 4, 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-10-04T03:58:44.654977Z 977a7ba 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.

@claude

claude Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 6m 32s —— View job


CI code review — /review:code-review (PR #6165)

  • Gather context (diff, changed files, PR body)
  • Understand request (/review:code-review lane: correctness + maintainability, security out-of-scope per the dedicated security lane)
  • Review changed files for correctness, maintainability, test coverage
  • Post findings

Summary

Reviewed the full diff (gh pr diff 6165) against the skill's criteria: correctness, CLAUDE.md/REVIEW.md alignment, architecture, error handling, and test coverage, scoped to the 18 changed files.

No blocking or high-confidence findings. This is a careful, well-contained change:

  • The core logic split — babysit_gh.view_pr enriching _base_compare/_base_merge_queue for the snapshot/delta/review-request paths, versus babysit_merge.evaluate doing its own independent fetch_base_compare call gated on not merge_queue_required and not rules.get("requireUpToDate") — is internally consistent and matches the module docstrings.
  • I traced the waiting/blockers interaction in babysit_merge.py (plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py#L1466-L1486) against the new tests in test_babysit_merge_base_freshness.py: the freshness hold fires only when the PR is otherwise ready or held solely by running checks, is excluded from waiting (so --auto cannot arm over a behind head), and fails closed on an unreadable compare.
  • branch_rules's new requireUpToDate fold (babysit_merge.py#L389-L446) correctly scopes strict_required_status_checks_policy per-ruleset and requires at least one context, matching the cited GitHub rules-API behavior and its own test (test_a_strict_rule_needs_a_required_check_of_its_own).
  • I checked every test file that calls merge.evaluate(...) for a gh_json stub update (grepped merge\.evaluate\( across tests/): all 5 matching files received the UP_TO_DATE compare stub consistently with the diff, so none is left calling into an unstubbed /compare/ path.
  • The one nuance I looked at closely — a stacked-PR layer's freshness compare running against its literal base_ref while requireUpToDate/mergeQueueRequired are read from the stack's trunk rules — is consistent with how the file already treats trunk rules as governing a stack layer elsewhere (e.g. the unprotected-base hold), not a new inconsistency introduced here.
  • Renames (_blocked_base_compare → _base_compare, fetch_blocked_base_compare → fetch_base_compare) are applied completely, including the one comment reference in babysit_resolve_thread.py.

Verification note: this sandbox's permission mode denied approval for running python3 -m pytest / scripts/run-ruff.sh here, so I could not independently execute the suite or linter — the PR body's "927 passed, 370 subtests" and ruff/typos/markdownlint/gitleaks results are author-claimed and unverified by me. My review is based on static reading of the diff and cross-referencing test fixtures against the production code paths they exercise.

No inline comments posted — nothing met the high-signal bar for a blocking or flaggable finding.

@claude

claude Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 55s —— View job


Security review (/review:security-review)

Scope: all 18 changed files (diffed via gh pr diff 6165, shallow checkout had no local merge-base with origin/main).

Result: no security findings.

This PR tightens a correctness/data-integrity guard in the babysit merge gate (holding a CLEAN/HAS_HOOKS head that is behind its base) rather than loosening one, so it nets out as a safety improvement, not a new exposure.

What I checked and why it's clean:

  • Injection surfaces. fetch_base_compare and base_requires_merge_queue in babysit_gh.py build gh argument lists (no shell), percent-encode base_ref with quote(..., safe=''), and regex-validate head_sha ([0-9a-fA-F]{7,40}) before use — consistent with the pre-existing pattern this function was renamed from (fetch_blocked_base_compare).
  • Fail-closed error handling. Both the base compare and the new merge-queue rules read (base_requires_merge_queue, babysit_gh.py#L351-L371) return None/fail on any RuntimeError/json.JSONDecodeError, and every caller treats an unreadable answer as "not proven" — the gate holds (babysit_merge.py#L1468-L1481) and the snapshot stays not_reported_behind (babysit_delta.py#L219-L227) rather than defaulting to "safe to merge/refresh" on ambiguous data.
  • No auto-merge bypass. The new freshness blocker is excluded from waiting, so it correctly lands in auto_blockers (babysit_merge.py#L1535) and prevents --auto from arming over a behind head, matching test_auto_merge_is_not_armed_over_a_behind_head.
  • No new trust-boundary input. base_ref/head_sha/rules come from GitHub's own API responses (baseRefName, headRefOid, rules/branches), not from PR-author-controlled free text, and no new field here is rendered or interpolated into a shell string.
  • Instruction-surface lens. docs/conventions/instruction-exception-register/README.md is present in this checkout. The freshness.md/safety.md edits strengthen the stale-base guidance (the gate now says it does add its own hold, where before it said it added none) — nothing matching a Gate 0 class (irreversible-action, secret-handling, data-integrity, external-publication, legal-compliance, agent-authority) is deleted, narrowed, or softened.
  • GitHub Actions hardening (zizmor's lane) doesn't apply: no workflow files changed.

No CRITICAL / IMPORTANT / SUGGESTION items to report.

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

claude-security-review has reviewed this pull request through 977a7ba; a later push is reviewed from there.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 977a7ba11d

ℹ️ 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".

Comment thread plugins/source-control/skills/babysit-prs/reference/freshness.md Outdated
@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

claude-review has reviewed this pull request through 977a7ba; a later push is reviewed from there.

…tream sections

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
@cursor
cursor Bot added this pull request to the merge queue Oct 4, 2026
Merged via the queue into main with commit 296f162 Oct 4, 2026
21 checks passed
@cursor
cursor Bot deleted the cursor/babysit-behind-base-e44b branch October 4, 2026 05:24
@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Babysit triage of the two bot review summaries on head b51ed9b7. Neither reports a finding, so no code change follows.

Source Severity token Classification Evidence
claude[bot] security review, comment 5976319270 CRITICAL INCORRECT: negated, not a finding The comment reads "No CRITICAL / IMPORTANT / SUGGESTION items to report" and "Result: no security findings."
claude[bot] security review, comment 5976319270 IMPORTANT INCORRECT: negated, not a finding Same sentence as above.
claude[bot] security review, comment 5976319270 SUGGESTION INCORRECT: negated, not a finding Same sentence as above.
claude[bot] code review, comment 5976319226 (none) VALID: non-blocking verdict, no action The comment reads "No blocking or high-confidence findings" and posted no inline comments.

Both reviews ran before b51ed9b7. The security review cites 977a7ba1. b51ed9b7 changes only docs (freshness.md, safety.md, CHANGELOG.md), so neither review covers that commit.

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.

fix(source-control): babysit gate does not hold a CLEAN head that is behind a non-strict base

2 participants