Add changelog fragment workflow#3609
Conversation
Let each pull request contribute a validated changelog fragment without waiting for a pull request number. Add maintenance commands for repeatable consolidation, release promotion, and post-release reconciliation while preserving existing history.
📝 WalkthroughWalkthroughThis PR adds fragment-based changelog tooling with validation, consolidation, release, reconciliation, and pull-request checks. It updates CI, pre-commit, ownership, contributor guidance, release workflows, audit procedures, and integration tests to use the new model. ChangesChangelog fragment workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Contributor
participant PRWorkflow
participant ChangelogCLI
participant CHANGELOG
Contributor->>PRWorkflow: Submit fragment or skip file
PRWorkflow->>ChangelogCLI: Run check or validate
ChangelogCLI->>CHANGELOG: Parse Unreleased and release marker
ChangelogCLI-->>PRWorkflow: Validation result
ChangelogCLI->>CHANGELOG: Build, release, or reconcile changelog
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Reuse the repository's existing release-management label for authorized changelog consolidation, release promotion, and reconciliation instead of introducing a dedicated label.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 @.claude/skills/release-audit/SKILL.md:
- Line 6: Replace the wildcard rm entries in the release-audit skill’s
allowed-tools list with fixed report filenames or a dedicated cleanup helper
that validates exact paths before deletion. Preserve cleanup for the three
report types while preventing deletion of arbitrary matching files in shared
/tmp.
In @.claude/skills/release-changelog/SKILL.md:
- Around line 108-109: Update the changelog-indicator search command in the
release changelog instructions to use ripgrep’s case-insensitive flag, changing
`rg -n` to `rg -ni` while preserving the existing patterns and file targets.
In @.github/workflows/pr.yml:
- Around line 26-29: Update the actions/checkout step in the workflow to set
persist-credentials to false alongside fetch-depth, ensuring the checkout token
is not retained in .git/config when scripts/changelog.py runs.
In `@AGENTS.md`:
- Around line 59-60: Replace the inline “- ” wording with “a hyphen followed by
a space” at AGENTS.md lines 59-60, changelog.d/README.md lines 29-30, and
changelog.d/README.md lines 47-48; make no other changes.
In `@docs/guide/release.rst`:
- Around line 252-261: Update the release checklist entries for the changelog.py
release and reconcile workflows to require running each command with --dry-run
first, reviewing and approving the preview, then rerunning the same command
without --dry-run. Apply this consistently to the release instructions around
the release-X.Y promotion and the corresponding reconciliation entry.
🪄 Autofix (Beta)
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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 65a4b68c-15da-49bd-9f4e-31b64c69d1ec
📒 Files selected for processing (18)
.claude/skills/release-audit/SKILL.md.claude/skills/release-audit/references/classification-rules.md.claude/skills/release-audit/references/language-review-examples.md.claude/skills/release-changelog/SKILL.md.claude/skills/release-notes/SKILL.md.coderabbit.yml.github/CODEOWNERS.github/PULL_REQUEST_TEMPLATE.md.github/workflows/pr.yml.pre-commit-config.yamlAGENTS.mdCHANGELOG.mdchangelog.d/README.mdchangelog.d/changelog-fragment-workflow-3956bf60.skipdocs/guide/development.rstdocs/guide/release.rstnewton/tests/test_changelog.pyscripts/changelog.py
| disable-model-invocation: true | ||
| argument-hint: "[target-version]" | ||
| allowed-tools: Bash(git log *) Bash(git show *) Bash(git grep *) Bash(git tag *) Bash(git rev-parse *) Bash(git diff *) Bash(python3 ${CLAUDE_SKILL_DIR}/scripts/list_commits.py *) Bash(python3 ${CLAUDE_SKILL_DIR}/scripts/license_audit.py *) Bash(rm /tmp/newton-*-prerelease-report.md) Bash(rm /tmp/newton-*-rc-report.md) Bash(rm /tmp/newton-*-retrospective-report.md) Bash(gh --version) Bash(gh auth status) Bash(gh gist create *) Bash(gh gist list *) Bash(gh gist view *) Bash(gh gist edit *) Bash(gh issue view *) Bash(gh issue list *) Read Write Grep Glob | ||
| allowed-tools: Bash(git log *) Bash(git show *) Bash(git grep *) Bash(git tag *) Bash(git rev-parse *) Bash(git diff *) Bash(git ls-tree *) Bash(uv run --no-project python scripts/changelog.py *) Bash(python3 ${CLAUDE_SKILL_DIR}/scripts/list_commits.py *) Bash(python3 ${CLAUDE_SKILL_DIR}/scripts/license_audit.py *) Bash(rm /tmp/newton-*-prerelease-report.md) Bash(rm /tmp/newton-*-rc-report.md) Bash(rm /tmp/newton-*-retrospective-report.md) Bash(gh --version) Bash(gh auth status) Bash(gh gist create *) Bash(gh gist list *) Bash(gh gist view *) Bash(gh gist edit *) Bash(gh issue view *) Bash(gh issue list *) Read Write Grep Glob |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid wildcard rm permissions in the skill allowlist.
These entries allow the model to delete any matching file in shared /tmp, not only reports created by this run. Replace them with fixed filenames or a dedicated cleanup helper that validates exact paths before deletion.
🧰 Tools
🪛 SkillSpector (2.3.11)
[error] 6: [TM1] Tool Parameter Abuse: Tool parameters are crafted to achieve unintended or unsafe behavior. Parameter abuse can bypass intended safety checks (e.g. shell=True, --force, dangerous glob patterns).
Remediation: Validate all tool parameters against an allowlist. Reject dangerous parameter values (shell=True, --force, -rf /) and use safe defaults.
(Tool Misuse (TM1))
[error] 6: [TM1] Tool Parameter Abuse: Tool parameters are crafted to achieve unintended or unsafe behavior. Parameter abuse can bypass intended safety checks (e.g. shell=True, --force, dangerous glob patterns).
Remediation: Validate all tool parameters against an allowlist. Reject dangerous parameter values (shell=True, --force, -rf /) and use safe defaults.
(Tool Misuse (TM1))
[error] 6: [TM1] Tool Parameter Abuse: Tool parameters are crafted to achieve unintended or unsafe behavior. Parameter abuse can bypass intended safety checks (e.g. shell=True, --force, dangerous glob patterns).
Remediation: Validate all tool parameters against an allowlist. Reject dangerous parameter values (shell=True, --force, -rf /) and use safe defaults.
(Tool Misuse (TM1))
🤖 Prompt for 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.
In @.claude/skills/release-audit/SKILL.md at line 6, Replace the wildcard rm
entries in the release-audit skill’s allowed-tools list with fixed report
filenames or a dedicated cleanup helper that validates exact paths before
deletion. Preserve cleanup for the three report types while preventing deletion
of arbitrary matching files in shared /tmp.
Source: Linters/SAST tools
| rg -n "removed|deprecated|in favor of|use .* instead|renam|replac|default|breaking" \ | ||
| CHANGELOG.md changelog.d |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the changelog-indicator search case-insensitive.
The current rg -n command will miss standard capitalized terms such as Removed, Deprecated, Default, Breaking, and Use. Change it to rg -ni so the documented check actually detects these entries.
🤖 Prompt for 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.
In @.claude/skills/release-changelog/SKILL.md around lines 108 - 109, Update the
changelog-indicator search command in the release changelog instructions to use
ripgrep’s case-insensitive flag, changing `rg -n` to `rg -ni` while preserving
the existing patterns and file targets.
| - name: Checkout repository | ||
| uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 | ||
| with: | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Disable checkout credential persistence.
actions/checkout leaves the token in .git/config by default. Because this job executes scripts/changelog.py from the checked-out pull request, malicious PR code could read and exfiltrate the token and any permissions it has. Set persist-credentials: false.
Proposed fix
- name: Checkout repository
uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
with:
fetch-depth: 0
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Checkout repository | |
| uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 | |
| with: | |
| fetch-depth: 0 | |
| - name: Checkout repository | |
| uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 | |
| with: | |
| fetch-depth: 0 | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 26-29: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for 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.
In @.github/workflows/pr.yml around lines 26 - 29, Update the actions/checkout
step in the workflow to set persist-credentials to false alongside fetch-depth,
ensuring the checkout token is not retained in .git/config when
scripts/changelog.py runs.
Source: Linters/SAST tools
| - A `.md` fragment may contain multiple `### Added`, `### Changed`, `### Deprecated`, `### Removed`, and `### Fixed` | ||
| sections in that exact order. Start every entry with `- `, end every entry with a period `.`, use imperative present tense ("Add X"), and avoid internal |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rephrase inline code spans containing trailing spaces.
The documented `- ` syntax triggers Markdownlint MD038. Describe it as “a hyphen followed by a space” instead.
AGENTS.md#L59-L60: replace the inline`- `wording.changelog.d/README.md#L29-L30: replace the inline`- `wording.changelog.d/README.md#L47-L48: replace the inline`- `wording.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 60-60: Spaces inside code span elements
(MD038, no-space-in-code)
📍 Affects 2 files
AGENTS.md#L59-L60(this comment)changelog.d/README.md#L29-L30changelog.d/README.md#L47-L48
🤖 Prompt for 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.
In `@AGENTS.md` around lines 59 - 60, Replace the inline “- ” wording with “a
hyphen followed by a space” at AGENTS.md lines 59-60, changelog.d/README.md
lines 29-30, and changelog.d/README.md lines 47-48; make no other changes.
Source: Linters/SAST tools
| - Review ``CHANGELOG.md``'s ``[Unreleased]`` section and pending files in | ||
| ``changelog.d/`` against the previous release tag and latest audit. | ||
| Promote the complete accumulator on ``release-X.Y`` with ``uv run | ||
| --no-project python scripts/changelog.py release --version X.Y.Z --date | ||
| YYYY-MM-DD``. Commit any editorial accumulator changes before running | ||
| the promotion so provenance is recoverable from Git history. Commit | ||
| ``CHANGELOG.md`` and any consumed fragment | ||
| deletions in a pull request labeled ``release-management`` before | ||
| preparing the final version. The ``release-changelog`` skill can assist | ||
| with the audit and promotion. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Require dry-run previews before release and reconciliation.
The companion release-changelog skill explicitly requires a dry-run followed by the mutating command. These checklist entries show only release and reconcile, so following them can consume fragments or remove accumulator entries before the result is reviewed. Add the corresponding --dry-run commands and require rerunning without the flag only after approval.
Also applies to: 336-342
🤖 Prompt for 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.
In `@docs/guide/release.rst` around lines 252 - 261, Update the release checklist
entries for the changelog.py release and reconcile workflows to require running
each command with --dry-run first, reviewing and approving the preview, then
rerunning the same command without --dry-run. Apply this consistently to the
release instructions around the release-X.Y promotion and the corresponding
reconciliation entry.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Is it a good idea to have a meta test that tests repo utilities placed in newton/tests, which is also shipped with the wheel?
There was a problem hiding this comment.
Probably not, but I at least wanted to test this fragment compilation somehow. Maybe a better way is to just move this under scripts/ or so? It definitely doesn't need to run like a regular unit test, I will adapt this tomorrow.
There was a problem hiding this comment.
Was there a reason why towncrier didn’t work as an existing solution?
There was a problem hiding this comment.
Yes, towncrier wants one file per category, like xyz.fixed, xyz.added, etc. That might work when xyz is the PR number but then this means you first create a PR and then need to push a second time to add these changelog fragments. Instead, I decided to follow Isaac Lab's approach and allow each PR to have a single changelog fragment with a name that doesn't have to be the PR number - the changelog.py script creates the file for you with a unique hex code appended. I thought if we had multiple files of this sort this would look very messy - it would only look neat if the file names had the PR number but that is not possible with a single-commit PR.
Other, less important, reasons are: we can consolidate the changelog at any time just using the changelog.py build command. We also introduce no additional dependency, which I think also makes sense for this rather trivial functionality.
There was a problem hiding this comment.
I think Towncrier can support the single-commit workflow after all. It supports orphan fragments, so contributors could hand-create files such as changelog.d/+camera-rays-a1b2c3d4.added.md before the PR exists. A multi-category PR would use the same random identifier with additional types, such as .deprecated.md or .fixed.md. Developers would not need to invoke Towncrier locally; CI would run towncrier check.
During a scheduled rollup or release, a bot could find the commit that introduced each orphan fragment, resolve that commit to its merged PR using GitHub’s associated-pull-requests API, rename the fragments in its temporary worktree to names such as 3609.added.md, and immediately run towncrier build. Because the build consumes the fragments, those intermediate renames never need a separate commit. If a commit cannot be mapped unambiguously, the fragment can remain orphaned or the rollup can request manual resolution.
This would preserve single-commit contributor PRs, produce PR-linked changelog entries, support multiple categories, and leave parsing and release generation to Towncrier. Towncrier would only be a pinned CI/release tool rather than a Newton runtime dependency. Would you be open to exploring this before we maintain the custom parser and reconciliation workflow?
There was a problem hiding this comment.
I still don't like PR-linked changelog entries. We should really move to issue-linked changelog entries.
There was a problem hiding this comment.
we can certainly make it a requirement that everything needing a changelog entry also deserves an issue.
There was a problem hiding this comment.
Hmm, the flexibility to have pull requests with changelog entries that don't have a linked issue still might be nice to preserve. If the fragment doesn't have an issue in the name then just don't create a link in the changelog.
There was a problem hiding this comment.
Yes, Towncrier supports exactly that distinction, so I think we can simplify my earlier proposal and drop the bot-based PR-number rewriting.
An issue-linked change would use a fragment such as changelog.d/3607.added.md, with issue_format configured to link #3607 to the corresponding GitHub issue. A change without an issue would use an orphan fragment such as changelog.d/+camera-rays-a1b2c3d4.added.md; Towncrier includes its content without adding a link.
We can configure issue_pattern = "\\d+" to require numeric identifiers for non-orphan fragments. Towncrier 24.8 and later explicitly exempt leading-+ orphan fragments from that check. Multiple categories use the same identifier, for example 3607.added.md and 3607.deprecated.md, or their orphan equivalents.
This preserves issue-linked release notes where appropriate while retaining flexibility for changes that do not need an issue. Developers can hand-create the fragments before opening the PR, CI can run towncrier check, and Towncrier can handle the eventual build. A small additional CI check would only be necessary if we want to verify that a numeric identifier refers to an existing issue rather than a pull request.
There was a problem hiding this comment.
towncrier check can probably be added to pre-commit, right? Ideally, towncrier won't require a uv.lock: Its config will be stored in pyproject.toml and its pin will be in .pre-commit-config.yaml (similar to our use of typos).
My understanding is that no OSRB request should be required in principle for towncrier usage since it doesn't interact with executable Newton code in the same process.
There was a problem hiding this comment.
We should double check but I think that's correct. Either way it's MIT licensed so should be fine?
There was a problem hiding this comment.
I think it would be better to follow one pattern for the fragment names if we decide to have a bot that renames these fragments (which may not be necessary, perhaps a bunch of fragments like changelog.d/+camera-rays-a1b2c3d4.added.md doesn't look as messy as I originally thought). So in that case it should be the PR number for the file name, not sometimes the issue number, sometimes the PR number.
There was a problem hiding this comment.
There might too much information placed in AGENTS.md about how to update the changelog. Why not use the fragment directory’s README.md as the source of truth and you could have one or two instructions here that instruct the agents to follow the process outlined in that file?
3 participants
Description
Implements the changelog fragment workflow discussed in #3607.
.mdor.skipfragment per normal pull request, without depending on a pull request number.CHANGELOG.mdas the durable[Unreleased]accumulator and retain all dated history.Checklist
changelog.d/*.mdfragment or justified.skip, or labeled thisrelease-managementTest plan
uv run --extra dev -m newton.tests -k test_changeloguvx --from pre-commit==4.2.0 pre-commit run -apython scripts/changelog.py check --base-ref origin/main --target-branch mainSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores