diff --git a/.agents/skills/update-changelog/SKILL.md b/.agents/skills/update-changelog/SKILL.md index 9d0276ef53..b6803bce46 100644 --- a/.agents/skills/update-changelog/SKILL.md +++ b/.agents/skills/update-changelog/SKILL.md @@ -5,16 +5,18 @@ description: Update the project CHANGES.md with issues from a given GitHub miles # Skill: update-changelog -Update `CHANGES.md` with entries for all issues and PRs in a given GitHub -milestone. Each entry references the user-facing issue (not the PR) as the -primary link, with the fix PR inline on the same line. +Entry point for all changelog work. All rules — sections, exclusions, entry +format, the `:boom:` and `:rocket:` label contracts, pre-flight, and the +anomaly report — live in `mem:workflow/updating-changelog` (file: +`.serena/memories/workflow/updating-changelog.md`). This skill routes to that +memory; it does not repeat it. ## When to Use - Before a new release, to populate the changelog with all fixed issues - When new issues are added to an existing milestone and the changelog needs to be refreshed -- To ensure every entry follows the correct format for the changelog +- To audit or repair an existing version section ## Prerequisites @@ -23,289 +25,28 @@ primary link, with the fix PR inline on the same line. - `scripts/gh.py` for GitHub queries, `scripts/changelog.py` for changelog checks (run `python3 scripts/changelog.py --help` for usage) -## Workflow +## What to Read -### 1. Determine the target version +1. `mem:workflow/updating-changelog` — the canonical flow: sources of truth, + layout, entry rules, the `## Breaking change note` comment contract, + pre-flight checks, anomaly types, and pitfalls. +2. `mem:scripts/changelog` — `boom`, `check-merged`, `cross-ref`, `report`. +3. `mem:scripts/gh` — `issues`, `issue`, `comments`, `prs`. -The version is typically a semver string like `2.15.3`. Confirm with the user -if not specified. - -### 2. Fetch all issues in the milestone +## Commands ```bash -# All closed issues (default) -python3 scripts/gh.py issues "2.16.0" - -# Include open issues too -python3 scripts/gh.py issues "2.16.0" --state all - -# Exclude entries that should not go in the changelog -python3 scripts/gh.py issues "2.16.0" --exclude "release blocker,no changelog" +python3 scripts/gh.py issues "" --exclude "release blocker,no changelog" +python3 scripts/changelog.py check-merged +python3 scripts/changelog.py boom "" # print the :boom: section +python3 scripts/changelog.py boom "" --write # replace it in CHANGES.md +python3 scripts/changelog.py cross-ref "" +python3 scripts/changelog.py report "" ``` -**Exclusion rules (issue-level):** -- `no changelog` label — chore/refactor work, no entry needed -- `release blocker` label — blocked issues not yet ready for changelog -- `Task` issue type — internal chores, not user-facing; excluded by `gh.py` - (use `--include-tasks` to override) -- **Rejected project status** — issues with "Rejected" status on the "Main" - project board are excluded by `gh.py` (use `--include-rejected` to override). - This status is independent of the GitHub issue `state`. +## Hard Rules -**Exclusion rules (PR-level):** PRs with these labels stay out regardless of -their linked issue's labels: `release blocker`, `no issue required`. - -Each entry carries `number`, `title`, `state`, `issue_type`, `labels`, -`closing_prs`, and `project_status`. - -### 3. Identify missing entries (optional) - -```bash -python3 scripts/gh.py issues "2.16.0" --exclude "release blocker,no changelog" --compare CHANGES.md -``` - -Returns only milestone issues not yet referenced in the changelog. Note: it -compares **issues** only. For unreferenced merged **PRs**, use the -cross-reference in step 8. - -### 4. Fetch additional PR details when needed - -```bash -# One or more PR numbers (also: --file prs.txt, or --stdin) -python3 scripts/gh.py prs 9179 9204 9311 - -# All merged PRs in a milestone (default); --state all/open/closed for others -python3 scripts/gh.py prs --milestone "2.16.0" -``` - -Returns `number`, `title`, `body`, `state`, `merged_at`, `author`, `labels`, -and `closing_issues`. Milestone mode uses paginated GraphQL (100 per page). - -### 5. Categorize entries — strictly by issue type, never by labels or emoji - -Use the **Issue Type** field (`issue_type` in the `gh.py` output). No separate -query is needed. - -> **⚠️ CRITICAL: Never use labels or title emoji prefixes for categorization.** -> Labels like `bug`/`enhancement` and prefixes like `:bug:`/`:sparkles:` are -> often wrong or missing. `issue_type` is the single source of truth. - -| `issue_type` value | Changelog section | -|--------------------|-------------------| -| `Bug` | `### :bug: Bugs fixed` | -| `Feature` or `Enhancement` | `### :sparkles: New features & Enhancements` | -| `Task` | **Exclude** — internal chores, not user-facing | -| `null` (not set) | Fallback to labels: `bug` label → bugs, otherwise enhancements | - -**Breaking changes override everything:** an issue with the `breaking change` -label goes under `### :boom: Breaking changes & Deprecations`, no matter its -issue type. This and `release highlight` below are the only label-based -rules — explicit exceptions to the "never by labels" principle above. The -`:boom:` subsection goes first in the version, before `:rocket:` (matching -the existing precedent). - -**Highlights come from the `release highlight` label:** an issue with that -label also gets an entry under `### :rocket: Epics and highlights`, on top of -its regular entry (usually `:sparkles:`). In the section being refreshed, -`:rocket:` lists exactly the labelled issues: never pick highlights yourself -and never keep an unlabelled entry there. Product decides the highlights by -labelling issues on GitHub, so a regeneration cannot undo that choice. Older -version sections are left as they are. - -**Community attribution:** if the issue or its fix PR has the -`community contribution` label, add `(by @)` on the entry -line, **before** the issue/PR references. Use the **PR author** (the `author` -field from step 4), not the issue author: - -```markdown -- Fix description of the bug (by @username) [#](...) (PR: [#](...)) -``` - -**Only closed issues are included**, even if open ones are tracked in the -milestone. - -**Pairing rules:** - -| Pattern | Changelog format | -|---------|-----------------| -| Closed issue + one or more fix PRs | Primary link = issue, PRs inline comma-separated | -| PR with no linked issue | Link the issue if a matching closed one exists in the milestone; otherwise skip (the issue is the changelog unit) | -| Closed issue with no fix PR in milestone | Link the issue directly, no PR reference | - -> **False-positive associations:** a PR may wrongly claim to close an issue -> from another context (ancient PR, cross-project reference). If titles are -> clearly unrelated or the PR predates the issue by years, treat it as a data -> glitch and skip it. - -### 5a. Verify PR merge status before writing - -A closed issue may list closing PRs that were **closed without merging** -(e.g. a superseded community PR). Only **merged** PRs go in the changelog: - -```bash -python3 scripts/changelog.py check-merged -# also accepts: --file prs.txt, or numbers via --stdin -``` - -If a closing PR is closed-unmerged, find the merged PR that superseded it -(other PRs in the issue's closing list, similar titles, or pointers in the -closed PR's timeline) and reference that one instead. - -### 5b. Security advisory (GHSA) entries - -Advisories fixed in a release go in the changelog even though they are -**neither milestone issues nor PRs**. The GHSA ID and description come from -the user or the release notes — never from the milestone fetch. - -```markdown -- Fix (https://github.com/penpot/penpot/security/advisories/GHSA-XXXX-XXXX-XXXX) -``` - -Rules: place under `### :bug: Bugs fixed` with **no issue or PR link**; do -**not** fetch or verify the URL (it may be draft/unpublished and 404); write -the description in imperative mood from the advisory title. These entries are -invisible to the automation — add them by hand, and in step 9 apply only the -backport/duplicate check to them. - -### 6. Read the current CHANGES.md and run pre-flight checks - -Newest version goes at the top, right after the `# CHANGELOG` header: - -```markdown -## - -### :boom: Breaking changes & Deprecations - -- [#](https://github.com/penpot/penpot/issues/) (PR: [#](https://github.com/penpot/penpot/pull/)) - -### :bug: Bugs fixed - -- Fix description of the bug [#](https://github.com/penpot/penpot/issues/) (PR: [#](https://github.com/penpot/penpot/pull/)) -- Fix another bug (by @contributor) [#](https://github.com/penpot/penpot/issues/) (PR: [#](https://github.com/penpot/penpot/pull/)) - -### :sparkles: New features & Enhancements - -- Add new feature description [#](https://github.com/penpot/penpot/issues/) (PR: [#](https://github.com/penpot/penpot/pull/)) -``` - -Format details: entries start with `- ` plus a short imperative description; -PR refs stay inline (`(PR: [#]())`, comma-separated for several); -`(by @)` goes before the issue link; only include non-empty -sections; blank line between a section's last entry and the next title; never -duplicate an entry from an earlier version section (the earlier version wins -for backports). - -**Pre-flight checks — fix violations directly in `CHANGES.md` before writing -the new section.** Reconcile every existing entry (any version section) and -every milestone candidate against the current milestone state (re-fetch, do -not trust cached data): - -1. **Duplicate across versions** → remove from the current section. -2. **Stale milestone assignment** (issue moved out of this milestone) → - remove the entry (or drop it if the section does not exist yet). -3. **Newly applied exclusion labels** (`no changelog`, `release blocker`) → - remove the entry. -4. **Issue no longer closed/deleted/Rejected** → remove the entry. -5. **Unmerged or moved PR reference** → fix the reference or remove the - entry (a PR merged in a *different* milestone is a step-9 anomaly, do not - silently remove it). -6. **Issue type changed to `Task`** → remove the entry. -7. **Breaking change misplaced** (issue has the `breaking change` label but - sits in another section) → move the entry to `:boom:`. -8. **Highlight out of sync** (`:rocket:` entry without the `release - highlight` label, or a labelled issue missing from `:rocket:`) → remove - the `:rocket:` entry or add it (step 7b). The regular entry stays. -9. **Missing valid issues** (closed, non-excluded, unreferenced anywhere) → - add them to the current section per step 5. - -### 7. Build the description text - -Derive it from the **issue title**, not the PR title. Strip leading emoji -prefixes (`:bug:`, `:sparkles:`, `:tada:`) and describe the user-facing -behavior: - -| Issue title | Changelog description | -|-------------|----------------------| -| `Plugin API token methods fail with schema validation error on PRO` | `Fix Plugin API token methods failing with schema validation error on PRO` | -| `Comment content is not sanitized before rendering, enabling stored XSS` | `Sanitize comment content on rendering` | -| `Custom uploaded font family names are not sanitized` | `Sanitize font family names on custom uploaded fonts` | - -Insert the new version section right after the `# CHANGELOG` header with the -`edit` tool and enough context for a unique match. - -### 7b. Populate `:rocket: Epics and highlights` from the label - -List the milestone issues that pass step 2 exclusions and carry the -`release highlight` label: - -```bash -python3 scripts/gh.py issues "2.16.0" --exclude "release blocker,no changelog" \ - | python3 -c "import sys,json; [print(i['number'], i['title']) for i in json.load(sys.stdin) if 'release highlight' in i['labels']]" -``` - -Create the subsection if missing (place it before `### :sparkles:`) with one -entry per labelled issue, same text and references as its regular entry. -Every `:rocket:` entry MUST carry issue AND PR references. If no issue is -labelled, leave `:rocket:` out and tell the user: the highlights are -product's call, not the agent's. To change them, add or remove the label on -GitHub and re-run; never edit `:rocket:` by hand. - -### 8. Cross-reference milestone PRs against the changelog - -```bash -python3 scripts/changelog.py cross-ref "" [--changes CHANGES.md] -``` - -Lists merged milestone PRs missing from the changelog section (decide per PR: -add it or confirm its exclusion labels) and warns about CLOSED (unmerged) PRs -in the milestone. GHSA entries never appear here — that absence is expected. - -### 9. Generate the anomaly report - -```bash -python3 scripts/changelog.py report "" [--changes CHANGES.md --output CHANGES-ISSUES.md] -``` - -Overwrites `CHANGES-ISSUES.md` with the current state. Every number renders as -a full `[#N](https://github.com/penpot/penpot/issues/N)` or -`[#N](https://github.com/penpot/penpot/pull/N)` link. - -**An anomaly is a milestone mismatch or a `:boom:` mislabel** (the changelog -pairing is misleading and a human must judge intent): - -1. Issue in this milestone, referenced PR in another milestone (or none). -2. PR in this milestone, closed issue in another milestone — except an issue - with *no* milestone, which belongs to another (probably private) project - and is neither anomaly nor changelog candidate. -3. `:boom:` entry whose issue lacks the `breaking change` label. These stay - listed in the report **and** in the changelog: either label the issue or - move the entry to its regular section. -4. `:rocket:` out of sync with the `release highlight` label. Step 6 - pre-flight should have fixed it: if it shows up, re-run the workflow. - -**Highlight gaps are warnings, not anomalies:** missing `:rocket:` on a -released X.Y.0 (patches never carry highlights); `:rocket:` entry without -issue AND PR references. Gaps never count toward the anomaly total. - -**Anything else is a rule violation**, not a report item: fix it in step 6 -pre-flight. If one shows up in the report, re-run the workflow. - -## Key Principles - -- **Issue = changelog unit**, PR = implementation detail inline. -- **Latest version first**, below the `# CHANGELOG` header. -- **Issue Type decides the section — exclusively**, except `breaking change` - label → `:boom:` first, and `release highlight` label → also `:rocket:`. -- **User-facing descriptions** in imperative mood. -- **Community attribution** uses the PR author, placed before the issue link. -- **Only closed issues**; **Rejected** project status excludes. -- **`no changelog` and `Task`** stay out. -- **Multiple fix PRs** go comma-separated inline. -- **Duplicates**: the earlier version section wins. -- **Taiga references**: resolve to the GitHub issue via description text or - PRs mentioning the Taiga URL, then link issue + PR. -- **GHSA entries** live under `:bug:` with only the advisory URL (see 5b). -- **Re-fetch before editing**; prefer `scripts/gh.py` over raw `gh api`. -- **Verify PR merge status** (`check-merged`); PR-level exclusions apply. -- **Cross-reference PRs, not just issues** (`cross-ref`); watch for - false-positive PR-to-issue links. +- `CHANGES.md` is edited only on explicit request or during the release process. +- Only closed, non-excluded milestone issues are included; the issue is the changelog unit and the PR is inline. +- `:boom:` and `:rocket:` are label-generated: never hand-edit them; edit the label or the `## Breaking change note` comment and re-run the command. +- Verify PR merge status before writing; only merged PRs are referenced. diff --git a/.serena/memories/critical-info.md b/.serena/memories/critical-info.md index 867eecdbd9..43f678a643 100644 --- a/.serena/memories/critical-info.md +++ b/.serena/memories/critical-info.md @@ -15,11 +15,12 @@ You are working on the GitHub project `penpot/penpot`, a monorepo. - Before `git commit` → `mem:workflow/creating-commits` (subject/body format, 80-char body wrapping enforced by `scripts/check-commit`, `AI-assisted-by: model-name` trailer) - Before `gh issue create` → `mem:workflow/creating-issues` (title derivation, body template, labels, Issue Type) - Before `gh pr create` / `gh pr edit` → `mem:workflow/creating-prs` (title format, body structure, "Note:" line) +- Before updating `CHANGES.md` → `mem:workflow/updating-changelog` (sources of truth, entry rules, the `:boom:`/`:rocket:` label contracts, pre-flight checks, anomaly report) - Before a repo-wide pnpm version update → `mem:workflow/updating-pnpm` (workspace layout, `scripts/sync-pnpm-version` flow, the stamp-missing-field and ignored-builds gotchas, verification steps) - Before a Node version update → `mem:workflow/updating-node` (the six pin sites in `.nvmrc`, the devenv and image Dockerfiles and the plugins-deploy workflows, the DHI tag gotcha, what to verify) - Before bumping a non-JS dependency pinned in `docker/images/` (JDK, ImageMagick) → `mem:workflow/docker-images` - **Never `git push`, force-push, or modify `git origin`** (or any other remote). The user pushes from their own shell; if a push is required, say so and wait. Never amend a commit that the user has already pushed unless explicitly asked. -- **Never edit `CHANGES.md` by hand.** The changelog is generated from GitHub milestones during the release process; update it only via the `update-changelog` skill flow or on explicit user request. +- **Never edit `CHANGES.md` by hand.** The changelog is generated from GitHub milestones during the release process; update it only via `mem:workflow/updating-changelog` (the `update-changelog` skill flow) or on explicit user request. - You have access to the GitHub CLI `gh` or corresponding MCP tools. - Issues are also managed on Taiga. Read issues using the `read_taiga_issue` tool. - Before writing code, analyze the task in depth and describe your plan. If the task is complex, break it down into atomic steps. After making changes, run the applicable lint and format checks for the affected module before considering the work done (per example `mem:backend/core` or `mem:frontend/core`). @@ -59,7 +60,7 @@ The memory is structured in a way that you can get the critical information abou - `scripts/taiga.py` — Fetch public issues, user stories, and tasks from the Penpot Taiga project without authentication. See `mem:scripts/taiga`. - `scripts/gh.py` — GitHub operations helper: list milestone issues, fetch PR details, compare against CHANGES.md. Requires `gh` CLI. See `mem:scripts/gh`. - `scripts/project-anomalies.py` — Main-board anomaly check for a milestone (`check` writes `tmp/-ANOMALIES.md`). Bulk resolution via `scripts/gh.py`. See `mem:scripts/project-anomalies`. -- `scripts/changelog.py` — Changelog checks for the `update-changelog` skill: `check-merged` (PR merge status), `cross-ref` (milestone PRs vs changelog section), `report` (anomaly report to CHANGES-ISSUES.md). Calls `scripts/gh.py` via subprocess. See `mem:scripts/changelog`. +- `scripts/changelog.py` — Changelog checks for `mem:workflow/updating-changelog`: `boom` (generate the `:boom:` section from `breaking change` issues), `check-merged` (PR merge status), `cross-ref` (milestone PRs vs changelog section), `report` (anomaly report to CHANGES-ISSUES.md). Calls `scripts/gh.py` via subprocess. See `mem:scripts/changelog`. - `scripts/error-reports.mjs` — Query error reports via RPC API with token authentication. Supports list/get operations with filtering and pagination. See `mem:scripts/error-reports`. - `scripts/clean-node-modules` — Remove stale `node_modules` from all pnpm workspaces (root, modules, member packages). Keeps the shared pnpm store at `/.pnpm-store` unless `--store`; ignores `external/` and `.opencode/`. Usage and reinstall steps: `mem:workflow/updating-pnpm`. - `scripts/setup-opencode` — Install the opencode CLI in-container as a global pnpm package in `$PNPM_HOME`. Not part of the devenv image; see `mem:devenv/core`. diff --git a/.serena/memories/scripts/changelog.md b/.serena/memories/scripts/changelog.md index 6dc0dd5c94..e04036ea9a 100644 --- a/.serena/memories/scripts/changelog.md +++ b/.serena/memories/scripts/changelog.md @@ -1,12 +1,13 @@ # Changelog helper -`scripts/changelog.py` holds the checks used by the `update-changelog` skill (extracted from the skill file so the skill stays workflow-only). Calls `scripts/gh.py` via subprocess; never call the GitHub API directly from this script. +`scripts/changelog.py` holds the checks used by the changelog flow. The rules and the step-by-step live in `mem:workflow/updating-changelog`; this memory documents the subcommands. Calls `scripts/gh.py` via subprocess; never call the GitHub API directly from this script. ## Subcommands - `check-merged ` (`--file`, `--stdin` accepted) — warn on any non-merged PR, exit 1 when found; run before writing changelog entries. +- `boom [--changes CHANGES.md --write]` — generate `### :boom: Breaking changes & Deprecations` from the closed, non-excluded milestone issues labelled `breaking change`, sorted by issue number. Each issue becomes a `#### ` block: content = body of the issue's last `## Breaking change note` comment (whole markdown, headings demoted to `#####` outside code fences; fallback = heading plus refs with a stderr warning), refs = issue link plus merged closing PRs on their own line, with `(by @author)` for `community contribution`. `--write` replaces only that subsection of the version (inserts it after the version header when missing, keeps the heading when there are no entries; idempotent). Print-only by default. Uses `gh.py comments`. - `cross-ref [--changes CHANGES.md]` — fetch milestone PRs with `--state all` once, report merged PRs missing from the version section plus CLOSED (unmerged) warnings; GHSA entries never appear here by design. -- `report [--changes CHANGES.md --output CHANGES-ISSUES.md]` — overwrite the anomaly report (milestone mismatches type A/B plus `:rocket:` gaps C/D); rule violations never belong in the report, they are fixed in CHANGES.md first. +- `report [--changes CHANGES.md --output CHANGES-ISSUES.md]` — overwrite the anomaly report: A/B milestone mismatches, E `:boom:` entry without the label, F `:rocket:` sync, G `:boom:` sync, plus the C/D `:rocket:` gaps (warnings). `:boom:`/`:rocket:` rule violations are fixed by regenerating or relabelling, not by editing entries in place. ## Conventions @@ -15,4 +16,5 @@ - Version-section lookup accepts an optional `(suffix)` after the version header (e.g. `(Unreleased)`); a missing section is an error for `cross-ref` and an empty section for `report`. - PR/issue reference parsing covers standard `[#N](.../pull/N)` / `[#N](.../issues/N)` links plus legacy `PR:[N]` / `[Github #N]` forms. - Shared parsing helpers (`extract_version_section`, `extract_subsection`, `collect_changelog_prs/issues`) live at module top so future subcommands reuse them instead of duplicating regexes. -- Report anomaly types: A/B milestone mismatches, E `:boom:` entry whose issue lacks the `breaking change` label (counted, entries preserved — human labels the issue or moves the entry); C/D are `:rocket:` gaps (warnings, never counted). +- Report anomaly types: A/B milestone mismatches, E `:boom:` entry whose issue lacks the `breaking change` label (counted, entries preserved — human labels the issue or moves the entry), G `breaking change` issue missing from `:boom:` (counted, same contract as F); C/D are `:rocket:` gaps (warnings, never counted). +- `issue_excluded` (closed, non-Task, non-Rejected, no `release blocker`/`no changelog` label) is module-level and shared by `report` and `boom` so the exclusions cannot drift. diff --git a/.serena/memories/scripts/gh.md b/.serena/memories/scripts/gh.md index 9c53875672..93fd4e37b8 100644 --- a/.serena/memories/scripts/gh.md +++ b/.serena/memories/scripts/gh.md @@ -11,6 +11,7 @@ repository via GraphQL and REST APIs through the authenticated `gh` CLI. - Comparing milestone issues against CHANGES.md to find missing entries. - Explicitly linking a GitHub issue to a pull request. - Listing or inspecting GitHub Security Advisories (GHSA). +- Fetching issue comments (the `## Breaking change note` markdown for the changelog). ## Prerequisites @@ -82,6 +83,16 @@ Fetch issue details by number in batches of 50 (same input styles as `prs`: numb **Output**: JSON array to stdout; progress to stderr. +### `comments` + +Fetch every comment of one or more issues via REST (paginated, 100 per page), oldest first. Same input styles as `issue`: numbers, `--file`, `--stdin`. + +```bash +python3 scripts/gh.py comments 11235 11236 +``` + +**Output fields per comment**: `issue`, `id`, `author`, `created_at`, `body`. JSON array to stdout; progress to stderr. `changelog.py boom` reads the `## Breaking change note` comment from this output. + ### `link-issue` Explicitly assign a GitHub issue to a pull request: diff --git a/.serena/memories/workflow/creating-issues.md b/.serena/memories/workflow/creating-issues.md index dbe55c9241..537e3e73ea 100644 --- a/.serena/memories/workflow/creating-issues.md +++ b/.serena/memories/workflow/creating-issues.md @@ -35,11 +35,35 @@ Command what should be built. Format: `[Imperative verb] [what] in/on [where]`. | Field | Rule | |-------|------| -| **Labels** | `community contribution` (PRs from non-core) · skip workflow labels (`backport candidate`, `team-qa`) · do **not** add `bug` or `enhancement` labels (use Issue Type instead) | +| **Labels** | `community contribution` (PRs from non-core) · `breaking change` when the change breaks compatibility (see below) · skip workflow labels (`backport candidate`, `team-qa`) · do **not** add `bug` or `enhancement` labels (use Issue Type instead) | | **Milestone** | Use the current or next planned milestone. Fetch available milestones: `gh api repos/penpot/penpot/milestones --jq '.[].title'`. If unsure, omit. | | **Project** | Always `Main` (project number 8). Use `--project "Main"` flag. | | **Issue Type** | See Issue Type section below. Cannot be set via `gh issue create` — use GraphQL after creation. | +## Breaking changes + +When the issue introduces a breaking change or a deprecation, it is the unit +`mem:workflow/updating-changelog` turns into a block under +`### :boom: Breaking changes & Deprecations`. The section is generated from +the issue (`scripts/changelog.py boom`), so the issue must carry both: + +1. The `breaking change` label. +2. A `## Breaking change note` **comment** (not a body section). Everything + after that marker line is copied to the changelog: paragraphs, bullet + lists, code blocks and headings are kept (headings are demoted to level 5 + in `CHANGES.md`). Write it for self-hosters: what changes, what they must + do, and what breaks if they do nothing. Keep it short: explain the + implications and link the runbook or docs for the detail, never paste the + runbook itself. Links must point to a long-lived branch (`staging` or + `main`), never to a feature branch. + +Post the note as a new comment. Do not add a breaking-change section to the +issue body and do not hand-write the changelog block. When the issue comes +from a PR, reuse the text of the PR's breaking-change section. + +The full changelog rules (sections, exclusions, `:rocket:`, pre-flight and +anomaly report) live in `mem:workflow/updating-changelog`. + ## Issue Body Template Write the body to a temp file to avoid shell quoting issues: @@ -237,6 +261,9 @@ Identify: description). Omit internal details. Use the templates above. - **Issue Type:** use the mapping table above (also handles `:bug:` / `:sparkles:` / `:tada:` title prefixes). +- **Breaking changes:** when the PR body has a breaking-change section, post + its user-facing text as a `## Breaking change note` comment on the issue and + add the `breaking change` label (see **Breaking changes** above). ### Create the issue @@ -303,6 +330,8 @@ rm -f /tmp/issue-body.md /tmp/pr-body.md label or the author is not a core team member, add the label to the issue. - **Don't put implementation details in the issue body** — the issue is for users, QA, and changelog readers. +- **Drop the PR's `no issue required` label** once the issue exists: it only + marks PRs that need no changelog unit, and it is stale from then on. ## Creating Issues from Draft Body diff --git a/.serena/memories/workflow/updating-changelog.md b/.serena/memories/workflow/updating-changelog.md new file mode 100644 index 0000000000..26ac7624af --- /dev/null +++ b/.serena/memories/workflow/updating-changelog.md @@ -0,0 +1,187 @@ +# Updating the changelog + +> Note about this memory: unlike other Serena memories (dense notes written for LLM agents, see `mem:memory-maintenance`), this document is also written for human release managers who need full context. That is why it uses plain language and examples. It is the canonical home of the changelog flow; the `update-changelog` skill is only an entry point that routes here. + +`CHANGES.md` (repository root) is the release notes file. Every entry comes from GitHub: a milestone issue, its merged PR, or a security advisory. It is never written from scratch by hand; the only manual parts are the security advisory entries and the wording of a description when the title is not enough. + +The commands live in `scripts/gh.py` (GitHub queries) and `scripts/changelog.py` (changelog checks). Their flags and internals are documented in `mem:scripts/gh` and `mem:scripts/changelog`. + +## 1. When to run it + +- Before a release, to fill the section of the version being released. +- When new issues are added to an existing milestone. +- When asked to audit or refresh `CHANGES.md`. + +Editing `CHANGES.md` happens only on explicit request or as part of the release process; see `mem:critical-info`. + +## 2. Sources of truth + +| Question | Source | +|---|---| +| Which issues belong to the version? | GitHub milestone | +| Which section? | Issue Type | +| Is it a breaking change? | `breaking change` label | +| Is it a highlight? | `release highlight` label | + +- **Issue Type decides the section**: `Bug` → `:bug:`, `Feature`/`Enhancement` → `:sparkles:`, `Task` is excluded (internal chores). Labels `bug`/`enhancement` and title emoji prefixes are often wrong or missing; do not use them. +- **Two labels override the Issue Type**: `breaking change` → an entry only under `:boom:`, never in its regular section; `release highlight` → an extra entry under `:rocket:` on top of its regular entry. +- **Only closed issues**. An open issue in the milestone is not generated, even if someone added a provisional entry by hand before the merge. +- **Excluded issues**: `no changelog`, `release blocker`, `Task` issue type, `Rejected` project status. +- **Excluded PRs**: a PR with `release blocker` or `no issue required` does not bring an entry of its own. +- **An issue with no milestone** belongs to another (probably private) project: it is neither changelog candidate nor anomaly. + +## 3. Layout of `CHANGES.md` + +The newest version goes at the top, right after the `# CHANGELOG` header. Inside a version, the section order is fixed: + +```markdown +# CHANGELOG + +## 2.19.0 (Unreleased) + +### :boom: Breaking changes & Deprecations + +#### + + + +[#ISSUE](https://github.com/penpot/penpot/issues/ISSUE) (PR: [#PR](https://github.com/penpot/penpot/pull/PR)) + +### :rocket: Epics and highlights + +- Add configurable keyboard shortcuts [#9924](https://github.com/penpot/penpot/issues/9924) (PR: [#10237](https://github.com/penpot/penpot/pull/10237)) + +### :bug: Bugs fixed + +- Fix [#ISSUE](https://github.com/penpot/penpot/issues/ISSUE) (PR: [#PR](https://github.com/penpot/penpot/pull/PR)) + +### :sparkles: New features & Enhancements + +- Add [#ISSUE](https://github.com/penpot/penpot/issues/ISSUE) (PR: [#PR](https://github.com/penpot/penpot/pull/PR)) +``` + +- `:boom:` first, then `:rocket:`, `:bug:`, `:sparkles:`. Other headings exist in old versions (`:heart:` community contributions); do not add them to new versions. +- Include a section only when it has entries. Historical versions may have an empty `:boom:` or `:rocket:` heading; `boom --write` keeps the `:boom:` heading even when there are no entries. +- One blank line between the last entry of a section and the next heading. +- No manual line wrapping: one line per bullet or paragraph, however long. + +## 4. Regular entries (`:bug:`, `:sparkles:`) + +The entry shape is: + +```markdown +- (by @github_username) [#ISSUE](https://github.com/penpot/penpot/issues/ISSUE) (PR: [#PR](https://github.com/penpot/penpot/pull/PR), [#PR2](https://github.com/penpot/penpot/pull/PR2)) +``` + +- **Description**: derived from the issue title, not the PR title. Strip emoji prefixes (`:bug:`, `:sparkles:`, `:tada:`) and write the user-facing behaviour in imperative mood. Examples: `Plugin API token methods fail with schema validation error` → `Fix Plugin API token methods failing with schema validation error`; `Custom uploaded font family names are not sanitized` → `Sanitize font family names on custom uploaded fonts`. +- **Issue is the primary link**: the issue is the changelog unit, the PR is the implementation. Multiple fix PRs go comma-separated inline. +- **Community attribution**: when the issue or its PR carries `community contribution`, add `(by @username)` before the issue link. Use the PR author, not the issue author. +- **Closed issue with no PR** in the milestone: link the issue only, no PR reference. +- **PR with no linked issue**: link a matching closed issue in the milestone when one exists; otherwise skip it, because the issue is the changelog unit. +- **False-positive PR links**: a PR may claim to close an unrelated issue (ancient PR, cross-project reference). If the titles are clearly unrelated or the PR predates the issue by years, treat it as a data glitch and ignore it. + +## 5. `:boom:`: breaking changes and deprecations + +Each breaking change is a `####` block, not a bullet. The title is the issue title without its emoji prefix; the content comes from a comment on the issue; the references go on their own line at the end. + +The **issue** must carry: + +1. The `breaking change` label. +2. A comment whose first line is `## Breaking change note`. Everything after that marker line is the changelog content: paragraphs, bullet lists, code blocks, tables, headings (they are demoted to `#####` on write), and so on. The marker itself is case-insensitive and accepts `#`, `##` or `###`; use `##`. + +Example comment: + +```markdown +## Breaking change note + +The Admin Console is now enabled by default and the `enable-admin-console` flag is no longer needed. + +Self-hosted instances must deploy the `penpot-admin-console` service and set `PENPOT_ADMIN_CONSOLE_URI` in frontend and backend (check the [upgrade guide](https://help.penpot.app/technical-guide/getting-started/docker/#upgrade-to-2.19)). +``` + +Generated block: + +```markdown +#### Enable the Admin Console by default + +The Admin Console is now enabled by default and the `enable-admin-console` flag is no longer needed. + +Self-hosted instances must deploy the `penpot-admin-console` service and set `PENPOT_ADMIN_CONSOLE_URI` in frontend and backend (check the [upgrade guide](https://help.penpot.app/technical-guide/getting-started/docker/#upgrade-to-2.19)). + +[#12807](https://github.com/penpot/penpot/issues/12807) (PR: [#12475](https://github.com/penpot/penpot/pull/12475)) +``` + +Rules for the note: + +- **Write it for admins**: what changes, what they must do, and what breaks if they do nothing. Do not paste the full runbook; explain the implications and link the runbook or docs. +- **Link a long-lived branch**, `staging` or `main`, never a feature branch (`issue-1234`). The link may 404 until the PR merges; that is expected. +- **Do not add issue or PR links**: the generator appends them. +- **One note per issue**. If several comments carry the marker, the generator uses the last one and warns. +- **No note**: the generator warns on stderr and emits only the heading and references; fix the issue, not `CHANGES.md`. +- **`####`, not `###`**: `###` is the level of the changelog sections. An entry at `###` would render as a sibling section and the scripts that read and rewrite `:boom:` would cut the section there. The note author never writes the `####`: the generator adds it. +- **Only merged closing PRs** are referenced; unmerged or superseded PRs are skipped. +- **Never hand-edit the section**. Generate it with `boom` (section 7). The note lives on GitHub; edit it there. + +## 6. `:rocket:`: epics and highlights + +- Lists exactly the milestone issues with the `release highlight` label, and nothing else. Product decides highlights by labelling issues; a regeneration cannot undo that choice. +- Every entry copies the text and references of the issue's regular entry and MUST carry both an issue link and a PR reference. +- If no issue is labelled, leave the section out; do not pick highlights yourself. +- Never edit `:rocket:` by hand: add or remove the label and regenerate. + +## 7. Flow + +1. **Confirm the version** (a semver string such as `2.19.0`) with the user when it is not obvious. +2. **Fetch the milestone issues**: `python3 scripts/gh.py issues "" --exclude "release blocker,no changelog"` (closed by default; add `--state all` only to inspect open issues). `gh.py` already drops `Task` and `Rejected`; pass `--include-tasks` / `--include-rejected` only to inspect them. To find what is missing from an existing section, add `--compare CHANGES.md`. +3. **Fetch PR details when needed**: `python3 scripts/gh.py prs ` (also `--milestone`). Use it for the PR author, labels and merge state. +4. **Categorize** by Issue Type and the two labels (section 2), and **build the descriptions** (section 4). +5. **Verify PR merge status** before writing: `python3 scripts/changelog.py check-merged `. Only merged PRs go in the changelog; if a closing PR was closed unmerged, look for the PR that superseded it. +6. **Reconcile the existing text** (section 8). +7. **Generate `:boom:`**: `python3 scripts/changelog.py boom ""` prints the section; add `--write` to replace it in `CHANGES.md`. The write replaces only that subsection, inserts it after the version header when missing, keeps the heading when there are no entries, and is idempotent. +8. **Generate `:rocket:`** from the `release highlight` label (section 6). +9. **Cross-check PRs**: `python3 scripts/changelog.py cross-ref ""`. It lists merged milestone PRs missing from the section and warns about closed (unmerged) PRs. GHSA entries never appear; that absence is expected. +10. **Generate the anomaly report**: `python3 scripts/changelog.py report ""`. It overwrites `CHANGES-ISSUES.md` with the current state (section 9). +11. **Security advisories (GHSA)**: add them by hand under `:bug:` as `- Fix ()`, with no issue or PR link. Do not fetch or verify the URL: it may be draft and return 404. Write the description in imperative mood from the advisory title. These entries are invisible to the automation; apply only the duplicate/backport check to them. + +## 8. Pre-flight: reconcile before writing + +Re-fetch the milestone (do not trust cached data) and fix these in `CHANGES.md` before adding the new section: + +1. **Duplicate across versions** → remove it from the current section; the earlier version wins for backports. +2. **Stale milestone assignment** (the issue moved out of this milestone) → remove the entry. +3. **Newly applied exclusion labels** (`no changelog`, `release blocker`) → remove the entry. +4. **Issue no longer closed, deleted, or now `Rejected`** → remove the entry. +5. **Unmerged or moved PR reference** → fix the reference or remove the entry. A PR merged in a different milestone is an anomaly (section 9); do not remove it silently. +6. **Issue Type changed to `Task`** → remove the entry. +7. **Breaking change out of sync** (a `breaking change` issue outside `:boom:`, or a `:boom:` entry without the label) → regenerate the section; a labelled issue has no regular entry. +8. **Highlight out of sync** (`:rocket:` entry without the label, or a labelled issue missing) → remove it or add it. +9. **Missing valid issues** (closed, non-excluded, referenced nowhere) → add them to their section. + +## 9. Anomaly report + +`report` classifies problems it cannot decide alone. The total counts A, B, E, F and G; C and D are warnings. + +- **A — issue in this milestone, referenced PR in another milestone (or none).** The claim "this issue is fixed by this PR in this milestone" is inconsistent. Fix the milestone on the issue or the PR, or remove the entry. +- **B — PR in this milestone whose closing issue is in another milestone.** Same fix. An issue with no milestone is not an anomaly (another, probably private, project). +- **E — `:boom:` entry whose issue lacks the `breaking change` label.** The entry stays in the changelog: either label the issue or move the entry to its regular section. +- **F — `:rocket:` out of sync with the `release highlight` label.** A rule violation: `:rocket:` must list exactly the labelled issues. Fix the label or re-run the flow. +- **G — `breaking change` issue missing from `:boom:`.** A rule violation: `:boom:` must list exactly the labelled issues. Regenerate the section or relabel. +- **C — released X.Y.0 version without a `:rocket:` section** (gap; patches never carry highlights). +- **D — `:rocket:` entry without issue AND PR references** (gap). + +Anything else is a rule violation, not a report item: fix it in the pre-flight and re-run. + +## 10. Pitfalls + +- **Open issues with provisional entries**: a PR author may add the entry before the merge. `boom` will not generate it until the issue closes; keep the hand-written block identical to what the generator would produce. +- **Labels, not emoji**: title prefixes are cosmetic and often missing; the section comes from Issue Type and the two labels. +- **Taiga references**: old entries link to Taiga. Resolve them to the GitHub issue via the description text or the PRs that mention the Taiga URL, then link issue and PR. +- **Long-lived links only**: runbooks and docs referenced from a note must point to `staging` or `main`, never to a feature branch that will be deleted. +- **Generated sections**: `:boom:` and `:rocket:` are the only label-generated parts. Never patch them by hand; edit the label or the note comment and re-run. + +## 11. Where the code is + +- `scripts/gh.py`: `issues`, `issue`, `comments`, `prs`, `link-issue`, `advisories`. Details: `mem:scripts/gh`. +- `scripts/changelog.py`: `check-merged`, `boom`, `cross-ref`, `report`. Details: `mem:scripts/changelog`. +- Tests: `scripts/test_gh.py`, `scripts/test_changelog.py`. +- The note contract for issue authors: `mem:workflow/creating-issues`, section **Breaking changes**. diff --git a/AGENTS.md b/AGENTS.md index c95fb06920..c39c02dcaf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,7 +10,8 @@ If the user pushes, treat that commit as final from the agent's side. - **Never edit `CHANGES.md` by hand** in commits or PRs. The changelog is generated from GitHub milestones during the release process; update it only - via the `update-changelog` skill flow or on explicit user request. + via `mem:workflow/updating-changelog` (the `update-changelog` skill flow) or + on explicit user request. - **Never pipe test output directly to filters** (`| head`, `| tail`, `| grep`, etc.). Always redirect to a file first: `command > /tmp/output.txt 2>&1`, then read/grep the file. This prevents hiding test failures. See `mem:testing` for details. diff --git a/scripts/changelog.py b/scripts/changelog.py index fbba459fbd..e964bacc18 100644 --- a/scripts/changelog.py +++ b/scripts/changelog.py @@ -1,18 +1,21 @@ #!/usr/bin/env python3 """ -changelog.py — Helper commands for the ``update-changelog`` skill. +changelog.py — Helper commands for the changelog flow. Uses ``scripts/gh.py`` (via subprocess) and the local ``CHANGES.md`` so the -skill file itself stays free of inline programs. +flow memory (`mem:workflow/updating-changelog`) stays free of inline programs. Subcommands: check-merged Verify that every given PR is merged (warns otherwise) + boom Generate the :boom: section from `breaking change` issues cross-ref Compare merged milestone PRs against the changelog section report Generate the anomaly report (CHANGES-ISSUES.md) Usage: python3 scripts/changelog.py check-merged 9179 9204 9311 cat prs.txt | python3 scripts/changelog.py check-merged --stdin + python3 scripts/changelog.py boom "2.19.0" + python3 scripts/changelog.py boom "2.19.0" --write python3 scripts/changelog.py cross-ref "2.16.0" python3 scripts/changelog.py report "2.16.0" python3 scripts/changelog.py report "2.16.0" --changes CHANGES.md --output CHANGES-ISSUES.md @@ -34,6 +37,21 @@ from pathlib import Path REPO = "penpot/penpot" GH_PY = Path(__file__).resolve().parent / "gh.py" +BOOM_HEADING = "### :boom: Breaking changes & Deprecations" +BREAKING_LABEL = "breaking change" +EXCLUDED_LABELS = {"release blocker", "no changelog"} + +# Marker line of the issue comment that carries the changelog text of a +# breaking change. Everything after the marker line is the entry content. +BREAKING_NOTE_RE = re.compile(r"(?im)^#{1,3}[ \t]+Breaking change note[ \t]*$") + +# Level used for the headings found inside a breaking change note. Entries +# themselves are level-4 headings, so note headings must live below them. +NOTE_HEADING = "#####" + +# Emoji prefixes used at the start of issue titles (`:bug:`, `:sparkles:`). +TITLE_PREFIX_RE = re.compile(r"^(?::[A-Za-z0-9_+-]+:\s*)+") + # ───────────────────────────────────────────── # Shared helpers @@ -125,6 +143,84 @@ def collect_changelog_issues(section: str) -> set[int]: return numbers +def issue_excluded(issue: dict | None) -> bool: + """Return True when an issue must stay out of the changelog. + + Shared by the ``report`` and ``boom`` subcommands so the exclusions + cannot drift apart. + """ + if not issue: + return True + if issue.get("state") != "CLOSED": + return True + if issue.get("issue_type") in {"Task"}: + return True + if issue.get("project_status") in {"Rejected"}: + return True + if EXCLUDED_LABELS & set(issue.get("labels", [])): + return True + return False + + +def extract_breaking_note(comment_body: str | None) -> str | None: + """Return the content that follows a ``## Breaking change note`` marker. + + The whole comment is used (paragraphs, lists, code blocks), not just its + first paragraph, so a breaking change can be as detailed as needed. + """ + if not comment_body: + return None + match = BREAKING_NOTE_RE.search(comment_body) + if not match: + return None + content = demote_headings(comment_body[match.end():].strip("\n")) + return content.strip() or None + + +def demote_headings(markdown: str) -> str: + """Push note headings below the entry heading, leaving code fences alone.""" + lines: list[str] = [] + in_fence = False + for line in markdown.splitlines(): + if line.lstrip().startswith(("```", "~~~")): + in_fence = not in_fence + lines.append(line) + continue + if not in_fence: + match = re.match(r"^#{1,4}([ \t]+.*)$", line) + if match: + line = NOTE_HEADING + match.group(1) + lines.append(line) + return "\n".join(lines) + + +def strip_title_prefix(title: str) -> str: + """Remove leading emoji shortcodes from a title.""" + return TITLE_PREFIX_RE.sub("", title).strip() + + +def closing_pr_numbers(issue: dict) -> list[int]: + """Return the sorted closing PR numbers of an issue.""" + return sorted(set(issue.get("closing_prs", []))) + + +def render_boom_entry( + issue: dict, + note: str | None, + pr_numbers: list[int], + author: str | None = None, +) -> str: + """Render one ``:boom:`` entry: title, note content and references.""" + title = strip_title_prefix(issue["title"]) + attribution = f"(by @{author}) " if author else "" + refs = attribution + issue_link(issue["number"]) + if pr_numbers: + refs += " (PR: " + ", ".join(pr_link(n) for n in pr_numbers) + ")" + if note: + return f"#### {title}\n\n{note}\n\n{refs}" + return f"#### {title}\n\n{refs}" + + # ───────────────────────────────────────────── # Subcommand: check-merged # ───────────────────────────────────────────── @@ -155,6 +251,153 @@ def cmd_check_merged(args: argparse.Namespace) -> None: print(f"OK: all {len(prs)} PRs are merged", file=sys.stderr) +# ───────────────────────────────────────────── +# Subcommand: boom +# ───────────────────────────────────────────── + + +def version_section_bounds(content: str, milestone: str) -> tuple[int, int] | None: + """Return the ``[start, end)`` offsets of a ``## `` section.""" + match = re.search( + rf"(?m)^## {re.escape(milestone)}(?:[ \t]*\([^)]*\))?[ \t]*$", content + ) + if not match: + return None + start = match.start() + next_heading = re.search(r"(?m)^## ", content[match.end():]) + end = match.end() + next_heading.start() if next_heading else len(content) + return start, end + + +def boom_body(entries: list[str]) -> str: + """Return the body text that follows the ``:boom:`` heading line.""" + if entries: + return "\n\n" + "\n\n".join(entries) + "\n\n" + return "\n\n" + + +def replace_boom_section(content: str, milestone: str, entries: list[str]) -> str: + """Return ``content`` with its ``:boom:`` subsection replaced. + + The subsection is inserted right after the version header when missing, + which keeps ``:boom:`` as the first section. + """ + bounds = version_section_bounds(content, milestone) + if bounds is None: + raise ValueError(f'no "## {milestone}" section found') + start, end = bounds + section = content[start:end] + heading = re.search(rf"(?m)^{re.escape(BOOM_HEADING)}[ \t]*$", section) + if heading: + body_start = start + heading.end() + next_heading = re.search(r"(?m)^### ", content[body_start:end]) + body_end = body_start + next_heading.start() if next_heading else end + return content[:body_start] + boom_body(entries) + content[body_end:] + header = re.match(r"(?m)^## [^\n]*\n", section) + if header is None: + raise ValueError(f'version "## {milestone}" header has no trailing newline') + insert_at = start + header.end() + block = "\n" + BOOM_HEADING + boom_body(entries)[:-1] + return content[:insert_at] + block + content[insert_at:] + + +def fetch_prs_by_issue(issues: list[dict]) -> dict[int, dict]: + """Fetch every closing PR of ``issues``, keyed by PR number.""" + numbers = sorted({n for issue in issues for n in issue.get("closing_prs", [])}) + if not numbers: + return {} + return {pr["number"]: pr for pr in fetch_prs_by_numbers(numbers)} + + +def fetch_breaking_notes(issues: list[dict]) -> dict[int, str]: + """Return the last ``## Breaking change note`` comment of each issue.""" + if not issues: + return {} + comments = json.loads( + run_gh_py("comments", *[str(issue["number"]) for issue in issues]) + ) + notes: dict[int, str] = {} + for comment in comments: + note = extract_breaking_note(comment.get("body")) + if note is None: + continue + number = comment["issue"] + if number in notes: + print( + f'WARNING: issue #{number} has several "## Breaking change note" ' + "comments; using the last one", + file=sys.stderr, + ) + notes[number] = note + return notes + + +def cmd_boom(args: argparse.Namespace) -> None: + """Generate the ``:boom:`` section from ``breaking change`` issues.""" + all_issues = json.loads(run_gh_py("issues", args.milestone, "--state", "all")) + labelled = sorted( + ( + issue + for issue in all_issues + if BREAKING_LABEL in issue.get("labels", []) + and not issue_excluded(issue) + ), + key=lambda issue: issue["number"], + ) + + notes: dict[int, str] = {} + prs: dict[int, dict] = {} + if labelled: + notes = fetch_breaking_notes(labelled) + prs = fetch_prs_by_issue(labelled) + + entries: list[str] = [] + for issue in labelled: + note = notes.get(issue["number"]) + if not note: + print( + f'WARNING: issue #{issue["number"]} has no "## Breaking change note" ' + "comment; the entry will have no description", + file=sys.stderr, + ) + merged = [ + prs[num] + for num in closing_pr_numbers(issue) + if num in prs and prs[num].get("state") == "MERGED" + ] + author = None + if "community contribution" in issue.get("labels", []): + authors = [pr["author"] for pr in merged if pr.get("author")] + author = authors[0] if authors else None + entries.append( + render_boom_entry(issue, note, [pr["number"] for pr in merged], author) + ) + + block = BOOM_HEADING + boom_body(entries) + print(block.rstrip("\n")) + + if not labelled: + print( + f"No closed issues labelled `{BREAKING_LABEL}` in {args.milestone}", + file=sys.stderr, + ) + + if args.write: + with open(args.changes) as f: + content = f.read() + try: + updated = replace_boom_section(content, args.milestone, entries) + except ValueError as err: + print(f"ERROR: {err}", file=sys.stderr) + sys.exit(1) + if updated == content: + print(f"{args.changes}: :boom: section already up to date", file=sys.stderr) + else: + with open(args.changes, "w") as f: + f.write(updated) + print(f"{args.changes}: :boom: section updated", file=sys.stderr) + + # ───────────────────────────────────────────── # Subcommand: cross-ref # ───────────────────────────────────────────── @@ -298,21 +541,6 @@ def cmd_report(args: argparse.Namespace) -> None: """Return the milestone title for an issue, or None if unassigned.""" return issue_milestone_cache.get(issue_num) - excluded_labels = {"release blocker", "no changelog"} - - def issue_excluded(issue: dict | None) -> bool: - if not issue: - return True - if issue.get("state") != "CLOSED": - return True - if issue.get("issue_type") in {"Task"}: - return True - if issue.get("project_status") in {"Rejected"}: - return True - if excluded_labels & set(issue.get("labels", [])): - return True - return False - # Type A: issue in milestone, referenced PR in different milestone or none anomalies_a = [] for issue_num in sorted(changelog_issues): @@ -361,13 +589,14 @@ def cmd_report(args: argparse.Namespace) -> None: # These stay in the changelog (they are preserved, not removed) — the # human decides whether to label the issue or move the entry elsewhere. anomalies_e = [] - boom_body = extract_subsection(section, r"### :boom:") - if boom_body: - for issue_num in sorted(collect_changelog_issues(boom_body)): + boom_section_body = extract_subsection(section, r"### :boom:") + boom_issues = collect_changelog_issues(boom_section_body) + if boom_section_body: + for issue_num in sorted(boom_issues): labels = get_issue_labels(issue_num) if labels is None: continue - if "breaking change" not in labels: + if BREAKING_LABEL not in labels: issue = issue_by_num.get(issue_num) or external_issues.get(issue_num) or {} anomalies_e.append( { @@ -406,6 +635,20 @@ def cmd_report(args: argparse.Namespace) -> None: } ) + # Type G: `breaking change` issue missing from :boom:. Same contract as + # type F for highlights: the label is the only source of the section. + anomalies_g = [] + for issue_num, issue in sorted(issue_by_num.items()): + if issue_excluded(issue) or issue_num in boom_issues: + continue + if BREAKING_LABEL in issue.get("labels", []): + anomalies_g.append( + { + "issue": issue_num, + "issue_title": issue.get("title", ""), + } + ) + # Type C: released X.Y.0 sections without a :rocket: subsection. # Patches (X.Y.Z with Z != 0) never carry :rocket: — only minors/majors. anomalies_c = [] @@ -459,13 +702,14 @@ def cmd_report(args: argparse.Namespace) -> None: ) f.write("---\n\n") - n_a, n_b, n_c, n_d, n_e, n_f = ( + n_a, n_b, n_c, n_d, n_e, n_f, n_g = ( len(anomalies_a), len(anomalies_b), len(anomalies_c), len(anomalies_d), len(anomalies_e), len(anomalies_f), + len(anomalies_g), ) f.write("## Summary\n\n") @@ -476,14 +720,15 @@ def cmd_report(args: argparse.Namespace) -> None: f"- **PR in {milestone}, closing issue in a different milestone:** {n_b}\n" ) f.write(f"- **:boom: entry without breaking change label:** {n_e}\n") + f.write(f"- **:boom: out of sync with breaking change label:** {n_g}\n") f.write(f"- **:rocket: out of sync with release highlight label:** {n_f}\n") - f.write(f"- **Total anomalies:** {n_a + n_b + n_e + n_f}\n") + f.write(f"- **Total anomalies:** {n_a + n_b + n_e + n_f + n_g}\n") f.write( f"- **Released X.Y.0 version missing :rocket: section (gap):** {n_c}\n" ) f.write(f"- **:rocket: entry without issue AND PR references (gap):** {n_d}\n\n") - if n_a or n_b or n_e or n_f: + if n_a or n_b or n_e or n_f or n_g: f.write("## Anomalies\n\n") f.write( "Types A and B are milestone mismatches between an issue in the changelog " @@ -494,8 +739,9 @@ def cmd_report(args: argparse.Namespace) -> None: "removing the misleading entry from the changelog.\n\n" "Type E entries stay in the changelog: either label the issue " "as `breaking change` or move the entry out of `:boom:`.\n\n" - "Type F is a rule violation: `:rocket:` must list exactly the " - "issues labelled `release highlight`. Fix the label on GitHub " + "Types F and G are rule violations: `:rocket:` must list exactly " + "the issues labelled `release highlight`, and `:boom:` exactly " + "the issues labelled `breaking change`. Fix the label on GitHub " "or re-run the workflow.\n\n" ) @@ -556,6 +802,18 @@ def cmd_report(args: argparse.Namespace) -> None: f"- ⚠️ {issue_link_title(e['issue'], e['issue_title'][:80])}: {e['reason']}\n" ) f.write("\n") + + if n_g: + f.write("\n### :boom: out of sync with breaking change label\n\n") + f.write( + "These milestone issues carry the `breaking change` label but " + "have no entry under `### :boom: Breaking changes & Deprecations`. " + "Regenerate the section (`changelog.py boom --write`) or add " + "the entry.\n\n" + ) + for e in anomalies_g: + f.write(f"- ⚠️ {issue_link_title(e['issue'], e['issue_title'][:80])}\n") + f.write("\n") else: f.write( "✅ No anomalies found. All (issue, PR) pairs in the changelog have aligned milestone assignments.\n\n" @@ -590,7 +848,7 @@ def cmd_report(args: argparse.Namespace) -> None: for d in anomalies_d: f.write(f"- **{d['version']}**: `{d['line']}`\n") f.write("\n") - elif not (n_a or n_b or n_e or n_f): + elif not (n_a or n_b or n_e or n_f or n_g): f.write( "✅ No highlight gaps found. All released X.Y.0 versions have properly referenced :rocket: entries.\n\n" ) @@ -618,7 +876,7 @@ def cmd_report(args: argparse.Namespace) -> None: def main() -> None: parser = argparse.ArgumentParser( - description="Helper commands for the update-changelog skill" + description="Helper commands for the changelog flow" ) sub = parser.add_subparsers(dest="command", required=True, title="subcommands") @@ -633,6 +891,16 @@ def main() -> None: help="Read PR numbers from stdin (one per line)") p_check.set_defaults(func=cmd_check_merged) + p_boom = sub.add_parser( + "boom", help="Generate the :boom: section from `breaking change` issues" + ) + p_boom.add_argument("milestone", help="Milestone title (e.g. '2.16.0')") + p_boom.add_argument("--changes", default="CHANGES.md", + help="Path to the changelog file (default: CHANGES.md)") + p_boom.add_argument("--write", action="store_true", + help="Replace the :boom: section in CHANGES.md in place") + p_boom.set_defaults(func=cmd_boom) + p_cross = sub.add_parser( "cross-ref", help="Compare merged milestone PRs against the changelog section", diff --git a/scripts/gh.py b/scripts/gh.py index c430fbe843..2ec3eb8dc3 100755 --- a/scripts/gh.py +++ b/scripts/gh.py @@ -6,6 +6,8 @@ Uses GitHub GraphQL and REST APIs via the authenticated ``gh`` CLI. Subcommands: issues List issues in a milestone (or unassigned with milestone=none) + issue Fetch issue details by number (batched) + comments Fetch every comment of one or more issues prs Fetch details for one or more PRs (by number or milestone) advisories List or inspect GitHub security advisories link-issue Explicitly link a GitHub issue to a pull request @@ -26,6 +28,7 @@ Usage: python3 scripts/gh.py prs --milestone "2.16.0" (default: state=merged) python3 scripts/gh.py prs --milestone "2.16.0" --state all python3 scripts/gh.py issue 11235 11236 + python3 scripts/gh.py comments 11235 11236 python3 scripts/gh.py advisories (list all advisories) python3 scripts/gh.py advisories --severity critical (filter by severity) python3 scripts/gh.py advisories GHSA-xvj6-fh9w-gjw7 (single advisory detail) @@ -888,6 +891,51 @@ def cmd_issue(args: argparse.Namespace) -> None: print(json.dumps(all_results, indent=2)) +# ───────────────────────────────────────────── +# Subcommand: comments +# ───────────────────────────────────────────── + +COMMENTS_PER_PAGE = 100 + + +def fetch_issue_comments(issue_number: int) -> list[dict]: + """Fetch every comment of an issue via REST, oldest first.""" + comments: list[dict] = [] + page = 1 + while True: + batch = run_gh_rest( + f"repos/{REPO}/issues/{issue_number}/comments" + f"?per_page={COMMENTS_PER_PAGE}&page={page}" + ) + for comment in batch: + comments.append({ + "issue": issue_number, + "id": comment["id"], + "author": (comment.get("user") or {}).get("login"), + "created_at": comment.get("created_at"), + "body": comment.get("body"), + }) + if len(batch) < COMMENTS_PER_PAGE: + return comments + page += 1 + + +def cmd_comments(args: argparse.Namespace) -> None: + """Handle the ``comments`` subcommand (REST, paginated per issue).""" + issue_numbers = read_numbers_from_args(args) + if not issue_numbers: + print("ERROR: no issue numbers provided (pass numbers, --file, or --stdin)", + file=sys.stderr) + sys.exit(1) + + all_comments: list[dict] = [] + for num in issue_numbers: + print(f"Fetching comments for issue #{num}...", file=sys.stderr) + all_comments.extend(fetch_issue_comments(num)) + + print(json.dumps(all_comments, indent=2)) + + # ───────────────────────────────────────────── # Subcommand: advisories # ───────────────────────────────────────────── @@ -1080,6 +1128,24 @@ def main() -> None: ) p_issue.set_defaults(func=cmd_issue) + # --- comments --- + p_comments = sub.add_parser( + "comments", help="Fetch every comment of one or more issues" + ) + p_comments.add_argument( + "numbers", type=int, nargs="*", + help="Issue numbers to fetch (space-separated)" + ) + p_comments.add_argument( + "--file", type=str, + help="File with one issue number per line" + ) + p_comments.add_argument( + "--stdin", action="store_true", + help="Read issue numbers from stdin (one per line)" + ) + p_comments.set_defaults(func=cmd_comments) + # --- advisories --- p_adv = sub.add_parser("advisories", help="List or inspect GitHub security advisories") p_adv.add_argument( diff --git a/scripts/test_changelog.py b/scripts/test_changelog.py new file mode 100644 index 0000000000..5d6a19946e --- /dev/null +++ b/scripts/test_changelog.py @@ -0,0 +1,383 @@ +#!/usr/bin/env python3 +"""Tests for scripts/changelog.py. + +Run with: + + python3 scripts/test_changelog.py +""" + +import argparse +import contextlib +import importlib.machinery +import importlib.util +import io +import json +import pathlib +import sys +import tempfile +import unittest +from unittest.mock import patch + + +# Loading scripts/changelog.py should not emit scripts/__pycache__/. +sys.dont_write_bytecode = True + +SCRIPT_PATH = pathlib.Path(__file__).resolve().parent / "changelog.py" + + +def load_changelog(): + """Load scripts/changelog.py as a module without running its CLI.""" + loader = importlib.machinery.SourceFileLoader("changelog_helper", str(SCRIPT_PATH)) + spec = importlib.util.spec_from_loader("changelog_helper", loader) + module = importlib.util.module_from_spec(spec) + loader.exec_module(module) + return module + + +ch = load_changelog() + +CHANGES = """# CHANGELOG + +## 2.19.0 (Unreleased) + +### :boom: Breaking changes & Deprecations + +- Old entry. + +### :rocket: Epics and highlights + +- Add highlights [#1](https://github.com/penpot/penpot/issues/1) +""" + + +class ExtractBreakingNoteTests(unittest.TestCase): + def test_uses_the_whole_comment_after_the_marker(self): + body = ( + "## Breaking change note\n\n" + "First paragraph.\n\n" + "- bullet one\n" + "- bullet two\n\n" + "```sql\nSELECT 1;\n```\n" + ) + self.assertEqual( + ch.extract_breaking_note(body), + "First paragraph.\n\n- bullet one\n- bullet two\n\n" + "```sql\nSELECT 1;\n```", + ) + + def test_accepts_any_heading_level_for_the_marker(self): + self.assertEqual( + ch.extract_breaking_note("# breaking change note\n\nText."), "Text." + ) + self.assertEqual( + ch.extract_breaking_note("### Breaking Change Note\n\nText."), "Text." + ) + + def test_returns_none_without_marker(self): + self.assertIsNone(ch.extract_breaking_note("### Description\n\ntext\n")) + self.assertIsNone(ch.extract_breaking_note("## Breaking change notes\n\ntext")) + self.assertIsNone(ch.extract_breaking_note(None)) + self.assertIsNone(ch.extract_breaking_note("")) + + def test_returns_none_for_a_marker_without_content(self): + self.assertIsNone(ch.extract_breaking_note("## Breaking change note\n")) + + +class DemoteHeadingsTests(unittest.TestCase): + def test_demotes_headings_below_the_entry_level(self): + markdown = "# One\n## Two\n### Three\n#### Four\n##### Five\n" + self.assertEqual( + ch.demote_headings(markdown), + "##### One\n##### Two\n##### Three\n##### Four\n##### Five", + ) + + def test_keeps_headings_inside_code_fences(self): + markdown = ( + "```bash\n# not a heading\n```\n" + "## Real heading\n" + "~~~\n### not a heading\n~~~\n" + ) + self.assertEqual( + ch.demote_headings(markdown), + "```bash\n# not a heading\n```\n" + "##### Real heading\n" + "~~~\n### not a heading\n~~~", + ) + + +class StripTitlePrefixTests(unittest.TestCase): + def test_strips_shortcodes(self): + self.assertEqual(ch.strip_title_prefix(":boom: Enable it"), "Enable it") + self.assertEqual(ch.strip_title_prefix(":bug: :tada: Fix it"), "Fix it") + self.assertEqual(ch.strip_title_prefix("Plain title"), "Plain title") + + +class IssueExcludedTests(unittest.TestCase): + def issue(self, **overrides): + base = { + "state": "CLOSED", + "issue_type": "Enhancement", + "project_status": "In progress", + "labels": [], + } + base.update(overrides) + return base + + def test_keeps_a_valid_closed_issue(self): + self.assertFalse(ch.issue_excluded(self.issue())) + + def test_excludes_open_task_rejected_and_labelled(self): + self.assertTrue(ch.issue_excluded(None)) + self.assertTrue(ch.issue_excluded(self.issue(state="OPEN"))) + self.assertTrue(ch.issue_excluded(self.issue(issue_type="Task"))) + self.assertTrue(ch.issue_excluded(self.issue(project_status="Rejected"))) + self.assertTrue(ch.issue_excluded(self.issue(labels=["release blocker"]))) + self.assertTrue(ch.issue_excluded(self.issue(labels=["no changelog"]))) + + +class RenderBoomEntryTests(unittest.TestCase): + def test_entry_is_a_heading_plus_note_plus_references(self): + issue = {"number": 7, "title": ":boom: Drop the flag", "labels": []} + self.assertEqual( + ch.render_boom_entry(issue, "One.\n\nTwo.", [8, 9]), + "#### Drop the flag\n\nOne.\n\nTwo.\n\n" + "[#7](https://github.com/penpot/penpot/issues/7)" + " (PR: [#8](https://github.com/penpot/penpot/pull/8)," + " [#9](https://github.com/penpot/penpot/pull/9))", + ) + + def test_entry_without_a_note_keeps_the_heading(self): + issue = {"number": 7, "title": "Drop the flag", "labels": []} + self.assertEqual( + ch.render_boom_entry(issue, None, []), + "#### Drop the flag\n\n[#7](https://github.com/penpot/penpot/issues/7)", + ) + + def test_community_attribution_goes_before_the_issue_link(self): + issue = {"number": 7, "title": "Drop the flag", "labels": []} + entry = ch.render_boom_entry(issue, "Text.", [8], "someone") + self.assertEqual( + entry, + "#### Drop the flag\n\nText.\n\n(by @someone)" + " [#7](https://github.com/penpot/penpot/issues/7)" + " (PR: [#8](https://github.com/penpot/penpot/pull/8))", + ) + + +class ReplaceBoomSectionTests(unittest.TestCase): + def test_replaces_the_existing_subsection(self): + entry = "#### New entry\n\nSome text.\n\n[#9](https://example.test/9)" + updated = ch.replace_boom_section(CHANGES, "2.19.0", [entry]) + self.assertIn( + "### :boom: Breaking changes & Deprecations\n\n" + f"{entry}\n\n### :rocket:", + updated, + ) + self.assertNotIn("Old entry.", updated) + + def test_replacement_is_idempotent(self): + entry = "#### New entry\n\nSome text.\n\n[#9](https://example.test/9)" + once = ch.replace_boom_section(CHANGES, "2.19.0", [entry]) + self.assertEqual(once, ch.replace_boom_section(once, "2.19.0", [entry])) + + def test_empty_section_keeps_the_heading(self): + updated = ch.replace_boom_section(CHANGES, "2.19.0", []) + self.assertIn( + "### :boom: Breaking changes & Deprecations\n\n### :rocket:", updated + ) + + def test_inserts_the_subsection_when_missing(self): + without = CHANGES.replace( + "### :boom: Breaking changes & Deprecations\n\n- Old entry.\n\n", "" + ) + updated = ch.replace_boom_section(without, "2.19.0", ["#### New entry"]) + self.assertIn( + "## 2.19.0 (Unreleased)\n\n" + "### :boom: Breaking changes & Deprecations\n\n#### New entry\n\n" + "### :rocket:", + updated, + ) + + def test_unknown_version_raises(self): + with self.assertRaises(ValueError): + ch.replace_boom_section(CHANGES, "9.9.9", ["#### Entry"]) + + +ISSUES = [ + { + "number": 21, + "title": ":boom: New default", + "state": "CLOSED", + "issue_type": "Enhancement", + "labels": ["breaking change"], + "closing_prs": [31], + "project_status": "Done", + }, + { + "number": 22, + "title": "No note", + "state": "CLOSED", + "issue_type": "Enhancement", + "labels": ["breaking change"], + "closing_prs": [], + "project_status": "Done", + }, + { + "number": 23, + "title": "Still open", + "state": "OPEN", + "issue_type": "Enhancement", + "labels": ["breaking change"], + "closing_prs": [], + "project_status": "In progress", + }, + { + "number": 24, + "title": "Not breaking", + "state": "CLOSED", + "issue_type": "Bug", + "labels": [], + "closing_prs": [], + "project_status": "Done", + }, +] + +COMMENTS = [ + {"issue": 21, "body": "## Breaking change note\n\nDeploy the new service.\n"}, + {"issue": 22, "body": "## Description\n\nNo marker here.\n"}, + {"issue": 21, "body": "## Breaking change note\n\nDeploy the new service.\n"}, +] + +PRS = [{"number": 31, "author": "someone", "labels": [], "state": "MERGED"}] + + +class CmdBoomTests(unittest.TestCase): + def setUp(self): + self.calls = [] + self.issues = ISSUES + self.comments = COMMENTS + self.prs = PRS + + def fake_run_gh_py(*args, input_text=None): + self.calls.append(args) + if args[0] == "issues": + return json.dumps(self.issues) + if args[0] == "comments": + return json.dumps(self.comments) + if args[0] == "prs": + return json.dumps(self.prs) + raise AssertionError(f"unexpected gh.py call: {args}") + + patcher = patch.object(ch, "run_gh_py", side_effect=fake_run_gh_py) + patcher.start() + self.addCleanup(patcher.stop) + + def run_boom(self, **kwargs): + args = argparse.Namespace( + milestone="2.19.0", + changes=CHANGES if "changes" not in kwargs else kwargs["changes"], + write=kwargs.get("write", False), + ) + out, err = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + ch.cmd_boom(args) + return out.getvalue(), err.getvalue() + + def test_renders_only_closed_breaking_issues(self): + out, err = self.run_boom() + expected = ( + "### :boom: Breaking changes & Deprecations\n\n" + "#### New default\n\n" + "Deploy the new service.\n\n" + "[#21](https://github.com/penpot/penpot/issues/21)" + " (PR: [#31](https://github.com/penpot/penpot/pull/31))\n\n" + "#### No note\n\n" + "[#22](https://github.com/penpot/penpot/issues/22)\n" + ) + self.assertEqual(out, expected) + self.assertIn("issue #22 has no", err) + self.assertIn("using the last one", err) + + def test_unmerged_closing_pr_is_not_referenced(self): + self.prs = [{"number": 31, "author": "someone", "state": "CLOSED"}] + out, _ = self.run_boom() + self.assertNotIn("pull/31", out) + self.assertIn("[#21](https://github.com/penpot/penpot/issues/21)", out) + + def test_no_labelled_issue_clears_the_section_on_write(self): + self.issues = [ISSUES[3]] + with tempfile.TemporaryDirectory() as tmp: + path = pathlib.Path(tmp) / "CHANGES.md" + path.write_text(CHANGES) + out, err = self.run_boom(changes=str(path), write=True) + updated = path.read_text() + self.assertEqual(out, "### :boom: Breaking changes & Deprecations\n") + self.assertNotIn("Old entry.", updated) + self.assertIn(":boom: section updated", err) + + def test_writes_the_section_in_place(self): + with tempfile.TemporaryDirectory() as tmp: + path = pathlib.Path(tmp) / "CHANGES.md" + path.write_text(CHANGES) + out, err = self.run_boom(changes=str(path), write=True) + updated = path.read_text() + self.assertIn("#### New default", updated) + self.assertNotIn("Old entry.", updated) + self.assertIn(":boom: section updated", err) + self.assertIn("Deploy the new service.", out) + + +class ReportTests(unittest.TestCase): + """Focused check of the `:boom:` sync anomaly (type G) in the report.""" + + def test_reports_breaking_issue_missing_from_boom(self): + issues = [ + { + "number": 1, + "title": "Unlabelled entry", + "state": "CLOSED", + "issue_type": "Enhancement", + "labels": [], + "closing_prs": [], + "project_status": "Done", + }, + { + "number": 2, + "title": "Labelled, not listed", + "state": "CLOSED", + "issue_type": "Enhancement", + "labels": ["breaking change"], + "closing_prs": [], + "project_status": "Done", + }, + ] + changes = ( + "# CHANGELOG\n\n## 2.19.0 (Unreleased)\n\n" + "### :boom: Breaking changes & Deprecations\n\n" + "#### Something changed\n\n" + "[#1](https://github.com/penpot/penpot/issues/1)\n" + ) + with tempfile.TemporaryDirectory() as tmp: + changes_path = pathlib.Path(tmp) / "CHANGES.md" + output_path = pathlib.Path(tmp) / "ISSUES.md" + changes_path.write_text(changes) + args = argparse.Namespace( + milestone="2.19.0", + changes=str(changes_path), + output=str(output_path), + ) + with patch.object(ch, "run_gh_py", return_value=json.dumps(issues)), \ + patch.object(ch, "fetch_milestone_prs", return_value=[]), \ + contextlib.redirect_stdout(io.StringIO()), \ + contextlib.redirect_stderr(io.StringIO()): + ch.cmd_report(args) + report = output_path.read_text() + + self.assertIn("**:boom: out of sync with breaking change label:** 1", report) + self.assertIn("### :boom: out of sync with breaking change label", report) + self.assertIn("issues/2", report) + self.assertIn("**:boom: entry without breaking change label:** 1", report) + + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/scripts/test_gh.py b/scripts/test_gh.py index 5fb6840a22..3255e8b495 100644 --- a/scripts/test_gh.py +++ b/scripts/test_gh.py @@ -110,5 +110,51 @@ class LinkIssueTests(unittest.TestCase): self.assertIn("link missing", stderr.getvalue()) +class CommentsTests(unittest.TestCase): + def test_paginates_until_a_short_page(self): + page_one = [ + {"id": i, "user": {"login": "u"}, "created_at": "t", "body": f"b{i}"} + for i in range(100) + ] + page_two = [{"id": 100, "user": None, "created_at": "t", "body": "last"}] + calls = [] + + def fake_rest(path): + calls.append(path) + return page_one if len(calls) == 1 else page_two + + with patch.object(gh, "run_gh_rest", side_effect=fake_rest): + comments = gh.fetch_issue_comments(7) + + self.assertEqual(len(comments), 101) + self.assertEqual( + comments[0], + {"issue": 7, "id": 0, "author": "u", "created_at": "t", "body": "b0"}, + ) + self.assertIsNone(comments[-1]["author"]) + self.assertIn("page=1", calls[0]) + self.assertIn("page=2", calls[1]) + + @patch.object(gh, "fetch_issue_comments") + def test_cmd_comments_outputs_every_issue(self, fetch_comments): + fetch_comments.side_effect = lambda num: [ + {"issue": num, "id": num, "author": "u", "created_at": "t", "body": "x"} + ] + args = types.SimpleNamespace(numbers=[1, 2], file=None, stdin=False) + stdout = io.StringIO() + + with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(io.StringIO()): + gh.cmd_comments(args) + + self.assertEqual([c["issue"] for c in json.loads(stdout.getvalue())], [1, 2]) + + def test_cmd_comments_requires_numbers(self): + args = types.SimpleNamespace(numbers=[], file=None, stdin=False) + with contextlib.redirect_stderr(io.StringIO()): + with self.assertRaises(SystemExit) as error: + gh.cmd_comments(args) + self.assertEqual(error.exception.code, 1) + + if __name__ == "__main__": unittest.main(verbosity=2)