-
Notifications
You must be signed in to change notification settings - Fork 27
fix(release): make release notes editorialization non-blocking #490
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary of ChangesHello @jdx, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the robustness and user experience of the release notes generation process. By refactoring the script into a Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Makes the “editorialized release notes” generation non-blocking and more CI-log friendly by moving it into a mise task, persisting outputs to files, and letting Claude output flow through normally.
Changes:
- Convert
gen-release-notesinto a mise task with structuredUSAGEargs and file outputs. - Update release workflow to use the mise task output files for title/notes.
- Make the release step non-blocking via
continue-on-error.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| tasks/gen-release-notes | Implements mise task interface and writes generated title/notes to an output directory. |
| docs/spec/reference/arg.md | Documents how variadic args are exposed and how to consume them in bash. |
| .github/workflows/publish-cli.yml | Switches release-note generation to the mise task and marks the step non-blocking. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.github/workflows/publish-cli.yml
Outdated
| run: npm install -g @anthropic-ai/claude-code | ||
| - name: Create draft release | ||
| if: ${{ startsWith(github.ref, 'refs/tags/v') }} | ||
| continue-on-error: true |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
continue-on-error: true is applied to the entire 'Create draft release' step, so any failure in that step (including failures unrelated to LLM editorialization, e.g., actually creating the draft release) will be ignored and the workflow can appear successful. To make only the editorialization non-blocking, remove step-level continue-on-error and instead make the LLM sub-command non-fatal (e.g., handle the failure within the script/if-block, which you already do) while keeping the overall release creation failing on real errors.
| continue-on-error: true |
| # First line is pithy title, rest is body | ||
| PITHY_TITLE="$(head -n1 /tmp/release-notes-full.txt)" | ||
| # Only concatenate if we got a real title (not just the version fallback) | ||
| OUTPUT_DIR=$(mktemp -d) |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mktemp -d creates a temporary directory that is never cleaned up. While runners are ephemeral, this is still easy to clean up and reduces clutter when iterating/debugging. Add a trap to rm -rf \"$OUTPUT_DIR\" (or explicitly remove it after copying the files).
| OUTPUT_DIR=$(mktemp -d) | |
| OUTPUT_DIR=$(mktemp -d) | |
| trap 'rm -rf "$OUTPUT_DIR"' EXIT |
tasks/gen-release-notes
Outdated
| echo "Error: No changes found for release" | ||
| exit 1 |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error messages are no longer written to stderr, and echo is used for file writes. For CLI/task tooling, it’s better to keep errors on stderr (so logs/consumers can distinguish failures cleanly) and prefer printf over echo for writing arbitrary content (to avoid edge cases like content equal to -n, escape handling differences, etc.).
tasks/gen-release-notes
Outdated
| echo "Error: Claude CLI failed" | ||
| exit 1 |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error messages are no longer written to stderr, and echo is used for file writes. For CLI/task tooling, it’s better to keep errors on stderr (so logs/consumers can distinguish failures cleanly) and prefer printf over echo for writing arbitrary content (to avoid edge cases like content equal to -n, escape handling differences, etc.).
tasks/gen-release-notes
Outdated
| echo "Error: Claude returned empty output" | ||
| exit 1 |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error messages are no longer written to stderr, and echo is used for file writes. For CLI/task tooling, it’s better to keep errors on stderr (so logs/consumers can distinguish failures cleanly) and prefer printf over echo for writing arbitrary content (to avoid edge cases like content equal to -n, escape handling differences, etc.).
| echo "$title" >"$output_dir/title" | ||
| echo "$body" >"$output_dir/notes" |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error messages are no longer written to stderr, and echo is used for file writes. For CLI/task tooling, it’s better to keep errors on stderr (so logs/consumers can distinguish failures cleanly) and prefer printf over echo for writing arbitrary content (to avoid edge cases like content equal to -n, escape handling differences, etc.).
| # Given: usage_files="arg1 'arg with space' arg3" | ||
|
|
||
| # Convert to bash array: | ||
| eval "files=($usage_files)" |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs recommend using eval to turn usage_<name> into an array. eval can execute arbitrary code if the variable content is attacker-controlled or accidentally contains unexpected shell fragments. If usage_files is guaranteed to be generated by a trusted tool, please document that assumption explicitly and add a warning; otherwise, consider documenting a safer serialization/transport format (e.g., newline/NUL-delimited output or JSON) that can be parsed without eval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request refactors the release notes generation script to be a non-blocking mise task, improving its integration into the CI/CD pipeline. The changes are generally good, especially writing output to files for better log visibility and simplifying stderr handling for the claude command.
I have a few suggestions to further improve the changes:
- In
docs/spec/reference/arg.md, I recommend adding a security note regarding the use ofevalto educate users on its safe usage in this context. - In the
tasks/gen-release-notesscript, I've pointed out that all informational and error messages should be redirected tostderrto follow shell scripting best practices. - I've also suggested a more efficient way to parse the output from the
claudecommand in the same script, using shell built-ins to avoid unnecessary process forking.
| # Given: usage_files="arg1 'arg with space' arg3" | ||
|
|
||
| # Convert to bash array: | ||
| eval "files=($usage_files)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using eval can be risky and should be handled with care. While it's necessary here to correctly parse the shell-escaped arguments, it would be beneficial to add a security note to the documentation. This note should explain why eval is safe in this specific context (i.e., usage generates a properly escaped string) and warn developers against using eval with untrusted input in general.
For example, you could add a note below the code block:
Security Note: The
evalcommand can execute arbitrary code and should be used with caution. In this example, it is safe because theusage_<name>variable is generated byusagewhich properly shell-escapes all arguments. Avoid usingevalwith unescaped or untrusted user input.
| echo "Generating release notes with Claude..." >&2 | ||
| echo "Version: $version" >&2 | ||
| echo "Previous version: ${prev_version:-none}" >&2 | ||
| echo "Changelog length: ${#changelog} chars" >&2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a best practice in shell scripting to write progress, informational, and error messages to stderr, reserving stdout for the script's primary data output. The previous version of this script followed this practice. Even though this script now writes its main output to files, adhering to this convention improves consistency and makes the script's behavior more predictable for users and other tools. All echo statements used for logging in this script should redirect to stderr (>&2). This applies to other logging lines as well (e.g., lines 22, 66, 72, 76, 85, 93).
echo "Generating release notes with Claude..." >&2
echo "Version: $version" >&2
echo "Previous version: ${prev_version:-none}" >&2
echo "Changelog length: ${#changelog} chars" >&2
| first_line=$(echo "$output" | head -n1) | ||
| if [[ $first_line == "# "* ]]; then | ||
| title="${first_line#\# }" | ||
| body=$(echo "$output" | tail -n +2) | ||
| else | ||
| echo "Warning: First line not a title, using version" >&2 | ||
| title="$version" | ||
| body="$output" | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block can be made more efficient by using shell built-ins instead of forking external commands like echo, head, and tail. This avoids creating subshells and processing the $output variable multiple times. The suggested change uses read to get the first line and parameter expansion to get the rest of the body, which is much faster. It also redirects the warning message to stderr, which is a best practice for logging.
read -r first_line <<< "$output"
if [[ $first_line == "# "* ]]; then
title="${first_line#\# }"
body="${output#*$'\n'}"
else
echo "Warning: First line not a title, using version" >&2
title="$version"
body="$output"
fi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON, but a Cloud Agent failed to start.
Clarifies that readarray/mapfile are alternatives in bash 4.0+, but macOS ships with bash 3.2 by default which doesn't include them. The eval pattern works with all bash versions. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
readarray/mapfile don't understand shell quoting, so they wouldn't work with the current format regardless of bash version. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move gen-release-notes.sh to a mise file task - Script writes parsed title/notes to an output directory instead of stdout for better CI log visibility - Claude stdout/stderr flows through naturally instead of being suppressed - Add continue-on-error so failures do not block releases Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The in-step fallback logic already handles LLM failures. Having continue-on-error on the step that also runs gh release create would swallow real release creation failures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
16cff24 to
c956a35
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #490 +/- ##
=======================================
Coverage 69.58% 69.58%
=======================================
Files 47 47
Lines 6789 6789
Branches 6789 6789
=======================================
Hits 4724 4724
Misses 1299 1299
Partials 766 766 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
gen-release-notes.shto a mise file taskcontinue-on-errorso failures don't block releasesTest plan
🤖 Generated with Claude Code
Note
Medium Risk
Touches the GitHub release/publish pipeline; misconfiguration could degrade release-note generation or titles, though it still falls back to
CHANGELOG.md.Overview
Release note generation is refactored to be file-based and driven by
mise. Thepublish-cliworkflow now runsmise run gen-release-notesinto a temp output directory, reads the generatedtitleto optionally augment the GitHub release title, and uses the generatednotesfile forgh release create.The release-notes generator is converted into a
misetask.tasks/gen-release-notesnow takesversion,output_dir, and optionalprev_versionvia mise#USAGEargs and writes parsed results to$output_dir/titleand$output_dir/notesinstead of emitting them on stdout.Written by Cursor Bugbot for commit c956a35. This will update automatically on new commits. Configure here.