Skip to content

fix(release): stamp the version into the changelog - #69

Merged
michen00 merged 5 commits into
mainfrom
fix/stamp-changelog-version
Aug 8, 2026
Merged

fix(release): stamp the version into the changelog#69
michen00 merged 5 commits into
mainfrom
fix/stamp-changelog-version

Conversation

@michen00

@michen00 michen00 commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Summary

Releases never wrote a changelog section for the version they released, and the next release then deleted the entries that had nowhere else to live. release-pr.yml ran scripts/update-unreleased.sh, which refreshes the Unreleased block; changelog-autoupdate.yml runs the same script weekly, which is correct for it; and release-tag.yml does not touch the changelog at all. Nothing in the pipeline ever converted [Unreleased] into ## [X.Y.Z].

The failure is silent rather than loud. update-unreleased.sh drops the stale Unreleased block by design and regenerates it from git cliff --unreleased, which reports commits since the newest tag. The moment a release is tagged, its commits stop being unreleased — so if no versioned section was written for them, the next refresh removes them and puts nothing back. v0.1.0 shipped on 2026-08-05 with no ## [0.1.0] section in CHANGELOG.md, which jumps straight from [Unreleased] to [0.0.4], and #67 consequently proposed deleting 134 lines of shipped history: 383 lines in, 264 out, with no [0.1.0] section in its output either.

scripts/release/stamp-changelog.sh regenerates with --tag, which rebuilds the file with a section per tag and the pending release on top. Run against this repository it grows the file rather than shrinking it — 413 lines against the committed 383 — with ## [0.1.0] restored, carrying the entries #67 had removed. Regenerating wholesale rather than splicing is safe because released sections derive from tags and commits, both immutable — the only differences against the committed file are whitespace that the workflow's existing prettier step normalizes on the very next line.

The script refuses rather than writes when the result looks wrong: empty output, no section for the version being released, or fewer sections than the file it replaces. A silent shrink is precisely the failure that cost this repository its v0.1.0 entries, so it is worth failing the release PR over. It also normalizes the tag through scripts/release/parse-version.sh — the strict vX.Y.Z parser the three release workflows already share — before that value reaches git cliff --tag or the section guard's grep pattern, so a run by hand cannot put a metacharacter into either.

Test plan

  • tests/test-unit.sh passes — 506 assertions across 7 suites, including 9 new ones
  • Every pre-commit hook passes, including shellcheck, shfmt, yamllint, actionlint, and gitlint
  • Verified against the real repository: git cliff --tag v0.1.1 yields 413 lines against the committed 383, with ## [0.1.0] restored above the [0.0.4] section, and that section contains commits chore(release): prepare v0.1.1 #67 deletes — "add signed release workflows", "bump the README rev pin on release", "add benchmark runner scripts" all present
  • Verified regeneration does not disturb already-released history: diffing the [0.0.4]-and-older sections against the committed file shows only a double space and two blank lines, all of which prettier normalizes, and a whitespace-insensitive diff of that range is empty
  • Each guard was exercised and observed to refuse and leave the file unchanged: missing pending-version section, dropped sections, and empty output
  • Helper resolution is covered by mutation rather than inspection: pointing the script at a bare parse-version.sh or at ./parse-version.sh each makes the suite fail, and only the script-relative form passes
  • CI is green

Reviewer guide

  • Effort: ~10 minutes. scripts/release/stamp-changelog.sh is the change; the workflow edit swaps one step for another and explains why in a comment.
  • Read only these: the three guards in that script, and whether refusing the release PR is the right response to each. They fail the workflow rather than warning, on the argument that a changelog that silently shrinks is worse than a release that stops.
  • Already covered, please skip: tests/test-stamp-changelog.sh is mechanical coverage of those guards, the tag validation, and helper resolution, all driven through a git-cliff stub, which keeps the suite hermetic and free of a fixture repository.
  • The calls only you can make: whether to keep this pipeline at all. The bug is a design gap rather than a typo, and release-please would remove the whole class by making its release PR the versioned section. It would also cost the two things this repository built deliberately: git tag -a -s with your own GPG key, guarded by a check that refuses to publish a tag that is not a validly signed annotated tag, and the release environment's one-approval-per-release gate. A hybrid — release-please for version and changelog, release-tag.yml retained for signed tagging — keeps both. This PR is the narrow fix and does not foreclose any of that.
  • Known weaknesses: the section-count guard compares totals, so a regeneration that swapped one section for another would pass it. Catching that needs comparing section identities, which is more machinery than the observed failure justifies. Separately, the sibling-helper lookup resolves against $0 and does not dereference symlinks, so the script would not find parse-version.sh if it were symlinked into a bin directory; nothing does that today.

Notes

v0.1.0 remains absent from CHANGELOG.md on main. This PR fixes the process; the missing section arrives with the next release PR, whose regeneration will include it. #67 should be closed rather than merged — it was generated by the old path and still carries the deletion.

