From 8114412d3018a7e110e57d2be02b3faaa05385c6 Mon Sep 17 00:00:00 2001 From: nelsonduarte Date: Sat, 12 Sep 2026 21:42:30 +0100 Subject: [PATCH 1/5] fix: stop publishing development noise and dropping security notes in release notes The release body generated by build.yml is not an internal artefact: the in-app updater downloads it and shows it to end users. Two defects in the old inline-bash generator were visible there. Development noise was published verbatim. Any commit type the old sed did not know (it handled only feat|fix|perf|a11y|docs) fell through to the catch-all "## Other" section with its raw prefix intact, so users read lines such as "Test: rename unused QApplication holder to satisfy CodeQL" and "Refactor(window): extract auto-update subsystem into UpdateController". Development-only types are now dropped instead of being dumped into a catch-all. Security notes were silently deleted. The dependabot heuristic 'bump.*from.*to' was unanchored and applied to every subject, including hand-written ones, so a commit like "security(deps): bump pillow from 12.2.0 to 12.3.0" was discarded without a trace. This was a near miss in practice: the real commit "fix: pin cryptography for the Flatpak and bump pypdf past six CVEs", already on main, survived only because its wording happens not to contain " from ... to ". The noise heuristic is now anchored and is applied only to subjects that carry no recognised type, so a hand-written security note can no longer be swallowed. Security changes also get their own "## Security" section, ordered first because for a PDF tool it is the change users most need to see. The heading has an update.section.security key in all 8 languages of app/translations.json, matching the existing section headings. The categorisation moved out of inline bash in the workflow YAML and into scripts/release_notes.py, which is why it was never testable before. The module is stdlib-only on purpose: the release job has no setup-python step and runs it with the runner's system interpreter. Routing is driven by a single _TYPE_DESTINATION table mapping each commit type to a heading or to None. The earlier two-structure version (a drop-set plus a routing map) grew a branch that could never be reached, because a type in the drop-set was re-tested for a scope further down. With one entry per type that class of bug is not expressible. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/build.yml | 56 +-- app/translations.json | 8 + app/updater.py | 6 +- scripts/release_notes.py | 225 ++++++++++++ tests/test_release_notes_generator.py | 476 ++++++++++++++++++++++++++ 5 files changed, 720 insertions(+), 51 deletions(-) create mode 100644 scripts/release_notes.py create mode 100644 tests/test_release_notes_generator.py diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3780e19..e8be884 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -234,57 +234,13 @@ jobs: echo "Generating notes from $PREV to $TAG" - # Collect conventional commit messages, skip chore/ci/deps/merge - FEATURES="" - FIXES="" - PERF="" - OTHER="" + # Categorisation lives in scripts/release_notes.py so it can be + # unit-tested; this body is published verbatim to end users by + # the in-app updater, so development-only commit types (test, + # refactor, chore, ci, build, docs, style) are dropped there. + # stdlib only: this job has no setup-python step. + python3 scripts/release_notes.py "${PREV}..${TAG}" -o release_notes.md - while IFS= read -r line; do - # Skip empty, merge, chore, ci, deps, bump, dependabot - echo "$line" | grep -qiE '^(chore|ci|build|Merge|docs\(deps\))' && continue - echo "$line" | grep -qiE 'dependabot|bump.*from.*to' && continue - echo "$line" | grep -qi '^$' && continue - - # Clean up: remove scope prefix for display, capitalize - clean=$(echo "$line" | sed -E 's/^(feat|fix|perf|a11y|docs)(\([^)]*\))?:\s*//') - clean="$(echo "${clean:0:1}" | tr '[:lower:]' '[:upper:]')${clean:1}" - - if echo "$line" | grep -qE '^feat'; then - FEATURES="${FEATURES}- ${clean}\n" - elif echo "$line" | grep -qE '^fix'; then - FIXES="${FIXES}- ${clean}\n" - elif echo "$line" | grep -qE '^perf'; then - PERF="${PERF}- ${clean}\n" - elif echo "$line" | grep -qE '^a11y'; then - FIXES="${FIXES}- ${clean}\n" - elif echo "$line" | grep -qE '^docs'; then - : # skip docs-only commits - else - OTHER="${OTHER}- ${clean}\n" - fi - done <<< "$(git log --pretty=format:'%s' "${PREV}..${TAG}" --no-merges)" - - BODY="" - if [ -n "$FEATURES" ]; then - BODY="${BODY}## New features\n${FEATURES}\n" - fi - if [ -n "$PERF" ]; then - BODY="${BODY}## Performance\n${PERF}\n" - fi - if [ -n "$FIXES" ]; then - BODY="${BODY}## Fixes & improvements\n${FIXES}\n" - fi - if [ -n "$OTHER" ]; then - BODY="${BODY}## Other\n${OTHER}\n" - fi - - if [ -z "$BODY" ]; then - BODY="Bug fixes and improvements." - fi - - # Write to file (multiline output) - printf '%b' "$BODY" > release_notes.md echo "Generated release notes:" cat release_notes.md diff --git a/app/translations.json b/app/translations.json index 0ccfa4c..21fdfc6 100644 --- a/app/translations.json +++ b/app/translations.json @@ -424,6 +424,7 @@ "viewer.night_mode": "Night reading mode (invert colors)", "update.changes": "What's new in this version:", "update.no_notes": "No release notes available.", + "update.section.security": "## Security", "update.section.features": "## New features", "update.section.performance": "## Performance", "update.section.fixes": "## Fixes & improvements", @@ -1036,6 +1037,7 @@ "viewer.night_mode": "Modo noturno (inverter cores)", "update.changes": "Novidades nesta versão:", "update.no_notes": "Sem notas de versão disponíveis.", + "update.section.security": "## Segurança", "update.section.features": "## Novidades", "update.section.performance": "## Desempenho", "update.section.fixes": "## Correções e melhorias", @@ -1648,6 +1650,7 @@ "viewer.night_mode": "Modo nocturno (invertir colores)", "update.changes": "Novedades en esta versión:", "update.no_notes": "Sin notas de versión disponibles.", + "update.section.security": "## Seguridad", "update.section.features": "## Novedades", "update.section.performance": "## Rendimiento", "update.section.fixes": "## Correcciones y mejoras", @@ -2260,6 +2263,7 @@ "viewer.night_mode": "Mode nuit (inverser les couleurs)", "update.changes": "Nouveautés de cette version :", "update.no_notes": "Aucune note de version disponible.", + "update.section.security": "## Sécurité", "update.section.features": "## Nouveautés", "update.section.performance": "## Performance", "update.section.fixes": "## Corrections et améliorations", @@ -2872,6 +2876,7 @@ "viewer.night_mode": "Nachtmodus (Farben invertieren)", "update.changes": "Neuigkeiten in dieser Version:", "update.no_notes": "Keine Versionshinweise verfügbar.", + "update.section.security": "## Sicherheit", "update.section.features": "## Neue Funktionen", "update.section.performance": "## Leistung", "update.section.fixes": "## Fehlerbehebungen und Verbesserungen", @@ -3484,6 +3489,7 @@ "viewer.night_mode": "夜间阅读模式（反转颜色）", "update.changes": "此版本的新功能：", "update.no_notes": "没有可用的版本说明。", + "update.section.security": "## 安全", "update.section.features": "## 新功能", "update.section.performance": "## 性能", "update.section.fixes": "## 修复与改进", @@ -4096,6 +4102,7 @@ "viewer.night_mode": "Modalità notturna (inverti colori)", "update.changes": "Novità in questa versione:", "update.no_notes": "Nessuna nota di versione disponibile.", + "update.section.security": "## Sicurezza", "update.section.features": "## Novità", "update.section.performance": "## Prestazioni", "update.section.fixes": "## Correzioni e miglioramenti", @@ -4708,6 +4715,7 @@ "viewer.night_mode": "Nachtmodus (kleuren inverteren)", "update.changes": "Wat is er nieuw in deze versie:", "update.no_notes": "Geen release notes beschikbaar.", + "update.section.security": "## Beveiliging", "update.section.features": "## Nieuwe functies", "update.section.performance": "## Prestaties", "update.section.fixes": "## Fixes en verbeteringen", diff --git a/app/updater.py b/app/updater.py index 26dd7eb..912e4bb 100644 --- a/app/updater.py +++ b/app/updater.py @@ -20,8 +20,12 @@ _API_URL = f"https://api.github.com/repos/{GITHUB_REPO}/releases/latest" -# Section headings used in auto-generated release notes (build.yml) +# Section headings used in auto-generated release notes, produced by +# scripts/release_notes.py. The match below is a plain string replace, +# so a heading added there without a key here (and in all 8 languages of +# app/translations.json) degrades to English silently. _SECTION_MAP = { + "## Security": "update.section.security", "## New features": "update.section.features", "## Performance": "update.section.performance", "## Fixes & improvements": "update.section.fixes", diff --git a/scripts/release_notes.py b/scripts/release_notes.py new file mode 100644 index 0000000..b71827c --- /dev/null +++ b/scripts/release_notes.py @@ -0,0 +1,225 @@ +#!/usr/bin/env python3 +"""Build the release-notes body from conventional-commit subjects. + +Used by the "Generate release notes" step of .github/workflows/build.yml. +The output is published as the GitHub release body, which the in-app +auto-updater downloads and shows to end users (app/updater.py: +_localize_notes translates the "## ..." headings). Anything emitted here +is read by users, not by reviewers, so development-only commit types are +dropped rather than dumped into a catch-all section. + +Stdlib only, on purpose: the release job has no setup-python step and +runs this with the runner's system interpreter. +""" + +from __future__ import annotations + +import argparse +import re +import subprocess +import sys + +# Heading text must stay byte-identical to the keys of +# app/updater.py:_SECTION_MAP, otherwise the in-app update dialog shows +# the English heading instead of the translated one (the replace is a +# plain string match, and a miss degrades silently). +HEADING_SECURITY = "## Security" +HEADING_FEATURES = "## New features" +HEADING_PERFORMANCE = "## Performance" +HEADING_FIXES = "## Fixes & improvements" +HEADING_OTHER = "## Other" + +FALLBACK_BODY = "Bug fixes and improvements." + +# Single source of truth for what each commit type does: a heading to +# publish under, or None to drop. One table instead of a drop-set plus a +# routing map, because the two-structure version grew a branch that +# could never run (a type in the drop-set was re-tested for a scope +# further down) and a test that passed through the wrong arm of the +# chain. Here a type has exactly one entry, so that class of bug cannot +# be expressed. +# +# Dropped types never reach end users: "test"/"refactor" describe +# internal churn ("extract pure dispatcher from TabEditar._run"); docs, +# chore, ci, build, style and seo are repo maintenance. "docs" is +# dropped unconditionally and deliberately: all 44 docs subjects in this +# repo's history are README/website/changelog upkeep, with no +# user-facing change among them. The nearest miss, "docs: document +# bookmarks/TOC and night reading mode", touches only CONTRIBUTING.md, +# README.md and TODO.txt and describes features that already publish +# their own notes via feat:, so publishing it would duplicate. +_TYPE_DESTINATION = { + "security": HEADING_SECURITY, + "feat": HEADING_FEATURES, + "perf": HEADING_PERFORMANCE, + "fix": HEADING_FIXES, + "a11y": HEADING_FIXES, + "i18n": HEADING_FIXES, + "design": HEADING_FIXES, + "chore": None, + "ci": None, + "build": None, + "docs": None, + "test": None, + "refactor": None, + "style": None, + "seo": None, +} + +# A recognised type is one the table knows about, whatever its verdict. +# An unrecognised type ("hotfix:") is treated as a fix rather than +# dropped, so a new prefix nobody registered here still reaches users. +_KNOWN_TYPES = frozenset(_TYPE_DESTINATION) + +# Section order in the published body. Security first: for a PDF tool it +# is the change users most need to see. +_SECTION_ORDER = ( + HEADING_SECURITY, + HEADING_FEATURES, + HEADING_PERFORMANCE, + HEADING_FIXES, + HEADING_OTHER, +) + +# "type(scope)!: subject"; scope and the breaking-change "!" are optional. +_CONVENTIONAL_RE = re.compile( + r"^(?P[A-Za-z0-9]+)(?:\((?P[^)]*)\))?!?:\s*(?P.*)$" +) + +# Machine-generated dependency subjects, matched only against subjects +# with no recognised type. Anchored on purpose: the previous +# "bump.*from.*to" was unanchored and substring-matched, so an ordinary +# sentence that happens to contain those three words ("Bump minimum +# zoom from 50 to 400 percent") was dropped in silence, which is the +# exact failure mode the "## Other" safety net exists to prevent. +# +# "^bumps? from " requires the single-token package/action name +# that dependabot always emits ("Bump actions/checkout from 4 to 5"), +# which prose with a multi-word object does not satisfy. The plural is +# accepted because "Bumps from X to Y" is the form dependabot uses +# in PR bodies and in squash-merge subjects. +# +# The bot alternative is anchored too: as a bare substring, "dependabot" +# swallowed ordinary prose that merely mentions it ("Move the dependabot +# config into .github"), the same silent-drop failure mode the "## Other" +# safety net exists to prevent. "\[bot\]" stays unanchored because it is +# a trailing account suffix, not a prefix. +# +# Deliberately NOT applied to a subject that already carries a +# user-facing type: a hand-written "security: bump pillow from 12.2.0 +# to 12.3.0" is precisely the note users must see. +_NOISE_RE = re.compile( + r"^(?:chore|build|ci)?\(?deps\)?:|^bumps?\s+\S+\s+from\s|^dependabot|\[bot\]", + re.IGNORECASE, +) +_MERGE_RE = re.compile(r"^(Merge|Revert)\b", re.IGNORECASE) + + +def classify(subject: str) -> str | None: + """Return the heading a commit subject belongs under, or None to drop. + + "## Other" is reserved for subjects with no recognised type: a + safety net for a commit whose prefix was forgotten, so a real + user-facing change is never silently dropped. A subject is only + dropped from that branch when it is recognisably machine-generated + (see _NOISE_RE), never merely for containing bump-like wording. A + *typed* commit never lands in "## Other", which is what used to + publish raw "Test:" / "Refactor(window):" prefixes to end users. + + Evaluation order is significant and each step is disjoint from the + next, so no step can shadow one below it: + + 1. structural rejects (empty, merge/revert) - never user-facing; + 2. recognised type -> whatever _TYPE_DESTINATION says, full stop. + A recognised type is decided by that table alone, so no scope or + wording heuristic can second-guess it (this is what keeps a + hand-written "security(deps): bump ..." published); + 3. everything else (unrecognised type, or no type at all) -> drop + if it looks machine-generated, otherwise "## Other". + """ + subject = subject.strip() + if not subject: + return None + if _MERGE_RE.match(subject): + return None + + match = _CONVENTIONAL_RE.match(subject) + ctype = (match.group("type") or "").lower() if match else "" + + if ctype in _KNOWN_TYPES: + return _TYPE_DESTINATION[ctype] + + if _NOISE_RE.search(subject): + return None + # An unrecognised *type* is still a deliberate prefix, so it is a + # fix rather than an unclassified "## Other" line. + return HEADING_FIXES if match else HEADING_OTHER + + +def clean_subject(subject: str) -> str: + """Strip any conventional-commit prefix and capitalise the first letter. + + The old sed only knew feat|fix|perf|a11y|docs, so every other type + reached users as "Test: ..." / "Refactor(window): ...". + """ + subject = subject.strip() + match = _CONVENTIONAL_RE.match(subject) + if match: + subject = match.group("subject").strip() + if not subject: + return subject + return subject[0].upper() + subject[1:] + + +def build_notes(subjects) -> str: + """Render the full release-notes body for an iterable of subjects.""" + sections: dict[str, list[str]] = {h: [] for h in _SECTION_ORDER} + + for subject in subjects: + heading = classify(subject) + if heading is None: + continue + entry = clean_subject(subject) + if not entry: + continue + bullet = f"- {entry}" + # Squash duplicate subjects (cherry-picks, reverted-then-redone + # work) so the same line is not published twice. + if bullet not in sections[heading]: + sections[heading].append(bullet) + + parts = [] + for heading in _SECTION_ORDER: + if sections[heading]: + parts.append(heading + "\n" + "\n".join(sections[heading]) + "\n") + + if not parts: + return FALLBACK_BODY + "\n" + return "\n".join(parts) + + +def _git_subjects(rev_range: str) -> list[str]: + out = subprocess.run( + ["git", "log", "--pretty=format:%s", "--no-merges", rev_range], + check=True, capture_output=True, text=True, encoding="utf-8", + ).stdout + return out.splitlines() + + +def main(argv=None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("rev_range", help="git revision range, e.g. v1.0.0..v1.1.0") + parser.add_argument("-o", "--output", help="write to this file instead of stdout") + args = parser.parse_args(argv) + + body = build_notes(_git_subjects(args.rev_range)) + if args.output: + with open(args.output, "w", encoding="utf-8", newline="\n") as fh: + fh.write(body) + else: + sys.stdout.write(body) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_release_notes_generator.py b/tests/test_release_notes_generator.py new file mode 100644 index 0000000..be2e74d --- /dev/null +++ b/tests/test_release_notes_generator.py @@ -0,0 +1,476 @@ +"""Tests for the release-notes generator (scripts/release_notes.py). + +Why this file exists: the generated body is not an internal artefact. +build.yml publishes it as the GitHub release body, and app/updater.py +downloads that body and shows it in the in-app update dialog. Before +this generator, the "## Other" section published raw development +commits to end users, prefix included ("Test: rename unused +QApplication holder to satisfy CodeQL", "Refactor(window): extract +auto-update subsystem into UpdateController"). + +The categorisation used to be bash inlined in the workflow YAML, which +is why it was never tested. It now lives in a stdlib-only Python module +that the workflow calls, so it can be exercised directly. These tests +import that module rather than re-implementing its rules, and a +separate test asserts the workflow really invokes it. +""" + +from __future__ import annotations + +import importlib.util +import json +import re +import subprocess +import sys +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +SCRIPT = ROOT / "scripts" / "release_notes.py" +WORKFLOW = ROOT / ".github" / "workflows" / "build.yml" +TRANSLATIONS = ROOT / "app" / "translations.json" + + +def _load_module(): + spec = importlib.util.spec_from_file_location("_release_notes", SCRIPT) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +# Module-level on purpose, eyes-open about the failure mode: if the +# script ever grows a non-stdlib import, an *uninstalled* one raises here +# during collection and takes all the tests in this file with it, rather +# than failing test_script_is_stdlib_only as a clean assertion (an +# installed one does fail cleanly). Deliberately not made lazy: the +# import is the thing under test, CI goes red either way, and the +# traceback names the offending import and line. Making it lazy would +# cost a fixture threaded through 60-odd tests to soften a signal that is +# already loud and unambiguous. +rn = _load_module() + + +# -- Development noise must not reach end users ----------------------- + +class TestDroppedTypes: + """The regression this change exists for.""" + + # Real subjects taken verbatim from v1.14.4..HEAD, which the old + # bash generator published under "## Other" with the prefix intact. + REAL_NOISE = [ + "test: rename unused QApplication holder to satisfy CodeQL", + "refactor(window): extract auto-update subsystem into UpdateController", + "test(editor): assert writer closed before os.replace in atomic write", + "refactor(editor): reuse shared atomic PDF write in TabEditar._run", + "test: close file handles in test_pdfapps to satisfy CodeQL", + "test(editor): cover image and highlight branches of apply_pending_edits", + "refactor(editor): extract pure apply_pending_edits dispatcher " + "from TabEditar._run", + "refactor(editor): extract pure text-reinsertion helpers to " + "text_reinsert module", + ] + + @pytest.mark.parametrize("subject", REAL_NOISE) + def test_test_and_refactor_are_dropped(self, subject): + assert rn.classify(subject) is None + + def test_real_interval_publishes_no_noise(self): + body = rn.build_notes( + self.REAL_NOISE + [ + "feat(editor): commit inline edits on focus-out", + "fix(flatpak): pin cryptography and bump pypdf past six CVEs", + ] + ) + assert "TabEditar" not in body + assert "CodeQL" not in body + assert "text_reinsert" not in body + assert "## Other" not in body + assert "- Commit inline edits on focus-out" in body + + @pytest.mark.parametrize("subject", [ + "chore: tidy up", + "ci(build): retry macOS job", + "build: bump pyinstaller", + "docs: rewrite README", + "docs(deps): note the pypdf floor", + "style(tools): satisfy CodeQL empty-except and unused-var notes", + "seo(docs): add OG/Twitter cards, canonical, JSON-LD", + "Merge pull request #175 from nelsonduarte/chore/bump", + "chore(deps): bump pillow from 12.2.0 to 12.3.0", + "Bump actions/checkout from 4 to 5", + ]) + def test_other_non_user_facing_subjects_are_dropped(self, subject): + assert rn.classify(subject) is None + + +# -- Prefix stripping now covers every surviving type ----------------- + +class TestCleanSubject: + @pytest.mark.parametrize("subject,expected", [ + ("feat: add thing", "Add thing"), + ("fix(editor): repair thing", "Repair thing"), + ("security: bump ghostscript", "Bump ghostscript"), + ("security(deps): align qtawesome floor to 1.4.2", + "Align qtawesome floor to 1.4.2"), + ("perf: faster render", "Faster render"), + ("a11y: focus indicators", "Focus indicators"), + ("i18n: translate updater errors", "Translate updater errors"), + ("feat!: breaking change", "Breaking change"), + ("feat(editor)!: breaking scoped change", "Breaking scoped change"), + ("Splash image with no background", "Splash image with no background"), + ]) + def test_prefix_stripped_and_capitalised(self, subject, expected): + assert rn.clean_subject(subject) == expected + + def test_no_surviving_bullet_keeps_a_raw_prefix(self): + """The old sed only knew feat|fix|perf|a11y|docs.""" + subjects = [ + "security(deps): bump pillow floor to 12.3.0", + "i18n: translate updater SHA256 error messages", + "design: novo icone com documento PDF", + "a11y: accessible names, focus indicators, keyboard navigation", + "perf: run compress off the UI thread", + ] + body = rn.build_notes(subjects) + for line in body.splitlines(): + if line.startswith("- "): + assert not re.match( + r"- [A-Za-z0-9]+(\([^)]*\))?!?:", line + ), f"raw conventional prefix published: {line!r}" + + def test_unicode_subject_survives(self): + body = rn.build_notes(["fix(i18n): corrigir acentuação e ícones"]) + assert "- Corrigir acentuação e ícones" in body + + +# -- Security is its own section, and it leads ------------------------ + +class TestSecuritySection: + # All the security commits in main's history (arrow normalised). + REAL_SECURITY = [ + "security(deps): align qtawesome floor to 1.4.2", + "security(deps): bump pillow floor to 12.3.0 and update " + "vulnerable flatpak pins", + "security: bump bundled Ghostscript 10.05.0 to 10.07.0 (#32)", + "security: verify SHA256 of bundled Tesseract and Ghostscript downloads", + "security: upgrade PDF encryption to AES-256 and fix toast use-after-free", + "security: harden updater hash verification and uninstaller " + "BAT generation", + "security: fix ZIP/TAR slip, temp perms, batch injection, " + "path validation", + ] + + @pytest.mark.parametrize("subject", REAL_SECURITY) + def test_security_commits_get_their_own_heading(self, subject): + assert rn.classify(subject) == "## Security" + + def test_security_deps_survives_but_docs_deps_does_not(self): + """Both match /deps/, only one is a user-facing fix.""" + assert rn.classify("security(deps): bump pillow") == "## Security" + assert rn.classify("docs(deps): note the pypdf floor") is None + + def test_hand_written_bump_not_eaten_by_dependabot_heuristic(self): + """'bump.*from.*to' is the dependabot filter. Applied blindly it + also swallows a real security note phrased the same way, which + is the worst possible thing for it to drop.""" + assert rn.classify( + "security(deps): bump pillow floor from 12.2.0 to 12.3.0" + ) == "## Security" + assert rn.classify( + "fix(flatpak): bump pypdf from 6.1.0 to 6.10.0" + ) == "## Fixes & improvements" + # Untyped and chore-typed dependabot subjects still go. + assert rn.classify("Bump actions/checkout from 4 to 5") is None + assert rn.classify( + "chore(deps): bump pillow from 12.2.0 to 12.3.0" + ) is None + + def test_security_section_comes_first(self): + body = rn.build_notes([ + "fix: a fix", + "feat: a feature", + "perf: a perf win", + "security: a security fix", + ]) + headings = [ln for ln in body.splitlines() if ln.startswith("## ")] + assert headings[0] == "## Security" + assert headings == [ + "## Security", "## New features", + "## Performance", "## Fixes & improvements", + ] + + +# -- "Other" is now only the forgotten-prefix safety net -------------- + +class TestOtherSection: + def test_typed_commit_never_lands_in_other(self): + for subject in [ + "feat: x", "fix: x", "perf: x", "security: x", + "a11y: x", "i18n: x", "design: x", + ]: + assert rn.classify(subject) != "## Other" + + def test_unprefixed_commit_is_kept_as_other(self): + """A real user-facing change whose prefix was forgotten must not + be dropped silently.""" + assert rn.classify("Splash image with no background") == "## Other" + body = rn.build_notes(["Splash image with no background"]) + assert "## Other" in body + assert "- Splash image with no background" in body + + def test_prose_that_merely_sounds_like_a_bump_is_kept(self): + """The safety net promised more than it delivered. + + The old noise filter was the unanchored "bump.*from.*to", which + substring-matches an ordinary sentence. A real user-facing + change with a forgotten prefix was therefore dropped in + silence, which is precisely what "## Other" exists to prevent. + """ + assert rn.classify( + "Bump minimum zoom from 50 to 400 percent" + ) == "## Other" + assert rn.classify( + "Raise the page limit from 500 to 2000 pages" + ) == "## Other" + body = rn.build_notes(["Bump minimum zoom from 50 to 400 percent"]) + assert "- Bump minimum zoom from 50 to 400 percent" in body + + def test_machine_generated_dependency_subjects_still_dropped(self): + """Narrowing the filter must not let dependabot back in. + + "Bumps from X to Y" (plural) is the form dependabot uses in + PR bodies and squash-merge subjects. It does not occur in this + repo's history, so it is a forward-looking case rather than a + regression: an earlier "^bump\\s" required a space straight after + "bump" and published the plural under "## Other". + """ + for subject in [ + "Bump actions/checkout from 4 to 5", + "Bump pypdf from 6.1.0 to 6.10.0", + "Bumps pillow from 12.2.0 to 12.3.0", + "chore(deps): bump pillow from 12.2.0 to 12.3.0", + "build(deps): bump pyinstaller from 6.0 to 6.1", + "Bumped by dependabot[bot]", + ]: + assert rn.classify(subject) is None, subject + + def test_prose_mentioning_dependabot_is_kept(self): + """The bot filter is anchored, so it cannot eat ordinary prose. + + As a bare substring "dependabot" dropped any subject that merely + named it. Both of these are plausible user-facing commits with a + forgotten prefix, and "## Other" exists precisely to catch those. + """ + assert rn.classify( + "Move the dependabot config into .github" + ) == "## Other" + assert rn.classify( + "Document the dependabot workflow for contributors" + ) == "## Other" + # The trailing-account form stays caught: it is not a prefix, so + # "\\[bot\\]" is deliberately left unanchored. + assert rn.classify("Bumped by dependabot[bot]") is None + # A typed commit naming the bot is dropped by its type, not by + # the noise filter; this is the real subject from this history. + assert rn.classify( + "ci: add dependabot config for github-actions and pip" + ) is None + + def test_unrecognised_type_is_published_as_a_fix(self): + """The _TYPE_DESTINATION lookup has a default, and it is reached. + + A prefix nobody registered ("hotfix:") must still reach users + rather than vanish. Flipping the default to HEADING_OTHER used + to leave every test green, so this pins it. + """ + assert rn.classify( + "hotfix: repair crash on open" + ) == "## Fixes & improvements" + assert rn.classify("wip: half a feature") == "## Fixes & improvements" + + def test_docs_is_dropped_unconditionally(self): + """Pins the S-3 decision: docs is maintenance, scope or not. + + The old code had an unreachable `ctype == "docs" and scope == + "deps"` branch sitting below the drop-set check. Asserting both + arms here means a future change that publishes plain `docs:` + has to face this test rather than silently resurrect the + ambiguity. + """ + assert rn.classify("docs: rewrite README") is None + assert rn.classify("docs(deps): note the pypdf floor") is None + assert rn.classify("docs(website): update tool list") is None + + def test_other_absent_from_typical_release(self): + body = rn.build_notes([ + "feat: something new", + "fix: something fixed", + "test: something tested", + "refactor: something moved", + "chore: something tidied", + ]) + assert "## Other" not in body + + +# -- Body assembly ---------------------------------------------------- + +class TestBuildNotes: + def test_empty_input_falls_back(self): + assert rn.build_notes([]).strip() == "Bug fixes and improvements." + + def test_all_dropped_falls_back(self): + body = rn.build_notes(["chore: a", "test: b", "refactor: c"]) + assert body.strip() == "Bug fixes and improvements." + + def test_blank_and_whitespace_subjects_ignored(self): + assert rn.build_notes(["", " ", "\t"]).strip() == \ + "Bug fixes and improvements." + + def test_duplicate_subjects_squashed(self): + body = rn.build_notes(["fix: same thing", "fix(scope): same thing"]) + assert body.count("- Same thing") == 1 + + def test_empty_subject_after_prefix_is_dropped(self): + assert "- \n" not in rn.build_notes(["fix:", "fix: real one"]) + + def test_body_ends_with_newline(self): + assert rn.build_notes(["fix: a"]).endswith("\n") + + def test_headings_are_markdown_h2(self): + body = rn.build_notes(["fix: a", "feat: b"]) + for line in body.splitlines(): + if line.startswith("#"): + assert line.startswith("## ") + + +# -- Contract with the downstream consumers --------------------------- + +class TestDownstreamContract: + def test_every_heading_has_an_updater_i18n_mapping(self): + """A heading emitted here but missing from _SECTION_MAP degrades + to English in the update dialog, silently.""" + from app.updater import _SECTION_MAP + for heading in rn._SECTION_ORDER: + assert heading in _SECTION_MAP, heading + + def test_every_updater_key_exists_in_all_eight_languages(self): + from app.updater import _SECTION_MAP + data = json.loads(TRANSLATIONS.read_text(encoding="utf-8")) + assert len(data) == 8, sorted(data) + for heading, key in _SECTION_MAP.items(): + for lang, table in data.items(): + assert key in table, f"{key} missing in {lang}" + assert table[key].strip(), f"{key} empty in {lang}" + + def test_security_heading_translated_in_every_language(self): + data = json.loads(TRANSLATIONS.read_text(encoding="utf-8")) + values = { + lang: table["update.section.security"] + for lang, table in data.items() + } + assert values["en"] == "## Security" + for lang, value in values.items(): + assert value.startswith("## "), (lang, value) + # English aside, a copy-pasted English string means an + # untranslated heading shipped by accident. + non_en = [v for lang, v in values.items() if lang != "en"] + assert "## Security" not in non_en + + def test_localize_notes_translates_the_security_heading(self): + from app import i18n + from app.updater import _localize_notes + original = i18n._LANG + try: + i18n._LANG = "pt" + out = _localize_notes(rn.build_notes(["security: corrigir X"])) + finally: + i18n._LANG = original + assert "SEGURAN" in out.upper() + assert "## Security" not in out + + def test_checksums_heading_is_not_emitted_by_the_generator(self): + """build.yml appends '## Checksums (SHA256)' after this body; + the updater parses it. The generator must not collide.""" + assert "Checksums" not in rn.build_notes(["fix: a", "feat: b"]) + + +# -- The workflow really uses the module ------------------------------ + +class TestWorkflowWiring: + def test_build_yml_invokes_the_script(self): + """A comment mentioning the script must not satisfy this test. + + The previous version asserted the bare substring + "scripts/release_notes.py" against the whole file, and build.yml + names the script in an explanatory comment right above the run + line. Replacing the invocation with a hardcoded + `echo ... > release_notes.md` therefore left this green: the + workflow would have stopped generating notes with nothing going + red. Comment lines are stripped before matching. + """ + run_lines = [ + ln.strip() + for ln in WORKFLOW.read_text(encoding="utf-8").splitlines() + if ln.strip() and not ln.strip().startswith("#") + ] + assert any( + re.search( + r"python3?\s+scripts/release_notes\.py\s+\S+\s+-o\s+release_notes\.md", + ln, + ) + for ln in run_lines + ), "build.yml has no uncommented invocation of the generator" + + def test_workflow_no_longer_categorises_inline(self): + text = WORKFLOW.read_text(encoding="utf-8") + assert 'OTHER="${OTHER}- ${clean}' not in text + + def test_script_is_stdlib_only(self): + """The release job has no setup-python step; it uses the runner's + bare system interpreter. + + Parsed, not substring-matched. The previous denylist of five + names missed every third-party import outside that list + ("import packaging", "import tomlkit") and every + "from X import Y" form, so it failed 3 of 5 probe cases. Walking + the AST and checking against sys.stdlib_module_names catches any + non-stdlib import, named or not. Note __future__ is itself in + stdlib_module_names, so the real import at the top of the script + is not a false positive. + """ + import ast + + tree = ast.parse(SCRIPT.read_text(encoding="utf-8")) + offenders = [] + for node in ast.walk(tree): + if isinstance(node, ast.Import): + for alias in node.names: + offenders.append(alias.name.split(".")[0]) + elif isinstance(node, ast.ImportFrom): + # level > 0 is a relative import; the script is a + # standalone file with no package to be relative to. + if node.level == 0 and node.module: + offenders.append(node.module.split(".")[0]) + + assert offenders, "no imports parsed; the AST walk is not looking" + non_stdlib = sorted( + {n for n in offenders if n not in sys.stdlib_module_names} + ) + assert not non_stdlib, ( + f"release_notes.py imports non-stdlib module(s): {non_stdlib}. " + "The release job runs it with the runner's bare python3." + ) + + def test_cli_writes_a_file(self, tmp_path): + out = tmp_path / "notes.md" + first = subprocess.run( + ["git", "rev-list", "--max-parents=0", "HEAD"], + cwd=ROOT, capture_output=True, text=True, check=True, + ).stdout.strip().splitlines()[0] + result = subprocess.run( + [sys.executable, str(SCRIPT), f"{first}..HEAD", "-o", str(out)], + cwd=ROOT, capture_output=True, text=True, + ) + assert result.returncode == 0, result.stderr + assert out.read_text(encoding="utf-8").strip() From 73db4249bcabcd49f132a7b76df257516297de7c Mon Sep 17 00:00:00 2001 From: Nelson Duarte Date: Sat, 12 Sep 2026 21:58:08 +0100 Subject: [PATCH 2/5] Potential fix for pull request finding 'CodeQL / Implicit string concatenation in a list' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- tests/test_release_notes_generator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_release_notes_generator.py b/tests/test_release_notes_generator.py index be2e74d..078dd12 100644 --- a/tests/test_release_notes_generator.py +++ b/tests/test_release_notes_generator.py @@ -157,7 +157,7 @@ class TestSecuritySection: "security: upgrade PDF encryption to AES-256 and fix toast use-after-free", "security: harden updater hash verification and uninstaller " "BAT generation", - "security: fix ZIP/TAR slip, temp perms, batch injection, " + "security: fix ZIP/TAR slip, temp perms, batch injection, " + "path validation", ] From 5c4ab4dbe429af96e966566210e57695ce6963f9 Mon Sep 17 00:00:00 2001 From: Nelson Duarte Date: Sat, 12 Sep 2026 21:58:37 +0100 Subject: [PATCH 3/5] Potential fix for pull request finding 'CodeQL / Implicit string concatenation in a list' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- tests/test_release_notes_generator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_release_notes_generator.py b/tests/test_release_notes_generator.py index 078dd12..40a590c 100644 --- a/tests/test_release_notes_generator.py +++ b/tests/test_release_notes_generator.py @@ -65,9 +65,9 @@ class TestDroppedTypes: "refactor(editor): reuse shared atomic PDF write in TabEditar._run", "test: close file handles in test_pdfapps to satisfy CodeQL", "test(editor): cover image and highlight branches of apply_pending_edits", - "refactor(editor): extract pure apply_pending_edits dispatcher " + "refactor(editor): extract pure apply_pending_edits dispatcher " + "from TabEditar._run", - "refactor(editor): extract pure text-reinsertion helpers to " + "refactor(editor): extract pure text-reinsertion helpers to " + "text_reinsert module", ] From dff11629682e68bbe66e61398402c9f4d9ab0eae Mon Sep 17 00:00:00 2001 From: Nelson Duarte Date: Sat, 12 Sep 2026 21:58:46 +0100 Subject: [PATCH 4/5] Potential fix for pull request finding 'CodeQL / Implicit string concatenation in a list' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- tests/test_release_notes_generator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_release_notes_generator.py b/tests/test_release_notes_generator.py index 40a590c..607805a 100644 --- a/tests/test_release_notes_generator.py +++ b/tests/test_release_notes_generator.py @@ -150,7 +150,7 @@ class TestSecuritySection: # All the security commits in main's history (arrow normalised). REAL_SECURITY = [ "security(deps): align qtawesome floor to 1.4.2", - "security(deps): bump pillow floor to 12.3.0 and update " + "security(deps): bump pillow floor to 12.3.0 and update " + "vulnerable flatpak pins", "security: bump bundled Ghostscript 10.05.0 to 10.07.0 (#32)", "security: verify SHA256 of bundled Tesseract and Ghostscript downloads", From dd4dbe753a23bd98daa4bad0263d33840f2de559 Mon Sep 17 00:00:00 2001 From: Nelson Duarte Date: Sat, 12 Sep 2026 22:00:57 +0100 Subject: [PATCH 5/5] Potential fix for pull request finding 'CodeQL / Implicit string concatenation in a list' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- tests/test_release_notes_generator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_release_notes_generator.py b/tests/test_release_notes_generator.py index 607805a..3cc8d62 100644 --- a/tests/test_release_notes_generator.py +++ b/tests/test_release_notes_generator.py @@ -155,7 +155,7 @@ class TestSecuritySection: "security: bump bundled Ghostscript 10.05.0 to 10.07.0 (#32)", "security: verify SHA256 of bundled Tesseract and Ghostscript downloads", "security: upgrade PDF encryption to AES-256 and fix toast use-after-free", - "security: harden updater hash verification and uninstaller " + "security: harden updater hash verification and uninstaller " + "BAT generation", "security: fix ZIP/TAR slip, temp perms, batch injection, " + "path validation",