Skip to content

fix(source-control): report a babysit merge GitHub queued as queued, and confirm it later - #6148

Merged
cursor[bot] merged 5 commits into
mainfrom
cursor/babysit-queue-auto-e44b
Oct 4, 2026
Merged

cursor[bot] merged 5 commits into
mainfrom
cursor/babysit-queue-auto-e44b

Conversation

@kyle-sexton

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

Copy link
Copy Markdown
Contributor

Summary

On a merge-queue base, gh pr merge adds the pull request to the queue (with or without --auto) and exits 0. When the gate's branch-rules read did not report the queue, babysit read that exit code as autoMergeEnabled: true (the --auto arm) or merged: true (the direct gh pr merge path). gh prints its "will be added to the merge queue" line only on a terminal, so the output text cannot be relied on. An enqueue was also treated as final, so no later run checked whether the queued PR merged or left the queue.

Fix

  • After every successful gh pr merge, the gate reads isInMergeQueue, isMergeQueueEnabled, mergeQueueEntry { state position } and autoMergeRequest back over GraphQL (REST exposes neither queue membership nor position).
    • In the queue: action: enqueue, enqueued: true, autoMergeEnabled: false, merged: false, mergeQueue { state, position }.
    • Armed to enter the queue: action: auto-merge, mergeQueue.entersWhenReady: true.
    • Queue base but neither queued, armed, nor merged: re-read up to 4 times at the 3s poll interval; still unseen, action: merge-pending, ready: false, mergeQueue.unconfirmed: true, exit 10, and recorded under --state-dir as an unconfirmed queue entry. A failed re-read never turns into a merge.
    • A base without a queue reports as before. A failed read also reports as before and adds merge.queueReadError.
  • With --state-dir, an enqueue from either the async API or gh pr merge is recorded with the vetted head. Every later run reads it first and sends no new merge:
    • still queued: merge-pending with the position
    • armed to enter the queue: merge-pending with mergeQueue.entersWhenReady
    • merged: the head is checked, then merged: true
    • an unconfirmed entry is confirmed by reading back as queued, armed, or merged; still unseen it holds as merge-pending, and the second later run that sees nothing reports it dequeued
    • left unmerged: dequeued: true, record cleared, and the next run gates it again (matching orchestration.md's "goes back through the gate")
    • unreadable: the record is kept and the run holds
  • safety.md, orchestration.md and SKILL.md document the new fields, with a dated claim record citing the merge-queue docs, the gh pr merge manual and source, and the GraphQL PullRequest object. One documented behavior changes: a later --state-dir run on an already-queued PR now holds as merge-pending (exit 10) instead of re-enqueueing and reporting enqueued.
  • source-control 0.79.7 (main took 0.79.6 while this PR was open).

Verification

Sources: Merging a pull request with a merge queue, gh pr merge, gh merge.go, PullRequest GraphQL object.

Related

Closes #5953
Follows #5935 (async merge and merge-queue enqueue).

Open in Web Open in Cursor 

…and confirm it later

gh pr merge routes every merge on a merge-queue base into the queue, --auto or
not, and exits 0, so a queue the branch-rules read missed was reported as
autoMergeEnabled or merged. The gate now reads isInMergeQueue and
mergeQueueEntry back after every successful gh pr merge and reports
action enqueue with the entry's state and position. With --state-dir an
enqueue from either path is recorded, and later runs report it still queued,
merged at the vetted head, or dequeued.

Closes #5953

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 4, 2026 02:07
@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-04T02:12:58.244764Z f5f3e7a 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 3m 18s —— View job


Security review — /review:security-review

  • Gather context (PR diff, changed files)
  • Understand the request (run /review:security-review per instructions)
  • Check skip gate (not skipped — security-relevant surface: merge-execution logic and state persisted across runs)
  • Review changed files for security-relevant issues
  • Post findings / summary

Scope. Reviewed the full diff (gh pr diff 6148) against this PR's security-relevant surface: plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py (the merge-queue read-back, the --state-dir queue-entry recording/confirmation, and the exit-code/reporting logic), plus the doc changes in safety.md, orchestration.md, and SKILL.md that describe the new behavior. No GitHub Actions workflow files are touched, so no zizmor-lane findings apply here.

No security issues found.

Specifically checked and found sound:

  • GraphQL injection. The new read_merge_queue query (MERGE_QUEUE_QUERY) is parameterized ($o, $r, $n passed via -F), not string-interpolated — no injection surface from repo/number.
  • TOCTOU / merge-pin invariant. The pre-existing --match-head-commit/--expected-head SHA pin on the actual gh pr merge call (L2764-2768) is untouched; the new queue read-back only happens after that call succeeds and only changes how the result is reported, so it doesn't widen what can be merged.
  • "Merged" can't be over-reported. check_queued_entry (L2048-L2076) routes a GitHub-reported merge back through the existing verify_request_landed head check before status: "merged" can stick; an unreadable or mismatched head keeps the record / escalates rather than confirming (covered by test_a_queue_merge_at_another_head_escalates). This preserves the existing security invariant that the gate never reports a merge it hasn't verified landed at the vetted head.
  • Privilege/tier gating. This diff only changes labeling of an outcome gh pr merge already produced (enqueue vs. auto-merge vs. merged); it doesn't touch the pre-existing tier checks that decide who may invoke --auto or a merge-capable run. If anything, correctly re-labeling a silent enqueue (previously misreported as autoMergeEnabled/merged) closes a reporting gap rather than opening one.
  • Instruction-surface deletions. The safety.md/orchestration.md edits replace one described behavior ("re-running the merge on a queued PR returns enqueued") with a stricter one (a later --state-dir run now holds as merge-pending instead of re-enqueueing) — checked against docs/conventions/instruction-exception-register/README.md's protected classes (irreversible-action, secret-handling, data-integrity, external-publication, legal-compliance, agent-authority); this is a tightening, not a removal of a guardrail, so no match.
  • State file writes (merge-requests.json under --state-dir) key on an operator-supplied repo/number, unchanged trust model from the existing pending-request machinery; no new untrusted-input path into update_pending/write_state.

@claude

claude Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 7m 26s —— View job


Code review complete

  • Gather context (PR diff, changed files)
  • Understand the request (CI code-review lane via /review:code-review)
  • Review babysit_merge.py changes
  • Review test changes
  • Review docs (safety.md, orchestration.md, SKILL.md, CHANGELOG)
  • Post findings

Summary

Reviewed the diff (git diff origin/main..HEAD, two-dot — this shallow checkout has no merge-base with origin/main) against the code-review lane's correctness/maintainability criteria, scoped to the 8 changed files.

Traced the new queue-handling logic end-to-end:

  • read_merge_queue / report_queue_routing for the gh pr merge fallback arm (both the direct and --auto arms), including the readError, inQueue, armed, and neither-queued-nor-armed branches.
  • check_queued_entry / queued_entry_hold for the --state-dir re-check path (queued, merged-and-verified, merged-but-unconfirmed, merged-at-another-head, dequeued, unreadable).
  • _record_pending / _queued_entry / _write_pending for recording an enqueue from either the async API or the gh pr merge fallback.

The logic holds up against the documented contract in safety.md/orchestration.md/SKILL.md, and the new tests in both test files exercise each branch (queued-with-position, armed-to-enter, neither, unreadable-read, merged-confirmed, merged-at-another-head, dequeued, unreadable-record). I did not find a correctness defect in the new code paths.

One minor doc/code mismatch, posted inline on babysit_merge.py (not blocking): check_queued_entry's docstring says an unreadable merge verification "keeps the record with status None," but the code sets status to "merged" before that early return — only the top readError branch actually leaves it None. queued_entry_hold keys off verified rather than status, so behavior is unaffected; it's a docstring-accuracy nit only.

Verification

I read the changed source and test files and the diff directly; I was not able to execute pytest or the ruff/markdownlint checks in this sandbox (those commands require approval that wasn't granted here), so I have not independently run the test suite. The PR body's "892 passed / 357 subtests", ruff, markdownlint, and check-changelog-parity.sh results are the author's own reported verification, not something I confirmed myself.

Not done / out of scope

  • Did not flag the pre-existing async_merge's direct_merge→enqueued status handling (unchanged by this diff) even though it has a similar-shaped gap to the one this PR fixes for gh pr merge — out of scope since those lines aren't touched here.
  • Security-relevant surface (none identified; this is read/merge orchestration against the GitHub API, no new untrusted input parsing) — left to the security lane per this repo's lane split.

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

claude-security-review has reviewed this pull request through f5f3e7a; 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: f5f3e7a9a5

ℹ️ 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/scripts/babysit_merge.py Outdated
Comment thread plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py Outdated
@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

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

cursoragent and others added 3 commits October 4, 2026 02:21
… not shown yet

A successful gh pr merge on a merge-queue base can read back as neither
queued, armed, nor merged for a moment. The gate now re-reads it a few times
at the async poll interval, then reports merge-pending with ready false and
mergeQueue.unconfirmed, and records it under --state-dir as an unconfirmed
queue entry. Later runs confirm it (queued, armed, merged) without sending
another merge, and the second later run that still sees nothing reports it
dequeued so the next run gates it again. Also fixes the queueing typo in a
test comment.

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…e entry

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…ed entry

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
@cursor
cursor Bot added this pull request to the merge queue Oct 4, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 4, 2026
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 af63181 Oct 4, 2026
38 of 39 checks passed
@cursor
cursor Bot deleted the cursor/babysit-queue-auto-e44b branch October 4, 2026 03:09
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 --auto on a merge-queue base reports auto-merge enabled, not queued

2 participants