Files touched

Status legend: + added, ~ modified, - removed, renamed, ~/→ renamed and modified.

status path role
+ scripts/release/stamp-changelog.sh Validates the tag, regenerates the changelog with the pending version stamped, and refuses results that lose content
+ tests/test-stamp-changelog.sh Covers argument handling, tag validation, helper resolution, and all three guards against a git-cliff stub
~ .github/workflows/release-pr.yml Calls the stamping script instead of the Unreleased refresh
~ tests/test-unit.sh Registers the new suite
~ AGENTS.md Records that the two changelog scripts are not interchangeable, and why
~ CLAUDE.md Same distinction in the project-memory summary

Closes #70 (review-convergence bulletin)

The release PR refreshed the Unreleased section and never wrote a
section for the version being released, so nothing in the pipeline
ever converted Unreleased into `## [X.Y.Z]`. release-tag.yml touches
the changelog not at all.

That is silently destructive. Once a tag exists, git cliff stops
reporting the commits it covers as unreleased, and
update-unreleased.sh drops the stale block by design -- so entries
that never got a versioned section have nowhere to go. v0.1.0 shipped
on 2026-08-05 with no `## [0.1.0]` section, and the v0.1.1 release PR
consequently deleted 134 lines of history: 383 lines in, 264 out.

stamp-changelog.sh regenerates with `--tag`, which rebuilds the file
with a section per tag and the pending release at the top. Verified
against the real repository: 398 lines, with `## [0.1.0]` restored and
carrying the entries the release PR had removed. Regenerating
wholesale is safe because released sections derive from tags and
commits; the only differences against the committed file are
whitespace that the workflow's prettier step normalizes right after.

It refuses rather than writes when the result looks wrong -- empty
output, no section for the pending version, or fewer sections than it
replaces -- because a silent shrink is exactly the failure that cost
this repository its v0.1.0 entries. Seven tests cover those guards
using a git-cliff stub, so the suite needs no fixture repository.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@michen00
michen00 marked this pull request as ready for review August 7, 2026 04:32
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix release workflow to stamp versioned changelog sections

🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Replace release PR changelog refresh with a version-stamping regeneration step.
• Add a guarded stamp-changelog.sh to rebuild CHANGELOG from tags and fail on shrink.
• Document the two changelog workflows and add hermetic unit coverage for the new script.
Diagram

graph TD
  rel(["Release PR workflow"]) --> stamp["stamp-changelog.sh"] --> cliff{{"git-cliff"}} --> chlog[/"CHANGELOG.md"/]
  auto(["Changelog autoupdate workflow"]) --> upd["update-unreleased.sh"] --> cliff
  rel --> fmt["prettier formatting"] --> chlog

  subgraph Legend
    direction LR
    _wf(["Workflow"]) ~~~ _script["Script"] ~~~ _ext{{"External tool"}} ~~~ _file[/"File"/]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Extend update-unreleased.sh with a --tag/--release mode
  • ➕ Fewer scripts to maintain; single entrypoint for changelog updates
  • ➕ Could centralize temp-file safety and publishing logic
  • ➖ Harder mental model (one script does two opposite operations)
  • ➖ Higher risk of accidentally using the wrong mode in workflows
2. Splice Unreleased into a new versioned section (in-place edit)
  • ➕ Smaller diff to CHANGELOG.md (no full regeneration)
  • ➕ Avoids relying on git-cliff’s full tagged reconstruction
  • ➖ More parsing/formatting complexity and edge cases in shell
  • ➖ Still needs strong guards to prevent silent loss; harder to validate correctness than regeneration

Recommendation: The chosen approach (full regeneration via git cliff --tag during releases) is the safest and simplest correctness model: released sections become purely tag-derived, and the explicit guards (non-empty output, must include pending version, must not reduce section count) directly prevent the silent-history-loss failure mode. Keeping a separate stamp-changelog.sh also makes the workflow intent unambiguous versus overloading update-unreleased.sh.

Files changed (6) +257 / -4

Enhancement (1) +80 / -0
stamp-changelog.shAdd guarded changelog stamping via git-cliff --tag +80/-0

Add guarded changelog stamping via git-cliff --tag

• Introduces a release helper that regenerates CHANGELOG.md using 'git cliff --tag <vX.Y.Z>' so the pending release gets its own versioned section. Adds safety checks to refuse empty output, missing pending-version section, or a reduction in section count, and writes via a temp file to avoid truncation on failure.

scripts/release/stamp-changelog.sh

Bug fix (1) +10 / -2
release-pr.ymlUse version-stamping changelog regeneration during release PRs +10/-2

Use version-stamping changelog regeneration during release PRs

• Replaces the release PR step that refreshed the Unreleased section with a new step that regenerates CHANGELOG.md as if the pending version were released. Passes the resolved release tag into the stamping script and documents why Unreleased refresh is incorrect during releases.

