Default merge submission to direct - #407
Conversation
WalkthroughThe PR makes ChangesMerge submission policy
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review summaryWhat this PR does: Adds Correctness / Test coverage — bug foundLeft an inline comment on Since 75 of the 77 Design/security observation (not a bug, but worth surfacing)Making Other notes
Overall: the core production logic in |
…-default * origin/main: Fix <PROJECT> digit ambiguity, anchor section extraction, ship coordination gate (#339)
Review follow-up summaryReview scope: full available PR history (no previous address-review cutoff was present). Fixed
Decision retained
No action required
Current head: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/adoption.md`:
- Around line 95-109: Update the queue policy documentation to state that
queue-enabled repositories must select either merge_queue_only or
merge_queue_or_guarded_direct, since both use canonical enqueue on queue-enabled
bases. In docs/adoption.md lines 95-109, revise the current
merge_queue_only-only wording; make the corresponding requirement update in
docs/seam-design.md lines 175-179. Preserve the existing descriptions of guarded
direct exceptions and queue-disabled behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c405e351-7ea8-4f4e-842e-bf6e94e1eadd
📒 Files selected for processing (10)
.agents/agent-workflow.ymlCHANGELOG.mdbin/agent-workflow-seam-doctorbin/agent-workflow-seam-doctor-test.rbdocs/adoption.mddocs/seam-design.mdskills/pr-batch/SKILL.mdskills/pr-batch/bin/pr-merge-submitskills/pr-batch/bin/pr-merge-submit-test.rbworkflows/pr-processing.md
Review summaryReviewed the Overall: the core logic is sound and internally consistent — Two things worth a look before merge, posted as inline comments:
No security vulnerabilities (injection, auth bypass, etc.) or correctness bugs found in the core submission logic itself. |
Review summaryReviewed the diff introducing Overall: the implementation is careful and consistent with the existing security posture of this file:
One finding posted inline: the new No security or correctness issues found in the core merge-submission logic itself. |
Review follow-up summaryReview scope: feedback since the previous address-review summary at Fixed
No additional findings
Current head: |
Review summaryReviewed the change that adds a portable
One correctness finding left as an inline comment: Worth a maintainer gut-check (not a bug, called out explicitly in the PR description as an intended decision): this PR flips the portable default from fail-closed ( Docs ( |
Review follow-up summaryReview scope: final consolidated sweep after the previous summary at Fixed
Decision retained
Final review state
Current head: |
|
Review: Default merge submission to Overview Code quality / correctness
Security Test coverage Performance Summary |
Final review follow-up summaryReview scope: final-head feedback after the previous summary at Decision retained
Final review state
|
|
Completed-batch audit: replay evidence follows. |
|
Completed-batch audit: replay evidence follows. |
…8-batch-docs-status-skill * origin/main: Default merge submission to direct (#407) Fix <PROJECT> digit ambiguity, anchor section extraction, ship coordination gate (#339) Enforce requested-versus-observed model route provenance (#348) Fix locale-dependent test, surface policy-only CLAUDE.md follow-ups, add read-only seam-drift audit (#337) Add PR #377 changelog entry (#382) Make PR descriptions human-first (#377) Remove unsupported signed-launch enforcement (#374) # Conflicts: # bin/validate
…gn-implementation * origin/main: Harden coordination parsing after PR #339 audit (#410) Change merge submission mode to merge_queue_only (#412) Scope archive-readiness, add retry heartbeat and launch modes, ship the batch-status skill (#362) Default merge submission to direct (#407) Fix <PROJECT> digit ambiguity, anchor section extraction, ship coordination gate (#339)
…itable-controls * origin/main: Suppress unchanged Goal monitor wakes (#395) Document single-target plan-pr-batch routing (#405) Require exact-head hosted QA before merge (#291) Change merge mode from queue to direct (#415) Harden coordination parsing after PR #339 audit (#410) Change merge submission mode to merge_queue_only (#412) Scope archive-readiness, add retry heartbeat and launch modes, ship the batch-status skill (#362) Default merge submission to direct (#407) Fix <PROJECT> digit ambiguity, anchor section extraction, ship coordination gate (#339) Enforce requested-versus-observed model route provenance (#348) # Conflicts: # CHANGELOG.md
Why
Most consumer repositories do not use GitHub Merge Queue, but the portable merge-submission default currently assumes queue-only behavior. That makes an omitted seam reject normal direct merges and prevents consumers from explicitly declaring the intended non-queue path.
What changed
merge_submission.mode: directas a closed, validated policy mode.directthe portable default when the seam is omitted and the initializer default for new consumers.mergePullRequestmutation only when live queue control is disabled.merge_queue_onlyandmerge_queue_or_guarded_directbehavior, including explicit queue fixtures for queue-specific tests.Impact
After this prerequisite is released, consumer repositories can opt out of Merge Queue in
.agents/agent-workflow.ymlwith:This PR does not change any GitHub repository setting and does not enable or disable Merge Queue.
Validation
bin/validateate75dd6126fbc810fc93c893d6d90737e9dde7f85— passed with exit code 0, includingPASS agent-workflows validationruby skills/pr-batch/bin/pr-merge-submit-test.rb --verbose— 97 runs, 807 assertions, 0 failures/errors/skipsruby -c skills/pr-batch/bin/pr-merge-submit— syntax OKgit diff --check origin/main...HEAD— cleanAgent details
Decision log
directas the portable omitted-seam default while retaining explicitmerge_queue_onlyand guarded-direct modes.SOURCE_REPO_POLICYsentinel as Must-Fix. Commit5e1af5eerestores the real-repository trusted-policy path and makes 24 queue-specific fixture call sites explicit.Review outcomes
5e1af5ee; focused and full validation passed; review thread resolved.ghfixture: removed in5e1af5ee.b73663c7; both affected docs now describe both canonical-enqueue modes; review thread resolved.e75dd612; both overlapping threads resolved without a production change.QA Evidence
/root/pr407_closeout_audit, read-only independent checker; candidate worktree/Users/justin/.codex/worktrees/agent-workflows-merge-default; completed on exact headorigin/main...e75dd6126fbc810fc93c893d6d90737e9dde7f85diff, direct-default and queue-only routing, trusted-policy sentinel repair, direct error-path reconciliation coverage, queue-capable-mode documentation, changelog, and validation coveragee75dd6126fbc810fc93c893d6d90737e9dde7f85bin/validate; focused merge helper suite 97 runs/807 assertions; seam doctor suite 280 runs/2405 assertions; Ruby syntax;git diff --check5e1af5ee,b73663c7, ande75dd612Audit receipts
Completed-batch audit
Status: Clean — no outstanding findings or follow-ups. Durable receipt.