Correct the Release-Caller Snippet's Publish-on-Merge Claim - #1498
Conversation
Three surfaces describing the release caller stub said its merges publish. `publish-plan-task.yml` has said the opposite since it was written, in its own header comment and in the branch its `push` arm takes: a human merge that reaches the trigger still leaves the plan at `publish=false`, and every job the plan gates skips. The claim was already stale before #1476 retired the `publish-on-merge` release trigger, and that retirement leaves nothing behind it at all. ## What changed - `docs/reusable-workflows.md` "Adopting the Release Chain" no longer says a NuGet or PyPI repo's merges should auto-publish. It says the push trigger is not the release gate, names the task that is, and routes the gate's own conditions to `WORKFLOW.md` D4.1 rather than paraphrasing them. The paragraph's first sentence already states the trigger policy, so the lead-in below it no longer restates one. - `catalog/snippets/workflows/publish-release.yml` and its embedded copy in the same document drop the descriptor entirely. The file's own `on:` block sits two lines under it and is the authority a reader wants. - `catalog/snippets/workflows/README.md`'s row for that stub does the same. Nothing here states the gate's conditions in prose. A first attempt did, and every one of the four findings it drew was against that paraphrase: it dropped the actor allowlist and the code-affecting filter, it read as if a `workflow_dispatch` from any ref publishes where the task exits non-zero off `main` and `develop`, and it described the human-merge path without the warning the task deliberately raises on it. The rule has three correct statements already, so a fourth was the defect. The snippet and the document's embedded copy stay byte-identical, which an assertion in the edit checked rather than the eye. ## Verification `prose_lint.py`, `repo_gate.py` across eol, eol-coverage and sha-pin, `spec/validate.py`, `spec/audit.py --selftest`, `spec/workflow_reuse.py --selftest`, `build_dist.py --check`, `ruff format --check`, `ruff check`, `mypy`, the `unittest` suite at 1178 passed, and `docker_lint.py` over its seven linters. `canonical_review.py check` reports no carried canonical unit changed. A tree-wide sweep for the same claim in any other wording found none outside these three surfaces. Every other statement of the rule already reads "a human merge never auto-publishes". Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 33 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
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 |
|
@coderabbitai review |
|
There was a problem hiding this comment.
🟢 Approval recommended
The changes are documentation/comment-only and align the described behavior with the existing release gate logic without introducing inconsistencies.
Pull request overview
Updates the release-caller documentation and snippets to remove the stale claim that merges to the default branch auto-publish releases, aligning the guidance with publish-plan-task.yml's actual gate behavior and WORKFLOW.md D4.1.
Changes:
- Correct
docs/reusable-workflows.mdto clarify that apushtrigger is not the release gate, and route the publish decision topublish-plan-task.yml/WORKFLOW.mdD4.1. - Remove the “whose merges publish” descriptor from the
publish-release.ymlsnippet and its catalog README entry.
File summaries
| File | Description |
|---|---|
| docs/reusable-workflows.md | Fixes the “publish on merge” claim and points readers to the authoritative gate (publish-plan-task.yml) and WORKFLOW.md D4.1. |
| catalog/snippets/workflows/README.md | Updates the publish-release.yml table entry to remove the stale “merges publish” descriptor. |
| catalog/snippets/workflows/publish-release.yml | Updates the snippet header comment to remove the stale “merges publish” descriptor. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Three surfaces describing the release caller stub said its merges publish.
publish-plan-task.ymlhas said the opposite since it was written, in its own header comment and in the branch itspusharm takes: a human merge that reaches the trigger still leaves the plan atpublish=false, and every job the plan gates skips.The claim was already stale before #1476 retired the
publish-on-mergerelease trigger, and that retirement leaves nothing behind it at all. Raised by CodeRabbit as an outside-diff finding on thedevelop -> mainpromotion PR #1489, where it cannot be fixed directly, so it lands here first.What changed
docs/reusable-workflows.md"Adopting the Release Chain" no longer says a NuGet or PyPI repo's merges should auto-publish. It says the push trigger is not the release gate, names the task that is, and routes the gate's own conditions toWORKFLOW.mdD4.1 rather than paraphrasing them. The paragraph's first sentence already states the trigger policy, so the lead-in below it no longer restates one.catalog/snippets/workflows/publish-release.ymland its embedded copy in the same document drop the descriptor entirely. The file's ownon:block sits two lines under it and is the authority a reader wants.catalog/snippets/workflows/README.md's row for that stub does the same.Why nothing here states the gate's conditions in prose
A first attempt did, and every one of the four findings the local review pass drew was against that paraphrase. It dropped the actor allowlist and the code-affecting filter that
WORKFLOW.mdD4.1 andGOVERNANCE.mdboth keep. It read as if aworkflow_dispatchfrom any ref publishes, where the task exits non-zero offmainanddevelop. It described the human-merge path without the warning the task deliberately raises on it. And the replacement trigger descriptor identified the shape by the one trigger that never publishes for a human, contradicting its own paragraph two clauses earlier.The rule has three correct statements already, so a fourth was the defect. The second round deleted the paraphrase rather than narrowing it.
Verification
prose_lint.py,repo_gate.pyacross eol, eol-coverage and sha-pin,spec/validate.py,spec/audit.py --selftest,spec/workflow_reuse.py --selftest,build_dist.py --check,ruff format --check,ruff check,mypy, theunittestsuite at 1178 passed, anddocker_lint.pyover its seven linters.canonical_review.py checkreports no carried canonical unit changed, and alocal_review.pyreceipt covers this content.A tree-wide sweep for the same claim in any other wording found none outside these three surfaces. Every other statement of the rule already reads "a human merge never auto-publishes".
The snippet and the document's embedded copy stay byte-identical, which an assertion in the edit checked rather than the eye, and the review pass re-checked programmatically afterwards.
Two local strict-review passes ran, raising five findings. The four above shaped the result. The fifth is pre-existing and untouched by this diff, filed as #1497 rather than fixed here: the Docker delta list in the same section never says to remove the
push:block, so a Docker repo following it keeps a trigger that section's own policy forbids.🤖 Generated with Claude Code
https://claude.ai/code/session_01817rckKywjfoaEV5WF9TKs