.github/workflows/release-pr.yml

Tests (2) +164 / -0
test-stamp-changelog.shAdd hermetic unit tests for stamp-changelog.sh guards +163/-0

Add hermetic unit tests for stamp-changelog.sh guards

• Adds a bash test suite that stubs 'git-cliff' on PATH to precisely control generated changelog output. Verifies argument validation and refusal behavior for empty output, missing pending-version section, and dropped section counts, and confirms successful publication on valid output.

tests/test-stamp-changelog.sh

test-unit.shInclude stamp-changelog unit tests in the test runner +1/-0

Include stamp-changelog unit tests in the test runner

• Registers the new 'test-stamp-changelog.sh' suite in the aggregated unit test script so it runs in CI with the other test suites.

tests/test-unit.sh

Documentation (2) +3 / -2
AGENTS.mdDocument stamp-changelog script and when to use it +2/-1

Document stamp-changelog script and when to use it

• Adds 'scripts/release/stamp-changelog.sh' to the documented release scripts list. Clarifies the distinct purposes of 'update-unreleased.sh' (weekly refresh) versus stamping the pending version during a release, including the failure mode it prevents.

AGENTS.md

CLAUDE.mdClarify release changelog responsibilities in repo guide +1/-1

Clarify release changelog responsibilities in repo guide

• Updates the scripts overview to distinguish release-time changelog stamping from weekly Unreleased refresh. Captures the rationale that prevents silently dropping entries after tagging.

CLAUDE.md

@qodo-code-review

qodo-code-review Bot commented Aug 7, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Helper lookup test masked ✓ Resolved 🐞 Bug ⚙ Maintainability ⭐ New
Description
The new decoy test can still pass if stamp-changelog.sh regresses to resolving parse-version.sh via
PATH, because the test prepends scripts/release to PATH for the invocation. This makes the test’s
pass message (“resolved next to the script”) stronger than what it actually proves, reducing
regression coverage.
Code

tests/test-stamp-changelog.sh[R199-202]

