From 683d5d94fc26aa33edc7f0b617ff5475bd3ee238 Mon Sep 17 00:00:00 2001 From: Samuel Galanakis Date: Sat, 25 Jul 2026 12:15:53 +0200 Subject: [PATCH] Fix release note extraction and PR validation Collect every bounded note from squash-merge bodies, group notes by category, and validate categorized PR notes in the existing lint job with an exact step-summary preview. Release-Notes: Internal: Release tooling now bounds squash-merge notes, previews grouped output, and validates categorized entries before merge. --- .github/workflows/ci.yml | 10 + scripts/release_notes.py | 247 ++++++++++++++++---- scripts/test_confidence_gate_ci_contract.py | 10 + scripts/test_release_notes.py | 205 ++++++++++++++-- 4 files changed, 416 insertions(+), 56 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e88c2153..00d89fbf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -186,6 +186,16 @@ jobs: steps: - name: Check out repository uses: actions/checkout@v7 + with: + fetch-depth: 0 + + - name: Check and preview PR release notes + if: github.event_name == 'pull_request' + run: | + merge_base="$(git merge-base "origin/${{ github.base_ref }}" HEAD)" + python3 scripts/release_notes.py check-pr \ + --range "${merge_base}..HEAD" \ + --summary "$GITHUB_STEP_SUMMARY" - name: Install Rust toolchain uses: dtolnay/rust-toolchain@stable diff --git a/scripts/release_notes.py b/scripts/release_notes.py index 99439931..43936930 100644 --- a/scripts/release_notes.py +++ b/scripts/release_notes.py @@ -1,13 +1,20 @@ #!/usr/bin/env python3 -"""Collect curated release notes from commit messages. +"""Collect and validate curated release notes from commit messages. -Convention: a commit that should contribute user-facing release notes carries a -`Release-Notes:` line in its body; everything after that line (to the end of -the message) is the note, written as Markdown. Notes are collected across the -release range (previous release tag → the release ref), oldest first, so the -release body reads chronologically. The publish-time version-injection flow -authors no synthetic commits, so every -commit in range is a real change eligible to contribute notes. +Convention: a commit that should contribute release notes carries one or more +`Release-Notes:` lines in its body. A note may start on the marker line or the +following line. It ends at the next `Release-Notes:` line, the next squash +subject line (`* `), or the end of the message. + +Notes may start with `Breaking:`, `Added:`, `Fixed:`, `Changed:`, or +`Internal:` (case-insensitive). The legacy `Fixed - ` and `Changed - ` forms +remain accepted. Publication groups categorized notes in that order, followed +by uncategorized historical notes under `Other`. + +The `check-pr` command requires a categorized note when a range changes +anything under `crates/`, and rejects uncategorized notes in every PR. Use an +`Internal:` note, with a short explanation, as the escape hatch for a product +change that intentionally has no public-facing release note. The release pipeline uses two entry points: - the manually dispatched release workflow runs `collect --require` for the @@ -22,15 +29,25 @@ from __future__ import annotations import argparse +import re import subprocess import sys from pathlib import Path ROOT = Path(__file__).resolve().parents[1] -MARKER = "Release-Notes:" +MARKER_RE = re.compile(r"^Release-Notes:(.*)$") +SQUASH_SUBJECT_RE = re.compile(r"^\*\s") RECORD_SEPARATOR = "\x1e" -FIELD_SEPARATOR = "\x1f" +CATEGORIES = ("Breaking", "Added", "Fixed", "Changed", "Internal") +CATEGORY_COLON_RE = re.compile( + r"^(Breaking|Added|Fixed|Changed|Internal):\s*(.*)$", re.IGNORECASE +) +LEGACY_CATEGORY_RE = re.compile(r"^(Fixed|Changed)\s+-\s+(.*)$", re.IGNORECASE) +MALFORMED_CATEGORY_ERROR = ( + "every release note must start with one of: " + "Breaking:, Added:, Fixed:, Changed:, Internal:" +) def previous_tag(end: str) -> str | None: @@ -67,31 +84,78 @@ def end_is_released(end: str) -> bool: return result.returncode == 0 and bool(result.stdout.strip()) -def extract_note(body: str) -> str | None: - """The Markdown after a standalone `Release-Notes:` line, or None.""" +def extract_notes(body: str) -> list[str]: + """Extract every bounded release-note block from one commit body.""" lines = body.splitlines() - for index, line in enumerate(lines): - if line.strip() == MARKER: - note = "\n".join(lines[index + 1 :]).strip() - return note or None - # Also accept inline form: "Release-Notes: text on the same line". - if line.strip().startswith(MARKER): - first = line.strip()[len(MARKER) :].strip() - rest = "\n".join(lines[index + 1 :]).strip() - note = "\n".join(part for part in (first, rest) if part).strip() - return note or None - return None + notes: list[str] = [] + index = 0 + while index < len(lines): + marker = MARKER_RE.match(lines[index]) + if marker is None: + index += 1 + continue + note_lines: list[str] = [] + inline = marker.group(1).strip() + if inline: + note_lines.append(inline) + index += 1 -def collect_notes(end: str) -> list[str]: - prev = previous_tag(end) - range_spec = f"{prev}..{end}" if prev else end + while index < len(lines): + line = lines[index] + if MARKER_RE.match(line) or SQUASH_SUBJECT_RE.match(line): + break + note_lines.append(line) + index += 1 + + note = "\n".join(note_lines).strip() + if note: + notes.append(note) + + return notes + + +def categorize_note(note: str) -> tuple[str | None, str]: + """Return the normalized category and note text without its prefix.""" + first_line, separator, remainder = note.partition("\n") + match = CATEGORY_COLON_RE.match(first_line) + if match is None: + match = LEGACY_CATEGORY_RE.match(first_line) + if match is None: + return None, note + + category = match.group(1).capitalize() + first_text = match.group(2).strip() + text = "\n".join( + part for part in (first_text, remainder if separator else "") if part + ).strip() + return category, text + + +def render_notes(notes: list[str]) -> str: + """Group notes by category and render the publication Markdown.""" + grouped: dict[str, list[str]] = {category: [] for category in CATEGORIES} + grouped["Other"] = [] + for note in notes: + category, text = categorize_note(note) + grouped[category or "Other"].append(text) + + sections: list[str] = [] + for category in (*CATEGORIES, "Other"): + category_notes = grouped[category] + if category_notes: + sections.append(f"## {category}\n\n" + "\n\n".join(category_notes)) + return "\n\n".join(sections).strip() + + +def commit_bodies(range_spec: str) -> list[str]: + """Return commit bodies in chronological order for an arbitrary range.""" result = subprocess.run( [ "git", "log", "--reverse", - f"--format=%H{FIELD_SEPARATOR}%s{FIELD_SEPARATOR}%B{RECORD_SEPARATOR}", + f"--format=%B{RECORD_SEPARATOR}", range_spec, ], cwd=ROOT, @@ -99,24 +163,94 @@ def collect_notes(end: str) -> list[str]: capture_output=True, text=True, ) + return [ + record.strip("\n") + for record in result.stdout.split(RECORD_SEPARATOR) + if record.strip() + ] + + +def collect_notes_for_range(range_spec: str) -> list[str]: + """Collect all notes from an arbitrary git range, oldest first.""" notes: list[str] = [] - for record in result.stdout.split(RECORD_SEPARATOR): - record = record.strip("\n") - if not record.strip(): - continue - parts = record.split(FIELD_SEPARATOR) - if len(parts) != 3: - continue - _sha, _subject, body = parts - note = extract_note(body) - if note: - notes.append(note) + for body in commit_bodies(range_spec): + notes.extend(extract_notes(body)) return notes +def collect_notes(end: str) -> list[str]: + """Collect notes for the previous-release-tag-to-end range.""" + prev = previous_tag(end) + range_spec = f"{prev}..{end}" if prev else end + return collect_notes_for_range(range_spec) + + +def changed_paths(range_spec: str) -> list[str]: + """Return paths changed in an arbitrary git range.""" + result = subprocess.run( + ["git", "diff", "--name-only", "--diff-filter=ACDMRTUXB", range_spec], + cwd=ROOT, + check=True, + capture_output=True, + text=True, + ) + return [line for line in result.stdout.splitlines() if line] + + +def validate_pr_notes(paths: list[str], notes: list[str]) -> list[str]: + """Validate the product-note requirement and every note's category.""" + errors: list[str] = [] + if any(path.startswith("crates/") for path in paths) and not notes: + errors.append( + "product changes under `crates/` require at least one categorized " + "release note" + ) + if any( + category is None or not text + for category, text in (categorize_note(note) for note in notes) + ): + errors.append(MALFORMED_CATEGORY_ERROR) + return errors + + +def marker_count(bodies: list[str]) -> int: + """Count marker lines, including empty or otherwise malformed notes.""" + return sum( + 1 + for body in bodies + for line in body.splitlines() + if MARKER_RE.match(line) is not None + ) + + +def write_pr_summary(path: Path, rendered: str) -> None: + """Append the exact PR release-note preview to a GitHub step summary.""" + preview = rendered or "No release notes in this PR." + with path.open("a", encoding="utf-8") as summary: + summary.write(f"## Release notes preview (this PR)\n\n{preview}\n") + + +def write_check_pr_failure(range_spec: str, errors: list[str]) -> None: + """Write an actionable category-and-shape guide for a failed PR check.""" + sys.stderr.write(f"release notes check failed for {range_spec}:\n") + for error in errors: + sys.stderr.write(f"- {error}\n") + sys.stderr.write( + "\nAdd a categorized note to a commit body in this PR.\n" + "Accepted categories: Breaking, Added, Fixed, Changed, Internal.\n" + "Use Internal for a product change with no public-facing release note.\n" + "\nAccepted inline shape:\n" + "Release-Notes: Fixed: Describe the user-visible change.\n" + "\nAccepted block shape:\n" + "Release-Notes:\n" + "Fixed: Describe the user-visible change.\n" + ) + + def main() -> int: parser = argparse.ArgumentParser(description=__doc__) sub = parser.add_subparsers(dest="command", required=True) + collect = sub.add_parser("collect", help="collect notes for a release range") collect.add_argument( "--end", @@ -133,10 +267,42 @@ def main() -> int: action="store_true", help="fail when the range contains no Release-Notes sections", ) + + check_pr = sub.add_parser( + "check-pr", help="validate and preview notes for an arbitrary PR range" + ) + check_pr.add_argument( + "--range", + dest="range_spec", + required=True, + help="git range from the PR merge base through its head", + ) + check_pr.add_argument( + "--summary", + type=Path, + help="append the rendered preview to this GitHub step-summary file", + ) + args = parser.parse_args() + if args.command == "check-pr": + bodies = commit_bodies(args.range_spec) + notes = [note for body in bodies for note in extract_notes(body)] + rendered = render_notes(notes) + if args.summary: + write_pr_summary(args.summary, rendered) + errors = validate_pr_notes(changed_paths(args.range_spec), notes) + if marker_count(bodies) != len(notes) and MALFORMED_CATEGORY_ERROR not in errors: + errors.append(MALFORMED_CATEGORY_ERROR) + if errors: + write_check_pr_failure(args.range_spec, errors) + return 2 + if rendered and not args.summary: + sys.stdout.write(rendered + "\n") + return 0 + notes = collect_notes(args.end) - rendered = "\n\n".join(notes).strip() + rendered = render_notes(notes) if args.out: args.out.parent.mkdir(parents=True, exist_ok=True) args.out.write_text(rendered + "\n" if rendered else "", encoding="utf-8") @@ -148,8 +314,7 @@ def main() -> int: sys.stderr.write( "no release notes found in " f"{prev or 'history start'}..{args.end}.\n" - "Add a `Release-Notes:` section to at least one commit body\n" - "(Markdown, everything after the marker line).\n" + "Add a `Release-Notes:` section to at least one commit body.\n" ) return 2 if rendered and not args.out: diff --git a/scripts/test_confidence_gate_ci_contract.py b/scripts/test_confidence_gate_ci_contract.py index 6e214702..81757cba 100644 --- a/scripts/test_confidence_gate_ci_contract.py +++ b/scripts/test_confidence_gate_ci_contract.py @@ -184,6 +184,16 @@ def test_lint_job_runs_clippy_fmt_and_boundary_guards(self) -> None: self.assertIn("bash scripts/check-workflow-graph-model.sh", lint) self.assertIn("bash scripts/check-production-file-size.sh", lint) + def test_lint_job_checks_and_previews_pr_release_notes(self) -> None: + workflow = WORKFLOW.read_text(encoding="utf-8") + lint = workflow_job_block(workflow, "lint") + + self.assertIn("fetch-depth: 0", lint) + self.assertIn("if: github.event_name == 'pull_request'", lint) + self.assertIn('git merge-base "origin/${{ github.base_ref }}" HEAD', lint) + self.assertIn("python3 scripts/release_notes.py check-pr", lint) + self.assertIn('--summary "$GITHUB_STEP_SUMMARY"', lint) + def test_workflow_graph_example_is_in_functional_matrix(self) -> None: workflow = WORKFLOW.read_text(encoding="utf-8") justfile = JUSTFILE.read_text(encoding="utf-8") diff --git a/scripts/test_release_notes.py b/scripts/test_release_notes.py index 40a94703..ca3118c4 100644 --- a/scripts/test_release_notes.py +++ b/scripts/test_release_notes.py @@ -3,32 +3,207 @@ from __future__ import annotations +import tempfile +from pathlib import Path + import release_notes -def test_extract_note_standalone_marker() -> None: - body = "Subject\n\nDetails here.\n\nRelease-Notes:\n- Added X\n- Fixed Y\n" - assert release_notes.extract_note(body) == "- Added X\n- Fixed Y" +ALPHA_112_SQUASH_BODY = """\ +Production recoverable-chat host reference and conformance gate (FIG-610) (#127) + +* Add recoverable chat observation contract + +Expose snapshot-first recovery updates with stable redelivery identity, replay-gap replacement, terminal replacement, and disconnect-safe subscription semantics. Add real observation API conformance gates for each contract edge. + +Release-Notes: +- Lash now exposes a recoverable_chat observation surface for snapshot-first chat hosts, including stable event identity, replay-gap snapshots, terminal replacement, and explicit observer-disconnect semantics. + +* Harden the workbench recoverable chat projection + +Split Lash observations from durable ordered product events, resume the remote observation cursor directly, deduplicate stable product and observation identities, and resynchronize on lag or terminal replacement. Retract provisional attempts, shield raw failures, consume typed turn-input application evidence, expose the authorization seam, and extend the real owner-restart gate. + +* Document recoverable chat host responsibilities + +Describe the separated Lash and product lanes, snapshot-first resumption, typed turn-input application evidence, projection replacement rules, same-turn recovery versus retry-copy semantics, uncertain tool effects, and the pluggable authorization seam. + +* Make recoverable chat handoff lossless + +Capture the read view and cursor from one installed runtime observation, clear subscription-scoped deduplication after replay discontinuity, and bound retained identities at terminal replacement. Add deterministic regressions for the snapshot publication race and replay-store cursor reuse. + +Release-Notes: +- Recoverable chat snapshots now pair the committed read view with its exact observation cursor, and replay-gap recovery discards pre-gap event identities before continuing. + +* Make workbench recovery convergent + +Fence asynchronous state recovery, scope cursors and deduplication to a session, merge canonical transcript history with product-only rows, and give every cancel settlement a unique product identity. Route observation through the recoverable-chat seam and keep internal diagnostics out of HTTP responses. +Replace projection string checks with route, turn-output, provider-failure, and executable browser-reducer gates, including production-handler mutation proof. +""" -def test_extract_note_inline_marker() -> None: - body = "Subject\n\nRelease-Notes: Single line note.\n" - assert release_notes.extract_note(body) == "Single line note." +def extracted(body: str) -> list[str]: + """Use the new API, with a pre-fix fallback for regression verification.""" + if hasattr(release_notes, "extract_notes"): + return release_notes.extract_notes(body) + note = release_notes.extract_note(body) + return [note] if note else [] -def test_extract_note_inline_marker_with_continuation() -> None: - body = "Subject\n\nRelease-Notes: First line.\nSecond line.\n" - assert release_notes.extract_note(body) == "First line.\nSecond line." +def test_extract_notes_block_form_preserves_markdown() -> None: + body = ( + "Subject\n\nRelease-Notes:\n" + "Fixed: First paragraph.\n\n- Detail one\n- Detail two\n" + "* Squash subject\n\nInternal prose.\n" + ) + assert extracted(body) == [ + "Fixed: First paragraph.\n\n- Detail one\n- Detail two" + ] -def test_extract_note_absent_or_empty() -> None: - assert release_notes.extract_note("Subject\n\nNo marker here.\n") is None - assert release_notes.extract_note("Subject\n\nRelease-Notes:\n\n") is None + +def test_extract_notes_inline_form_stops_at_squash_subject() -> None: + body = ( + "Subject\n\n" + "Release-Notes: Added: Single line note.\n" + "* Squash subject\n\nInternal prose.\n" + ) + assert extracted(body) == ["Added: Single line note."] + + +def test_extract_notes_multiple_markers_preserve_order() -> None: + body = ( + "* First change\n\n" + "Release-Notes: Fixed: First public note.\n" + "* Second change\n\nImplementation details.\n\n" + "Release-Notes:\nChanged: Second public note.\n\n- Detail.\n" + "* Third change\n\nMore implementation details.\n" + ) + assert extracted(body) == [ + "Fixed: First public note.", + "Changed: Second public note.\n\n- Detail.", + ] + + +def test_alpha_112_squash_fixture_extracts_only_both_notes() -> None: + assert extracted(ALPHA_112_SQUASH_BODY) == [ + ( + "- Lash now exposes a recoverable_chat observation surface for " + "snapshot-first chat hosts, including stable event identity, " + "replay-gap snapshots, terminal replacement, and explicit " + "observer-disconnect semantics." + ), + ( + "- Recoverable chat snapshots now pair the committed read view with " + "its exact observation cursor, and replay-gap recovery discards " + "pre-gap event identities before continuing." + ), + ] + + +def test_extract_notes_absent_or_empty() -> None: + assert extracted("Subject\n\nNo marker here.\n") == [] + assert extracted("Subject\n\nRelease-Notes:\n\n") == [] def test_marker_must_start_the_line() -> None: body = "Subject\n\nSee the Release-Notes: convention for details.\n" - assert release_notes.extract_note(body) is None + assert extracted(body) == [] + + +def test_render_notes_groups_categories_in_publication_order() -> None: + notes = [ + "fixed: Repair the first issue.", + "Added: Add the new surface.", + "BREAKING: Replace the old API.", + "Internal: Exercise the release-note gate.", + "Changed: Adjust the behavior.", + "Fixed: Repair the second issue.", + ] + assert release_notes.render_notes(notes) == """\ +## Breaking + +Replace the old API. + +## Added + +Add the new surface. + +## Fixed + +Repair the first issue. + +Repair the second issue. + +## Changed + +Adjust the behavior. + +## Internal + +Exercise the release-note gate.""" + + +def test_render_notes_accepts_legacy_dash_categories() -> None: + notes = [ + "Fixed - repair legacy behavior.", + "Changed - preserve historical syntax.", + ] + assert release_notes.render_notes(notes) == """\ +## Fixed + +repair legacy behavior. + +## Changed + +preserve historical syntax.""" + + +def test_render_notes_keeps_uncategorized_notes_under_other() -> None: + notes = ["A historical uncategorized note."] + assert release_notes.render_notes(notes) == """\ +## Other + +A historical uncategorized note.""" + + +def test_pr_rule_requires_note_for_product_changes() -> None: + errors = release_notes.validate_pr_notes(["crates/lash/src/lib.rs"], []) + assert errors == [ + "product changes under `crates/` require at least one categorized " + "release note" + ] + + +def test_pr_rule_rejects_uncategorized_notes_even_when_exempt() -> None: + errors = release_notes.validate_pr_notes( + ["scripts/release_notes.py"], ["An uncategorized note."] + ) + assert errors == [ + "every release note must start with one of: Breaking:, Added:, Fixed:, " + "Changed:, Internal:" + ] + + +def test_internal_category_is_the_no_public_note_escape_hatch() -> None: + errors = release_notes.validate_pr_notes( + ["crates/lash/src/lib.rs"], + ["Internal: This product change has no public-facing effect."], + ) + assert errors == [] + + +def test_exempt_pr_without_notes_is_valid() -> None: + assert release_notes.validate_pr_notes(["scripts/release_notes.py"], []) == [] + + +def test_pr_summary_is_clear_when_exempt_pr_has_no_notes() -> None: + with tempfile.TemporaryDirectory() as directory: + summary = Path(directory) / "summary.md" + release_notes.write_pr_summary(summary, "") + assert summary.read_text(encoding="utf-8") == ( + "## Release notes preview (this PR)\n\n" + "No release notes in this PR.\n" + ) def main() -> int: @@ -38,9 +213,9 @@ def main() -> int: try: test() print(f"ok {name}") - except AssertionError as err: + except Exception as err: # noqa: BLE001 - tiny dependency-free harness failures += 1 - print(f"FAIL {name}: {err}") + print(f"FAIL {name}: {type(err).__name__}: {err}") return 1 if failures else 0