fix(gh): allow protected auto-merge requests

[steipete]

Problem

Protected gh pr merge --auto requests are rejected before GitHub CLI starts, preventing maintainer workflows from handing required-check waiting to GitHub.

Change

Allow only numeric, exact-head squash auto-merge requests with an explicit nonempty subject and body file/stdin. Both fields pass the active rewrite policy; the native child receives sanitized text and a private immutable body snapshot with empty stdin. Immediate merges retain their existing REST path. Admin, non-squash, unpinned, ambiguous, and implicit-publication requests remain blocked.

The head check is a submission-time precondition; GitHub owns later auto-merge/queue behavior. Existing opt-in diagnostics distinguish a local no-child refusal from an uncertain child outcome, without changing native exit codes or weakening publication protection.

Validation

• New regression fails against the original guard and passes with the change.
• Focused merge, policy-transition, snapshot, and diagnostic tests: 2.579 s package / 6.52 s wall.
• Full go test ./...: passed, 153.93 s wall; go vet ./... passed.
• Formatting and diff checks passed; isolated independent review found no actionable P0–P2 findings.

Related: openclaw/openclaw#154673.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed September 21, 2026, 11:55 AM ET / 15:55 UTC.

ClawSweeper review

What this changes

Allows protected squash auto-merge requests through native GitHub CLI using an explicit head SHA, rewritten subject, and private body snapshot, with diagnostic tests and documentation.

Merge readiness

⛔ Blocked before merge - 5 items remain

The capability remains absent from main, but the new queue path bypasses the promised commit-text protection. Real behavior proof is also required before merge.

Priority: P2
Reviewed head: 404e4ae11bd30900d98c75c4d15f8e3091410822

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	Focused implementation and substantial regression coverage are limited by a concrete publication-boundary defect and simulated-only proof.
Proof confidence	🦪 silver shellfish (2/6)	Needs real behavior proof before merge: Authority-chain proof required: the changed lifecycle helper is exercised only through a fake gh child. Provide redacted real Octopool/native-gh evidence showing sanitized deferred publication and rejection before mutation of a queue case that would substitute unchecked metadata. Terminal output or logs are appropriate; screenshots or recordings also count when they show the relevant diagnostics. 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	🦐 gold shrimp (3/6)	Security review found an item that needs attention.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: Authority-chain proof required: the changed lifecycle helper is exercised only through a fake gh child. Provide redacted real Octopool/native-gh evidence showing sanitized deferred publication and rejection before mutation of a queue case that would substitute unchecked metadata. Terminal output or logs are appropriate; screenshots or recordings also count when they show the relevant diagnostics. 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	7 items	Pinned introduced change: Read the complete introduced diff across five files. The verified test merge has the pinned main and PR head as its ordered parents and contains the same five-file change. Still necessary on main and released baseline: Main rejects --auto in the protected lifecycle parser and explicitly uses immediate REST merges to avoid queue reinterpretation. The fetched main revision is tagged v0.7.0. The merged REST-defaults work at #177 does not implement protected native auto-merge. Native dependency boundary: The introduced helper constructs native gh pr merge --auto arguments; gh_fallback.go executes the resulting command with caller credentials. Its safety therefore depends directly on GitHub CLI and GitHub auto-merge semantics. The documentation acknowledges queue-controlled metadata, but no independent maintainer acceptance of that protection exception is recorded.
Findings	1 actionable finding	[P1] Block queue merges that discard the sanitized commit text
Security	Needs attention	Queue-generated metadata escapes rewrite protection: The new native auto-merge delegation allows GitHub to ignore the checked subject/body and choose final metadata, potentially publishing text covered by active protection rules.

How this fits together

Octopool wraps GitHub CLI and checks outgoing publication text against rewrite rules before dispatching writes using the caller’s credentials. This change delegates eligible merge requests to GitHub’s immediate or deferred merge lifecycle.