+release_dir="$(cd "$TEST_SCRIPT_DIR/../scripts/release" && pwd)"
+if (
+	cd "$decoy_dir" &&
+		PATH="$release_dir:$PATH" stamp-changelog.sh v0.2.0 "$changelog" >/dev/null 2>&1
Evidence
The test prepends the real release directory to PATH for the command invocation, so a PATH-based
helper lookup would still find the real helper and pass. The production script currently uses a
script-relative helper path, which the test message implies is being enforced, but the PATH setup
reduces the test’s ability to detect regressions away from that behavior.

tests/test-stamp-changelog.sh[199-203]
scripts/release/stamp-changelog.sh[35-37]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The helper-resolution regression test is intended to ensure `stamp-changelog.sh` does **not** resolve `parse-version.sh` from the caller’s working directory (and ideally not via `PATH`), but the test currently prepends the real `scripts/release` directory to `PATH`. That means a regression from `"$script_dir/parse-version.sh"` to `parse-version.sh` would still find the real helper via `PATH` and the test would incorrectly pass.

## Issue Context
- The test constructs a decoy `parse-version.sh` in `$decoy_dir` and runs from that directory.
- However, it also runs `stamp-changelog.sh` with `PATH="$release_dir:$PATH"`, which makes the real helper discoverable via `PATH` and masks PATH-based helper lookup regressions.

## Fix Focus Areas
- tests/test-stamp-changelog.sh[199-203]

### Suggested change (one viable approach)
Change the invocation so that **if** the production script ever tries `parse-version.sh` via `PATH`, it would hit the decoy and fail.

For example:
- Invoke the script by absolute path (`"$STAMP"`) instead of via `PATH`.
- Set `PATH` to include `$decoy_dir` (and existing stub bin) but **not** `$release_dir`.

Example snippet:
```sh
if (
 cd "$decoy_dir" &&
   PATH="$decoy_dir:$PATH" "$STAMP" v0.2.0 "$changelog" >/dev/null 2>&1
) && grep -q '^## \[0.2.0\]' "$changelog"; then
 ...
fi
```

(Alternative: create a temporary `stamp-changelog.sh` shim in the stub bin directory that `exec`s the real script by absolute path, so the command is still invoked via PATH without putting `$release_dir` on PATH.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Unchecked release_dir assignment ⊘ Outdated 📘 Rule violation ☼ Reliability ⭐ New
Description
The command substitution used to compute release_dir does not have its exit status checked, and
the script does not use set -e, so a failed cd would be silently ignored and could make the test
behave incorrectly. This violates the requirement to not ignore exit codes of executed commands in
shell scripts.
Code

tests/test-stamp-changelog.sh[199]

+release_dir="$(cd "$TEST_SCRIPT_DIR/../scripts/release" && pwd)"
Evidence
PR Compliance ID 2535276 requires that shell scripts do not ignore exit codes (use set -e or
explicitly check status). In the focused change, release_dir is computed via a command
substitution whose failure is not checked, and the script’s flags (set -uo pipefail) do not cause
an exit on error.

Rule 2535276: Do not ignore exit codes of executed commands in shell scripts
tests/test-stamp-changelog.sh[199-199]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`release_dir="$(cd ... && pwd)"` ignores failures because the script does not use `set -e`, so a failing `cd` can silently continue and lead to incorrect PATH/test behavior.

## Issue Context
This is in the new test that validates `stamp-changelog.sh` resolves helpers relative to its own directory; the test should fail loudly if it cannot compute the intended `release_dir`.

## Fix Focus Areas
- tests/test-stamp-changelog.sh[199-203]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Wrong helper script resolved ✗ Dismissed 🐞 Bug ☼ Reliability
Description
stamp-changelog.sh derives script_dir from dirname("$0"), which can be "." or otherwise CWD-relative
when invoked via PATH or from a different working directory. This can make it fail to find the
repo’s parse-version.sh (or execute an unintended ./parse-version.sh), undermining the version
normalization the rest of the script relies on.
Code

scripts/release/stamp-changelog.sh[R35-38]

+script_dir="$(dirname "$0")"
+if ! tag="$("$script_dir/parse-version.sh" "$1")"; then
+	exit 1
+fi
Evidence
The new script resolves parse-version.sh relative to dirname "$0", which is not a stable way to
locate sibling scripts across different invocation styles. Existing repo scripts demonstrate the
intended robust pattern by canonicalizing the script directory with cd+pwd and clearing
CDPATH.

scripts/release/stamp-changelog.sh[30-38]
scripts/release/build-artifacts.sh[21-22]
scripts/benchmark/run.sh[8-9]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`scripts/release/stamp-changelog.sh` computes `script_dir` using `dirname "$0"` and then executes `"$script_dir/parse-version.sh"`. When the script is invoked from `PATH` (where `$0` may be just `stamp-changelog.sh`) or from outside the repo using a relative path, `script_dir` can resolve to `.` or to a directory relative to the caller’s CWD, causing the script to:
- fail to locate the intended `parse-version.sh`, or
- run an unintended `./parse-version.sh` from the current directory.

This breaks manual usage and weakens the guarantee that the tag is normalized/sanitized before being used as a tag argument and in regex construction.

### Issue Context
Other scripts in this repo already use a more robust pattern to canonicalize the script directory using `cd`+`pwd` with `CDPATH` cleared.

### Fix Focus Areas
- scripts/release/stamp-changelog.sh[30-39]

### Implementation guidance
- Resolve the script path first, handling both “has slash” and “PATH invocation” cases, then canonicalize to an absolute directory:
 - If `$0` contains no `/`, get the real path via `command -v -- "$0"`.
 - Use `script_dir="$(CDPATH='' cd -- "$(dirname -- "$script_path")" && pwd)"`.
- Then call `"$script_dir/parse-version.sh"`.
- (Optional) Add a regression test that runs the script from a different working directory (e.g., `cd "$work"`) while invoking it via an absolute path or via PATH to ensure it still finds the correct helper.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (2)
4. Non-atomic changelog write ✗ Dismissed 🐞 Bug ☼ Reliability
Description
scripts/release/stamp-changelog.sh publishes the regenerated changelog via cat tmp > CHANGELOG.md,
which truncates the destination before writing; an interrupt or I/O failure can still leave a
partially written/empty changelog. This contradicts the script’s earlier rationale about avoiding
half-written CHANGELOG.md on failures.
Code

scripts/release/stamp-changelog.sh[R75-78]

+# git-cliff succeeded and the result passed inspection, so publish it. Writing
+# through the existing file rather than renaming over it keeps the changelog's
+# permissions and inode.
+cat "$tmp_changelog" >"$changelog_file"
Evidence
The script calls out truncation risks from direct writes, but still uses a truncating redirection
when publishing the final changelog.

scripts/release/stamp-changelog.sh[46-49]
scripts/release/stamp-changelog.sh[75-79]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The final publish step uses truncating shell redirection (`>"$changelog_file"`). If the process is interrupted (TERM) or the write fails (disk full, I/O error), `CHANGELOG.md` can be left empty/partial even though generation/validation succeeded.

### Issue Context
The script explicitly avoids pointing `git cliff --output` at the real changelog to prevent truncation on generation failures, but the final copy step reintroduces a similar truncation hazard.

### Fix Focus Areas
- scripts/release/stamp-changelog.sh[43-50]
- scripts/release/stamp-changelog.sh[75-78]

### Implementation notes
- Write to a temporary file in the same directory as `CHANGELOG.md` and `mv` it into place (atomic on same filesystem).
- If preserving permissions is important, copy mode from the original file onto the temp file before `mv` (best-effort, portable fallback).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Regex version match ✓ Resolved 🐞 Bug ≡ Correctness
Description
scripts/release/stamp-changelog.sh checks for the pending version section using a grep regex built
from the version string, so dots are treated as wildcards and unexpected characters can change what
matches. In the normal release workflow the tag is validated, but this guard can still incorrectly
accept/reject output when run directly or if integrated elsewhere without parse-version protection.
Code

scripts/release/stamp-changelog.sh[R62-65]

+version="${tag#v}"
+if ! grep -q "^## \[${version}\]" "$tmp_changelog"; then
+	echo "Error: no '## [${version}]' section in the regenerated changelog." >&2
+	exit 1
Evidence
The script interpolates ${version} directly into a grep regex used to validate output, and the
project’s changelog headings are derived from version strings (which contain dots). While workflows
validate tags, this script itself currently does not enforce a safe/literal match.

scripts/release/stamp-changelog.sh[62-66]
cliff.toml[19-28]
scripts/release/parse-version.sh[46-71]
.github/workflows/release-pr.yml[34-60]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`stamp-changelog.sh` validates the regenerated changelog by searching for the pending version header using a grep regex containing the raw `${version}`. Because `grep` treats `.` as “any char” (and other metacharacters have special meaning), the guard is not a strict literal match.

### Issue Context
The release workflows generate a safe tag via `scripts/release/parse-version.sh`, but `stamp-changelog.sh` is also a standalone script and its internal guard should be robust even if invoked without that workflow.

### Fix Focus Areas
- scripts/release/stamp-changelog.sh[62-66]

### Implementation notes
- Prefer a fixed-string check (e.g., `grep -Fq "## [${version}]" ...`) or escape the version for regex before using it.
- Optionally reuse `parse-version.sh` (from the same directory) to normalize/validate `tag` inside `stamp-changelog.sh` before generating and validating output.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

6. test-stamp-changelog.sh uses bash shebang ✗ Dismissed 📘 Rule violation ⚙ Maintainability
Description
The new test script starts with #!/usr/bin/env bash instead of the required #!/bin/sh. This
violates the repository requirement that shell scripts must use the /bin/sh shebang, and may break
portability where bash is unavailable.
Code

tests/test-stamp-changelog.sh[1]

+#!/usr/bin/env bash
Evidence
PR Compliance ID 2535260 requires that shell scripts with a shebang use #!/bin/sh exactly. The
added test script begins with #!/usr/bin/env bash, which is explicitly disallowed by that rule.

Rule 2535260: Shell scripts must use /bin/sh shebang
tests/test-stamp-changelog.sh[1-5]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`tests/test-stamp-changelog.sh` uses a bash shebang (`#!/usr/bin/env bash`), but compliance requires `#!/bin/sh` for shell scripts.

## Issue Context
This file is newly added in the PR and is executed by `tests/test-unit.sh`, so it is treated as an executable shell script subject to the `/bin/sh` shebang requirement.

## Fix Focus Areas
- tests/test-stamp-changelog.sh[1-10]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context used
✅ Compliance rules (platform): 14 rules

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Previous review results

Review updated until commit 86b964b

Results up to commit 246fd4b ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Remediation recommended
1. Non-atomic changelog write ✗ Dismissed 🐞 Bug ☼ Reliability
Description
scripts/release/stamp-changelog.sh publishes the regenerated changelog via cat tmp > CHANGELOG.md,
which truncates the destination before writing; an interrupt or I/O failure can still leave a
partially written/empty changelog. This contradicts the script’s earlier rationale about avoiding
half-written CHANGELOG.md on failures.
Code

scripts/release/stamp-changelog.sh[R75-78]

+# git-cliff succeeded and the result passed inspection, so publish it. Writing
+# through the existing file rather than renaming over it keeps the changelog's
+# permissions and inode.
+cat "$tmp_changelog" >"$changelog_file"
Evidence
The script calls out truncation risks from direct writes, but still uses a truncating redirection
when publishing the final changelog.

scripts/release/stamp-changelog.sh[46-49]
scripts/release/stamp-changelog.sh[75-79]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The final publish step uses truncating shell redirection (`>"$changelog_file"`). If the process is interrupted (TERM) or the write fails (disk full, I/O error), `CHANGELOG.md` can be left empty/partial even though generation/validation succeeded.

### Issue Context
The script explicitly avoids pointing `git cliff --output` at the real changelog to prevent truncation on generation failures, but the final copy step reintroduces a similar truncation hazard.

### Fix Focus Areas
- scripts/release/stamp-changelog.sh[43-50]
- scripts/release/stamp-changelog.sh[75-78]

### Implementation notes
- Write to a temporary file in the same directory as `CHANGELOG.md` and `mv` it into place (atomic on same filesystem).
- If preserving permissions is important, copy mode from the original file onto the temp file before `mv` (best-effort, portable fallback).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Regex version match ✓ Resolved 🐞 Bug ≡ Correctness
Description
scripts/release/stamp-changelog.sh checks for the pending version section using a grep regex built
from the version string, so dots are treated as wildcards and unexpected characters can change what
matches. In the normal release workflow the tag is validated, but this guard can still incorrectly
accept/reject output when run directly or if integrated elsewhere without parse-version protection.
Code

scripts/release/stamp-changelog.sh[R62-65]

+version="${tag#v}"
+if ! grep -q "^## \[${version}\]" "$tmp_changelog"; then
+	echo "Error: no '## [${version}]' section in the regenerated changelog." >&2
+	exit 1
Evidence
The script interpolates ${version} directly into a grep regex used to validate output, and the
project’s changelog headings are derived from version strings (which contain dots). While workflows
validate tags, this script itself currently does not enforce a safe/literal match.

scripts/release/stamp-changelog.sh[62-66]
cliff.toml[19-28]
scripts/release/parse-version.sh[46-71]
.github/workflows/release-pr.yml[34-60]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`stamp-changelog.sh` validates the regenerated changelog by searching for the pending version header using a grep regex containing the raw `${version}`. Because `grep` treats `.` as “any char” (and other metacharacters have special meaning), the guard is not a strict literal match.

### Issue Context
The release workflows generate a safe tag via `scripts/release/parse-version.sh`, but `stamp-changelog.sh` is also a standalone script and its internal guard should be robust even if invoked without that workflow.

### Fix Focus Areas
- scripts/release/stamp-changelog.sh[62-66]

### Implementation notes
- Prefer a fixed-string check (e.g., `grep -Fq "## [${version}]" ...`) or escape the version for regex before using it.
- Optionally reuse `parse-version.sh` (from the same directory) to normalize/validate `tag` inside `stamp-changelog.sh` before generating and validating output.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational
3. test-stamp-changelog.sh uses bash shebang ✗ Dismissed 📘 Rule violation ⚙ Maintainability
Description
The new test script starts with #!/usr/bin/env bash instead of the required #!/bin/sh. This
violates the repository requirement that shell scripts must use the /bin/sh shebang, and may break
portability where bash is unavailable.
Code

tests/test-stamp-changelog.sh[1]

+#!/usr/bin/env bash
Evidence
PR Compliance ID 2535260 requires that shell scripts with a shebang use #!/bin/sh exactly. The
added test script begins with #!/usr/bin/env bash, which is explicitly disallowed by that rule.

Rule 2535260: Shell scripts must use /bin/sh shebang
tests/test-stamp-changelog.sh[1-5]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`tests/test-stamp-changelog.sh` uses a bash shebang (`#!/usr/bin/env bash`), but compliance requires `#!/bin/sh` for shell scripts.

## Issue Context
This file is newly added in the PR and is executed by `tests/test-unit.sh`, so it is treated as an executable shell script subject to the `/bin/sh` shebang requirement.

## Fix Focus Areas
- tests/test-stamp-changelog.sh[1-10]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit abeb632 ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Remediation recommended
1. Wrong helper script resolved ✗ Dismissed 🐞 Bug ☼ Reliability
Description
stamp-changelog.sh derives script_dir from dirname("$0"), which can be "." or otherwise CWD-relative
when invoked via PATH or from a different working directory. This can make it fail to find the
repo’s parse-version.sh (or execute an unintended ./parse-version.sh), undermining the version
normalization the rest of the script relies on.
Code

scripts/release/stamp-changelog.sh[R35-38]

+script_dir="$(dirname "$0")"
+if ! tag="$("$script_dir/parse-version.sh" "$1")"; then
+	exit 1
+fi
Evidence
The new script resolves parse-version.sh relative to dirname "$0", which is not a stable way to
locate sibling scripts across different invocation styles. Existing repo scripts demonstrate the
intended robust pattern by canonicalizing the script directory with cd+pwd and clearing
CDPATH.

scripts/release/stamp-changelog.sh[30-38]
scripts/release/build-artifacts.sh[21-22]
scripts/benchmark/run.sh[8-9]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`scripts/release/stamp-changelog.sh` computes `script_dir` using `dirname "$0"` and then executes `"$script_dir/parse-version.sh"`. When the script is invoked from `PATH` (where `$0` may be just `stamp-changelog.sh`) or from outside the repo using a relative path, `script_dir` can resolve to `.` or to a directory relative to the caller’s CWD, causing the script to:
- fail to locate the intended `parse-version.sh`, or
- run an unintended `./parse-version.sh` from the current directory.

This breaks manual usage and weakens the guarantee that the tag is normalized/sanitized before being used as a tag argument and in regex construction.

### Issue Context
Other scripts in this repo already use a more robust pattern to canonicalize the script directory using `cd`+`pwd` with `CDPATH` cleared.

### Fix Focus Areas
- scripts/release/stamp-changelog.sh[30-39]

### Implementation guidance
- Resolve the script path first, handling both “has slash” and “PATH invocation” cases, then canonicalize to an absolute directory:
 - If `$0` contains no `/`, get the real path via `command -v -- "$0"`.
 - Use `script_dir="$(CDPATH='' cd -- "$(dirname -- "$script_path")" && pwd)"`.
- Then call `"$script_dir/parse-version.sh"`.
- (Optional) Add a regression test that runs the script from a different working directory (e.g., `cd "$work"`) while invoking it via an absolute path or via PATH to ensure it still finds the correct helper.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

Comment thread tests/test-stamp-changelog.sh
Comment thread scripts/release/stamp-changelog.sh
Comment thread scripts/release/stamp-changelog.sh
Repository owner deleted a comment from github-actions Bot Aug 7, 2026
Repository owner deleted a comment from chatgpt-codex-connector Bot Aug 7, 2026
The pending version reaches git-cliff as a tag and the section guard
as a grep pattern, but stamp-changelog.sh trusted whatever it was
handed. The release workflow validates first, so this only bites a
standalone run -- which the usage line invites.

Normalize through parse-version.sh, the same strict parser the three
release workflows already share, and escape the dots that survive it
so `## [1.2.3]` no longer also matches `## [1x2x3]`.

Reported by Qodo.
@michen00

michen00 commented Aug 7, 2026

Copy link
Copy Markdown
Owner Author

/agentic_review

@michen00

michen00 commented Aug 7, 2026

Copy link
Copy Markdown
Owner Author

[Review-Convergence] Round 1: active

  • Head: abeb632
  • Base ref: main
  • Base: e4fe9d2
  • CI: passing (11 success, 1 skipped)
  • Bot threads: 0 unresolved
  • Human threads: 0 unresolved
  • Clean signals: none yet (target 1, Qodo)
  • Pending reviewers: Qodo
  • Catch-up: none
  • Next action: wait for a Qodo real-review marker naming abeb632
  • Next wakeup: none — single-shot mode, re-invoke /converge-pr-reviews for round 2
  • Bulletin: Review convergence: PR #69 #70
Round 1 dispositions — 3 Qodo findings, all thread-backed
# Finding Disposition
1 Regex version match fixed in abeb632 — tag normalized through parse-version.sh, surviving dots escaped, new test case
2 Non-atomic changelog write wontfix — the line above records the trade as deliberate (permissions + inode); the write is validation-gated and the file is git-recoverable
3 test-stamp-changelog.sh uses bash shebang wontfixtests/ is bash by convention and by tooling; the file reads ${BASH_SOURCE[0]}, so /bin/sh would break it

All three threads carry a reply and are resolved. Qodo has since struck all three in its persistent summary (✓ Resolved, ✗ Dismissed, ✗ Dismissed) and now reports Bugs (0) / Rule violations (0).

Why Qodo still counts as pending. Its summary updated_at advanced, but that is bookkeeping from the thread resolutions rather than a review pass, and no real-review marker names abeb632. A fresh /agentic_review was posted to earn a markered pass on the current head.

Comment thread scripts/release/stamp-changelog.sh
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit abeb632

Normalizing the tag through parse-version.sh made locating that helper
load-bearing, and nothing covered where the lookup happens. Invoke
through PATH from an unrelated directory with a decoy parse-version.sh
planted in it, and assert the real sibling still wins.

Prompted by a Qodo finding that read `dirname "$0"` as CWD-relative
under PATH invocation. It is not -- the shell resolves through PATH and
execs the absolute path, so "$0" already carries the real directory --
but the property is worth pinning now that it matters.
@michen00

michen00 commented Aug 7, 2026

Copy link
Copy Markdown
Owner Author

/agentic_review

@michen00

michen00 commented Aug 7, 2026

Copy link
Copy Markdown
Owner Author

[Review-Convergence] Round 2: active

  • Head: ba08193
  • Base ref: main
  • Base: e4fe9d2
  • CI: pending (8 pass, 4 running, 1 skipped)
  • Bot threads: 0 unresolved
  • Human threads: 0 unresolved
  • Clean signals: none yet (target 1, Qodo)
  • Pending reviewers: Qodo
  • Catch-up: none
  • Next action: wait for CI and a Qodo real-review marker naming ba08193
  • Bulletin: Review convergence: PR #69 #70
Round 2 disposition — 1 new Qodo finding on the round-1 fix

Qodo's pass on abeb632 flagged dirname "$0" as CWD-relative under a PATH invocation, which would let the script miss parse-version.sh or run an unintended one.

Declined on measured evidence. The repo-convention half was accurate — build-artifacts.sh:21 and benchmark/run.sh:8 do use CDPATH='' cd -- … && pwd. The $0 premise was not: under a PATH invocation the shell execves the resolved absolute path, which the kernel hands to the interpreter, so $0 already carries the real directory. Only sh script.sh from the script's own directory yields a bare $0, where . is already correct.

I implemented the suggested fix first and tested it against the decoy case before and after — identical results, which is what prompted checking the premise. Reverted rather than carry an unreachable branch, which also keeps this consistent with bump-pins.sh:42.

Kept: a test in ba08193 that invokes through PATH from an unrelated directory with a decoy parse-version.sh, asserting the real sibling wins. That lookup only became load-bearing in round 1.

Comment thread tests/test-stamp-changelog.sh Outdated
Comment thread tests/test-stamp-changelog.sh Outdated
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit ba08193

The decoy test put the real scripts/release on PATH so the script could
be named bare, which meant a regression to a bare `parse-version.sh`
would still find the real helper there and the test would pass anyway.
It only ever caught the `./` shape.

Invoke through $STAMP by absolute path and put the decoy directory on
PATH instead. Now a bare lookup and a `./` lookup both land on the
decoy, and only a script-relative one wins. Verified by mutating the
script both ways: each fails, baseline passes.

Dropping the release_dir line also drops its unchecked command
substitution, which the script's `set -uo pipefail` would not have
caught.

Reported by Qodo.
@michen00

michen00 commented Aug 7, 2026

Copy link
Copy Markdown
Owner Author

/agentic_review

@michen00

michen00 commented Aug 7, 2026

Copy link
Copy Markdown
Owner Author

[Review-Convergence] Round 3: active

  • Head: 86b964b
  • Base ref: main
  • Base: e4fe9d2
  • CI: pending (7 pass, 5 running, 1 skipped)
  • Bot threads: 0 unresolved
  • Human threads: 0 unresolved
  • Clean signals: none yet (target 1, Qodo)
  • Pending reviewers: Qodo
  • Catch-up: none
  • Next action: wait for CI and a Qodo real-review marker naming 86b964b
  • Bulletin: Review convergence: PR #69 #70
Round 3 dispositions — 2 new Qodo findings, both fixed

Qodo's pass on ba08193 returned a marker and two new findings, both against the test added in round 2.

Helper lookup test masked — fixed. The better catch of the two. The test put the real scripts/release on PATH so the script could be named bare, which meant a regression to a bare parse-version.sh would still find the real helper there. The decoy only bit the ./ shape. Now invokes "$STAMP" by absolute path with $decoy_dir on PATH instead. Verified by mutation:

Resolves via Result
"$script_dir/parse-version.sh" (baseline) passes
parse-version.sh (bare, PATH) fails
./parse-version.sh (CWD) fails

The bare-PATH row passed before this change.

Unchecked release_dir assignment — fixed by deletion. Accurate: the suite runs set -uo pipefail with no set -e, so a failed cd would have left the variable empty. The masked-decoy fix removed the line's only use, so the command substitution went with it.

Both findings were on runner-authored test code, which is worth naming: the round-2 test inherited its PATH framing from a round-2 finding whose premise turned out to be wrong. Qodo's read of what it actually proved was correct.

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 86b964b

@michen00

michen00 commented Aug 7, 2026

Copy link
Copy Markdown
Owner Author

[Review-Convergence] Round 4: converged

  • Head: 86b964b
  • Base ref: main
  • Base: e4fe9d2
  • CI: green (12 pass, 1 skipped)
  • Bot threads: 0 unresolved
  • Human threads: 0 unresolved
  • Clean signals: Qodo (qodo family) at 86b964b — 1/1, target met
  • Pending reviewers: none
  • Catch-up: none (base is an ancestor of head)
  • Merge gates: MERGEABLE / CLEAN, not draft, no blocking labels
  • Bulletin: Review convergence: PR #69 #70

Qodo's pass on 86b964b posted a real-review marker, cleared its summary to Bugs (0) / Rule violations (0) / Skill insights (0), and left no unresolved threads. Nothing further is pending.

Four rounds, six findings — full disposition
Round Finding Disposition
1 Regex version match fixedabeb632
1 Non-atomic changelog write wontfix — documented deliberate trade at stamp-changelog.sh:75-77
1 test-stamp-changelog.sh uses bash shebang wontfixtests/ is bash by convention and by tooling
2 Wrong helper script resolved wontfix$0 premise disproven by measurement
3 Helper lookup test masked fixed86b964b
3 Unchecked release_dir assignment fixed86b964b, by deletion

Three commits landed: abeb632 (validate the tag), ba08193 (pin helper lookup), 86b964b (make the decoy actually bite).

Rounds 2 and 3 were spent on code this runner introduced rather than on the original change. Round 3's findings were both correct — the round-2 test claimed more than it proved, and Qodo caught it.

This runner does not merge, approve, or resolve human threads. Merge policy is yours — note the repo is squash-merge only.

@michen00
michen00 merged commit 03c6133 into main Aug 8, 2026
13 checks passed
@michen00
michen00 deleted the fix/stamp-changelog-version branch August 8, 2026 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review convergence: PR #69

1 participant