fix(release): make release notes editorialization non-blocking#8116
fix(release): make release notes editorialization non-blocking#8116
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 significantly enhances the reliability of both the release process and the Conda backend. It ensures that release note generation is non-blocking and provides clearer output, while also introducing critical fixes for Conda package installations on Linux. These fixes include automatically correcting hardcoded build prefixes in text files and refining the handling of RPATHs and dynamic linkers for ELF binaries, thereby improving the overall stability and compatibility of Conda-managed tools. 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.
Code Review
This pull request contains two main sets of changes. The first, reflected in the PR title, modifies the gen-release-notes script to be non-blocking in CI by writing output to files. The second, larger set of changes significantly improves the conda backend by adding support for fixing hardcoded prefixes in text files and robustly handling library paths (RPATH and interpreter) on Linux. These are excellent improvements that will make conda-installed tools much more reliable.
My review focuses on suggesting a small improvement to the shell script's error handling. The Rust code changes for the conda backend are well-implemented and a great addition.
xtasks/gen-release-notes
Outdated
|
|
||
| if [[ -z $changelog ]]; then | ||
| echo "Error: No changes found for release" >&2 | ||
| echo "Error: No changes found for release" |
There was a problem hiding this comment.
xtasks/gen-release-notes
Outdated
| else | ||
| echo "(no stderr output from Claude CLI)" >&2 | ||
| fi | ||
| echo "Error: Claude CLI failed" |
xtasks/gen-release-notes
Outdated
| if [[ -z $output ]]; then | ||
| echo "Error: Claude returned empty output" >&2 | ||
| cat "$stderr_file" >&2 | ||
| echo "Error: Claude returned empty output" |
xtasks/gen-release-notes
Outdated
| body=$(echo "$output" | tail -n +3) | ||
|
|
||
| if [[ -z $body ]]; then | ||
| echo "Error: Empty body after parsing" |
There was a problem hiding this comment.
Pull request overview
This PR aims to make the “editorialize release notes” step non-blocking and improve CI log visibility by writing parsed release-note artifacts to a directory instead of stdout.
Changes:
- Update
gen-release-notesto writetitleandnotesfiles into an output directory and stop suppressing Claude CLI output. - Make the GitHub Release creation step
continue-on-errorand update it to read generated title/notes files. - Enhance conda-based installs on Linux by improving RPATH/interpreter handling and adding text-prefix rewrites; update some registry entries to prefer conda backends + add tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| xtasks/gen-release-notes | Writes parsed title/notes to an output dir and changes logging/Claude output handling. |
| .github/workflows/release.yml | Uses output dir artifacts for release notes and makes the release step non-blocking. |
| src/backend/conda_linux.rs | Expands Linux patchelf handling (RPATH + interpreter) and library directory discovery. |
| src/backend/conda_common.rs | Adds logic to rewrite conda placeholder prefixes in extracted text files. |
| src/backend/conda.rs | Invokes text-prefix rewriting and tweaks skip-package policy docs/behavior. |
| src/cli/test_tool.rs | Improves diagnostics when a command exits 127 by logging captured output. |
| registry/vim.toml | Prefers conda backend and adds a simple runtime test. |
| registry/ghc.toml | Prefers conda backend ordering. |
| registry/ffmpeg.toml | Prefers conda backend and adds a simple runtime test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
xtasks/gen-release-notes
Outdated
| title=$(echo "$output" | head -1) | ||
| body=$(echo "$output" | tail -n +3) | ||
|
|
||
| if [[ -z $body ]]; then | ||
| echo "Error: Empty body after parsing" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
The parsing assumes Claude output is exactly: first line title, second line blank, body from line 3 (tail -n +3). If Claude returns no blank line, line 2 will be dropped and body may be empty even when valid content exists. Consider splitting on the first blank line when present, otherwise treat everything after the first line as the body (with optional trimming of a single leading blank line).
xtasks/gen-release-notes
Outdated
|
|
||
| if [[ -z $changelog ]]; then | ||
| echo "Error: No changes found for release" >&2 | ||
| echo "Error: No changes found for release" |
There was a problem hiding this comment.
These error messages were previously written to stderr; now they go to stdout, which can make it harder for CI tooling/users to distinguish failures from normal output (especially since you also print raw Claude output). Recommend sending errors to stderr (e.g., >&2) while keeping the raw Claude output printing behavior unchanged.
xtasks/gen-release-notes
Outdated
| else | ||
| echo "(no stderr output from Claude CLI)" >&2 | ||
| fi | ||
| echo "Error: Claude CLI failed" |
There was a problem hiding this comment.
These error messages were previously written to stderr; now they go to stdout, which can make it harder for CI tooling/users to distinguish failures from normal output (especially since you also print raw Claude output). Recommend sending errors to stderr (e.g., >&2) while keeping the raw Claude output printing behavior unchanged.
xtasks/gen-release-notes
Outdated
| if [[ -z $output ]]; then | ||
| echo "Error: Claude returned empty output" >&2 | ||
| cat "$stderr_file" >&2 | ||
| echo "Error: Claude returned empty output" |
There was a problem hiding this comment.
These error messages were previously written to stderr; now they go to stdout, which can make it harder for CI tooling/users to distinguish failures from normal output (especially since you also print raw Claude output). Recommend sending errors to stderr (e.g., >&2) while keeping the raw Claude output printing behavior unchanged.
xtasks/gen-release-notes
Outdated
| title=$(echo "$output" | head -1) | ||
| body=$(echo "$output" | tail -n +3) |
There was a problem hiding this comment.
Using echo to pipe arbitrary LLM output is brittle (some echo implementations treat leading -n/-e as options, and escape handling varies). Prefer printf '%s\n' \"$output\" (or equivalent) to ensure the content is passed through exactly as received.
.github/workflows/release.yml
Outdated
| if: startsWith(github.event.ref, 'refs/tags/v') | ||
| continue-on-error: true | ||
| run: | | ||
| VERSION="$(./scripts/get-version.sh)" | ||
| PREV_VERSION="$(git describe --tags --abbrev=0 --match 'v[0-9]*' "$VERSION^" 2>/dev/null || echo "")" | ||
| # Generate rich release notes using LLM (first line is title, rest is body) | ||
| mise run gen-release-notes "$VERSION" "$PREV_VERSION" >/tmp/release-full.txt | ||
| RELEASE_TITLE="$VERSION: $(head -1 /tmp/release-full.txt)" | ||
| tail -n +3 /tmp/release-full.txt >/tmp/release-notes.txt | ||
| # Generate rich release notes using LLM | ||
| OUTPUT_DIR=$(mktemp -d) | ||
| mise run gen-release-notes "$VERSION" "$OUTPUT_DIR" "$PREV_VERSION" | ||
| RELEASE_TITLE="$VERSION: $(cat "$OUTPUT_DIR/title")" | ||
| cp "$OUTPUT_DIR/notes" /tmp/release-notes.txt |
There was a problem hiding this comment.
Setting continue-on-error: true on the entire 'Create Draft GitHub Release' step makes any failure in the step non-blocking (including failures unrelated to editorialization or even the actual release creation). If the intent is only to make the LLM editorialization non-blocking, consider isolating the editorialization into its own step with continue-on-error, or handling just the mise run gen-release-notes ... failure in-shell and falling back to non-editorialized notes.
| RELEASE_TITLE="$VERSION: $(head -1 /tmp/release-full.txt)" | ||
| tail -n +3 /tmp/release-full.txt >/tmp/release-notes.txt | ||
| # Generate rich release notes using LLM | ||
| OUTPUT_DIR=$(mktemp -d) |
There was a problem hiding this comment.
mktemp -d creates a directory that isn't cleaned up. While runners are ephemeral, it’s still good hygiene to trap a cleanup (rm -rf \"$OUTPUT_DIR\") in the step to avoid clutter and reduce the chance of leaking sensitive artifacts if the environment is re-used.
| OUTPUT_DIR=$(mktemp -d) | |
| OUTPUT_DIR=$(mktemp -d) | |
| trap 'rm -rf "$OUTPUT_DIR"' EXIT |
| if let Ok(stdout) = String::from_utf8(res.stdout.clone()) { | ||
| let stdout = stdout.trim(); | ||
| if !stdout.is_empty() { | ||
| info!("command output:\n{stdout}"); | ||
| } |
There was a problem hiding this comment.
This clones res.stdout just to decode it, which can be unnecessarily expensive for large outputs. Consider decoding without cloning (e.g., with String::from_utf8_lossy(&res.stdout)) and logging the trimmed view.
| if let Ok(stdout) = String::from_utf8(res.stdout.clone()) { | |
| let stdout = stdout.trim(); | |
| if !stdout.is_empty() { | |
| info!("command output:\n{stdout}"); | |
| } | |
| let stdout = String::from_utf8_lossy(&res.stdout); | |
| let stdout = stdout.trim(); | |
| if !stdout.is_empty() { | |
| info!("command output:\n{stdout}"); |
| // Walk backward to find start of the absolute path | ||
| let before = &content[..idx]; | ||
| let pos = before | ||
| .rfind(|c: char| !c.is_alphanumeric() && !matches!(c, '/' | '_' | '-' | '.' | '+'))?; | ||
| // Skip past the delimiter character (handles multi-byte UTF-8 correctly) | ||
| let start = pos + before[pos..].chars().next()?.len_utf8(); | ||
|
|
||
| // Validate it starts with / | ||
| if !content[start..].starts_with('/') { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
rfind(...)? returns None if the placeholder path begins at the start of the content (i.e., there is no preceding delimiter). In that case, a valid prefix won't be detected and no replacements will occur. Consider handling the 'no delimiter found' case by treating start as 0 when before itself starts with /.
| // Add all discovered lib directories as $ORIGIN-relative paths | ||
| for lib_dir in lib_dirs { | ||
| if let Ok(rel_path) = lib_dir.strip_prefix(install_dir) { | ||
| let Some(rel) = rel_path.to_str() else { | ||
| continue; // Skip non-UTF8 paths | ||
| }; | ||
| if let Some(parent) = path.parent() | ||
| && let Ok(from_parent) = parent.strip_prefix(install_dir) | ||
| { | ||
| let depth = from_parent.components().count(); | ||
| let up = "../".repeat(depth); | ||
| let entry = format!("$ORIGIN/{}{}", up, rel); | ||
| if !entries.contains(&entry) { | ||
| entries.push(entry); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
entries.contains(&entry) inside the loop makes dedup O(n²) in the number of discovered library directories. For installs with many .so directories, this can become a noticeable bottleneck. Consider tracking seen entries with a HashSet<String> (or IndexSet) while preserving order in entries.
- 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>
536dff7 to
ca9097b
Compare
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.9 x -- echo |
22.2 ± 0.4 | 21.0 | 25.0 | 1.00 |
mise x -- echo |
22.9 ± 0.5 | 21.8 | 25.5 | 1.03 ± 0.03 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.9 env |
22.0 ± 0.7 | 20.9 | 27.9 | 1.00 |
mise env |
22.5 ± 0.5 | 21.6 | 25.7 | 1.02 ± 0.04 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.9 hook-env |
22.0 ± 0.4 | 21.2 | 24.8 | 1.00 |
mise hook-env |
22.8 ± 0.5 | 21.9 | 25.7 | 1.04 ± 0.03 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.9 ls |
20.3 ± 0.4 | 19.3 | 22.6 | 1.01 ± 0.03 |
mise ls |
20.2 ± 0.3 | 19.6 | 22.4 | 1.00 |
xtasks/test/perf
| Command | mise-2026.2.9 | mise | Variance |
|---|---|---|---|
| install (cached) | 120ms | 122ms | -1% |
| ls (cached) | 73ms | 75ms | -2% |
| bin-paths (cached) | 77ms | 77ms | +0% |
| task-ls (cached) | 535ms | -32% |
The workflow expects gen-release-notes to accept an output_dir arg and write title/notes files there. Update the script to match, using mise task arg syntax. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add >&2 to missing error message in gen-release-notes - Use printf instead of echo for LLM output to avoid -n/-e interpretation - Make gen-release-notes non-blocking: fall back to git-cliff on failure - Add trap cleanup for mktemp directory Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
) ## Summary - Script writes parsed title/notes to an output directory instead of stdout for better CI log visibility - Claude's stdout/stderr flows through naturally instead of being suppressed - Add `continue-on-error` so failures don't block releases ## Test plan - [ ] Trigger a release and verify the editorialize step logs are visible - [ ] If Claude produces bad output, the release still succeeds 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Release automation-only changes with a straightforward fallback path; main risk is malformed parsing/output causing slightly degraded release notes rather than a failed release. > > **Overview** > Draft release creation is updated to treat LLM-generated release notes as **best-effort**: it now runs `gen-release-notes` into a temp output directory, uses the generated `title`/`notes` files when successful, and *falls back to `git cliff`* notes (with a warning) when it fails. > > The `gen-release-notes` task is refactored to accept `<version> <output_dir> [prev_version]`, emit more useful logs, parse Claude output into separate title/body files, and documentation is updated accordingly; the workflow also switches to `printf` when appending the Aqua section to avoid shell quoting issues. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 0440455. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Summary
continue-on-errorso failures don't block releasesTest plan
🤖 Generated with Claude Code
Note
Low Risk
Release automation-only changes with a straightforward fallback path; main risk is malformed parsing/output causing slightly degraded release notes rather than a failed release.
Overview
Draft release creation is updated to treat LLM-generated release notes as best-effort: it now runs
gen-release-notesinto a temp output directory, uses the generatedtitle/notesfiles when successful, and falls back togit cliffnotes (with a warning) when it fails.The
gen-release-notestask is refactored to accept<version> <output_dir> [prev_version], emit more useful logs, parse Claude output into separate title/body files, and documentation is updated accordingly; the workflow also switches toprintfwhen appending the Aqua section to avoid shell quoting issues.Written by Cursor Bugbot for commit 0440455. This will update automatically on new commits. Configure here.