flowchart TD
  A[Merge request and text] --> B[Final rewrite policy]
  B --> C[Validate and sanitize]
  C --> D[Private body snapshot]
  D --> E[Native GitHub CLI]
  E --> F[Immediate or deferred merge]
  F --> G[Published commit metadata]

Loading

Before merge

• Add real behavior proof - Needs real behavior proof before merge: Authority-chain proof required: the changed lifecycle helper is exercised only through a fake gh child. Provide redacted real Octopool/native-gh evidence showing sanitized deferred publication and rejection before mutation of a queue case that would substitute unchecked metadata. Terminal output or logs are appropriate; screenshots or recordings also count when they show the relevant diagnostics. 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.
• Block queue merges that discard the sanitized commit text (P1) - When the target branch requires a merge queue, these arguments authorize a native auto-merge whose final metadata is outside the sanitizer. GitHub’s EnablePullRequestAutoMergeInput contract explicitly ignores commitHeadline, commitBody, and mergeMethod for queues, so unchecked remote text can become the published commit message despite the required sanitized snapshot. Reject that route before enabling it, or enforce protection at final publication; the documentation caveat and fake-child assertions do not prevent the bypass.
• Resolve security concern: Queue-generated metadata escapes rewrite protection - The new native auto-merge delegation allows GitHub to ignore the checked subject/body and choose final metadata, potentially publishing text covered by active protection rules.
• Resolve merge risk (P1) - A queue-required branch can ignore the sanitized subject/body and publish queue-generated metadata outside the active rewrite checks; documentation of that limitation does not resolve the protection bypass.
• Complete next step (P2) - Repair the queue publication bypass and supply real native-gh after-fix evidence before merge.

Findings

• [P1] Block queue merges that discard the sanitized commit text — cmd/octopool/string_rewrites_pr.go:141-146
• [high] Queue-generated metadata escapes rewrite protection — cmd/octopool/string_rewrites_pr.go:145

Agent review details

Security

Needs attention: Deferred queue publication can bypass the active commit-text rewrite boundary; no unrelated supply-chain changes were found.

Review metrics

Metric	Value	Why it matters
Production and test delta	Production +48/-1; tests +151/-18; docs and changelog +36/-3	The added production path is justified by deferred merge support, while most growth covers argument and policy handling.

Merge-risk options

Maintainer options:

1. Preserve protection across deferred merges (recommended)
   Exclude queue-controlled publication before enabling auto-merge, or implement an enforceable final-publication check, and prove that unchecked metadata cannot be published.
2. Retain the existing refusal
   Pause the new protected auto-merge path if GitHub’s deferred lifecycle cannot preserve the required publication boundary.

Technical review

Best possible solution:

Support deferred merges only where sanitized commit metadata remains authoritative, retaining refusal for queue behavior that cannot uphold publication protection.

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

Yes, at source level: main explicitly rejects protected --auto requests, and the proposed queue route passes sanitized fields that GitHub’s documented API ignores. No live merge was executed during this read-only review.

Is this the best way to solve the issue?

No. Reusing the existing sanitizer and snapshots is appropriate, but native queue delegation does not preserve their effect on published metadata.

Full review comments:

• [P1] Block queue merges that discard the sanitized commit text — cmd/octopool/string_rewrites_pr.go:141-146
  When the target branch requires a merge queue, these arguments authorize a native auto-merge whose final metadata is outside the sanitizer. GitHub’s EnablePullRequestAutoMergeInput contract explicitly ignores commitHeadline, commitBody, and mergeMethod for queues, so unchecked remote text can become the published commit message despite the required sanitized snapshot. Reject that route before enabling it, or enforce protection at final publication; the documentation caveat and fake-child assertions do not prevent the bypass.
  Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.97

AGENTS.md: found, but no applicable review policy affected this item.

Codex review notes: model internal, reasoning medium; reviewed against 59aaed9fef09.

Labels

Label changes:

• add P2: This adds a bounded maintainer workflow capability; no current production outage is established.
• add merge-risk: 🚨 security-boundary: The newly allowed native queue path can publish metadata that bypasses active rewrite protection.
• add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Authority-chain proof required: the changed lifecycle helper is exercised only through a fake gh child. Provide redacted real Octopool/native-gh evidence showing sanitized deferred publication and rejection before mutation of a queue case that would substitute unchecked metadata. Terminal output or logs are appropriate; screenshots or recordings also count when they show the relevant diagnostics. 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: This adds a bounded maintainer workflow capability; no current production outage is established.
• merge-risk: 🚨 security-boundary: The newly allowed native queue path can publish metadata that bypasses active rewrite protection.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Authority-chain proof required: the changed lifecycle helper is exercised only through a fake gh child. Provide redacted real Octopool/native-gh evidence showing sanitized deferred publication and rejection before mutation of a queue case that would substitute unchecked metadata. Terminal output or logs are appropriate; screenshots or recordings also count when they show the relevant diagnostics. 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

Security concerns:

• [high] Queue-generated metadata escapes rewrite protection — cmd/octopool/string_rewrites_pr.go:145
  The new native auto-merge delegation allows GitHub to ignore the checked subject/body and choose final metadata, potentially publishing text covered by active protection rules.
  Confidence: 0.98

What I checked:

• Pinned introduced change: Read the complete introduced diff across five files. The verified test merge has the pinned main and PR head as its ordered parents and contains the same five-file change. (cmd/octopool/string_rewrites_pr.go:63, 404e4ae11bd3)
• Still necessary on main and released baseline: Main rejects --auto in the protected lifecycle parser and explicitly uses immediate REST merges to avoid queue reinterpretation. The fetched main revision is tagged v0.7.0. The merged REST-defaults work at fix(rewrites): accept merge defaults and explain invalid payloads #177 does not implement protected native auto-merge. (cmd/octopool/string_rewrites_pr.go:24, 59aaed9fef09)
• Native dependency boundary: The introduced helper constructs native gh pr merge --auto arguments; gh_fallback.go executes the resulting command with caller credentials. Its safety therefore depends directly on GitHub CLI and GitHub auto-merge semantics. The documentation acknowledges queue-controlled metadata, but no independent maintainer acceptance of that protection exception is recorded. (cmd/octopool/string_rewrites_pr.go:143, 404e4ae11bd3)
• Native auto-merge dispatch: GitHub CLI forwards explicit headline, body, and expected head to enablePullRequestAutoMerge. Its merge.go queue path forces auto-merge and warns that the queue chooses the merge strategy. (pkg/cmd/pr/merge/http.go:88, 0cf1092493af)
• Queue ignores sanitized metadata: The authoritative GitHub.com schema states that merge queues ignore commitBody, commitHeadline, and mergeMethod inputs to EnablePullRequestAutoMergeInput. Sanitizing those inputs therefore cannot protect the eventual queue-generated metadata. (src/graphql/data/fpt/schema.docs.graphql:14068, 95cd2ed6c6fc)
• Proof coverage: The complete supplied body matches the live body at the pinned head and reports tests, vet, formatting, and review only. New auto-merge tests use captureRewriteGH, which replaces gh with TestRewriteCaptureProcess; they establish arguments, snapshots, cleanup, and exit handling, but never exercise GitHub’s merge lifecycle. Discussion contains only review-status bot comments. (cmd/octopool/string_rewrites_process_test.go:182, 404e4ae11bd3)

Likely related people:

• unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)

Rank-up moves

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

• Preserve the rewrite guarantee for queue-required branches before enabling deferred merge authority.
• Add real native-gh evidence of sanitized eventual commit metadata and pre-mutation rejection of the unsafe queue case; redact credentials, private endpoints, IP addresses, and other private details.
• Update the PR body with that evidence to trigger a fresh review; if needed, 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.